fix(web): confirm session before rendering and recover concurrent 401s

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<unknown>()) {
     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<unknown>(),
+    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);
+  });
+});

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()) {
-      <!-- Full-screen login (no chrome) -->
+    @if (!showChrome()) {
+      <!-- Full-screen login / welcome (no chrome) -->
       <router-outlet />
     } @else {
       <div class="shell">
@@ -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<boolean>;
+  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<HTMLButtonElement>;
@@ -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<StatusResponse>('/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']),

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'])));
 };

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<T>(req: HttpRequest<T>, token: string): HttpRequest<T> {
+  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))),
+      );
     }),
   );
 };

apps/rustnzb/frontend/src/app/core/security.spec.ts

@@ -9,46 +9,50 @@ 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 },
         { provide: Router, useValue: router },
       ],
     });
     const result = TestBed.runInInjectionContext(() => authGuard({} as never, {} as never));
-    return { result, router };
+    return { result: result as Observable<unknown>, 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<unknown> = 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<unknown>, next: HttpHandlerFn) =>
+    TestBed.runInInjectionContext(() => authInterceptor(request, next));
+  const authHeader = (next: ReturnType<typeof vi.fn>, call: number) =>
+    (next.mock.calls[call][0] as HttpRequest<unknown>).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<unknown>).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<unknown>) =>
+      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']);