From adfbc2ab839f8e272a033104768bfab632a8d888 Mon Sep 17 00:00:00 2001 From: Nick Szaunig Date: Wed, 26 Aug 2026 14:17:49 +0200 Subject: [PATCH] fix: Access-Token wird bei deaktivierten Usern sofort ungueltig Bisher griff der isActive-Check nur auf Routen mit @RequirePermission. Ein deaktivierter User mit gueltigem Token konnte Routen ohne Permission-Decorator bis zum Token-Ablauf (bis zu 15 Min) weiter erreichen. JwtAuthGuard prueft jetzt PermissionResolverService.isUserActive auf jedem authentifizierten Request, ueber denselben 30s-Cache wie die Permission-Aufloesung, also ohne zusaetzliche DB-Last. --- .../src/auth/guards/jwt-auth.guard.spec.ts | 70 +++++++++++++++++++ .../src/auth/guards/jwt-auth.guard.ts | 39 +++++++++-- .../permission-resolver.service.spec.ts | 51 ++++++++++++++ .../permission-resolver.service.ts | 50 +++++++++---- 4 files changed, 191 insertions(+), 19 deletions(-) create mode 100644 ApiCore-Backend/src/auth/guards/jwt-auth.guard.spec.ts diff --git a/ApiCore-Backend/src/auth/guards/jwt-auth.guard.spec.ts b/ApiCore-Backend/src/auth/guards/jwt-auth.guard.spec.ts new file mode 100644 index 0000000..a5b01c5 --- /dev/null +++ b/ApiCore-Backend/src/auth/guards/jwt-auth.guard.spec.ts @@ -0,0 +1,70 @@ +import { ExecutionContext, UnauthorizedException } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { JwtAuthGuard } from './jwt-auth.guard'; +import { PermissionResolverService } from '../permissions/permission-resolver.service'; + +describe('JwtAuthGuard', () => { + let guard: JwtAuthGuard; + let reflector: { getAllAndOverride: jest.Mock }; + let permissionResolver: { isUserActive: jest.Mock }; + + function contextWithUser(user: { id: string } | undefined): ExecutionContext { + const request = { user }; + return { + getHandler: () => ({}), + getClass: () => ({}), + switchToHttp: () => ({ getRequest: () => request }), + } as unknown as ExecutionContext; + } + + beforeEach(() => { + reflector = { getAllAndOverride: jest.fn().mockReturnValue(false) }; + permissionResolver = { isUserActive: jest.fn() }; + guard = new JwtAuthGuard( + reflector as unknown as Reflector, + permissionResolver as unknown as PermissionResolverService, + ); + }); + + it('allows public routes through without checking authentication', async () => { + reflector.getAllAndOverride.mockReturnValue(true); + + await expect(guard.canActivate(contextWithUser(undefined))).resolves.toBe( + true, + ); + expect(permissionResolver.isUserActive).not.toHaveBeenCalled(); + }); + + it('allows the request when the JWT is valid and the user is active', async () => { + const context = contextWithUser({ id: 'user-1' }); + jest + .spyOn( + Object.getPrototypeOf(JwtAuthGuard.prototype) as { + canActivate: (context: ExecutionContext) => boolean; + }, + 'canActivate', + ) + .mockReturnValue(true); + permissionResolver.isUserActive.mockResolvedValue(true); + + await expect(guard.canActivate(context)).resolves.toBe(true); + expect(permissionResolver.isUserActive).toHaveBeenCalledWith('user-1'); + }); + + it('throws UnauthorizedException when the authenticated user has been deactivated', async () => { + const context = contextWithUser({ id: 'user-1' }); + jest + .spyOn( + Object.getPrototypeOf(JwtAuthGuard.prototype) as { + canActivate: (context: ExecutionContext) => boolean; + }, + 'canActivate', + ) + .mockReturnValue(true); + permissionResolver.isUserActive.mockResolvedValue(false); + + await expect(guard.canActivate(context)).rejects.toBeInstanceOf( + UnauthorizedException, + ); + }); +}); diff --git a/ApiCore-Backend/src/auth/guards/jwt-auth.guard.ts b/ApiCore-Backend/src/auth/guards/jwt-auth.guard.ts index 345c4ad..091602c 100644 --- a/ApiCore-Backend/src/auth/guards/jwt-auth.guard.ts +++ b/ApiCore-Backend/src/auth/guards/jwt-auth.guard.ts @@ -1,15 +1,37 @@ -import { ExecutionContext, Injectable } from '@nestjs/common'; +import { + ExecutionContext, + Injectable, + UnauthorizedException, +} from '@nestjs/common'; import { Reflector } from '@nestjs/core'; import { AuthGuard } from '@nestjs/passport'; import { IS_PUBLIC_KEY } from '../decorators/public.decorator'; +import { PermissionResolverService } from '../permissions/permission-resolver.service'; +import { AuthenticatedUser } from '../types/authenticated-user'; +interface RequestWithUser { + user?: AuthenticatedUser; +} + +/** + * Enforces User.isActive on every authenticated request, not just ones + * gated by @RequirePermission. Without this, a deactivated user's + * still-valid access token would keep working on any route that carries no + * permission decorator until the token's own expiry (up to + * JWT_ACCESS_EXPIRES_IN). isUserActive shares PermissionsGuard's cache, so + * this check costs no extra query beyond what permission resolution already + * does. + */ @Injectable() export class JwtAuthGuard extends AuthGuard('jwt') { - constructor(private readonly reflector: Reflector) { + constructor( + private readonly reflector: Reflector, + private readonly permissionResolver: PermissionResolverService, + ) { super(); } - canActivate(context: ExecutionContext) { + async canActivate(context: ExecutionContext): Promise { const isPublic = this.reflector.getAllAndOverride(IS_PUBLIC_KEY, [ context.getHandler(), context.getClass(), @@ -17,6 +39,15 @@ export class JwtAuthGuard extends AuthGuard('jwt') { if (isPublic) { return true; } - return super.canActivate(context); + const authenticated = (await super.canActivate(context)) as boolean; + if (!authenticated) { + return false; + } + const request = context.switchToHttp().getRequest(); + const user = request.user; + if (!user || !(await this.permissionResolver.isUserActive(user.id))) { + throw new UnauthorizedException(); + } + return true; } } diff --git a/ApiCore-Backend/src/auth/permissions/permission-resolver.service.spec.ts b/ApiCore-Backend/src/auth/permissions/permission-resolver.service.spec.ts index 1a7da8a..9564598 100644 --- a/ApiCore-Backend/src/auth/permissions/permission-resolver.service.spec.ts +++ b/ApiCore-Backend/src/auth/permissions/permission-resolver.service.spec.ts @@ -93,6 +93,57 @@ describe('PermissionResolverService', () => { expect(prisma.user.findUnique).toHaveBeenCalledTimes(2); }); + describe('isUserActive', () => { + it('returns true for an active user with an active role', async () => { + prisma.user.findUnique.mockResolvedValue( + activeUserWithPermissions('colonies:read'), + ); + + await expect(resolver.isUserActive('user-1')).resolves.toBe(true); + }); + + it('returns false for a deactivated user', async () => { + prisma.user.findUnique.mockResolvedValue({ + isActive: false, + role: { + status: 'ACTIVE', + permissions: [{ permission: { key: 'colonies:read' } }], + }, + }); + + await expect(resolver.isUserActive('user-1')).resolves.toBe(false); + }); + + it('returns false when the role is inactive', async () => { + prisma.user.findUnique.mockResolvedValue({ + isActive: true, + role: { + status: 'INACTIVE', + permissions: [{ permission: { key: 'colonies:read' } }], + }, + }); + + await expect(resolver.isUserActive('user-1')).resolves.toBe(false); + }); + + it('returns false when the user does not exist', async () => { + prisma.user.findUnique.mockResolvedValue(null); + + await expect(resolver.isUserActive('missing')).resolves.toBe(false); + }); + + it('shares the cached entry with getPermissions instead of re-querying', async () => { + prisma.user.findUnique.mockResolvedValue( + activeUserWithPermissions('colonies:read'), + ); + + await resolver.getPermissions('user-1'); + await resolver.isUserActive('user-1'); + + expect(prisma.user.findUnique).toHaveBeenCalledTimes(1); + }); + }); + it('re-queries after invalidateAll', async () => { prisma.user.findUnique.mockResolvedValue( activeUserWithPermissions('colonies:read'), diff --git a/ApiCore-Backend/src/auth/permissions/permission-resolver.service.ts b/ApiCore-Backend/src/auth/permissions/permission-resolver.service.ts index e17acaa..d35f2de 100644 --- a/ApiCore-Backend/src/auth/permissions/permission-resolver.service.ts +++ b/ApiCore-Backend/src/auth/permissions/permission-resolver.service.ts @@ -3,6 +3,7 @@ import { PrismaService } from '../../prisma/prisma.service'; import { PermissionKey } from './permission-catalog'; interface CacheEntry { + isActive: boolean; permissions: ReadonlySet; expiresAt: number; } @@ -17,6 +18,11 @@ const CACHE_TTL_MS = 30_000; * invalidateUser/invalidateAll after any write to Role, Permission, * RolePermission, or User.roleId so the change is visible immediately * instead of waiting out the cache window. + * + * isUserActive shares the same cached entry: JwtAuthGuard calls it on every + * authenticated request (including routes with no @RequirePermission) so a + * deactivated user's still-valid access token stops working within the + * cache TTL instead of staying usable until the token expires. */ @Injectable() export class PermissionResolverService { @@ -25,16 +31,13 @@ export class PermissionResolverService { constructor(private readonly prisma: PrismaService) {} async getPermissions(userId: string): Promise> { - const cached = this.cache.get(userId); - if (cached && cached.expiresAt > Date.now()) { - return cached.permissions; - } - const permissions = await this.loadPermissions(userId); - this.cache.set(userId, { - permissions, - expiresAt: Date.now() + CACHE_TTL_MS, - }); - return permissions; + const entry = await this.getEntry(userId); + return entry.permissions; + } + + async isUserActive(userId: string): Promise { + const entry = await this.getEntry(userId); + return entry.isActive; } async hasAny( @@ -56,7 +59,17 @@ export class PermissionResolverService { this.cache.clear(); } - private async loadPermissions(userId: string): Promise> { + private async getEntry(userId: string): Promise { + const cached = this.cache.get(userId); + if (cached && cached.expiresAt > Date.now()) { + return cached; + } + const entry = await this.loadEntry(userId); + this.cache.set(userId, entry); + return entry; + } + + private async loadEntry(userId: string): Promise { const user = await this.prisma.user.findUnique({ where: { id: userId }, select: { @@ -71,9 +84,16 @@ export class PermissionResolverService { }, }, }); - if (!user || !user.isActive || user.role.status !== 'ACTIVE') { - return new Set(); - } - return new Set(user.role.permissions.map((grant) => grant.permission.key)); + const isEligible = Boolean( + user && user.isActive && user.role.status === 'ACTIVE', + ); + const permissions = isEligible + ? new Set(user!.role.permissions.map((grant) => grant.permission.key)) + : new Set(); + return { + isActive: isEligible, + permissions, + expiresAt: Date.now() + CACHE_TTL_MS, + }; } }