Skip to content

Commit

Permalink
refactor(core): build SAML app sign in URL scope with attribute mappi…
Browse files Browse the repository at this point in the history
…ng (#6930)

refactor(core): build sign in URL scope with attribute mapping
  • Loading branch information
darcyYe authored Jan 9, 2025
1 parent b335ad0 commit 3efca11
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 14 deletions.
102 changes: 102 additions & 0 deletions packages/core/src/saml-applications/SamlApplication/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { UserScope } from '@logto/core-kit';
import { NameIdFormat } from '@logto/schemas';
import nock from 'nock';

Expand All @@ -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', () => {
Expand Down Expand Up @@ -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
});
});
});
58 changes: 44 additions & 14 deletions packages/core/src/saml-applications/SamlApplication/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -46,6 +48,7 @@ type ValidSamlApplicationDetails = {
redirectUri: string;
privateKey: string;
certificate: string;
attributeMapping: SamlAttributeMapping;
nameIdFormat: NameIdFormat;
encryption: Nullable<SamlEncryption>;
};
Expand Down Expand Up @@ -92,6 +95,7 @@ const validateSamlApplicationDetails = (
secret,
nameIdFormat,
encryption,
attributeMapping,
} = details;

assertThat(acsUrl, 'application.saml.acs_url_required');
Expand All @@ -110,6 +114,7 @@ const validateSamlApplicationDetails = (
certificate,
nameIdFormat,
encryption,
attributeMapping,
};
};

Expand Down Expand Up @@ -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({
Expand All @@ -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) {
Expand Down Expand Up @@ -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<UserScope>();
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(
Expand Down

0 comments on commit 3efca11

Please sign in to comment.