diff --git a/packages/cli/src/modules/provisioning.ee/__tests__/provisioning.service.ee.test.ts b/packages/cli/src/modules/provisioning.ee/__tests__/provisioning.service.ee.test.ts index fcc2aa2060a..c5c00c03eb3 100644 --- a/packages/cli/src/modules/provisioning.ee/__tests__/provisioning.service.ee.test.ts +++ b/packages/cli/src/modules/provisioning.ee/__tests__/provisioning.service.ee.test.ts @@ -983,6 +983,36 @@ describe('ProvisioningService', () => { ); }); + it('should emit a debug log summarising the resolution without leaking claim values', async () => { + roleResolverService.resolveRoles.mockResolvedValue({ + instanceRole: { + role: 'global:member', + matchedRuleId: null, + expression: null, + isFallback: true, + }, + projectRoles: new Map(), + }); + roleRepository.findOneOrFail.mockResolvedValue( + mock({ slug: 'global:member', roleType: 'global' }), + ); + + const context = { + $claims: { groups: ['admins'], email: 'test@example.com' }, + $provider: 'oidc' as const, + }; + await provisioningService.provisionExpressionMappedRolesForUser(user, context); + + expect(logger.debug).toHaveBeenCalledWith('SSO role resolution complete', { + userId: 'user-1', + provider: 'oidc', + claimKeys: ['email', 'groups'], + matchedInstanceRuleId: null, + isFallback: true, + matchedProjectRuleIds: [], + }); + }); + it('should detect removed projects and role changes', async () => { const existingProject = mock({ id: 'old-proj-1', diff --git a/packages/cli/src/modules/provisioning.ee/__tests__/role-resolver.service.ee.test.ts b/packages/cli/src/modules/provisioning.ee/__tests__/role-resolver.service.ee.test.ts index 00e00fdcc69..fe0be424b6d 100644 --- a/packages/cli/src/modules/provisioning.ee/__tests__/role-resolver.service.ee.test.ts +++ b/packages/cli/src/modules/provisioning.ee/__tests__/role-resolver.service.ee.test.ts @@ -209,7 +209,30 @@ describe('RoleResolverService', () => { expect(result.instanceRole).toBe('global:member'); expect(logger.warn).toHaveBeenCalledWith( 'Role resolver expression evaluation failed, treating as false', - expect.objectContaining({ expression: '{{ throw new Error("bad") }}' }), + expect.objectContaining({ + ruleId: 'r1', + expression: '{{ throw new Error("bad") }}', + }), + ); + }); + + it('should emit a debug log for each evaluated rule', async () => { + const config = makeConfig({ + instanceRoleRules: [ + makeRule({ id: 'r1', expression: '{{ false }}', role: 'global:admin' }), + makeRule({ id: 'r2', expression: '{{ true }}', role: 'global:member' }), + ], + }); + + await resolveRolesSimple(config, makeContext()); + + expect(logger.debug).toHaveBeenCalledWith( + 'Role mapping rule evaluated', + expect.objectContaining({ ruleId: 'r1', matched: false }), + ); + expect(logger.debug).toHaveBeenCalledWith( + 'Role mapping rule evaluated', + expect.objectContaining({ ruleId: 'r2', matched: true }), ); }); diff --git a/packages/cli/src/modules/provisioning.ee/provisioning.service.ee.ts b/packages/cli/src/modules/provisioning.ee/provisioning.service.ee.ts index 036e121f363..92887f67afb 100644 --- a/packages/cli/src/modules/provisioning.ee/provisioning.service.ee.ts +++ b/packages/cli/src/modules/provisioning.ee/provisioning.service.ee.ts @@ -637,6 +637,15 @@ export class ProvisioningService { const config = await this.buildRoleMappingConfig(); const resolved = await this.roleResolverService.resolveRoles(config, context); + this.logger.debug('SSO role resolution complete', { + userId: user.id, + provider: context.$provider, + claimKeys: Object.keys(context.$claims ?? {}).sort(), + matchedInstanceRuleId: resolved.instanceRole.matchedRuleId, + isFallback: resolved.instanceRole.isFallback, + matchedProjectRuleIds: [...resolved.projectRoles.values()].map((r) => r.matchedRuleId), + }); + await this.applyExpressionMappedRoles(user, resolved); const newInstanceRole = resolved.instanceRole; diff --git a/packages/cli/src/modules/provisioning.ee/role-resolver.service.ee.ts b/packages/cli/src/modules/provisioning.ee/role-resolver.service.ee.ts index 377b8da522f..42557ff729d 100644 --- a/packages/cli/src/modules/provisioning.ee/role-resolver.service.ee.ts +++ b/packages/cli/src/modules/provisioning.ee/role-resolver.service.ee.ts @@ -74,7 +74,7 @@ export class RoleResolverService { ): ResolvedInstanceRole { for (const rule of rules) { if (!rule.enabled) continue; - if (this.evaluateExpression(rule.expression, context)) { + if (this.evaluateExpression(rule.expression, context, rule.id)) { return { role: rule.role, matchedRuleId: rule.id, @@ -107,7 +107,7 @@ export class RoleResolverService { } const enrichedContext = withProjectContext(context, project); - if (this.evaluateExpression(rule.expression, enrichedContext)) { + if (this.evaluateExpression(rule.expression, enrichedContext, rule.id)) { matched.set(rule.projectId, { projectId: rule.projectId, role: rule.role, @@ -120,15 +120,26 @@ export class RoleResolverService { return matched; } - private evaluateExpression(expression: string, context: RoleResolverContext): boolean { + private evaluateExpression( + expression: string, + context: RoleResolverContext, + ruleId: string, + ): boolean { try { const result = Expression.resolveWithoutWorkflow( expression, context as unknown as IDataObject, ); - return String(result) === 'true'; + const matched = String(result) === 'true'; + this.logger.debug('Role mapping rule evaluated', { + ruleId, + matched, + resultType: typeof result, + }); + return matched; } catch (error) { this.logger.warn('Role resolver expression evaluation failed, treating as false', { + ruleId, expression, error: error instanceof Error ? error.message : String(error), }); diff --git a/packages/cli/src/modules/sso-oidc/__tests__/oidc-test-result.test.ts b/packages/cli/src/modules/sso-oidc/__tests__/oidc-test-result.test.ts new file mode 100644 index 00000000000..cef2eb41cea --- /dev/null +++ b/packages/cli/src/modules/sso-oidc/__tests__/oidc-test-result.test.ts @@ -0,0 +1,40 @@ +import { renderOidcTestSuccess } from '../views/oidc-test-result'; + +describe('renderOidcTestSuccess', () => { + const claims = { sub: 'user-1', acr: 'urn:loa:1' }; + const userInfo = { + email: 'jane.doe@example.com', + given_name: 'Jane', + family_name: 'Doe', + sub: 'user-1', + groups: ['admins', 'engineers'], + }; + + it('renders the email, names and subject from userInfo', () => { + const html = renderOidcTestSuccess({ claims, userInfo }); + + expect(html).toContain('jane.doe@example.com'); + expect(html).toContain('Jane'); + expect(html).toContain('Doe'); + expect(html).toContain('user-1'); + }); + + it('embeds the full claims and userInfo as JSON for debugging', () => { + const html = renderOidcTestSuccess({ claims, userInfo }); + + // JSON is HTML-escaped, so quotes become " inside
+		expect(html).toContain('"groups"');
+		expect(html).toContain('"admins"');
+		expect(html).toContain('"acr"');
+	});
+
+	it('escapes HTML metacharacters in claim values', () => {
+		const html = renderOidcTestSuccess({
+			claims: { sub: 'user-1' },
+			userInfo: { email: '@example.com', sub: 'user-1' },
+		});
+
+		expect(html).not.toContain('');
+		expect(html).toContain('<script>alert(1)</script>');
+	});
+});
diff --git a/packages/cli/src/modules/sso-oidc/oidc.service.ee.ts b/packages/cli/src/modules/sso-oidc/oidc.service.ee.ts
index c1eb7566bfb..a032506b6e8 100644
--- a/packages/cli/src/modules/sso-oidc/oidc.service.ee.ts
+++ b/packages/cli/src/modules/sso-oidc/oidc.service.ee.ts
@@ -240,7 +240,18 @@ export class OidcService {
 				expectedNonce,
 			});
 		} catch (error) {
-			this.logger.error('Failed to exchange authorization code for tokens', { error });
+			const e = error as {
+				error?: string;
+				error_description?: string;
+				cause?: unknown;
+				message?: string;
+			};
+			this.logger.error('Failed to exchange authorization code for tokens', {
+				oauthError: e.error,
+				oauthErrorDescription: e.error_description,
+				cause: e.cause ? JSON.stringify(e.cause) : undefined,
+				message: e.message,
+			});
 			throw new BadRequestError('Invalid authorization code');
 		}
 
@@ -415,7 +426,18 @@ export class OidcService {
 				expectedNonce,
 			});
 		} catch (error) {
-			this.logger.error('Failed to exchange authorization code for tokens', { error });
+			const e = error as {
+				error?: string;
+				error_description?: string;
+				cause?: unknown;
+				message?: string;
+			};
+			this.logger.error('Failed to exchange authorization code for tokens', {
+				oauthError: e.error,
+				oauthErrorDescription: e.error_description,
+				cause: e.cause ? JSON.stringify(e.cause) : undefined,
+				message: e.message,
+			});
 			throw new BadRequestError('Invalid authorization code');
 		}
 
diff --git a/packages/cli/src/modules/sso-oidc/views/oidc-test-result.ts b/packages/cli/src/modules/sso-oidc/views/oidc-test-result.ts
index 1166f7896da..ef99757cbc3 100644
--- a/packages/cli/src/modules/sso-oidc/views/oidc-test-result.ts
+++ b/packages/cli/src/modules/sso-oidc/views/oidc-test-result.ts
@@ -1,10 +1,16 @@
 const PAGE_STYLES = `
-body { background: rgb(251,252,254); font-family: 'Open Sans', sans-serif; padding: 10px; margin: auto; width: 500px; top: 40%; position: relative; }
+body { background: rgb(251,252,254); font-family: 'Open Sans', sans-serif; padding: 10px; margin: auto; max-width: 700px; }
 h1 { font-size: 16px; font-weight: 400; margin: 0 0 10px 0; }
 h2 { color: rgb(0, 0, 0); font-size: 12px; font-weight: 400; margin: 0 0 10px 0; }
-button { border: 1px solid rgb(219, 223, 231); background: rgb(255, 255, 255); border-radius: 4px; padding: 10px; cursor: pointer; }
+button { border: 1px solid rgb(219, 223, 231); background: rgb(255, 255, 255); border-radius: 4px; padding: 10px; cursor: pointer; font: inherit; }
 ul { border: 1px solid rgb(219, 223, 231); border-radius: 4px; padding: 10px; }
 li { list-style: none; margin: 0; color: rgb(125, 125, 125); font-size: 12px; }
+details { margin-top: 24px; text-align: left; border: 1px solid rgb(219, 223, 231); border-radius: 4px; padding: 8px 12px; }
+summary { cursor: pointer; font-size: 12px; color: rgb(70, 70, 70); padding: 4px 0; }
+.claims-section { margin-top: 12px; }
+.claims-section h3 { font-size: 12px; font-weight: 600; margin: 0 0 6px 0; color: rgb(0, 0, 0); }
+.claims-section pre { background: white; border: 1px solid rgb(219, 223, 231); border-radius: 4px; padding: 10px; margin: 0; max-height: 320px; overflow: auto; font-family: ui-monospace, SFMono-Regular, Menlo, monospace; font-size: 11px; white-space: pre; }
+.claims-warning { color: rgb(125, 125, 125); font-size: 11px; margin: 0 0 12px 0; }
 `;
 
 function escapeHtml(value: unknown): string {
@@ -15,7 +21,17 @@ function escapeHtml(value: unknown): string {
 		.replace(/"/g, '"');
 }
 
+function renderClaimsSection(label: string, payload: Record): string {
+	const json = JSON.stringify(payload, null, 2);
+	return `
+	
+

${escapeHtml(label)}

+
${escapeHtml(json)}
+
`; +} + export function renderOidcTestSuccess({ + claims, userInfo, }: { claims: Record; @@ -41,6 +57,12 @@ export function renderOidcTestSuccess({
  • Last Name: ${lastName}
  • Subject: ${sub}
  • +
    + Show full claims (for debugging role mapping) +

    Contains identity data from your IdP. Avoid sharing publicly.

    + ${renderClaimsSection('ID token claims', claims)} + ${renderClaimsSection('Userinfo endpoint response', userInfo)} +
    `; diff --git a/packages/cli/src/modules/sso-saml/__tests__/saml.controller.ee.test.ts b/packages/cli/src/modules/sso-saml/__tests__/saml.controller.ee.test.ts index 59616dd3c1b..2a65b7fb275 100644 --- a/packages/cli/src/modules/sso-saml/__tests__/saml.controller.ee.test.ts +++ b/packages/cli/src/modules/sso-saml/__tests__/saml.controller.ee.test.ts @@ -51,6 +51,13 @@ const attributes: SamlUserAttributes = { lastName: 'User', userPrincipalName: 'upn:test@example.com', }; +const rawAttributes = { + email: 'test@example.com', + givenName: 'Test', + surname: 'User', + groups: ['admins', 'engineers'], +}; +const rawAttributesJson = JSON.stringify(rawAttributes, null, 2); describe('Test views', () => { const RelayState = getServiceProviderConfigTestReturnUrl(); @@ -72,12 +79,16 @@ describe('Test views', () => { samlService.handleSamlLogin.mockResolvedValueOnce({ authenticatedUser: user, attributes, + rawAttributes, onboardingRequired: false, }); await controller.acsPost(req, res, { RelayState }); - expect(res.render).toBeCalledWith('saml-connection-test-success', attributes); + expect(res.render).toBeCalledWith('saml-connection-test-success', { + ...attributes, + rawAttributesJson, + }); }); test('Should render failure with template', async () => { @@ -90,12 +101,17 @@ describe('Test views', () => { samlService.handleSamlLogin.mockResolvedValueOnce({ authenticatedUser: undefined, attributes, + rawAttributes, onboardingRequired: false, }); await controller.acsPost(req, res, { RelayState }); - expect(res.render).toBeCalledWith('saml-connection-test-failed', { message: '', attributes }); + expect(res.render).toBeCalledWith('saml-connection-test-failed', { + message: '', + attributes, + rawAttributesJson, + }); }); test('Should render error with template', async () => { @@ -127,6 +143,7 @@ describe('Test views', () => { samlService.handleSamlLogin.mockResolvedValueOnce({ authenticatedUser: user, attributes, + rawAttributes, onboardingRequired: false, }); @@ -134,7 +151,10 @@ describe('Test views', () => { expect(samlService.consumePendingTestConfig).toHaveBeenCalledWith(testId); expect(samlService.handleSamlLogin).toHaveBeenCalledWith(req, 'post', metadata); - expect(res.render).toBeCalledWith('saml-connection-test-success', attributes); + expect(res.render).toBeCalledWith('saml-connection-test-success', { + ...attributes, + rawAttributesJson, + }); }); test('Should still call handleSamlLogin without override when no test token in RelayState', async () => { @@ -147,6 +167,7 @@ describe('Test views', () => { samlService.handleSamlLogin.mockResolvedValueOnce({ authenticatedUser: user, attributes, + rawAttributes, onboardingRequired: false, }); @@ -229,6 +250,7 @@ describe('SAML Login Flow', () => { samlService.handleSamlLogin.mockResolvedValueOnce({ authenticatedUser: user, attributes, + rawAttributes, onboardingRequired: false, }); @@ -250,6 +272,7 @@ describe('SAML Login Flow', () => { samlService.handleSamlLogin.mockResolvedValueOnce({ authenticatedUser: user, attributes, + rawAttributes, onboardingRequired: true, }); @@ -268,6 +291,7 @@ describe('SAML Login Flow', () => { samlService.handleSamlLogin.mockResolvedValueOnce({ authenticatedUser: user, attributes, + rawAttributes, onboardingRequired: false, }); @@ -285,6 +309,7 @@ describe('SAML Login Flow', () => { samlService.handleSamlLogin.mockResolvedValueOnce({ authenticatedUser: user, attributes, + rawAttributes, onboardingRequired: false, }); await controller.acsPost(req, res, { RelayState: '/workflow/123' }); @@ -342,6 +367,7 @@ describe('SAML Login Flow', () => { samlService.handleSamlLogin.mockResolvedValueOnce({ authenticatedUser: user, attributes, + rawAttributes, onboardingRequired: false, }); await controller.acsPost(req, res, { RelayState: blockedUrl }); diff --git a/packages/cli/src/modules/sso-saml/__tests__/saml.service.ee.test.ts b/packages/cli/src/modules/sso-saml/__tests__/saml.service.ee.test.ts index 46f149284ca..075d787e868 100644 --- a/packages/cli/src/modules/sso-saml/__tests__/saml.service.ee.test.ts +++ b/packages/cli/src/modules/sso-saml/__tests__/saml.service.ee.test.ts @@ -459,6 +459,7 @@ describe('SamlService', () => { expect(loginResult).toEqual({ authenticatedUser: mockUser, attributes: samlAttributes, + rawAttributes: {}, onboardingRequired: false, }); }); @@ -487,6 +488,7 @@ describe('SamlService', () => { expect(loginResult).toEqual({ authenticatedUser: mockUser, attributes: samlAttributes, + rawAttributes: {}, onboardingRequired: true, }); }); @@ -511,6 +513,7 @@ describe('SamlService', () => { expect(loginResult).toEqual({ authenticatedUser: undefined, attributes: samlAttributes, + rawAttributes: {}, onboardingRequired: false, }); }); @@ -539,6 +542,7 @@ describe('SamlService', () => { expect(loginResult).toEqual({ authenticatedUser: mockUser, attributes: samlAttributes, + rawAttributes: {}, onboardingRequired: true, }); }); @@ -567,6 +571,7 @@ describe('SamlService', () => { expect(loginResult).toEqual({ authenticatedUser: mockUser, attributes: samlAttributes, + rawAttributes: {}, onboardingRequired: false, }); }); diff --git a/packages/cli/src/modules/sso-saml/saml.controller.ee.ts b/packages/cli/src/modules/sso-saml/saml.controller.ee.ts index 1695553a4a0..f7193efedf8 100644 --- a/packages/cli/src/modules/sso-saml/saml.controller.ee.ts +++ b/packages/cli/src/modules/sso-saml/saml.controller.ee.ts @@ -139,12 +139,17 @@ export class SamlController { const loginResult = await this.samlService.handleSamlLogin(req, binding, metadataOverride); // if RelayState is set to the test connection Url, this is a test connection if (isConnectionTestRequest(payload)) { + const rawAttributesJson = JSON.stringify(loginResult.rawAttributes ?? {}, null, 2); if (loginResult.authenticatedUser) { - return res.render('saml-connection-test-success', loginResult.attributes); + return res.render('saml-connection-test-success', { + ...loginResult.attributes, + rawAttributesJson, + }); } else { return res.render('saml-connection-test-failed', { message: '', attributes: loginResult.attributes, + rawAttributesJson, }); } } diff --git a/packages/cli/src/modules/sso-saml/saml.service.ee.ts b/packages/cli/src/modules/sso-saml/saml.service.ee.ts index 7715f1d5e6c..fb3d331ce12 100644 --- a/packages/cli/src/modules/sso-saml/saml.service.ee.ts +++ b/packages/cli/src/modules/sso-saml/saml.service.ee.ts @@ -362,6 +362,7 @@ export class SamlService { ): Promise<{ authenticatedUser: User | undefined; attributes: SamlUserAttributes; + rawAttributes: Record; onboardingRequired: boolean; }> { const { mapped: attributes, raw: rawAttributes } = await this.getAttributesFromLoginResponse( @@ -392,6 +393,7 @@ export class SamlService { return { authenticatedUser: user, attributes, + rawAttributes, onboardingRequired: false, }; } else { @@ -402,6 +404,7 @@ export class SamlService { return { authenticatedUser: updatedUser, attributes, + rawAttributes, onboardingRequired, }; } @@ -413,6 +416,7 @@ export class SamlService { return { authenticatedUser: newUser, attributes, + rawAttributes, onboardingRequired: !newUser.firstName || !newUser.lastName, }; } @@ -422,6 +426,7 @@ export class SamlService { return { authenticatedUser: undefined, attributes, + rawAttributes, onboardingRequired: false, }; } diff --git a/packages/cli/templates/saml-connection-test-failed.handlebars b/packages/cli/templates/saml-connection-test-failed.handlebars index f93cea5c2d6..53bcdf69d02 100644 --- a/packages/cli/templates/saml-connection-test-failed.handlebars +++ b/packages/cli/templates/saml-connection-test-failed.handlebars @@ -2,12 +2,16 @@ n8n - SAML Connection Test Result @@ -23,8 +27,15 @@
  • 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}} + {{/with}} + {{#if rawAttributesJson}} +
    + Show full raw SAML attributes (for debugging role mapping) +

    Contains identity data from your IdP. Avoid sharing publicly.

    +
    {{rawAttributesJson}}
    +
    + {{/if}} diff --git a/packages/cli/templates/saml-connection-test-success.handlebars b/packages/cli/templates/saml-connection-test-success.handlebars index e65d29483d8..77da46ed5a0 100644 --- a/packages/cli/templates/saml-connection-test-success.handlebars +++ b/packages/cli/templates/saml-connection-test-success.handlebars @@ -2,12 +2,16 @@ n8n - SAML Connection Test Result @@ -22,6 +26,13 @@
  • Last Name: {{#if lastName}}{{lastName}}{{else}}(n/a){{/if}}
  • UPN: {{#if userPrincipalName}}{{userPrincipalName}}{{else}}(n/a){{/if}}
  • + {{#if rawAttributesJson}} +
    + Show full raw SAML attributes (for debugging role mapping) +

    Contains identity data from your IdP. Avoid sharing publicly.

    +
    {{rawAttributesJson}}
    +
    + {{/if}}