Skip to content

Commit

Permalink
Separate FAPI-initiated redirect flows to requiring user input or not (
Browse files Browse the repository at this point in the history
…#1343)

* fix(clerk-js): Separate FAPI-initiated redirect flows to requiring user input or not

Previously, for all valid FAPI-initiated redirect flows,
we would require the user to be signed in.
Although this was working properly for oauth, it doesn't
work for magic links or other ticket flows.

This commit updates the logic to separate the allowed
FAPI redirect flows to the ones that require user input,
like the oauth, and the ones that don't, e.g. magic links.

* Create selfish-swans-smell.md

* chore(clerk-js): Added description in changeset

---------

Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com>
  • Loading branch information
alex-ntousias and nikosdouvlis authored Jun 12, 2023
1 parent 010484f commit 846b00b
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/selfish-swans-smell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': minor
---

Fix magic link flows for development instances when url-based session syncing is used.
3 changes: 2 additions & 1 deletion packages/clerk-js/src/core/clerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ import {
pickRedirectionProp,
removeClerkQueryParam,
replaceClerkQueryParam,
requiresUserInput,
sessionExistsAndSingleSessionModeEnabled,
setDevBrowserJWTInURL,
stripOrigin,
Expand Down Expand Up @@ -1416,7 +1417,7 @@ export default class Clerk implements ClerkInterface {
const referrerIsSignInUrl = signInUrl && window.location.href.startsWith(signInUrl);

// don't redirect if user is not signed in and referrer is sign in url
if (!userSignedIn && referrerIsSignInUrl) {
if (requiresUserInput(redirectUrl) && !userSignedIn && referrerIsSignInUrl) {
return false;
}

Expand Down
17 changes: 17 additions & 0 deletions packages/clerk-js/src/utils/__tests__/url.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
isRedirectForFAPIInitiatedFlow,
isValidUrl,
mergeFragmentIntoUrl,
requiresUserInput,
trimTrailingSlash,
} from '../url';

Expand Down Expand Up @@ -386,6 +387,22 @@ describe('isRedirectForFAPIInitiatedFlow(frontendAp: string, redirectUrl: string
);
});

describe('requiresUserInput(redirectUrl: string)', () => {
const testCases: Array<[string, boolean]> = [
['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/v1/verify', false],
['https://clerk.foo.bar-53.lcl.dev/v1/tickets/accept', false],
['https://google.com', false],
['https://google.com/v1/verify', false],
];

test.each(testCases)('redirectUrl=(%s), expected value=(%s)', (redirectUrl, expectedValue) => {
expect(requiresUserInput(redirectUrl)).toEqual(expectedValue);
});
});

describe('getETLDPlusOneFromFrontendApi(frontendAp: string)', () => {
const testCases: Array<[string, string]> = [
['clerk.foo.bar-53.lcl.dev', 'foo.bar-53.lcl.dev'],
Expand Down
14 changes: 12 additions & 2 deletions packages/clerk-js/src/utils/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,8 +337,11 @@ export const pathFromFullPath = (fullPath: string) => {
return fullPath.replace(/CLERK-ROUTER\/(.*?)\//, '');
};

const frontendApiRedirectPaths: string[] = [
const frontendApiRedirectPathsWithUserInput: string[] = [
'/oauth/authorize', // OAuth2 identify provider flow
];

const frontendApiRedirectPathsNoUserInput: string[] = [
'/v1/verify', // magic links
'/v1/tickets/accept', // ticket flow
];
Expand All @@ -347,7 +350,14 @@ export function isRedirectForFAPIInitiatedFlow(frontendApi: string, redirectUrl:
const url = new URL(redirectUrl, DUMMY_URL_BASE);
const path = url.pathname;

return frontendApi === url.host && frontendApiRedirectPaths.includes(path);
const isValidFrontendRedirectPath =
frontendApiRedirectPathsWithUserInput.includes(path) || frontendApiRedirectPathsNoUserInput.includes(path);
return frontendApi === url.host && isValidFrontendRedirectPath;
}

export function requiresUserInput(redirectUrl: string): boolean {
const url = new URL(redirectUrl, DUMMY_URL_BASE);
return frontendApiRedirectPathsWithUserInput.includes(url.pathname);
}

export const isAllowedRedirectOrigin = (_url: string, allowedRedirectOrigins: Array<string | RegExp> | undefined) => {
Expand Down

0 comments on commit 846b00b

Please sign in to comment.