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/chubby-memes-kiss.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': minor
---

When a session already exists on sign in, complete the sign in and redirect instead of only rendering an error.
32 changes: 32 additions & 0 deletions integration/tests/oauth-flows.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,5 +256,37 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withLegalConsent] })(

await u.page.waitForAppUrl('/protected');
});

test('redirects when attempting OAuth sign in with existing session in another tab', async ({
page,
context,
browser,
}) => {
const u = createTestUtils({ app, page, context, browser });

// Open sign-in page in both tabs before signing in
await u.po.signIn.goTo();

let secondTabUtils: any;
await u.tabs.runInNewTab(async u2 => {
secondTabUtils = u2;
await u2.po.signIn.goTo();
});

// Sign in via OAuth on the first tab
await u.page.getByRole('button', { name: 'E2E OAuth Provider' }).click();
await u.page.getByText('Sign in to oauth-provider').waitFor();
await u.po.signIn.setIdentifier(fakeUser.email);
await u.po.signIn.continue();
await u.po.signIn.enterTestOtpCode();
await u.page.getByText('SignedIn').waitFor();
await u.po.expect.toBeSignedIn();

// Attempt to sign in via OAuth on the second tab (which already has sign-in mounted)
await secondTabUtils.page.getByRole('button', { name: 'E2E OAuth Provider' }).click();

// Should redirect and be signed in without error
await secondTabUtils.po.expect.toBeSignedIn();
});
Comment on lines +260 to +290
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Avoid any type for secondTabUtils.

Line 270 declares secondTabUtils with any type, which violates the TypeScript coding guidelines. The return type of createTestUtils is complex but can be properly typed.

Apply this diff to properly type the variable:

-    let secondTabUtils: any;
+    let secondTabUtils: Awaited<ReturnType<typeof u.tabs.runInNewTab>>;
     await u.tabs.runInNewTab(async u2 => {
       secondTabUtils = u2;
       await u2.po.signIn.goTo();
     });

As per coding guidelines.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In integration/tests/oauth-flows.test.ts around lines 260 to 290, the variable
secondTabUtils is declared as any on line 270; replace this with a proper type
by using the factory return type: declare it as let secondTabUtils!:
ReturnType<typeof createTestUtils>; (or let secondTabUtils: ReturnType<typeof
createTestUtils> | null = null and add a null-check) and remove any usage of any
so the value assigned inside u.tabs.runInNewTab has the correct typed shape and
the later accesses (secondTabUtils.page, secondTabUtils.po, etc.) are type-safe.

},
);
31 changes: 31 additions & 0 deletions integration/tests/sign-in-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,4 +150,35 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('sign in f

await u.po.expect.toBeSignedIn();
});

test('redirects when attempting to sign in with existing session in another tab', async ({
page,
context,
browser,
}) => {
const u = createTestUtils({ app, page, context, browser });

// Open sign-in page in both tabs before signing in
await u.po.signIn.goTo();

let secondTabUtils: any;
await u.tabs.runInNewTab(async u2 => {
secondTabUtils = u2;
await u2.po.signIn.goTo();
});

// Sign in on the first tab
await u.po.signIn.setIdentifier(fakeUser.email);
await u.po.signIn.continue();
await u.po.signIn.setPassword(fakeUser.password);
await u.po.signIn.continue();
await u.po.expect.toBeSignedIn();

// Attempt to sign in on the second tab (which already has sign-in mounted)
await secondTabUtils.po.signIn.setIdentifier(fakeUser.email);
await secondTabUtils.po.signIn.continue();

// Should redirect and be signed in without error
await secondTabUtils.po.expect.toBeSignedIn();
});
});
1 change: 1 addition & 0 deletions packages/clerk-js/src/core/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const ERROR_CODES = {
SAML_USER_ATTRIBUTE_MISSING: 'saml_user_attribute_missing',
USER_LOCKED: 'user_locked',
EXTERNAL_ACCOUNT_NOT_FOUND: 'external_account_not_found',
SESSION_EXISTS: 'session_exists',
SIGN_UP_MODE_RESTRICTED: 'sign_up_mode_restricted',
SIGN_UP_MODE_RESTRICTED_WAITLIST: 'sign_up_restricted_waitlist',
ENTERPRISE_SSO_USER_ATTRIBUTE_MISSING: 'enterprise_sso_user_attribute_missing',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { isClerkAPIResponseError } from '@clerk/shared/error';
import { useOrganizationList, useUser } from '@clerk/shared/react';
import type { OrganizationResource } from '@clerk/shared/types';

import { isClerkAPIResponseError } from '@/index.headless';
import { sharedMainIdentifierSx } from '@/ui/common/organizations/OrganizationPreview';
import { localizationKeys, useLocalizations } from '@/ui/customizables';
import { useCardState, withCardStateProvider } from '@/ui/elements/contexts';
Expand Down
30 changes: 26 additions & 4 deletions packages/clerk-js/src/ui/components/SignIn/SignInSocialButtons.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import type { ClerkAPIError } from '@clerk/shared/error';
import { isClerkAPIResponseError } from '@clerk/shared/error';
import { useClerk } from '@clerk/shared/react';
import type { PhoneCodeChannel } from '@clerk/shared/types';
import React from 'react';

import { handleError } from '@/ui/utils/errorHandler';
import { ERROR_CODES } from '@/core/constants';
import { handleError as _handleError } from '@/ui/utils/errorHandler';
import { originPrefersPopup } from '@/ui/utils/originPrefersPopup';
import { web3CallbackErrorHandler } from '@/ui/utils/web3CallbackErrorHandler';

Expand Down Expand Up @@ -30,10 +33,29 @@ export const SignInSocialButtons = React.memo((props: SignInSocialButtonsProps)
const shouldUsePopup = ctx.oauthFlow === 'popup' || (ctx.oauthFlow === 'auto' && originPrefersPopup());
const { onAlternativePhoneCodeProviderClick, ...rest } = props;

const handleError = (err: any) => {
if (isClerkAPIResponseError(err)) {
const sessionAlreadyExistsError: ClerkAPIError | undefined = err.errors.find(
(e: ClerkAPIError) => e.code === ERROR_CODES.SESSION_EXISTS,
);

if (sessionAlreadyExistsError) {
return clerk.setActive({
session: clerk.client.lastActiveSessionId,
navigate: async ({ session }) => {
await ctx.navigateOnSetActive({ session, redirectUrl: ctx.afterSignInUrl });
},
});
}
}

return _handleError(err, [], card.setError);
};

return (
<SocialButtons
{...rest}
showLastAuthenticationStrategy={true}
showLastAuthenticationStrategy
idleAfterDelay={!shouldUsePopup}
oauthCallback={strategy => {
if (shouldUsePopup) {
Expand All @@ -50,12 +72,12 @@ export const SignInSocialButtons = React.memo((props: SignInSocialButtonsProps)

return signIn
.authenticateWithPopup({ strategy, redirectUrl, redirectUrlComplete, popup, oidcPrompt: ctx.oidcPrompt })
.catch(err => handleError(err, [], card.setError));
.catch(err => handleError(err));
}

return signIn
.authenticateWithRedirect({ strategy, redirectUrl, redirectUrlComplete, oidcPrompt: ctx.oidcPrompt })
.catch(err => handleError(err, [], card.setError));
.catch(err => handleError(err));
}}
web3Callback={strategy => {
return clerk
Expand Down
10 changes: 10 additions & 0 deletions packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,9 @@ function SignInStartInternal(): JSX.Element {
e.code === ERROR_CODES.FORM_PASSWORD_PWNED,
);

const sessionAlreadyExistsError: ClerkAPIError = e.errors.find(
(e: ClerkAPIError) => e.code === ERROR_CODES.SESSION_EXISTS,
);
const alreadySignedInError: ClerkAPIError = e.errors.find(
(e: ClerkAPIError) => e.code === 'identifier_already_signed_in',
);
Expand All @@ -442,6 +445,13 @@ function SignInStartInternal(): JSX.Element {

if (instantPasswordError) {
await signInWithFields(identifierField);
} else if (sessionAlreadyExistsError) {
await clerk.setActive({
session: clerk.client.lastActiveSessionId,
navigate: async ({ session }) => {
await navigateOnSetActive({ session, redirectUrl: afterSignInUrl });
},
});
} else if (alreadySignedInError) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const sid = alreadySignedInError.meta!.sessionId!;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,38 @@ describe('SignInStart', () => {
expect(icon.length).toEqual(1);
});
});

it('redirects user when session_exists error is returned during OAuth sign-in', async () => {
const { wrapper, fixtures } = await createFixtures(f => {
f.withSocialProvider({ provider: 'google' });
});

const sessionExistsError = new ClerkAPIResponseError('Error', {
data: [
{
code: 'session_exists',
long_message: 'A session already exists',
message: 'Session exists',
},
],
status: 422,
});

fixtures.clerk.client.lastActiveSessionId = 'sess_123';
fixtures.signIn.authenticateWithRedirect.mockRejectedValueOnce(sessionExistsError);

const { userEvent } = render(<SignInStart />, { wrapper });

const googleButton = screen.getByText('Continue with Google');
await userEvent.click(googleButton);

await waitFor(() => {
expect(fixtures.clerk.setActive).toHaveBeenCalledWith({
session: 'sess_123',
navigate: expect.any(Function),
});
});
});
});

describe('navigation', () => {
Expand Down Expand Up @@ -523,6 +555,76 @@ describe('SignInStart', () => {
});
});

describe('Session already exists error handling', () => {
it('redirects user when session_exists error is returned during sign-in', async () => {
const { wrapper, fixtures } = await createFixtures(f => {
f.withEmailAddress();
});

const sessionExistsError = new ClerkAPIResponseError('Error', {
data: [
{
code: 'session_exists',
long_message: 'A session already exists',
message: 'Session exists',
},
],
status: 422,
});

fixtures.clerk.client.lastActiveSessionId = 'sess_123';
fixtures.signIn.create.mockRejectedValueOnce(sessionExistsError);

const { userEvent } = render(<SignInStart />, { wrapper });

await userEvent.type(screen.getByLabelText(/email address/i), 'hello@clerk.com');
await userEvent.click(screen.getByText('Continue'));

await waitFor(() => {
expect(fixtures.clerk.setActive).toHaveBeenCalledWith({
session: 'sess_123',
navigate: expect.any(Function),
});
});
});

it('calls navigate after setting session active on session_exists error', async () => {
const { wrapper, fixtures } = await createFixtures(f => {
f.withEmailAddress();
});

const sessionExistsError = new ClerkAPIResponseError('Error', {
data: [
{
code: 'session_exists',
long_message: 'A session already exists',
message: 'Session exists',
},
],
status: 422,
});

fixtures.clerk.client.lastActiveSessionId = 'sess_123';
fixtures.signIn.create.mockRejectedValueOnce(sessionExistsError);

const mockSession = { id: 'sess_123' } as any;
(fixtures.clerk.setActive as any).mockImplementation(
async ({ navigate }: { navigate: ({ session }: { session: any }) => Promise<void> }) => {
await navigate({ session: mockSession });
},
);

const { userEvent } = render(<SignInStart />, { wrapper });

await userEvent.type(screen.getByLabelText(/email address/i), 'hello@clerk.com');
await userEvent.click(screen.getByText('Continue'));

await waitFor(() => {
expect(fixtures.clerk.setActive).toHaveBeenCalled();
});
});
});

describe('ticket flow', () => {
it('calls the appropriate resource function upon detecting the ticket', async () => {
const { wrapper, fixtures } = await createFixtures(f => {
Expand Down
2 changes: 1 addition & 1 deletion packages/clerk-js/src/ui/components/SignUp/SignUpStart.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { getAlternativePhoneCodeProviderData } from '@clerk/shared/alternativePhoneCode';
import { isClerkAPIResponseError } from '@clerk/shared/error';
import { useClerk } from '@clerk/shared/react';
import type { PhoneCodeChannel, PhoneCodeChannelData, SignUpResource } from '@clerk/shared/types';
import React from 'react';

import { isClerkAPIResponseError } from '@/index.headless';
import { Card } from '@/ui/elements/Card';
import { useCardState, withCardStateProvider } from '@/ui/elements/contexts';
import { Header } from '@/ui/elements/Header';
Expand Down
Loading