Refactor SAML relayState structure (#20430)

# Introduction
Restructure the RelayState and avoid asserting the idp identifier from
this opaque blob
Inferring the id from the secured validated and signed request params

packages/twenty-server/src/engine/core-modules/auth/strategies/saml.auth.strategy.spec.ts

@@ -0,0 +1,205 @@
+/* @license Enterprise */
+
+import { type Request } from 'express';
+
+import { type SSOService } from 'src/engine/core-modules/sso/services/sso.service';
+
+import { SamlAuthStrategy } from './saml.auth.strategy';
+
+const IDP_A = 'idp-a-uuid';
+const IDP_B = 'idp-b-uuid';
+const VALID_EMAIL = 'alice@example.com';
+
+type RelayStateInput =
+  | { kind: 'absent' }
+  | { kind: 'raw'; value: string }
+  | { kind: 'json'; value: Record<string, unknown> };
+
+const buildRequest = ({
+  paramsIdpId,
+  relayState = { kind: 'absent' },
+}: {
+  paramsIdpId: string;
+  relayState?: RelayStateInput;
+}) => {
+  let body: Record<string, unknown> = {};
+
+  if (relayState.kind === 'raw') {
+    body = { RelayState: relayState.value };
+  } else if (relayState.kind === 'json') {
+    body = { RelayState: JSON.stringify(relayState.value) };
+  }
+
+  return {
+    params: { identityProviderId: paramsIdpId },
+    body,
+  } as unknown as Request;
+};
+
+const buildProfile = (email: string = VALID_EMAIL) =>
+  ({
+    email,
+    nameID: email,
+    issuer: 'irrelevant',
+  }) as unknown as Parameters<SamlAuthStrategy['validate']>[1];
+
+describe('SamlAuthStrategy.validate', () => {
+  let strategy: SamlAuthStrategy;
+
+  beforeEach(() => {
+    const ssoService = {
+      findSSOIdentityProviderById: jest.fn(),
+      isSAMLIdentityProvider: jest.fn(),
+      buildIssuerURL: jest.fn(),
+      buildCallbackUrl: jest.fn(),
+    } as unknown as SSOService;
+
+    strategy = new SamlAuthStrategy(ssoService);
+  });
+
+  // Regression test for the workspace-confusion finding. An attacker-controlled
+  // RelayState that claims a different identity-provider id than the one whose
+  // cert verified the assertion must have zero influence on the resolved IdP.
+  it('ignores RelayState.identityProviderId and sources it exclusively from the URL path', async () => {
+    const request = buildRequest({
+      paramsIdpId: IDP_A,
+      relayState: { kind: 'json', value: { identityProviderId: IDP_B } },
+    });
+    const done = jest.fn();
+
+    await strategy.validate(request, buildProfile(), done);
+
+    expect(done).toHaveBeenCalledTimes(1);
+    expect(done.mock.calls[0][0]).toBeNull();
+    expect(done.mock.calls[0][1]).toEqual({
+      identityProviderId: IDP_A,
+      workspaceInviteHash: undefined,
+      email: VALID_EMAIL,
+    });
+  });
+
+  it('accepts a request with no RelayState at all (no workspaceInviteHash threaded through)', async () => {
+    const request = buildRequest({ paramsIdpId: IDP_A });
+    const done = jest.fn();
+
+    await strategy.validate(request, buildProfile(), done);
+
+    expect(done).toHaveBeenCalledTimes(1);
+    expect(done.mock.calls[0][0]).toBeNull();
+    expect(done.mock.calls[0][1]).toEqual({
+      identityProviderId: IDP_A,
+      workspaceInviteHash: undefined,
+      email: VALID_EMAIL,
+    });
+  });
+
+  it('threads workspaceInviteHash from RelayState through to req.user', async () => {
+    const request = buildRequest({
+      paramsIdpId: IDP_A,
+      relayState: {
+        kind: 'json',
+        value: { workspaceInviteHash: 'invite-hash-123' },
+      },
+    });
+    const done = jest.fn();
+
+    await strategy.validate(request, buildProfile(), done);
+
+    expect(done).toHaveBeenCalledTimes(1);
+    expect(done.mock.calls[0][0]).toBeNull();
+    expect(done.mock.calls[0][1]).toEqual({
+      identityProviderId: IDP_A,
+      workspaceInviteHash: 'invite-hash-123',
+      email: VALID_EMAIL,
+    });
+  });
+
+  it('still resolves identityProviderId from the URL path when RelayState mixes a malicious id with a real invite hash', async () => {
+    const request = buildRequest({
+      paramsIdpId: IDP_A,
+      relayState: {
+        kind: 'json',
+        value: {
+          identityProviderId: IDP_B,
+          workspaceInviteHash: 'invite-hash-123',
+        },
+      },
+    });
+    const done = jest.fn();
+
+    await strategy.validate(request, buildProfile(), done);
+
+    expect(done).toHaveBeenCalledTimes(1);
+    expect(done.mock.calls[0][0]).toBeNull();
+    expect(done.mock.calls[0][1]).toEqual({
+      identityProviderId: IDP_A,
+      workspaceInviteHash: 'invite-hash-123',
+      email: VALID_EMAIL,
+    });
+  });
+
+  it('tolerates malformed RelayState JSON without throwing (no invite hash extracted)', async () => {
+    const request = buildRequest({
+      paramsIdpId: IDP_A,
+      relayState: { kind: 'raw', value: 'not-json{' },
+    });
+    const done = jest.fn();
+
+    await strategy.validate(request, buildProfile(), done);
+
+    expect(done).toHaveBeenCalledTimes(1);
+    expect(done.mock.calls[0][0]).toBeNull();
+    expect(done.mock.calls[0][1]).toEqual({
+      identityProviderId: IDP_A,
+      workspaceInviteHash: undefined,
+      email: VALID_EMAIL,
+    });
+  });
+
+  it('ignores non-string workspaceInviteHash payloads in RelayState', async () => {
+    const request = buildRequest({
+      paramsIdpId: IDP_A,
+      relayState: {
+        kind: 'json',
+        value: { workspaceInviteHash: { nested: 'object' } },
+      },
+    });
+    const done = jest.fn();
+
+    await strategy.validate(request, buildProfile(), done);
+
+    expect(done).toHaveBeenCalledTimes(1);
+    expect(done.mock.calls[0][0]).toBeNull();
+    expect(done.mock.calls[0][1]).toEqual({
+      identityProviderId: IDP_A,
+      workspaceInviteHash: undefined,
+      email: VALID_EMAIL,
+    });
+  });
+
+  it('rejects auth when the email claim is invalid', async () => {
+    const request = buildRequest({ paramsIdpId: IDP_A });
+    const done = jest.fn();
+
+    await strategy.validate(request, buildProfile('not-an-email'), done);
+
+    expect(done).toHaveBeenCalledTimes(1);
+    expect(done.mock.calls[0][0]).toBeInstanceOf(Error);
+    expect(done.mock.calls[0][0].message).toBe('Invalid email');
+  });
+
+  it('rejects auth when the profile is missing', async () => {
+    const request = buildRequest({ paramsIdpId: IDP_A });
+    const done = jest.fn();
+
+    await strategy.validate(
+      request,
+      undefined as unknown as Parameters<SamlAuthStrategy['validate']>[1],
+      done,
+    );
+
+    expect(done).toHaveBeenCalledTimes(1);
+    expect(done.mock.calls[0][0]).toBeInstanceOf(Error);
+    expect(done.mock.calls[0][0].message).toBe('Profile must be provided');
+  });
+});

packages/twenty-server/src/engine/core-modules/auth/strategies/saml.auth.strategy.ts

@@ -13,13 +13,32 @@ import {
 import { type AuthenticateOptions } from '@node-saml/passport-saml/lib/types';
 import { isEmail } from 'class-validator';
 import { type Request } from 'express';
+import { z } from 'zod';
 
-import {
-  AuthException,
-  AuthExceptionCode,
-} from 'src/engine/core-modules/auth/auth.exception';
 import { SSOService } from 'src/engine/core-modules/sso/services/sso.service';
 
+const WORKSPACE_INVITE_HASH_PAYLOAD_SCHEMA = z.object({
+  workspaceInviteHash: z.string().optional(),
+});
+
+const RELAY_STATE_BODY_SCHEMA = z.object({
+  RelayState: z
+    .string()
+    .transform((raw, ctx) => {
+      try {
+        return JSON.parse(raw) as unknown;
+      } catch {
+        ctx.addIssue({
+          code: 'custom',
+          message: 'RelayState is not valid JSON',
+        });
+
+        return z.NEVER;
+      }
+    })
+    .pipe(WORKSPACE_INVITE_HASH_PAYLOAD_SCHEMA),
+});
+
 export type SAMLRequest = Omit<
   Request,
   'user' | 'workspace' | 'workspaceMetadataVersion'
@@ -83,43 +102,37 @@ export class SamlAuthStrategy extends PassportStrategy(
   }
 
   authenticate(req: Request, options: AuthenticateOptions) {
+    const queryParseResult = WORKSPACE_INVITE_HASH_PAYLOAD_SCHEMA.safeParse(
+      req.query,
+    );
+    const workspaceInviteHash = queryParseResult.success
+      ? queryParseResult.data.workspaceInviteHash
+      : undefined;
+
     super.authenticate(req, {
       ...options,
-      additionalParams: {
-        RelayState: JSON.stringify({
-          identityProviderId: req.params.identityProviderId,
-          ...(req.query.workspaceInviteHash
-            ? { workspaceInviteHash: req.query.workspaceInviteHash }
-            : {}),
-        }),
-      },
+      ...(workspaceInviteHash !== undefined
+        ? {
+            additionalParams: {
+              RelayState: JSON.stringify({ workspaceInviteHash }),
+            },
+          }
+        : {}),
     });
   }
 
-  private extractState(req: Request): {
-    identityProviderId: string;
-    workspaceInviteHash?: string;
-  } {
-    try {
-      if ('RelayState' in req.body && typeof req.body.RelayState === 'string') {
-        const RelayState = JSON.parse(req.body.RelayState);
+  private extractWorkspaceInviteHash(req: Request): string | undefined {
+    const result = RELAY_STATE_BODY_SCHEMA.safeParse(req.body);
 
-        return {
-          identityProviderId: RelayState.identityProviderId,
-          workspaceInviteHash: RelayState.workspaceInviteHash,
-        };
-      }
-
-      throw new Error();
-    } catch {
-      throw new AuthException('Invalid state', AuthExceptionCode.INVALID_INPUT);
-    }
+    return result.success
+      ? result.data.RelayState.workspaceInviteHash
+      : undefined;
   }
 
   validate: VerifyWithRequest = async (request, profile, done) => {
     try {
       if (!profile) {
-        return done(new Error('Profile is must be provided'));
+        return done(new Error('Profile must be provided'));
       }
 
       const email = profile.email ?? profile.mail ?? profile.nameID;
@@ -127,9 +140,12 @@ export class SamlAuthStrategy extends PassportStrategy(
       if (!isEmail(email)) {
         return done(new Error('Invalid email'));
       }
-      const state = this.extractState(request);
 
-      done(null, { ...state, email });
+      done(null, {
+        identityProviderId: request.params.identityProviderId,
+        workspaceInviteHash: this.extractWorkspaceInviteHash(request),
+        email,
+      });
     } catch (err) {
       done(err);
     }