From 5922a8cba8e3c80f6e01ea245945b6c054071b35 Mon Sep 17 00:00:00 2001 From: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:26:20 +0000 Subject: [PATCH] fix(web): confirm session before rendering and recover concurrent 401s Two symptoms, one root cause: the SPA trusted any access token in localStorage, but the server keeps tokens in memory (15 min TTL, wiped on restart). Slow table population: after token expiry every startup request 401s, but only the first refreshed and retried; the rest waited for their next poll (2s queue, 5s history) or never retried (servers, categories). All 401s now share one in-flight refresh and retry; a request sent with an already rotated token retries without spending another single-use refresh token. Token expiry is persisted so an expired token is refreshed up front. Content flash before login bounce: the guard passed on a stale token, so the chrome and Downloads page rendered until the API rejected it. The guard now confirms the session with the server once per page load, and chrome follows a reactive session signal (plus route: login/welcome stay full-screen) instead of a 2s localStorage poll. The login page uses the same check so a stale token no longer leaves it stuck on "Checking status...". Co-Authored-By: Claude Opus 5.5 --- apps/rustnzb/frontend/src/app/app.spec.ts | 25 +++- apps/rustnzb/frontend/src/app/app.ts | 30 +++-- .../src/app/core/guards/auth.guard.ts | 12 +- .../app/core/interceptors/auth.interceptor.ts | 47 ++++---- .../frontend/src/app/core/security.spec.ts | 111 +++++++++++++----- .../app/core/services/auth.service.spec.ts | 68 ++++++++++- .../src/app/core/services/auth.service.ts | 97 ++++++++++++--- .../app/features/auth/login.component.spec.ts | 8 +- .../src/app/features/auth/login.component.ts | 17 ++- 9 files changed, 325 insertions(+), 90 deletions(-) diff --git a/apps/rustnzb/frontend/src/app/app.spec.ts b/apps/rustnzb/frontend/src/app/app.spec.ts index a73f78e3..6cb50f48 100644 --- a/apps/rustnzb/frontend/src/app/app.spec.ts +++ b/apps/rustnzb/frontend/src/app/app.spec.ts @@ -1,9 +1,10 @@ import '@angular/compiler'; +import { signal } from '@angular/core'; import { Subject, of } from 'rxjs'; import { describe, expect, it, vi } from 'vitest'; -import { App, isDemoPath } from './app'; +import { App, isBareRoute, isDemoPath } from './app'; import { AddNzbService } from './core/services/add-nzb.service'; import { PauseStateService } from './core/services/pause-state.service'; @@ -13,10 +14,14 @@ function makeApp(postResult = new Subject()) { post: vi.fn(() => postResult.asObservable()), }; const auth = { - isLoggedIn: vi.fn(() => false), + authenticated: signal(false), logout: vi.fn(() => of({})), }; - const router = { url: '/downloads', navigate: vi.fn(() => Promise.resolve(true)) }; + const router = { + url: '/downloads', + events: new Subject(), + navigate: vi.fn(() => Promise.resolve(true)), + }; const pauseState = new PauseStateService(); const app = new App( api as never, @@ -64,3 +69,17 @@ describe('demo path detection', () => { expect(isDemoPath('/demonstration')).toBe(false); }); }); + +describe('bare route detection', () => { + it('keeps login and welcome full-screen', () => { + expect(isBareRoute('/login')).toBe(true); + expect(isBareRoute('/welcome')).toBe(true); + expect(isBareRoute('/login?returnUrl=%2Fsettings')).toBe(true); + }); + + it('shows chrome on application pages', () => { + expect(isBareRoute('/downloads')).toBe(false); + expect(isBareRoute('/settings')).toBe(false); + expect(isBareRoute('/welcomes')).toBe(false); + }); +}); diff --git a/apps/rustnzb/frontend/src/app/app.ts b/apps/rustnzb/frontend/src/app/app.ts index 1c0cd664..e5a787e6 100644 --- a/apps/rustnzb/frontend/src/app/app.ts +++ b/apps/rustnzb/frontend/src/app/app.ts @@ -3,13 +3,16 @@ import { ElementRef, OnInit, OnDestroy, + Signal, ViewChild, + computed, signal, WritableSignal, } from '@angular/core'; import { CommonModule } from '@angular/common'; import { FormsModule } from '@angular/forms'; -import { Router, RouterModule } from '@angular/router'; +import { NavigationEnd, Router, RouterModule } from '@angular/router'; +import { filter } from 'rxjs'; import { ApiService } from './core/services/api.service'; import { AuthService } from './core/services/auth.service'; import { StatusResponse } from './core/models/queue.model'; @@ -23,13 +26,19 @@ export function isDemoPath(pathname: string): boolean { return pathname === '/demo' || pathname.startsWith('/demo/'); } +// Pages that render full-screen, without the app chrome, even when signed in. +export function isBareRoute(url: string): boolean { + const path = url.split(/[?#]/)[0]; + return path === '/login' || path === '/welcome'; +} + @Component({ selector: 'app-root', standalone: true, imports: [CommonModule, FormsModule, RouterModule, IconComponent], template: ` - @if (!authenticated()) { - + @if (!showChrome()) { + } @else {
@@ -411,7 +420,12 @@ export class App implements OnInit, OnDestroy { queueCount = signal(0); diskFree = signal(0); webdavEnabled = signal(false); - authenticated = signal(false); + readonly authenticated: Signal; + private readonly currentUrl = signal(''); + // Keyed on the route as well as the session: swapping branches rebuilds the + // router outlet, and doing that under /login mid-submit would re-run the + // login page's redirect and race the navigation to /welcome. + readonly showChrome = computed(() => this.authenticated() && !isBareRoute(this.currentUrl())); pauseMenuOpen = false; customPauseMin: number | null = null; @ViewChild('pauseCaretBtn') pauseCaretBtn?: ElementRef; @@ -439,10 +453,14 @@ export class App implements OnInit, OnDestroy { pauseState: PauseStateService, ) { this.paused = pauseState.paused; + this.authenticated = authService.authenticated; + this.currentUrl.set(router.url); + router.events + .pipe(filter((e): e is NavigationEnd => e instanceof NavigationEnd)) + .subscribe((e) => this.currentUrl.set(e.urlAfterRedirects)); } ngOnInit(): void { - this.authenticated.set(this.authService.isLoggedIn()); this.pollStatus(); this.pollTimer = setInterval(() => this.pollStatus(), 2000); document.addEventListener('click', this.docClickHandler); @@ -454,7 +472,6 @@ export class App implements OnInit, OnDestroy { } pollStatus(): void { - this.authenticated.set(this.authService.isLoggedIn()); if (!this.authenticated()) return; this.api.get('/status').subscribe({ next: (s) => { @@ -470,7 +487,6 @@ export class App implements OnInit, OnDestroy { } onLogout(): void { - this.authenticated.set(false); this.authService.logout().subscribe({ complete: () => this.router.navigate(['/login']), error: () => this.router.navigate(['/login']), diff --git a/apps/rustnzb/frontend/src/app/core/guards/auth.guard.ts b/apps/rustnzb/frontend/src/app/core/guards/auth.guard.ts index 1ec353e5..fefef165 100644 --- a/apps/rustnzb/frontend/src/app/core/guards/auth.guard.ts +++ b/apps/rustnzb/frontend/src/app/core/guards/auth.guard.ts @@ -1,15 +1,15 @@ import { CanActivateFn, Router } from '@angular/router'; import { inject } from '@angular/core'; +import { map } from 'rxjs'; import { AuthService } from '../services/auth.service'; +// Hold the navigation until the stored token is confirmed, so a stale token +// (expired, or from before a server restart) never renders protected content. export const authGuard: CanActivateFn = () => { const authService = inject(AuthService); const router = inject(Router); - if (authService.isLoggedIn()) { - return true; - } - - router.navigate(['/login']); - return false; + return authService + .ensureSession() + .pipe(map((ok) => ok || router.createUrlTree(['/login']))); }; diff --git a/apps/rustnzb/frontend/src/app/core/interceptors/auth.interceptor.ts b/apps/rustnzb/frontend/src/app/core/interceptors/auth.interceptor.ts index 0c797f81..f29f72de 100644 --- a/apps/rustnzb/frontend/src/app/core/interceptors/auth.interceptor.ts +++ b/apps/rustnzb/frontend/src/app/core/interceptors/auth.interceptor.ts @@ -1,10 +1,12 @@ -import { HttpInterceptorFn, HttpErrorResponse } from '@angular/common/http'; +import { HttpInterceptorFn, HttpErrorResponse, HttpRequest } from '@angular/common/http'; import { inject } from '@angular/core'; import { Router } from '@angular/router'; import { catchError, switchMap, throwError } from 'rxjs'; import { AuthService } from '../services/auth.service'; -let isRefreshing = false; +function withToken(req: HttpRequest, token: string): HttpRequest { + return req.clone({ setHeaders: { Authorization: `Bearer ${token}` } }); +} export const authInterceptor: HttpInterceptorFn = (req, next) => { // Don't intercept auth endpoints @@ -15,29 +17,34 @@ export const authInterceptor: HttpInterceptorFn = (req, next) => { const authService = inject(AuthService); const router = inject(Router); + const token = authService.getAccessToken(); + if (token && !req.headers.has('Authorization')) { + req = withToken(req, token); + } + return next(req).pipe( catchError((error: HttpErrorResponse) => { - if (error.status === 401 && !isRefreshing) { - isRefreshing = true; + if (error.status !== 401) { + return throwError(() => error); + } - return authService.refresh().pipe( - switchMap((tokens) => { - isRefreshing = false; - const cloned = req.clone({ - setHeaders: { Authorization: `Bearer ${tokens.access_token}` }, - }); - return next(cloned); - }), - catchError((refreshError) => { - isRefreshing = false; - authService.clearTokens(); - router.navigate(['/login']); - return throwError(() => refreshError); - }), - ); + // Tokens already rotated while this request was in flight: retry with + // the current one rather than spending another single-use refresh token. + const current = authService.getAccessToken(); + if (current && req.headers.get('Authorization') !== `Bearer ${current}`) { + return next(withToken(req, current)); } - return throwError(() => error); + // Every concurrent 401 waits on the same refresh and then retries, so + // parallel page loads all recover instead of only the first request. + return authService.refresh().pipe( + catchError((refreshError) => { + authService.clearTokens(); + router.navigate(['/login']); + return throwError(() => refreshError); + }), + switchMap((tokens) => next(withToken(req, tokens.access_token))), + ); }), ); }; diff --git a/apps/rustnzb/frontend/src/app/core/security.spec.ts b/apps/rustnzb/frontend/src/app/core/security.spec.ts index 3a40ca2a..58ad5582 100644 --- a/apps/rustnzb/frontend/src/app/core/security.spec.ts +++ b/apps/rustnzb/frontend/src/app/core/security.spec.ts @@ -9,16 +9,17 @@ import { import { TestBed } from '@angular/core/testing'; import { Router } from '@angular/router'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import { firstValueFrom, of, throwError } from 'rxjs'; +import { Observable, Subject, firstValueFrom, of, throwError } from 'rxjs'; import { authGuard } from './guards/auth.guard'; import { authInterceptor } from './interceptors/auth.interceptor'; import { AuthService } from './services/auth.service'; describe('authGuard', () => { - function run(loggedIn: boolean) { - const auth = { isLoggedIn: vi.fn(() => loggedIn) }; - const router = { navigate: vi.fn(() => Promise.resolve(true)) }; + function run(session: boolean) { + const auth = { ensureSession: vi.fn(() => of(session)) }; + const loginTree = { login: true }; + const router = { createUrlTree: vi.fn(() => loginTree) }; TestBed.configureTestingModule({ providers: [ { provide: AuthService, useValue: auth }, @@ -26,29 +27,32 @@ describe('authGuard', () => { ], }); const result = TestBed.runInInjectionContext(() => authGuard({} as never, {} as never)); - return { result, router }; + return { result: result as Observable, router, loginTree }; } afterEach(() => TestBed.resetTestingModule()); - it('allows authenticated navigation', () => { + it('allows navigation once the session is confirmed', async () => { const { result, router } = run(true); - expect(result).toBe(true); - expect(router.navigate).not.toHaveBeenCalled(); + expect(await firstValueFrom(result)).toBe(true); + expect(router.createUrlTree).not.toHaveBeenCalled(); }); - it('redirects anonymous navigation to login', () => { - const { result, router } = run(false); - expect(result).toBe(false); - expect(router.navigate).toHaveBeenCalledWith(['/login']); + it('redirects to login when the stored session is rejected', async () => { + const { result, router, loginTree } = run(false); + expect(await firstValueFrom(result)).toBe(loginTree); + expect(router.createUrlTree).toHaveBeenCalledWith(['/login']); }); }); describe('authInterceptor', () => { - function configure(refreshResult = of({ access_token: 'new-access' })) { + function configure(refreshResult: Observable = of({ access_token: 'new-access' })) { + let token: string | null = 'old-access'; const auth = { + getAccessToken: vi.fn(() => token), refresh: vi.fn(() => refreshResult), clearTokens: vi.fn(), + setToken: (t: string | null) => (token = t), }; const router = { navigate: vi.fn(() => Promise.resolve(true)) }; TestBed.configureTestingModule({ @@ -60,6 +64,13 @@ describe('authInterceptor', () => { return { auth, router }; } + const unauthorized = () => throwError(() => new HttpErrorResponse({ status: 401 })); + const ok = () => of(new HttpResponse({ status: 200 })); + const intercept = (request: HttpRequest, next: HttpHandlerFn) => + TestBed.runInInjectionContext(() => authInterceptor(request, next)); + const authHeader = (next: ReturnType, call: number) => + (next.mock.calls[call][0] as HttpRequest).headers.get('Authorization'); + afterEach(() => TestBed.resetTestingModule()); it('does not intercept authentication endpoints', async () => { @@ -69,37 +80,79 @@ describe('authInterceptor', () => { expect(next).toHaveBeenCalledWith(request); }); + it('attaches the current access token when the request has none', async () => { + configure(); + const next = vi.fn(ok); + await firstValueFrom(intercept(new HttpRequest('GET', '/api/status'), next as HttpHandlerFn)); + expect(authHeader(next, 0)).toBe('Bearer old-access'); + }); + it('refreshes after a 401 and retries with the rotated access token', async () => { const { auth } = configure(); - const next = vi - .fn() - .mockReturnValueOnce(throwError(() => new HttpErrorResponse({ status: 401 }))) - .mockReturnValueOnce(of(new HttpResponse({ status: 200 }))); - const request = new HttpRequest('GET', '/api/queue'); + const next = vi.fn().mockReturnValueOnce(unauthorized()).mockReturnValueOnce(ok()); - await firstValueFrom( - TestBed.runInInjectionContext(() => authInterceptor(request, next as HttpHandlerFn)), - ); + await firstValueFrom(intercept(new HttpRequest('GET', '/api/queue'), next as HttpHandlerFn)); expect(auth.refresh).toHaveBeenCalledTimes(1); expect(next).toHaveBeenCalledTimes(2); - expect((next.mock.calls[1][0] as HttpRequest).headers.get('Authorization')).toBe( - 'Bearer new-access', + expect(authHeader(next, 1)).toBe('Bearer new-access'); + }); + + it('recovers every concurrent 401, not just the first', async () => { + const refresh = new Subject<{ access_token: string }>(); + const { auth } = configure(refresh); + // Mirrors AuthService.refresh(): concurrent callers share one request. + auth.refresh.mockReturnValue(refresh); + const next = vi.fn((req: HttpRequest) => + req.headers.get('Authorization') === 'Bearer new-access' ? ok() : unauthorized(), ); + + const results = ['/api/queue', '/api/history', '/api/config/servers'].map((url) => + firstValueFrom(intercept(new HttpRequest('GET', url), next as HttpHandlerFn)), + ); + refresh.next({ access_token: 'new-access' }); + refresh.complete(); + + await expect(Promise.all(results)).resolves.toHaveLength(3); + expect(next).toHaveBeenCalledTimes(6); + }); + + it('retries with an already-rotated token without refreshing again', async () => { + const { auth } = configure(); + auth.setToken('rotated-access'); + const next = vi.fn().mockReturnValueOnce(unauthorized()).mockReturnValueOnce(ok()); + const request = new HttpRequest('GET', '/api/queue').clone({ + setHeaders: { Authorization: 'Bearer old-access' }, + }); + + await firstValueFrom(intercept(request, next as HttpHandlerFn)); + + expect(auth.refresh).not.toHaveBeenCalled(); + expect(authHeader(next, 1)).toBe('Bearer rotated-access'); + }); + + it('keeps the session when the retried request fails for another reason', async () => { + const { auth, router } = configure(); + const next = vi + .fn() + .mockReturnValueOnce(unauthorized()) + .mockReturnValueOnce(throwError(() => new HttpErrorResponse({ status: 500 }))); + + await expect( + firstValueFrom(intercept(new HttpRequest('GET', '/api/queue'), next as HttpHandlerFn)), + ).rejects.toMatchObject({ status: 500 }); + expect(auth.clearTokens).not.toHaveBeenCalled(); + expect(router.navigate).not.toHaveBeenCalled(); }); it('clears credentials and redirects when refresh fails', async () => { const { auth, router } = configure( throwError(() => new HttpErrorResponse({ status: 403 })), ); - const next = vi.fn(() => throwError(() => new HttpErrorResponse({ status: 401 }))); + const next = vi.fn(unauthorized); await expect( - firstValueFrom( - TestBed.runInInjectionContext(() => - authInterceptor(new HttpRequest('GET', '/api/queue'), next as HttpHandlerFn), - ), - ), + firstValueFrom(intercept(new HttpRequest('GET', '/api/queue'), next as HttpHandlerFn)), ).rejects.toMatchObject({ status: 403 }); expect(auth.clearTokens).toHaveBeenCalledTimes(1); expect(router.navigate).toHaveBeenCalledWith(['/login']); diff --git a/apps/rustnzb/frontend/src/app/core/services/auth.service.spec.ts b/apps/rustnzb/frontend/src/app/core/services/auth.service.spec.ts index 93910ab8..ec643af6 100644 --- a/apps/rustnzb/frontend/src/app/core/services/auth.service.spec.ts +++ b/apps/rustnzb/frontend/src/app/core/services/auth.service.spec.ts @@ -1,8 +1,8 @@ import '@angular/compiler'; -import { HttpClient } from '@angular/common/http'; +import { HttpClient, HttpErrorResponse } from '@angular/common/http'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { of } from 'rxjs'; +import { Subject, firstValueFrom, of, throwError } from 'rxjs'; import { AuthService, TokenResponse } from './auth.service'; @@ -74,4 +74,68 @@ describe('AuthService', () => { localStorage.setItem('access_token', 'access'); expect(service.isLoggedIn()).toBe(true); }); + + it('shares one in-flight refresh between concurrent callers', () => { + const response = new Subject(); + http.post.mockReturnValue(response); + localStorage.setItem('refresh_token', 'old-refresh'); + + const received: string[] = []; + service.refresh().subscribe((t) => received.push(t.access_token)); + service.refresh().subscribe((t) => received.push(t.access_token)); + response.next(TOKENS); + response.complete(); + + expect(http.post).toHaveBeenCalledTimes(1); + expect(received).toEqual(['access-1', 'access-1']); + }); + + it('does not treat a stored token as authenticated until the server confirms it', async () => { + localStorage.setItem('access_token', 'stored'); + service = new AuthService(http as unknown as HttpClient); + expect(service.authenticated()).toBe(false); + + await expect(firstValueFrom(service.ensureSession())).resolves.toBe(true); + + expect(http.get).toHaveBeenCalledWith('/api/status'); + expect(service.authenticated()).toBe(true); + }); + + it('clears a stored token the server rejects', async () => { + localStorage.setItem('access_token', 'stale'); + localStorage.setItem('refresh_token', 'stale-refresh'); + service = new AuthService(http as unknown as HttpClient); + http.get.mockReturnValue(throwError(() => new HttpErrorResponse({ status: 401 }))); + + await expect(firstValueFrom(service.ensureSession())).resolves.toBe(false); + + expect(service.isLoggedIn()).toBe(false); + expect(service.authenticated()).toBe(false); + expect(localStorage.getItem('refresh_token')).toBeNull(); + }); + + it('refreshes up front instead of probing with a known-expired token', async () => { + localStorage.setItem('access_token', 'expired'); + localStorage.setItem('refresh_token', 'old-refresh'); + localStorage.setItem('access_token_expires_at', String(Date.now() - 1000)); + service = new AuthService(http as unknown as HttpClient); + + await expect(firstValueFrom(service.ensureSession())).resolves.toBe(true); + + expect(http.get).not.toHaveBeenCalled(); + expect(http.post).toHaveBeenCalledWith('/api/auth/refresh', { refresh_token: 'old-refresh' }); + expect(service.getAccessToken()).toBe('access-1'); + }); + + it('reports no session without contacting the server when no token is stored', async () => { + await expect(firstValueFrom(service.ensureSession())).resolves.toBe(false); + expect(http.get).not.toHaveBeenCalled(); + }); + + it('marks the session authenticated immediately after login', () => { + service.login('alice', 'secret').subscribe(); + expect(service.authenticated()).toBe(true); + service.clearTokens(); + expect(service.authenticated()).toBe(false); + }); }); diff --git a/apps/rustnzb/frontend/src/app/core/services/auth.service.ts b/apps/rustnzb/frontend/src/app/core/services/auth.service.ts index 350b353c..6b320677 100644 --- a/apps/rustnzb/frontend/src/app/core/services/auth.service.ts +++ b/apps/rustnzb/frontend/src/app/core/services/auth.service.ts @@ -1,6 +1,6 @@ -import { Injectable } from '@angular/core'; -import { HttpClient } from '@angular/common/http'; -import { Observable, tap } from 'rxjs'; +import { Injectable, computed, signal } from '@angular/core'; +import { HttpClient, HttpErrorResponse } from '@angular/common/http'; +import { Observable, catchError, finalize, map, of, shareReplay, tap, throwError } from 'rxjs'; export interface AuthStatus { auth_enabled: boolean; @@ -14,10 +14,26 @@ export interface TokenResponse { expires_in: number; } +const ACCESS_KEY = 'access_token'; +const REFRESH_KEY = 'refresh_token'; +const EXPIRES_KEY = 'access_token_expires_at'; +// Refresh slightly early so a token never expires between check and use. +const EXPIRY_SKEW_MS = 30_000; + @Injectable({ providedIn: 'root' }) export class AuthService { private baseUrl = '/api/auth'; + private readonly accessToken = signal(localStorage.getItem(ACCESS_KEY)); + // A stored token is only a claim: the server keeps tokens in memory, so a + // restart (or expiry) invalidates it. Chrome and guarded routes wait until + // the token has been confirmed against the server once per page load. + private readonly verified = signal(false); + readonly authenticated = computed(() => !!this.accessToken() && this.verified()); + + private refresh$: Observable | null = null; + private verify$: Observable | null = null; + constructor(private http: HttpClient) {} checkAuth(): Observable { @@ -36,34 +52,87 @@ export class AuthService { .pipe(tap((res) => this.storeTokens(res))); } + /** + * Rotate tokens. Concurrent callers share one in-flight request: refresh + * tokens are single-use, so parallel refreshes would revoke each other. + */ refresh(): Observable { - const refreshToken = localStorage.getItem('refresh_token'); - return this.http - .post(`${this.baseUrl}/refresh`, { refresh_token: refreshToken }) - .pipe(tap((res) => this.storeTokens(res))); + if (!this.refresh$) { + const refreshToken = localStorage.getItem(REFRESH_KEY); + this.refresh$ = this.http + .post(`${this.baseUrl}/refresh`, { refresh_token: refreshToken }) + .pipe( + tap((res) => this.storeTokens(res)), + finalize(() => (this.refresh$ = null)), + shareReplay({ bufferSize: 1, refCount: false }), + ); + } + return this.refresh$; } logout(): Observable { - const refreshToken = localStorage.getItem('refresh_token'); + const refreshToken = localStorage.getItem(REFRESH_KEY); this.clearTokens(); return this.http.post(`${this.baseUrl}/logout`, { refresh_token: refreshToken }); } + /** + * Resolve whether the stored session is usable, confirming it with the + * server on first use. Emits false (and clears tokens) when the server + * rejects it; a network failure leaves the session in place. + */ + ensureSession(): Observable { + if (!this.getAccessToken()) return of(false); + if (this.verified()) return of(true); + if (!this.verify$) { + const probe$: Observable = this.accessTokenExpired() + ? this.refresh() + : this.http.get('/api/status'); + this.verify$ = probe$.pipe( + map(() => true), + catchError((err) => { + const rejected = + err instanceof HttpErrorResponse && (err.status === 401 || err.status === 403); + if (rejected) this.clearTokens(); + return of(!rejected); + }), + map((ok) => ok && this.isLoggedIn()), + tap((ok) => this.verified.set(ok)), + finalize(() => (this.verify$ = null)), + shareReplay({ bufferSize: 1, refCount: false }), + ); + } + return this.verify$; + } + isLoggedIn(): boolean { - return !!localStorage.getItem('access_token'); + return !!localStorage.getItem(ACCESS_KEY); } getAccessToken(): string | null { - return localStorage.getItem('access_token'); + return localStorage.getItem(ACCESS_KEY); + } + + private accessTokenExpired(): boolean { + const expiresAt = Number(localStorage.getItem(EXPIRES_KEY)); + return !!expiresAt && Date.now() >= expiresAt - EXPIRY_SKEW_MS; } private storeTokens(res: TokenResponse): void { - localStorage.setItem('access_token', res.access_token); - localStorage.setItem('refresh_token', res.refresh_token); + localStorage.setItem(ACCESS_KEY, res.access_token); + localStorage.setItem(REFRESH_KEY, res.refresh_token); + if (res.expires_in) { + localStorage.setItem(EXPIRES_KEY, String(Date.now() + res.expires_in * 1000)); + } + this.accessToken.set(res.access_token); + this.verified.set(true); } clearTokens(): void { - localStorage.removeItem('access_token'); - localStorage.removeItem('refresh_token'); + localStorage.removeItem(ACCESS_KEY); + localStorage.removeItem(REFRESH_KEY); + localStorage.removeItem(EXPIRES_KEY); + this.accessToken.set(null); + this.verified.set(false); } } diff --git a/apps/rustnzb/frontend/src/app/features/auth/login.component.spec.ts b/apps/rustnzb/frontend/src/app/features/auth/login.component.spec.ts index ba10a934..7b145e3e 100644 --- a/apps/rustnzb/frontend/src/app/features/auth/login.component.spec.ts +++ b/apps/rustnzb/frontend/src/app/features/auth/login.component.spec.ts @@ -16,7 +16,7 @@ const TOKENS: TokenResponse = { describe('LoginComponent', () => { let auth: { - isLoggedIn: ReturnType; + ensureSession: ReturnType; checkAuth: ReturnType; setup: ReturnType; login: ReturnType; @@ -26,7 +26,7 @@ describe('LoginComponent', () => { beforeEach(() => { auth = { - isLoggedIn: vi.fn(() => false), + ensureSession: vi.fn(() => of(false)), checkAuth: vi.fn(() => of({ auth_enabled: true, setup_required: false })), setup: vi.fn(() => of(TOKENS)), login: vi.fn(() => of(TOKENS)), @@ -38,8 +38,8 @@ describe('LoginComponent', () => { ); }); - it('redirects an existing session without checking server auth state', () => { - auth.isLoggedIn.mockReturnValue(true); + it('redirects a server-confirmed session without checking server auth state', () => { + auth.ensureSession.mockReturnValue(of(true)); component.ngOnInit(); expect(router.navigate).toHaveBeenCalledWith(['/downloads']); expect(auth.checkAuth).not.toHaveBeenCalled(); diff --git a/apps/rustnzb/frontend/src/app/features/auth/login.component.ts b/apps/rustnzb/frontend/src/app/features/auth/login.component.ts index 4a252615..d9a3f7ab 100644 --- a/apps/rustnzb/frontend/src/app/features/auth/login.component.ts +++ b/apps/rustnzb/frontend/src/app/features/auth/login.component.ts @@ -150,12 +150,19 @@ export class LoginComponent implements OnInit { ) {} ngOnInit(): void { - // If already logged in, go to downloads - if (this.authService.isLoggedIn()) { - this.router.navigate(['/downloads']); - return; - } + // Only a session the server accepts skips the form. A stale stored token + // is cleared here; redirecting on it would bounce straight back to this + // already-active route and leave the form stuck on "Checking status...". + this.authService.ensureSession().subscribe((ok) => { + if (ok) { + this.router.navigate(['/downloads']); + } else { + this.loadAuthStatus(); + } + }); + } + private loadAuthStatus(): void { this.authService.checkAuth().subscribe({ next: (status) => { if (!status.auth_enabled && !status.setup_required) {