Uh oh!
There was an error while loading. Please reload this page.
feat(clerk-js,types): Add support for modal SSO sign-ins#7026
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@clerk/clerk-js': minor | ||
| '@clerk/types': minor | ||
| --- | ||
| [Experimental] Add support for modal SSO sign-ins to new APIs | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -79,3 +79,74 @@ export async function _authenticateWithPopup( | ||
| navigateCallback, | ||
| ); | ||
| } | ||
| /** | ||
| * Creates new redirect and callback URLs that point to the `/popup-callback` route on Account Portal. These URLs will | ||
| * be used by FAPI to redirect after the OAuth flow completes, and will result in a message being sent to the parent | ||
| * window. | ||
| */ | ||
| export function wrapWithPopupRoutes( | ||
| client: Clerk, | ||
| { | ||
| redirectCallbackUrl, | ||
| redirectUrl, | ||
| }: { | ||
| /** | ||
| * The route to navigate to if a session was not created. | ||
| */ | ||
| redirectCallbackUrl: string; | ||
| /** | ||
| * The route to navigate to if a session was created. | ||
| */ | ||
| redirectUrl: string; | ||
| }, | ||
| ): { redirectCallbackUrl: string; redirectUrl: string } { | ||
| const accountPortalHost = buildAccountsBaseUrl(client.frontendApi); | ||
| // We set the force_redirect_url query parameter to ensure that the user is redirected to the correct page even | ||
| // in situations like a modal transfer flow. | ||
| const r = new URL(redirectCallbackUrl); | ||
| r.searchParams.set('sign_in_force_redirect_url', redirectUrl); | ||
| r.searchParams.set('sign_up_force_redirect_url', redirectUrl); | ||
| // All URLs are decorated with the dev browser token in development mode since we're moving between AP and the app. | ||
| const redirectUrlWithForceRedirectUrl = client.buildUrlWithAuth(r.toString()); | ||
| const popupRedirectUrlComplete = client.buildUrlWithAuth(`${accountPortalHost}/popup-callback`); | ||
| const popupRedirectUrl = client.buildUrlWithAuth( | ||
| `${accountPortalHost}/popup-callback?return_url=${encodeURIComponent(redirectUrlWithForceRedirectUrl)}`, | ||
| ); | ||
| return { redirectCallbackUrl: popupRedirectUrl, redirectUrl: popupRedirectUrlComplete }; | ||
| } | ||
dstaley marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| export function _futureAuthenticateWithPopup( | ||
| client: Clerk, | ||
| params: { popup: { location: { href: string } }; externalVerificationRedirectURL: URL }, | ||
| ): Promise<void> { | ||
| return new Promise((resolve, reject) => { | ||
| if (!client.client || !params.popup) { | ||
| reject(); | ||
| return; | ||
| } | ||
| const messageHandler = async (event: MessageEvent) => { | ||
| if (event.origin !== buildAccountsBaseUrl(client.frontendApi)) { | ||
| return; | ||
| } | ||
| // The OAuth flow was successful, and we received a message with either a session or a return URL. | ||
| if (event.data.session || event.data.return_url) { | ||
| window.removeEventListener('message', messageHandler); | ||
| resolve(); | ||
| } else { | ||
| reject(); | ||
| } | ||
| }; | ||
| // Listen for messages from the popup window. | ||
| window.addEventListener('message', messageHandler); | ||
| // Navigate the popup window to the external verification redirect URL, which kicks off the OAuth flow. | ||
| params.popup.location.href = params.externalVerificationRedirectURL.toString(); | ||
| }); | ||
| } | ||
Comment on lines
+122
to
+152
Contributor There was a problem hiding this comment. Fix Promise rejection/control-flow and ensure listener cleanup.
Apply: export function _futureAuthenticateWithPopup(
client: Clerk,
params: { popup: { location: { href: string } }; externalVerificationRedirectURL: URL },
): Promise<void> {
return new Promise((resolve, reject) => {
- if (!client.client || !params.popup) {- reject();- }+ if (!client.client || !params.popup) {+ return reject(new Error('Popup auth requires an initialized client and a popup window.'));+ }- const messageHandler = async (event: MessageEvent) => {- if (event.origin !== buildAccountsBaseUrl(client.frontendApi)) {+ const apOrigin = buildAccountsBaseUrl(client.frontendApi);+ const cleanup = () => window.removeEventListener('message', messageHandler);+ const messageHandler = (event: MessageEvent) => {+ if (event.origin !== apOrigin) {
return;
}
// The OAuth flow was successful, and we received a message with either a session or a return URL.
if (event.data.session || event.data.return_url) {
- window.removeEventListener('message', messageHandler);- resolve();+ cleanup();+ return resolve();
} else {
- reject();+ cleanup();+ return reject(new Error('Popup auth did not return a session or return_url.'));
}
};
// Listen for messages from the popup window.
window.addEventListener('message', messageHandler);
// Navigate the popup window to the external verification redirect URL, which kicks off the OAuth flow.
params.popup.location.href = params.externalVerificationRedirectURL.toString();
});
}Optional: add a timeout to reject if no message is received within N seconds. 🤖 Prompt for AI Agents | ||
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
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.
🧩 Analysis chain
Semver: this includes breaking type changes; minor bump is insufficient for @clerk/types.
SignInFutureSSOParamsremovesflowand drops'saml'fromstrategy. That breaks user code compiling against@clerk/types. Please bump @clerk/types as a major (and consider whether @clerk/clerk-js also needs a major if runtime behavior changes), or gate these under experimental types with a pre-release tag.🏁 Script executed:
Length of output: 27
🏁 Script executed:
Length of output: 27
🏁 Script executed:
Length of output: 0
🏁 Script executed:
Length of output: 27
🏁 Script executed:
Length of output: 3074
🏁 Script executed:
Length of output: 170
🏁 Script executed:
Length of output: 170
🏁 Script executed:
Length of output: 9531
🏁 Script executed:
Length of output: 117
🏁 Script executed:
Length of output: 664
🏁 Script executed:
Length of output: 1466
🏁 Script executed:
Length of output: 1677
🏁 Script executed:
Length of output: 22
Breaking type changes require major version bump for @clerk/types; current minor classification is incorrect.
The git diff confirms the breaking changes claimed:
SignInFutureSSOParamsremoves the optionalflowproperty and removes'saml'from thestrategyunion. Both changes break user code at compile-time or runtime. OnlyenterpriseConnectionIdis marked@experimental; the breaking removals are not gated. Per semver, these require either a major version bump or experimental/pre-release tagging on the breaking properties themselves.