From 3efca1169fb10403702c3ccccf8c3551ba90f686 Mon Sep 17 00:00:00 2001 From: Darcy Ye Date: Thu, 9 Jan 2025 17:55:29 +0800 Subject: [PATCH] refactor(core): build SAML app sign in URL scope with attribute mapping (#6930) refactor(core): build sign in URL scope with attribute mapping --- .../SamlApplication/index.test.ts | 102 ++++++++++++++++++ .../SamlApplication/index.ts | 58 +++++++--- 2 files changed, 146 insertions(+), 14 deletions(-) diff --git a/packages/core/src/saml-applications/SamlApplication/index.test.ts b/packages/core/src/saml-applications/SamlApplication/index.test.ts index cdfa18b1e..d3fb0af6c 100644 --- a/packages/core/src/saml-applications/SamlApplication/index.test.ts +++ b/packages/core/src/saml-applications/SamlApplication/index.test.ts @@ -1,3 +1,4 @@ +import { UserScope } from '@logto/core-kit'; import { NameIdFormat } from '@logto/schemas'; import nock from 'nock'; @@ -11,6 +12,7 @@ class TestSamlApplication extends SamlApplication { public exposedExchangeAuthorizationCode = this.exchangeAuthorizationCode; public exposedGetUserInfo = this.getUserInfo; public exposedFetchOidcConfig = this.fetchOidcConfig; + public exposedGetScopesFromAttributeMapping = this.getScopesFromAttributeMapping; } describe('SamlApplication', () => { @@ -169,4 +171,104 @@ describe('SamlApplication', () => { await expect(samlApp.exposedGetUserInfo({ accessToken: mockAccessToken })).rejects.toThrow(); }); }); + + describe('getScopesFromAttributeMapping', () => { + it('should include email scope when nameIdFormat is EmailAddress', () => { + const app = new TestSamlApplication( + // @ts-expect-error + { + ...mockDetails, + nameIdFormat: NameIdFormat.EmailAddress, + attributeMapping: {}, + }, + mockSamlApplicationId, + mockIssuer, + mockTenantId + ); + + const scopes = app.exposedGetScopesFromAttributeMapping(); + expect(scopes).toContain(UserScope.Email); + }); + + it('should return only nameIdFormat related scope when attributeMapping is empty', () => { + const app = new TestSamlApplication( + // @ts-expect-error + { + ...mockDetails, + nameIdFormat: NameIdFormat.EmailAddress, + attributeMapping: {}, + }, + mockSamlApplicationId, + mockIssuer, + mockTenantId + ); + + const scopes = app.exposedGetScopesFromAttributeMapping(); + expect(scopes).toHaveLength(1); + expect(scopes).toEqual([UserScope.Email]); + }); + + it('should return correct scopes based on attributeMapping', () => { + const app = new TestSamlApplication( + // @ts-expect-error + { + ...mockDetails, + attributeMapping: { + name: 'name', + email: 'email', + custom_data: 'customData', + }, + }, + mockSamlApplicationId, + mockIssuer, + mockTenantId + ); + + const scopes = app.exposedGetScopesFromAttributeMapping(); + expect(scopes).toContain(UserScope.Profile); // For 'name' + expect(scopes).toContain(UserScope.Email); // For 'email' + expect(scopes).toContain(UserScope.CustomData); // For 'custom_data' + }); + + it('should ignore id claim in attributeMapping', () => { + const app = new TestSamlApplication( + // @ts-expect-error + { + ...mockDetails, + attributeMapping: { + id: 'userId', + name: 'name', + }, + }, + mockSamlApplicationId, + mockIssuer, + mockTenantId + ); + + const scopes = app.exposedGetScopesFromAttributeMapping(); + expect(scopes).toHaveLength(1); + expect(scopes).toContain(UserScope.Profile); // Only for 'name' + }); + + it('should deduplicate scopes when multiple claims map to the same scope', () => { + const app = new TestSamlApplication( + // @ts-expect-error + { + ...mockDetails, + attributeMapping: { + name: 'name', + given_name: 'givenName', + family_name: 'familyName', + }, + }, + mockSamlApplicationId, + mockIssuer, + mockTenantId + ); + + const scopes = app.exposedGetScopesFromAttributeMapping(); + expect(scopes).toHaveLength(1); + expect(scopes).toContain(UserScope.Profile); // All claims map to 'profile' scope + }); + }); }); diff --git a/packages/core/src/saml-applications/SamlApplication/index.ts b/packages/core/src/saml-applications/SamlApplication/index.ts index 4caaa333b..f623ee61f 100644 --- a/packages/core/src/saml-applications/SamlApplication/index.ts +++ b/packages/core/src/saml-applications/SamlApplication/index.ts @@ -1,12 +1,14 @@ /* eslint-disable max-lines */ // TODO: refactor this file to reduce LOC import { parseJson } from '@logto/connector-kit'; -import { Prompt, QueryKey, ReservedScope, UserScope } from '@logto/js'; +import { userClaims, type UserClaim, UserScope } from '@logto/core-kit'; +import { Prompt, QueryKey, ReservedScope } from '@logto/js'; import { type SamlAcsUrl, BindingType, - type NameIdFormat, + NameIdFormat, type SamlEncryption, + type SamlAttributeMapping, } from '@logto/schemas'; import { generateStandardId } from '@logto/shared'; import { tryThat, appendPath, deduplicate, type Nullable, cond } from '@silverhand/essentials'; @@ -46,6 +48,7 @@ type ValidSamlApplicationDetails = { redirectUri: string; privateKey: string; certificate: string; + attributeMapping: SamlAttributeMapping; nameIdFormat: NameIdFormat; encryption: Nullable; }; @@ -92,6 +95,7 @@ const validateSamlApplicationDetails = ( secret, nameIdFormat, encryption, + attributeMapping, } = details; assertThat(acsUrl, 'application.saml.acs_url_required'); @@ -110,6 +114,7 @@ const validateSamlApplicationDetails = ( certificate, nameIdFormat, encryption, + attributeMapping, }; }; @@ -277,7 +282,7 @@ export class SamlApplication { return this.getUserInfo({ accessToken }); }; - public getSignInUrl = async ({ scope, state }: { scope?: string; state?: string }) => { + public getSignInUrl = async ({ state }: { state?: string }) => { const { authorizationEndpoint } = await this.fetchOidcConfig(); const queryParameters = new URLSearchParams({ @@ -287,20 +292,10 @@ export class SamlApplication { [QueryKey.Prompt]: Prompt.Login, }); - // TODO: get value of `scope` parameters according to setup in attribute mapping. queryParameters.append( QueryKey.Scope, // For security reasons, DO NOT include the offline_access scope by default. - deduplicate([ - ReservedScope.OpenId, - UserScope.Profile, - UserScope.Roles, - UserScope.Organizations, - UserScope.OrganizationRoles, - UserScope.CustomData, - UserScope.Identities, - ...(scope?.split(' ') ?? []), - ]).join(' ') + deduplicate([ReservedScope.OpenId, ...this.getScopesFromAttributeMapping()]).join(' ') ); if (state) { @@ -376,6 +371,41 @@ export class SamlApplication { return result.data; }; + // Get required scopes based on attribute mapping configuration + protected getScopesFromAttributeMapping = (): UserScope[] => { + const requiredScopes = new Set(); + if (this.details.nameIdFormat === NameIdFormat.EmailAddress) { + requiredScopes.add(UserScope.Email); + } + + // If no attribute mapping, return empty array + if (Object.keys(this.details.attributeMapping).length === 0) { + return Array.from(requiredScopes); + } + + // Iterate through all claims in attribute mapping + // eslint-disable-next-line no-restricted-syntax + for (const claim of Object.keys(this.details.attributeMapping) as Array< + keyof SamlAttributeMapping + >) { + // Ignore `id` claim since this will always be included. + if (claim === 'id') { + continue; + } + + // Find which scope includes this claim + // eslint-disable-next-line no-restricted-syntax + for (const [scope, claims] of Object.entries(userClaims) as Array<[UserScope, UserClaim[]]>) { + if (claims.includes(claim)) { + requiredScopes.add(scope); + break; + } + } + } + + return Array.from(requiredScopes); + }; + protected createSamlTemplateCallback = (user: IdTokenProfileStandardClaims) => (template: string) => { const assertionConsumerServiceUrl = this.sp.entityMeta.getAssertionConsumerService(