-
-
Notifications
You must be signed in to change notification settings - Fork 393
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
feat: Expo Auth without setting AUTH_URL. #1054
Merged
juliusmarminge
merged 9 commits into
t3-oss:11-02-feat_expo_auth
from
Wundero:11-02-feat_expo_auth
Jun 9, 2024
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
4072aa9
feat: expo-auth without auth_url env var
Wundero f422d5d
Merge branch 't3-oss:11-02-feat_expo_auth' into 11-02-feat_expo_auth
Wundero 6f46135
Fix session cookie matching
Wundero 2fcbe9b
feat: Restore old CSRF checks in non-dev environments
Wundero 711e756
chore: Documenting some decisions with comments
Wundero fea6cfa
Use node env instead of vercel-specific env var
Wundero a78f862
Update readme to describe oauth changes
Wundero 8e903ed
Fix redirectTo being missing and enforce home nav since it was showin…
Wundero e31a9d1
Disallow backwards navigation upon auth change
Wundero File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why do we need to rewrite if we disable csrf in dev?
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.
Because the generated callback URLs point to
localhost:3000
no matter what. NextJS sets thenextUrl
to the deployment URL, which is alwayslocalhost
on development, but we need it to be the IP of the machine, i.e.192.168.x.y
so that it can properly route the request to the oauth provider and back. This function rewrites the URL to be the machine's IP, so that we can force nextauth to use that instead of the preset localhost.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.
Having looked through authjs's source code, the callback url is generated by
const baseUrl = env.AUTH_URL ?? request.url
(pseudocode), so because we don't setAUTH_URL
, we need to ensurerequest.url
is set to the proper value.EDIT: Reference code:
Auth
function (where request is passed by this app): https://github.com/nextauthjs/next-auth/blob/main/packages/core/src/index.ts#L101AuthInternal
function (request.url is retrieved) https://github.com/nextauthjs/next-auth/blob/main/packages/core/src/lib/index.ts#L24init
function call (handles events, we care about callback stuff): https://github.com/nextauthjs/next-auth/blob/main/packages/core/src/lib/init.ts#L68C35-L68C49parseProviders
call, which generates the callback URL: https://github.com/nextauthjs/next-auth/blob/main/packages/core/src/lib/utils/providers.ts#L29The code above uses the request URL's origin (and the request URL origin gets overwritten with
AUTH_URL
if it is set) to generate the callback URL, so it must be set to the desired return IP for expo to work.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.
So this means we have to set the ip at the oauth provider? Hmm...
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.
Just modifying the request URL seems to be sufficient. In my local testing, this has worked great both with and without the auth proxy server.
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.
Aight I'll try and test it out asap