diff --git a/packages/core/src/saml-applications/SamlApplication/index.ts b/packages/core/src/saml-applications/SamlApplication/index.ts index f623ee61f41..8f2f7150df7 100644 --- a/packages/core/src/saml-applications/SamlApplication/index.ts +++ b/packages/core/src/saml-applications/SamlApplication/index.ts @@ -1,8 +1,8 @@ /* eslint-disable max-lines */ // TODO: refactor this file to reduce LOC import { parseJson } from '@logto/connector-kit'; -import { userClaims, type UserClaim, UserScope } from '@logto/core-kit'; -import { Prompt, QueryKey, ReservedScope } from '@logto/js'; +import { userClaims, type UserClaim, UserScope, ReservedScope } from '@logto/core-kit'; +import { Prompt, QueryKey } from '@logto/js'; import { type SamlAcsUrl, BindingType, @@ -11,7 +11,7 @@ import { type SamlAttributeMapping, } from '@logto/schemas'; import { generateStandardId } from '@logto/shared'; -import { tryThat, appendPath, deduplicate, type Nullable, cond } from '@silverhand/essentials'; +import { tryThat, type Nullable, cond } from '@silverhand/essentials'; import camelcaseKeys, { type CamelCaseKeys } from 'camelcase-keys'; import { XMLValidator } from 'fast-xml-parser'; import saml from 'samlify'; @@ -39,7 +39,7 @@ import { import { buildSingleSignOnUrl, buildSamlIdentityProviderEntityId } from '../libraries/utils.js'; import { type SamlApplicationDetails } from '../queries/index.js'; -import { buildSamlAssertionNameId } from './utils.js'; +import { buildSamlAssertionNameId, getSamlAppCallbackUrl } from './utils.js'; type ValidSamlApplicationDetails = { secret: string; @@ -237,10 +237,7 @@ export class SamlApplication { } public get samlAppCallbackUrl() { - return appendPath( - this.tenantEndpoint, - `api/saml-applications/${this.samlApplicationId}/callback` - ).toString(); + return getSamlAppCallbackUrl(this.tenantEndpoint, this.samlApplicationId).toString(); } public async parseLoginRequest( @@ -295,7 +292,7 @@ export class SamlApplication { queryParameters.append( QueryKey.Scope, // For security reasons, DO NOT include the offline_access scope by default. - deduplicate([ReservedScope.OpenId, ...this.getScopesFromAttributeMapping()]).join(' ') + this.getScopesFromAttributeMapping().join(' ') ); if (state) { @@ -372,8 +369,13 @@ export class SamlApplication { }; // Get required scopes based on attribute mapping configuration - protected getScopesFromAttributeMapping = (): UserScope[] => { - const requiredScopes = new Set(); + protected getScopesFromAttributeMapping = (): Array => { + const requiredScopes = new Set(); + + // Add default scopes. + requiredScopes.add(ReservedScope.OpenId); + requiredScopes.add(UserScope.Profile); + if (this.details.nameIdFormat === NameIdFormat.EmailAddress) { requiredScopes.add(UserScope.Email); } diff --git a/packages/core/src/saml-applications/SamlApplication/utils.ts b/packages/core/src/saml-applications/SamlApplication/utils.ts index 02a2aafdc94..e2976f35fe8 100644 --- a/packages/core/src/saml-applications/SamlApplication/utils.ts +++ b/packages/core/src/saml-applications/SamlApplication/utils.ts @@ -1,5 +1,6 @@ import { NameIdFormat } from '@logto/schemas'; import { generateStandardId } from '@logto/shared'; +import { appendPath } from '@silverhand/essentials'; import RequestError from '#src/errors/RequestError/index.js'; import { type IdTokenProfileStandardClaims } from '#src/sso/types/oidc.js'; @@ -68,3 +69,6 @@ export const generateAutoSubmitForm = (actionUrl: string, samlResponse: string): `; }; + +export const getSamlAppCallbackUrl = (baseUrl: URL, samlAppId: string) => + appendPath(baseUrl, `api/saml-applications/${samlAppId}/callback`); diff --git a/packages/core/src/saml-applications/routes/index.ts b/packages/core/src/saml-applications/routes/index.ts index 446be17d901..f523583a333 100644 --- a/packages/core/src/saml-applications/routes/index.ts +++ b/packages/core/src/saml-applications/routes/index.ts @@ -10,6 +10,7 @@ import { generateStandardId } from '@logto/shared'; import { removeUndefinedKeys } from '@silverhand/essentials'; import { z } from 'zod'; +import { EnvSet } from '#src/env-set/index.js'; import RequestError from '#src/errors/RequestError/index.js'; import koaGuard from '#src/middleware/koa-guard.js'; import { buildOidcClientMetadata } from '#src/oidc/utils.js'; @@ -17,6 +18,8 @@ import { generateInternalSecret } from '#src/routes/applications/application-sec import type { ManagementApiRouter, RouterInitArgs } from '#src/routes/types.js'; import assertThat from '#src/utils/assert-that.js'; +import { getTenantEndpoint } from '../../env-set/utils.js'; +import { getSamlAppCallbackUrl } from '../SamlApplication/utils.js'; import { calculateCertificateFingerprints, ensembleSamlApplication, @@ -24,7 +27,7 @@ import { } from '../libraries/utils.js'; export default function samlApplicationRoutes( - ...[router, { queries, libraries }]: RouterInitArgs + ...[router, { id: tenantId, queries, libraries }]: RouterInitArgs ) { const { applications: { insertApplication, findApplicationById, deleteApplicationById }, @@ -58,14 +61,24 @@ export default function samlApplicationRoutes( validateAcsUrl(config.acsUrl); } + const id = generateStandardId(); + // Set the default redirect URI for SAML apps when creating a new SAML app. + const redirectUri = getSamlAppCallbackUrl( + getTenantEndpoint(tenantId, EnvSet.values), + id + ).toString(); + const application = await insertApplication( removeUndefinedKeys({ - id: generateStandardId(), + id, secret: generateInternalSecret(), name, description, customData, - oidcClientMetadata: buildOidcClientMetadata(), + oidcClientMetadata: { + ...buildOidcClientMetadata(), + redirectUris: [redirectUri], + }, type: ApplicationType.SAML, }) ); diff --git a/packages/integration-tests/src/tests/api/application/saml-application.test.ts b/packages/integration-tests/src/tests/api/application/saml-application.test.ts index 28a3848a6a7..14fde07f91a 100644 --- a/packages/integration-tests/src/tests/api/application/saml-application.test.ts +++ b/packages/integration-tests/src/tests/api/application/saml-application.test.ts @@ -1,7 +1,12 @@ import { ApplicationType, BindingType, NameIdFormat } from '@logto/schemas'; import { conditional } from '@silverhand/essentials'; -import { createApplication, deleteApplication, updateApplication } from '#src/api/application.js'; +import { + createApplication, + deleteApplication, + getApplications, + updateApplication, +} from '#src/api/application.js'; import { createSamlApplication, deleteSamlApplication, @@ -27,6 +32,20 @@ describe('SAML application', () => { expect(createdSamlApplication.nameIdFormat).toBe(NameIdFormat.Persistent); + // Check if the SAML application's OIDC metadata redirect URI is properly set. + // We need to do this since we do not return OIDC related info when using SAML app APIs. + const samlApplications = await getApplications([ApplicationType.SAML]); + const pickedSamlApplication = samlApplications.find( + ({ id }) => id === createdSamlApplication.id + ); + expect(pickedSamlApplication).toBeDefined(); + expect(pickedSamlApplication!.oidcClientMetadata.redirectUris.length).toBe(1); + expect( + pickedSamlApplication!.oidcClientMetadata.redirectUris[0]!.endsWith( + `api/saml-applications/${createdSamlApplication.id}/callback` + ) + ).toBe(true); + await deleteSamlApplication(createdSamlApplication.id); });