fix(clerk-js): Disambiguate redirect url options between flows (#5645)

Author: brkalow

.changeset/cruel-baths-sleep.md

@@ -0,0 +1,5 @@
+---
+'@clerk/clerk-js': patch
+---
+
+Fix an issue where `fallbackRedirectUrl` and `forceRedirectUrl` were being improperly passed from sign up to sign in and vice versa. These props will now only apply to the specific flow they were passed to initially.

integration/tests/oauth-flows.test.ts

@@ -94,7 +94,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('oauth flo
   test('sign up modal', async ({ page, context }) => {
     const u = createTestUtils({ app, page, context });
     // The SignUpModal will only redirect to its provided forceRedirectUrl if the user is signing up; it will not
-    // redirect if the sign up is transfered to a sign in.
+    // redirect if the sign up is transferred to a sign in.
     await u.services.users.deleteIfExists({ email: fakeUser.email });
 
     await u.page.goToRelative('/buttons');
@@ -114,6 +114,46 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('oauth flo
     await u.page.waitForAppUrl('/protected');
   });
 
+  test('sign in modal to sign up modal to transfer respects original forceRedirectUrl', async ({ page, context }) => {
+    const u = createTestUtils({ app, page, context });
+
+    try {
+      await u.services.users.createBapiUser(fakeUser);
+    } catch {
+      // User already exists, so we don't need to create it
+    }
+
+    await u.page.goToRelative('/buttons');
+    await u.page.waitForClerkJsLoaded();
+    await u.po.expect.toBeSignedOut();
+
+    // Call clerk.openSignIn with custom redirect URLs
+    await u.page.evaluate(() => {
+      window.Clerk.openSignIn({
+        forceRedirectUrl: '/?from=signin',
+        signUpForceRedirectUrl: '/?from=signup',
+      });
+    });
+
+    await u.po.signIn.waitForModal();
+
+    // Click the Sign Up button to switch to sign up mode
+    await u.page.getByRole('dialog').getByRole('link', { name: 'Sign up' }).click();
+
+    // Use OAuth provider
+    await u.page.getByRole('button', { name: 'E2E OAuth Provider' }).click();
+    await u.page.getByText('Sign in to oauth-provider').waitFor();
+
+    // Sign in with existing account
+    await u.po.signIn.setIdentifier(fakeUser.email);
+    await u.po.signIn.continue();
+    await u.po.signIn.enterTestOtpCode();
+
+    // Should redirect to the sign in redirect URL since we already had an account
+    await u.page.waitForAppUrl('/?from=signin');
+    await u.po.expect.toBeSignedIn();
+  });
+
   test.describe('authenticateWithPopup', () => {
     test('SignIn with oauthFlow=popup opens popup', async ({ page, context }) => {
       const u = createTestUtils({ app, page, context });

packages/clerk-js/src/ui/Components.tsx

@@ -18,6 +18,7 @@ import React, { Suspense } from 'react';
 
 import { clerkUIErrorDOMElementNotFound } from '../core/errors';
 import { buildVirtualRouterUrl } from '../utils';
+import { disambiguateRedirectOptions } from '../utils/disambiguateRedirectOptions';
 import type { AppearanceCascade } from './customizables/parseAppearance';
 // NOTE: Using `./hooks` instead of `./hooks/useClerkModalStateParams` will increase the bundle size
 import { useClerkModalStateParams } from './hooks/useClerkModalStateParams';
@@ -396,7 +397,7 @@ const Components = (props: ComponentsProps) => {
       componentName={'SignInModal'}
     >
       <SignInModal {...signInModal} />
-      <SignUpModal {...signInModal} />
+      <SignUpModal {...disambiguateRedirectOptions(signInModal, 'signin')} />
       <WaitlistModal {...waitlistModal} />
     </LazyModalRenderer>
   );
@@ -412,7 +413,7 @@ const Components = (props: ComponentsProps) => {
       startPath={buildVirtualRouterUrl({ base: '/sign-up', path: urlStateParam?.path })}
       componentName={'SignUpModal'}
     >
-      <SignInModal {...signUpModal} />
+      <SignInModal {...disambiguateRedirectOptions(signUpModal, 'signup')} />
       <SignUpModal {...signUpModal} />
       <WaitlistModal {...waitlistModal} />
     </LazyModalRenderer>

packages/clerk-js/src/ui/components/SignIn/SignIn.tsx

@@ -5,7 +5,6 @@ import React from 'react';
 import { SessionTasks as LazySessionTasks } from '../../../ui/lazyModules/components';
 import { normalizeRoutingOptions } from '../../../utils/normalizeRoutingOptions';
 import { SignInEmailLinkFlowComplete, SignUpEmailLinkFlowComplete } from '../../common/EmailLinkCompleteFlowCard';
-import type { SignUpContextType } from '../../contexts';
 import {
   SignInContext,
   SignUpContext,
@@ -17,6 +16,7 @@ import { Flow } from '../../customizables';
 import { useFetch } from '../../hooks';
 import { usePreloadTasks } from '../../hooks/usePreloadTasks';
 import { Route, Switch, useRouter, VIRTUAL_ROUTER_BASE_PATH } from '../../router';
+import type { SignUpCtx } from '../../types';
 import {
   LazySignUpContinue,
   LazySignUpSSOCallback,
@@ -173,7 +173,7 @@ function SignInRoot() {
     signInUrl: signInContext.signInUrl,
     unsafeMetadata: signInContext.unsafeMetadata,
     ...normalizeRoutingOptions({ routing: signInContext?.routing, path: signInContext?.path }),
-  } as SignUpContextType;
+  } as SignUpCtx;
 
   /**
    * Preload Sign Up when in Combined Flow.

packages/clerk-js/src/ui/contexts/components/SignIn.ts

@@ -17,7 +17,7 @@ import { useRouter } from '../../router';
 import type { SignInCtx } from '../../types';
 import { getInitialValuesFromQueryParams } from '../utils';
 
-export type SignInContextType = SignInCtx & {
+export type SignInContextType = Omit<SignInCtx, 'fallbackRedirectUrl' | 'forceRedirectUrl'> & {
   navigateAfterSignIn: () => any;
   queryParams: ParsedQueryString;
   signUpUrl: string;
@@ -66,13 +66,16 @@ export const useSignInContext = (): SignInContextType => {
     options,
     {
       ...ctx,
-      signInFallbackRedirectUrl: ctx.fallbackRedirectUrl,
-      signInForceRedirectUrl: ctx.forceRedirectUrl,
+      signInFallbackRedirectUrl: ctx.signInFallbackRedirectUrl || ctx.fallbackRedirectUrl,
+      signInForceRedirectUrl: ctx.signInForceRedirectUrl || ctx.forceRedirectUrl,
     },
     queryParams,
     mode,
   );
 
+  delete ctx.fallbackRedirectUrl;
+  delete ctx.forceRedirectUrl;
+
   const afterSignInUrl = clerk.buildUrlWithAuth(redirectUrls.getAfterSignInUrl());
   const afterSignUpUrl = clerk.buildUrlWithAuth(redirectUrls.getAfterSignUpUrl());
 

packages/clerk-js/src/ui/contexts/components/SignUp.ts

@@ -17,7 +17,7 @@ import { useRouter } from '../../router';
 import type { SignUpCtx } from '../../types';
 import { getInitialValuesFromQueryParams } from '../utils';
 
-export type SignUpContextType = SignUpCtx & {
+export type SignUpContextType = Omit<SignUpCtx, 'fallbackRedirectUrl' | 'forceRedirectUrl'> & {
   navigateAfterSignUp: () => any;
   queryParams: ParsedQueryString;
   signInUrl: string;
@@ -65,13 +65,16 @@ export const useSignUpContext = (): SignUpContextType => {
     options,
     {
       ...ctx,
-      signUpFallbackRedirectUrl: ctx.fallbackRedirectUrl,
-      signUpForceRedirectUrl: ctx.forceRedirectUrl,
+      signUpFallbackRedirectUrl: ctx.signUpFallbackRedirectUrl || ctx.fallbackRedirectUrl,
+      signUpForceRedirectUrl: ctx.signUpForceRedirectUrl || ctx.forceRedirectUrl,
     },
     queryParams,
     mode,
   );
 
+  delete ctx.fallbackRedirectUrl;
+  delete ctx.forceRedirectUrl;
+
   const afterSignUpUrl = clerk.buildUrlWithAuth(redirectUrls.getAfterSignUpUrl());
   const afterSignInUrl = clerk.buildUrlWithAuth(redirectUrls.getAfterSignInUrl());
 

packages/clerk-js/src/ui/types.ts

@@ -11,7 +11,11 @@ import type {
   OrganizationListProps,
   OrganizationProfileProps,
   OrganizationSwitcherProps,
+  SignInFallbackRedirectUrl,
+  SignInForceRedirectUrl,
   SignInProps,
+  SignUpFallbackRedirectUrl,
+  SignUpForceRedirectUrl,
   SignUpProps,
   UserButtonProps,
   UserProfileProps,
@@ -51,7 +55,8 @@ type ComponentMode = 'modal' | 'mounted';
 export type SignInCtx = SignInProps & {
   componentName: 'SignIn';
   mode?: ComponentMode;
-};
+} & SignInFallbackRedirectUrl &
+  SignInForceRedirectUrl;
 
 export type UserVerificationCtx = __internal_UserVerificationProps & {
   componentName: 'UserVerification';
@@ -68,7 +73,8 @@ export type SignUpCtx = SignUpProps & {
   mode?: ComponentMode;
   emailLinkRedirectUrl?: string;
   ssoCallbackUrl?: string;
-};
+} & SignUpFallbackRedirectUrl &
+  SignUpForceRedirectUrl;
 
 export type UserButtonCtx = UserButtonProps & {
   componentName: 'UserButton';

packages/clerk-js/src/utils/disambiguateRedirectOptions.ts

@@ -0,0 +1,43 @@
+type Flow = 'signin' | 'signup';
+
+interface RedirectOptions {
+  forceRedirectUrl?: string | null;
+  fallbackRedirectUrl?: string | null;
+}
+
+type PrefixedRedirectOptions<F extends Flow> = {
+  [K in keyof RedirectOptions as `${F extends 'signin' ? 'signIn' : 'signUp'}${Capitalize<K>}`]: RedirectOptions[K];
+};
+
+type DisambiguatedRedirectOptions<T extends RedirectOptions, F extends Flow> = {
+  [K in keyof T as K extends 'forceRedirectUrl' | 'fallbackRedirectUrl' ? never : K]: T[K];
+} & PrefixedRedirectOptions<F>;
+
+/**
+ * Ensures redirect props have the appropriate prefix depending on the flow they were originally provided to. Used when passing props from sign up -> sign in, or vice versa, when rendering modals.
+ */
+export function disambiguateRedirectOptions<T extends RedirectOptions, F extends Flow>(
+  props: T | null,
+  flow: F,
+): DisambiguatedRedirectOptions<T, F> {
+  if (!props) {
+    return {} as DisambiguatedRedirectOptions<T, F>;
+  }
+
+  const prefix = flow === 'signin' ? 'signIn' : 'signUp';
+
+  const prefixedOptions = {
+    [`${prefix}ForceRedirectUrl`]: props.forceRedirectUrl,
+    [`${prefix}FallbackRedirectUrl`]: props.fallbackRedirectUrl,
+  } as PrefixedRedirectOptions<F>;
+
+  const result = {
+    ...props,
+    ...prefixedOptions,
+  };
+
+  delete result.forceRedirectUrl;
+  delete result.fallbackRedirectUrl;
+
+  return result;
+}