-
Notifications
You must be signed in to change notification settings - Fork 971
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
getCurrentUser given both decoded & access tokens #779
Conversation
I also just realized that if this gets merged, the generated https://github.com/redwoodjs/redwood/tree/main/packages/cli/src/commands/generate/auth/templates |
packages/cli/src/commands/generate/auth/templates/firebase.auth.js.template
Outdated
Show resolved
Hide resolved
This is great, thanks for this @dthyresson ! |
be1afec
to
2affd4d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey!
I've left some feedback, this is starting to look really good - let me know if there are things that aren't clear and I'll be happy to jump on a call to clarify them.
Thanks @peterp appreciate that! Starting to get a feel for how you and the project are designing and implementing the framework -- it has to be designed for extensibility from the start (well from an early iteration) when solving a problem... not just solving the problem if you know what I mean. I think I have a handle on your suggestions but will reach out otherwise. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great, I just want a few stylistic changes - Feel free to merge them in batches, or by yourself. After that I'll run it locally and merge it.
I might change a few things post deployment - but over all it's way better than it was before, thanks for much for this!
@peterp All request style changes are in. Thanks for noting them -- I'll remember for future.
Thanks.
I could see revisiting some naming: auth vs authentication vs authorization. Client vs Provider. Decoder vs Authorization. Decoded vs token vs "bearer token in header". I imagine when roles and permissions are added, these terms might get more used and possible source of confusion.
Glad to help. |
@peter one last item -- but this could be a separate issue/task is to update the auth generators/templates: https://github.com/redwoodjs/redwood/tree/main/packages/cli/src/commands/generate/auth/templates
to use new
and even add a new Auth0 template that has an example of fetching the full userProfile from API
|
@@ -147,7 +147,7 @@ export const handler = async ({ provider, force }) => { | |||
const tasks = new Listr( | |||
[ | |||
{ | |||
title: 'Adding required packages...', | |||
title: 'Adding required web packages...', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now can add packages to either the web or api side, since Auth0's api side benefits from having the its AuthenticationClient
to implement the getCurrentUser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peterp ok with ☝️ to add both web/api packages?
@@ -0,0 +1,101 @@ | |||
// Define what you want `currentUser` to return throughout your app. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be way too much but included as an example of how to fetch userProfile now that have the access token.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I had problems testing my changes to the cli.
While I built the cli package via yarn build:js && yarn build:clean-dist
and I see the new/updated files:
and when I do a
yarn rwt copy ../../redwoodjs/redwood
from there and I see the files move
when i try to test with a
yarn rw g auth auth0
the files in dist seem to revert to the old files and I get a file not found error for the new auth0 template
✔ Adding required web packages...
✔ Adding required api packages...
✔ Installing packages...
✖ Generating auth lib...
→ ENOENT: no such file or directory, open '<MYDIRPATH>/projects/redwoodjs/auth0-auth-test-app
…
Adding auth config to web...
Adding auth config to GraphQL API...
One more thing...
ENOENT: no such file or directory, open <MYDIRPATH>/projects/redwoodjs/auth0-auth-test-app/node_modules/@redwoodjs/cli/dist/commands/generate/auth/templates/auth0.auth.js.template'
So, unfortunately these template changes haven't been tested with a new rw app. Sorry. I'll keep trying to figure out why the files revert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strange, I'm having the same issue
TODO
|
Overview and Why
When implementing a custom getCurrentUser() in a Redwood app's
auth.js
, there's a need to access not just the decoded JWT, but the access token itself.This is especially helpful when implementing authentication with Auth0 because an access token (https://auth0.com/docs/tokens/concepts/access-tokens) is required by Auth0 in order to fetch the userProfile from the
/userinfo
endpoint (https://auth0.com/docs/api/authentication#get-user-info).One may then persist the profile information (such as email) in a
User
or update thecurrentUser
in the context to present email, name, or other profile info subsequently on theweb
side.Changes Made
This PR makes the following changes:
api/src/auth/authHeaders.ts
, moves parsing the access token from the event headers (ie, the "Bearer ") into its own function to be used both independently and also when decoding the token. This is used when creating the context handler inapi/src/functions/graphql.ts
.api/src/functions/graphql.ts
, thecreateContextHandler
now providesdecoded
token as well as optionalauthType
and accesstoken
togetCurrentUser()
.Example Use
The following is an example of how having the access token is helpful when using Auth0 as an authentication provider.
yarn workspace api add auth0
decoded
token provides the user's id in the JWT'ssub
(aka subject) claim.token
is the access token required by Auth0 to call the/userinfo
endpointsub
claim to see if a User exists in the db; if not ...Now in a page (or better yet, a User component) one can present the user profile:
Considerations
{ token, authType }
should be optional so as to not break existinggetCurrentUser()
implementations?