fix: PermissionsGuard blockiert seit dem Fail-Closed-Umbau alle @Public-Routen
Beim Umstellen von PermissionsGuard auf Fail-Closed wurde der @Public()-Check vergessen. JwtAuthGuard ueberspringt bei @Public() zwar die Authentifizierung, aber PermissionsGuard laeuft als eigenstaendiger, separat registrierter APP_GUARD trotzdem mit - und verweigerte seitdem jede Route ohne @RequirePermission/ @AnyAuthenticatedUser, selbst wenn sie @Public() war. Betroffen: health und health/ready (die gemeldeten 403), aber ebenso Login, SSO-Flow und die oeffentliche Herkunftsseite. PermissionsGuard prueft jetzt @Public() als erstes und laesst diese Routen durch, genau wie JwtAuthGuard es bereits tut. Regressionstest ergaenzt. Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
1 parent
351f9ef34c
commit
cd24523ea5
2 files changed
+30
-8
No files matched your search
@@ -3,6 +3,7 @@ import { Reflector } from '@nestjs/core';
|
|||||||
import { PermissionsGuard } from './permissions.guard';
|
import { PermissionsGuard } from './permissions.guard';
|
||||||
import { REQUIRE_PERMISSION_KEY } from '../decorators/require-permission.decorator';
|
import { REQUIRE_PERMISSION_KEY } from '../decorators/require-permission.decorator';
|
||||||
import { ANY_AUTHENTICATED_USER_KEY } from '../decorators/any-authenticated-user.decorator';
|
import { ANY_AUTHENTICATED_USER_KEY } from '../decorators/any-authenticated-user.decorator';
|
||||||
|
import { IS_PUBLIC_KEY } from '../decorators/public.decorator';
|
||||||
import { PermissionResolverService } from '../permissions/permission-resolver.service';
|
import { PermissionResolverService } from '../permissions/permission-resolver.service';
|
||||||
|
|
||||||
describe('PermissionsGuard', () => {
|
describe('PermissionsGuard', () => {
|
||||||
@@ -32,6 +33,15 @@ describe('PermissionsGuard', () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('allows a @Public route through with no user on the request (e.g. health checks, login)', async () => {
|
||||||
|
reflectorValues.set(IS_PUBLIC_KEY, true);
|
||||||
|
|
||||||
|
await expect(guard.canActivate(contextWithUser(undefined))).resolves.toBe(
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
expect(permissionResolver.hasAny).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it('denies a route that carries neither @RequirePermission nor @AnyAuthenticatedUser (fails closed)', async () => {
|
it('denies a route that carries neither @RequirePermission nor @AnyAuthenticatedUser (fails closed)', async () => {
|
||||||
await expect(
|
await expect(
|
||||||
guard.canActivate(contextWithUser({ id: 'user-1' })),
|
guard.canActivate(contextWithUser({ id: 'user-1' })),
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
|
import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
|
||||||
import { Reflector } from '@nestjs/core';
|
import { Reflector } from '@nestjs/core';
|
||||||
import { ANY_AUTHENTICATED_USER_KEY } from '../decorators/any-authenticated-user.decorator';
|
import { ANY_AUTHENTICATED_USER_KEY } from '../decorators/any-authenticated-user.decorator';
|
||||||
|
import { IS_PUBLIC_KEY } from '../decorators/public.decorator';
|
||||||
import { REQUIRE_PERMISSION_KEY } from '../decorators/require-permission.decorator';
|
import { REQUIRE_PERMISSION_KEY } from '../decorators/require-permission.decorator';
|
||||||
import { PermissionKey } from '../permissions/permission-catalog';
|
import { PermissionKey } from '../permissions/permission-catalog';
|
||||||
import { PermissionResolverService } from '../permissions/permission-resolver.service';
|
import { PermissionResolverService } from '../permissions/permission-resolver.service';
|
||||||
@@ -11,14 +12,17 @@ interface RequestWithUser {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Fails closed by decorator absence: a non-public route reaches this guard
|
* Fails closed by decorator absence: a route reaches this guard whether or
|
||||||
* only after JwtAuthGuard confirms a valid, active session, and from here it
|
* not it's @Public (JwtAuthGuard only skips authentication for those, it
|
||||||
* is denied unless it carries either @RequirePermission (checked against the
|
* doesn't remove this guard from the chain - NestJS APP_GUARD entries all
|
||||||
* caller's live permission set) or the explicit @AnyAuthenticatedUser opt-out
|
* run, there's no early-exit between them), so @Public is checked here too,
|
||||||
* for routes that are meant to be reachable by any logged-in user regardless
|
* the same way JwtAuthGuard checks it. A non-public route is denied unless
|
||||||
* of role. A route with neither is a bug - forgetting to decorate a new
|
* it carries either @RequirePermission (checked against the caller's live
|
||||||
* route now fails shut at runtime instead of silently granting access to
|
* permission set) or the explicit @AnyAuthenticatedUser opt-out for routes
|
||||||
* every authenticated user and relying solely on
|
* meant to be reachable by any logged-in user regardless of role. A route
|
||||||
|
* with none of the three is a bug - forgetting to decorate a new route now
|
||||||
|
* fails shut at runtime instead of silently granting access to every
|
||||||
|
* authenticated user and relying solely on
|
||||||
* test/authorization-coverage.e2e-spec.ts to catch it before deploy.
|
* test/authorization-coverage.e2e-spec.ts to catch it before deploy.
|
||||||
*/
|
*/
|
||||||
@Injectable()
|
@Injectable()
|
||||||
@@ -29,6 +33,14 @@ export class PermissionsGuard implements CanActivate {
|
|||||||
) {}
|
) {}
|
||||||
|
|
||||||
async canActivate(context: ExecutionContext): Promise<boolean> {
|
async canActivate(context: ExecutionContext): Promise<boolean> {
|
||||||
|
const isPublic = this.reflector.getAllAndOverride<boolean>(IS_PUBLIC_KEY, [
|
||||||
|
context.getHandler(),
|
||||||
|
context.getClass(),
|
||||||
|
]);
|
||||||
|
if (isPublic) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
const requiredPermissions = this.reflector.getAllAndOverride<
|
const requiredPermissions = this.reflector.getAllAndOverride<
|
||||||
PermissionKey[]
|
PermissionKey[]
|
||||||
>(REQUIRE_PERMISSION_KEY, [context.getHandler(), context.getClass()]);
|
>(REQUIRE_PERMISSION_KEY, [context.getHandler(), context.getClass()]);
|
||||||
|
|||||||
Reference in new issue
Block a user