refactor(core): Auto-register controllers at startup (no-changelog) (#9781)

packages/cli/test/integration/shared/utils/testServer.ts

@@ -8,21 +8,21 @@ import { URL } from 'url';
 import config from '@/config';
 import { AUTH_COOKIE_NAME } from '@/constants';
 import type { User } from '@db/entities/User';
-import { registerController } from '@/decorators';
+import { ControllerRegistry } from '@/decorators';
 import { rawBodyReader, bodyParser } from '@/middlewares';
 import { PostHogClient } from '@/posthog';
 import { Push } from '@/push';
 import { License } from '@/License';
 import { Logger } from '@/Logger';
 import { InternalHooks } from '@/InternalHooks';
+import { AuthService } from '@/auth/auth.service';
+import type { APIRequest } from '@/requests';
 
 import { mockInstance } from '../../../shared/mocking';
 import * as testDb from '../../shared/testDb';
 import { PUBLIC_API_REST_PATH_SEGMENT, REST_PATH_SEGMENT } from '../constants';
 import type { SetupProps, TestServer } from '../types';
 import { LicenseMocker } from '../license';
-import { AuthService } from '@/auth/auth.service';
-import type { APIRequest } from '@/requests';
 
 /**
  * Plugin to prefix a path segment into a request URL pathname.
@@ -125,30 +125,23 @@ export const setupTestServer = ({
 			for (const group of endpointGroups) {
 				switch (group) {
 					case 'credentials':
-						const { CredentialsController } = await import('@/credentials/credentials.controller');
-						registerController(app, CredentialsController);
+						await import('@/credentials/credentials.controller');
 						break;
 
 					case 'workflows':
-						const { WorkflowsController } = await import('@/workflows/workflows.controller');
-						registerController(app, WorkflowsController);
+						await import('@/workflows/workflows.controller');
 						break;
 
 					case 'executions':
-						const { ExecutionsController } = await import('@/executions/executions.controller');
-						registerController(app, ExecutionsController);
+						await import('@/executions/executions.controller');
 						break;
 
 					case 'variables':
-						const { VariablesController } = await import(
-							'@/environments/variables/variables.controller.ee'
-						);
-						registerController(app, VariablesController);
+						await import('@/environments/variables/variables.controller.ee');
 						break;
 
 					case 'license':
-						const { LicenseController } = await import('@/license/license.controller');
-						registerController(app, LicenseController);
+						await import('@/license/license.controller');
 						break;
 
 					case 'metrics':
@@ -157,123 +150,93 @@ export const setupTestServer = ({
 						break;
 
 					case 'eventBus':
-						const { EventBusController } = await import('@/eventbus/eventBus.controller');
-						registerController(app, EventBusController);
+						await import('@/eventbus/eventBus.controller');
 						break;
 
 					case 'auth':
-						const { AuthController } = await import('@/controllers/auth.controller');
-						registerController(app, AuthController);
+						await import('@/controllers/auth.controller');
 						break;
 
 					case 'mfa':
-						const { MFAController } = await import('@/controllers/mfa.controller');
-						registerController(app, MFAController);
+						await import('@/controllers/mfa.controller');
 						break;
 
 					case 'ldap':
 						const { LdapService } = await import('@/Ldap/ldap.service');
-						const { LdapController } = await import('@/Ldap/ldap.controller');
+						await import('@/Ldap/ldap.controller');
 						testServer.license.enable('feat:ldap');
 						await Container.get(LdapService).init();
-						registerController(app, LdapController);
 						break;
 
 					case 'saml':
 						const { setSamlLoginEnabled } = await import('@/sso/saml/samlHelpers');
-						const { SamlController } = await import('@/sso/saml/routes/saml.controller.ee');
+						await import('@/sso/saml/routes/saml.controller.ee');
 						await setSamlLoginEnabled(true);
-						registerController(app, SamlController);
 						break;
 
 					case 'sourceControl':
-						const { SourceControlController } = await import(
-							'@/environments/sourceControl/sourceControl.controller.ee'
-						);
-						registerController(app, SourceControlController);
+						await import('@/environments/sourceControl/sourceControl.controller.ee');
 						break;
 
 					case 'community-packages':
-						const { CommunityPackagesController } = await import(
-							'@/controllers/communityPackages.controller'
-						);
-						registerController(app, CommunityPackagesController);
+						await import('@/controllers/communityPackages.controller');
 						break;
 
 					case 'me':
-						const { MeController } = await import('@/controllers/me.controller');
-						registerController(app, MeController);
+						await import('@/controllers/me.controller');
 						break;
 
 					case 'passwordReset':
-						const { PasswordResetController } = await import(
-							'@/controllers/passwordReset.controller'
-						);
-						registerController(app, PasswordResetController);
+						await import('@/controllers/passwordReset.controller');
 						break;
 
 					case 'owner':
-						const { OwnerController } = await import('@/controllers/owner.controller');
-						registerController(app, OwnerController);
+						await import('@/controllers/owner.controller');
 						break;
 
 					case 'users':
-						const { UsersController } = await import('@/controllers/users.controller');
-						registerController(app, UsersController);
+						await import('@/controllers/users.controller');
 						break;
 
 					case 'invitations':
-						const { InvitationController } = await import('@/controllers/invitation.controller');
-						registerController(app, InvitationController);
+						await import('@/controllers/invitation.controller');
 						break;
 
 					case 'tags':
-						const { TagsController } = await import('@/controllers/tags.controller');
-						registerController(app, TagsController);
+						await import('@/controllers/tags.controller');
 						break;
 
 					case 'externalSecrets':
-						const { ExternalSecretsController } = await import(
-							'@/ExternalSecrets/ExternalSecrets.controller.ee'
-						);
-						registerController(app, ExternalSecretsController);
+						await import('@/ExternalSecrets/ExternalSecrets.controller.ee');
 						break;
 
 					case 'workflowHistory':
-						const { WorkflowHistoryController } = await import(
-							'@/workflows/workflowHistory/workflowHistory.controller.ee'
-						);
-						registerController(app, WorkflowHistoryController);
+						await import('@/workflows/workflowHistory/workflowHistory.controller.ee');
 						break;
 
 					case 'binaryData':
-						const { BinaryDataController } = await import('@/controllers/binaryData.controller');
-						registerController(app, BinaryDataController);
+						await import('@/controllers/binaryData.controller');
 						break;
 
 					case 'debug':
-						const { DebugController } = await import('@/controllers/debug.controller');
-						registerController(app, DebugController);
+						await import('@/controllers/debug.controller');
 						break;
 
 					case 'project':
-						const { ProjectController } = await import('@/controllers/project.controller');
-						registerController(app, ProjectController);
+						await import('@/controllers/project.controller');
 						break;
 
 					case 'role':
-						const { RoleController } = await import('@/controllers/role.controller');
-						registerController(app, RoleController);
+						await import('@/controllers/role.controller');
 						break;
 
 					case 'dynamic-node-parameters':
-						const { DynamicNodeParametersController } = await import(
-							'@/controllers/dynamicNodeParameters.controller'
-						);
-						registerController(app, DynamicNodeParametersController);
+						await import('@/controllers/dynamicNodeParameters.controller');
 						break;
 				}
 			}
+
+			Container.get(ControllerRegistry).activate(app);
 		}
 	});
 

packages/cli/test/unit/decorators/controller.registry.test.ts

@@ -0,0 +1,115 @@
+jest.mock('@/constants', () => ({
+	inProduction: true,
+}));
+
+import express from 'express';
+import { agent as testAgent } from 'supertest';
+import { mock } from 'jest-mock-extended';
+
+import { ControllerRegistry, Get, Licensed, RestController } from '@/decorators';
+import type { AuthService } from '@/auth/auth.service';
+import type { License } from '@/License';
+import type { SuperAgentTest } from '@test-integration/types';
+
+describe('ControllerRegistry', () => {
+	const license = mock<License>();
+	const authService = mock<AuthService>();
+	let agent: SuperAgentTest;
+
+	beforeEach(() => {
+		jest.resetAllMocks();
+		const app = express();
+		new ControllerRegistry(license, authService).activate(app);
+		agent = testAgent(app);
+	});
+
+	describe('Rate limiting', () => {
+		@RestController('/test')
+		// @ts-expect-error tsc complains about unused class
+		class TestController {
+			@Get('/unlimited')
+			unlimited() {
+				return { ok: true };
+			}
+
+			@Get('/rate-limited', { rateLimit: true })
+			rateLimited() {
+				return { ok: true };
+			}
+		}
+
+		beforeEach(() => {
+			authService.authMiddleware.mockImplementation(async (_req, _res, next) => next());
+		});
+
+		it('should not rate-limit by default', async () => {
+			for (let i = 0; i < 6; i++) {
+				await agent.get('/rest/test/unlimited').expect(200);
+			}
+		});
+
+		it('should rate-limit when configured', async () => {
+			for (let i = 0; i < 5; i++) {
+				await agent.get('/rest/test/rate-limited').expect(200);
+			}
+			await agent.get('/rest/test/rate-limited').expect(429);
+		});
+	});
+
+	describe('Authorization', () => {
+		@RestController('/test')
+		// @ts-expect-error tsc complains about unused class
+		class TestController {
+			@Get('/no-auth', { skipAuth: true })
+			noAuth() {
+				return { ok: true };
+			}
+
+			@Get('/auth')
+			auth() {
+				return { ok: true };
+			}
+		}
+
+		it('should not require auth if configured to skip', async () => {
+			await agent.get('/rest/test/no-auth').expect(200);
+			expect(authService.authMiddleware).not.toHaveBeenCalled();
+		});
+
+		it('should require auth by default', async () => {
+			authService.authMiddleware.mockImplementation(async (_req, res) => {
+				res.status(401).send();
+			});
+			await agent.get('/rest/test/auth').expect(401);
+			expect(authService.authMiddleware).toHaveBeenCalled();
+		});
+	});
+
+	describe('License checks', () => {
+		@RestController('/test')
+		// @ts-expect-error tsc complains about unused class
+		class TestController {
+			@Get('/with-sharing')
+			@Licensed('feat:sharing')
+			sharing() {
+				return { ok: true };
+			}
+		}
+
+		beforeEach(() => {
+			authService.authMiddleware.mockImplementation(async (_req, _res, next) => next());
+		});
+
+		it('should disallow when feature is missing', async () => {
+			license.isFeatureEnabled.calledWith('feat:sharing').mockReturnValue(false);
+			await agent.get('/rest/test/with-sharing').expect(403);
+			expect(license.isFeatureEnabled).toHaveBeenCalled();
+		});
+
+		it('should allow when feature is available', async () => {
+			license.isFeatureEnabled.calledWith('feat:sharing').mockReturnValue(true);
+			await agent.get('/rest/test/with-sharing').expect(200);
+			expect(license.isFeatureEnabled).toHaveBeenCalled();
+		});
+	});
+});

packages/cli/test/unit/decorators/registerController.test.ts

@@ -1,39 +0,0 @@
-jest.mock('@/constants', () => ({
-	inProduction: true,
-}));
-
-import express from 'express';
-import { agent as testAgent } from 'supertest';
-
-import { Get, RestController, registerController } from '@/decorators';
-import { AuthService } from '@/auth/auth.service';
-import { mockInstance } from '../../shared/mocking';
-
-describe('registerController', () => {
-	@RestController('/test')
-	class TestController {
-		@Get('/unlimited', { skipAuth: true })
-		@Get('/rate-limited', { skipAuth: true, rateLimit: {} })
-		endpoint() {
-			return { ok: true };
-		}
-	}
-
-	mockInstance(AuthService);
-	const app = express();
-	registerController(app, TestController);
-	const agent = testAgent(app);
-
-	it('should not rate-limit by default', async () => {
-		for (let i = 0; i < 6; i++) {
-			await agent.get('/rest/test/unlimited').expect(200);
-		}
-	});
-
-	it('should rate-limit when configured', async () => {
-		for (let i = 0; i < 5; i++) {
-			await agent.get('/rest/test/rate-limited').expect(200);
-		}
-		await agent.get('/rest/test/rate-limited').expect(429);
-	});
-});