Refactor guard decorators

.changeset/great-hotels-unite.md

@@ -0,0 +1,10 @@
+---
+"@comet/cms-api": major
+---
+
+Refactor auth-decorators
+
+-   Remove `@PublicApi()`-decorator
+-   Rename `@DisableGlobalGuard()`-decorator to `@DisableCometGuards()`
+
+The `@DisableCometGuards()`-decorator will only disable the AuthGuard when no `x-include-invisible-content`-header is set.

demo/api/src/menus/menus.resolver.ts

@@ -1,4 +1,4 @@
-import { PageTreeNodeInterface, PageTreeReadApiService, PublicApi } from "@comet/cms-api";
+import { PageTreeNodeInterface, PageTreeReadApiService, RequiredPermission } from "@comet/cms-api";
 import { InjectRepository } from "@mikro-orm/nestjs";
 import { EntityRepository } from "@mikro-orm/postgresql";
 import { Args, Query, Resolver } from "@nestjs/graphql";
@@ -10,7 +10,7 @@ import { MainMenuObject } from "./dto/main-menu.object";
 import { MainMenuItem } from "./entities/main-menu-item.entity";
 
 @Resolver(() => MainMenuObject)
-@PublicApi()
+@RequiredPermission("pageTree")
 export class MenusResolver {
     constructor(
         private readonly pageTreeReadApi: PageTreeReadApiService,

docs/docs/auth/index.md

@@ -1,30 +1,20 @@
 ---
-title: Authentication
+title: Authorization
 sidebar_position: 10
 ---
 
-TODO general information about how we handle authentication
+TODO general information about how we handle authorization
 
 ## Available Decorators
 
-### @PublicApi
+### @DisableCometGuards
 
-`@PublicApi()` can be used to expose a single handler (query, mutation or route) or a whole class (resolver or controller) publicly.
-
-:::caution
-
-Using the decorator at class level causes later added handlers to be automatically public. Prefer using the decorator for single handlers only.
-
-:::
-
-### @DisableGlobalAuthGuard
-
-`@DisableGlobalAuthGuard()` disables the global auth guard (`CometAuthGuard`). This may be used if a different authentication method is desired (e.g., basic authentication) for a specific handler or class. It should be used in combination with a custom guard. The custom guard may leverage `@PublicApi` as well to expose handlers publicly.
+`@DisableCometGuards()` disables the global auth guards (`CometAuthGuard`, `UserPermissionsGuard`). This may be used if a different authentication method is desired (e.g., basic authentication) for a specific handler or class in combination with a custom guard.
 
 e.g.:
 
 ```typescript
-@DisableGlobalGuard()
+@DisableCometGuards()
 @UseGuards(MyCustomGuard)
 async handlerThatUsesACustomGuard(): {
     ...

packages/api/cms-api/src/auth/decorators/disable-comet-guards.decorator.ts

@@ -0,0 +1,5 @@
+import { CustomDecorator, SetMetadata } from "@nestjs/common";
+
+export const DisableCometGuards = (): CustomDecorator<string> => {
+    return SetMetadata("disableCometGuards", true);
+};

packages/api/cms-api/src/auth/guards/comet.guard.ts

@@ -30,14 +30,9 @@ export function createCometAuthGuard(type?: string | string[]): Type<IAuthGuard>
         }
 
         async canActivate(context: ExecutionContext): Promise<boolean> {
-            if (this.reflector.getAllAndOverride("disableGlobalGuard", [context.getHandler(), context.getClass()])) {
-                return true;
-            }
-
-            const isPublicApi = this.reflector.getAllAndOverride("publicApi", [context.getHandler(), context.getClass()]);
-            const includeInvisibleContent = !!this.getRequest(context).headers["x-include-invisible-content"];
-
-            if (isPublicApi && !includeInvisibleContent) {
+            const disableCometGuard = this.reflector.getAllAndOverride("disableCometGuards", [context.getHandler(), context.getClass()]);
+            const hasIncludeInvisibleContentHeader = !!this.getRequest(context).headers["x-include-invisible-content"];
+            if (disableCometGuard && !hasIncludeInvisibleContentHeader) {
                 return true;
             }
 

packages/api/cms-api/src/auth/resolver/auth.resolver.ts

@@ -3,9 +3,9 @@ import { Context, Mutation, Query, Resolver } from "@nestjs/graphql";
 import { IncomingMessage } from "http";
 
 import { SkipBuild } from "../../builds/skip-build.decorator";
+import { DisablePermissionCheck, RequiredPermission } from "../../user-permissions/decorators/required-permission.decorator";
 import { CurrentUser } from "../../user-permissions/dto/current-user";
 import { GetCurrentUser } from "../decorators/get-current-user.decorator";
-import { PublicApi } from "../decorators/public-api.decorator";
 
 interface AuthResolverConfig {
     currentUser?: Type<CurrentUser>; // TODO Remove in future version as it is not used and here for backwards compatibility
@@ -15,7 +15,7 @@ interface AuthResolverConfig {
 
 export function createAuthResolver(config?: AuthResolverConfig): Type<unknown> {
     @Resolver(() => CurrentUser)
-    @PublicApi()
+    @RequiredPermission(DisablePermissionCheck)
     class AuthResolver {
         @Query(() => CurrentUser)
         async currentUser(@GetCurrentUser() user: CurrentUser): Promise<CurrentUser> {

packages/api/cms-api/src/dam/files/files.controller.ts

@@ -19,8 +19,8 @@ import { validate } from "class-validator";
 import { Response } from "express";
 import { OutgoingHttpHeaders } from "http";
 
+import { DisableCometGuards } from "../../auth/decorators/disable-comet-guards.decorator";
 import { GetCurrentUser } from "../../auth/decorators/get-current-user.decorator";
-import { DisableGlobalGuard } from "../../auth/decorators/global-guard-disable.decorator";
 import { BlobStorageBackendService } from "../../blob-storage/backends/blob-storage-backend.service";
 import { CometValidationException } from "../../common/errors/validation.exception";
 import { RequiredPermission } from "../../user-permissions/decorators/required-permission.decorator";
@@ -111,7 +111,7 @@ export function createFilesController({ Scope: PassedScope }: { Scope?: Type<Dam
             return this.streamFile(file, res, { range, overrideHeaders: { "Cache-control": "private" } });
         }
 
-        @DisableGlobalGuard()
+        @DisableCometGuards()
         @Get(`/:hash/${fileUrl}`)
         async hashedFileUrl(@Param() { hash, ...params }: HashFileParams, @Res() res: Response, @Headers("range") range?: string): Promise<void> {
             if (!this.isValidHash(hash, params)) {

packages/api/cms-api/src/dam/images/images.controller.ts

@@ -6,8 +6,8 @@ import mime from "mime";
 import fetch from "node-fetch";
 import { PassThrough } from "stream";
 
+import { DisableCometGuards } from "../../auth/decorators/disable-comet-guards.decorator";
 import { GetCurrentUser } from "../../auth/decorators/get-current-user.decorator";
-import { DisableGlobalGuard } from "../../auth/decorators/global-guard-disable.decorator";
 import { BlobStorageBackendService } from "../../blob-storage/backends/blob-storage-backend.service";
 import { RequiredPermission } from "../../user-permissions/decorators/required-permission.decorator";
 import { CurrentUser } from "../../user-permissions/dto/current-user";
@@ -106,7 +106,7 @@ export class ImagesController {
         });
     }
 
-    @DisableGlobalGuard()
+    @DisableCometGuards()
     @Get(`/:hash/${smartImageUrl}`)
     async smartCroppedImage(@Param() params: HashImageParams, @Headers("Accept") accept: string, @Res() res: Response): Promise<void> {
         if (!this.isValidHash(params) || params.cropArea.focalPoint !== FocalPoint.SMART) {
@@ -122,7 +122,7 @@ export class ImagesController {
         return this.getCroppedImage(file, params, accept, res);
     }
 
-    @DisableGlobalGuard()
+    @DisableCometGuards()
     @Get(`/:hash/${focusImageUrl}`)
     async focusCroppedImage(@Param() params: HashImageParams, @Headers("Accept") accept: string, @Res() res: Response): Promise<void> {
         if (!this.isValidHash(params) || params.cropArea.focalPoint === FocalPoint.SMART) {

packages/api/cms-api/src/index.ts

@@ -1,9 +1,8 @@
 import "reflect-metadata";
 
 export { AccessLogModule } from "./access-log/access-log.module";
+export { DisableCometGuards } from "./auth/decorators/disable-comet-guards.decorator";
 export { GetCurrentUser } from "./auth/decorators/get-current-user.decorator";
-export { DisableGlobalGuard } from "./auth/decorators/global-guard-disable.decorator";
-export { PublicApi } from "./auth/decorators/public-api.decorator";
 export { createCometAuthGuard } from "./auth/guards/comet.guard";
 export { createAuthResolver } from "./auth/resolver/auth.resolver";
 export { createAuthProxyJwtStrategy } from "./auth/strategies/auth-proxy-jwt.strategy";

packages/api/cms-api/src/public-upload/public-uploads.controller.ts

@@ -2,20 +2,20 @@ import { EntityManager } from "@mikro-orm/postgresql";
 import { Controller, Post, UploadedFile, UseInterceptors } from "@nestjs/common";
 import rimraf from "rimraf";
 
-import { PublicApi } from "../auth/decorators/public-api.decorator";
+import { DisableCometGuards } from "../auth/decorators/disable-comet-guards.decorator";
 import { PublicUploadFileUploadInterface } from "./dto/public-upload-file-upload.interface";
 import { PublicUpload } from "./entities/public-upload.entity";
 import { PublicUploadFileInterceptor } from "./public-upload-file.interceptor";
 import { PublicUploadsService } from "./public-uploads.service";
 
 @Controller("public-upload/files")
-@PublicApi()
+@DisableCometGuards()
 export class PublicUploadsController {
     constructor(private readonly publicUploadsService: PublicUploadsService, private readonly entityManager: EntityManager) {}
 
     @Post("upload")
     @UseInterceptors(PublicUploadFileInterceptor("file"))
-    @PublicApi()
+    @DisableCometGuards()
     async upload(@UploadedFile() file: PublicUploadFileUploadInterface): Promise<PublicUpload> {
         const publicUploadsFile = await this.publicUploadsService.upload(file);
 

packages/api/cms-api/src/user-permissions/auth/user-permissions.guard.ts

@@ -3,7 +3,7 @@ import { Reflector } from "@nestjs/core";
 import { GqlContextType, GqlExecutionContext } from "@nestjs/graphql";
 
 import { ContentScopeService } from "../content-scope.service";
-import { RequiredPermissionMetadata } from "../decorators/required-permission.decorator";
+import { DisablePermissionCheck, RequiredPermissionMetadata } from "../decorators/required-permission.decorator";
 import { CurrentUser } from "../dto/current-user";
 import { ACCESS_CONTROL_SERVICE } from "../user-permissions.constants";
 import { AccessControlServiceInterface, SystemUser } from "../user-permissions.types";
@@ -19,8 +19,7 @@ export class UserPermissionsGuard implements CanActivate {
     async canActivate(context: ExecutionContext): Promise<boolean> {
         const location = `${context.getClass().name}::${context.getHandler().name}()`;
 
-        if (this.getDecorator(context, "disableGlobalGuard")) return true;
-        if (this.getDecorator(context, "publicApi")) return true;
+        if (this.getDecorator(context, "disableCometGuards")) return true;
 
         const user = this.getUser(context);
         if (!user) return false;
@@ -32,6 +31,7 @@ export class UserPermissionsGuard implements CanActivate {
         if (!requiredPermission && this.isResolvingGraphQLField(context)) return true;
         if (!requiredPermission) throw new Error(`RequiredPermission decorator is missing in ${location}`);
         const requiredPermissions = requiredPermission.requiredPermission;
+        if (requiredPermissions.includes(DisablePermissionCheck)) return true;
         if (requiredPermissions.length === 0) throw new Error(`RequiredPermission decorator has empty permissions in ${location}`);
         if (this.isResolvingGraphQLField(context) || requiredPermission.options?.skipScopeCheck) {
             // At least one permission is required

packages/api/cms-api/src/user-permissions/decorators/required-permission.decorator.ts

@@ -9,7 +9,12 @@ export type RequiredPermissionMetadata = {
     options: RequiredPermissionOptions | undefined;
 };
 
-export const RequiredPermission = (requiredPermission: string | string[], options?: RequiredPermissionOptions): CustomDecorator<string> => {
+export const DisablePermissionCheck = "disablePermissionCheck";
+
+export const RequiredPermission = (
+    requiredPermission: string | string[] | "disablePermissionCheck",
+    options?: RequiredPermissionOptions,
+): CustomDecorator<string> => {
     return SetMetadata<string, RequiredPermissionMetadata>("requiredPermission", {
         requiredPermission: Array.isArray(requiredPermission) ? requiredPermission : [requiredPermission],
         options,

packages/api/cms-api/src/user-permissions/user-permissions.service.ts

@@ -6,7 +6,7 @@ import { isFuture, isPast } from "date-fns";
 import isEqual from "lodash.isequal";
 import getUuid from "uuid-by-string";
 
-import { RequiredPermissionMetadata } from "./decorators/required-permission.decorator";
+import { DisablePermissionCheck, RequiredPermissionMetadata } from "./decorators/required-permission.decorator";
 import { CurrentUser } from "./dto/current-user";
 import { FindUsersArgs } from "./dto/paginated-user-list";
 import { User } from "./dto/user";
@@ -52,6 +52,7 @@ export class UserPermissionsService {
                     ...(await this.discoveryService.controllersWithMetaAtKey<RequiredPermissionMetadata>("requiredPermission")),
                 ]
                     .flatMap((p) => p.meta.requiredPermission)
+                    .filter((p) => p !== DisablePermissionCheck)
                     .sort(),
             ),
         ];