fix(saml): use whole-entry config writes; correct test request shapes

E2E tests + dashboard SSO page were writing per-field deep dot-keys like `auth.saml.connections.X.displayName`, which the config normalizer drops because the parent record entry doesn't yet exist when the connection is being created. Match the existing auth.oauth.providers convention: write the whole connection entry as a single value on create. Also fixed two test-harness issues uncovered while running the suite: - Round-trip ACS POST was using niceBackendFetch which always JSON.stringifies the body. Switched to plain niceFetch so URLSearchParams gets sent as application/x-www-form-urlencoded. - Mock IdP /metadata returns application/xml, which makes niceFetch return ArrayBuffer; added a TextDecoder pass before regex matching.
2026-06-04 21:04:37 +08:00 · 2026-04-29 16:39:56 -07:00 · 2026-04-29 16:39:56 -07:00 · 5fa9629ded
commit 5fa9629ded
parent edb13f3107
5 changed files with 72 additions and 44 deletions
--- a/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sso/page-client.tsx
+++ b/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sso/page-client.tsx
@ -144,19 +144,23 @@ function CreateDialog({ open, onOpenChange }: { open: boolean, onOpenChange: (op
      okButton={{ label: "Create" }}
      cancelButton
      onSubmit={async (values) => {
-        const overlay: Record<string, unknown> = {
-          [`auth.saml.connections.${values.id}.displayName`]: values.displayName,
-          [`auth.saml.connections.${values.id}.allowSignIn`]: values.allowSignIn,
-          [`auth.saml.connections.${values.id}.idpEntityId`]: values.idpEntityId,
-          [`auth.saml.connections.${values.id}.idpSsoUrl`]: values.idpSsoUrl,
-          [`auth.saml.connections.${values.id}.idpCertificate`]: (values.idpCertificate ?? "").replace(/-----BEGIN CERTIFICATE-----|-----END CERTIFICATE-----|\s+/g, ""),
-        };
-        if (values.domain) {
-          overlay[`auth.saml.connections.${values.id}.domain`] = values.domain;
-        }
+        // Set the whole connection entry as a single value. Deep dot-keys
+        // (e.g. `auth.saml.connections.X.displayName`) get dropped during
+        // config normalization when the parent record entry doesn't yet
+        // exist — same convention as auth.oauth.providers in the
+        // auth-methods page.
        await updateConfig({
          adminApp: stackAdminApp,
-          configUpdate: overlay as Parameters<typeof updateConfig>[0]["configUpdate"],
+          configUpdate: {
+            [`auth.saml.connections.${values.id}`]: {
+              displayName: values.displayName,
+              allowSignIn: values.allowSignIn,
+              domain: values.domain || undefined,
+              idpEntityId: values.idpEntityId,
+              idpSsoUrl: values.idpSsoUrl,
+              idpCertificate: (values.idpCertificate ?? "").replace(/-----BEGIN CERTIFICATE-----|-----END CERTIFICATE-----|\s+/g, ""),
+            },
+          } as Parameters<typeof updateConfig>[0]["configUpdate"],
          pushable: true,
        });
      }}
--- a/apps/e2e/tests/backend/endpoints/api/v1/auth/saml/discover.test.ts
+++ b/apps/e2e/tests/backend/endpoints/api/v1/auth/saml/discover.test.ts
@ -15,16 +15,21 @@ import { Project, niceBackendFetch } from "../../../../../backend-helpers";

 async function createProjectWithSamlConnection(slug: string, domain: string) {
  const { projectId } = await Project.createAndSwitch();
-  // Push the SAML connection at the environment level — that's where the
-  // IdP-side fields live. The discovery endpoint reads from the rendered
-  // organization config which folds in env overrides.
+  // Set the entire connection entry as a single value. The override
+  // system handles `auth.saml.connections.{id}: {full object}` cleanly,
+  // but per-field deep dot-keys (e.g. .displayName) on a record entry
+  // that doesn't yet exist get dropped during config normalization with
+  // onDotIntoNonObject="ignore" — same convention as auth.oauth.providers
+  // (see auth-methods/page-client.tsx).
  await Project.updateConfig({
-    [`auth.saml.connections.${slug}.displayName`]: `${slug} SSO`,
-    [`auth.saml.connections.${slug}.allowSignIn`]: true,
-    [`auth.saml.connections.${slug}.domain`]: domain,
-    [`auth.saml.connections.${slug}.idpEntityId`]: `https://idp.${domain}/saml/metadata`,
-    [`auth.saml.connections.${slug}.idpSsoUrl`]: `https://idp.${domain}/saml/sso`,
-    [`auth.saml.connections.${slug}.idpCertificate`]: "MIICertificatePlaceholderForDiscoveryTest=",
+    [`auth.saml.connections.${slug}`]: {
+      displayName: `${slug} SSO`,
+      allowSignIn: true,
+      domain,
+      idpEntityId: `https://idp.${domain}/saml/metadata`,
+      idpSsoUrl: `https://idp.${domain}/saml/sso`,
+      idpCertificate: "MIICertificatePlaceholderForDiscoveryTest=",
+    },
  });
  return { projectId };
 }
--- a/apps/e2e/tests/backend/endpoints/api/v1/auth/saml/login.test.ts
+++ b/apps/e2e/tests/backend/endpoints/api/v1/auth/saml/login.test.ts
@ -18,11 +18,13 @@ async function setupProjectWithSamlConnection(slug: string, idpHost: string) {
  await Project.createAndSwitch();
  await InternalApiKey.createAndSetProjectKeys();
  await Project.updateConfig({
-    [`auth.saml.connections.${slug}.displayName`]: `${slug} SSO`,
-    [`auth.saml.connections.${slug}.allowSignIn`]: true,
-    [`auth.saml.connections.${slug}.idpEntityId`]: `https://${idpHost}/saml/metadata`,
-    [`auth.saml.connections.${slug}.idpSsoUrl`]: `https://${idpHost}/saml/sso`,
-    [`auth.saml.connections.${slug}.idpCertificate`]: "MIICertificatePlaceholderForLoginTest=",
+    [`auth.saml.connections.${slug}`]: {
+      displayName: `${slug} SSO`,
+      allowSignIn: true,
+      idpEntityId: `https://${idpHost}/saml/metadata`,
+      idpSsoUrl: `https://${idpHost}/saml/sso`,
+      idpCertificate: "MIICertificatePlaceholderForLoginTest=",
+    },
  });
 }

@ -91,6 +93,8 @@ it("returns 404 for an unknown connection ID", async ({ expect }) => {

 it("returns 403 when allowSignIn is false on the connection", async ({ expect }) => {
  await setupProjectWithSamlConnection("acme", "idp.acme.test");
+  // The connection already exists, so deep-key updates work (the parent
+  // record entry is navigable now).
  await Project.updateConfig({ "auth.saml.connections.acme.allowSignIn": false });

  const response = await niceBackendFetch("/api/v1/auth/saml/login/acme", {
--- a/apps/e2e/tests/backend/endpoints/api/v1/auth/saml/metadata.test.ts
+++ b/apps/e2e/tests/backend/endpoints/api/v1/auth/saml/metadata.test.ts
@ -12,11 +12,13 @@ import { Project, niceBackendFetch } from "../../../../../backend-helpers";
 async function setupSamlConnection(slug: string) {
  const { projectId } = await Project.createAndSwitch();
  await Project.updateConfig({
-    [`auth.saml.connections.${slug}.displayName`]: `${slug} SSO`,
-    [`auth.saml.connections.${slug}.allowSignIn`]: true,
-    [`auth.saml.connections.${slug}.idpEntityId`]: `https://idp.${slug}.test/saml/metadata`,
-    [`auth.saml.connections.${slug}.idpSsoUrl`]: `https://idp.${slug}.test/saml/sso`,
-    [`auth.saml.connections.${slug}.idpCertificate`]: "MIICertificatePlaceholderForMetadataTest=",
+    [`auth.saml.connections.${slug}`]: {
+      displayName: `${slug} SSO`,
+      allowSignIn: true,
+      idpEntityId: `https://idp.${slug}.test/saml/metadata`,
+      idpSsoUrl: `https://idp.${slug}.test/saml/sso`,
+      idpCertificate: "MIICertificatePlaceholderForMetadataTest=",
+    },
  });
  return { projectId };
 }
@ -54,11 +56,13 @@ it("returns 404 for an unknown connection ID", async ({ expect }) => {

 it("returns 404 when the connection exists but has no IdP cert configured", async ({ expect }) => {
  const { projectId } = await Project.createAndSwitch();
-  // Create a connection at branch level but skip the env-level IdP fields.
+  // Create a connection but skip the IdP-side fields.
  await Project.updateConfig({
-    "auth.saml.connections.partial.displayName": "Partial",
-    "auth.saml.connections.partial.allowSignIn": true,
-    // No idpEntityId / idpSsoUrl / idpCertificate.
+    "auth.saml.connections.partial": {
+      displayName: "Partial",
+      allowSignIn: true,
+      // No idpEntityId / idpSsoUrl / idpCertificate.
+    },
  });

  const response = await niceBackendFetch(
--- a/apps/e2e/tests/backend/endpoints/api/v1/auth/saml/round-trip.test.ts
+++ b/apps/e2e/tests/backend/endpoints/api/v1/auth/saml/round-trip.test.ts
@ -27,6 +27,7 @@ import { localhostUrl } from "../../../../../../helpers/ports";
 import { InternalApiKey, Project, backendContext, niceBackendFetch } from "../../../../../backend-helpers";

 const MOCK_SAML_BASE = localhostUrl("15");
+const BACKEND_BASE = localhostUrl("02");

 // ---------- helpers ----------

@ -39,7 +40,10 @@ async function fetchMockIdpCertificate(tenantSlug: string): Promise<{
  if (res.status !== 200) {
    throw new Error(`Mock IdP returned ${res.status} for ${tenantSlug} metadata — is mock-saml-idp running on port 8115?`);
  }
-  const xml = res.body as string;
+  // application/xml content-type makes niceFetch return ArrayBuffer; decode it.
+  const xml = typeof res.body === "string"
+    ? res.body
+    : new TextDecoder("utf-8").decode(res.body as ArrayBuffer);
  const entityIdMatch = xml.match(/entityID="([^"]+)"/);
  const ssoMatch = xml.match(/Binding="urn:oasis:names:tc:SAML:2\.0:bindings:HTTP-Redirect"[^>]*Location="([^"]+)"/);
  const certMatch = xml.match(/<X509Certificate>([\s\S]+?)<\/X509Certificate>/);
@ -58,11 +62,13 @@ async function setupProjectWithMockSamlConnection(connectionId: string, tenantSl
  await InternalApiKey.createAndSetProjectKeys();
  const idp = await fetchMockIdpCertificate(tenantSlug);
  await Project.updateConfig({
-    [`auth.saml.connections.${connectionId}.displayName`]: `${connectionId} SSO`,
-    [`auth.saml.connections.${connectionId}.allowSignIn`]: true,
-    [`auth.saml.connections.${connectionId}.idpEntityId`]: idp.entityId,
-    [`auth.saml.connections.${connectionId}.idpSsoUrl`]: idp.ssoUrl,
-    [`auth.saml.connections.${connectionId}.idpCertificate`]: idp.certificate,
+    [`auth.saml.connections.${connectionId}`]: {
+      displayName: `${connectionId} SSO`,
+      allowSignIn: true,
+      idpEntityId: idp.entityId,
+      idpSsoUrl: idp.ssoUrl,
+      idpCertificate: idp.certificate,
+    },
  });
  return { connectionId, tenantSlug };
 }
@ -120,13 +126,18 @@ async function runSamlRoundTrip(options: {
  }

  // Step 3: extract SAMLResponse from the auto-POST HTML.
-  const html = idpLoginRes.body as string;
+  const html = typeof idpLoginRes.body === "string"
+    ? idpLoginRes.body
+    : new TextDecoder("utf-8").decode(idpLoginRes.body as ArrayBuffer);
  const samlResponseMatch = html.match(/name="SAMLResponse" value="([^"]+)"/);
  if (!samlResponseMatch) throw new Error(`Mock IdP did not return a SAMLResponse form: ${html.slice(0, 200)}`);
  const samlResponse = samlResponseMatch[1].replace(/&#x2F;/g, "/").replace(/&#x3D;/g, "=").replace(/&amp;/g, "&");

-  // Step 4: POST to ACS.
-  const acsRes = await niceBackendFetch(`/api/v1/auth/saml/acs/${options.connectionId}`, {
+  // Step 4: POST to ACS — use niceFetch directly so URLSearchParams gets
+  // sent as application/x-www-form-urlencoded. niceBackendFetch always
+  // JSON.stringifies the body, which doesn't work for the IdP-style
+  // form POST that ACS expects.
+  const acsRes = await niceFetch(`${BACKEND_BASE}/api/v1/auth/saml/acs/${options.connectionId}`, {
    method: "POST",
    redirect: "manual",
    body: new URLSearchParams({
@ -223,7 +234,7 @@ it("rejects replay of a previously-consumed assertion", async ({ expect }) => {
  // Replay the same SAMLResponse — backend should reject because the
  // SamlOuterInfo row was deleted at the end of the first ACS call, so
  // the InResponseTo lookup misses.
-  const replayRes = await niceBackendFetch(`/api/v1/auth/saml/acs/acme`, {
+  const replayRes = await niceFetch(`${BACKEND_BASE}/api/v1/auth/saml/acs/acme`, {
    method: "POST",
    redirect: "manual",
    body: new URLSearchParams({