From f7c02b7fc9daab98de1543e17cdbdd3cf9529cda Mon Sep 17 00:00:00 2001 From: Schalli Date: Mon, 21 Sep 2026 11:27:36 +0200 Subject: [PATCH] fix(api): erzwungenen Passwortwechsel an der API wirklich durchsetzen JwtStrategy.validate liess mustChangePassword auf dem Weg vom Token zu request.user fallen; der global registrierte ForcePasswordChangeInterceptor prueft genau dieses Feld und hat seit seiner Einfuehrung nie etwas blockiert. validate() reicht das Feld jetzt durch (strenger Vergleich mit true, Alt-Sitzungen ohne den Anspruch bleiben unveraendert unbetroffen). Zusaetzlich die Erlaubnisliste des Abfangers von Teilstring-Vergleich auf exakten Abgleich von Methode UND Pfad umgestellt (Absicherung gegen eine kuenftige kollidierende Route, heute nicht ausnutzbar). Nahttest gepinnt, der gegen den alten Quelltext nachweislich scheitert (6 von 12 neuen Faellen rot vor der Aenderung, gruen danach). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TPPB4ApQxzSU1rwV2Ffj9J --- .../force-password-change.interceptor.spec.ts | 136 ++++++++++++++++++ .../force-password-change.interceptor.ts | 41 ++++-- .../src/auth/strategies/jwt.strategy.spec.ts | 61 ++++++++ apps/api/src/auth/strategies/jwt.strategy.ts | 4 + 4 files changed, 230 insertions(+), 12 deletions(-) create mode 100644 apps/api/src/auth/interceptors/force-password-change.interceptor.spec.ts create mode 100644 apps/api/src/auth/strategies/jwt.strategy.spec.ts diff --git a/apps/api/src/auth/interceptors/force-password-change.interceptor.spec.ts b/apps/api/src/auth/interceptors/force-password-change.interceptor.spec.ts new file mode 100644 index 0000000..a45054d --- /dev/null +++ b/apps/api/src/auth/interceptors/force-password-change.interceptor.spec.ts @@ -0,0 +1,136 @@ +import { ForbiddenException } from '@nestjs/common'; +import { of } from 'rxjs'; +import { describe, expect, it } from 'vitest'; +import { JwtStrategy } from '../strategies/jwt.strategy'; +import { ForcePasswordChangeInterceptor } from './force-password-change.interceptor'; + +/** + * ForcePasswordChangeInterceptor.intercept — pinnt Sperre, Erlaubnisliste + * und die Teilstring-Falle (260921-fi3, Aufgabe 1, Befund 1/D-01/D-02/D-03). + * Muster fuer `makeContext` aus `../../tenant/tenant.guard.spec.ts`, ergaenzt + * um `getHandler()`/`getClass()`, weil der Abfanger den Reflektor darauf + * anwendet. + */ + +function makeContext(request: any) { + return { + getHandler: () => ({}), + getClass: () => ({}), + switchToHttp: () => ({ + getRequest: () => request, + }), + } as any; +} + +function makeReflector(isPublic: boolean) { + return { getAllAndOverride: () => isPublic } as any; +} + +const nextHandle = { handle: () => of('ok') } as any; + +describe('ForcePasswordChangeInterceptor.intercept', () => { + it('Nahttest (D-03): JwtStrategy.validate() -> request.user -> GET /users wirft ForbiddenException — scheitert gegen den heutigen Quelltext, weil das Feld auf dem Weg verloren geht', async () => { + const strategy = new JwtStrategy({ get: () => 'test-secret' } as any); + const user = await strategy.validate({ + sub: 'u1', + username: 'admin', + role: 'ADMIN', + tenantId: 't1', + mustChangePassword: true, + }); + const interceptor = new ForcePasswordChangeInterceptor(makeReflector(false)); + const request = { user, method: 'GET', route: { path: '/users' } }; + + expect(() => interceptor.intercept(makeContext(request), nextHandle)).toThrow( + ForbiddenException, + ); + }); + + it('Erlaubnisliste: GET /auth/me laeuft durch, auch mit gesetzter Kennzeichnung', () => { + const interceptor = new ForcePasswordChangeInterceptor(makeReflector(false)); + const request = { + user: { id: 'u1', mustChangePassword: true }, + method: 'GET', + route: { path: '/auth/me' }, + }; + + expect(() => interceptor.intercept(makeContext(request), nextHandle)).not.toThrow(); + }); + + it('Erlaubnisliste: POST /auth/logout laeuft durch, auch mit gesetzter Kennzeichnung', () => { + const interceptor = new ForcePasswordChangeInterceptor(makeReflector(false)); + const request = { + user: { id: 'u1', mustChangePassword: true }, + method: 'POST', + route: { path: '/auth/logout' }, + }; + + expect(() => interceptor.intercept(makeContext(request), nextHandle)).not.toThrow(); + }); + + it('Erlaubnisliste: POST /auth/change-password laeuft durch, auch mit gesetzter Kennzeichnung', () => { + const interceptor = new ForcePasswordChangeInterceptor(makeReflector(false)); + const request = { + user: { id: 'u1', mustChangePassword: true }, + method: 'POST', + route: { path: '/auth/change-password' }, + }; + + expect(() => interceptor.intercept(makeContext(request), nextHandle)).not.toThrow(); + }); + + it('Falsche Methode auf erlaubtem Pfad: GET /auth/logout wird blockiert', () => { + const interceptor = new ForcePasswordChangeInterceptor(makeReflector(false)); + const request = { + user: { id: 'u1', mustChangePassword: true }, + method: 'GET', + route: { path: '/auth/logout' }, + }; + + expect(() => interceptor.intercept(makeContext(request), nextHandle)).toThrow( + ForbiddenException, + ); + }); + + it('Teilstring-Falle: /modules/auth/me enthaelt einen erlaubten Pfad nur als Teilzeichenkette und wird blockiert', () => { + const interceptor = new ForcePasswordChangeInterceptor(makeReflector(false)); + const request = { + user: { id: 'u1', mustChangePassword: true }, + method: 'GET', + route: { path: '/modules/auth/me' }, + }; + + expect(() => interceptor.intercept(makeContext(request), nextHandle)).toThrow( + ForbiddenException, + ); + }); + + it('Kennzeichnung nicht gesetzt: GET /users laeuft durch (keine Verschaerfung fuer normale Sitzungen, D-02)', () => { + const interceptor = new ForcePasswordChangeInterceptor(makeReflector(false)); + const request = { + user: { id: 'u1', mustChangePassword: false }, + method: 'GET', + route: { path: '/users' }, + }; + + expect(() => interceptor.intercept(makeContext(request), nextHandle)).not.toThrow(); + }); + + it('Oeffentliche Route (Reflektor liefert true): laeuft durch, auch bei gesetzter Kennzeichnung', () => { + const interceptor = new ForcePasswordChangeInterceptor(makeReflector(true)); + const request = { + user: { id: 'u1', mustChangePassword: true }, + method: 'GET', + route: { path: '/users' }, + }; + + expect(() => interceptor.intercept(makeContext(request), nextHandle)).not.toThrow(); + }); + + it('Kein request.user: laeuft durch', () => { + const interceptor = new ForcePasswordChangeInterceptor(makeReflector(false)); + const request = { method: 'GET', route: { path: '/users' } }; + + expect(() => interceptor.intercept(makeContext(request), nextHandle)).not.toThrow(); + }); +}); diff --git a/apps/api/src/auth/interceptors/force-password-change.interceptor.ts b/apps/api/src/auth/interceptors/force-password-change.interceptor.ts index 222a8b5..9488da3 100644 --- a/apps/api/src/auth/interceptors/force-password-change.interceptor.ts +++ b/apps/api/src/auth/interceptors/force-password-change.interceptor.ts @@ -13,14 +13,30 @@ import { IS_PUBLIC_KEY } from '../decorators/public.decorator'; * Global interceptor: forces users with mustChangePassword=true to change * their password before accessing any other resource (Pitfall 5 / D-06). * - * Allowed routes when mustChangePassword=true: + * Allowed routes when mustChangePassword=true (method AND path, exact match): * - POST /auth/change-password (the password change endpoint itself) * - POST /auth/logout (user should always be able to log out) * - GET /auth/me (so frontend can detect mustChangePassword flag) * * All other routes return 403 with FORCE_PASSWORD_CHANGE message. - * T-02-14: Prevents bypass via direct API access. + * T-02-14: this is now true — and only true because JwtStrategy.validate() + * passes mustChangePassword through on request.user. If that field is ever + * trimmed from validate()'s return value again, this interceptor goes back + * to never blocking anything, silently (260921-fi3, D-01). */ +const ALLOWED_ROUTES = Object.freeze([ + { method: 'POST', path: '/auth/change-password' }, + { method: 'POST', path: '/auth/logout' }, + { method: 'GET', path: '/auth/me' }, +]); + +function normalizePath(request: any): string { + const raw = request.route?.path || request.url || ''; + const withoutQuery = raw.split('?')[0]; + const withoutTrailingSlash = withoutQuery.replace(/\/+$/, ''); + return withoutTrailingSlash === '' ? '/' : withoutTrailingSlash; +} + @Injectable() export class ForcePasswordChangeInterceptor implements NestInterceptor { constructor(private reflector: Reflector) {} @@ -44,20 +60,21 @@ export class ForcePasswordChangeInterceptor implements NestInterceptor { } // User doesn't need to change password - if (!user.mustChangePassword) { + if (user.mustChangePassword !== true) { return next.handle(); } - // Allow specific routes even when password change is required - const path = request.route?.path || request.url; + // Allow specific routes even when password change is required — + // method AND path must match exactly (no substring matching: a path + // that merely contains an allowed path, e.g. /modules/auth/me, must + // not slip through). + const method = String(request.method || '').toUpperCase(); + const path = normalizePath(request); - const allowedPaths = [ - '/auth/change-password', - '/auth/logout', - '/auth/me', - ]; - - if (allowedPaths.some((allowed) => path.includes(allowed))) { + const isAllowed = ALLOWED_ROUTES.some( + (route) => route.method === method && route.path === path, + ); + if (isAllowed) { return next.handle(); } diff --git a/apps/api/src/auth/strategies/jwt.strategy.spec.ts b/apps/api/src/auth/strategies/jwt.strategy.spec.ts new file mode 100644 index 0000000..83e4f16 --- /dev/null +++ b/apps/api/src/auth/strategies/jwt.strategy.spec.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from 'vitest'; +import { JwtStrategy } from './jwt.strategy'; + +/** + * JwtStrategy.validate — pinnt die Durchreichung von mustChangePassword + * (260921-fi3, Aufgabe 1, Befund 1/D-01). Direkte Konstruktion ohne + * Nest-Testmodul, Muster aus `../../tenant/tenant.guard.spec.ts`. + */ + +function makeConfigService() { + return { get: () => 'test-secret' } as any; +} + +describe('JwtStrategy.validate', () => { + it('Anspruch mustChangePassword=true im Token: liefert request.user.mustChangePassword === true', async () => { + const strategy = new JwtStrategy(makeConfigService()); + + const result = await strategy.validate({ + sub: 'u1', + username: 'admin', + role: 'ADMIN', + tenantId: 't1', + mustChangePassword: true, + }); + + expect(result.mustChangePassword).toBe(true); + }); + + it('Anspruch fehlt im Token (Alt-Sitzung, vor dieser Aenderung ausgestellt): liefert false statt undefined', async () => { + const strategy = new JwtStrategy(makeConfigService()); + + const result = await strategy.validate({ + sub: 'u1', + username: 'admin', + role: 'ADMIN', + tenantId: 't1', + }); + + expect(result.mustChangePassword).toBe(false); + }); + + it('id, username, role und tenantId werden unveraendert wie bisher durchgereicht', async () => { + const strategy = new JwtStrategy(makeConfigService()); + + const result = await strategy.validate({ + sub: 'u1', + username: 'nutzer1', + role: 'USER', + tenantId: 't2', + mustChangePassword: false, + }); + + expect(result).toEqual({ + id: 'u1', + username: 'nutzer1', + role: 'USER', + tenantId: 't2', + mustChangePassword: false, + }); + }); +}); diff --git a/apps/api/src/auth/strategies/jwt.strategy.ts b/apps/api/src/auth/strategies/jwt.strategy.ts index bb8b970..b80a480 100644 --- a/apps/api/src/auth/strategies/jwt.strategy.ts +++ b/apps/api/src/auth/strategies/jwt.strategy.ts @@ -30,6 +30,10 @@ export class JwtStrategy extends PassportStrategy(Strategy) { username: payload.username, role: payload.role, tenantId: payload.tenantId, + // Ein vor dieser Aenderung ausgestelltes Token traegt diesen Anspruch + // nicht; der strenge Vergleich ergibt dann false, laufende Sitzungen + // verhalten sich unveraendert (260921-fi3, D-01 — keine Aussperrwelle). + mustChangePassword: payload.mustChangePassword === true, }; } }