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

3rd party cookies disabled can't renew a token - auth code overhaul required. #76

Open
camallen opened this issue Jan 30, 2018 · 8 comments

Comments

@camallen
Copy link
Contributor

Browsers with third party cookies disabled won't be able to renew a token via the iframe after #75.

After #75 is deployed, these users will get a token from hash fragment of the page redirection of the implicit oauth flow and then it'll be set for re-use via local storage. The implementation in #75 will use a timeout function to clear the expiring tokens and attempt to get a new one via the iframe.
This will fail and those users may be left with an inconsistent UI state depending on how the calling code detects the underlying logged-in state change, most likely signalling logged in status but appearing to the API as unauthenticated.

The above will be poor UX on our behalf and will cause issues with tracking seen subjects and recents in the API among other things. Also noting that these users will be logged in to talk as it is on the zooniverse.org domain. The users can make notes on the subjects they classified but probably won't have that subject tracked in their seen subjects and may be served them again (this riles users no end and wastes effort).

This library needs an overhaul in how it manages and re-uses tokens and this code can be shared between the supported authentication flows we currently use:

  1. Auth.js uses the password credentials flow and gets a refresh token
  2. Oauth.js uses the implicit flow and does not get a refresh token

We should split out the token retrieval to different strategies patterns and consolidate the token storage and management code where we can. The different token flows above will require different token renewal strategies as well:

  1. flows with a refresh token can get a new access token directly
  2. flows without a refresh token will have to re-authenticate or rely on an existing session to gain a new token.

The updated code will also provide management hooks that the including app can use to configure and customize the authentication flows as well. This will allow events like failing to refresh a token to bubble up to the calling app to better manage the experience of our users on sites that use implicit flows.

@eatyourgreens
Copy link
Contributor

A volunteer could also refresh their token (without being aware of it) by making a comment on Talk, or an API request on any other page on zooniverse.org, just before the two hours is up. In that case, the shared Talk client and API client HTTP Auth headers would update to use the new token, so their SW classifications would still be recorded as them. The HTTP headers in that case would then be out of sync with browser storage on shakespearesworld.org, which would still have the expired token.

@eatyourgreens
Copy link
Contributor

eatyourgreens commented Feb 5, 2018

Thinking some more about what I just said, I'm not sure what happens to the client request headers in this scenario:

  1. Volunteer logs in on shakepearesworld.org: sets apiClient.headers.
  2. Visits Talk on zooniverse.org and makes a comment, which refreshes their token: updates apiClient.headers.
  3. Returns to classify shakespearesworld.org, which reloads and re-initialises the Angular app, loading the old token from local storage and overwrites apiClient.headers?

I've assumed the above 3 steps happen the same tab, but it's more likely that someone will classify in one tab and comment in another tab, in which case they have two different instances of apiClient I guess, and don't share the headers.

@camallen
Copy link
Contributor Author

camallen commented Feb 6, 2018

The code in 1 & 2 run on different domains and thus have 2 different API Client instances...right?

@eatyourgreens
Copy link
Contributor

D'oh! Yes you are right. I'm confusing myself because the Talk client and API client share settings on zooniverse.org.

@eatyourgreens
Copy link
Contributor

I think I broke token refresh in 2.9 because it now deletes the current token before trying to refresh. See 49f3aae.

@camallen
Copy link
Contributor Author

camallen commented Feb 14, 2018

How so? On refresh in the iframe, it relies on the panoptes session cookie to get a new implicit oauth token. It doesn't rely on the existing token to get a new one.

@eatyourgreens
Copy link
Contributor

Bad wording on my part. I was thinking you might want to use the old token to save unfinished work, before it expires. However, my commit deletes the token and clears the client headers.

@camallen
Copy link
Contributor Author

Gotcha, yeah good catch, any api requests between deleteBearerToken and renewal will be authenticated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants