diff --git a/.changeset/cruel-baths-sleep.md b/.changeset/cruel-baths-sleep.md new file mode 100644 index 00000000000..8751b2b17fc --- /dev/null +++ b/.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. diff --git a/integration/tests/oauth-flows.test.ts b/integration/tests/oauth-flows.test.ts index 0e3cbfb9ca7..e2ed1eab9f9 100644 --- a/integration/tests/oauth-flows.test.ts +++ b/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 }); diff --git a/packages/clerk-js/src/ui/Components.tsx b/packages/clerk-js/src/ui/Components.tsx index 31d19687709..82b16bdaa9b 100644 --- a/packages/clerk-js/src/ui/Components.tsx +++ b/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'} > - + ); @@ -412,7 +413,7 @@ const Components = (props: ComponentsProps) => { startPath={buildVirtualRouterUrl({ base: '/sign-up', path: urlStateParam?.path })} componentName={'SignUpModal'} > - + diff --git a/packages/clerk-js/src/ui/components/SignIn/SignIn.tsx b/packages/clerk-js/src/ui/components/SignIn/SignIn.tsx index 7bb9a3316a8..a0f8b078514 100644 --- a/packages/clerk-js/src/ui/components/SignIn/SignIn.tsx +++ b/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. diff --git a/packages/clerk-js/src/ui/contexts/components/SignIn.ts b/packages/clerk-js/src/ui/contexts/components/SignIn.ts index aa6603ce78b..67f6442bb3a 100644 --- a/packages/clerk-js/src/ui/contexts/components/SignIn.ts +++ b/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 & { 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()); diff --git a/packages/clerk-js/src/ui/contexts/components/SignUp.ts b/packages/clerk-js/src/ui/contexts/components/SignUp.ts index 5f9be1727af..7632e782f2d 100644 --- a/packages/clerk-js/src/ui/contexts/components/SignUp.ts +++ b/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 & { 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()); diff --git a/packages/clerk-js/src/ui/types.ts b/packages/clerk-js/src/ui/types.ts index bc00740a219..e6693982092 100644 --- a/packages/clerk-js/src/ui/types.ts +++ b/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'; diff --git a/packages/clerk-js/src/utils/disambiguateRedirectOptions.ts b/packages/clerk-js/src/utils/disambiguateRedirectOptions.ts new file mode 100644 index 00000000000..935009f9805 --- /dev/null +++ b/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 = { + [K in keyof RedirectOptions as `${F extends 'signin' ? 'signIn' : 'signUp'}${Capitalize}`]: RedirectOptions[K]; +}; + +type DisambiguatedRedirectOptions = { + [K in keyof T as K extends 'forceRedirectUrl' | 'fallbackRedirectUrl' ? never : K]: T[K]; +} & PrefixedRedirectOptions; + +/** + * 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( + props: T | null, + flow: F, +): DisambiguatedRedirectOptions { + if (!props) { + return {} as DisambiguatedRedirectOptions; + } + + const prefix = flow === 'signin' ? 'signIn' : 'signUp'; + + const prefixedOptions = { + [`${prefix}ForceRedirectUrl`]: props.forceRedirectUrl, + [`${prefix}FallbackRedirectUrl`]: props.fallbackRedirectUrl, + } as PrefixedRedirectOptions; + + const result = { + ...props, + ...prefixedOptions, + }; + + delete result.forceRedirectUrl; + delete result.fallbackRedirectUrl; + + return result; +}