fix(core): fix SAML attributes data (#6953)

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

@@ -13,6 +13,8 @@ class TestSamlApplication extends SamlApplication {
   public exposedGetUserInfo = this.getUserInfo;
   public exposedFetchOidcConfig = this.fetchOidcConfig;
   public exposedGetScopesFromAttributeMapping = this.getScopesFromAttributeMapping;
+  public exposedBuildLoginResponseTemplate = this.buildLoginResponseTemplate;
+  public exposedBuildSamlAttributesTagValues = this.buildSamlAttributesTagValues;
 }
 
 describe('SamlApplication', () => {
@@ -29,12 +31,15 @@ describe('SamlApplication', () => {
     certificate: 'mock-certificate',
     secret: 'mock-secret',
     nameIdFormat: NameIdFormat.Persistent,
+    attributeMapping: {},
   };
 
   const mockUser = {
     sub: 'user123',
     email: 'user@example.com',
     name: 'Test User',
+    phone: '+1234567890',
+    phone_verified: true,
   };
 
   const mockTenantId = 'tenant-id';
@@ -287,4 +292,108 @@ describe('SamlApplication', () => {
       expect(scopes).toHaveLength(7);
     });
   });
+
+  describe('buildLoginResponseTemplate', () => {
+    it('should generate correct SAML response template with attribute mapping', () => {
+      const mockDetailsWithMapping = {
+        ...mockDetails,
+        attributeMapping: {
+          sub: 'userId',
+          email: 'emailAddress',
+          name: 'displayName',
+        },
+      };
+
+      const samlApp = new TestSamlApplication(
+        // @ts-expect-error
+        mockDetailsWithMapping,
+        mockSamlApplicationId,
+        mockIssuer,
+        mockTenantId
+      );
+
+      const template = samlApp.exposedBuildLoginResponseTemplate();
+
+      expect(template.attributes).toEqual([
+        {
+          name: 'userId',
+          valueTag: 'attrUserId',
+          nameFormat: 'urn:oasis:names:tc:SAML:2.0:attrname-format:basic',
+          valueXsiType: 'xs:string',
+        },
+        {
+          name: 'emailAddress',
+          valueTag: 'attrEmailAddress',
+          nameFormat: 'urn:oasis:names:tc:SAML:2.0:attrname-format:basic',
+          valueXsiType: 'xs:string',
+        },
+        {
+          name: 'displayName',
+          valueTag: 'attrDisplayName',
+          nameFormat: 'urn:oasis:names:tc:SAML:2.0:attrname-format:basic',
+          valueXsiType: 'xs:string',
+        },
+      ]);
+    });
+  });
+
+  describe('buildSamlAttributesTagValues', () => {
+    it('should generate correct SAML attribute tag values from user info', () => {
+      const mockDetailsWithMapping = {
+        ...mockDetails,
+        attributeMapping: {
+          sub: 'userId',
+          email: 'emailAddress',
+          name: 'displayName',
+          phone: 'phoneNumber',
+        },
+      };
+
+      const samlApp = new TestSamlApplication(
+        // @ts-expect-error
+        mockDetailsWithMapping,
+        mockSamlApplicationId,
+        mockIssuer,
+        mockTenantId
+      );
+
+      const tagValues = samlApp.exposedBuildSamlAttributesTagValues(mockUser);
+
+      expect(tagValues).toEqual({
+        attrUserId: 'user123',
+        attrEmailAddress: 'user@example.com',
+        attrDisplayName: 'Test User',
+        attrPhoneNumber: '+1234567890',
+      });
+    });
+
+    it('should skip undefined or null values from user info', () => {
+      const mockDetailsWithMapping = {
+        ...mockDetails,
+        attributeMapping: {
+          sub: 'userId',
+          email: 'emailAddress',
+          name: 'displayName',
+          picture: 'avatar', // This field doesn't exist in mockUser
+        },
+      };
+
+      const samlApp = new TestSamlApplication(
+        // @ts-expect-error
+        mockDetailsWithMapping,
+        mockSamlApplicationId,
+        mockIssuer,
+        mockTenantId
+      );
+
+      const tagValues = samlApp.exposedBuildSamlAttributesTagValues(mockUser);
+
+      expect(tagValues).toEqual({
+        attrUserId: 'user123',
+        attrEmailAddress: 'user@example.com',
+        attrDisplayName: 'Test User',
+      });
+      expect(tagValues).not.toHaveProperty('attrAvatar');
+    });
+  });
 });

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

@@ -10,7 +10,7 @@ import {
   type SamlAttributeMapping,
 } from '@logto/schemas';
 import { generateStandardId } from '@logto/shared';
-import { tryThat, type Nullable, cond } from '@silverhand/essentials';
+import { cond, tryThat, type Nullable } from '@silverhand/essentials';
 import camelcaseKeys, { type CamelCaseKeys } from 'camelcase-keys';
 import { XMLValidator } from 'fast-xml-parser';
 import saml from 'samlify';
@@ -38,7 +38,11 @@ import {
 import { buildSingleSignOnUrl, buildSamlIdentityProviderEntityId } from '../libraries/utils.js';
 import { type SamlApplicationDetails } from '../queries/index.js';
 
-import { buildSamlAssertionNameId, getSamlAppCallbackUrl } from './utils.js';
+import {
+  buildSamlAssertionNameId,
+  getSamlAppCallbackUrl,
+  generateSamlAttributeTag,
+} from './utils.js';
 
 type SamlIdentityProviderConfig = {
   entityId: string;
@@ -70,81 +74,6 @@ saml.setSchemaValidator({
   },
 });
 
-const buildLoginResponseTemplate = () => {
-  return {
-    context: samlLogInResponseTemplate,
-    attributes: [
-      {
-        name: 'email',
-        valueTag: 'email',
-        nameFormat: samlAttributeNameFormatBasic,
-        valueXsiType: samlValueXmlnsXsi.string,
-      },
-      {
-        name: 'name',
-        valueTag: 'name',
-        nameFormat: samlAttributeNameFormatBasic,
-        valueXsiType: samlValueXmlnsXsi.string,
-      },
-    ],
-  };
-};
-
-const buildSamlIdentityProvider = ({
-  entityId,
-  certificate,
-  singleSignOnUrl,
-  privateKey,
-  nameIdFormat,
-  encryptSamlAssertion,
-}: SamlIdentityProviderConfig): saml.IdentityProviderInstance => {
-  // eslint-disable-next-line new-cap
-  return saml.IdentityProvider({
-    entityID: entityId,
-    signingCert: certificate,
-    singleSignOnService: [
-      {
-        Location: singleSignOnUrl,
-        Binding: BindingType.Redirect,
-      },
-      {
-        Location: singleSignOnUrl,
-        Binding: BindingType.Post,
-      },
-    ],
-    privateKey,
-    isAssertionEncrypted: encryptSamlAssertion,
-    loginResponseTemplate: buildLoginResponseTemplate(),
-    nameIDFormat: [nameIdFormat],
-  });
-};
-
-const buildSamlServiceProvider = ({
-  entityId,
-  acsUrl,
-  certificate,
-  isWantAuthnRequestsSigned,
-}: {
-  entityId: string;
-  acsUrl: SamlAcsUrl;
-  certificate: string;
-  isWantAuthnRequestsSigned: boolean;
-}): saml.ServiceProviderInstance => {
-  // eslint-disable-next-line new-cap
-  return saml.ServiceProvider({
-    entityID: entityId,
-    assertionConsumerService: [
-      {
-        Binding: acsUrl.binding,
-        Location: acsUrl.url,
-      },
-    ],
-    signingCert: certificate,
-    authnRequestsSigned: isWantAuthnRequestsSigned,
-    allowCreate: false,
-  });
-};
-
 class SamlApplicationConfig {
   constructor(private readonly _details: SamlApplicationDetails) {}
 
@@ -210,18 +139,12 @@ export class SamlApplication {
   }
 
   public get idp(): saml.IdentityProviderInstance {
-    this._idp ||= buildSamlIdentityProvider(this.buildIdpConfig());
+    this._idp ||= this.buildSamlIdentityProvider();
     return this._idp;
   }
 
   public get sp(): saml.ServiceProviderInstance {
-    const { certificate: encryptCert, ...rest } = this.buildSpConfig();
-    this._sp ||= buildSamlServiceProvider({
-      ...rest,
-      certificate: this.config.certificate,
-      isWantAuthnRequestsSigned: this.idp.entityMeta.isWantAuthnRequestsSigned(),
-      ...cond(encryptCert && { encryptCert }),
-    });
+    this._sp ||= this.buildSamlServiceProvider();
     return this._sp;
   }
 
@@ -306,6 +229,54 @@ export class SamlApplication {
     return new URL(`${authorizationEndpoint}?${queryParameters.toString()}`);
   };
 
+  protected buildSamlIdentityProvider = (): saml.IdentityProviderInstance => {
+    const {
+      entityId,
+      certificate,
+      singleSignOnUrl,
+      privateKey,
+      nameIdFormat,
+      encryptSamlAssertion,
+    } = this.buildIdpConfig();
+    // eslint-disable-next-line new-cap
+    return saml.IdentityProvider({
+      entityID: entityId,
+      signingCert: certificate,
+      singleSignOnService: [
+        {
+          Location: singleSignOnUrl,
+          Binding: BindingType.Redirect,
+        },
+        {
+          Location: singleSignOnUrl,
+          Binding: BindingType.Post,
+        },
+      ],
+      privateKey,
+      isAssertionEncrypted: encryptSamlAssertion,
+      loginResponseTemplate: this.buildLoginResponseTemplate(),
+      nameIDFormat: [nameIdFormat],
+    });
+  };
+
+  protected buildSamlServiceProvider = (): saml.ServiceProviderInstance => {
+    const { certificate: encryptCert, entityId, acsUrl } = this.buildSpConfig();
+    // eslint-disable-next-line new-cap
+    return saml.ServiceProvider({
+      entityID: entityId,
+      assertionConsumerService: [
+        {
+          Binding: acsUrl.binding,
+          Location: acsUrl.url,
+        },
+      ],
+      signingCert: this.config.certificate,
+      authnRequestsSigned: this.idp.entityMeta.isWantAuthnRequestsSigned(),
+      allowCreate: false,
+      ...cond(encryptCert && { encryptCert }),
+    });
+  };
+
   protected getOidcConfig = async (): Promise<CamelCaseKeys<OidcConfigResponse>> => {
     const oidcConfig = await tryThat(
       async () => fetchOidcConfigRaw(this.issuer),
@@ -460,9 +431,11 @@ export class SamlApplication {
          * @remarks
          * By examining the code provided in the link above, we can define all the attributes supported by the attribute mapping here. Only the attributes defined in the `loginResponseTemplate.attributes` added when creating the IdP instance will appear in the SAML response.
          */
+        // Keep the `attrSub`, `attrEmail` and `attrName` attributes since attribute mapping can be empty.
         attrSub: userInfo.sub,
         attrEmail: userInfo.email,
         attrName: userInfo.name,
+        ...this.buildSamlAttributesTagValues(userInfo),
       };
 
       const context = saml.SamlLib.replaceTagsByValue(template, tagValues);
@@ -473,6 +446,32 @@ export class SamlApplication {
       };
     };
 
+  protected readonly buildLoginResponseTemplate = () => {
+    return {
+      context: samlLogInResponseTemplate,
+      attributes: Object.values(this.config.attributeMapping).map((value) => ({
+        name: value,
+        valueTag: generateSamlAttributeTag(value),
+        nameFormat: samlAttributeNameFormatBasic,
+        valueXsiType: samlValueXmlnsXsi.string,
+      })),
+    };
+  };
+
+  protected readonly buildSamlAttributesTagValues = (
+    userInfo: IdTokenProfileStandardClaims
+  ): Record<string, string> => {
+    return Object.fromEntries(
+      Object.entries(this.config.attributeMapping)
+        .map(([key, value]) => {
+          // eslint-disable-next-line no-restricted-syntax
+          return [value, userInfo[key as keyof IdTokenProfileStandardClaims]] as [string, unknown];
+        })
+        .filter(([_, value]) => Boolean(value))
+        .map(([key, value]) => [generateSamlAttributeTag(key), String(value)])
+    );
+  };
+
   private buildIdpConfig(): SamlIdentityProviderConfig {
     return {
       entityId: buildSamlIdentityProviderEntityId(this.tenantEndpoint, this.samlApplicationId),

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

@@ -1,6 +1,7 @@
 import { NameIdFormat } from '@logto/schemas';
 import { generateStandardId } from '@logto/shared';
 import { appendPath } from '@silverhand/essentials';
+import camelCase from 'camelcase';
 
 import RequestError from '#src/errors/RequestError/index.js';
 import { type IdTokenProfileStandardClaims } from '#src/sso/types/oidc.js';
@@ -72,3 +73,14 @@ export const generateAutoSubmitForm = (actionUrl: string, samlResponse: string):
 
 export const getSamlAppCallbackUrl = (baseUrl: URL, samlAppId: string) =>
   appendPath(baseUrl, `api/saml-applications/${samlAppId}/callback`);
+
+/**
+ * @desc Tag normalization, copied from https://github.com/tngan/samlify/blob/master/src/libsaml.ts#L230-L240 to get SAML attribute tag name.
+ * @param {string} prefix     prefix of the tag
+ * @param {content} content   normalize it to capitalized camel case
+ * @return {string}
+ */
+export const generateSamlAttributeTag = (content: string, prefix = 'attr'): string => {
+  const camelContent = camelCase(content, { locale: 'en-us' });
+  return prefix + camelContent.charAt(0).toUpperCase() + camelContent.slice(1);
+};