Skip to content

feat(clerk-js): Support transferable prop on SignIn #3845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
brkalow merged 7 commits into from
Aug 6, 2024
Merged

feat(clerk-js): Support transferable prop on SignIn #3845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
18435f7
Support transferable prop on SignIn
brkalow Jul 30, 2024
97b12b9
adds changeset
brkalow Jul 30, 2024
b054afb
Add external_account_not_found to UnstableErrors type
brkalow Aug 6, 2024
7d4fc41
Adds a test case
brkalow Aug 6, 2024
9f194dc
Merge branch 'main' into brk.feat/sign-in-transferable-prop
brkalow Aug 6, 2024
8d0c50f
Merge branch 'main' into brk.feat/sign-in-transferable-prop
brkalow Aug 6, 2024
3641552
Update .changeset/gold-emus-talk.md
brkalow Aug 6, 2024
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
7 changes: 7 additions & 0 deletions .changeset/gold-emus-talk.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@clerk/clerk-js": minor
"@clerk/types": minor
"@clerk/clerk-react": minor
---

Introduce `transferable` prop for `<SignIn />` to disable the automatic transfer of a sign in attempt to a sign up attempt when attempting to sign in with a social provider when the account does not exist. Also adds a `transferable` option to `Clerk.handleRedirectCallback()` with the same functionality.
86 changes: 73 additions & 13 deletions packages/clerk-js/src/core/__tests__/clerk.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ describe('Clerk singleton', () => {
await sut.setActive({ session: null });
await waitFor(() => {
expect(mockSession.touch).not.toHaveBeenCalled();
expect(evenBusSpy).toBeCalledWith('token:update', { token: null });
expect(evenBusSpy).toHaveBeenCalledWith('token:update', { token: null });
});
});

Expand All @@ -207,7 +207,7 @@ describe('Clerk singleton', () => {
await sut.setActive({ session: mockSession as any as ActiveSessionResource });
await waitFor(() => {
expect(mockSession.touch).not.toHaveBeenCalled();
expect(mockSession.getToken).toBeCalled();
expect(mockSession.getToken).toHaveBeenCalled();
});
});

Expand Down Expand Up @@ -308,7 +308,7 @@ describe('Clerk singleton', () => {
expect(executionOrder).toEqual(['session.touch', 'set cookie', 'before emit']);
expect(mockSession2.touch).toHaveBeenCalled();
expect(mockSession2.getToken).toHaveBeenCalled();
expect(beforeEmitMock).toBeCalledWith(mockSession2);
expect(beforeEmitMock).toHaveBeenCalledWith(mockSession2);
expect(sut.session).toMatchObject(mockSession2);
});
});
Expand Down Expand Up @@ -342,7 +342,7 @@ describe('Clerk singleton', () => {
expect(mockSession.touch).toHaveBeenCalled();
expect(mockSession.getToken).toHaveBeenCalled();
expect((mockSession as any as ActiveSessionResource)?.lastActiveOrganizationId).toEqual('org_id');
expect(beforeEmitMock).toBeCalledWith(mockSession);
expect(beforeEmitMock).toHaveBeenCalledWith(mockSession);
expect(sut.session).toMatchObject(mockSession);
});
});
Expand Down Expand Up @@ -408,8 +408,8 @@ describe('Clerk singleton', () => {
expect(executionOrder).toEqual(['session.touch', 'before emit']);
expect(mockSession.touch).toHaveBeenCalled();
expect((mockSession as any as ActiveSessionResource)?.lastActiveOrganizationId).toEqual('org_id');
expect(mockSession.getToken).toBeCalled();
expect(beforeEmitMock).toBeCalledWith(mockSession);
expect(mockSession.getToken).toHaveBeenCalled();
expect(beforeEmitMock).toHaveBeenCalledWith(mockSession);
expect(sut.session).toMatchObject(mockSession);
});
});
Expand Down Expand Up @@ -619,15 +619,15 @@ describe('Clerk singleton', () => {
const toUrl = 'http://test.host/';
await sut.navigate(toUrl);
expect(mockHref).toHaveBeenCalledWith(toUrl);
expect(logSpy).not.toBeCalled();
expect(logSpy).not.toHaveBeenCalled();
});

it('uses window location if a custom navigate is defined but destination has different origin', async () => {
await sut.load(mockedLoadOptions);
const toUrl = 'https://www.origindifferent.com/';
await sut.navigate(toUrl);
expect(mockHref).toHaveBeenCalledWith(toUrl);
expect(logSpy).not.toBeCalled();
expect(logSpy).not.toHaveBeenCalled();
});

it('wraps custom navigate method in a promise if provided and it sync', async () => {
Expand All @@ -637,7 +637,7 @@ describe('Clerk singleton', () => {
expect(res.then).toBeDefined();
expect(mockHref).not.toHaveBeenCalled();
expect(mockNavigate.mock.calls[0][0]).toBe('/path#hash');
expect(logSpy).not.toBeCalled();
expect(logSpy).not.toHaveBeenCalled();
});

it('logs navigation external navigation when routerDebug is enabled', async () => {
Expand All @@ -646,8 +646,8 @@ describe('Clerk singleton', () => {
await sut.navigate(toUrl);
expect(mockHref).toHaveBeenCalledWith(toUrl);

expect(logSpy).toBeCalledTimes(1);
expect(logSpy).toBeCalledWith(`Clerk is navigating to: ${toUrl}`);
expect(logSpy).toHaveBeenCalledTimes(1);
expect(logSpy).toHaveBeenCalledWith(`Clerk is navigating to: ${toUrl}`);
});

it('logs navigation custom navigation when routerDebug is enabled', async () => {
Expand All @@ -658,8 +658,8 @@ describe('Clerk singleton', () => {
expect(mockHref).not.toHaveBeenCalled();
expect(mockNavigate.mock.calls[0][0]).toBe('/path#hash');

expect(logSpy).toBeCalledTimes(1);
expect(logSpy).toBeCalledWith(`Clerk is navigating to: ${toUrl}`);
expect(logSpy).toHaveBeenCalledTimes(1);
expect(logSpy).toHaveBeenCalledWith(`Clerk is navigating to: ${toUrl}`);
});
});

Expand Down Expand Up @@ -728,6 +728,66 @@ describe('Clerk singleton', () => {
});
});

it('does not initiate the transfer flow when transferable: false is passed', async () => {
mockEnvironmentFetch.mockReturnValue(
Promise.resolve({
authConfig: {},
userSettings: mockUserSettings,
displayConfig: mockDisplayConfig,
isSingleSession: () => false,
isProduction: () => false,
isDevelopmentOrStaging: () => true,
onWindowLocationHost: () => false,
}),
);

mockClientFetch.mockReturnValue(
Promise.resolve({
activeSessions: [],
signIn: new SignIn({
status: 'needs_identifier',
first_factor_verification: {
status: 'transferable',
strategy: 'oauth_google',
external_verification_redirect_url: '',
error: {
code: 'external_account_not_found',
long_message: 'The External Account was not found.',
message: 'Invalid external account',
},
},
second_factor_verification: null,
identifier: '',
user_data: null,
created_session_id: null,
created_user_id: null,
} as any as SignInJSON),
signUp: new SignUp(null),
}),
);

const mockSetActive = jest.fn();
const mockSignUpCreate = jest
.fn()
.mockReturnValue(Promise.resolve({ status: 'complete', createdSessionId: '123' }));

const sut = new Clerk(productionPublishableKey);
await sut.load(mockedLoadOptions);
if (!sut.client) {
fail('we should always have a client');
}
sut.client.signUp.create = mockSignUpCreate;
sut.setActive = mockSetActive;

await sut.handleRedirectCallback({ transferable: false });

await waitFor(() => {
expect(mockSignUpCreate).not.toHaveBeenCalledWith({ transfer: true });
expect(mockSetActive).not.toHaveBeenCalled();
expect(mockNavigate).toHaveBeenCalledWith('/sign-in', undefined);
});
});

it('creates a new sign up and navigates to the continue sign-up path if the user was not found during sso signup and there are missing requirements', async () => {
mockEnvironmentFetch.mockReturnValue(
Promise.resolve({
Expand Down
4 changes: 4 additions & 0 deletions packages/clerk-js/src/core/clerk.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,10 @@ export class Clerk implements ClerkInterface {
const userNeedsToBeCreated = si.firstFactorVerificationStatus === 'transferable';

if (userNeedsToBeCreated) {
if (params.transferable === false) {
return navigateToSignIn();
}

const res = await signUp.create({ transfer: true });
switch (res.status) {
case 'complete':
Expand Down
3 changes: 2 additions & 1 deletion packages/clerk-js/src/core/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ export const ERROR_CODES = {
NOT_ALLOWED_ACCESS: 'not_allowed_access',
SAML_USER_ATTRIBUTE_MISSING: 'saml_user_attribute_missing',
USER_LOCKED: 'user_locked',
};
EXTERNAL_ACCOUNT_NOT_FOUND: 'external_account_not_found',
} as const;

export const SIGN_IN_INITIAL_VALUE_KEYS = ['email_address', 'phone_number', 'username'];
export const SIGN_UP_INITIAL_VALUE_KEYS = ['email_address', 'phone_number', 'username', 'first_name', 'last_name'];
Expand Down
1 change: 1 addition & 0 deletions packages/clerk-js/src/ui/components/SignIn/SignIn.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ function SignInRoutes(): JSX.Element {
signInForceRedirectUrl={signInContext.afterSignInUrl}
signUpForceRedirectUrl={signInContext.afterSignUpUrl}
continueSignUpUrl={signInContext.signUpContinueUrl}
transferable={signInContext.transferable}
firstFactorUrl={'../factor-one'}
secondFactorUrl={'../factor-two'}
resetPasswordUrl={'../reset-password'}
Expand Down
10 changes: 8 additions & 2 deletions packages/clerk-js/src/ui/components/SignIn/SignInStart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,11 @@ export function _SignInStart(): JSX.Element {

useEffect(() => {
async function handleOauthError() {
const defaultErrorHandler = () => {
// Error from server may be too much information for the end user, so set a generic error
card.setError('Unable to complete action at this time. If the problem persists please contact support.');
};

const error = signIn?.firstFactorVerification?.error;
if (error) {
switch (error.code) {
Expand All @@ -214,12 +219,13 @@ export function _SignInStart(): JSX.Element {
case ERROR_CODES.SAML_USER_ATTRIBUTE_MISSING:
case ERROR_CODES.OAUTH_EMAIL_DOMAIN_RESERVED_BY_SAML:
case ERROR_CODES.USER_LOCKED:
case ERROR_CODES.EXTERNAL_ACCOUNT_NOT_FOUND:
card.setError(error);
break;
default:
// Error from server may be too much information for the end user, so set a generic error
card.setError('Unable to complete action at this time. If the problem persists please contact support.');
defaultErrorHandler();
}

// TODO: This is a workaround in order to reset the sign in attempt
// so that the oauth error does not persist on full page reloads.
void (await signIn.create({}));
Expand Down
2 changes: 2 additions & 0 deletions packages/clerk-js/src/ui/contexts/ClerkUIComponentsContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ export type SignInContextType = SignInCtx & {
authQueryString: string | null;
afterSignUpUrl: string;
afterSignInUrl: string;
transferable: boolean;
};

export const useSignInContext = (): SignInContextType => {
Expand Down Expand Up @@ -175,6 +176,7 @@ export const useSignInContext = (): SignInContextType => {

return {
...ctx,
transferable: ctx.transferable ?? true,
componentName,
signUpUrl,
signInUrl,
Expand Down
15 changes: 13 additions & 2 deletions packages/types/src/clerk.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,8 @@ export interface Clerk {
handleUnauthenticated: () => Promise<unknown>;
}

export type HandleOAuthCallbackParams = SignInForceRedirectUrl &
export type HandleOAuthCallbackParams = TransferableOption &
SignInForceRedirectUrl &
SignInFallbackRedirectUrl &
SignUpForceRedirectUrl &
SignUpFallbackRedirectUrl &
Expand Down Expand Up @@ -729,11 +730,21 @@ export type SignInProps = RoutingOptions & {
* Initial values that are used to prefill the sign in form.
*/
initialValues?: SignInInitialValues;
} & SignUpForceRedirectUrl &
} & TransferableOption &
SignUpForceRedirectUrl &
SignUpFallbackRedirectUrl &
LegacyRedirectProps &
AfterSignOutUrl;

interface TransferableOption {
/**
* Indicates whether or not sign in attempts are transferable to the sign up flow.
* Prevents opaque sign ups when a user attempts to sign in via OAuth with an email that doesn't exist.
* @default true
*/
transferable?: boolean;
}

export type SignInModalProps = WithoutRouting<SignInProps>;

type GoogleOneTapRedirectUrlProps = SignInForceRedirectUrl & SignUpForceRedirectUrl;
Expand Down
1 change: 1 addition & 0 deletions packages/types/src/localization.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,7 @@ type _LocalizationResource = {
type WithParamName<T> = T &
Partial<Record<`${keyof T & string}__${CamelToSnake<Exclude<FieldId, 'role'>>}`, LocalizationValue>>;
type UnstableErrors = WithParamName<{
external_account_not_found: LocalizationValue;
identification_deletion_failed: LocalizationValue;
phone_number_exists: LocalizationValue;
form_identifier_not_found: LocalizationValue;
Expand Down