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

(expo/use-sso) Add redirectUrl parameter to startSSOFlow; remove expo partials #2005

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

LauraBeatris
Copy link
Member

@LauraBeatris LauraBeatris commented Feb 8, 2025

🔎 Previews:

What does this solve?

Adds a new redirectUrl parameter to startSSOFlow

Checklist

  • I have clicked on "Files changed" and performed a thorough self-review
  • I have added the "deploy-preview" label and added the preview link(s) to this PR description
  • All existing checks pass

Copy link

github-actions bot commented Feb 8, 2025

Hey, here’s your docs preview: https://clerk.com/docs/pr/2005

@LauraBeatris LauraBeatris marked this pull request as ready for review February 12, 2025 20:35
@LauraBeatris LauraBeatris requested a review from a team as a code owner February 12, 2025 20:35
@LauraBeatris
Copy link
Member Author

Blocked until clerk/javascript#5127 gets merged

@alexisintech alexisintech changed the title Add redirectUrl parameter to startSSOFlow (expo/use-sso) Add redirectUrl parameter to startSSOFlow; remove expo partials Feb 12, 2025
const { createdSessionId, setActive, signIn, signUp } = await startSSOFlow({
strategy: 'enterprise_sso',
identifier: email,
redirectUrl: '/', // The URL to redirect to after successful authentication
Copy link
Member Author

@LauraBeatris LauraBeatris Feb 13, 2025

Choose a reason for hiding this comment

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

This is optional, by default it's sso-callback, I think we should update the useSSO reference

It shouldn't be a raw string - it's on the type level, but you'd call some Expo API to generate the URL based on the platform: Native would use a deep link scheme vs a web URL

@LauraBeatris
Copy link
Member Author

The commit to add redirectUrl parameter in the startSSOFlow reference got lost - not sure if it was intentional or due to a wrong rebase @alexisintech - I added it back here 78b67f4

@LauraBeatris LauraBeatris force-pushed the laura/use-sso-updates-1 branch 2 times, most recently from 5218396 to fbf01cb Compare February 13, 2025 13:12
@LauraBeatris
Copy link
Member Author

Blocked until clerk/javascript#5127 gets merged

Changed got release, therefore this PR is unblocked

@alexisintech
Copy link
Member

@LauraBeatris I'm not sure how to do linking with expo. I did redirectUrl: 'myapp://hello' as myapp is whats in my config. but after authenticating, it redirects me to the homepage and ignores the redirectUrl

docs/custom-flows/enterprise-connections.mdx Outdated Show resolved Hide resolved
docs/custom-flows/enterprise-connections.mdx Outdated Show resolved Hide resolved
docs/custom-flows/oauth-connections.mdx Outdated Show resolved Hide resolved
docs/custom-flows/oauth-connections.mdx Outdated Show resolved Hide resolved
- `redirectUrl?`
- `string`

The full URL or path to redirect to after the SSO flow is complete. If not specified, defaults to `sso-callback` path.
Copy link
Member

Choose a reason for hiding this comment

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

If not specified, defaults to sso-callback path.

But I don't see where the /sso-callback is used - users don't have to set anything up for this page, so if this /sso-callback path is actually hit behind the scenes, is this even worth mentioning?

Copy link
Member Author

Choose a reason for hiding this comment

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

If deep linking is working - in this case, for Android since iOS currently has a bug from our tests so far (expo/expo#34187)

And redirectUrl is not defined, then we'll navigate to sso-callback, and if they don't have this route configured on the app, it'll lead to a 404 - reason why I think it's worth mentioning the default

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants