refactor(core): Setup decorator based RBAC (no-changelog) (#5787)

packages/cli/src/UserManagement/UserManagementHelper.ts

@@ -196,30 +196,6 @@ export async function getUserById(userId: string): Promise<User> {
 	return user;
 }
 
-/**
- * Check if a URL contains an auth-excluded endpoint.
- */
-export function isAuthExcluded(url: string, ignoredEndpoints: Readonly<string[]>): boolean {
-	return !!ignoredEndpoints
-		.filter(Boolean) // skip empty paths
-		.find((ignoredEndpoint) => url.startsWith(`/${ignoredEndpoint}`));
-}
-
-/**
- * Check if the endpoint is `POST /users/:id`.
- */
-export function isPostUsersId(req: express.Request, restEndpoint: string): boolean {
-	return (
-		req.method === 'POST' &&
-		new RegExp(`/${restEndpoint}/users/[\\w\\d-]*`).test(req.url) &&
-		!req.url.includes('reinvite')
-	);
-}
-
-export function isAuthenticatedRequest(request: express.Request): request is AuthenticatedRequest {
-	return request.user !== undefined;
-}
-
 // ----------------------------------
 //            hashing
 // ----------------------------------

packages/cli/src/controllers/auth.controller.ts

@@ -1,5 +1,5 @@
 import validator from 'validator';
-import { Get, Post, RestController } from '@/decorators';
+import { Authorized, Get, Post, RestController } from '@/decorators';
 import { AuthError, BadRequestError, InternalServerError } from '@/ResponseHelper';
 import { sanitizeUser, withFeatureFlags } from '@/UserManagement/UserManagementHelper';
 import { issueCookie, resolveJwt } from '@/auth/jwt';
@@ -58,7 +58,6 @@ export class AuthController {
 
 	/**
 	 * Log in a user.
-	 * Authless endpoint.
 	 */
 	@Post('/login')
 	async login(req: LoginRequest, res: Response): Promise<PublicUser | undefined> {
@@ -135,7 +134,6 @@ export class AuthController {
 
 	/**
 	 * Validate invite token to enable invitee to set up their account.
-	 * Authless endpoint.
 	 */
 	@Get('/resolve-signup-token')
 	async resolveSignupToken(req: UserRequest.ResolveSignUp) {
@@ -196,8 +194,8 @@ export class AuthController {
 
 	/**
 	 * Log out a user.
-	 * Authless endpoint.
 	 */
+	@Authorized()
 	@Post('/logout')
 	logout(req: Request, res: Response) {
 		res.clearCookie(AUTH_COOKIE_NAME);

packages/cli/src/controllers/ldap.controller.ts

@@ -1,5 +1,5 @@
 import pick from 'lodash.pick';
-import { Get, Post, Put, RestController } from '@/decorators';
+import { Authorized, Get, Post, Put, RestController } from '@/decorators';
 import { getLdapConfig, getLdapSynchronizations, updateLdapConfig } from '@/Ldap/helpers';
 import { LdapService } from '@/Ldap/LdapService.ee';
 import { LdapSync } from '@/Ldap/LdapSync.ee';
@@ -8,6 +8,7 @@ import { BadRequestError } from '@/ResponseHelper';
 import { NON_SENSIBLE_LDAP_CONFIG_PROPERTIES } from '@/Ldap/constants';
 import { InternalHooks } from '@/InternalHooks';
 
+@Authorized(['global', 'owner'])
 @RestController('/ldap')
 export class LdapController {
 	constructor(

packages/cli/src/controllers/me.controller.ts

@@ -1,6 +1,6 @@
 import validator from 'validator';
 import { plainToInstance } from 'class-transformer';
-import { Delete, Get, Patch, Post, RestController } from '@/decorators';
+import { Authorized, Delete, Get, Patch, Post, RestController } from '@/decorators';
 import {
 	compareHash,
 	hashPassword,
@@ -30,6 +30,7 @@ import { randomBytes } from 'crypto';
 import { isSamlLicensedAndEnabled } from '../sso/saml/samlHelpers';
 import { UserService } from '@/user/user.service';
 
+@Authorized()
 @RestController('/me')
 export class MeController {
 	private readonly logger: ILogger;

packages/cli/src/controllers/nodeTypes.controller.ts

@@ -2,11 +2,12 @@ import { readFile } from 'fs/promises';
 import get from 'lodash.get';
 import { Request } from 'express';
 import type { INodeTypeDescription, INodeTypeNameVersion } from 'n8n-workflow';
-import { Post, RestController } from '@/decorators';
+import { Authorized, Post, RestController } from '@/decorators';
 import { getNodeTranslationPath } from '@/TranslationHelpers';
 import type { Config } from '@/config';
 import type { NodeTypes } from '@/NodeTypes';
 
+@Authorized()
 @RestController('/node-types')
 export class NodeTypesController {
 	private readonly config: Config;

packages/cli/src/controllers/nodes.controller.ts

@@ -4,7 +4,7 @@ import {
 	STARTER_TEMPLATE_NAME,
 	UNKNOWN_FAILURE_REASON,
 } from '@/constants';
-import { Delete, Get, Middleware, Patch, Post, RestController } from '@/decorators';
+import { Authorized, Delete, Get, Middleware, Patch, Post, RestController } from '@/decorators';
 import { NodeRequest } from '@/requests';
 import { BadRequestError, InternalServerError } from '@/ResponseHelper';
 import {
@@ -30,10 +30,10 @@ import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
 import { InternalHooks } from '@/InternalHooks';
 import { Push } from '@/push';
 import { Config } from '@/config';
-import { isAuthenticatedRequest } from '@/UserManagement/UserManagementHelper';
 
 const { PACKAGE_NOT_INSTALLED, PACKAGE_NAME_NOT_PROVIDED } = RESPONSE_ERROR_MESSAGES;
 
+@Authorized(['global', 'owner'])
 @RestController('/nodes')
 export class NodesController {
 	constructor(
@@ -43,14 +43,6 @@ export class NodesController {
 		private internalHooks: InternalHooks,
 	) {}
 
-	// TODO: move this into a new decorator `@Authorized`
-	@Middleware()
-	checkIfOwner(req: Request, res: Response, next: NextFunction) {
-		if (!isAuthenticatedRequest(req) || req.user.globalRole.name !== 'owner')
-			res.status(403).json({ status: 'error', message: 'Unauthorized' });
-		else next();
-	}
-
 	// TODO: move this into a new decorator `@IfConfig('executions.mode', 'queue')`
 	@Middleware()
 	checkIfCommunityNodesEnabled(req: Request, res: Response, next: NextFunction) {

packages/cli/src/controllers/owner.controller.ts

@@ -1,6 +1,6 @@
 import validator from 'validator';
 import { validateEntity } from '@/GenericHelpers';
-import { Get, Post, RestController } from '@/decorators';
+import { Authorized, Get, Post, RestController } from '@/decorators';
 import { BadRequestError } from '@/ResponseHelper';
 import {
 	hashPassword,
@@ -20,6 +20,7 @@ import type {
 	WorkflowRepository,
 } from '@db/repositories';
 
+@Authorized(['global', 'owner'])
 @RestController('/owner')
 export class OwnerController {
 	private readonly config: Config;

packages/cli/src/controllers/passwordReset.controller.ts

@@ -65,7 +65,6 @@ export class PasswordResetController {
 
 	/**
 	 * Send a password reset email.
-	 * Authless endpoint.
 	 */
 	@Post('/forgot-password')
 	async forgotPassword(req: PasswordResetRequest.Email) {
@@ -171,7 +170,6 @@ export class PasswordResetController {
 
 	/**
 	 * Verify password reset token and user ID.
-	 * Authless endpoint.
 	 */
 	@Get('/resolve-password-token')
 	async resolvePasswordToken(req: PasswordResetRequest.Credentials) {
@@ -213,7 +211,6 @@ export class PasswordResetController {
 
 	/**
 	 * Verify password reset token and user ID and update password.
-	 * Authless endpoint.
 	 */
 	@Post('/change-password')
 	async changePassword(req: PasswordResetRequest.NewPassword, res: Response) {

packages/cli/src/controllers/tags.controller.ts

@@ -1,13 +1,14 @@
 import { Request, Response, NextFunction } from 'express';
 import type { Config } from '@/config';
-import { Delete, Get, Middleware, Patch, Post, RestController } from '@/decorators';
+import { Authorized, Delete, Get, Middleware, Patch, Post, RestController } from '@/decorators';
 import type { IDatabaseCollections, IExternalHooksClass, ITagWithCountDb } from '@/Interfaces';
 import { TagEntity } from '@db/entities/TagEntity';
 import type { TagRepository } from '@db/repositories';
 import { validateEntity } from '@/GenericHelpers';
-import { BadRequestError, UnauthorizedError } from '@/ResponseHelper';
+import { BadRequestError } from '@/ResponseHelper';
 import { TagsRequest } from '@/requests';
 
+@Authorized()
 @RestController('/tags')
 export class TagsController {
 	private config: Config;
@@ -91,15 +92,9 @@ export class TagsController {
 		return tag;
 	}
 
+	@Authorized(['global', 'owner'])
 	@Delete('/:id(\\d+)')
 	async deleteTag(req: TagsRequest.Delete) {
-		const isInstanceOwnerSetUp = this.config.getEnv('userManagement.isInstanceOwnerSetUp');
-		if (isInstanceOwnerSetUp && req.user.globalRole.name !== 'owner') {
-			throw new UnauthorizedError(
-				'You are not allowed to perform this action',
-				'Only owners can remove tags',
-			);
-		}
 		const { id } = req.params;
 		await this.externalHooks.run('tag.beforeDelete', [id]);
 

packages/cli/src/controllers/translation.controller.ts

@@ -2,7 +2,7 @@ import type { Request } from 'express';
 import { ICredentialTypes } from 'n8n-workflow';
 import { join } from 'path';
 import { access } from 'fs/promises';
-import { Get, RestController } from '@/decorators';
+import { Authorized, Get, RestController } from '@/decorators';
 import { BadRequestError, InternalServerError } from '@/ResponseHelper';
 import { Config } from '@/config';
 import { NODES_BASE_DIR } from '@/constants';
@@ -14,6 +14,7 @@ export declare namespace TranslationRequest {
 	export type Credential = Request<{}, {}, {}, { credentialType: string }>;
 }
 
+@Authorized()
 @RestController('/')
 export class TranslationController {
 	constructor(private config: Config, private credentialTypes: ICredentialTypes) {}

packages/cli/src/controllers/users.controller.ts

@@ -5,7 +5,7 @@ import { ErrorReporterProxy as ErrorReporter } from 'n8n-workflow';
 import { User } from '@db/entities/User';
 import { SharedCredentials } from '@db/entities/SharedCredentials';
 import { SharedWorkflow } from '@db/entities/SharedWorkflow';
-import { Delete, Get, Post, RestController } from '@/decorators';
+import { Authorized, NoAuthRequired, Delete, Get, Post, RestController } from '@/decorators';
 import {
 	addInviteLinkToUser,
 	generateUserInviteUrl,
@@ -41,6 +41,7 @@ import type {
 	UserRepository,
 } from '@db/repositories';
 
+@Authorized(['global', 'owner'])
 @RestController('/users')
 export class UsersController {
 	private config: Config;
@@ -282,6 +283,7 @@ export class UsersController {
 	/**
 	 * Fill out user shell with first name, last name, and password.
 	 */
+	@NoAuthRequired()
 	@Post('/:id')
 	async updateUser(req: UserRequest.Update, res: Response) {
 		const { id: inviteeId } = req.params;
@@ -343,6 +345,7 @@ export class UsersController {
 		return withFeatureFlags(this.postHog, sanitizeUser(updatedUser));
 	}
 
+	@Authorized('any')
 	@Get('/')
 	async listUsers(req: UserRequest.List) {
 		const users = await this.userRepository.find({ relations: ['globalRole', 'authIdentities'] });

packages/cli/src/decorators/Authorized.ts

@@ -0,0 +1,16 @@
+/* eslint-disable @typescript-eslint/ban-types */
+/* eslint-disable @typescript-eslint/naming-convention */
+import { CONTROLLER_AUTH_ROLES } from './constants';
+import type { AuthRoleMetadata } from './types';
+
+export function Authorized(authRole: AuthRoleMetadata[string] = 'any'): Function {
+	return function (target: Function | Object, handlerName?: string) {
+		const controllerClass = handlerName ? target.constructor : target;
+		const authRoles = (Reflect.getMetadata(CONTROLLER_AUTH_ROLES, controllerClass) ??
+			{}) as AuthRoleMetadata;
+		authRoles[handlerName ?? '*'] = authRole;
+		Reflect.defineMetadata(CONTROLLER_AUTH_ROLES, authRoles, controllerClass);
+	};
+}
+
+export const NoAuthRequired = () => Authorized('none');

packages/cli/src/decorators/constants.ts

@@ -1,3 +1,4 @@
 export const CONTROLLER_ROUTES = 'CONTROLLER_ROUTES';
 export const CONTROLLER_BASE_PATH = 'CONTROLLER_BASE_PATH';
 export const CONTROLLER_MIDDLEWARES = 'CONTROLLER_MIDDLEWARES';
+export const CONTROLLER_AUTH_ROLES = 'CONTROLLER_AUTH_ROLES';

packages/cli/src/decorators/index.ts

@@ -1,3 +1,4 @@
+export { Authorized, NoAuthRequired } from './Authorized';
 export { RestController } from './RestController';
 export { Get, Post, Put, Patch, Delete } from './Route';
 export { Middleware } from './Middleware';

packages/cli/src/decorators/registerController.ts

@@ -1,10 +1,36 @@
 /* eslint-disable @typescript-eslint/naming-convention */
 import { Router } from 'express';
-import type { Config } from '@/config';
-import { CONTROLLER_BASE_PATH, CONTROLLER_MIDDLEWARES, CONTROLLER_ROUTES } from './constants';
-import { send } from '@/ResponseHelper'; // TODO: move `ResponseHelper.send` to this file
 import type { Application, Request, Response, RequestHandler } from 'express';
-import type { Controller, MiddlewareMetadata, RouteMetadata } from './types';
+import type { Config } from '@/config';
+import type { AuthenticatedRequest } from '@/requests';
+import { send } from '@/ResponseHelper'; // TODO: move `ResponseHelper.send` to this file
+import {
+	CONTROLLER_AUTH_ROLES,
+	CONTROLLER_BASE_PATH,
+	CONTROLLER_MIDDLEWARES,
+	CONTROLLER_ROUTES,
+} from './constants';
+import type {
+	AuthRole,
+	AuthRoleMetadata,
+	Controller,
+	MiddlewareMetadata,
+	RouteMetadata,
+} from './types';
+
+export const createAuthMiddleware =
+	(authRole: AuthRole): RequestHandler =>
+	({ user }: AuthenticatedRequest, res, next) => {
+		if (authRole === 'none') return next();
+
+		if (!user) return res.status(401).json({ status: 'error', message: 'Unauthorized' });
+
+		const { globalRole } = user;
+		if (authRole === 'any' || (globalRole.scope === authRole[0] && globalRole.name === authRole[1]))
+			return next();
+
+		res.status(403).json({ status: 'error', message: 'Unauthorized' });
+	};
 
 export const registerController = (app: Application, config: Config, controller: object) => {
 	const controllerClass = controller.constructor;
@@ -14,11 +40,16 @@ export const registerController = (app: Application, config: Config, controller:
 	if (!controllerBasePath)
 		throw new Error(`${controllerClass.name} is missing the RestController decorator`);
 
+	const authRoles = Reflect.getMetadata(CONTROLLER_AUTH_ROLES, controllerClass) as
+		| AuthRoleMetadata
+		| undefined;
 	const routes = Reflect.getMetadata(CONTROLLER_ROUTES, controllerClass) as RouteMetadata[];
 	if (routes.length > 0) {
 		const router = Router({ mergeParams: true });
 		const restBasePath = config.getEnv('endpoints.rest');
-		const prefix = `/${[restBasePath, controllerBasePath].join('/')}`.replace(/\/+/g, '/');
+		const prefix = `/${[restBasePath, controllerBasePath].join('/')}`
+			.replace(/\/+/g, '/')
+			.replace(/\/$/, '');
 
 		const controllerMiddlewares = (
 			(Reflect.getMetadata(CONTROLLER_MIDDLEWARES, controllerClass) ?? []) as MiddlewareMetadata[]
@@ -28,8 +59,10 @@ export const registerController = (app: Application, config: Config, controller:
 		);
 
 		routes.forEach(({ method, path, middlewares: routeMiddlewares, handlerName }) => {
+			const authRole = authRoles && (authRoles[handlerName] ?? authRoles['*']);
 			router[method](
 				path,
+				...(authRole ? [createAuthMiddleware(authRole)] : []),
 				...controllerMiddlewares,
 				...routeMiddlewares,
 				send(async (req: Request, res: Response) =>

packages/cli/src/decorators/types.ts

@@ -1,7 +1,11 @@
 import type { Request, Response, RequestHandler } from 'express';
+import type { RoleNames, RoleScopes } from '@db/entities/Role';
 
 export type Method = 'get' | 'post' | 'put' | 'patch' | 'delete';
 
+export type AuthRole = [RoleScopes, RoleNames] | 'any' | 'none';
+export type AuthRoleMetadata = Record<string, AuthRole>;
+
 export interface MiddlewareMetadata {
 	handlerName: string;
 }

packages/cli/src/eventbus/eventBus.controller.ts

@@ -28,15 +28,13 @@ import type {
 	IRunExecutionData,
 } from 'n8n-workflow';
 import { MessageEventBusDestinationTypeNames, EventMessageTypeNames } from 'n8n-workflow';
-import type { User } from '@db/entities/User';
-import * as ResponseHelper from '@/ResponseHelper';
 import type { EventMessageNodeOptions } from './EventMessageClasses/EventMessageNode';
 import { EventMessageNode } from './EventMessageClasses/EventMessageNode';
 import { recoverExecutionDataFromEventLogMessages } from './MessageEventBus/recoverEvents';
-import { RestController, Get, Post, Delete } from '@/decorators';
+import { RestController, Get, Post, Delete, Authorized } from '@/decorators';
 import type { MessageEventBusDestination } from './MessageEventBusDestination/MessageEventBusDestination.ee';
-import { isOwnerMiddleware } from '../middlewares/isOwner';
 import type { DeleteResult } from 'typeorm';
+import { AuthenticatedRequest } from '@/requests';
 
 // ----------------------------------------
 // TypeGuards
@@ -74,12 +72,14 @@ const isMessageEventBusDestinationOptions = (
 // Controller
 // ----------------------------------------
 
+@Authorized()
 @RestController('/eventbus')
 export class EventBusController {
 	// ----------------------------------------
 	// Events
 	// ----------------------------------------
-	@Get('/event', { middlewares: [isOwnerMiddleware] })
+	@Authorized(['global', 'owner'])
+	@Get('/event')
 	async getEvents(
 		req: express.Request,
 	): Promise<EventMessageTypes[] | Record<string, EventMessageTypes[]>> {
@@ -132,7 +132,8 @@ export class EventBusController {
 		return;
 	}
 
-	@Post('/event', { middlewares: [isOwnerMiddleware] })
+	@Authorized(['global', 'owner'])
+	@Post('/event')
 	async postEvent(req: express.Request): Promise<EventMessageTypes | undefined> {
 		let msg: EventMessageTypes | undefined;
 		if (isEventMessageOptions(req.body)) {
@@ -172,12 +173,9 @@ export class EventBusController {
 		}
 	}
 
-	@Post('/destination', { middlewares: [isOwnerMiddleware] })
-	async postDestination(req: express.Request): Promise<any> {
-		if (!req.user || (req.user as User).globalRole.name !== 'owner') {
-			throw new ResponseHelper.UnauthorizedError('Invalid request');
-		}
-
+	@Authorized(['global', 'owner'])
+	@Post('/destination')
+	async postDestination(req: AuthenticatedRequest): Promise<any> {
 		let result: MessageEventBusDestination | undefined;
 		if (isMessageEventBusDestinationOptions(req.body)) {
 			switch (req.body.__type) {
@@ -228,11 +226,9 @@ export class EventBusController {
 		return false;
 	}
 
-	@Delete('/destination', { middlewares: [isOwnerMiddleware] })
-	async deleteDestination(req: express.Request): Promise<DeleteResult | undefined> {
-		if (!req.user || (req.user as User).globalRole.name !== 'owner') {
-			throw new ResponseHelper.UnauthorizedError('Invalid request');
-		}
+	@Authorized(['global', 'owner'])
+	@Delete('/destination')
+	async deleteDestination(req: AuthenticatedRequest): Promise<DeleteResult | undefined> {
 		if (isWithIdString(req.query)) {
 			return eventBus.removeDestination(req.query.id);
 		} else {

packages/cli/src/middlewares/auth.ts

@@ -10,13 +10,7 @@ import type { AuthenticatedRequest } from '@/requests';
 import config from '@/config';
 import { AUTH_COOKIE_NAME, EDITOR_UI_DIST_DIR } from '@/constants';
 import { issueCookie, resolveJwtContent } from '@/auth/jwt';
-import {
-	isAuthenticatedRequest,
-	isAuthExcluded,
-	isPostUsersId,
-	isUserManagementEnabled,
-} from '@/UserManagement/UserManagementHelper';
-import { SamlUrls } from '@/sso/saml/constants';
+import { isUserManagementEnabled } from '@/UserManagement/UserManagementHelper';
 import type { UserRepository } from '@db/repositories';
 
 const jwtFromRequest = (req: Request) => {
@@ -66,6 +60,17 @@ const staticAssets = globSync(['**/*.html', '**/*.svg', '**/*.png', '**/*.ico'],
 	cwd: EDITOR_UI_DIST_DIR,
 });
 
+// TODO: delete this
+const isPostUsersId = (req: Request, restEndpoint: string): boolean =>
+	req.method === 'POST' &&
+	new RegExp(`/${restEndpoint}/users/[\\w\\d-]*`).test(req.url) &&
+	!req.url.includes('reinvite');
+
+const isAuthExcluded = (url: string, ignoredEndpoints: Readonly<string[]>): boolean =>
+	!!ignoredEndpoints
+		.filter(Boolean) // skip empty paths
+		.find((ignoredEndpoint) => url.startsWith(`/${ignoredEndpoint}`));
+
 /**
  * This sets up the auth middlewares in the correct order
  */
@@ -85,20 +90,16 @@ export const setupAuthMiddlewares = (
 			// skip authentication for preflight requests
 			req.method === 'OPTIONS' ||
 			staticAssets.includes(req.url.slice(1)) ||
+			isAuthExcluded(req.url, ignoredEndpoints) ||
 			req.url.startsWith(`/${restEndpoint}/settings`) ||
 			req.url.startsWith(`/${restEndpoint}/login`) ||
-			req.url.startsWith(`/${restEndpoint}/logout`) ||
 			req.url.startsWith(`/${restEndpoint}/resolve-signup-token`) ||
 			isPostUsersId(req, restEndpoint) ||
 			req.url.startsWith(`/${restEndpoint}/forgot-password`) ||
 			req.url.startsWith(`/${restEndpoint}/resolve-password-token`) ||
 			req.url.startsWith(`/${restEndpoint}/change-password`) ||
 			req.url.startsWith(`/${restEndpoint}/oauth2-credential/callback`) ||
-			req.url.startsWith(`/${restEndpoint}/oauth1-credential/callback`) ||
-			req.url.startsWith(`/${restEndpoint}/sso/saml${SamlUrls.metadata}`) ||
-			req.url.startsWith(`/${restEndpoint}/sso/saml${SamlUrls.initSSO}`) ||
-			req.url.startsWith(`/${restEndpoint}/sso/saml${SamlUrls.acs}`) ||
-			isAuthExcluded(req.url, ignoredEndpoints)
+			req.url.startsWith(`/${restEndpoint}/oauth1-credential/callback`)
 		) {
 			return next();
 		}
@@ -115,43 +116,5 @@ export const setupAuthMiddlewares = (
 		return passportMiddleware(req, res, next);
 	});
 
-	app.use((req: Request | AuthenticatedRequest, res: Response, next: NextFunction) => {
-		// req.user is empty for public routes, so just proceed
-		// owner can do anything, so proceed as well
-		if (!req.user || (isAuthenticatedRequest(req) && req.user.globalRole.name === 'owner')) {
-			next();
-			return;
-		}
-		// Not owner and user exists. We now protect restricted urls.
-		const postRestrictedUrls = [
-			`/${restEndpoint}/users`,
-			`/${restEndpoint}/owner`,
-			`/${restEndpoint}/ldap/sync`,
-			`/${restEndpoint}/ldap/test-connection`,
-		];
-		const getRestrictedUrls = [`/${restEndpoint}/ldap/sync`, `/${restEndpoint}/ldap/config`];
-		const putRestrictedUrls = [`/${restEndpoint}/ldap/config`];
-		const trimmedUrl = req.url.endsWith('/') ? req.url.slice(0, -1) : req.url;
-		if (
-			(req.method === 'POST' && postRestrictedUrls.includes(trimmedUrl)) ||
-			(req.method === 'GET' && getRestrictedUrls.includes(trimmedUrl)) ||
-			(req.method === 'PUT' && putRestrictedUrls.includes(trimmedUrl)) ||
-			(req.method === 'DELETE' &&
-				new RegExp(`/${restEndpoint}/users/[^/]+`, 'gm').test(trimmedUrl)) ||
-			(req.method === 'POST' &&
-				new RegExp(`/${restEndpoint}/users/[^/]+/reinvite`, 'gm').test(trimmedUrl)) ||
-			new RegExp(`/${restEndpoint}/owner/[^/]+`, 'gm').test(trimmedUrl)
-		) {
-			Logger.verbose('User attempted to access endpoint without authorization', {
-				endpoint: `${req.method} ${trimmedUrl}`,
-				userId: isAuthenticatedRequest(req) ? req.user.id : 'unknown',
-			});
-			res.status(403).json({ status: 'error', message: 'Unauthorized' });
-			return;
-		}
-
-		next();
-	});
-
 	app.use(refreshExpiringCookie);
 };

packages/cli/src/middlewares/isOwner.ts

@@ -1,12 +0,0 @@
-import type { RequestHandler } from 'express';
-import { LoggerProxy } from 'n8n-workflow';
-import type { AuthenticatedRequest } from '@/requests';
-
-export const isOwnerMiddleware: RequestHandler = (req: AuthenticatedRequest, res, next) => {
-	if (req.user.globalRole.name === 'owner') {
-		next();
-	} else {
-		LoggerProxy.debug('Request failed because user is not owner');
-		res.status(401).send('Unauthorized');
-	}
-};

packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts

@@ -1,24 +1,11 @@
 import type { RequestHandler } from 'express';
-import type { AuthenticatedRequest } from '@/requests';
 import { isSamlLicensed, isSamlLicensedAndEnabled } from '../samlHelpers';
 
-export const samlLicensedOwnerMiddleware: RequestHandler = (
-	req: AuthenticatedRequest,
-	res,
-	next,
-) => {
-	if (isSamlLicensed() && req.user?.globalRole.name === 'owner') {
-		next();
-	} else {
-		res.status(401).json({ status: 'error', message: 'Unauthorized' });
-	}
-};
-
 export const samlLicensedAndEnabledMiddleware: RequestHandler = (req, res, next) => {
 	if (isSamlLicensedAndEnabled()) {
 		next();
 	} else {
-		res.status(401).json({ status: 'error', message: 'Unauthorized' });
+		res.status(403).json({ status: 'error', message: 'Unauthorized' });
 	}
 };
 
@@ -26,6 +13,6 @@ export const samlLicensedMiddleware: RequestHandler = (req, res, next) => {
 	if (isSamlLicensed()) {
 		next();
 	} else {
-		res.status(401).json({ status: 'error', message: 'Unauthorized' });
+		res.status(403).json({ status: 'error', message: 'Unauthorized' });
 	}
 };

packages/cli/src/sso/saml/routes/saml.controller.ee.ts

@@ -1,10 +1,9 @@
 import express from 'express';
-import { Get, Post, RestController } from '@/decorators';
+import { Authorized, Get, Post, RestController } from '@/decorators';
 import { SamlUrls } from '../constants';
 import {
 	samlLicensedAndEnabledMiddleware,
 	samlLicensedMiddleware,
-	samlLicensedOwnerMiddleware,
 } from '../middleware/samlEnabledMiddleware';
 import { SamlService } from '../saml.service.ee';
 import { SamlConfiguration } from '../types/requests';
@@ -39,7 +38,8 @@ export class SamlController {
 	 * GET /sso/saml/config
 	 * Return SAML config
 	 */
-	@Get(SamlUrls.config, { middlewares: [samlLicensedOwnerMiddleware] })
+	@Authorized(['global', 'owner'])
+	@Get(SamlUrls.config, { middlewares: [samlLicensedMiddleware] })
 	async configGet() {
 		const prefs = this.samlService.samlPreferences;
 		return {
@@ -53,7 +53,8 @@ export class SamlController {
 	 * POST /sso/saml/config
 	 * Set SAML config
 	 */
-	@Post(SamlUrls.config, { middlewares: [samlLicensedOwnerMiddleware] })
+	@Authorized(['global', 'owner'])
+	@Post(SamlUrls.config, { middlewares: [samlLicensedMiddleware] })
 	async configPost(req: SamlConfiguration.Update) {
 		const validationResult = await validate(req.body);
 		if (validationResult.length === 0) {
@@ -71,7 +72,8 @@ export class SamlController {
 	 * POST /sso/saml/config/toggle
 	 * Set SAML config
 	 */
-	@Post(SamlUrls.configToggleEnabled, { middlewares: [samlLicensedOwnerMiddleware] })
+	@Authorized(['global', 'owner'])
+	@Post(SamlUrls.configToggleEnabled, { middlewares: [samlLicensedMiddleware] })
 	async toggleEnabledPost(req: SamlConfiguration.Toggle, res: express.Response) {
 		if (req.body.loginEnabled === undefined) {
 			throw new BadRequestError('Body should contain a boolean "loginEnabled" property');
@@ -155,7 +157,8 @@ export class SamlController {
 	 * Test SAML config
 	 * This endpoint is available if SAML is licensed and the requestor is an instance owner
 	 */
-	@Get(SamlUrls.configTest, { middlewares: [samlLicensedOwnerMiddleware] })
+	@Authorized(['global', 'owner'])
+	@Get(SamlUrls.configTest, { middlewares: [samlLicensedMiddleware] })
 	async configTestGet(req: AuthenticatedRequest, res: express.Response) {
 		return this.handleInitSSO(res, getServiceProviderConfigTestReturnUrl());
 	}

packages/cli/test/integration/shared/constants.ts

@@ -42,7 +42,7 @@ export const ROUTES_REQUIRING_AUTHORIZATION: Readonly<string[]> = [
 	'POST /users',
 	'DELETE /users/123',
 	'POST /users/123/reinvite',
-	'POST /owner/pre-setup',
+	'GET /owner/pre-setup',
 	'POST /owner/setup',
 	'POST /owner/skip-setup',
 ];

packages/cli/test/integration/users.api.test.ts

@@ -31,6 +31,7 @@ let credentialOwnerRole: Role;
 let owner: User;
 let authlessAgent: SuperAgentTest;
 let authOwnerAgent: SuperAgentTest;
+let authAgentFor: (user: User) => SuperAgentTest;
 
 beforeAll(async () => {
 	const app = await utils.initTestServer({ endpointGroups: ['users'] });
@@ -49,7 +50,8 @@ beforeAll(async () => {
 	owner = await testDb.createUser({ globalRole: globalOwnerRole });
 
 	authlessAgent = utils.createAgent(app);
-	authOwnerAgent = utils.createAuthAgent(app)(owner);
+	authAgentFor = utils.createAuthAgent(app);
+	authOwnerAgent = authAgentFor(owner);
 });
 
 beforeEach(async () => {
@@ -69,7 +71,7 @@ afterAll(async () => {
 });
 
 describe('GET /users', () => {
-	test('should return all users', async () => {
+	test('should return all users (for owner)', async () => {
 		await testDb.createUser({ globalRole: globalMemberRole });
 
 		const response = await authOwnerAgent.get('/users');
@@ -103,6 +105,14 @@ describe('GET /users', () => {
 			expect(apiKey).not.toBeDefined();
 		});
 	});
+
+	test('should return all users (for member)', async () => {
+		const member = await testDb.createUser({ globalRole: globalMemberRole });
+		const response = await authAgentFor(member).get('/users');
+
+		expect(response.statusCode).toBe(200);
+		expect(response.body.data.length).toBe(2);
+	});
 });
 
 describe('DELETE /users/:id', () => {