Adjust SCIM and SAML impl to use username / nameID, patch LDAP edge-cases

2026-09-22 13:39:35 +00:00 · 2024-02-25 18:16:26 -08:00
parent 8c31566e17
commit a33c50b75a
15 changed files with 136 additions and 82 deletions
--- a/backend/src/ee/routes/v1/ldap-router.ts
+++ b/backend/src/ee/routes/v1/ldap-router.ts
@@ -27,7 +27,7 @@ export const registerLdapRouter = async (server: FastifyZodProvider) => {

  passport.use(
    new LdapStrategy(
-      server.services.ldap.getLDAPConfiguration,
+      server.services.ldap.getLdapPassportOpts,
      // eslint-disable-next-line
        async (req, user, cb) => {
        try {
@@ -98,7 +98,7 @@ export const registerLdapRouter = async (server: FastifyZodProvider) => {
      }
    },
    handler: async (req) => {
-      const ldap = await server.services.ldap.getLdapCfg({
+      const ldap = await server.services.ldap.getLdapCfgWithPermissionCheck({
        actor: req.permission.type,
        actorId: req.permission.id,
        orgId: req.query.organizationId,
--- a/backend/src/ee/routes/v1/saml-router.ts
+++ b/backend/src/ee/routes/v1/saml-router.ts
@@ -94,15 +94,14 @@ export const registerSamlRouter = async (server: FastifyZodProvider) => {
      async (req, profile, cb) => {
        try {
          if (!profile) throw new BadRequestError({ message: "Missing profile" });
-          const { firstName } = profile;
-          const email = profile?.email ?? (profile?.emailAddress as string); // emailRippling is added because in Rippling the field `email` reserved

-          if (!email || !firstName) {
+          if (!profile.nameID || !profile.firstName) {
            throw new BadRequestError({ message: "Invalid request. Missing email or first name" });
          }

          const { isUserCompleted, providerAuthToken } = await server.services.saml.samlLogin({
-            email,
+            username: profile.nameID,
+            email: profile.email,
            firstName: profile.firstName as string,
            lastName: profile.lastName as string,
            relayState: (req.body as { RelayState?: string }).RelayState,
--- a/backend/src/ee/routes/v1/scim-router.ts
+++ b/backend/src/ee/routes/v1/scim-router.ts
@@ -122,7 +122,7 @@ export const registerScimRouter = async (server: FastifyZodProvider) => {
              emails: z.array(
                z.object({
                  primary: z.boolean(),
-                  value: z.string().email(),
+                  value: z.string(),
                  type: z.string().trim()
                })
              ),
@@ -168,7 +168,7 @@ export const registerScimRouter = async (server: FastifyZodProvider) => {
          emails: z.array(
            z.object({
              primary: z.boolean(),
-              value: z.string().email(),
+              value: z.string(),
              type: z.string().trim()
            })
          ),
@@ -198,13 +198,15 @@ export const registerScimRouter = async (server: FastifyZodProvider) => {
          familyName: z.string().trim(),
          givenName: z.string().trim()
        }),
-        // emails: z.array( // optional?
-        //   z.object({
-        //     primary: z.boolean(),
-        //     value: z.string().email(),
-        //     type: z.string().trim()
-        //   })
-        // ),
+        emails: z
+          .array(
+            z.object({
+              primary: z.boolean(),
+              value: z.string().email(),
+              type: z.string().trim()
+            })
+          )
+          .optional(),
        // displayName: z.string().trim(),
        active: z.boolean()
      }),
@@ -231,8 +233,11 @@ export const registerScimRouter = async (server: FastifyZodProvider) => {
    },
    onRequest: verifyAuth([AuthMode.SCIM_TOKEN]),
    handler: async (req) => {
+      const primaryEmail = req.body.emails?.find((email) => email.primary)?.value;
+
      const user = await req.server.services.scim.createScimUser({
-        email: req.body.userName,
+        username: req.body.userName,
+        email: primaryEmail as string,
        firstName: req.body.name.givenName,
        lastName: req.body.name.familyName,
        orgId: req.permission.orgId as string
--- a/backend/src/ee/services/ldap-config/ldap-config-service.ts
+++ b/backend/src/ee/services/ldap-config/ldap-config-service.ts
@@ -13,6 +13,7 @@ import {
  infisicalSymmetricEncypt
 } from "@app/lib/crypto/encryption";
 import { BadRequestError } from "@app/lib/errors";
+import { logger } from "@app/lib/logger";
 import { TOrgPermission } from "@app/lib/types";
 import { AuthMethod, AuthTokenType } from "@app/services/auth/auth-type";
 import { TOrgBotDALFactory } from "@app/services/org/org-bot-dal";
@@ -204,8 +205,8 @@ export const ldapConfigServiceFactory = ({
    return ldapConfig;
  };

-  const getLdapCfg2 = async (orgId: string) => {
-    const ldapConfig = await ldapConfigDAL.findOne({ orgId });
+  const getLdapCfg = async (filter: { orgId: string; isActive?: boolean }) => {
+    const ldapConfig = await ldapConfigDAL.findOne(filter);
    if (!ldapConfig) throw new BadRequestError({ message: "Failed to find organization LDAP data" });

    const orgBot = await orgBotDAL.findOne({ orgId: ldapConfig.orgId });
@@ -272,44 +273,55 @@ export const ldapConfigServiceFactory = ({
    };
  };

-  const getLdapCfg = async ({ actor, actorId, orgId, actorOrgId }: TOrgPermission) => {
+  const getLdapCfgWithPermissionCheck = async ({ actor, actorId, orgId, actorOrgId }: TOrgPermission) => {
    const { permission } = await permissionService.getOrgPermission(actor, actorId, orgId, actorOrgId);
    ForbiddenError.from(permission).throwUnlessCan(OrgPermissionActions.Read, OrgPermissionSubjects.Sso);
-    return getLdapCfg2(orgId);
+    return getLdapCfg({
+      orgId
+    });
  };

  // eslint-disable-next-line
-  const getLDAPConfiguration = (req: FastifyRequest, callback: any) => {
+  const getLdapPassportOpts = (req: FastifyRequest, done: any) => {
    const { organizationSlug } = req.body as {
      organizationSlug: string;
    };

    const boot = async () => {
-      const organization = await orgDAL.findOne({ slug: organizationSlug });
-      const ldapConfig = await getLdapCfg2(organization.id); // repeat?
-      req.ldapConfig = ldapConfig;
+      try {
+        const organization = await orgDAL.findOne({ slug: organizationSlug });
+        const ldapConfig = await getLdapCfg({
+          orgId: organization.id,
+          isActive: true
+        });
+        req.ldapConfig = ldapConfig;

-      const opts = {
-        server: {
-          url: ldapConfig.url,
-          bindDN: ldapConfig.bindDN,
-          bindCredentials: ldapConfig.bindPass,
-          searchBase: ldapConfig.searchBase,
-          searchFilter: "(uid={{username}})",
-          searchAttributes: ["uid", "givenName", "sn"],
-          ...(ldapConfig.caCert !== ""
-            ? {
-                tlsOptions: {
-                  ca: [ldapConfig.caCert]
+        const opts = {
+          server: {
+            url: ldapConfig.url,
+            bindDN: ldapConfig.bindDN,
+            bindCredentials: ldapConfig.bindPass,
+            searchBase: ldapConfig.searchBase,
+            searchFilter: "(uid={{username}})",
+            searchAttributes: ["uid", "givenName", "sn"],
+            ...(ldapConfig.caCert !== ""
+              ? {
+                  tlsOptions: {
+                    ca: [ldapConfig.caCert]
+                  }
                }
-              }
-            : {})
-        },
-        passReqToCallback: true
-      };
+              : {})
+          },
+          passReqToCallback: true
+        };

-      // eslint-disable-next-line
-      callback(null, opts);
+        // eslint-disable-next-line
+          done(null, opts);
+      } catch (err) {
+        logger.error(err);
+        // eslint-disable-next-line
+          done(err as Error);
+      }
    };

    process.nextTick(async () => {
@@ -403,8 +415,9 @@ export const ldapConfigServiceFactory = ({
  return {
    createLdapCfg,
    updateLdapCfg,
+    getLdapCfgWithPermissionCheck,
    getLdapCfg,
-    getLDAPConfiguration,
+    getLdapPassportOpts,
    ldapLogin
  };
 };
--- a/backend/src/ee/services/license/mocks/licence-fns.ts
+++ b/backend/src/ee/services/license/mocks/licence-fns.ts
@@ -17,7 +17,8 @@ export const getDefaultOnPremFeatures = () => {
    customAlerts: false,
    auditLogs: false,
    auditLogsRetentionDays: 0,
-    samlSSO: false,
+    samlSSO: true,
+    scim: true,
    status: null,
    trial_end: null,
    has_used_trial: true,
--- a/backend/src/ee/services/license/licence-fns.ts
+++ b/backend/src/ee/services/license/licence-fns.ts
@@ -23,8 +23,8 @@ export const getDefaultOnPremFeatures = (): TFeatureSet => ({
  customAlerts: false,
  auditLogs: false,
  auditLogsRetentionDays: 0,
-  samlSSO: false,
-  scim: false,
+  samlSSO: true,
+  scim: true,
  ldap: true,
  status: null,
  trial_end: null,
--- a/backend/src/ee/services/license/license-types.ts
+++ b/backend/src/ee/services/license/license-types.ts
@@ -24,8 +24,8 @@ export type TFeatureSet = {
  customAlerts: false;
  auditLogs: false;
  auditLogsRetentionDays: 0;
-  samlSSO: false;
-  scim: false;
+  samlSSO: true;
+  scim: true;
  ldap: true;
  status: null;
  trial_end: null;
--- a/backend/src/ee/services/saml-config/saml-config-service.ts
+++ b/backend/src/ee/services/saml-config/saml-config-service.ts
@@ -5,6 +5,7 @@ import {
  OrgMembershipRole,
  OrgMembershipStatus,
  SecretKeyEncoding,
+  TableName,
  TSamlConfigs,
  TSamlConfigsUpdate
 } from "@app/db/schemas";
@@ -31,7 +32,7 @@ import { TCreateSamlCfgDTO, TGetSamlCfgDTO, TSamlLoginDTO, TUpdateSamlCfgDTO } f

 type TSamlConfigServiceFactoryDep = {
  samlConfigDAL: TSamlConfigDALFactory;
-  userDAL: Pick<TUserDALFactory, "create" | "findUserByEmail" | "transaction" | "updateById">;
+  userDAL: Pick<TUserDALFactory, "create" | "findOne" | "transaction" | "updateById">;
  orgDAL: Pick<
    TOrgDALFactory,
    "createMembership" | "updateMembershipById" | "findMembership" | "findOrgById" | "findOne" | "updateById"
@@ -299,16 +300,30 @@ export const samlConfigServiceFactory = ({
    };
  };

-  const samlLogin = async ({ firstName, email, lastName, authProvider, orgId, relayState }: TSamlLoginDTO) => {
+  const samlLogin = async ({
+    username,
+    email,
+    firstName,
+    lastName,
+    authProvider,
+    orgId,
+    relayState
+  }: TSamlLoginDTO) => {
    const appCfg = getConfig();
-    let user = await userDAL.findUserByEmail(email);
+    let user = await userDAL.findOne({ username });

    const organization = await orgDAL.findOrgById(orgId);
    if (!organization) throw new BadRequestError({ message: "Org not found" });

    if (user) {
      await userDAL.transaction(async (tx) => {
-        const [orgMembership] = await orgDAL.findMembership({ userId: user.id, orgId }, { tx });
+        const [orgMembership] = await orgDAL.findMembership(
+          {
+            userId: user.id,
+            [`${TableName.OrgMembership}.orgId` as "id"]: orgId
+          },
+          { tx }
+        );
        if (!orgMembership) {
          await orgDAL.createMembership(
            {
@@ -334,7 +349,7 @@ export const samlConfigServiceFactory = ({
      user = await userDAL.transaction(async (tx) => {
        const newUser = await userDAL.create(
          {
-            username: email,
+            username,
            email,
            firstName,
            lastName,
--- a/backend/src/ee/services/saml-config/saml-config-types.ts
+++ b/backend/src/ee/services/saml-config/saml-config-types.ts
@@ -36,7 +36,8 @@ export type TGetSamlCfgDTO =
    };

 export type TSamlLoginDTO = {
-  email: string;
+  username: string;
+  email?: string;
  firstName: string;
  lastName?: string;
  authProvider: string;
--- a/backend/src/ee/services/scim/scim-fns.ts
+++ b/backend/src/ee/services/scim/scim-fns.ts
@@ -28,12 +28,12 @@ export const buildScimUser = ({
 }: {
  userId: string;
  username: string;
-  email: string;
+  email?: string | null;
  firstName: string;
  lastName: string;
  active: boolean;
 }): TScimUser => {
-  return {
+  const scimUser = {
    schemas: ["urn:ietf:params:scim:schemas:core:2.0:User"],
    id: userId,
    userName: username,
@@ -43,13 +43,15 @@ export const buildScimUser = ({
      middleName: null,
      familyName: lastName
    },
-    emails: [
-      {
-        primary: true,
-        value: email,
-        type: "work"
-      }
-    ],
+    emails: email
+      ? [
+          {
+            primary: true,
+            value: email,
+            type: "work"
+          }
+        ]
+      : [],
    active,
    groups: [],
    meta: {
@@ -57,4 +59,6 @@ export const buildScimUser = ({
      location: null
    }
  };
+
+  return scimUser;
 };
--- a/backend/src/ee/services/scim/scim-service.ts
+++ b/backend/src/ee/services/scim/scim-service.ts
@@ -1,7 +1,7 @@
 import { ForbiddenError } from "@casl/ability";
 import jwt from "jsonwebtoken";

-import { OrgMembershipRole, OrgMembershipStatus } from "@app/db/schemas";
+import { OrgMembershipRole, OrgMembershipStatus, TableName } from "@app/db/schemas";
 import { TScimDALFactory } from "@app/ee/services/scim/scim-dal";
 import { getConfig } from "@app/lib/config/env";
 import { BadRequestError, ScimRequestError, UnauthorizedError } from "@app/lib/errors";
@@ -146,7 +146,7 @@ export const scimServiceFactory = ({

    const users = await orgDAL.findMembership(
      {
-        orgId,
+        [`${TableName.OrgMembership}.orgId` as "id"]: orgId,
        ...parseFilter(filter)
      },
      findOpts
@@ -155,10 +155,10 @@ export const scimServiceFactory = ({
    const scimUsers = users.map(({ userId, username, firstName, lastName, email }) =>
      buildScimUser({
        userId: userId ?? "",
-        username: username ?? "",
+        username,
        firstName: firstName ?? "",
        lastName: lastName ?? "",
-        email: email ?? "",
+        email,
        active: true
      })
    );
@@ -174,7 +174,7 @@ export const scimServiceFactory = ({
    const [membership] = await orgDAL
      .findMembership({
        userId,
-        orgId
+        [`${TableName.OrgMembership}.orgId` as "id"]: orgId
      })
      .catch(() => {
        throw new ScimRequestError({
@@ -205,8 +205,7 @@ export const scimServiceFactory = ({
    });
  };

-  // TODO: update SCIM endpoints to add username
-  const createScimUser = async ({ firstName, lastName, email, orgId }: TCreateScimUserDTO) => {
+  const createScimUser = async ({ username, email, firstName, lastName, orgId }: TCreateScimUserDTO) => {
    const org = await orgDAL.findById(orgId);

    if (!org)
@@ -222,12 +221,18 @@ export const scimServiceFactory = ({
      });

    let user = await userDAL.findOne({
-      email
+      username
    });

    if (user) {
      await userDAL.transaction(async (tx) => {
-        const [orgMembership] = await orgDAL.findMembership({ userId: user.id, orgId }, { tx });
+        const [orgMembership] = await orgDAL.findMembership(
+          {
+            userId: user.id,
+            [`${TableName.OrgMembership}.orgId` as "id"]: orgId
+          },
+          { tx }
+        );
        if (orgMembership)
          throw new ScimRequestError({
            detail: "User already exists in the database",
@@ -251,7 +256,7 @@ export const scimServiceFactory = ({
      user = await userDAL.transaction(async (tx) => {
        const newUser = await userDAL.create(
          {
-            username: email,
+            username,
            email,
            firstName,
            lastName,
@@ -279,7 +284,7 @@ export const scimServiceFactory = ({
    await smtpService.sendMail({
      template: SmtpTemplates.ScimUserProvisioned,
      subjectLine: "Infisical organization invitation",
-      recipients: [email],
+      recipients: email ? [email] : [],
      substitutions: {
        organizationName: org.name,
        callback_url: `${appCfg.SITE_URL}/api/v1/sso/redirect/saml2/organizations/${org.slug}`
@@ -300,7 +305,7 @@ export const scimServiceFactory = ({
    const [membership] = await orgDAL
      .findMembership({
        userId,
-        orgId
+        [`${TableName.OrgMembership}.orgId` as "id"]: orgId
      })
      .catch(() => {
        throw new ScimRequestError({
@@ -348,7 +353,7 @@ export const scimServiceFactory = ({
    return buildScimUser({
      userId: membership.userId as string,
      username: membership.username,
-      email: membership.email ?? "",
+      email: membership.email,
      firstName: membership.firstName as string,
      lastName: membership.lastName as string,
      active
@@ -359,7 +364,7 @@ export const scimServiceFactory = ({
    const [membership] = await orgDAL
      .findMembership({
        userId,
-        orgId
+        [`${TableName.OrgMembership}.orgId` as "id"]: orgId
      })
      .catch(() => {
        throw new ScimRequestError({
@@ -394,7 +399,7 @@ export const scimServiceFactory = ({
    return buildScimUser({
      userId: membership.userId as string,
      username: membership.username,
-      email: membership.email ?? "",
+      email: membership.email,
      firstName: membership.firstName as string,
      lastName: membership.lastName as string,
      active
--- a/backend/src/ee/services/scim/scim-types.ts
+++ b/backend/src/ee/services/scim/scim-types.ts
@@ -32,7 +32,8 @@ export type TGetScimUserDTO = {
 };

 export type TCreateScimUserDTO = {
-  email: string;
+  username: string;
+  email?: string;
  firstName: string;
  lastName: string;
  orgId: string;
--- a/backend/src/services/auth/auth-login-service.ts
+++ b/backend/src/services/auth/auth-login-service.ts
@@ -301,7 +301,6 @@ export const authLoginServiceFactory = ({ userDAL, tokenService, smtpService }:
      {
        authTokenType: AuthTokenType.PROVIDER_TOKEN,
        userId: user.id,
-        // email: user.email,
        username: user.username,
        firstName: user.firstName,
        lastName: user.lastName,
--- a/backend/src/services/org/org-dal.ts
+++ b/backend/src/services/org/org-dal.ts
@@ -250,7 +250,9 @@ export const orgDALFactory = (db: TDbClient) => {
          db.ref("firstName").withSchema(TableName.Users),
          db.ref("lastName").withSchema(TableName.Users),
          db.ref("scimEnabled").withSchema(TableName.Organization)
-        );
+        )
+        .where({ isGhost: false });
+
      if (limit) void query.limit(limit);
      if (offset) void query.offset(offset);
      if (sort) {
--- a/frontend/src/views/Login/components/LDAPStep/LDAPStep.tsx
+++ b/frontend/src/views/Login/components/LDAPStep/LDAPStep.tsx
@@ -30,6 +30,15 @@ export const LDAPStep = ({
                password
            });
            
+            if (!nextUrl) {
+                createNotification({
+                    text: "Login unsuccessful. Double-check your credentials and try again.",
+                    type: "error"
+                });
+                
+                return;
+            }
+            
            createNotification({
                text: "Successfully logged in",
                type: "success"