fix(sso): accept HTTP-POST binding and surface descriptive parser errors (#21051)

Fixes https://github.com/twentyhq/twenty/issues/21044 ## Summary - Closes [#490](https://github.com/twentyhq/private-issues/issues/490) — JumpCloud customers (and anyone else whose IdP only advertises `HTTP-POST` for `SingleSignOnService`) could not upload their SAML metadata; the parser silently rejected them with a generic `Invalid file` toast. - The SAML IdP metadata parser now falls back to `HTTP-POST` when `HTTP-Redirect` is not advertised. Both are valid SAML 2.0 bindings. - The parser now returns a descriptive `reason` string (Zod issues + custom errors) instead of an opaque `error: unknown`, and the upload snack bar surfaces it so the customer can self-diagnose (e.g. `entityID: entityID is not a valid URL` if they forgot to fill in their IdP Entity ID). - Added unit tests for HTTP-POST-only metadata, HTTP-Redirect preference, and each descriptive-error path. ## Test plan - [x] `npx jest parseSAMLMetadataFromXMLFile --config=packages/twenty-front/jest.config.mjs` — 8/8 pass - [x] `npx oxlint -c packages/twenty-front/.oxlintrc.json` on changed files — clean - [x] `npx oxfmt --check` on changed files — clean - [ ] Manual: upload the customer's JumpCloud metadata (HTTP-POST only, placeholder `entityID`) and confirm the error now says `Invalid file: entityID: entityID is not a valid URL` instead of `Invalid file` - [ ] Manual: upload metadata with a real `entityID` and HTTP-POST-only binding, confirm the form populates correctly
2026-05-29 15:22:50 +02:00
parent 4e5d47168c
commit 667cb95730
3 changed files with 148 additions and 25 deletions
@@ -69,9 +69,9 @@ export const SettingsSSOSAMLForm = () => {
      e.target.value = '';
      if (!samlMetadataParsed.success) {
        return enqueueErrorSnackBar({
-          message: t`Invalid File`,
+          message: t`Invalid file: ${samlMetadataParsed.reason}`,
          options: {
-            duration: 2000,
+            duration: 5000,
          },
        });
      }
@@ -53,12 +53,106 @@ describe('parseSAMLMetadataFromXMLFile', () => {
      },
    });
  });
+  it('should fall back to HTTP-POST binding when HTTP-Redirect is not advertised', () => {
+    const xmlString = `<?xml version="1.0" encoding="UTF-8"?><md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" entityID="https://test.com">
+    <md:IDPSSODescriptor WantAuthnRequestsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+        <md:KeyDescriptor use="signing">
+            <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+                <ds:X509Data>
+                    <ds:X509Certificate>test</ds:X509Certificate>
+                </ds:X509Data>
+            </ds:KeyInfo>
+        </md:KeyDescriptor>
+        <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://sso.jumpcloud.com/saml2/twenty"/>
+    </md:IDPSSODescriptor>
+</md:EntityDescriptor>`;
+    const result = parseSAMLMetadataFromXMLFile(xmlString);
+    expect(result).toEqual({
+      success: true,
+      data: {
+        entityID: 'https://test.com',
+        ssoUrl: 'https://sso.jumpcloud.com/saml2/twenty',
+        certificate: 'test',
+      },
+    });
+  });
+  it('should prefer HTTP-Redirect over HTTP-POST when both are advertised', () => {
+    const xmlString = `<?xml version="1.0" encoding="UTF-8"?><md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" entityID="https://test.com">
+    <md:IDPSSODescriptor WantAuthnRequestsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+        <md:KeyDescriptor use="signing">
+            <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+                <ds:X509Data>
+                    <ds:X509Certificate>test</ds:X509Certificate>
+                </ds:X509Data>
+            </ds:KeyInfo>
+        </md:KeyDescriptor>
+        <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://test.com/post"/>
+        <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://test.com/redirect"/>
+    </md:IDPSSODescriptor>
+</md:EntityDescriptor>`;
+    const result = parseSAMLMetadataFromXMLFile(xmlString);
+    expect(result).toEqual({
+      success: true,
+      data: {
+        entityID: 'https://test.com',
+        ssoUrl: 'https://test.com/redirect',
+        certificate: 'test',
+      },
+    });
+  });
+  it('should return a descriptive reason when no supported binding is found', () => {
+    const xmlString = `<?xml version="1.0" encoding="UTF-8"?><md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" entityID="https://test.com">
+    <md:IDPSSODescriptor WantAuthnRequestsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+        <md:KeyDescriptor use="signing">
+            <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+                <ds:X509Data>
+                    <ds:X509Certificate>test</ds:X509Certificate>
+                </ds:X509Data>
+            </ds:KeyInfo>
+        </md:KeyDescriptor>
+        <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:SOAP" Location="https://test.com/soap"/>
+    </md:IDPSSODescriptor>
+</md:EntityDescriptor>`;
+    const result = parseSAMLMetadataFromXMLFile(xmlString);
+    expect(result).toEqual({
+      success: false,
+      reason:
+        'No SingleSignOnService with HTTP-Redirect or HTTP-POST binding was found',
+    });
+  });
+  it('should return a descriptive reason when entityID is not a valid URL', () => {
+    const xmlString = `<?xml version="1.0" encoding="UTF-8"?><md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" entityID="IdP Entity ID">
+    <md:IDPSSODescriptor WantAuthnRequestsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+        <md:KeyDescriptor use="signing">
+            <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+                <ds:X509Data>
+                    <ds:X509Certificate>test</ds:X509Certificate>
+                </ds:X509Data>
+            </ds:KeyInfo>
+        </md:KeyDescriptor>
+        <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://sso.jumpcloud.com/saml2/twenty"/>
+    </md:IDPSSODescriptor>
+</md:EntityDescriptor>`;
+    const result = parseSAMLMetadataFromXMLFile(xmlString);
+    expect(result).toEqual({
+      success: false,
+      reason: 'entityID: entityID is not a valid URL',
+    });
+  });
+  it('should return a descriptive reason when EntityDescriptor is missing', () => {
+    const xmlString = `<?xml version="1.0" encoding="UTF-8"?><root />`;
+    const result = parseSAMLMetadataFromXMLFile(xmlString);
+    expect(result).toEqual({
+      success: false,
+      reason: 'EntityDescriptor element is missing',
+    });
+  });
  it('should return error if XML is invalid', () => {
    const xmlString = 'invalid xml';
    const result = parseSAMLMetadataFromXMLFile(xmlString);
    expect(result).toEqual({
      success: false,
-      error: new Error('Error parsing XML'),
+      reason: 'File is not valid XML',
    });
  });
 });
@@ -2,9 +2,13 @@

 import { z } from 'zod';

+const HTTP_REDIRECT_BINDING =
+  'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect';
+const HTTP_POST_BINDING = 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST';
+
 const validator = z.object({
-  entityID: z.url(),
-  ssoUrl: z.url(),
+  entityID: z.url('entityID is not a valid URL'),
+  ssoUrl: z.url('SingleSignOnService Location is not a valid URL'),
  certificate: z.string().min(1),
 });

@@ -41,61 +45,86 @@ const getAllByPrefixAndKey = (
  return Array.from(xmlDoc.getElementsByTagName(`${key}`));
 };

+const formatErrorReason = (error: unknown): string => {
+  if (error instanceof z.ZodError) {
+    return error.issues
+      .map((issue) => {
+        const path = issue.path.join('.');
+        return path.length > 0 ? `${path}: ${issue.message}` : issue.message;
+      })
+      .join('; ');
+  }
+  if (error instanceof Error) return error.message;
+  return 'Unknown parsing error';
+};
+
 export const parseSAMLMetadataFromXMLFile = (
  xmlString: string,
 ):
  | { success: true; data: z.infer<typeof validator> }
-  | { success: false; error: unknown } => {
+  | { success: false; reason: string } => {
  try {
    const parser = new DOMParser();
    const xmlDoc = parser.parseFromString(xmlString, 'application/xml');
    if (xmlDoc.getElementsByTagName('parsererror').length > 0) {
-      throw new Error('Error parsing XML');
+      throw new Error('File is not valid XML');
    }

    const entityDescriptor = getByPrefixAndKey(xmlDoc, 'EntityDescriptor');
-    if (!entityDescriptor) throw new Error('No EntityDescriptor found');
+    if (!entityDescriptor)
+      throw new Error('EntityDescriptor element is missing');

    const IDPSSODescriptor = getByPrefixAndKey(xmlDoc, 'IDPSSODescriptor');
-    if (!IDPSSODescriptor) throw new Error('No IDPSSODescriptor found');
+    if (!IDPSSODescriptor)
+      throw new Error('IDPSSODescriptor element is missing');

    const keyDescriptors = getByPrefixAndKey(IDPSSODescriptor, 'KeyDescriptor');
-    if (!keyDescriptors) throw new Error('No KeyDescriptor found');
+    if (!keyDescriptors) throw new Error('KeyDescriptor element is missing');

    const keyInfo = getByPrefixAndKey(keyDescriptors, 'KeyInfo');
-    if (!keyInfo) throw new Error('No KeyInfo found');
+    if (!keyInfo) throw new Error('KeyInfo element is missing');

    const x509Data = getByPrefixAndKey(keyInfo, 'X509Data');
-    if (!x509Data) throw new Error('No X509Data found');
+    if (!x509Data) throw new Error('X509Data element is missing');

    const x509Certificate = getByPrefixAndKey(
      x509Data,
      'X509Certificate',
    )?.textContent?.trim();
-    if (!x509Certificate) throw new Error('No X509Certificate found');
+    if (!x509Certificate)
+      throw new Error('X509Certificate is missing or empty');

    const singleSignOnServices = getAllByPrefixAndKey(
      IDPSSODescriptor,
      'SingleSignOnService',
-    );
+    ).map((service) => ({
+      binding: service.getAttribute('Binding'),
+      location: service.getAttribute('Location'),
+    }));
+
+    // Prefer HTTP-Redirect (the default authnRequestBinding on the SP side),
+    // fall back to HTTP-POST since both are valid SAML 2.0 bindings and many
+    // IdPs (e.g. JumpCloud) only advertise HTTP-POST.
+    const ssoUrl =
+      singleSignOnServices.find((s) => s.binding === HTTP_REDIRECT_BINDING)
+        ?.location ??
+      singleSignOnServices.find((s) => s.binding === HTTP_POST_BINDING)
+        ?.location;
+
+    if (!ssoUrl) {
+      throw new Error(
+        'No SingleSignOnService with HTTP-Redirect or HTTP-POST binding was found',
+      );
+    }

    const result = {
-      ssoUrl: singleSignOnServices
-        .map((service) => ({
-          Binding: service.getAttribute('Binding'),
-          Location: service.getAttribute('Location'),
-        }))
-        .find(
-          (singleSignOnService) =>
-            singleSignOnService.Binding ===
-            'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect',
-        )?.Location,
+      ssoUrl,
      certificate: x509Certificate,
      entityID: entityDescriptor?.getAttribute('entityID'),
    };

    return { success: true, data: validator.parse(result) };
  } catch (error) {
-    return { success: false, error };
+    return { success: false, reason: formatErrorReason(error) };
  }
 };