Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/red-windows-train.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/shared': patch
---

Fix OAuth application flows to handle redirect to `redirect_url` from `/oauth/authorize/continue`
4 changes: 3 additions & 1 deletion packages/shared/src/internal/clerk-js/__tests__/url.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,7 @@ describe('isRedirectForFAPIInitiatedFlow(frontendAp: string, redirectUrl: string
['clerk.foo.bar-53.lcl.dev', 'foo', false],
['clerk.foo.bar-53.lcl.dev', 'https://clerk.foo.bar-53.lcl.dev/deadbeef.', false],
['clerk.foo.bar-53.lcl.dev', 'https://clerk.foo.bar-53.lcl.dev/oauth/authorize', true],
['clerk.foo.bar-53.lcl.dev', 'https://clerk.foo.bar-53.lcl.dev/oauth/authorize/continue', true],
['clerk.foo.bar-53.lcl.dev', 'https://clerk.foo.bar-53.lcl.dev/v1/verify', true],
['clerk.foo.bar-53.lcl.dev', 'https://clerk.foo.bar-53.lcl.dev/v1/tickets/accept', true],
['clerk.foo.bar-53.lcl.dev', 'https://clerk.foo.bar-53.lcl.dev/oauth/authorize-with-immediate-redirect', true],
Expand All @@ -518,9 +519,10 @@ describe('requiresUserInput(redirectUrl: string)', () => {
['foo', false],
['https://clerk.foo.bar-53.lcl.dev/deadbeef.', false],
['https://clerk.foo.bar-53.lcl.dev/oauth/authorize', true],
['https://clerk.foo.bar-53.lcl.dev/oauth/authorize/continue', true],
['https://clerk.foo.bar-53.lcl.dev/v1/verify', false],
['https://clerk.foo.bar-53.lcl.dev/v1/tickets/accept', false],
['https://clerk.foo.bar-53.lcl.dev/oauth/authorize-with-immediate-redirect', false],
['https://clerk.foo.bar-53.lcl.dev/oauth/authorize-with-immediate-redirect', true],
['https://clerk.foo.bar-53.lcl.dev/oauth/end_session', false],
['https://google.com', false],
['https://google.com/v1/verify', false],
Expand Down
11 changes: 10 additions & 1 deletion packages/shared/src/internal/clerk-js/redirectUrls.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { applyFunctionToObj, filterProps, removeUndefined } from '../../object';
import type { ClerkOptions, RedirectOptions } from '../../types';
import { camelToSnake } from '../../underscore';
import { isAllowedRedirect, relativeToAbsoluteUrl } from './url';
import { isAllowedRedirect, isFAPIInitiatedFlowPath, relativeToAbsoluteUrl } from './url';

type ComponentMode = 'modal' | 'mounted';

Expand Down Expand Up @@ -99,6 +99,15 @@ export class RedirectUrls {
const forceKey = `${prefix}ForceRedirectUrl` as const;
const fallbackKey = `${prefix}FallbackRedirectUrl` as const;

// FAPI-initiated flow redirect URLs (e.g. /oauth/authorize/continue) must
// take highest priority to ensure the IDP authorization flow completes.
// Without this, a configured signInForceRedirectUrl would override the
// redirect_url needed to resume the OAuth IDP flow after sign-in.
const fapiRedirectUrl = this.fromSearchParams.redirectUrl;
if (fapiRedirectUrl && isFAPIInitiatedFlowPath(fapiRedirectUrl)) {
return fapiRedirectUrl;
}
Comment on lines +102 to +109
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

FAPI priority check only validates pathname, not host — risk of matching non-FAPI URLs.

isFAPIInitiatedFlowPath (in url.ts, lines 427–435) checks only parsed.pathname against the allowlist, unlike the sibling isRedirectForFAPIInitiatedFlow which also verifies frontendApi === url.host. Any redirect_url whose path happens to be /oauth/authorize/continue, /oauth/authorize, /v1/verify, /oauth/end_session, etc. — even on the customer's own origin or another allowed origin — will now override signInForceRedirectUrl. This silently breaks the documented force-redirect contract for those paths and, depending on how the returned URL is later consumed, could also be abused as an open-redirect-ish escape from force URLs (the value still passes isAllowedRedirect, but force URL is a stronger configured invariant).

Suggest gating on the frontend API host as well, mirroring isRedirectForFAPIInitiatedFlow:

Proposed fix
-    // FAPI-initiated flow redirect URLs (e.g. /oauth/authorize/continue) must
-    // take highest priority to ensure the IDP authorization flow completes.
-    // Without this, a configured signInForceRedirectUrl would override the
-    // redirect_url needed to resume the OAuth IDP flow after sign-in.
-    const fapiRedirectUrl = this.fromSearchParams.redirectUrl;
-    if (fapiRedirectUrl && isFAPIInitiatedFlowPath(fapiRedirectUrl)) {
-      return fapiRedirectUrl;
-    }
+    // FAPI-initiated flow redirect URLs (e.g. /oauth/authorize/continue) must
+    // take highest priority to ensure the IDP authorization flow completes.
+    const fapiRedirectUrl = this.fromSearchParams.redirectUrl;
+    if (
+      fapiRedirectUrl &&
+      isRedirectForFAPIInitiatedFlow(this.options.frontendApi ?? '', fapiRedirectUrl)
+    ) {
+      return fapiRedirectUrl;
+    }

Also worth adding a unit test covering: (a) a non-FAPI-host URL whose path matches the allowlist should NOT bypass signInForceRedirectUrl, and (b) a genuine FAPI continuation URL should bypass it.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// FAPI-initiated flow redirect URLs (e.g. /oauth/authorize/continue) must
// take highest priority to ensure the IDP authorization flow completes.
// Without this, a configured signInForceRedirectUrl would override the
// redirect_url needed to resume the OAuth IDP flow after sign-in.
const fapiRedirectUrl = this.fromSearchParams.redirectUrl;
if (fapiRedirectUrl && isFAPIInitiatedFlowPath(fapiRedirectUrl)) {
return fapiRedirectUrl;
}
// FAPI-initiated flow redirect URLs (e.g. /oauth/authorize/continue) must
// take highest priority to ensure the IDP authorization flow completes.
const fapiRedirectUrl = this.fromSearchParams.redirectUrl;
if (
fapiRedirectUrl &&
isRedirectForFAPIInitiatedFlow(this.options.frontendApi ?? '', fapiRedirectUrl)
) {
return fapiRedirectUrl;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/shared/src/internal/clerk-js/redirectUrls.ts` around lines 102 -
109, The FAPI priority check uses isFAPIInitiatedFlowPath which only inspects
the pathname and can wrongly match non-FAPI hosts; change the gating to verify
the host as well by using the same logic as isRedirectForFAPIInitiatedFlow (or
call isRedirectForFAPIInitiatedFlow with the redirect URL and this.frontendApi)
instead of isFAPIInitiatedFlowPath in the block that reads
this.fromSearchParams.redirectUrl, and add unit tests verifying (a) a URL on a
non-FAPI host with an allowed FAPI path does NOT bypass signInForceRedirectUrl,
and (b) a valid FAPI continuation URL on the frontend API host DOES bypass it.


let result;
// Prioritize forceRedirectUrl
result = this.fromSearchParams[forceKey] || this.fromProps[forceKey] || this.fromOptions[forceKey];
Expand Down
25 changes: 24 additions & 1 deletion packages/shared/src/internal/clerk-js/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -400,12 +400,13 @@ export const pathFromFullPath = (fullPath: string) => {

const frontendApiRedirectPathsWithUserInput: string[] = [
'/oauth/authorize', // OAuth2 identify provider flow
'/oauth/authorize/continue', // OAuth 2 identity provider continuation
'/oauth/authorize-with-immediate-redirect', // OAuth 2 identity provider (requires sign-in first)
];

const frontendApiRedirectPathsNoUserInput: string[] = [
'/v1/verify', // magic links
'/v1/tickets/accept', // ticket flow
'/oauth/authorize-with-immediate-redirect', // OAuth 2 identity provider
'/oauth/end_session', // OIDC logout
];

Expand All @@ -423,6 +424,28 @@ export function requiresUserInput(redirectUrl: string): boolean {
return frontendApiRedirectPathsWithUserInput.includes(url.pathname);
}

/**
* Checks if a URL points to a known FAPI-initiated flow endpoint.
* Only matches absolute URLs whose origin differs from the current window origin,
* preventing customer app routes from accidentally matching FAPI paths.
*/
export function isFAPIInitiatedFlowPath(url: string): boolean {
try {
// Only match absolute URLs — relative paths like "/oauth/authorize" should not match
// as they would be customer app routes, not FAPI endpoints.
// new URL(url) without a base will throw for relative URLs.
const parsed = new URL(url);
// Ensure the URL is not on the same origin as the current page (FAPI is always cross-origin)
if (typeof window !== 'undefined' && parsed.origin === window.location.origin) {
return false;
}
const allFAPIFlowPaths = [...frontendApiRedirectPathsWithUserInput, ...frontendApiRedirectPathsNoUserInput];
return allFAPIFlowPaths.includes(parsed.pathname);
} catch {
return false;
}
}

export const isAllowedRedirect =
(allowedRedirectOrigins: Array<string | RegExp> | undefined, currentOrigin: string) => (_url: URL | string) => {
let url = _url;
Expand Down
Loading