fix(core): Surface SSO claims and role mapping diagnostics (#30753)

This commit is contained in:
phyllis-noester 2026-05-20 18:44:14 +02:00 committed by GitHub
parent c1a4dbf094
commit 449abdd180
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 238 additions and 18 deletions

View File

@ -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<Role>({ 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<Project>({
id: 'old-proj-1',

View File

@ -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 }),
);
});

View File

@ -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;

View File

@ -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),
});

View File

@ -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 &quot; inside <pre>
expect(html).toContain('&quot;groups&quot;');
expect(html).toContain('&quot;admins&quot;');
expect(html).toContain('&quot;acr&quot;');
});
it('escapes HTML metacharacters in claim values', () => {
const html = renderOidcTestSuccess({
claims: { sub: 'user-1' },
userInfo: { email: '<script>alert(1)</script>@example.com', sub: 'user-1' },
});
expect(html).not.toContain('<script>alert(1)</script>');
expect(html).toContain('&lt;script&gt;alert(1)&lt;/script&gt;');
});
});

View File

@ -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');
}

View File

@ -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, '&quot;');
}
function renderClaimsSection(label: string, payload: Record<string, unknown>): string {
const json = JSON.stringify(payload, null, 2);
return `
<div class="claims-section">
<h3>${escapeHtml(label)}</h3>
<pre>${escapeHtml(json)}</pre>
</div>`;
}
export function renderOidcTestSuccess({
claims,
userInfo,
}: {
claims: Record<string, unknown>;
@ -41,6 +57,12 @@ export function renderOidcTestSuccess({
<li><strong>Last Name:</strong> ${lastName}</li>
<li><strong>Subject:</strong> ${sub}</li>
</ul>
<details>
<summary>Show full claims (for debugging role mapping)</summary>
<p class="claims-warning">Contains identity data from your IdP. Avoid sharing publicly.</p>
${renderClaimsSection('ID token claims', claims)}
${renderClaimsSection('Userinfo endpoint response', userInfo)}
</details>
</div>
</body>
</html>`;

View File

@ -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 });

View File

@ -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,
});
});

View File

@ -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,
});
}
}

View File

@ -362,6 +362,7 @@ export class SamlService {
): Promise<{
authenticatedUser: User | undefined;
attributes: SamlUserAttributes;
rawAttributes: Record<string, unknown>;
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,
};
}

View File

@ -2,12 +2,16 @@
<head>
<title>n8n - SAML Connection Test Result</title>
<style>
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 { color: rgb(240, 60, 60); 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; }
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 { decoration: none; list-style: none; margin: 0 0 0px 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-warning { color: rgb(125, 125, 125); font-size: 11px; margin: 6px 0 12px 0; }
pre.saml-raw-attributes { 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; }
</style>
</head>
<body>
@ -23,8 +27,15 @@
<li><strong>First Name:</strong> {{#if firstName}}{{firstName}}{{else}}(n/a){{/if}}</li>
<li><strong>Last Name:</strong> {{#if lastName}}{{lastName}}{{else}}(n/a){{/if}}</li>
<li><strong>UPN:</strong> {{#if userPrincipalName}}{{userPrincipalName}}{{else}}(n/a){{/if}}</li>
{{/with}}
</ul>
{{/with}}
{{#if rawAttributesJson}}
<details>
<summary>Show full raw SAML attributes (for debugging role mapping)</summary>
<p class="claims-warning">Contains identity data from your IdP. Avoid sharing publicly.</p>
<pre class="saml-raw-attributes">{{rawAttributesJson}}</pre>
</details>
{{/if}}
</div>
</body>
</html>

View File

@ -2,12 +2,16 @@
<head>
<title>n8n - SAML Connection Test Result</title>
<style>
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 { color: rgb(0, 0, 0); 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; }
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 { decoration: none; list-style: none; margin: 0 0 0px 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-warning { color: rgb(125, 125, 125); font-size: 11px; margin: 6px 0 12px 0; }
pre.saml-raw-attributes { 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; }
</style>
</head>
<body>
@ -22,6 +26,13 @@
<li><strong>Last Name:</strong> {{#if lastName}}{{lastName}}{{else}}(n/a){{/if}}</li>
<li><strong>UPN:</strong> {{#if userPrincipalName}}{{userPrincipalName}}{{else}}(n/a){{/if}}</li>
</ul>
{{#if rawAttributesJson}}
<details>
<summary>Show full raw SAML attributes (for debugging role mapping)</summary>
<p class="claims-warning">Contains identity data from your IdP. Avoid sharing publicly.</p>
<pre class="saml-raw-attributes">{{rawAttributesJson}}</pre>
</details>
{{/if}}
</div>
</body>
</html>