Skip to content
Merged
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
96 changes: 17 additions & 79 deletions apps/web/src/components/auth/SignInForm.signInOptions.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
/* eslint-disable @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires -- Jest node-environment mocks must be registered before loading the component. */
// The sign-in landing keeps the email prompt but must also offer the OAuth
// providers (including 'Continue with ChatGPT'), the same group the sign-up page
// renders. The ChatGPT option is behind the PostHog flag, which for a
// signed-out visitor is evaluated against the email the visitor typed; the hook
// is stubbed here and the filter is asserted. The provider buttons and the
// The sign-in landing keeps the email prompt but must offer the same OAuth
// provider group as sign-up, 'Continue with ChatGPT' included. The ChatGPT
// button is not decided from the visitor's address: it renders on the first
// screen, before the visitor takes the email step. The provider buttons and the
// email form are stubbed the way `SignInForm.test.ts` stubs them (their CSS
// module cannot load in jest), but the stubs render the real provider labels
// and the form's submit label.
Expand All @@ -14,20 +13,11 @@ import { renderToStaticMarkup } from 'react-dom/server';

let mockFlowEmail = '';
let mockHintEmail = '';
let mockChatGptAllowed = false;
let mockHookEmail: string | null = null;

jest.mock('@/components/AnimatedLogoMark', () => ({
AnimatedLogoMark: () => null,
}));

jest.mock('@/hooks/useChatGptSignInAccess', () => ({
useChatGptSignInAccess: (email: string | null) => {
mockHookEmail = email;
return mockChatGptAllowed;
},
}));

jest.mock('@/hooks/useSignInFlow', () => ({
useSignInFlow: ({ isSignUp }: { searchParams: Record<string, string>; isSignUp?: boolean }) => ({
isHintLoaded: true,
Expand Down Expand Up @@ -99,89 +89,37 @@ const { SignInForm } = require('./SignInForm') as {
beforeEach(() => {
mockFlowEmail = '';
mockHintEmail = '';
mockChatGptAllowed = false;
mockHookEmail = null;
});

describe('SignInForm sign-in options', () => {
it('hides ChatGPT on sign-in when the flag is off for the submitted email', () => {
const html = renderToStaticMarkup(
createElement(SignInForm, { searchParams: {}, title: 'Welcome.' })
);

expect(html).not.toContain('Continue with ChatGPT');
expect(html).toContain('Continue with Google');
expect(html).toContain('Continue with Email');
// Nothing is known before a submit, so the hook gets no address.
expect(mockHookEmail).toBe(null);
});

it('offers ChatGPT on sign-in when the flag is on for the submitted email', () => {
mockFlowEmail = 'person@kilo.ai';
mockChatGptAllowed = true;
it('offers ChatGPT with the other OAuth providers before the email step', () => {
const html = renderToStaticMarkup(
createElement(SignInForm, { searchParams: {}, title: 'Welcome.' })
);

// No address is known and the visitor has not submitted one, so the button
// cannot come from the ChatGPT access flag: it is a plain sign-in option.
expect(html.match(/Continue with ChatGPT/g)).toHaveLength(1);
expect(html).toContain('Continue with Google');
expect(html).toContain('Continue with Email');
// Typing alone is not evaluated; the hook waits for the submit.
expect(mockHookEmail).toBe(null);
});

it('evaluates a prefilled ?email= address before the providers render', () => {
mockChatGptAllowed = true;
renderToStaticMarkup(
createElement(SignInForm, {
searchParams: { email: 'prefill@kilo.ai' },
title: 'Welcome.',
})
// It renders inside the provider group, in the shared provider order, so it
// reads as one option beside the others rather than a lone button.
expect(html.indexOf('Continue with Google')).toBeLessThan(
html.indexOf('Continue with ChatGPT')
);

expect(mockHookEmail).toBe('prefill@kilo.ai');
});

it('evaluates a stored returning-user address', () => {
mockFlowEmail = 'returning@kilo.ai';
mockHintEmail = 'returning@kilo.ai';
mockChatGptAllowed = true;
renderToStaticMarkup(createElement(SignInForm, { searchParams: {}, title: 'Welcome.' }));

expect(mockHookEmail).toBe('returning@kilo.ai');
});

it('evaluates the query prefill over a stored returning-user address', () => {
mockFlowEmail = 'prefill@kilo.ai';
mockHintEmail = 'hint@kilo.ai';
mockChatGptAllowed = true;
renderToStaticMarkup(
it('offers ChatGPT on the first sign-up screen', () => {
const html = renderToStaticMarkup(
createElement(SignInForm, {
searchParams: { email: 'prefill@kilo.ai' },
title: 'Welcome.',
searchParams: {},
isSignUp: true,
title: 'Create your account',
})
);

expect(mockHookEmail).toBe('prefill@kilo.ai');
});

it('hides ChatGPT on sign-up when the flag is off for the typed email', () => {
const html = renderToStaticMarkup(
createElement(SignInForm, { searchParams: {}, isSignUp: true, title: 'Create your account' })
);

expect(html).not.toContain('Continue with ChatGPT');
expect(html.match(/Continue with ChatGPT/g)).toHaveLength(1);
expect(html).toContain('Continue with Google');
expect(html).toContain('Continue with Email');
});

it('offers ChatGPT on sign-up when the flag is on for the typed email', () => {
mockFlowEmail = 'person@openai.com';
mockChatGptAllowed = true;
const html = renderToStaticMarkup(
createElement(SignInForm, { searchParams: {}, isSignUp: true, title: 'Create your account' })
);

expect(html.match(/Continue with ChatGPT/g)).toHaveLength(1);
});
});
58 changes: 11 additions & 47 deletions apps/web/src/components/auth/SignInForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,11 @@ import Link from 'next/link';
import { SquareUserRound } from 'lucide-react';
import React from 'react';
import type { SignInFormInitialState } from '@/hooks/useSignInFlow';
import { useChatGptSignInAccess } from '@/hooks/useChatGptSignInAccess';
import type { AuthProviderId } from '@kilocode/db/schema-types';
import { OAuthProviderIds } from '@/lib/auth/provider-metadata';
import { buildEnterpriseSsoHref, buildNormalSignInHref } from '@/lib/auth/sign-in-navigation';
import type { SsoAccountMismatch } from '@/lib/auth/sso-account-mismatch';
import getSignInCallbackUrl from '@/lib/getSignInCallbackUrl';

/**
* 'Sign in with ChatGPT' is restricted by the PostHog flag's email allow-list.
* A signed-out visitor is not known to PostHog, so the sign-in page evaluates
* the flag against the email the visitor typed and hides the ChatGPT button
* when the flag is off for that email.
*/
function withoutChatGptWhenUnavailable(
providers: readonly AuthProviderId[],
chatGptAllowed: boolean
): AuthProviderId[] {
return chatGptAllowed ? [...providers] : providers.filter(id => id !== 'openai');
}

type SignInFormProps = {
searchParams: Record<string, string>;
error?: string;
Expand Down Expand Up @@ -70,18 +55,6 @@ export function SignInForm({
isSignUp,
storybookInitialState,
});
// The ChatGPT option is decided from the address the visitor submits, or from
// an address already known without typing. A `?email=` prefill wins over a
// stored returning-user hint: the flow auto-triggers Turnstile for the
// prefill and shows it on the provider screen, so the prefill is the address
// in use. Typing alone never evaluates, so one address costs one reload.
const [submittedEmail, setSubmittedEmail] = React.useState<string | null>(null);
const knownEmail = (searchParams.email || flow.hint?.lastEmail || '').trim();
const chatGptAllowed = useChatGptSignInAccess(submittedEmail ?? (knownEmail || null));
const handleEmailSubmit = (event: React.FormEvent) => {
setSubmittedEmail(flow.email);
flow.handleEmailSubmit(event);
};

// An Enterprise SSO request for a different address than the signed-in
// session cannot proceed; offer the one-tap switch before any normal
Expand Down Expand Up @@ -161,7 +134,7 @@ export function SignInForm({
{errorNotification}
<ProviderSelectView
email={flow.email}
providers={withoutChatGptWhenUnavailable(flow.availableProviders, chatGptAllowed)}
providers={flow.availableProviders}
onProviderSelect={flow.handleProviderSelect}
onBack={flow.handleBack}
purpose={flow.isNewUser ? 'sign-up' : 'sign-in'}
Expand Down Expand Up @@ -242,17 +215,9 @@ export function SignInForm({
? { email: 'Email me a magic link' }
: undefined;

// A returning ChatGPT user keeps the shortcut only while the
// flag allows it; otherwise the full, filtered group is offered
// so they are not left with a single hidden button.
const preferredProviders = withoutChatGptWhenUnavailable(
[lastAuthMethod],
chatGptAllowed
);
const displayedProviders =
preferredProviders.length > 0
? preferredProviders
: withoutChatGptWhenUnavailable(OAuthProviderIds, chatGptAllowed);
// The remembered provider is the whole list; "see other
// sign-in methods" below opens the full group.
const displayedProviders = [lastAuthMethod];

return (
<div className="mx-auto max-w-md space-y-4">
Expand Down Expand Up @@ -284,7 +249,7 @@ export function SignInForm({
<EmailInputForm
email={flow.email}
emailValidation={flow.emailValidation}
onSubmit={handleEmailSubmit}
onSubmit={flow.handleEmailSubmit}
onEmailChange={flow.handleEmailChange}
placeholder="you@example.com"
autoFocus={true}
Expand Down Expand Up @@ -334,7 +299,7 @@ export function SignInForm({
<EmailInputForm
email={flow.email}
emailValidation={flow.emailValidation}
onSubmit={handleEmailSubmit}
onSubmit={flow.handleEmailSubmit}
onEmailChange={flow.handleEmailChange}
placeholder="you@example.com"
autoFocus={true}
Expand Down Expand Up @@ -365,18 +330,17 @@ export function SignInForm({
<>
{/* The sign-in page keeps the email prompt first, but the
OAuth providers (including 'Sign in with ChatGPT') are
offered beside it, as they are on sign-up. */}
offered beside it, as they are on sign-up. ChatGPT is a
plain provider here: the access flag gates the BYOK
connection, not this option. */}
<div className="my-6 flex items-center gap-3">
<Separator className="flex-1" />
<span className="text-muted-foreground text-xs font-medium">or</span>
<Separator className="flex-1" />
</div>
<div className="space-y-2">
<AuthProviderButtons
providers={withoutChatGptWhenUnavailable(
OAuthProviderIds,
chatGptAllowed
)}
providers={OAuthProviderIds}
Comment thread
iscekic marked this conversation as resolved.
onProviderClick={flow.handleOAuthClick}
/>
</div>
Expand All @@ -402,7 +366,7 @@ export function SignInForm({
<PasskeySignInButton callbackUrl={passkeyCallbackUrl} />
{/* OAuth provider buttons - Google first */}
<AuthProviderButtons
providers={withoutChatGptWhenUnavailable(OAuthProviderIds, chatGptAllowed)}
providers={OAuthProviderIds}
onProviderClick={flow.handleOAuthClick}
/>
<SignInButton onClick={flow.handleShowEmailInput}>
Expand Down
4 changes: 0 additions & 4 deletions apps/web/src/components/auth/auth-touch-targets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ jest.mock('@/components/AnimatedLogoMark', () => ({ AnimatedLogoMark: () => null
// buttons are real, only their stylesheet name is stubbed.
jest.mock('./sign-in/AuthProviderButtons.module.css', () => ({ anacondaButton: 'anacondaButton' }));

jest.mock('@/hooks/useChatGptSignInAccess', () => ({
useChatGptSignInAccess: () => false,
}));

jest.mock('@/hooks/usePasskeySignIn', () => ({ usePasskeySignIn: () => mockPasskey }));

jest.mock('next-auth/react', () => ({ signOut: jest.fn(async () => undefined) }));
Expand Down
Loading