fix: PermissionsGuard schliesst jetzt statt bei fehlendem Decorator zu oeffnen
PermissionsGuard erlaubte bisher jede Route ohne @RequirePermission automatisch fuer jeden eingeloggten Nutzer (fail-open) und verliess sich allein auf test/authorization-coverage.e2e-spec.ts, um eine vergessene Decorator-Angabe vor dem Deploy zu erwischen - der Guard selbst hatte kein Default-Deny. Neuer Decorator @AnyAuthenticatedUser() macht das bisher stillschweigend erlaubte Verhalten explizit fuer die 12 Routen, die es tatsaechlich brauchen (ProfileController komplett, NotificationsController komplett, AuthController.me - alle bereits vorher auf der Allowlist des e2e-Tests). PermissionsGuard verweigert jetzt jede Route, die weder @RequirePermission noch @AnyAuthenticatedUser traegt - eine vergessene Decorator-Angabe schlaegt jetzt zur Laufzeit fehl (403), nicht mehr nur im Test. Der e2e-Test prueft jetzt denselben Decorator-Vertrag statt einer separat gepflegten Allowlist, kann also nicht mehr vom Guard-Verhalten abweichen. Neue Unit-Tests fuer PermissionsGuard decken beide Codepfade und das Fail-Closed-Verhalten ab. Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
1 parent
b7ff92abd3
commit
19d5e5d65c
7 files changed
+151
-36
No files matched your search
@@ -9,28 +9,20 @@ import { PATH_METADATA } from '@nestjs/common/constants';
|
||||
import { AppModule } from '../src/app.module';
|
||||
import { IS_PUBLIC_KEY } from '../src/auth/decorators/public.decorator';
|
||||
import { REQUIRE_PERMISSION_KEY } from '../src/auth/decorators/require-permission.decorator';
|
||||
import { ANY_AUTHENTICATED_USER_KEY } from '../src/auth/decorators/any-authenticated-user.decorator';
|
||||
import { PERMISSION_CATALOG } from '../src/auth/permissions/permission-catalog';
|
||||
|
||||
// Routes intentionally left without a role/permission check: they act on the
|
||||
// caller's own data (notifications) or are the auth bootstrap endpoint that
|
||||
// every authenticated user must be able to reach regardless of permissions.
|
||||
const ALLOWLIST = new Set([
|
||||
'NotificationsController.findAll',
|
||||
'NotificationsController.markRead',
|
||||
'AuthController.me',
|
||||
'ProfileController.updateProfile',
|
||||
'ProfileController.changePassword',
|
||||
'ProfileController.deactivateAccount',
|
||||
'ProfileController.uploadAvatar',
|
||||
'ProfileController.deleteAvatar',
|
||||
'ProfileController.getAvatar',
|
||||
'ProfileController.enrollMfa',
|
||||
'ProfileController.confirmMfaEnrollment',
|
||||
'ProfileController.disableMfa',
|
||||
]);
|
||||
|
||||
/**
|
||||
* Mirrors PermissionsGuard's own runtime contract exactly (see
|
||||
* src/auth/guards/permissions.guard.ts): every non-public route must carry
|
||||
* either @RequirePermission or @AnyAuthenticatedUser, or the guard denies it
|
||||
* at runtime regardless of what this test says. This test exists to catch
|
||||
* the mistake at build time - with a clear message pointing at the missing
|
||||
* decorator - rather than as a live production 403 discovered by a user, but
|
||||
* a route that fails it would also fail closed live, not open.
|
||||
*/
|
||||
describe('Authorization coverage (e2e)', () => {
|
||||
it('every non-public route requires a catalog-valid permission', async () => {
|
||||
it('every non-public route requires a permission or an explicit any-authenticated-user opt-in', async () => {
|
||||
const moduleRef = await Test.createTestingModule({
|
||||
imports: [AppModule, DiscoveryModule],
|
||||
}).compile();
|
||||
@@ -68,7 +60,7 @@ describe('Authorization coverage (e2e)', () => {
|
||||
const routeKey = `${controllerName}.${methodName}`;
|
||||
const isPublicMethod =
|
||||
reflector.get<boolean>(IS_PUBLIC_KEY, handler) === true;
|
||||
if (isPublicClass || isPublicMethod || ALLOWLIST.has(routeKey)) {
|
||||
if (isPublicClass || isPublicMethod) {
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -76,10 +68,15 @@ describe('Authorization coverage (e2e)', () => {
|
||||
REQUIRE_PERMISSION_KEY,
|
||||
[handler, constructor],
|
||||
);
|
||||
|
||||
const hasPermissions =
|
||||
Array.isArray(permissions) && permissions.length > 0;
|
||||
if (!hasPermissions) {
|
||||
const isAnyAuthenticatedUser =
|
||||
reflector.getAllAndOverride<boolean>(ANY_AUTHENTICATED_USER_KEY, [
|
||||
handler,
|
||||
constructor,
|
||||
]) === true;
|
||||
|
||||
if (!hasPermissions && !isAnyAuthenticatedUser) {
|
||||
uncoveredRoutes.push(routeKey);
|
||||
}
|
||||
if (hasPermissions) {
|
||||
|
||||
Reference in new issue
Block a user