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

RFC: Remove authToken from authContext #1608

Closed
dac09 opened this issue Jan 7, 2021 · 4 comments
Closed

RFC: Remove authToken from authContext #1608

dac09 opened this issue Jan 7, 2021 · 4 comments

Comments

@dac09
Copy link
Contributor

dac09 commented Jan 7, 2021

Hey, Creating RFC to track discussion.

So I've been looking into solving #1576 and its many duplicates, and am close to a solution (pending testing on various providers etc.) - but as part of this process I have 3 changes that I suggest making:

1. Remove the authToken from useAuth and AuthContext
Breaking change - technically
The main issue is that authTokens expire, and the state isn't refreshed automatically at the moment by redwood. By removing this value, we prevent users from accidentally using a stale token, but instead forcing them to use getToken or client.getToken

2. Introduce apollo client middleware, introduce new getFreshToken into authClient shape
Transparent change, non-breaking
I've already done this bit locally, but it's purpose is to always get a fresh token and perform a refresh if required. Now for firebase, we don't need to implement anything new here, but for future proofing we may want to introduce getFreshToken for authClients, incase certain providers have extra logic required to refresh the token before returning.

3. Remove fetchConfigProvider
Transparent change, non-breaking
I'm not a 100% sure about this yet, but using the new AuthMiddleware might make the FetchConfigProvider redundant. All of the functionality will be maintained, its just that we'll remove this extra Context layer that may not be required anymore

@dac09
Copy link
Contributor Author

dac09 commented Jan 7, 2021

On second thought, for (2) is there ever a need to get a stale token? Maybe just use the getToken to always return a fresh token.

@peterp
Copy link
Contributor

peterp commented Jan 7, 2021

This sounds totally reasonable to me.

1. Remove the authToken from useAuth and AuthContext

We could set it as deprecated for a few releases and point people to the new direction.

** Introduce apollo client middleware, introduce new getFreshToken**

I love fresh things as much as the next person, but maybe something like getAccessToken is good enough?

3. Remove fetchConfigProvider

I would like to propose that you keep the FetchConfigProvider file around, but remove it from RedwoodProvider. I'm not sure that something like Apollo's middleware exists for all GraphQL providers. So, this would be a valid option for people using a custom client until we know more.

@Tobbe
Copy link
Member

Tobbe commented Jan 7, 2021

For firebase auth in particular this seems relevant: #1555 (comment)

@dac09
Copy link
Contributor Author

dac09 commented Feb 8, 2021

Implemented. Tokens must now be retrieved using the function getToken in useAuth

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

3 participants