refactor(core): refactor GET saml app metadata endpoint (#6938)

* refactor(core): refactor GET saml app metadata endpoint

* refactor: refactor SamlApplication class to validate only necessary fields

packages/core/src/saml-applications/SamlApplication/index.ts

@@ -7,7 +7,6 @@ import {
   type SamlAcsUrl,
   BindingType,
   NameIdFormat,
-  type SamlEncryption,
   type SamlAttributeMapping,
 } from '@logto/schemas';
 import { generateStandardId } from '@logto/shared';
@@ -41,18 +40,6 @@ import { type SamlApplicationDetails } from '../queries/index.js';
 
 import { buildSamlAssertionNameId, getSamlAppCallbackUrl } from './utils.js';
 
-type ValidSamlApplicationDetails = {
-  secret: string;
-  entityId: string;
-  acsUrl: SamlAcsUrl;
-  redirectUri: string;
-  privateKey: string;
-  certificate: string;
-  attributeMapping: SamlAttributeMapping;
-  nameIdFormat: NameIdFormat;
-  encryption: Nullable<SamlEncryption>;
-};
-
 type SamlIdentityProviderConfig = {
   entityId: string;
   certificate: string;
@@ -83,41 +70,6 @@ saml.setSchemaValidator({
   },
 });
 
-const validateSamlApplicationDetails = (
-  details: SamlApplicationDetails
-): ValidSamlApplicationDetails => {
-  const {
-    entityId,
-    acsUrl,
-    oidcClientMetadata: { redirectUris },
-    privateKey,
-    certificate,
-    secret,
-    nameIdFormat,
-    encryption,
-    attributeMapping,
-  } = details;
-
-  assertThat(acsUrl, 'application.saml.acs_url_required');
-  assertThat(entityId, 'application.saml.entity_id_required');
-  assertThat(redirectUris[0], 'oidc.invalid_redirect_uri');
-
-  assertThat(privateKey, 'application.saml.private_key_required');
-  assertThat(certificate, 'application.saml.certificate_required');
-
-  return {
-    secret,
-    entityId,
-    acsUrl,
-    redirectUri: redirectUris[0],
-    privateKey,
-    certificate,
-    nameIdFormat,
-    encryption,
-    attributeMapping,
-  };
-};
-
 const buildLoginResponseTemplate = () => {
   return {
     context: samlLogInResponseTemplate,
@@ -193,8 +145,53 @@ const buildSamlServiceProvider = ({
   });
 };
 
+class SamlApplicationConfig {
+  constructor(private readonly _details: SamlApplicationDetails) {}
+
+  public get secret() {
+    return this._details.secret;
+  }
+
+  public get entityId() {
+    assertThat(this._details.entityId, 'application.saml.entity_id_required');
+    return this._details.entityId;
+  }
+
+  public get acsUrl() {
+    assertThat(this._details.acsUrl, 'application.saml.acs_url_required');
+    return this._details.acsUrl;
+  }
+
+  public get redirectUri() {
+    assertThat(this._details.oidcClientMetadata.redirectUris[0], 'oidc.invalid_redirect_uri');
+    return this._details.oidcClientMetadata.redirectUris[0];
+  }
+
+  public get privateKey() {
+    assertThat(this._details.privateKey, 'application.saml.private_key_required');
+    return this._details.privateKey;
+  }
+
+  public get certificate() {
+    assertThat(this._details.certificate, 'application.saml.certificate_required');
+    return this._details.certificate;
+  }
+
+  public get nameIdFormat() {
+    return this._details.nameIdFormat;
+  }
+
+  public get encryption() {
+    return this._details.encryption;
+  }
+
+  public get attributeMapping() {
+    return this._details.attributeMapping;
+  }
+}
+
 export class SamlApplication {
-  public details: ValidSamlApplicationDetails;
+  public config: SamlApplicationConfig;
 
   protected tenantEndpoint: URL;
   protected oidcConfig?: CamelCaseKeys<OidcConfigResponse>;
@@ -208,7 +205,7 @@ export class SamlApplication {
     protected issuer: string,
     tenantId: string
   ) {
-    this.details = validateSamlApplicationDetails(details);
+    this.config = new SamlApplicationConfig(details);
     this.tenantEndpoint = getTenantEndpoint(tenantId, EnvSet.values);
   }
 
@@ -221,7 +218,7 @@ export class SamlApplication {
     const { certificate: encryptCert, ...rest } = this.buildSpConfig();
     this._sp ||= buildSamlServiceProvider({
       ...rest,
-      certificate: this.details.certificate,
+      certificate: this.config.certificate,
       isWantAuthnRequestsSigned: this.idp.entityMeta.isWantAuthnRequestsSigned(),
       ...cond(encryptCert && { encryptCert }),
     });
@@ -233,7 +230,7 @@ export class SamlApplication {
   }
 
   public get idPCertificate() {
-    return this.details.certificate;
+    return this.config.certificate;
   }
 
   public get samlAppCallbackUrl() {
@@ -265,7 +262,7 @@ export class SamlApplication {
       'post',
       userInfo,
       this.createSamlTemplateCallback({ userInfo, samlRequestId }),
-      this.details.encryption?.encryptThenSign,
+      this.config.encryption?.encryptThenSign,
       relayState ?? undefined
     );
 
@@ -291,7 +288,7 @@ export class SamlApplication {
 
     const queryParameters = new URLSearchParams({
       [QueryKey.ClientId]: this.samlApplicationId,
-      [QueryKey.RedirectUri]: this.details.redirectUri,
+      [QueryKey.RedirectUri]: this.config.redirectUri,
       [QueryKey.ResponseType]: 'code',
       [QueryKey.Prompt]: Prompt.Login,
     });
@@ -333,8 +330,8 @@ export class SamlApplication {
     const result = await handleTokenExchange(tokenEndpoint, {
       code,
       clientId: this.samlApplicationId,
-      clientSecret: this.details.secret,
+      clientSecret: this.config.secret,
-      redirectUri: this.details.redirectUri,
+      redirectUri: this.config.redirectUri,
     });
 
     if (!result.success) {
@@ -383,18 +380,18 @@ export class SamlApplication {
     requiredScopes.add(ReservedScope.OpenId);
     requiredScopes.add(UserScope.Profile);
 
-    if (this.details.nameIdFormat === NameIdFormat.EmailAddress) {
+    if (this.config.nameIdFormat === NameIdFormat.EmailAddress) {
       requiredScopes.add(UserScope.Email);
     }
 
     // If no attribute mapping, return empty array
-    if (Object.keys(this.details.attributeMapping).length === 0) {
+    if (Object.keys(this.config.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<
+    for (const claim of Object.keys(this.config.attributeMapping) as Array<
       keyof SamlAttributeMapping
     >) {
       // Ignore `id` claim since this will always be included.
@@ -479,19 +476,19 @@ export class SamlApplication {
   private buildIdpConfig(): SamlIdentityProviderConfig {
     return {
       entityId: buildSamlIdentityProviderEntityId(this.tenantEndpoint, this.samlApplicationId),
-      privateKey: this.details.privateKey,
+      privateKey: this.config.privateKey,
-      certificate: this.details.certificate,
+      certificate: this.config.certificate,
       singleSignOnUrl: buildSingleSignOnUrl(this.tenantEndpoint, this.samlApplicationId),
-      nameIdFormat: this.details.nameIdFormat,
+      nameIdFormat: this.config.nameIdFormat,
-      encryptSamlAssertion: this.details.encryption?.encryptAssertion ?? false,
+      encryptSamlAssertion: this.config.encryption?.encryptAssertion ?? false,
     };
   }
 
   private buildSpConfig(): SamlServiceProviderConfig {
     return {
-      entityId: this.details.entityId,
+      entityId: this.config.entityId,
-      acsUrl: this.details.acsUrl,
+      acsUrl: this.config.acsUrl,
-      certificate: this.details.encryption?.certificate,
+      certificate: this.config.encryption?.certificate,
     };
   }
 }

packages/core/src/saml-applications/libraries/saml-applications.ts

@@ -3,23 +3,15 @@ import {
   type SamlApplicationResponse,
   type PatchSamlApplication,
   type SamlApplicationSecret,
-  BindingType,
 } from '@logto/schemas';
 import { generateStandardId } from '@logto/shared';
 import { removeUndefinedKeys, pick } from '@silverhand/essentials';
-import saml from 'samlify';
 
-import { EnvSet, getTenantEndpoint } from '#src/env-set/index.js';
 import RequestError from '#src/errors/RequestError/index.js';
 import type Queries from '#src/tenants/Queries.js';
 import assertThat from '#src/utils/assert-that.js';
 
-import {
+import { ensembleSamlApplication, generateKeyPairAndCertificate } from './utils.js';
-  ensembleSamlApplication,
-  generateKeyPairAndCertificate,
-  buildSingleSignOnUrl,
-  buildSamlIdentityProviderEntityId,
-} from './utils.js';
 
 export const createSamlApplicationsLibrary = (queries: Queries) => {
   const {
@@ -27,7 +19,6 @@ export const createSamlApplicationsLibrary = (queries: Queries) => {
     samlApplicationSecrets: {
       insertInactiveSamlApplicationSecret,
       insertActiveSamlApplicationSecret,
-      findActiveSamlApplicationSecretByApplicationId,
     },
     samlApplicationConfigs: {
       findSamlApplicationConfigByApplicationId,
@@ -117,39 +108,9 @@ export const createSamlApplicationsLibrary = (queries: Queries) => {
     });
   };
 
-  const getSamlIdPMetadataByApplicationId = async (id: string): Promise<{ metadata: string }> => {
-    const [{ tenantId }, { certificate }] = await Promise.all([
-      findSamlApplicationConfigByApplicationId(id),
-      findActiveSamlApplicationSecretByApplicationId(id),
-    ]);
-
-    const tenantEndpoint = getTenantEndpoint(tenantId, EnvSet.values);
-
-    // eslint-disable-next-line new-cap
-    const idp = saml.IdentityProvider({
-      entityID: buildSamlIdentityProviderEntityId(tenantEndpoint, id),
-      signingCert: certificate,
-      singleSignOnService: [
-        {
-          Location: buildSingleSignOnUrl(tenantEndpoint, id),
-          Binding: BindingType.Redirect,
-        },
-        {
-          Location: buildSingleSignOnUrl(tenantEndpoint, id),
-          Binding: BindingType.Post,
-        },
-      ],
-    });
-
-    return {
-      metadata: idp.getMetadata(),
-    };
-  };
-
   return {
     createSamlApplicationSecret,
     findSamlApplicationById,
     updateSamlApplicationById,
-    getSamlIdPMetadataByApplicationId,
   };
 };

packages/core/src/saml-applications/routes/anonymous.ts

@@ -32,9 +32,6 @@ const samlApplicationSignInCallbackQueryParametersGuard = z.union([
 export default function samlApplicationAnonymousRoutes<T extends AnonymousRouter>(
   ...[router, { id: tenantId, libraries, queries, envSet }]: RouterInitArgs<T>
 ) {
-  const {
-    samlApplications: { getSamlIdPMetadataByApplicationId },
-  } = libraries;
   const {
     samlApplications: { getSamlApplicationDetailsById },
     samlApplicationSessions: {
@@ -49,16 +46,17 @@ export default function samlApplicationAnonymousRoutes<T extends AnonymousRouter
     '/saml-applications/:id/metadata',
     koaGuard({
       params: z.object({ id: z.string() }),
-      status: [200, 404],
+      status: [200, 400, 404],
       response: z.string(),
     }),
     async (ctx, next) => {
       const { id } = ctx.guard.params;
 
-      const { metadata } = await getSamlIdPMetadataByApplicationId(id);
+      const details = await getSamlApplicationDetailsById(id);
+      const samlApplication = new SamlApplication(details, id, envSet.oidc.issuer, tenantId);
 
       ctx.status = 200;
-      ctx.body = metadata;
+      ctx.body = samlApplication.idPMetadata;
       ctx.type = 'text/xml;charset=utf-8';
 
       return next();
@@ -99,7 +97,7 @@ export default function samlApplicationAnonymousRoutes<T extends AnonymousRouter
       const samlApplication = new SamlApplication(details, id, envSet.oidc.issuer, tenantId);
 
       assertThat(
-        samlApplication.details.redirectUri === samlApplication.samlAppCallbackUrl,
+        samlApplication.config.redirectUri === samlApplication.samlAppCallbackUrl,
         'oidc.invalid_redirect_uri'
       );
 
@@ -246,7 +244,7 @@ export default function samlApplicationAnonymousRoutes<T extends AnonymousRouter
         log.append({ extractResultData: extractResult.data });
 
         assertThat(
-          extractResult.data.issuer === samlApplication.details.entityId,
+          extractResult.data.issuer === samlApplication.config.entityId,
           'application.saml.auth_request_issuer_not_match'
         );
 
@@ -345,7 +343,7 @@ export default function samlApplicationAnonymousRoutes<T extends AnonymousRouter
         log.append({ extractResultData: extractResult.data });
 
         assertThat(
-          extractResult.data.issuer === samlApplication.details.entityId,
+          extractResult.data.issuer === samlApplication.config.entityId,
           'application.saml.auth_request_issuer_not_match'
         );