diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 63f0a6b6be..374cd9b9e9 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -337,6 +337,7 @@ export interface IDiagnosticInfo { n8n_multi_user_allowed: boolean; smtp_set_up: boolean; ldap_allowed: boolean; + saml_enabled: boolean; } export interface ITelemetryUserDeletionData { diff --git a/packages/cli/src/InternalHooks.ts b/packages/cli/src/InternalHooks.ts index 965f84642b..dd29b0874e 100644 --- a/packages/cli/src/InternalHooks.ts +++ b/packages/cli/src/InternalHooks.ts @@ -5,6 +5,7 @@ import { Service } from 'typedi'; import { snakeCase } from 'change-case'; import { BinaryDataManager } from 'n8n-core'; import type { + AuthenticationMethod, ExecutionStatus, INodesGraphResult, IRun, @@ -80,6 +81,7 @@ export class InternalHooks implements IInternalHooksClass { n8n_multi_user_allowed: diagnosticInfo.n8n_multi_user_allowed, smtp_set_up: diagnosticInfo.smtp_set_up, ldap_allowed: diagnosticInfo.ldap_allowed, + saml_enabled: diagnosticInfo.saml_enabled, }; return Promise.all([ @@ -732,6 +734,38 @@ export class InternalHooks implements IInternalHooksClass { ]); } + async onUserLoginSuccess(userLoginData: { + user: User; + authenticationMethod: AuthenticationMethod; + }): Promise { + void Promise.all([ + eventBus.sendAuditEvent({ + eventName: 'n8n.audit.user.login.success', + payload: { + authenticationMethod: userLoginData.authenticationMethod, + ...userToPayload(userLoginData.user), + }, + }), + ]); + } + + async onUserLoginFailed(userLoginData: { + user: string; + authenticationMethod: AuthenticationMethod; + reason?: string; + }): Promise { + void Promise.all([ + eventBus.sendAuditEvent({ + eventName: 'n8n.audit.user.login.failed', + payload: { + authenticationMethod: userLoginData.authenticationMethod, + user: userLoginData.user, + reason: userLoginData.reason, + }, + }), + ]); + } + /** * Credentials */ diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index e8eb44eba3..ddaf2a6f90 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -159,7 +159,11 @@ import { SamlService } from './sso/saml/saml.service.ee'; import { variablesController } from './environments/variables/variables.controller'; import { LdapManager } from './Ldap/LdapManager.ee'; import { getVariablesLimit, isVariablesEnabled } from '@/environments/variables/enviromentHelpers'; -import { getCurrentAuthenticationMethod } from './sso/ssoHelpers'; +import { + getCurrentAuthenticationMethod, + isLdapCurrentAuthenticationMethod, + isSamlCurrentAuthenticationMethod, +} from './sso/ssoHelpers'; import { isVersionControlLicensed } from '@/environments/versionControl/versionControlHelper'; import { VersionControlService } from '@/environments/versionControl/versionControl.service.ee'; import { VersionControlController } from '@/environments/versionControl/versionControl.controller.ee'; @@ -1419,7 +1423,8 @@ export async function start(): Promise { binaryDataMode: binaryDataConfig.mode, n8n_multi_user_allowed: isUserManagementEnabled(), smtp_set_up: config.getEnv('userManagement.emails.mode') === 'smtp', - ldap_allowed: isLdapEnabled(), + ldap_allowed: isLdapCurrentAuthenticationMethod(), + saml_enabled: isSamlCurrentAuthenticationMethod(), }; // Set up event handling diff --git a/packages/cli/src/controllers/auth.controller.ts b/packages/cli/src/controllers/auth.controller.ts index 7aeb7e1cfe..3492fe7c14 100644 --- a/packages/cli/src/controllers/auth.controller.ts +++ b/packages/cli/src/controllers/auth.controller.ts @@ -19,10 +19,13 @@ import type { import { handleEmailLogin, handleLdapLogin } from '@/auth'; import type { PostHogClient } from '@/posthog'; import { + getCurrentAuthenticationMethod, isLdapCurrentAuthenticationMethod, isSamlCurrentAuthenticationMethod, } from '@/sso/ssoHelpers'; import type { UserRepository } from '@db/repositories'; +import { InternalHooks } from '../InternalHooks'; +import Container from 'typedi'; @RestController() export class AuthController { @@ -67,12 +70,15 @@ export class AuthController { let user: User | undefined; + let usedAuthenticationMethod = getCurrentAuthenticationMethod(); + if (isSamlCurrentAuthenticationMethod()) { // attempt to fetch user data with the credentials, but don't log in yet const preliminaryUser = await handleEmailLogin(email, password); // if the user is an owner, continue with the login if (preliminaryUser?.globalRole?.name === 'owner') { user = preliminaryUser; + usedAuthenticationMethod = 'email'; } else { throw new AuthError('SAML is enabled, please log in with SAML'); } @@ -83,9 +89,17 @@ export class AuthController { } if (user) { await issueCookie(res, user); + void Container.get(InternalHooks).onUserLoginSuccess({ + user, + authenticationMethod: usedAuthenticationMethod, + }); return withFeatureFlags(this.postHog, sanitizeUser(user)); } - + void Container.get(InternalHooks).onUserLoginFailed({ + user: email, + authenticationMethod: usedAuthenticationMethod, + reason: 'wrong credentials', + }); throw new AuthError('Wrong username or password. Do you have caps lock on?'); } diff --git a/packages/cli/src/eventbus/EventMessageClasses/index.ts b/packages/cli/src/eventbus/EventMessageClasses/index.ts index 4b4fe47f8d..28da7c5ecc 100644 --- a/packages/cli/src/eventbus/EventMessageClasses/index.ts +++ b/packages/cli/src/eventbus/EventMessageClasses/index.ts @@ -11,6 +11,8 @@ export const eventNamesWorkflow = [ ] as const; export const eventNamesNode = ['n8n.node.started', 'n8n.node.finished'] as const; export const eventNamesAudit = [ + 'n8n.audit.user.login.success', + 'n8n.audit.user.login.failed', 'n8n.audit.user.signedup', 'n8n.audit.user.updated', 'n8n.audit.user.deleted', diff --git a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts index c6b04cae46..d93bfe5fdb 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts @@ -1,6 +1,6 @@ import express from 'express'; import { getInstanceBaseUrl } from '@/UserManagement/UserManagementHelper'; -import { Authorized, Get, Post, RestController } from '@/decorators'; +import { Authorized, Get, NoAuthRequired, Post, RestController } from '@/decorators'; import { SamlUrls } from '../constants'; import { samlLicensedAndEnabledMiddleware, @@ -23,11 +23,14 @@ import { } from '../serviceProvider.ee'; import { getSamlConnectionTestSuccessView } from '../views/samlConnectionTestSuccess'; import { getSamlConnectionTestFailedView } from '../views/samlConnectionTestFailed'; +import Container from 'typedi'; +import { InternalHooks } from '@/InternalHooks'; @RestController('/sso/saml') export class SamlController { constructor(private samlService: SamlService) {} + @NoAuthRequired() @Get(SamlUrls.metadata) async getServiceProviderMetadata(req: express.Request, res: express.Response) { return res @@ -39,7 +42,7 @@ export class SamlController { * GET /sso/saml/config * Return SAML config */ - @Authorized(['global', 'owner']) + @Authorized('any') @Get(SamlUrls.config, { middlewares: [samlLicensedMiddleware] }) async configGet() { const prefs = this.samlService.samlPreferences; @@ -87,6 +90,7 @@ export class SamlController { * GET /sso/saml/acs * Assertion Consumer Service endpoint */ + @NoAuthRequired() @Get(SamlUrls.acs, { middlewares: [samlLicensedMiddleware] }) async acsGet(req: SamlConfiguration.AcsRequest, res: express.Response) { return this.acsHandler(req, res, 'redirect'); @@ -96,6 +100,7 @@ export class SamlController { * POST /sso/saml/acs * Assertion Consumer Service endpoint */ + @NoAuthRequired() @Post(SamlUrls.acs, { middlewares: [samlLicensedMiddleware] }) async acsPost(req: SamlConfiguration.AcsRequest, res: express.Response) { return this.acsHandler(req, res, 'post'); @@ -122,6 +127,10 @@ export class SamlController { } } if (loginResult.authenticatedUser) { + void Container.get(InternalHooks).onUserLoginSuccess({ + user: loginResult.authenticatedUser, + authenticationMethod: 'saml', + }); // Only sign in user if SAML is enabled, otherwise treat as test connection if (isSamlLicensedAndEnabled()) { await issueCookie(res, loginResult.authenticatedUser); @@ -134,11 +143,19 @@ export class SamlController { return res.status(202).send(loginResult.attributes); } } + void Container.get(InternalHooks).onUserLoginFailed({ + user: loginResult.attributes.email ?? 'unknown', + authenticationMethod: 'saml', + }); throw new AuthError('SAML Authentication failed'); } catch (error) { if (isConnectionTestRequest(req)) { return res.send(getSamlConnectionTestFailedView((error as Error).message)); } + void Container.get(InternalHooks).onUserLoginFailed({ + user: 'unknown', + authenticationMethod: 'saml', + }); throw new AuthError('SAML Authentication failed: ' + (error as Error).message); } } @@ -148,6 +165,7 @@ export class SamlController { * Access URL for implementing SP-init SSO * This endpoint is available if SAML is licensed and enabled */ + @NoAuthRequired() @Get(SamlUrls.initSSO, { middlewares: [samlLicensedAndEnabledMiddleware] }) async initSsoGet(req: express.Request, res: express.Response) { return this.handleInitSSO(res); diff --git a/packages/cli/test/integration/saml/saml.api.test.ts b/packages/cli/test/integration/saml/saml.api.test.ts index fa52693b1b..08dfa72419 100644 --- a/packages/cli/test/integration/saml/saml.api.test.ts +++ b/packages/cli/test/integration/saml/saml.api.test.ts @@ -3,13 +3,21 @@ import type { SuperAgentTest } from 'supertest'; import type { User } from '@db/entities/User'; import { setSamlLoginEnabled } from '@/sso/saml/samlHelpers'; import { getCurrentAuthenticationMethod, setCurrentAuthenticationMethod } from '@/sso/ssoHelpers'; +import { SamlUrls } from '@/sso/saml/constants'; import { License } from '@/License'; import { randomEmail, randomName, randomValidPassword } from '../shared/random'; import * as testDb from '../shared/testDb'; import * as utils from '../shared/utils'; import { sampleConfig } from './sampleMetadata'; +import { InternalHooks } from '@/InternalHooks'; +import { SamlService } from '@/sso/saml/saml.service.ee'; +import { SamlUserAttributes } from '@/sso/saml/types/samlUserAttributes'; +import { AuthenticationMethod } from 'n8n-workflow'; +let someUser: User; let owner: User; +let noAuthMemberAgent: SuperAgentTest; +let authMemberAgent: SuperAgentTest; let authOwnerAgent: SuperAgentTest; async function enableSaml(enable: boolean) { @@ -20,7 +28,10 @@ beforeAll(async () => { Container.get(License).isSamlEnabled = () => true; const app = await utils.initTestServer({ endpointGroups: ['me', 'saml'] }); owner = await testDb.createOwner(); + someUser = await testDb.createUser(); authOwnerAgent = utils.createAuthAgent(app)(owner); + authMemberAgent = utils.createAgent(app, { auth: true, user: someUser }); + noAuthMemberAgent = utils.createAgent(app, { auth: false, user: someUser }); }); afterAll(async () => { @@ -129,6 +140,216 @@ describe('Instance owner', () => { .expect(500); expect(getCurrentAuthenticationMethod()).toBe('ldap'); + await setCurrentAuthenticationMethod('saml'); }); }); }); + +describe('Check endpoint permissions', () => { + beforeEach(async () => { + await enableSaml(true); + }); + describe('Owner', () => { + test(`should be able to access ${SamlUrls.metadata}`, async () => { + await authOwnerAgent.get(`/sso/saml${SamlUrls.metadata}`).expect(200); + }); + + test(`should be able to access GET ${SamlUrls.config}`, async () => { + await authOwnerAgent.get(`/sso/saml${SamlUrls.config}`).expect(200); + }); + + test(`should be able to access POST ${SamlUrls.config}`, async () => { + await authOwnerAgent.post(`/sso/saml${SamlUrls.config}`).expect(200); + }); + + test(`should be able to access POST ${SamlUrls.configToggleEnabled}`, async () => { + await authOwnerAgent.post(`/sso/saml${SamlUrls.configToggleEnabled}`).expect(400); + }); + + test(`should be able to access GET ${SamlUrls.acs}`, async () => { + // Note that 401 here is coming from the missing SAML object, + // not from not being able to access the endpoint, so this is expected! + const response = await authOwnerAgent.get(`/sso/saml${SamlUrls.acs}`).expect(401); + expect(response.text).toContain('SAML Authentication failed'); + }); + + test(`should be able to access POST ${SamlUrls.acs}`, async () => { + // Note that 401 here is coming from the missing SAML object, + // not from not being able to access the endpoint, so this is expected! + const response = await authOwnerAgent.post(`/sso/saml${SamlUrls.acs}`).expect(401); + expect(response.text).toContain('SAML Authentication failed'); + }); + + test(`should be able to access GET ${SamlUrls.initSSO}`, async () => { + const response = await authOwnerAgent.get(`/sso/saml${SamlUrls.initSSO}`).expect(200); + }); + + test(`should be able to access GET ${SamlUrls.configTest}`, async () => { + await authOwnerAgent.get(`/sso/saml${SamlUrls.configTest}`).expect(200); + }); + }); + describe('Authenticated Member', () => { + test(`should be able to access ${SamlUrls.metadata}`, async () => { + await authMemberAgent.get(`/sso/saml${SamlUrls.metadata}`).expect(200); + }); + + test(`should be able to access GET ${SamlUrls.config}`, async () => { + await authMemberAgent.get(`/sso/saml${SamlUrls.config}`).expect(200); + }); + + test(`should NOT be able to access POST ${SamlUrls.config}`, async () => { + await authMemberAgent.post(`/sso/saml${SamlUrls.config}`).expect(403); + }); + + test(`should NOT be able to access POST ${SamlUrls.configToggleEnabled}`, async () => { + await authMemberAgent.post(`/sso/saml${SamlUrls.configToggleEnabled}`).expect(403); + }); + + test(`should be able to access GET ${SamlUrls.acs}`, async () => { + // Note that 401 here is coming from the missing SAML object, + // not from not being able to access the endpoint, so this is expected! + const response = await authMemberAgent.get(`/sso/saml${SamlUrls.acs}`).expect(401); + expect(response.text).toContain('SAML Authentication failed'); + }); + + test(`should be able to access POST ${SamlUrls.acs}`, async () => { + // Note that 401 here is coming from the missing SAML object, + // not from not being able to access the endpoint, so this is expected! + const response = await authMemberAgent.post(`/sso/saml${SamlUrls.acs}`).expect(401); + expect(response.text).toContain('SAML Authentication failed'); + }); + + test(`should be able to access GET ${SamlUrls.initSSO}`, async () => { + const response = await authMemberAgent.get(`/sso/saml${SamlUrls.initSSO}`).expect(200); + }); + + test(`should NOT be able to access GET ${SamlUrls.configTest}`, async () => { + await authMemberAgent.get(`/sso/saml${SamlUrls.configTest}`).expect(403); + }); + }); + describe('Non-Authenticated User', () => { + test(`should be able to access ${SamlUrls.metadata}`, async () => { + await noAuthMemberAgent.get(`/sso/saml${SamlUrls.metadata}`).expect(200); + }); + + test(`should NOT be able to access GET ${SamlUrls.config}`, async () => { + await noAuthMemberAgent.get(`/sso/saml${SamlUrls.config}`).expect(401); + }); + + test(`should NOT be able to access POST ${SamlUrls.config}`, async () => { + await noAuthMemberAgent.post(`/sso/saml${SamlUrls.config}`).expect(401); + }); + + test(`should NOT be able to access POST ${SamlUrls.configToggleEnabled}`, async () => { + await noAuthMemberAgent.post(`/sso/saml${SamlUrls.configToggleEnabled}`).expect(401); + }); + + test(`should be able to access GET ${SamlUrls.acs}`, async () => { + // Note that 401 here is coming from the missing SAML object, + // not from not being able to access the endpoint, so this is expected! + const response = await noAuthMemberAgent.get(`/sso/saml${SamlUrls.acs}`).expect(401); + expect(response.text).toContain('SAML Authentication failed'); + }); + + test(`should be able to access POST ${SamlUrls.acs}`, async () => { + // Note that 401 here is coming from the missing SAML object, + // not from not being able to access the endpoint, so this is expected! + const response = await noAuthMemberAgent.post(`/sso/saml${SamlUrls.acs}`).expect(401); + expect(response.text).toContain('SAML Authentication failed'); + }); + + test(`should be able to access GET ${SamlUrls.initSSO}`, async () => { + const response = await noAuthMemberAgent.get(`/sso/saml${SamlUrls.initSSO}`).expect(200); + }); + + test(`should NOT be able to access GET ${SamlUrls.configTest}`, async () => { + await noAuthMemberAgent.get(`/sso/saml${SamlUrls.configTest}`).expect(401); + }); + }); +}); + +describe('SAML login flow', () => { + beforeEach(async () => { + await enableSaml(true); + }); + + test('should trigger onUserLoginSuccess hook', async () => { + const mockedHandleSamlLogin = jest.spyOn(Container.get(SamlService), 'handleSamlLogin'); + + mockedHandleSamlLogin.mockImplementation( + async (): Promise<{ + authenticatedUser: User; + attributes: SamlUserAttributes; + onboardingRequired: false; + }> => { + return { + authenticatedUser: someUser, + attributes: { + email: someUser.email, + firstName: someUser.firstName, + lastName: someUser.lastName, + userPrincipalName: someUser.email, + }, + onboardingRequired: false, + }; + }, + ); + + const mockedHookOnUserLoginSuccess = jest.spyOn( + Container.get(InternalHooks), + 'onUserLoginSuccess', + ); + mockedHookOnUserLoginSuccess.mockImplementation( + async (userLoginData: { user: User; authenticationMethod: AuthenticationMethod }) => { + expect(userLoginData.authenticationMethod).toEqual('saml'); + return; + }, + ); + const response = await authOwnerAgent.post(`/sso/saml${SamlUrls.acs}`).expect(302); + expect(mockedHookOnUserLoginSuccess).toBeCalled(); + mockedHookOnUserLoginSuccess.mockRestore(); + mockedHandleSamlLogin.mockRestore(); + }); + + test('should trigger onUserLoginFailed hook', async () => { + const mockedHandleSamlLogin = jest.spyOn(Container.get(SamlService), 'handleSamlLogin'); + + mockedHandleSamlLogin.mockImplementation( + async (): Promise<{ + authenticatedUser: User | undefined; + attributes: SamlUserAttributes; + onboardingRequired: false; + }> => { + return { + authenticatedUser: undefined, + attributes: { + email: someUser.email, + firstName: someUser.firstName, + lastName: someUser.lastName, + userPrincipalName: someUser.email, + }, + onboardingRequired: false, + }; + }, + ); + + const mockedHookOnUserLoginFailed = jest.spyOn( + Container.get(InternalHooks), + 'onUserLoginFailed', + ); + mockedHookOnUserLoginFailed.mockImplementation( + async (userLoginData: { + user: string; + authenticationMethod: AuthenticationMethod; + reason?: string; + }) => { + expect(userLoginData.authenticationMethod).toEqual('saml'); + return; + }, + ); + const response = await authOwnerAgent.post(`/sso/saml${SamlUrls.acs}`).expect(401); + expect(mockedHookOnUserLoginFailed).toBeCalled(); + mockedHookOnUserLoginFailed.mockRestore(); + mockedHandleSamlLogin.mockRestore(); + }); +});