Explorar o código

fix(auth): don't discard a valid stored token on a transient load-time error (#1889)

On mount, AuthContext.checkAuthStatus restores the persisted "Remember Me"
token from localStorage and validates it via GET /auth/me. The catch around
that call cleared the token on ANY failure, not just a definitive 401
invalid-token — so a brief backend-not-ready or reverse-proxy hiccup during
page load (plausible right after a container restart, e.g. on Unraid) would
delete a still-valid token. Because the token was deleted, a reload couldn't
recover it and the user was bounced to the login screen.

Token validation now retries transient failures (up to 3 attempts with short
backoff) and only discards the token on a definitive 401 — which request()
already handles (clears the token and dispatches auth:expired). Transient /
5xx / network errors leave the persisted token intact so the session survives
a slow load. "Remember Me" stays client-storage only; it does not extend the
server-side JWT lifetime (session_max_hours, default 24h).

Adds AuthContext tests: transient /auth/me failure keeps the token, a
definitive 401 clears it, and a valid token loads the user. Rebuilt frontend
bundle.
maziggy hai 2 meses
pai
achega
646a8b13fd

A diferenza do arquivo foi suprimida porque é demasiado grande
+ 1 - 0
CHANGELOG.md


+ 74 - 2
frontend/src/__tests__/contexts/AuthContext.test.tsx

@@ -2,7 +2,7 @@
  * Tests for the AuthContext permission helpers.
  */
 
-import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
 import { act, renderHook, waitFor } from '@testing-library/react';
 import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
 import { BrowserRouter } from 'react-router-dom';
@@ -11,7 +11,7 @@ import { server } from '../mocks/server';
 import { AuthProvider, useAuth } from '../../contexts/AuthContext';
 import { ThemeProvider } from '../../contexts/ThemeContext';
 import { ToastProvider } from '../../contexts/ToastContext';
-import { setAuthToken, type Permission } from '../../api/client';
+import { getAuthToken, setAuthToken, type Permission } from '../../api/client';
 
 function createWrapper() {
   const queryClient = new QueryClient({
@@ -325,4 +325,76 @@ describe('AuthContext', () => {
       }).not.toThrow();
     });
   });
+
+  describe('token validation on mount (#1889)', () => {
+    beforeEach(() => {
+      // Persisted "Remember Me" token — lives in localStorage.
+      setAuthToken('valid-token', 'persistent');
+      server.use(
+        http.get('/api/v1/auth/status', () =>
+          HttpResponse.json({ auth_enabled: true, requires_setup: false })
+        )
+      );
+    });
+
+    afterEach(() => {
+      setAuthToken(null);
+      localStorage.removeItem('auth_token');
+    });
+
+    it('keeps the persisted token when /auth/me fails transiently (does not force re-login)', async () => {
+      // Backend not ready yet / brief blip → 500 on every attempt.
+      server.use(http.get('/api/v1/auth/me', () => new HttpResponse(null, { status: 500 })));
+      vi.mocked(window.localStorage.removeItem).mockClear();
+
+      const { result } = renderHook(() => useAuth(), { wrapper: createWrapper() });
+
+      await waitFor(() => expect(result.current.loading).toBe(false), { timeout: 4000 });
+
+      // No user this load, but the token MUST survive so a reload can recover —
+      // the pre-#1889 blanket catch deleted it, making the session unrecoverable.
+      expect(result.current.user).toBeNull();
+      expect(getAuthToken()).toBe('valid-token');
+      // Persisted copy must not be wiped (setAuthToken(null) removes it).
+      expect(window.localStorage.removeItem).not.toHaveBeenCalledWith('auth_token');
+    });
+
+    it('clears the token on a definitive 401 invalid-token response', async () => {
+      server.use(
+        http.get('/api/v1/auth/me', () =>
+          HttpResponse.json({ detail: 'Could not validate credentials' }, { status: 401 })
+        )
+      );
+      vi.mocked(window.localStorage.removeItem).mockClear();
+
+      const { result } = renderHook(() => useAuth(), { wrapper: createWrapper() });
+
+      await waitFor(() => expect(result.current.loading).toBe(false), { timeout: 4000 });
+
+      expect(result.current.user).toBeNull();
+      expect(getAuthToken()).toBeNull();
+      // Definitive invalid-token → persisted copy is removed.
+      expect(window.localStorage.removeItem).toHaveBeenCalledWith('auth_token');
+    });
+
+    it('loads the user when the persisted token is valid', async () => {
+      server.use(
+        http.get('/api/v1/auth/me', () =>
+          HttpResponse.json({
+            id: 1,
+            username: 'alice',
+            is_active: true,
+            permissions: [],
+            groups: [],
+          })
+        )
+      );
+
+      const { result } = renderHook(() => useAuth(), { wrapper: createWrapper() });
+
+      await waitFor(() => expect(result.current.user).not.toBeNull());
+      expect(result.current.user?.username).toBe('alice');
+      expect(getAuthToken()).toBe('valid-token');
+    });
+  });
 });

+ 41 - 8
frontend/src/contexts/AuthContext.tsx

@@ -1,5 +1,5 @@
 import React, { createContext, useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react';
-import { api, getAuthToken, setAuthToken } from '../api/client';
+import { ApiError, api, getAuthToken, setAuthToken } from '../api/client';
 import type { LoginResponse, Permission, TokenPersistence, UserResponse } from '../api/client';
 
 interface AuthContextType {
@@ -58,18 +58,51 @@ export function AuthProvider({ children }: { children: React.ReactNode }) {
       if (status.auth_enabled) {
         const token = getAuthToken();
         if (token) {
-          try {
-            const currentUser = await api.getCurrentUser();
-            if (!mountedRef.current) return;
+          // Validate the stored token. A transient failure here (backend not
+          // yet ready after a container/proxy restart, a brief network blip)
+          // must NOT discard a valid persisted token — doing so logs the user
+          // out and, because the token is deleted, a reload can't recover it
+          // (#1889). Only a definitive 401 invalid-token response clears the
+          // token, and `request()` already does that clearing + dispatches
+          // `auth:expired`; here we just retry the transient cases and keep the
+          // token so the session survives a slow load.
+          let currentUser: UserResponse | null = null;
+          let definitiveAuthFailure = false;
+          const maxAttempts = 3;
+          for (let attempt = 1; attempt <= maxAttempts; attempt++) {
+            try {
+              currentUser = await api.getCurrentUser();
+              break;
+            } catch (err) {
+              if (!mountedRef.current) return;
+              // 401 invalid-token → genuinely logged out. `request()` has
+              // already cleared the token; stop retrying.
+              if (err instanceof ApiError && err.status === 401) {
+                definitiveAuthFailure = true;
+                break;
+              }
+              // Transient (network / 5xx / other) → back off and retry. Leave
+              // the token in place so a subsequent load can recover.
+              if (attempt < maxAttempts) {
+                await new Promise((r) => setTimeout(r, 400 * attempt));
+              }
+            }
+          }
+          if (!mountedRef.current) return;
+          if (currentUser) {
             setUser(currentUser);
             // Persist kiosk token only after the server confirms it is valid.
             if (urlToken && token === urlToken) {
               setAuthToken(urlToken, 'persistent');
             }
-          } catch {
-            // Token invalid, clear it (removes from both sessionStorage and localStorage)
-            setAuthToken(null);
-            if (!mountedRef.current) return;
+          } else {
+            // No user: either a definitive 401 (token already cleared by
+            // request()) or transient failures exhausted their retries. In the
+            // transient case we deliberately keep the token so a reload retries
+            // rather than forcing a re-login.
+            if (definitiveAuthFailure) {
+              setAuthToken(null);
+            }
             setUser(null);
           }
         } else {

A diferenza do arquivo foi suprimida porque é demasiado grande
+ 0 - 0
static/assets/index-CaHy_042.js


+ 1 - 1
static/index.html

@@ -26,7 +26,7 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-0IaClY0k.js"></script>
+    <script type="module" crossorigin src="/assets/index-CaHy_042.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-BxVhuRti.css">
   </head>
   <body>

Algúns arquivos non se mostraron porque demasiados arquivos cambiaron neste cambio