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

fix: use configured redirect URL for external providers #1114

Merged
merged 1 commit into from
May 23, 2023

Conversation

hf
Copy link
Contributor

@hf hf commented May 22, 2023

With #999 custom domains were introduced, however for OAuth, the redirect URLs should in fact be the ones specified in the config and not ones interpreted from the X-Forwarded-Host header.

@hf hf requested a review from a team as a code owner May 22, 2023 16:41
@hf hf force-pushed the hf/use-configured-oauth-redirect branch from 7da4d05 to de5e6df Compare May 22, 2023 16:45
Copy link
Member

@kangmingtay kangmingtay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm i added this initially because i thought that the config doesn't matter. It can be any custom domain the developer decides to use and it's up to them to set it within the oauth app.

@hf
Copy link
Contributor Author

hf commented May 23, 2023

Hmm i added this initially because i thought that the config doesn't matter. It can be any custom domain the developer decides to use and it's up to them to set it within the oauth app.

Yes but this setting configures what redirect URL is being sent to the OAuth provider, which is controlled by the GOTRUE_EXTERNAL_<PROVIDER>_REDIRECT_URL config. Had this URL been a relative path, then it would have made sense to use the hostname. But it appears that it's not, at least not on Supabase projects.

@kangmingtay
Copy link
Member

ah good point, yeah that will result in breaking behaviour if the user attempts to sign in with oauth from a custom domain

@J0
Copy link
Contributor

J0 commented May 23, 2023

Ah okay, so with custom domains the user would still configure GOTRUE_EXTERNAL_<PROVIDER>_REDIRECT_URL except that they would need to adjust the URL as per their custom domain?

Had this URL been a relative path, then it would have made sense to use the hostname

Oh what would be the difference here? By relative path would it mean that the user specifying the suffix (e.g. /<url> ...) ?

@kangmingtay
Copy link
Member

@J0 yeah the user would have to be able to update the GOTRUE_EXTERNAL_<PROVIDER>_REDIRECT_URL and those URLs should correspond to the callback URLs used in the oauth app

@kangmingtay
Copy link
Member

i initially made the assumption that a developer would add all their associated custom domains to the oauth app but that might not be the case and will result in an error - maybe we should expose the GOTRUE_EXTERNAL_<PROVIDER>_REDIRECT_URL config in the dashboard? I'm also not sure if it makes sense for every oauth provider to have their own GOTRUE_EXTERNAL_<PROVIDER>_REDIRECT_URL config or maybe we can refactor this to be shared across all the providers.

@J0
Copy link
Contributor

J0 commented May 23, 2023

Oh okay I'm still slightly lost - could I trouble you for a quick check - would the breaking behaviour result from the following:

  1. User configures GOTRUE_EXTERNAL_<PROVIDER>_REDIRECT_URL to https://<ref>.supabase.co/callback hoping to redirect to Supabase Auth
  2. User signs in with OAuth on custom domains page http://mysite.com/
  3. Supabase Auth calls getExternalHost and redirects to mysite.com/callback when the intended callback was https://<ref>.supabase.co/callback ?

@kangmingtay
Copy link
Member

@J0 yup that's right

@hf hf merged commit 42bb1e0 into master May 23, 2023
@hf hf deleted the hf/use-configured-oauth-redirect branch May 23, 2023 12:48
@github-actions
Copy link
Contributor

🎉 This PR is included in version 2.69.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
With supabase#999 custom domains were introduced, however for OAuth, the
redirect URLs should in fact be the ones specified in the config and not
ones interpreted from the `X-Forwarded-Host` header.
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
With supabase#999 custom domains were introduced, however for OAuth, the
redirect URLs should in fact be the ones specified in the config and not
ones interpreted from the `X-Forwarded-Host` header.
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 15, 2024
With supabase#999 custom domains were introduced, however for OAuth, the
redirect URLs should in fact be the ones specified in the config and not
ones interpreted from the `X-Forwarded-Host` header.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants