Skip to content

clean up tokens on deauthorize #3872

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

Merged
merged 2 commits into from
Apr 21, 2021
Merged

clean up tokens on deauthorize #3872

merged 2 commits into from
Apr 21, 2021

Conversation

AlexTugarev
Copy link
Member

@AlexTugarev AlexTugarev commented Apr 9, 2021

clean up tokens on deauthorize

fixes #3800

how to test

  1. authorize with two providers
  2. disconnect one
  3. verify (in the db 🤷🏻‍♂️) that tokens are (marked as) deleted

temporarily disable terms acceptance check in API

@AlexTugarev AlexTugarev changed the title at/bugfixes Misc Bugfixes Apr 9, 2021
@AlexTugarev AlexTugarev requested a review from jankeromnes April 9, 2021 15:46
@jankeromnes
Copy link
Contributor

jankeromnes commented Apr 9, 2021

Thanks Alex!

access_denied on OAuth

fixes #3801

how to test

  1. revoke permissions for the OAuth App of Gitpod preview environments
  2. try to login and decline
  3. verify to see an error instead of a hanging redirect (and timeout)

This is where I end up at (correct-looking /sorry URL, but since I'm not logged in, it shows the login screen instead):

Screenshot 2021-04-09 at 17 58 33

EDIT: Actually, why even attempt to redirect to /sorry? You could simply redirect to /login in that case (I wouldn't be surprised to see the login screen again if I decline the OAuth prompt).

@AlexTugarev
Copy link
Member Author

@jankeromnes, thanks for testing!

First of, it tries to redirect is an error page (and not to login) and that's what's expected.

An error page that is only rendered if logged in is problematic at least. :-(

In this particular case though the flow and the resulting redirect is happening in a separate browser tab, so the target for redirect should not be the sorry page. Indeed it should redirect to /login-error (symmetrical to /login-success) and propagate to the still open login page, which will close the extra tab and, that's the plan, render a message.

One step at a time.

@AlexTugarev AlexTugarev marked this pull request as draft April 13, 2021 09:03
@AlexTugarev AlexTugarev marked this pull request as ready for review April 13, 2021 09:30
@AlexTugarev
Copy link
Member Author

@jankeromnes, I extracted #3915 from here. I turns out there is a tail to that problem.

@AlexTugarev
Copy link
Member Author

AlexTugarev commented Apr 13, 2021

/werft run

👍 started the job as gitpod-build-at-bugfixes.3

@AlexTugarev
Copy link
Member Author

@csweichel, could you have a look at the changes left in this PR, please?

@AlexTugarev AlexTugarev changed the title Misc Bugfixes clean up tokens on deauthorize Apr 19, 2021
@AlexTugarev AlexTugarev added this to the April 2021 milestone Apr 19, 2021
Copy link
Contributor

@csweichel csweichel left a comment

Choose a reason for hiding this comment

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

Change LGTM

@AlexTugarev AlexTugarev merged commit c174dbf into main Apr 21, 2021
@AlexTugarev AlexTugarev deleted the at/bugfixes branch April 21, 2021 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up token on deauthorize
4 participants