From b468459b1aa1bd2f56b01e268083597bda4ed2f3 Mon Sep 17 00:00:00 2001 From: bo0tzz Date: Tue, 28 Jul 2026 15:44:51 +0200 Subject: [PATCH] chore: add test for admin controller permissions (#30316) --- server/src/controllers/index.spec.ts | 71 ++++++++++++++++++++++++++++ server/src/middleware/auth.guard.ts | 19 ++++---- 2 files changed, 82 insertions(+), 8 deletions(-) create mode 100644 server/src/controllers/index.spec.ts diff --git a/server/src/controllers/index.spec.ts b/server/src/controllers/index.spec.ts new file mode 100644 index 0000000000..67962e8b3c --- /dev/null +++ b/server/src/controllers/index.spec.ts @@ -0,0 +1,71 @@ +import { RequestMethod } from '@nestjs/common'; +import { METHOD_METADATA, PATH_METADATA } from '@nestjs/common/constants'; +import { Reflector } from '@nestjs/core'; +import { controllers } from 'src/controllers'; +import { AuthenticatedOptions, getAuthenticatedOptions } from 'src/middleware/auth.guard'; + +const UNAUTHENTICATED_ADMIN_ROUTES = new Set([ + 'GET admin/maintenance/status', + 'POST admin/maintenance/login', + 'POST admin/database-backups/start-restore', +]); + +const isAdminPermission = (permission: AuthenticatedOptions['permission']) => + typeof permission === 'string' && permission.startsWith('admin'); + +const getRoutes = () => { + const reflector = new Reflector(); + + return controllers.flatMap((Controller) => { + const prefix = reflector.get(PATH_METADATA, Controller); + + return Object.getOwnPropertyNames(Controller.prototype).flatMap((name) => { + const handler = Object.getOwnPropertyDescriptor(Controller.prototype, name)?.value; + if (typeof handler !== 'function') { + return []; + } + + const requestMethod = reflector.get(METHOD_METADATA, handler); + if (requestMethod === undefined) { + return []; + } + + const method = RequestMethod[requestMethod]; + const path = [prefix, reflector.get(PATH_METADATA, handler)].filter((part) => part !== '/').join('/'); + + return { + id: `${method} ${path}`, + label: `${Controller.name}.${name} (${method} /${path})`, + path, + auth: getAuthenticatedOptions(reflector, handler), + }; + }); + }); +}; + +describe('controllers', () => { + const routes = getRoutes(); + const adminRoutes = routes.filter((route) => route.path === 'admin' || route.path.startsWith('admin/')); + + it('should only allow non-admin access to bootstrap routes under admin/', () => { + const reachableByNonAdmins = adminRoutes.filter((route) => !route.auth?.admin).map((route) => route.id); + + expect(new Set(reachableByNonAdmins)).toEqual(UNAUTHENTICATED_ADMIN_ROUTES); + }); + + it('should not authenticate the bootstrap routes under admin/', () => { + const authenticated = routes + .filter((route) => UNAUTHENTICATED_ADMIN_ROUTES.has(route.id) && route.auth !== undefined) + .map((route) => route.label); + + expect(authenticated).toEqual([]); + }); + + it('should require admin access for routes with an admin permission', () => { + const offenders = routes + .filter((route) => isAdminPermission(route.auth?.permission) && !route.auth?.admin) + .map((route) => route.label); + + expect(offenders).toEqual([]); + }); +}); diff --git a/server/src/middleware/auth.guard.ts b/server/src/middleware/auth.guard.ts index 4964fefbbc..d9870ec7b9 100644 --- a/server/src/middleware/auth.guard.ts +++ b/server/src/middleware/auth.guard.ts @@ -17,7 +17,15 @@ import { getUserAgentDetails } from 'src/utils/request'; type AdminRoute = { admin?: true }; type SharedLinkRoute = { sharedLink?: true }; -type AuthenticatedOptions = { permission?: Permission | false } & (AdminRoute | SharedLinkRoute); +export type AuthenticatedOptions = { permission?: Permission | false } & (AdminRoute | SharedLinkRoute); + +type ReflectorTarget = Parameters[1]; + +/** Resolves the `@Authenticated()` options of a route handler, with the defaults applied. */ +export const getAuthenticatedOptions = (reflector: Reflector, target: ReflectorTarget) => { + const options = reflector.getAllAndOverride(MetadataKey.AuthRoute, [target]); + return options && { sharedLink: false, admin: false, ...options }; +}; export const Authenticated = (options: AuthenticatedOptions = {}): MethodDecorator => { const decorators: MethodDecorator[] = [ @@ -86,17 +94,12 @@ export class AuthGuard implements CanActivate { } async canActivate(context: ExecutionContext): Promise { - const targets = [context.getHandler()]; - const options = this.reflector.getAllAndOverride(MetadataKey.AuthRoute, targets); + const options = getAuthenticatedOptions(this.reflector, context.getHandler()); if (!options) { return true; } - const { - admin: adminRoute, - sharedLink: sharedLinkRoute, - permission, - } = { sharedLink: false, admin: false, ...options }; + const { admin: adminRoute, sharedLink: sharedLinkRoute, permission } = options; const request = context.switchToHttp().getRequest(); request.user = await this.authService.authenticate({