Skip to content

Commit 27b73b1

Browse files
committed
fix(ui): route the challenge raised from the enterprise connections picker
`SignInFactorOneEnterpriseConnections` called `authenticateWithRedirect` and returned it directly. A challenge raised while preparing the hand-off makes that call resolve *without* issuing a redirect, so the user clicked their connection and the card sat there unchanged — no challenge, no error, nothing. Every other first- and second-factor call site funnels through `navigateOnSignInProtectGate`; this one did not. Route it to `../protect-check` like the rest. Also records why `resumeEnterpriseSSO` passes no `enterpriseConnectionId`: it runs only under the single-connection guard, so the server has exactly one connection to prepare. Resuming a connection the user picked from this card would need the id carried across the challenge, which nothing does today.
1 parent 9252160 commit 27b73b1

4 files changed

Lines changed: 77 additions & 3 deletions

File tree

‎.changeset/enterprise-sso-hand-off-challenge.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@
33
'@clerk/ui': patch
44
---
55

6-
Fix enterprise SSO sign-ins erroring instead of showing a verification challenge raised while handing off to the identity provider.
6+
Fix enterprise SSO sign-ins erroring, or appearing to do nothing, instead of showing a verification challenge raised while handing off to the identity provider. This covers the card for choosing between multiple enterprise connections, where clicking a connection left the user on an unchanged card.

‎packages/ui/src/components/SignIn/SignInFactorOneEnterpriseConnections.tsx‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import { Flow, localizationKeys } from '@/ui/customizables';
88
import { withCardStateProvider } from '@/ui/elements/contexts';
99
import type { AvailableComponentProps } from '@/ui/types';
1010

11+
import { useRouter } from '../../router';
12+
import { navigateOnSignInProtectGate } from './handleProtectCheck';
1113
import { hasMultipleEnterpriseConnections } from './shared';
1214

1315
/**
@@ -16,6 +18,7 @@ import { hasMultipleEnterpriseConnections } from './shared';
1618
const SignInFactorOneEnterpriseConnectionsInternal = () => {
1719
const ctx = useSignInContext();
1820
const clerk = useClerk();
21+
const { navigate } = useRouter();
1922
const signIn = clerk.client.signIn;
2023

2124
if (!hasMultipleEnterpriseConnections(signIn.supportedFirstFactors)) {
@@ -28,18 +31,23 @@ const SignInFactorOneEnterpriseConnectionsInternal = () => {
2831
name: ff.enterpriseConnectionName,
2932
}));
3033

31-
const handleEnterpriseSSO = (enterpriseConnectionId: string) => {
34+
const handleEnterpriseSSO = async (enterpriseConnectionId: string) => {
3235
const redirectUrl = ctx.ssoCallbackUrl;
3336
const redirectUrlComplete = ctx.afterSignInUrl || '/';
3437

35-
return signIn.authenticateWithRedirect({
38+
await signIn.authenticateWithRedirect({
3639
strategy: 'enterprise_sso',
3740
redirectUrl,
3841
redirectUrlComplete,
3942
oidcPrompt: ctx.oidcPrompt,
4043
continueSignIn: true,
4144
enterpriseConnectionId,
4245
});
46+
47+
// Preparing the hand-off can itself raise a challenge, in which case no redirect was issued
48+
// and the sign-in is sitting on the gate instead. Without this the picker looks inert: the
49+
// user clicks their connection and nothing happens.
50+
navigateOnSignInProtectGate(signIn, navigate, '../protect-check');
4351
};
4452

4553
return (

‎packages/ui/src/components/SignIn/SignInProtectCheck.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ function SignInProtectCheckInternal(): JSX.Element | null {
8282
}
8383
await resumeSignInAfterProtectCheck(updatedSignIn, {
8484
navigate,
85+
// No `enterpriseConnectionId` is passed: this runs only under
86+
// `shouldHandOffToEnterpriseConnection`, which requires a single connection, so the server
87+
// has exactly one to prepare. If that guard is ever loosened to resume a connection the
88+
// user chose, the id has to be carried across the challenge and passed here.
8589
resumeEnterpriseSSO: async () => {
8690
await signIn.authenticateWithRedirect({
8791
strategy: 'enterprise_sso',
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import type { SignInResource } from '@clerk/shared/types';
2+
import { waitFor } from '@testing-library/react';
3+
import { describe, expect, it } from 'vitest';
4+
5+
import { bindCreateFixtures } from '@/test/create-fixtures';
6+
import { render, screen } from '@/test/utils';
7+
8+
import { SignInFactorOneEnterpriseConnections } from '../SignInFactorOneEnterpriseConnections';
9+
10+
const { createFixtures } = bindCreateFixtures('SignIn');
11+
12+
/** Two connections is what puts the user on this card rather than a direct hand-off. */
13+
const TWO_CONNECTIONS = [
14+
{ strategy: 'enterprise_sso', enterpriseConnectionId: 'ent_acme', enterpriseConnectionName: 'Acme SSO' },
15+
{ strategy: 'enterprise_sso', enterpriseConnectionId: 'ent_globex', enterpriseConnectionName: 'Globex SSO' },
16+
];
17+
18+
describe('SignInFactorOneEnterpriseConnections', () => {
19+
it('routes to the challenge when preparing the hand-off raises one', async () => {
20+
// GIVEN a user choosing between two enterprise connections
21+
const { wrapper, fixtures } = await createFixtures(f => {
22+
f.withEmailAddress();
23+
f.startSignInWithEmailAddress();
24+
});
25+
(fixtures.signIn as unknown as SignInResource).supportedFirstFactors = TWO_CONNECTIONS as never;
26+
// WHEN preparing the hand-off comes back gated: no redirect is issued, the call just resolves.
27+
fixtures.signIn.authenticateWithRedirect.mockImplementationOnce(() => {
28+
(fixtures.signIn as any).protectCheck = { status: 'pending', token: 'challenge-token-abc' };
29+
return Promise.resolve();
30+
});
31+
32+
const { userEvent } = render(<SignInFactorOneEnterpriseConnections />, { wrapper });
33+
await userEvent.click(await screen.findByText('Acme SSO'));
34+
35+
// THEN the challenge is shown, instead of the card sitting there looking inert.
36+
await waitFor(() => {
37+
expect(fixtures.router.navigate).toHaveBeenCalledWith('../protect-check');
38+
});
39+
expect(fixtures.signIn.authenticateWithRedirect).toHaveBeenCalledWith(
40+
expect.objectContaining({ strategy: 'enterprise_sso', enterpriseConnectionId: 'ent_acme' }),
41+
);
42+
});
43+
44+
it('does not route to the challenge when the hand-off is issued normally', async () => {
45+
// GIVEN the same card, but nothing gates the hand-off
46+
const { wrapper, fixtures } = await createFixtures(f => {
47+
f.withEmailAddress();
48+
f.startSignInWithEmailAddress();
49+
});
50+
(fixtures.signIn as unknown as SignInResource).supportedFirstFactors = TWO_CONNECTIONS as never;
51+
fixtures.signIn.authenticateWithRedirect.mockResolvedValueOnce(undefined as never);
52+
53+
const { userEvent } = render(<SignInFactorOneEnterpriseConnections />, { wrapper });
54+
await userEvent.click(await screen.findByText('Globex SSO'));
55+
56+
// THEN the redirect owns the navigation and we must not steal it.
57+
await waitFor(() => {
58+
expect(fixtures.signIn.authenticateWithRedirect).toHaveBeenCalled();
59+
});
60+
expect(fixtures.router.navigate).not.toHaveBeenCalledWith('../protect-check');
61+
});
62+
});

0 commit comments

Comments
 (0)