fix(clerk-js): Fix FAPI initiated redirect flow for OAuth2 IDP flow with email_link verification#2677
Merged
mzhong9723 merged 1 commit intomainfrom Jan 31, 2024
Conversation
🦋 Changeset detectedLatest commit: 575478b The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
mzhong9723
requested review from
a team and
octoper
and removed request for
a team and
octoper
panteliselef
approved these changes
Jan 30, 2024
Contributor
panteliselef
left a comment
panteliselef
left a comment
There was a problem hiding this comment.
@mzhong9723 Seems like this was reported for v4 components and we should probably need to backport this PR to v4.
I will help with that once the PR is merged to main. Could we just add a description in the changeset ?
Comment on lines
+1
to
+3
| --- | ||
| --- |
Contributor
There was a problem hiding this comment.
Maybe something like this ?
Suggested change
| --- | |
| --- | |
| --- | |
| '@clerk/clerk-js': patch | |
| --- | |
| Fix redirect flow for OAuth2 IDP flow with email_link verification. |
SokratisVidros
suggested changes
Jan 30, 2024
| @@ -1615,9 +1615,11 @@ export class Clerk implements ClerkInterface { | |||
| const userSignedIn = this.session; | |||
| const signInUrl = this.#environment?.displayConfig.signInUrl; | |||
Contributor
There was a problem hiding this comment.
In ClerkJS both signInUrl and signUpUrl can be injected via ClerkOptions. These ClerkOptions come from CLERK_SIGN_IN_URL in frameworks such as Next.js. The display config settings are left as a fallback and are downplayed in v5.
So for this method, we should pick signInUrl and signUpUrl from ClerkOptions first.
Member
Author
There was a problem hiding this comment.
@SokratisVidros updated both sign in and sign up urls to use ClerkOptions first and display config urls as a second fallback option
mzhong9723
force-pushed
the
mz/core-1567-oauth-provider-email-link-verification
branch
from
37851d3 to
6574d50
Compare
mzhong9723
force-pushed
the
mz/core-1567-oauth-provider-email-link-verification
branch
from
6574d50 to
f5e6a4f
Compare
|
|
||
| const userSignedIn = this.session; | ||
| const signInUrl = this.#environment?.displayConfig.signInUrl; | ||
| const signInUrl = this.#options.signInUrl ? this.#options.signInUrl : this.#environment?.displayConfig.signInUrl; |
Contributor
There was a problem hiding this comment.
Suggested change
| const signInUrl = this.#options.signInUrl ? this.#options.signInUrl : this.#environment?.displayConfig.signInUrl; | |
| const signInUrl = this.#options.signInUrl || this.#environment?.displayConfig.signInUrl; |
SokratisVidros
approved these changes
Jan 31, 2024
…ith email_link verification For the OAuth2 IDP flow, we should not redirect when the referrer is the sign up url. This way, the second factor can be completed after a first factor like email verification link. Previously, users were being redirected back to FAPI /oauth/authorize prematurely. This change ensures that users will not be redirected as such and have the chance to complete their second factor verification, like phone code.
mzhong9723
force-pushed
the
mz/core-1567-oauth-provider-email-link-verification
branch
from
f5e6a4f to
575478b
Compare
mzhong9723
deleted the
mz/core-1567-oauth-provider-email-link-verification
branch
panteliselef
pushed a commit
that referenced
this pull request
Feb 1, 2024
…ith email_link verification (#2677) For the OAuth2 IDP flow, we should not redirect when the referrer is the sign up url. This way, the second factor can be completed after a first factor like email verification link. Previously, users were being redirected back to FAPI /oauth/authorize prematurely. This change ensures that users will not be redirected as such and have the chance to complete their second factor verification, like phone code. (cherry picked from commit 7503376)
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Feb 1, 2024
…ith email_link verification (#2677) (#2702) For the OAuth2 IDP flow, we should not redirect when the referrer is the sign up url. This way, the second factor can be completed after a first factor like email verification link. Previously, users were being redirected back to FAPI /oauth/authorize prematurely. This change ensures that users will not be redirected as such and have the chance to complete their second factor verification, like phone code. (cherry picked from commit 7503376) Co-authored-by: Mary Zhong <mary@clerk.dev>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Description
For the OAuth2 IDP flow, we should not redirect when the referrer is the sign up url. This way, a second factor can be completed after a first factor like email verification link. Previously, users were being redirected back to
FAPI /oauth/authorizeprematurely. This change ensures that users will not be redirected as such and have the chance to complete their second factor verification, like phone code.Fixes CORE-1567
Video of fix below (there's a jump in the middle because I cut out retrieving the email link from my inbox):
https://github.com/clerk/javascript/assets/27433835/63e4ec43-1107-4c90-8ee5-f9196be7210c
Previously, the user would be redirected back to the sign in page without being able to complete sign up.
Checklist
npm testruns as expected.npm run buildruns as expected.Type of change
Packages affected
@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/clerk-expo@clerk/fastifygatsby-plugin-clerk@clerk/localizations@clerk/nextjs@clerk/clerk-react@clerk/remix@clerk/clerk-sdk-node@clerk/shared@clerk/themes@clerk/typesbuild/tooling/chore