diff --git a/packages/cli/src/sso/saml/routes/__tests__/newFile.1.ts b/packages/cli/src/sso/saml/routes/__tests__/newFile.1.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/cli/src/sso/saml/routes/__tests__/newFile.ts b/packages/cli/src/sso/saml/routes/__tests__/newFile.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/cli/src/sso/saml/routes/__tests__/saml.controller.ee.test.ts b/packages/cli/src/sso/saml/routes/__tests__/saml.controller.ee.test.ts new file mode 100644 index 0000000000..c4a33ed441 --- /dev/null +++ b/packages/cli/src/sso/saml/routes/__tests__/saml.controller.ee.test.ts @@ -0,0 +1,77 @@ +import { type Response } from 'express'; +import { mock } from 'jest-mock-extended'; + +import type { User } from '@/databases/entities/user'; +import { UrlService } from '@/services/url.service'; +import { mockInstance } from '@test/mocking'; + +import { SamlService } from '../../saml.service.ee'; +import { getServiceProviderConfigTestReturnUrl } from '../../service-provider.ee'; +import type { SamlConfiguration } from '../../types/requests'; +import type { SamlUserAttributes } from '../../types/saml-user-attributes'; +import { SamlController } from '../saml.controller.ee'; + +const urlService = mockInstance(UrlService); +urlService.getInstanceBaseUrl.mockReturnValue(''); +const samlService = mockInstance(SamlService); +const controller = new SamlController(mock(), samlService, mock(), mock()); + +const user = mock({ + id: '123', + password: 'password', + authIdentities: [], + role: 'global:owner', +}); + +const attributes: SamlUserAttributes = { + email: 'test@example.com', + firstName: 'Test', + lastName: 'User', + userPrincipalName: 'upn:test@example.com', +}; + +describe('Test views', () => { + test('Should render success with template', async () => { + const req = mock(); + const res = mock(); + + req.body.RelayState = getServiceProviderConfigTestReturnUrl(); + samlService.handleSamlLogin.mockResolvedValueOnce({ + authenticatedUser: user, + attributes, + onboardingRequired: false, + }); + + await controller.acsPost(req, res); + + expect(res.render).toBeCalledWith('saml-connection-test-success', attributes); + }); + + test('Should render failure with template', async () => { + const req = mock(); + const res = mock(); + + req.body.RelayState = getServiceProviderConfigTestReturnUrl(); + samlService.handleSamlLogin.mockResolvedValueOnce({ + authenticatedUser: undefined, + attributes, + onboardingRequired: false, + }); + + await controller.acsPost(req, res); + + expect(res.render).toBeCalledWith('saml-connection-test-failed', { message: '', attributes }); + }); + + test('Should render error with template', async () => { + const req = mock(); + const res = mock(); + + req.body.RelayState = getServiceProviderConfigTestReturnUrl(); + samlService.handleSamlLogin.mockRejectedValueOnce(new Error('Test Error')); + + await controller.acsPost(req, res); + + expect(res.render).toBeCalledWith('saml-connection-test-failed', { message: 'Test Error' }); + }); +}); 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 77c8c19291..6558efa381 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts @@ -26,8 +26,6 @@ import { import type { SamlLoginBinding } from '../types'; import { SamlConfiguration } from '../types/requests'; import { getInitSSOFormView } from '../views/init-sso-post'; -import { getSamlConnectionTestFailedView } from '../views/saml-connection-test-failed'; -import { getSamlConnectionTestSuccessView } from '../views/saml-connection-test-success'; @RestController('/sso/saml') export class SamlController { @@ -92,7 +90,7 @@ export class SamlController { /** * Assertion Consumer Service endpoint */ - @Get('/acs', { middlewares: [samlLicensedMiddleware], skipAuth: true }) + @Get('/acs', { middlewares: [samlLicensedMiddleware], skipAuth: true, usesTemplates: true }) async acsGet(req: SamlConfiguration.AcsRequest, res: express.Response) { return await this.acsHandler(req, res, 'redirect'); } @@ -100,7 +98,7 @@ export class SamlController { /** * Assertion Consumer Service endpoint */ - @Post('/acs', { middlewares: [samlLicensedMiddleware], skipAuth: true }) + @Post('/acs', { middlewares: [samlLicensedMiddleware], skipAuth: true, usesTemplates: true }) async acsPost(req: SamlConfiguration.AcsRequest, res: express.Response) { return await this.acsHandler(req, res, 'post'); } @@ -120,9 +118,12 @@ export class SamlController { // if RelayState is set to the test connection Url, this is a test connection if (isConnectionTestRequest(req)) { if (loginResult.authenticatedUser) { - return res.send(getSamlConnectionTestSuccessView(loginResult.attributes)); + return res.render('saml-connection-test-success', loginResult.attributes); } else { - return res.send(getSamlConnectionTestFailedView('', loginResult.attributes)); + return res.render('saml-connection-test-failed', { + message: '', + attributes: loginResult.attributes, + }); } } if (loginResult.authenticatedUser) { @@ -151,7 +152,7 @@ export class SamlController { throw new AuthError('SAML Authentication failed'); } catch (error) { if (isConnectionTestRequest(req)) { - return res.send(getSamlConnectionTestFailedView((error as Error).message)); + return res.render('saml-connection-test-failed', { message: (error as Error).message }); } this.eventService.emit('user-login-failed', { userEmail: 'unknown', diff --git a/packages/cli/src/sso/saml/views/__tests__/saml-connection-test-failed.test.ts b/packages/cli/src/sso/saml/views/__tests__/saml-connection-test-failed.test.ts deleted file mode 100644 index 3cc0cf16bb..0000000000 --- a/packages/cli/src/sso/saml/views/__tests__/saml-connection-test-failed.test.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { getSamlConnectionTestFailedView } from '../saml-connection-test-failed'; - -const basicXssPayload = ''; -const basicXssMitigated = '<script>alert("1");</script>'; - -describe('SAML Connection Test Failed', () => { - test('should not allow XSS via error message', () => { - const result = getSamlConnectionTestFailedView('Test ' + basicXssPayload); - expect(result).not.toMatch(basicXssPayload); - expect(result).toMatch(basicXssMitigated); - }); - - test('should not allow XSS via attributes', () => { - const result = getSamlConnectionTestFailedView('', { - email: 'test@example.com', - firstName: 'Test', - lastName: 'McXss' + basicXssPayload, - userPrincipalName: 'test@example.com', - }); - expect(result).not.toMatch(basicXssPayload); - expect(result).toMatch(basicXssMitigated); - }); - - test('should replace undefined with (n/a)', () => { - expect( - getSamlConnectionTestFailedView('', { - firstName: 'No', - lastName: 'Email', - userPrincipalName: 'test@example.com', - }), - ).toMatch('Email: (n/a)'); - }); -}); diff --git a/packages/cli/src/sso/saml/views/__tests__/saml-connection-test-success.test.ts b/packages/cli/src/sso/saml/views/__tests__/saml-connection-test-success.test.ts deleted file mode 100644 index 2d448abab4..0000000000 --- a/packages/cli/src/sso/saml/views/__tests__/saml-connection-test-success.test.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { getSamlConnectionTestSuccessView } from '../saml-connection-test-success'; - -const basicXssPayload = ''; -const basicXssMitigated = '<script>alert("1");</script>'; - -describe('SAML Connection Test Succeeded', () => { - test('should not allow XSS via attributes', () => { - const result = getSamlConnectionTestSuccessView({ - email: 'test@example.com', - firstName: 'Test', - lastName: 'McXss' + basicXssPayload, - userPrincipalName: 'test@example.com', - }); - expect(result).not.toMatch(basicXssPayload); - expect(result).toMatch(basicXssMitigated); - }); - - test('should replace undefined with (n/a)', () => { - expect( - getSamlConnectionTestSuccessView({ - firstName: 'No', - lastName: 'Email', - userPrincipalName: 'test@example.com', - }), - ).toMatch('Email: (n/a)'); - }); -}); diff --git a/packages/cli/src/sso/saml/views/saml-connection-test-failed.ts b/packages/cli/src/sso/saml/views/saml-connection-test-failed.ts deleted file mode 100644 index 9890ffc94b..0000000000 --- a/packages/cli/src/sso/saml/views/saml-connection-test-failed.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { compile } from 'handlebars'; - -import type { SamlUserAttributes } from '../types/saml-user-attributes'; - -const failedTemplate = compile<{ message?: string; attributes?: SamlUserAttributes }>(` - - - n8n - SAML Connection Test Result - - - -
-

SAML Connection Test failed

-

{{message}}

- -

- {{#with attributes}} -

Here are the attributes returned by your SAML IdP:

-
    -
  • Email: {{email}}
    • -
  • First Name: {{firstName}}
    • -
  • Last Name: {{lastName}}
    • -
  • UPN: {{userPrincipalName}}
    • - {{/with}} -
-
- - -`); - -export function getSamlConnectionTestFailedView( - message?: string, - attributes?: Partial, -): string { - return failedTemplate({ - message: message ?? 'A common issue could be that no email attribute is set', - attributes: attributes && { - email: attributes.email ?? '(n/a)', - firstName: attributes.firstName ?? '(n/a)', - lastName: attributes.lastName ?? '(n/a)', - userPrincipalName: attributes.userPrincipalName ?? '(n/a)', - }, - }); -} diff --git a/packages/cli/src/sso/saml/views/saml-connection-test-success.ts b/packages/cli/src/sso/saml/views/saml-connection-test-success.ts deleted file mode 100644 index 29d9f0f009..0000000000 --- a/packages/cli/src/sso/saml/views/saml-connection-test-success.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { compile } from 'handlebars'; - -import type { SamlUserAttributes } from '../types/saml-user-attributes'; - -const successTemplate = compile(` - - - n8n - SAML Connection Test Result - - - -
-

SAML Connection Test was successful

- -

-

Here are the attributes returned by your SAML IdP:

-
    -
  • Email: {{email}}
    • -
  • First Name: {{firstName}}
    • -
  • Last Name: {{lastName}}
    • -
  • UPN: {{userPrincipalName}}
    • -
-
- - - `); - -export function getSamlConnectionTestSuccessView(attributes: Partial): string { - return successTemplate({ - email: attributes.email ?? '(n/a)', - firstName: attributes.firstName ?? '(n/a)', - lastName: attributes.lastName ?? '(n/a)', - userPrincipalName: attributes.userPrincipalName ?? '(n/a)', - }); -} diff --git a/packages/cli/templates/saml-connection-test-failed.handlebars b/packages/cli/templates/saml-connection-test-failed.handlebars new file mode 100644 index 0000000000..f93cea5c2d --- /dev/null +++ b/packages/cli/templates/saml-connection-test-failed.handlebars @@ -0,0 +1,30 @@ + + + n8n - SAML Connection Test Result + + + +
+

SAML Connection Test failed

+

{{#if message}}{{message}}{{else}}A common issue could be that no email attribute is set{{/if}}

+ +

+ {{#with attributes}} +

Here are the attributes returned by your SAML IdP:

+
    +
  • Email: {{#if email}}{{email}}{{else}}(n/a){{/if}}
    • +
  • First Name: {{#if firstName}}{{firstName}}{{else}}(n/a){{/if}}
    • +
  • Last Name: {{#if lastName}}{{lastName}}{{else}}(n/a){{/if}}
    • +
  • UPN: {{#if userPrincipalName}}{{userPrincipalName}}{{else}}(n/a){{/if}}
    • + {{/with}} +
+
+ + diff --git a/packages/cli/templates/saml-connection-test-success.handlebars b/packages/cli/templates/saml-connection-test-success.handlebars new file mode 100644 index 0000000000..e65d29483d --- /dev/null +++ b/packages/cli/templates/saml-connection-test-success.handlebars @@ -0,0 +1,27 @@ + + + n8n - SAML Connection Test Result + + + +
+

SAML Connection Test was successful

+ +

+

Here are the attributes returned by your SAML IdP:

+
    +
  • Email: {{#if email}}{{email}}{{else}}(n/a){{/if}}
    • +
  • First Name: {{#if firstName}}{{firstName}}{{else}}(n/a){{/if}}
    • +
  • Last Name: {{#if lastName}}{{lastName}}{{else}}(n/a){{/if}}
    • +
  • UPN: {{#if userPrincipalName}}{{userPrincipalName}}{{else}}(n/a){{/if}}
    • +
+
+ +