diff --git a/.changeset/fix-expo-native-oauth-name.md b/.changeset/fix-expo-native-oauth-name.md new file mode 100644 index 00000000000..4bd6cc6196b --- /dev/null +++ b/.changeset/fix-expo-native-oauth-name.md @@ -0,0 +1,7 @@ +--- +'@clerk/expo': patch +--- + +Fix `useSignInWithApple` dropping the user's first and last name on native sign-up. The hook now signs up first with the name from the Apple credential and falls back to sign-in when the account already exists, including on instances with restricted or waitlist sign-up mode. + +Note: Apple only shares the name on the app's first authorization. To verify the fix with an Apple ID that has already signed in to your app, delete the user in the Clerk Dashboard and remove the app's entry under Settings > Sign in with Apple on the device, then sign in again. diff --git a/packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts b/packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts index c918d2ee07d..e5f6d4db55e 100644 --- a/packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts +++ b/packages/expo/src/hooks/__tests__/useSignInWithApple.test.ts @@ -1,3 +1,4 @@ +import { ClerkAPIResponseError } from '@clerk/shared/error'; import { renderHook } from '@testing-library/react'; import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; @@ -52,14 +53,16 @@ describe('useSignInWithApple', () => { const mockSignIn = { create: vi.fn(), createdSessionId: 'test-session-id', - firstFactorVerification: { - status: 'verified', - }, }; const mockSignUp = { create: vi.fn(), - createdSessionId: null, + createdSessionId: 'new-user-session-id', + verifications: { + externalAccount: { + status: 'unverified', + }, + }, }; const mockSetActive = vi.fn(); @@ -94,19 +97,25 @@ describe('useSignInWithApple', () => { expect(typeof result.current.startAppleAuthenticationFlow).toBe('function'); }); - test('should successfully sign in existing user', async () => { + test('should sign up a new user and pass through their name', async () => { const mockIdentityToken = 'mock-identity-token'; mocks.signInAsync.mockResolvedValue({ identityToken: mockIdentityToken, + fullName: { + givenName: 'Jane', + familyName: 'Appleseed', + }, }); - mockSignIn.create.mockResolvedValue(undefined); - mockSignIn.firstFactorVerification.status = 'verified'; - mockSignIn.createdSessionId = 'test-session-id'; + mockSignUp.create.mockResolvedValue(undefined); + mockSignUp.verifications.externalAccount.status = 'unverified'; + mockSignUp.createdSessionId = 'new-user-session-id'; const { result } = renderHook(() => useSignInWithApple()); - const response = await result.current.startAppleAuthenticationFlow(); + const response = await result.current.startAppleAuthenticationFlow({ + unsafeMetadata: { source: 'test' }, + }); expect(mocks.isAvailableAsync).toHaveBeenCalled(); expect(mocks.randomUUID).toHaveBeenCalled(); @@ -116,44 +125,169 @@ describe('useSignInWithApple', () => { nonce: 'test-nonce-uuid', }), ); - expect(mockSignIn.create).toHaveBeenCalledWith({ + expect(mockSignUp.create).toHaveBeenCalledWith({ strategy: 'oauth_token_apple', token: mockIdentityToken, + firstName: 'Jane', + lastName: 'Appleseed', + unsafeMetadata: { source: 'test' }, }); - expect(response.createdSessionId).toBe('test-session-id'); + expect(mockSignIn.create).not.toHaveBeenCalled(); + expect(response.createdSessionId).toBe('new-user-session-id'); expect(response.setActive).toBe(mockSetActive); }); - test('should handle transfer flow for new user', async () => { + test('should transfer to sign-in when the account already exists', async () => { const mockIdentityToken = 'mock-identity-token'; mocks.signInAsync.mockResolvedValue({ identityToken: mockIdentityToken, + fullName: { + givenName: 'Jane', + familyName: 'Appleseed', + }, }); - mockSignIn.create.mockResolvedValue(undefined); - mockSignIn.firstFactorVerification.status = 'transferable'; + mockSignUp.create.mockResolvedValue(undefined); + mockSignUp.verifications.externalAccount.status = 'transferable'; - const mockSignUpWithSession = { ...mockSignUp, createdSessionId: 'new-user-session-id' }; - mocks.useSignUp.mockReturnValue({ - signUp: mockSignUpWithSession, + const mockSignInWithSession = { ...mockSignIn, createdSessionId: 'existing-user-session-id' }; + mocks.useSignIn.mockReturnValue({ + signIn: mockSignInWithSession, + setActive: mockSetActive, isLoaded: true, }); const { result } = renderHook(() => useSignInWithApple()); - const response = await result.current.startAppleAuthenticationFlow({ - unsafeMetadata: { source: 'test' }, - }); + const response = await result.current.startAppleAuthenticationFlow(); - expect(mockSignIn.create).toHaveBeenCalledWith({ + expect(mockSignUp.create).toHaveBeenCalledWith({ strategy: 'oauth_token_apple', token: mockIdentityToken, + firstName: 'Jane', + lastName: 'Appleseed', + unsafeMetadata: undefined, }); - expect(mockSignUp.create).toHaveBeenCalledWith({ + expect(mockSignInWithSession.create).toHaveBeenCalledWith({ transfer: true, - unsafeMetadata: { source: 'test' }, }); - expect(response.createdSessionId).toBe('new-user-session-id'); + expect(response.createdSessionId).toBe('existing-user-session-id'); + }); + + test('should handle a credential without a name (returning user)', async () => { + const mockIdentityToken = 'mock-identity-token'; + mocks.signInAsync.mockResolvedValue({ + identityToken: mockIdentityToken, + fullName: null, + }); + + mockSignUp.create.mockResolvedValue(undefined); + mockSignUp.verifications.externalAccount.status = 'transferable'; + + const mockSignInWithSession = { ...mockSignIn, createdSessionId: 'existing-user-session-id' }; + mocks.useSignIn.mockReturnValue({ + signIn: mockSignInWithSession, + setActive: mockSetActive, + isLoaded: true, + }); + + const { result } = renderHook(() => useSignInWithApple()); + + const response = await result.current.startAppleAuthenticationFlow(); + + expect(mockSignUp.create).toHaveBeenCalledWith({ + strategy: 'oauth_token_apple', + token: mockIdentityToken, + firstName: undefined, + lastName: undefined, + unsafeMetadata: undefined, + }); + expect(response.createdSessionId).toBe('existing-user-session-id'); + }); + + test('should fall back to sign-in when the instance restricts sign-ups', async () => { + const mockIdentityToken = 'mock-identity-token'; + mocks.signInAsync.mockResolvedValue({ + identityToken: mockIdentityToken, + fullName: null, + }); + + mockSignUp.create.mockRejectedValue( + new ClerkAPIResponseError('Sign-ups restricted', { + data: [{ code: 'sign_up_mode_restricted', message: 'Sign-ups restricted' }], + status: 403, + }), + ); + + const mockSignInWithSession = { + ...mockSignIn, + createdSessionId: 'existing-user-session-id', + firstFactorVerification: { status: 'verified' }, + }; + mocks.useSignIn.mockReturnValue({ + signIn: mockSignInWithSession, + setActive: mockSetActive, + isLoaded: true, + }); + + const { result } = renderHook(() => useSignInWithApple()); + + const response = await result.current.startAppleAuthenticationFlow(); + + expect(mockSignInWithSession.create).toHaveBeenCalledWith({ + strategy: 'oauth_token_apple', + token: mockIdentityToken, + }); + expect(response.createdSessionId).toBe('existing-user-session-id'); + expect(response.setActive).toBe(mockSetActive); + }); + + test('should surface the restriction error for a new user when sign-ups are restricted', async () => { + mocks.signInAsync.mockResolvedValue({ + identityToken: 'mock-identity-token', + fullName: null, + }); + + mockSignUp.create.mockRejectedValue( + new ClerkAPIResponseError('Sign-ups restricted', { + data: [{ code: 'sign_up_mode_restricted', message: 'Sign-ups restricted' }], + status: 403, + }), + ); + + const mockSignInTransferable = { + ...mockSignIn, + createdSessionId: null, + firstFactorVerification: { status: 'transferable' }, + }; + mocks.useSignIn.mockReturnValue({ + signIn: mockSignInTransferable, + setActive: mockSetActive, + isLoaded: true, + }); + + const { result } = renderHook(() => useSignInWithApple()); + + await expect(result.current.startAppleAuthenticationFlow()).rejects.toThrow('Sign-ups restricted'); + }); + + test('should rethrow sign-up errors that are not sign-up restrictions', async () => { + mocks.signInAsync.mockResolvedValue({ + identityToken: 'mock-identity-token', + fullName: null, + }); + + mockSignUp.create.mockRejectedValue( + new ClerkAPIResponseError('Invalid token', { + data: [{ code: 'oauth_token_invalid', message: 'Invalid token' }], + status: 422, + }), + ); + + const { result } = renderHook(() => useSignInWithApple()); + + await expect(result.current.startAppleAuthenticationFlow()).rejects.toThrow('Invalid token'); + expect(mockSignIn.create).not.toHaveBeenCalled(); }); test('should handle user cancellation gracefully', async () => { diff --git a/packages/expo/src/hooks/useSignInWithApple.ios.ts b/packages/expo/src/hooks/useSignInWithApple.ios.ts index 76ae982d687..4dccb714f6b 100644 --- a/packages/expo/src/hooks/useSignInWithApple.ios.ts +++ b/packages/expo/src/hooks/useSignInWithApple.ios.ts @@ -1,4 +1,5 @@ import { useSignIn, useSignUp } from '@clerk/react/legacy'; +import { isClerkAPIResponseError } from '@clerk/shared/error'; import type { SetActive, SignInResource, SignUpResource } from '@clerk/shared/types'; import { errorThrower } from '../utils/errors'; @@ -99,40 +100,68 @@ export function useSignInWithApple() { nonce, }); - // Extract the identity token from the credential - const { identityToken } = credential; + // Apple only returns the name on first authorization, so capture it now before it's lost. + const { identityToken, fullName } = credential; if (!identityToken) { return errorThrower.throw('No identity token received from Apple Sign-In.'); } - // Create a SignIn with the Apple ID token strategy - await signIn.create({ - strategy: 'oauth_token_apple', - token: identityToken, - }); + try { + // Sign-up first so a new user's name is recorded; an existing account resolves as transferable below. + await signUp.create({ + strategy: 'oauth_token_apple', + token: identityToken, + firstName: fullName?.givenName ?? undefined, + lastName: fullName?.familyName ?? undefined, + unsafeMetadata: startAppleAuthenticationFlowParams?.unsafeMetadata, + }); + } catch (signUpError: unknown) { + // Restricted/waitlist instances reject sign-up before the account-exists check, so existing users sign in. + if ( + isClerkAPIResponseError(signUpError) && + signUpError.errors?.some( + err => err.code === 'sign_up_mode_restricted' || err.code === 'sign_up_restricted_waitlist', + ) + ) { + await signIn.create({ + strategy: 'oauth_token_apple', + token: identityToken, + }); + + // A transferable sign-in means the user doesn't exist; surface the original restriction error. + if (signIn.firstFactorVerification.status === 'transferable') { + throw signUpError; + } + + return { + createdSessionId: signIn.createdSessionId, + setActive, + signIn, + signUp, + }; + } + + throw signUpError; + } - // Check if we need to transfer to SignUp (user doesn't exist yet) - const userNeedsToBeCreated = signIn.firstFactorVerification.status === 'transferable'; + const accountAlreadyExists = signUp.verifications.externalAccount.status === 'transferable'; - if (userNeedsToBeCreated) { - // User doesn't exist - create a new SignUp with transfer - await signUp.create({ + if (accountAlreadyExists) { + await signIn.create({ transfer: true, - unsafeMetadata: startAppleAuthenticationFlowParams?.unsafeMetadata, }); return { - createdSessionId: signUp.createdSessionId, + createdSessionId: signIn.createdSessionId, setActive, signIn, signUp, }; } - // User exists - return the SignIn session return { - createdSessionId: signIn.createdSessionId, + createdSessionId: signUp.createdSessionId, setActive, signIn, signUp,