From 9e240d6d748381a441c9f02ea5311da3f229f74b Mon Sep 17 00:00:00 2001 From: Stephen Wright Date: Mon, 10 Nov 2025 11:47:53 +0000 Subject: [PATCH] feat: Add unit tests for getAttributesFromLoginResponse and handleSamlLogin (#21678) Co-authored-by: konstantintieber --- .../saml/__tests__/saml-helpers.test.ts | 82 ++++++++++++- .../saml/__tests__/saml.service.ee.test.ts | 114 +++++++++++++++++- 2 files changed, 190 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/sso.ee/saml/__tests__/saml-helpers.test.ts b/packages/cli/src/sso.ee/saml/__tests__/saml-helpers.test.ts index ff21f14a964..9ada785fb9c 100644 --- a/packages/cli/src/sso.ee/saml/__tests__/saml-helpers.test.ts +++ b/packages/cli/src/sso.ee/saml/__tests__/saml-helpers.test.ts @@ -133,7 +133,6 @@ describe('sso/saml/samlHelpers', () => { firstName: 'test', lastName: 'test', userPrincipalName: 'test', - projectRoles: ['projectRole1', 'projectRole2'], instanceRole: 'instanceRole', }, }, @@ -161,10 +160,89 @@ describe('sso/saml/samlHelpers', () => { firstName: 'test', lastName: 'test', userPrincipalName: 'test', - n8nProjectRoles: ['projectRole1', 'projectRole2'], }, missingAttributes: [], }); }); }); + + test('returns the attributes from the flow result with project roles', () => { + const flowResult = { + extract: { + attributes: { + email: 'test@test.com', + firstName: 'test', + lastName: 'test', + userPrincipalName: 'test', + projectRoles: ['projectRole1', 'projectRole2'], + }, + }, + } as any; + const attributeMapping = { + email: 'email', + instanceRole: 'instanceRole', + firstName: 'firstName', + lastName: 'lastName', + userPrincipalName: 'userPrincipalName', + }; + const jitClaimNames = { + instanceRole: 'instanceRole', + projectRoles: 'projectRoles', + }; + const result = helpers.getMappedSamlAttributesFromFlowResult( + flowResult, + attributeMapping, + jitClaimNames, + ); + expect(result).toEqual({ + attributes: { + email: 'test@test.com', + firstName: 'test', + lastName: 'test', + userPrincipalName: 'test', + n8nProjectRoles: ['projectRole1', 'projectRole2'], + }, + missingAttributes: [], + }); + }); + + test('maps single projectRoles string to array', () => { + const flowResult = { + extract: { + attributes: { + email: 'test@test.com', + firstName: 'test', + lastName: 'test', + userPrincipalName: 'test', + projectRoles: 'projectRole1', + }, + }, + } as any; + const attributeMapping = { + email: 'email', + instanceRole: 'instanceRole', + firstName: 'firstName', + lastName: 'lastName', + userPrincipalName: 'userPrincipalName', + }; + const jitClaimNames = { + instanceRole: 'instanceRole', + projectRoles: 'projectRoles', + }; + const result = helpers.getMappedSamlAttributesFromFlowResult( + flowResult, + attributeMapping, + jitClaimNames, + ); + expect(result).toEqual({ + attributes: { + email: 'test@test.com', + firstName: 'test', + lastName: 'test', + userPrincipalName: 'test', + n8nProjectRoles: ['projectRole1'], + }, + missingAttributes: [], + }); + }); }); diff --git a/packages/cli/src/sso.ee/saml/__tests__/saml.service.ee.test.ts b/packages/cli/src/sso.ee/saml/__tests__/saml.service.ee.test.ts index 4e94331b713..784a631d9e1 100644 --- a/packages/cli/src/sso.ee/saml/__tests__/saml.service.ee.test.ts +++ b/packages/cli/src/sso.ee/saml/__tests__/saml.service.ee.test.ts @@ -2,7 +2,7 @@ import type { SamlPreferences } from '@n8n/api-types'; import { mockInstance, mockLogger } from '@n8n/backend-test-utils'; import type { GlobalConfig } from '@n8n/config'; import { SettingsRepository } from '@n8n/db'; -import type { UserRepository, Settings } from '@n8n/db'; +import type { UserRepository, Settings, User } from '@n8n/db'; import { Container } from '@n8n/di'; import axios from 'axios'; import type express from 'express'; @@ -251,6 +251,46 @@ describe('SamlService', () => { 'SAML Authentication failed. Invalid SAML response (missing attributes: http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress, http://schemas.xmlsoap.org/ws/2005/05/identity/claims/firstname, http://schemas.xmlsoap.org/ws/2005/05/identity/claims/lastname, http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn).', ); }); + + test('returns the attributes when they are present', async () => { + jest + .spyOn(samlService, 'getIdentityProviderInstance') + .mockReturnValue(mock()); + const serviceProviderInstance = mock(); + serviceProviderInstance.parseLoginResponse.mockResolvedValue({ + samlContent: '', + extract: {}, + }); + jest + .spyOn(samlService, 'getServiceProviderInstance') + .mockReturnValue(serviceProviderInstance); + + jest.spyOn(samlHelpers, 'getMappedSamlAttributesFromFlowResult').mockReturnValue({ + attributes: { + email: 'test@test.com', + firstName: 'test', + lastName: 'test', + userPrincipalName: 'test', + n8nInstanceRole: 'global:admin', + n8nProjectRoles: ['projectRole1', 'projectRole2'], + }, + missingAttributes: [], + }); + + const attributes = await samlService.getAttributesFromLoginResponse( + mock(), + 'post', + ); + + expect(attributes).toEqual({ + email: 'test@test.com', + firstName: 'test', + lastName: 'test', + userPrincipalName: 'test', + n8nInstanceRole: 'global:admin', + n8nProjectRoles: ['projectRole1', 'projectRole2'], + }); + }); }); describe('init', () => { @@ -320,10 +360,7 @@ describe('SamlService', () => { }); }); - // TODO: add tests for getAttributesFromLoginResponse - describe('handleSamlLogin', () => { - // TODO: add test cases for remaining logic (so far only for onboarding user) it('throws error for invalid email', async () => { jest.spyOn(samlService, 'getAttributesFromLoginResponse').mockResolvedValue({ email: 'invalid', @@ -363,6 +400,75 @@ describe('SamlService', () => { }); }); + it('logs in user that has not completed onboarding', async () => { + const samlAttributes = { + email: 'foo@bar.com', + firstName: '', + lastName: '', + userPrincipalName: 'foo@bar.com', + }; + const mockUser = { + id: '123', + email: samlAttributes.email, + authIdentities: [], + } as any; + jest.spyOn(samlService, 'getAttributesFromLoginResponse').mockResolvedValue(samlAttributes); + jest.spyOn(userRepository, 'findOne').mockResolvedValue(mockUser); + jest.spyOn(samlHelpers, 'updateUserFromSamlAttributes').mockResolvedValue(mockUser); + + const loginResult = await samlService.handleSamlLogin(mock(), 'post'); + + expect(loginResult).toEqual({ + authenticatedUser: mockUser, + attributes: samlAttributes, + onboardingRequired: true, + }); + }); + + it('does not log in the user if sso just-in-time provisioning is disabled', async () => { + const samlAttributes = { + email: 'foo@bar.com', + firstName: '', + lastName: '', + userPrincipalName: 'foo@bar.com', + }; + + jest.spyOn(samlService, 'getAttributesFromLoginResponse').mockResolvedValue(samlAttributes); + jest.spyOn(userRepository, 'findOne').mockResolvedValue(null); + jest.spyOn(ssoHelpers, 'isSsoJustInTimeProvisioningEnabled').mockReturnValue(false); + + const loginResult = await samlService.handleSamlLogin(mock(), 'post'); + + expect(loginResult).toEqual({ + authenticatedUser: undefined, + attributes: samlAttributes, + onboardingRequired: false, + }); + }); + + it('logs in the user if just-in-time provisioning is enabled', async () => { + const samlAttributes = { + email: 'foo@bar.com', + firstName: '', + lastName: '', + userPrincipalName: 'foo@bar.com', + }; + const mockUser = mock(); + + jest.spyOn(samlService, 'getAttributesFromLoginResponse').mockResolvedValue(samlAttributes); + jest.spyOn(userRepository, 'findOne').mockResolvedValue(null); + jest.spyOn(samlHelpers, 'createUserFromSamlAttributes').mockResolvedValue(mockUser); + jest.spyOn(ssoHelpers, 'isSsoJustInTimeProvisioningEnabled').mockReturnValue(true); + + const loginResult = await samlService.handleSamlLogin(mock(), 'post'); + + expect(loginResult).toEqual({ + authenticatedUser: mockUser, + attributes: samlAttributes, + onboardingRequired: true, + }); + }); + it('provisions instance and project role for onboarded user', async () => { const samlAttributes = { email: 'foo@bar.com',