reactioncommerce / example-storefront

Example Storefront is Reaction Commerce’s headless ecommerce storefront - Next.js, GraphQL, React. Built using Apollo Client and the commerce-focused React UI components provided in the Storefront Component Library (reactioncommerce/reaction-component-library). It connects with Reaction backend with the GraphQL API.
https://www.mailchimp.com/developer/open-commerce
Apache License 2.0
604 stars 288 forks source link

Enable access token refreshing #350

Closed impactmass closed 5 years ago

impactmass commented 6 years ago

Type: major

Details

In adding refresh token setup, it's better NOT to do refresh tokens approach, and instead trigger a “silent” re-authentication on expiration. Considering that:

Notes for implementation

aldeed commented 5 years ago

@impactmass @ticean I'm not sure what's wrong with storing the refresh token in local storage on the client and attempting a refresh whenever a 401 response comes. This is how every SPWA I've ever worked on does it, not to mention the Apollo Client docs and most articles I've read.

This can be combined with silent re-auth for the best experience.

Otherwise won't we have to full refresh the page when a 401 comes? That won't be good UX.

impactmass commented 5 years ago

I’ve seen from a number of OAuth related docs, that refresh tokens should not be exposed to the client. Below are a few:

Refresh Tokens are subject to strict storage requirements to ensure that they are not leaked. A Single Page Application (normally implementing Implicit Grant) should not under any circumstances get a Refresh Token. The reason for that is the sensitivity of this piece of information. You can think of it as user credentials, since a Refresh Token allows a user to remain authenticated essentially forever. Therefore you cannot have this information in a browser, it must be stored securely.

... from Auth0 docs

This from the OAuth spec also applies:

Because refresh tokens are typically long-lasting credentials used to request additional access tokens, the refresh token is bound to the client to which it was issued. If the client type is confidential or the client was issued client credentials (or assigned other authentication requirements), the client MUST authenticate with the authorization server as described in Section 3.2.1.

I’m open to the direction you suggested, but it'll be good to clear out the concerns listed.

I’m trying to find docs doing this via the browser. I saw this about access tokens. I haven't seen for refresh token yet. Can you link me directly to any?

aldeed commented 5 years ago

@impactmass @ticean After reading more about the current state of recommendations (seems to keep changing), I think you are correct that silent auth is recommended now, and it IS possible to do it from the client for a single page app.

In https://auth0.com/docs/api-auth/tutorials/silent-authentication:

In the case of single-page applications, the checkSession method from auth0.js can be used to perform silent authentication within a hidden iframe, which results in no UX disruption at all.

More details here: https://auth0.com/docs/libraries/auth0js/v9#using-checksession-to-acquire-new-tokens

In particular, note:

The actual redirect to /authorize happens inside an iframe, so it will not reload your application or redirect away from it.

However, the browser must have third-party cookies enabled. Otherwise, checkSession() is unable to access the current user's session (making it impossible to obtain a new token without displaying anything to the user). The same will happen if users have Safari's ITP enabled.

There is also a similar but maybe slightly better/newer option mentioned here: https://connect2id.com/products/server/docs/api/check-session

Also note this Safari issue here: https://auth0.com/docs/api-auth/token-renewal-in-safari

I do think we should be treating starter kit as a single page app. It has the NextJS Node server only for purposes of prerendering, so whether we are initiating auth from Node or the browser, it should always be done in an SPA-compatible browser way. Obviously an iframe re-auth would only work in the browser, though, so we'll need a slightly different silent re-auth flow if the access token is expired when doing the initial render on the server.

See also:

impactmass commented 5 years ago

thanks @aldeed, these are great links. I'll be going through them and comparing side-by-side with Hydra's implementation of the spec.

impactmass commented 5 years ago

Update: Hydra's token refresh endpoint requires the client credentials for authentication, based on the auth code flow spec.

This blocks for performing the refresh in the browser (even with the hidden iframe approach). But if we have an API on the Express server setup to make this Hydra refresh call, that may solve this(? I'm about testing if that can work).

POST /oauth2/token HTTP/1.1
Host: localhost:4444
Authorization: Basic bXktaWQ6
Accept: application/json
Content-Type: application/x-www-form-urlencoded
Cache-Control: no-cache

grant_type=refresh_token&refresh_token=9F0BRebxWerAiisrzdlDrknzFccJ7FEvBSlelaygZnWY.EV2ul08rjoKuLMAhTpS6-s2A5bmS_ExzlqfBO-A17QY&response_type=id_token+token

PS: List of Hydra API & doc: https://www.ory.sh/docs/api/hydra/

impactmass commented 5 years ago

We decided that being a SPA, we'll be making the call to Hydra from the app in the browser. We'll provide the needed params for the Hydra API call via cookies.

Next steps: In implementing this, I discovered that enabling CORS on Hydra isn't working as expected.

After trying different combination of options from the docs + online, and even trying the recommended options from the core Hydra maintainer:

- CORS_ALLOWED_ORIGINS=*
- CORS_ALLOWED_METHODS=GET,POST,PUT,DELETE,PATCH
- CORS_ALLOWED_CREDENTIALS=true
- CORS_ALLOWED_HEADERS=Authorization,Accept,Accept-Language,Content-Language,Content-Type

See relevant links on Hydra CORS here and here.

To walk-around Hydra CORS and prevent delay on this task, we'll setup a CORS enabled API in our Identity Provider server (i.e the hydra plugin in the Meteor app) that will talk to Hydra. I've opened this issue reactioncommerce/reaction-hydra#9 to revisit this Hydra issue.

impactmass commented 5 years ago

Status update:

Note about task 2 I ran into a number of issues with the app's apollo-client (link) setup. First, the documented steps of setting up error-handling required using an object key (forward) that was missing in our error link's callback. Later found that this was a version issue. So an eventual PR for this issue will update apollo-link-error from 1.0.9 to 1.1.1.

Another issue was with using promises in the error link. Just resolved that now too.

Note about task 3 While server-rendering the app, we currently write token info into a cookie as a base-64 encoded string. When a new token is received after refresh, we'll update the cookie with a new encoded string. This will require some changes to our earlier setup (I'm just looking into that now to be sure what needs to change).

spencern commented 5 years ago

@impactmass do you have a WIP PR we can link to this issue?

impactmass commented 5 years ago

@spencern It's coming up. I'm working on it