Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving flow when user denies Bitbucket access to Che #19620

Merged
merged 6 commits into from
Apr 28, 2021

Conversation

xbaran4
Copy link
Contributor

@xbaran4 xbaran4 commented Apr 19, 2021

Signed-off-by: xbaran4 pbaran@redhat.com

What does this PR do?

After user denies authorization of che with bitbucket they are redirected to redirect_after_login URI with error query parameter.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

Improve workflow when user denies Bitbucket access for Che #19274

How to test this PR?

  1. Deploy che-server with image quay.io/pbaran/che-server:bitbucket
  2. Setup che with bitbucket (this will help: https://github.com/skabashnyuk/gitsrv/tree/main/bitbucket
    https://www.eclipse.org/che/docs/che-7/administration-guide/configuring-authorization/#proc_configuring-bitbucket-server-oauth1_che)
  3. Deny che access on bitbucket authorization page.

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Signed-off-by: xbaran4 <pbaran@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Apr 19, 2021

Can one of the admins verify this patch?

@che-bot che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/task Internal things, technical debt, and to-do tasks to be performed. labels Apr 19, 2021
Signed-off-by: xbaran4 <pbaran@redhat.com>
Signed-off-by: xbaran4 <pbaran@redhat.com>
@xbaran4 xbaran4 marked this pull request as ready for review April 22, 2021 10:49
@skabashnyuk
Copy link
Contributor

what about unit tests?

@skabashnyuk
Copy link
Contributor

Please merge the latest master to fix the build check.

@xbaran4
Copy link
Contributor Author

xbaran4 commented Apr 27, 2021

If there are not any apparent issues, I believe this PR can be merged in order to help with dashboard reaction implementation. I will provide tests for OAuthAuthenticationService in a subsequent PR. Testing this PR is described above. It was also tested by me in the current state.

Copy link
Contributor

@skabashnyuk skabashnyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for me to merge the code and add tests in separate pr

@skabashnyuk skabashnyuk merged commit f162778 into eclipse-che:master Apr 28, 2021
@eclipse-che eclipse-che deleted a comment from che-bot Apr 28, 2021
@eclipse-che eclipse-che deleted a comment from che-bot Apr 28, 2021
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Apr 28, 2021
@che-bot che-bot added this to the 7.30 milestone Apr 28, 2021
@xbaran4 xbaran4 deleted the bitbucket_flow branch May 19, 2021 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants