-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
WIP: Gentle re-authentication modal/popup. #560
Conversation
b3951f2
to
367f751
Compare
367f751
to
6a00dc8
Compare
9464334
to
b22f85e
Compare
c3bf297
to
9f563bb
Compare
86978e4
to
3d8f54c
Compare
@tech4him1 thought this was WIP for some reason, will review next week. In the meantime, can you rebase? Some guy pushed a bunch of changes and broke everything 😛 |
3d8f54c
to
bba1361
Compare
Deploy preview ready! Built with commit 1607dd3 |
Deploy preview ready! Built with commit 1607dd3 |
9937289
to
8f69f0e
Compare
We were clearing out any old user state when sending an authentication request (or failure), which makes sense, but we have to keep the old user object while re-authenticating or it will ruin our app's current state. This doesn't change our current (initial) auth at all.
8f69f0e
to
1607dd3
Compare
@tech4him1 I thought I remembered there being complications with this, but not seeing any mention in the PR. Can you confirm one last time whether this is ready for review? |
@erquhart This was originally intended for GitHub, because it seemed that the token was sometimes being timing out if you logged out of GitHub.com in a different tab. That couldn't be consistently reproduced, however (and shouldn't logically be happening), so the PR was put on hold. We did decide to leave it open for use in any other new backend that may have token timeouts, such as BitBucket (and likely GitLab). |
@tech4him1 thoughts on this one? |
@erquhart The current use for this would be adding implicit authentication for BitBucket. We could just close the PR and reference it from the issue if you want? |
Awesome, thanks @tech4him1 |
- Summary
We need to have a way to gently re-authenticate the user, especially for backends for which the OAuth token times out or is otherwise invalidated.
With this implementation, we don't re-run the user's action at all, we just re-authenticate on API failure. For example, if the user is saving a post and we get a 401, we re-authenticate the user, but then they have to click the "Save" button again. Do you guys think that is clear enough?
This is not currently implemented at all, but it is needed (will be used) by both #517 and #525.
#559 needs merged before this can be finished.DoneWe can also use theDoneDialog
from #554 for this as well.This PR fixes #569.No longer applicable.- Test plan:
Current authentication works correctly.
- Description for the changelog
Add gentle re-auth modal for backends (not currently implemented).
- A picture of a cute animal (not mandatory but encouraged)