Skip to content

Commit 5ebd959

Browse files
kyoshinokyoshino
committed
Fix infinite loop with local backend on certain conditions
1 parent 28e915d commit 5ebd959

3 files changed

Lines changed: 75 additions & 11 deletions

File tree

‎src/lib/components/entrance/sign-in.svelte‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
import {
1010
signInAutomatically,
1111
signInError,
12+
signingIn,
1213
signInManually,
13-
unauthenticated,
1414
} from '$lib/services/user/auth';
1515
1616
let isLocalHost = $state(false);
@@ -46,7 +46,7 @@
4646
</script>
4747
4848
<div role="none" class="buttons">
49-
{#if !$unauthenticated}
49+
{#if $signingIn}
5050
<div role="alert" class="message">{$_('signing_in')}</div>
5151
{:else if !configuredBackend}
5252
<div role="alert">

‎src/lib/services/user/auth.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ export const signInError = writable({ message: '', context: 'authentication' });
3131
*/
3232
export const unauthenticated = writable(true);
3333

34+
/**
35+
* @type {Writable<boolean>}
36+
*/
37+
export const signingIn = writable(false);
38+
3439
/**
3540
* Reset the sign-in error store.
3641
*/
@@ -136,7 +141,7 @@ export const signInAutomatically = async () => {
136141

137142
const { token, refreshToken } = _user;
138143

139-
unauthenticated.set(false);
144+
signingIn.set(true);
140145

141146
try {
142147
_user = await _backend.signIn({ token, refreshToken, auto: true });
@@ -147,6 +152,7 @@ export const signInAutomatically = async () => {
147152
}
148153
}
149154

155+
signingIn.set(false);
150156
unauthenticated.set(!_user);
151157

152158
if (!_user || !_backend) {
@@ -192,11 +198,12 @@ export const signInManually = async (_backendName, token) => {
192198

193199
let _user;
194200

195-
unauthenticated.set(false);
201+
signingIn.set(true);
196202

197203
try {
198204
_user = await _backend.signIn({ token, auto: false });
199205
} catch (/** @type {any} */ ex) {
206+
signingIn.set(false);
200207
unauthenticated.set(true);
201208

202209
if (!!token && ex.cause?.status === 401) {
@@ -212,6 +219,7 @@ export const signInManually = async (_backendName, token) => {
212219
return;
213220
}
214221

222+
signingIn.set(false);
215223
unauthenticated.set(!_user);
216224

217225
if (!_user) {

‎src/lib/services/user/auth.test.js‎

Lines changed: 63 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,11 @@ describe('auth service', () => {
116116

117117
// Import the module after mocks are set up
118118
authModule = await import('./auth.js');
119+
120+
// Spy on the exported stores rather than reassigning them
121+
vi.spyOn(authModule.signInError, 'set');
122+
vi.spyOn(authModule.unauthenticated, 'set');
123+
vi.spyOn(authModule.signingIn, 'set');
119124
});
120125

121126
describe('resetError', () => {
@@ -229,9 +234,36 @@ describe('auth service', () => {
229234
await authModule.signInAutomatically();
230235

231236
expect(mockBackend.signIn).not.toHaveBeenCalled();
237+
// Should not call unauthenticated.set because function returns early
238+
expect(authModule.unauthenticated.set).not.toHaveBeenCalled();
232239
});
233240

234-
it('should sign in with cached user data', async () => {
241+
it('should check multiple cache sources', async () => {
242+
mockLocalStorage.get
243+
.mockResolvedValueOnce(null) // sveltia-cms.user
244+
.mockResolvedValueOnce(null) // decap-cms-user
245+
.mockResolvedValueOnce({ token: 'netlify-token', backendName: 'github' }); // netlify-cms-user
246+
247+
mockGet.mockImplementation((store) => {
248+
if (store === mockBackendStore) return mockBackend;
249+
if (store === mockSiteConfigStore) return mockSiteConfig;
250+
if (store === mockGetLocaleText) return mockGetLocaleText;
251+
252+
return mockSiteConfig;
253+
});
254+
mockBackend.signIn.mockResolvedValue({ token: 'netlify-token' });
255+
mockBackend.fetchFiles.mockResolvedValue(undefined);
256+
257+
await authModule.signInAutomatically();
258+
259+
expect(mockLocalStorage.get).toHaveBeenCalledWith('sveltia-cms.user');
260+
expect(mockLocalStorage.get).toHaveBeenCalledWith('decap-cms-user');
261+
expect(mockLocalStorage.get).toHaveBeenCalledWith('netlify-cms-user');
262+
expect(authModule.signingIn.set).toHaveBeenCalledWith(true);
263+
expect(authModule.signingIn.set).toHaveBeenCalledWith(false);
264+
});
265+
266+
it('should sign in with cached user data and set signingIn state', async () => {
235267
const cachedUser = {
236268
token: 'test-token',
237269
backendName: 'github',
@@ -254,11 +286,16 @@ describe('auth service', () => {
254286

255287
await authModule.signInAutomatically();
256288

289+
expect(mockUser.set).toHaveBeenCalledWith(cachedUser); // Set before sign-in
290+
expect(authModule.signingIn.set).toHaveBeenCalledWith(true);
257291
expect(mockBackend.signIn).toHaveBeenCalledWith({
258292
token: 'test-token',
259293
refreshToken: undefined,
260294
auto: true,
261295
});
296+
expect(authModule.signingIn.set).toHaveBeenCalledWith(false);
297+
expect(authModule.unauthenticated.set).toHaveBeenCalledWith(false);
298+
expect(mockUser.set).toHaveBeenCalledWith(cachedUser); // Set after sign-in
262299
expect(mockBackend.fetchFiles).toHaveBeenCalled();
263300
});
264301

@@ -288,15 +325,17 @@ describe('auth service', () => {
288325
await authModule.signInAutomatically();
289326

290327
expect(mockGoto).toHaveBeenCalledWith('', { replaceState: true });
328+
expect(authModule.signingIn.set).toHaveBeenCalledWith(true);
291329
expect(mockBackend.signIn).toHaveBeenCalledWith({
292330
token: 'qr-token',
293331
refreshToken: undefined,
294332
auto: true,
295333
});
334+
expect(authModule.signingIn.set).toHaveBeenCalledWith(false);
296335
expect(mockPrefs.update).toHaveBeenCalled();
297336
});
298337

299-
it('should handle sign in failure', async () => {
338+
it('should handle sign in failure gracefully', async () => {
300339
const cachedUser = { token: 'test-token', backendName: 'github' };
301340

302341
mockLocalStorage.get.mockResolvedValue(cachedUser);
@@ -311,6 +350,10 @@ describe('auth service', () => {
311350

312351
await authModule.signInAutomatically();
313352

353+
expect(mockUser.set).toHaveBeenCalledWith(cachedUser); // Set before sign-in
354+
expect(authModule.signingIn.set).toHaveBeenCalledWith(true);
355+
expect(authModule.signingIn.set).toHaveBeenCalledWith(false);
356+
expect(mockUser.set).toHaveBeenCalledWith(undefined); // Reset after failure
314357
expect(authModule.unauthenticated.set).toHaveBeenCalledWith(true);
315358
});
316359

@@ -362,7 +405,7 @@ describe('auth service', () => {
362405
});
363406

364407
describe('signInManually', () => {
365-
it('should sign in with provided credentials', async () => {
408+
it('should sign in with provided credentials and set signingIn state', async () => {
366409
const user = { token: 'manual-token' };
367410

368411
mockGet.mockReturnValue(mockBackend);
@@ -371,15 +414,18 @@ describe('auth service', () => {
371414

372415
await authModule.signInManually('github', 'manual-token');
373416

417+
expect(authModule.signingIn.set).toHaveBeenCalledWith(true);
374418
expect(mockBackend.signIn).toHaveBeenCalledWith({
375419
token: 'manual-token',
376420
auto: false,
377421
});
422+
expect(authModule.signingIn.set).toHaveBeenCalledWith(false);
423+
expect(authModule.unauthenticated.set).toHaveBeenCalledWith(false);
378424
expect(mockUser.set).toHaveBeenCalledWith(user);
379425
expect(mockBackend.fetchFiles).toHaveBeenCalled();
380426
});
381427

382-
it('should handle sign in failure', async () => {
428+
it('should handle sign in failure and set signingIn state', async () => {
383429
mockGet.mockImplementation((store) => {
384430
if (store === mockBackend) return mockBackend;
385431
if (store === mockGetLocaleText) return mockGetLocaleText;
@@ -394,6 +440,8 @@ describe('auth service', () => {
394440

395441
await authModule.signInManually('github', 'invalid-token');
396442

443+
expect(authModule.signingIn.set).toHaveBeenCalledWith(true);
444+
expect(authModule.signingIn.set).toHaveBeenCalledWith(false);
397445
expect(authModule.unauthenticated.set).toHaveBeenCalledWith(true);
398446
expect(authModule.signInError.set).toHaveBeenCalled();
399447
});
@@ -415,6 +463,8 @@ describe('auth service', () => {
415463

416464
await authModule.signInManually('github', 'invalid-pat-token');
417465

466+
expect(authModule.signingIn.set).toHaveBeenCalledWith(true);
467+
expect(authModule.signingIn.set).toHaveBeenCalledWith(false);
418468
expect(authModule.unauthenticated.set).toHaveBeenCalledWith(true);
419469
expect(authModule.signInError.set).toHaveBeenCalledWith({
420470
message: 'The provided token is invalid',
@@ -428,17 +478,17 @@ describe('auth service', () => {
428478
await authModule.signInManually('github', 'token');
429479

430480
expect(mockBackend.signIn).not.toHaveBeenCalled();
481+
expect(authModule.signingIn.set).not.toHaveBeenCalled();
431482
});
432483

433484
it('should handle fetch files failure', async () => {
434485
const user = { token: 'manual-token' };
435486

436487
mockGet.mockImplementation((store) => {
437-
if (store === mockBackend) return mockBackend;
488+
if (store === mockBackendStore) return mockBackend;
438489
if (store === mockGetLocaleText) return mockGetLocaleText;
439-
if (store && typeof store.subscribe === 'function') return mockSiteConfig;
440490

441-
return mockBackend;
491+
return mockSiteConfig;
442492
});
443493
mockBackend.signIn.mockResolvedValue(user);
444494

@@ -448,6 +498,11 @@ describe('auth service', () => {
448498

449499
await authModule.signInManually('github', 'manual-token');
450500

501+
expect(authModule.signingIn.set).toHaveBeenCalledWith(true);
502+
expect(authModule.signingIn.set).toHaveBeenCalledWith(false);
503+
// User is still authenticated even if fetchFiles fails
504+
expect(authModule.unauthenticated.set).toHaveBeenCalledWith(false);
505+
expect(mockUser.set).toHaveBeenCalledWith(user);
451506
expect(authModule.signInError.set).toHaveBeenCalled();
452507
});
453508
});
@@ -461,6 +516,7 @@ describe('auth service', () => {
461516

462517
expect(mockBackend.signOut).toHaveBeenCalled();
463518
expect(mockLocalStorage.set).toHaveBeenCalledWith('sveltia-cms.user', {});
519+
expect(mockBackendName.set).toHaveBeenCalledWith(undefined);
464520
expect(mockUser.set).toHaveBeenCalledWith(undefined);
465521
expect(authModule.unauthenticated.set).toHaveBeenCalledWith(true);
466522
expect(mockDataLoaded.set).toHaveBeenCalledWith(false);

0 commit comments

Comments
 (0)