fix(feature-auth): use AUTH_PATH_PREFIX to skip the /me endpoint in the 401 interceptor (#135)
## Summary Regression introduced by PR #134 / merged minutes ago: opening portal-admin while anonymous fires the bootstrap `/admin/auth/me` → 401 → `bffUnauthorizedInterceptor` triggers `AuthService.refresh()` → which re-fires `/admin/auth/me` → 401 → tight loop that exhausts the BFF's 120/min rate limiter in seconds. The user lands on a `rate_limited` error instead of the "Sign in" panel. ## Root cause [`bffUnauthorizedInterceptor`](libs/feature/auth/src/lib/bff-unauthorized.interceptor.ts) deliberately skips the `/me` endpoint to avoid the loop (the bootstrap `/me` legitimately 401s when anonymous). But the skip target was **hardcoded** to `${bffBaseUrl}/auth/me`: ```ts !req.url.startsWith(`${bffBaseUrl}/auth/me`) ``` When portal-admin overrides `AUTH_PATH_PREFIX` to `/admin/auth` (per PR #134), the bootstrap URL becomes `${bffBaseUrl}/admin/auth/me`, which does **not** start with `${bffBaseUrl}/auth/me`. The interceptor treats it as a regular protected route, calls `refresh()`, and we're off to the races. The reported log shows the rate-limit counter dropping `remaining=3 → 2 → 1 → 0` over four `/me` calls within ~25 ms, all 401. ## Fix Derive the skip URL from `AUTH_PATH_PREFIX` (which the interceptor already has access to via DI — same module as the AuthService): ```ts const meUrl = `${bffBaseUrl}${pathPrefix}/me`; … !req.url.startsWith(meUrl) ``` Same behaviour for portal-shell (the default `/auth` factory keeps the skip URL at `/auth/me`), correct behaviour for portal-admin (`/admin/auth/me`), and any future surface that picks a different prefix inherits the fix automatically. ## Test plan - [x] `pnpm nx test feature-auth` — **30 specs pass** (was 29; +1 regression spec asserting that no follow-up `/me` is issued after a bootstrap 401 when the prefix is `/admin/auth`). - [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean. - [ ] Manual: `pnpm nx serve portal-admin`, visit `http://localhost:4300/`, observe the BFF log — exactly **one** `/admin/auth/me` request (the bootstrap), 401, no follow-up, no rate-limit hit. The home page renders the "No admin session detected" + Sign in button. ## Notes for the reviewer - The bug pattern is symmetric: any future host that overrides `AUTH_PATH_PREFIX` would have hit the same trap. Making the skip target derive from the token is the structural fix; the regression spec pins it. - portal-shell tests still pass without changes — the default factory returns `/auth`, so the existing fixtures continue exercising `${bffBaseUrl}/auth/me` as the skip target. - No SPA-side changes needed in portal-shell or portal-admin app code. The fix is entirely inside the lib. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #135
This commit was merged in pull request #135.
This commit is contained in:
@@ -2,7 +2,7 @@ import { HttpClient, provideHttpClient, withInterceptors } from '@angular/common
|
|||||||
import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing';
|
import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing';
|
||||||
import { TestBed } from '@angular/core/testing';
|
import { TestBed } from '@angular/core/testing';
|
||||||
import { firstValueFrom } from 'rxjs';
|
import { firstValueFrom } from 'rxjs';
|
||||||
import { AUTH_BFF_BASE_URL, AUTH_NAVIGATOR } from './auth.config';
|
import { AUTH_BFF_BASE_URL, AUTH_NAVIGATOR, AUTH_PATH_PREFIX } from './auth.config';
|
||||||
import { AuthService } from './auth.service';
|
import { AuthService } from './auth.service';
|
||||||
import { bffUnauthorizedInterceptor } from './bff-unauthorized.interceptor';
|
import { bffUnauthorizedInterceptor } from './bff-unauthorized.interceptor';
|
||||||
|
|
||||||
@@ -106,6 +106,39 @@ describe('bffUnauthorizedInterceptor', () => {
|
|||||||
expect(auth.state().kind).toBe('authenticated');
|
expect(auth.state().kind).toBe('authenticated');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('does NOT fire refresh on a 401 from the admin /me when AUTH_PATH_PREFIX is /admin/auth (would loop)', async () => {
|
||||||
|
// Regression for PR #134 follow-up: the original interceptor
|
||||||
|
// hard-coded `/auth/me` as the skip target. With portal-admin
|
||||||
|
// overriding the path prefix to `/admin/auth`, the 401 from
|
||||||
|
// `/admin/auth/me` fell through, triggering AuthService.refresh()
|
||||||
|
// which re-fired the same request — a tight loop that exhausted
|
||||||
|
// the BFF rate limiter on every admin landing.
|
||||||
|
TestBed.configureTestingModule({
|
||||||
|
providers: [
|
||||||
|
provideHttpClient(withInterceptors([bffUnauthorizedInterceptor])),
|
||||||
|
provideHttpClientTesting(),
|
||||||
|
{ provide: AUTH_BFF_BASE_URL, useValue: BFF_BASE },
|
||||||
|
{ provide: AUTH_PATH_PREFIX, useValue: '/admin/auth' },
|
||||||
|
{ provide: AUTH_NAVIGATOR, useValue: vi.fn() },
|
||||||
|
],
|
||||||
|
});
|
||||||
|
const adminMeUrl = `${BFF_BASE}/admin/auth/me`;
|
||||||
|
const httpCtrl = TestBed.inject(HttpTestingController);
|
||||||
|
const auth = TestBed.inject(AuthService);
|
||||||
|
const refreshSpy = vi.spyOn(auth, 'refresh');
|
||||||
|
httpCtrl
|
||||||
|
.expectOne(adminMeUrl)
|
||||||
|
.flush({ error: 'unauthenticated' }, { status: 401, statusText: 'Unauthorized' });
|
||||||
|
await Promise.resolve();
|
||||||
|
expect(refreshSpy).not.toHaveBeenCalled();
|
||||||
|
expect(auth.state().kind).toBe('anonymous');
|
||||||
|
// Crucially: no follow-up /me request was issued by the
|
||||||
|
// interceptor. If one had been, the testing backend would
|
||||||
|
// either record a pending request or this assertion would
|
||||||
|
// fail trying to verify().
|
||||||
|
httpCtrl.verify();
|
||||||
|
});
|
||||||
|
|
||||||
it('does NOT touch 401s from non-BFF origins', async () => {
|
it('does NOT touch 401s from non-BFF origins', async () => {
|
||||||
const { http, httpCtrl, auth } = setup();
|
const { http, httpCtrl, auth } = setup();
|
||||||
httpCtrl
|
httpCtrl
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ import {
|
|||||||
} from '@angular/common/http';
|
} from '@angular/common/http';
|
||||||
import { Injector, inject } from '@angular/core';
|
import { Injector, inject } from '@angular/core';
|
||||||
import { catchError, throwError } from 'rxjs';
|
import { catchError, throwError } from 'rxjs';
|
||||||
import { AUTH_BFF_BASE_URL } from './auth.config';
|
import { AUTH_BFF_BASE_URL, AUTH_PATH_PREFIX } from './auth.config';
|
||||||
import { AuthService } from './auth.service';
|
import { AuthService } from './auth.service';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -24,9 +24,12 @@ import { AuthService } from './auth.service';
|
|||||||
* its failure and can show its own fallback UI / route guards
|
* its failure and can show its own fallback UI / route guards
|
||||||
* can react on the next navigation.
|
* can react on the next navigation.
|
||||||
*
|
*
|
||||||
* **Skips `/auth/me` itself.** `AuthService.refresh()` calls `/me`,
|
* **Skips the `/me` endpoint itself.** `AuthService.refresh()` calls
|
||||||
* which legitimately 401s when anonymous. Triggering `refresh()`
|
* `/me`, which legitimately 401s when anonymous. Triggering
|
||||||
* again on that response would loop indefinitely.
|
* `refresh()` again on that response would loop indefinitely. The
|
||||||
|
* skip target is composed from `AUTH_PATH_PREFIX` so it matches
|
||||||
|
* whichever surface this app runs on (`/auth/me` for portal-shell,
|
||||||
|
* `/admin/auth/me` for portal-admin).
|
||||||
*
|
*
|
||||||
* Other 4xx / 5xx pass through untouched — they are domain errors,
|
* Other 4xx / 5xx pass through untouched — they are domain errors,
|
||||||
* not session-state signals.
|
* not session-state signals.
|
||||||
@@ -36,6 +39,8 @@ export const bffUnauthorizedInterceptor: HttpInterceptorFn = (
|
|||||||
next: HttpHandlerFn,
|
next: HttpHandlerFn,
|
||||||
) => {
|
) => {
|
||||||
const bffBaseUrl = inject(AUTH_BFF_BASE_URL);
|
const bffBaseUrl = inject(AUTH_BFF_BASE_URL);
|
||||||
|
const pathPrefix = inject(AUTH_PATH_PREFIX);
|
||||||
|
const meUrl = `${bffBaseUrl}${pathPrefix}/me`;
|
||||||
// Inject the parent injector here rather than `AuthService` directly:
|
// Inject the parent injector here rather than `AuthService` directly:
|
||||||
// the bootstrap `/me` round-trip is fired from `AuthService`'s own
|
// the bootstrap `/me` round-trip is fired from `AuthService`'s own
|
||||||
// constructor, so a synchronous `inject(AuthService)` here would
|
// constructor, so a synchronous `inject(AuthService)` here would
|
||||||
@@ -52,7 +57,7 @@ export const bffUnauthorizedInterceptor: HttpInterceptorFn = (
|
|||||||
err instanceof HttpErrorResponse &&
|
err instanceof HttpErrorResponse &&
|
||||||
err.status === 401 &&
|
err.status === 401 &&
|
||||||
req.url.startsWith(bffBaseUrl) &&
|
req.url.startsWith(bffBaseUrl) &&
|
||||||
!req.url.startsWith(`${bffBaseUrl}/auth/me`)
|
!req.url.startsWith(meUrl)
|
||||||
) {
|
) {
|
||||||
// Best-effort sync — swallow the refresh-side error so the
|
// Best-effort sync — swallow the refresh-side error so the
|
||||||
// original 401 is what bubbles up to the caller.
|
// original 401 is what bubbles up to the caller.
|
||||||
|
|||||||
Reference in New Issue
Block a user