From 01855f8784ab32eb4fe8efcd45262b536fa50bdf Mon Sep 17 00:00:00 2001 From: Laura Beatris <48022589+LauraBeatris@users.noreply.github.com> Date: Tue, 25 Feb 2025 11:49:02 -0500 Subject: [PATCH] Introduce new base skeleton for URL resolution --- integration/tests/session-tasks.test.ts | 4 +- .../core/__tests__/clerk.redirects.test.ts | 4 +- packages/clerk-js/src/core/clerk.ts | 69 +++++++++++++++---- packages/clerk-js/src/ui/common/tasks.ts | 2 +- .../components/SignUp/SignUpEmailLinkCard.tsx | 2 - .../components/SignUp/SignUpVerifyEmail.tsx | 2 - .../clerk-js/src/ui/router/BaseRouter.tsx | 1 + 7 files changed, 63 insertions(+), 21 deletions(-) diff --git a/integration/tests/session-tasks.test.ts b/integration/tests/session-tasks.test.ts index b582b5982a1..2ecf8b8da69 100644 --- a/integration/tests/session-tasks.test.ts +++ b/integration/tests/session-tasks.test.ts @@ -1,7 +1,7 @@ import { appConfigs } from '../presets'; import { testAgainstRunningApps } from '../testUtils'; -// TODO ORGS-566 - write integration tests for after-auth flow +// TODO ORGS-566 - Write integration tests for after-auth flow testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('after-auth flows @generic @nextjs', () => { describe('after sign-in', () => { // /sign-in -> /sign-in/select-organization @@ -25,7 +25,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('after-aut it.todo('on single-session mode, sign-up redirects back to tasks when accessed with a pending session'); }); - describe('middle app', () => { + describe('when user is using the app and session transitions to active to pending', () => { // /my-dashboard/recipes -> /sign-in/select-organization it.todo('on session transition to pending with tasks, redirects to tasks'); diff --git a/packages/clerk-js/src/core/__tests__/clerk.redirects.test.ts b/packages/clerk-js/src/core/__tests__/clerk.redirects.test.ts index b35569f0d57..31d1d7046ad 100644 --- a/packages/clerk-js/src/core/__tests__/clerk.redirects.test.ts +++ b/packages/clerk-js/src/core/__tests__/clerk.redirects.test.ts @@ -314,7 +314,7 @@ describe('Clerk singleton - Redirects', () => { describe('.redirectToTasks', () => { describe('after sign-in with pending session', () => { - it.todo('redirects to tasks URL with after sign-in URL appended as query param'); + it('redirects to tasks URL with after sign-in URL appended as query param'); }); describe('after sign-up with pending session', () => { @@ -325,7 +325,7 @@ describe('Clerk singleton - Redirects', () => { it.todo('redirects to tasks URL with after sign-up URL appended as query param'); }); - describe('user already exists and session transitions from active to pending on middle app', () => { + describe('from a protected route', () => { it.todo('redirects to tasks URL with app origin appended as query param'); }); }); diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 4a1cad60800..654c2a19179 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -1007,8 +1007,6 @@ export class Clerk implements ClerkInterface { return; } - console.log('Clerk.navigate is navigating to', { to }); - /** * Trigger all navigation listeners. In order for modal UI components to close. */ @@ -1042,6 +1040,7 @@ export class Clerk implements ClerkInterface { ...(options?.metadata ? { __internal_metadata: options?.metadata } : {}), windowNavigate, }; + // React router only wants the path, search or hash portion. return await customNavigate(stripOrigin(toURL), metadata); }; @@ -1126,7 +1125,54 @@ export class Clerk implements ClerkInterface { return ''; } - return buildURL({ hashPath: sessionTaskKeyToRoutePaths[currentTask.key] }, { stringify: true }); + const referrerIsTaskUrl = window.location.href.includes(sessionTaskKeyToRoutePaths[currentTask.key]); + if (referrerIsTaskUrl) { + return ''; + } + + const signInUrl = this.#options.signInUrl || this.environment?.displayConfig.signInUrl; + const signUpUrl = this.#options.signUpUrl || this.environment?.displayConfig.signUpUrl; + const referrerIsSignInUrl = signInUrl && window.location.href.includes(signInUrl); + const referrerIsSignUpUrl = signUpUrl && window.location.href.includes(signUpUrl); + + let redirectUrl = ''; + if (referrerIsSignInUrl) { + redirectUrl = this.buildAfterSignInUrl(); + } else if (referrerIsSignUpUrl) { + redirectUrl = this.buildAfterSignUpUrl(); + } else { + /** + * User already has a active session and gets transition to a pending status + * on the client update + * + * It could happen on instance configuration changes that lead to new tasks that + * need to get resolved by the user, eg: Force MFA + */ + // Preserves the origin path, eg: /my-app/recipes -> /sign-in/select-organization -> /my-app/recipes + redirectUrl = window.location.href; + } + + /** + * For after sign-in or after sign-up, it's agnostic to the original base path + * in order to avoid having to check for the referrer + * + * If it's coming from a protected route where the user already exists, then + * the base path becomes the sign in URL + */ + const shouldAppendBasePath = !referrerIsSignInUrl && !referrerIsSignUpUrl; + + return buildURL( + { + ...(shouldAppendBasePath + ? { + base: signInUrl, + } + : {}), + hashPath: sessionTaskKeyToRoutePaths[currentTask.key], + hashSearchParams: { redirect_url: redirectUrl }, + }, + { stringify: true }, + ); } public buildAfterMultiSessionSingleSignOutUrl(): string { @@ -1750,10 +1796,6 @@ export class Clerk implements ClerkInterface { if (this.session) { const session = this.#getSessionFromClient(this.session.id); - // TODO - Resolve after-task redirection - // TODO - Resolve issue on closing modals on navigation within Clerk.navigate - // sign-in/select-organization -> /home - // sign-in -> sign-in/select-organization const hasResolvedPreviousTask = this.session.currentTask != session?.currentTask; // Note: this might set this.session to null @@ -1762,8 +1804,6 @@ export class Clerk implements ClerkInterface { // A client response contains its associated sessions, along with a fresh token, so we dispatch a token update event. eventBus.dispatch(events.TokenUpdate, { token: this.session?.lastActiveToken }); - this.#emit(); - // Tasks handling must be reactive on client piggybacking to support // immediate instance-level configuration changes by app owners if (session?.currentTask) { @@ -1771,6 +1811,8 @@ export class Clerk implements ClerkInterface { } else if (session && hasResolvedPreviousTask) { eventBus.dispatch(events.ResolvedSessionTask, session); } + + this.#emit(); } }; @@ -2114,13 +2156,16 @@ export class Clerk implements ClerkInterface { }); eventBus.on(events.NewSessionTask, () => { - console.log('new session task'); void this.redirectToTasks(); }); eventBus.on(events.ResolvedSessionTask, () => { - console.log('resolved task'); - void this.redirectToAfterSignIn(); + // `redirect_url` gets appended based on the origin (after sign-in vs after sign-up), + // if it gets accidentally deleted, then fallbacks to the sign in URL + console.log(window.location.search, 'search on event'); + const redirectUrl = new URLSearchParams(window.location.search).get('redirect_url') ?? this.buildSignInUrl(); + console.log({ redirectUrl }, 'Resolving session task 🍪'); + void this.navigate(redirectUrl); }); }; diff --git a/packages/clerk-js/src/ui/common/tasks.ts b/packages/clerk-js/src/ui/common/tasks.ts index e23b93a323e..d2465f6a4a3 100644 --- a/packages/clerk-js/src/ui/common/tasks.ts +++ b/packages/clerk-js/src/ui/common/tasks.ts @@ -8,5 +8,5 @@ type SessionTaskRoutePath = (typeof sessionTaskRoutePaths)[number]; * @internal */ export const sessionTaskKeyToRoutePaths: Record = { - org: 'select-organization', + org: '/select-organization', }; diff --git a/packages/clerk-js/src/ui/components/SignUp/SignUpEmailLinkCard.tsx b/packages/clerk-js/src/ui/components/SignUp/SignUpEmailLinkCard.tsx index 9f49e130af4..b1a9aec6c0d 100644 --- a/packages/clerk-js/src/ui/components/SignUp/SignUpEmailLinkCard.tsx +++ b/packages/clerk-js/src/ui/components/SignUp/SignUpEmailLinkCard.tsx @@ -13,8 +13,6 @@ import { handleError } from '../../utils'; import { completeSignUpFlow } from './util'; export const SignUpEmailLinkCard = () => { - console.log('rendering'); - const { t } = useLocalizations(); const signUp = useCoreSignUp(); const signUpContext = useSignUpContext(); diff --git a/packages/clerk-js/src/ui/components/SignUp/SignUpVerifyEmail.tsx b/packages/clerk-js/src/ui/components/SignUp/SignUpVerifyEmail.tsx index 10e5a602dbe..4057763d3e0 100644 --- a/packages/clerk-js/src/ui/components/SignUp/SignUpVerifyEmail.tsx +++ b/packages/clerk-js/src/ui/components/SignUp/SignUpVerifyEmail.tsx @@ -4,8 +4,6 @@ import { SignUpEmailCodeCard } from './SignUpEmailCodeCard'; import { SignUpEmailLinkCard } from './SignUpEmailLinkCard'; export const SignUpVerifyEmail = withCardStateProvider(() => { - console.log('rendering'); - const { userSettings } = useEnvironment(); const { attributes } = userSettings; const emailLinkStrategyEnabled = attributes.email_address.verifications.includes('email_link'); diff --git a/packages/clerk-js/src/ui/router/BaseRouter.tsx b/packages/clerk-js/src/ui/router/BaseRouter.tsx index 1874116aa04..cd67ae75853 100644 --- a/packages/clerk-js/src/ui/router/BaseRouter.tsx +++ b/packages/clerk-js/src/ui/router/BaseRouter.tsx @@ -96,6 +96,7 @@ export const BaseRouter = ({ const isCrossOrigin = toURL.origin !== window.location.origin; const isOutsideOfUIComponent = !toURL.pathname.startsWith('/' + basePath); + console.log({ isCrossOrigin, isOutsideOfUIComponent }); if (isOutsideOfUIComponent || isCrossOrigin) { const res = await clerkNavigate(toURL.href);