Skip to content

Commit fb9eb0f

Browse files
committed
feat: Improve error handling and storage cleanup for Firebase UI authentication
1 parent 67f690b commit fb9eb0f

File tree

4 files changed

+202
-19
lines changed

4 files changed

+202
-19
lines changed

packages/firebaseui-core/src/auth.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,9 +266,11 @@ export async function fuiCompleteEmailLinkSignIn(
266266
if (!email) return null;
267267

268268
const result = await fuiSignInWithEmailLink(auth, email, currentUrl, opts);
269-
window.localStorage.removeItem('emailForSignIn');
270269
return handlePendingCredential(result);
271270
} catch (error) {
272271
return handleFirebaseError(error, opts);
272+
} finally {
273+
window.localStorage.removeItem('emailForSignIn');
274+
window.localStorage.removeItem('emailLinkAnonymousUpgrade');
273275
}
274276
}

packages/firebaseui-core/tests/unit/auth.test.ts

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import {
2828
fuiSignInWithOAuth,
2929
fuiCompleteEmailLinkSignIn,
3030
} from '../../src/auth';
31-
import { FirebaseUIError } from '../../src/errors';
3231

3332
// Mock all Firebase Auth functions
3433
vi.mock('firebase/auth', async () => {
@@ -69,6 +68,7 @@ describe('Firebase UI Auth', () => {
6968
vi.clearAllMocks();
7069
mockAuth = { currentUser: null } as Auth;
7170
window.localStorage.clear();
71+
window.sessionStorage.clear();
7272
(EmailAuthProvider.credential as any).mockReturnValue(mockCredential);
7373
(EmailAuthProvider.credentialWithLink as any).mockReturnValue(mockCredential);
7474
(PhoneAuthProvider.credential as any).mockReturnValue(mockCredential);
@@ -338,5 +338,85 @@ describe('Firebase UI Auth', () => {
338338

339339
expect(result).toBeNull();
340340
});
341+
342+
it('should clean up storage even when sign in fails', async () => {
343+
window.localStorage.setItem('emailForSignIn', '[email protected]');
344+
window.localStorage.setItem('emailLinkAnonymousUpgrade', 'true');
345+
346+
(isSignInWithEmailLink as any).mockReturnValue(true);
347+
(signInWithCredential as any).mockRejectedValue(new Error('Sign in failed'));
348+
349+
await expect(fuiCompleteEmailLinkSignIn(mockAuth, 'mock-url')).rejects.toThrow();
350+
351+
expect(window.localStorage.getItem('emailForSignIn')).toBeNull();
352+
expect(window.localStorage.getItem('emailLinkAnonymousUpgrade')).toBeNull();
353+
});
354+
});
355+
356+
describe('Pending Credential Handling', () => {
357+
it('should handle pending credential during email sign in', async () => {
358+
const storedCred = { type: 'google.com', token: 'stored-token' };
359+
window.sessionStorage.setItem('pendingCred', JSON.stringify(storedCred));
360+
(signInWithCredential as any).mockResolvedValue(mockUserCredential);
361+
(linkWithCredential as any).mockResolvedValue({ ...mockUserCredential, user: { uid: 'linked-uid' } });
362+
363+
const result = await fuiSignInWithEmailAndPassword(mockAuth, '[email protected]', 'password');
364+
365+
expect(linkWithCredential).toHaveBeenCalledWith(mockUserCredential.user, storedCred);
366+
expect(window.sessionStorage.getItem('pendingCred')).toBeNull();
367+
expect(result.user.uid).toBe('linked-uid');
368+
});
369+
370+
it('should handle invalid pending credential gracefully', async () => {
371+
window.sessionStorage.setItem('pendingCred', 'invalid-json');
372+
(signInWithCredential as any).mockResolvedValue(mockUserCredential);
373+
374+
const result = await fuiSignInWithEmailAndPassword(mockAuth, '[email protected]', 'password');
375+
376+
expect(result).toBe(mockUserCredential);
377+
expect(window.sessionStorage.getItem('pendingCred')).toBeNull();
378+
});
379+
380+
it('should handle linking failure gracefully', async () => {
381+
const storedCred = { type: 'google.com', token: 'stored-token' };
382+
window.sessionStorage.setItem('pendingCred', JSON.stringify(storedCred));
383+
(signInWithCredential as any).mockResolvedValue(mockUserCredential);
384+
(linkWithCredential as any).mockRejectedValue(new Error('Linking failed'));
385+
386+
const result = await fuiSignInWithEmailAndPassword(mockAuth, '[email protected]', 'password');
387+
388+
expect(result).toBe(mockUserCredential);
389+
expect(window.sessionStorage.getItem('pendingCred')).toBeNull();
390+
});
391+
});
392+
393+
describe('Storage Management', () => {
394+
it('should clean up all storage items after successful email link sign in', async () => {
395+
window.localStorage.setItem('emailForSignIn', '[email protected]');
396+
window.localStorage.setItem('emailLinkAnonymousUpgrade', 'true');
397+
window.sessionStorage.setItem('pendingCred', JSON.stringify(mockCredential));
398+
399+
(isSignInWithEmailLink as any).mockReturnValue(true);
400+
(signInWithCredential as any).mockResolvedValue(mockUserCredential);
401+
402+
await fuiCompleteEmailLinkSignIn(mockAuth, 'mock-url');
403+
404+
expect(window.localStorage.getItem('emailForSignIn')).toBeNull();
405+
expect(window.localStorage.getItem('emailLinkAnonymousUpgrade')).toBeNull();
406+
expect(window.sessionStorage.getItem('pendingCred')).toBeNull();
407+
});
408+
409+
it('should clean up storage even when sign in fails', async () => {
410+
window.localStorage.setItem('emailForSignIn', '[email protected]');
411+
window.localStorage.setItem('emailLinkAnonymousUpgrade', 'true');
412+
413+
(isSignInWithEmailLink as any).mockReturnValue(true);
414+
(signInWithCredential as any).mockRejectedValue(new Error('Sign in failed'));
415+
416+
await expect(fuiCompleteEmailLinkSignIn(mockAuth, 'mock-url')).rejects.toThrow();
417+
418+
expect(window.localStorage.getItem('emailForSignIn')).toBeNull();
419+
expect(window.localStorage.getItem('emailLinkAnonymousUpgrade')).toBeNull();
420+
});
341421
});
342422
});

packages/firebaseui-core/tests/unit/config.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ describe('Config', () => {
3131
const config: FUIConfig = {
3232
app: mockApp,
3333
enableAutoAnonymousLogin: false,
34+
enableHandleExistingCredential: false,
3435
};
3536
const store = initializeUI(config);
3637
expect(store.get()).toEqual(config);
@@ -46,6 +47,7 @@ describe('Config', () => {
4647
const config: FUIConfig = {
4748
app: mockApp,
4849
enableAutoAnonymousLogin: false,
50+
enableHandleExistingCredential: true,
4951
};
5052
const store = initializeUI(config, 'custom');
5153
expect(store.get()).toEqual(config);
@@ -61,6 +63,7 @@ describe('Config', () => {
6163
const config: FUIConfig = {
6264
app: mockApp,
6365
enableAutoAnonymousLogin: true,
66+
enableHandleExistingCredential: false,
6467
};
6568
initializeUI(config);
6669
expect(onAuthStateChanged).toHaveBeenCalled();
@@ -78,10 +81,27 @@ describe('Config', () => {
7881
const config: FUIConfig = {
7982
app: mockApp,
8083
enableAutoAnonymousLogin: false,
84+
enableHandleExistingCredential: true,
8185
};
8286
initializeUI(config);
8387
expect(onAuthStateChanged).not.toHaveBeenCalled();
8488
});
89+
90+
it('should handle both auto features being enabled', () => {
91+
const mockApp = {
92+
name: 'test',
93+
options: {},
94+
automaticDataCollectionEnabled: false,
95+
} as FirebaseApp;
96+
const config: FUIConfig = {
97+
app: mockApp,
98+
enableAutoAnonymousLogin: true,
99+
enableHandleExistingCredential: true,
100+
};
101+
const store = initializeUI(config);
102+
expect(store.get()).toEqual(config);
103+
expect(onAuthStateChanged).toHaveBeenCalled();
104+
});
85105
});
86106

87107
describe('getTranslations', () => {

packages/firebaseui-core/tests/unit/errors.test.ts

Lines changed: 98 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, it, expect } from 'vitest';
1+
import { describe, it, expect, beforeEach } from 'vitest';
22
import { FirebaseUIError, handleFirebaseError } from '../../src/errors';
33

44
describe('FirebaseUIError', () => {
@@ -64,27 +64,26 @@ describe('FirebaseUIError', () => {
6464
});
6565

6666
describe('handleFirebaseError', () => {
67-
it('should throw FirebaseUIError for Firebase errors', () => {
67+
beforeEach(() => {
68+
window.sessionStorage.clear();
69+
});
70+
71+
it('should throw FirebaseUIError for Firebase errors', async () => {
6872
const firebaseError = {
6973
name: 'FirebaseError',
7074
code: 'auth/user-not-found',
7175
};
7276

73-
expect(() => handleFirebaseError(firebaseError)).toThrow(FirebaseUIError);
77+
await expect(handleFirebaseError(firebaseError)).rejects.toThrow(FirebaseUIError);
7478
});
7579

76-
it('should throw FirebaseUIError with unknown code for non-Firebase errors', () => {
80+
it('should throw FirebaseUIError with unknown code for non-Firebase errors', async () => {
7781
const error = new Error('Random error');
7882

79-
try {
80-
handleFirebaseError(error);
81-
} catch (e) {
82-
expect(e).toBeInstanceOf(FirebaseUIError);
83-
expect(e.code).toBe('unknown');
84-
}
83+
await expect(handleFirebaseError(error)).rejects.toThrow(FirebaseUIError);
8584
});
8685

87-
it('should pass translations to FirebaseUIError', () => {
86+
it('should pass translations and language to FirebaseUIError', async () => {
8887
const firebaseError = {
8988
name: 'FirebaseError',
9089
code: 'auth/user-not-found',
@@ -99,29 +98,111 @@ describe('handleFirebaseError', () => {
9998
};
10099

101100
try {
102-
handleFirebaseError(firebaseError, translations, 'es');
101+
await handleFirebaseError(firebaseError, { translations, language: 'es' });
103102
} catch (e) {
104103
expect(e).toBeInstanceOf(FirebaseUIError);
105104
expect(e.message).toBe('Usuario no encontrado');
106105
}
107106
});
108107

109-
it('should handle null/undefined errors', () => {
110-
expect(() => handleFirebaseError(null)).toThrow(FirebaseUIError);
111-
expect(() => handleFirebaseError(undefined)).toThrow(FirebaseUIError);
108+
it('should handle null/undefined errors', async () => {
109+
await expect(handleFirebaseError(null)).rejects.toThrow(FirebaseUIError);
110+
await expect(handleFirebaseError(undefined)).rejects.toThrow(FirebaseUIError);
112111
});
113112

114-
it('should preserve the error code in thrown error', () => {
113+
it('should preserve the error code in thrown error', async () => {
115114
const firebaseError = {
116115
name: 'FirebaseError',
117116
code: 'auth/wrong-password',
118117
};
119118

120119
try {
121-
handleFirebaseError(firebaseError);
120+
await handleFirebaseError(firebaseError);
122121
} catch (e) {
123122
expect(e).toBeInstanceOf(FirebaseUIError);
124123
expect(e.code).toBe('auth/wrong-password');
125124
}
126125
});
126+
127+
describe('account exists with different credential handling', () => {
128+
beforeEach(() => {
129+
window.sessionStorage.clear();
130+
});
131+
132+
it('should store credential and throw error when enableHandleExistingCredential is true', async () => {
133+
const existingCredentialError = {
134+
name: 'FirebaseError',
135+
code: 'auth/account-exists-with-different-credential',
136+
credential: { type: 'google.com', token: 'mock-token' },
137+
customData: {
138+
139+
},
140+
};
141+
142+
await expect(
143+
handleFirebaseError(existingCredentialError, { enableHandleExistingCredential: true })
144+
).rejects.toThrow(FirebaseUIError);
145+
expect(window.sessionStorage.getItem('pendingCred')).toBe(JSON.stringify(existingCredentialError.credential));
146+
});
147+
148+
it('should not store credential when enableHandleExistingCredential is false', async () => {
149+
const existingCredentialError = {
150+
name: 'FirebaseError',
151+
code: 'auth/account-exists-with-different-credential',
152+
credential: { type: 'google.com', token: 'mock-token' },
153+
customData: {
154+
155+
},
156+
};
157+
158+
await expect(handleFirebaseError(existingCredentialError)).rejects.toThrow(FirebaseUIError);
159+
expect(window.sessionStorage.getItem('pendingCred')).toBeNull();
160+
});
161+
162+
it('should not store credential when no credential in error', async () => {
163+
const existingCredentialError = {
164+
name: 'FirebaseError',
165+
code: 'auth/account-exists-with-different-credential',
166+
customData: {
167+
168+
},
169+
};
170+
171+
await expect(
172+
handleFirebaseError(existingCredentialError, { enableHandleExistingCredential: true })
173+
).rejects.toThrow(FirebaseUIError);
174+
expect(window.sessionStorage.getItem('pendingCred')).toBeNull();
175+
});
176+
177+
it('should include email in error and use translations when provided', async () => {
178+
const existingCredentialError = {
179+
name: 'FirebaseError',
180+
code: 'auth/account-exists-with-different-credential',
181+
credential: { type: 'google.com', token: 'mock-token' },
182+
customData: {
183+
184+
},
185+
};
186+
187+
const translations = {
188+
es: {
189+
errors: {
190+
accountExistsWithDifferentCredential: 'La cuenta ya existe con otras credenciales',
191+
},
192+
},
193+
};
194+
195+
try {
196+
await handleFirebaseError(existingCredentialError, {
197+
enableHandleExistingCredential: true,
198+
translations,
199+
language: 'es',
200+
});
201+
} catch (e) {
202+
expect(e).toBeInstanceOf(FirebaseUIError);
203+
expect(e.code).toBe('auth/account-exists-with-different-credential');
204+
expect(e.message).toBe('La cuenta ya existe con otras credenciales');
205+
}
206+
});
207+
});
127208
});

0 commit comments

Comments
 (0)