Merge pull request #2336 from akhilmhdh/fix/scim-error

fix: resolved scim group update failing

backend/src/ee/routes/v1/scim-router.ts

@@ -9,7 +9,10 @@ export const registerScimRouter = async (server: FastifyZodProvider) => {
   server.addContentTypeParser("application/scim+json", { parseAs: "string" }, (_, body, done) => {
     try {
       const strBody = body instanceof Buffer ? body.toString() : body;
-
+      if (!strBody) {
+        done(null, undefined);
+        return;
+      }
       const json: unknown = JSON.parse(strBody);
       done(null, json);
     } catch (err) {
@@ -474,18 +477,18 @@ export const registerScimRouter = async (server: FastifyZodProvider) => {
         Operations: z.array(
           z.union([
             z.object({
-              op: z.literal("replace"),
+              op: z.union([z.literal("replace"), z.literal("Replace")]),
               value: z.object({
                 id: z.string().trim(),
                 displayName: z.string().trim()
               })
             }),
             z.object({
-              op: z.literal("remove"),
+              op: z.union([z.literal("remove"), z.literal("Remove")]),
               path: z.string().trim()
             }),
             z.object({
-              op: z.literal("add"),
+              op: z.union([z.literal("add"), z.literal("Add")]),
               path: z.string().trim(),
               value: z.array(
                 z.object({

backend/src/ee/services/scim/scim-fns.ts

@@ -50,8 +50,8 @@ export const buildScimUser = ({
   orgMembershipId: string;
   username: string;
   email?: string | null;
-  firstName: string;
-  lastName: string;
+  firstName: string | null | undefined;
+  lastName: string | null | undefined;
   groups?: {
     value: string;
     display: string;
@@ -64,9 +64,9 @@ export const buildScimUser = ({
     userName: username,
     displayName: `${firstName} ${lastName}`,
     name: {
-      givenName: firstName,
+      givenName: firstName || "",
       middleName: null,
-      familyName: lastName
+      familyName: lastName || ""
     },
     emails: email
       ? [

backend/src/ee/services/scim/scim-service.ts

@@ -267,8 +267,8 @@ export const scimServiceFactory = ({
       orgMembershipId: membership.id,
       username: membership.externalId ?? membership.username,
       email: membership.email ?? "",
-      firstName: membership.firstName as string,
-      lastName: membership.lastName as string,
+      firstName: membership.firstName,
+      lastName: membership.lastName,
       active: membership.isActive,
       groups: groupMembershipsInOrg.map((group) => ({
         value: group.groupId,
@@ -427,8 +427,8 @@ export const scimServiceFactory = ({
     return buildScimUser({
       orgMembershipId: createdOrgMembership.id,
       username: externalId,
-      firstName: createdUser.firstName as string,
-      lastName: createdUser.lastName as string,
+      firstName: createdUser.firstName,
+      lastName: createdUser.lastName,
       email: createdUser.email ?? "",
       active: createdOrgMembership.isActive
     });
@@ -483,8 +483,8 @@ export const scimServiceFactory = ({
       orgMembershipId: membership.id,
       username: membership.externalId ?? membership.username,
       email: membership.email,
-      firstName: membership.firstName as string,
-      lastName: membership.lastName as string,
+      firstName: membership.firstName,
+      lastName: membership.lastName,
       active
     });
   };
@@ -527,8 +527,8 @@ export const scimServiceFactory = ({
       orgMembershipId: membership.id,
       username: membership.externalId ?? membership.username,
       email: membership.email,
-      firstName: membership.firstName as string,
-      lastName: membership.lastName as string,
+      firstName: membership.firstName,
+      lastName: membership.lastName,
       active,
       groups: groupMembershipsInOrg.map((group) => ({
         value: group.groupId,
@@ -884,59 +884,50 @@ export const scimServiceFactory = ({
     }
 
     for await (const operation of operations) {
-      switch (operation.op) {
-        case "replace": {
-          group = await groupDAL.updateById(group.id, {
-            name: operation.value.displayName
+      if (operation.op === "replace" || operation.op === "Replace") {
+        group = await groupDAL.updateById(group.id, {
+          name: operation.value.displayName
+        });
+      } else if (operation.op === "add" || operation.op === "Add") {
+        try {
+          const orgMemberships = await orgMembershipDAL.find({
+            $in: {
+              id: operation.value.map((member) => member.value)
+            }
           });
-          break;
-        }
-        case "add": {
-          try {
-            const orgMemberships = await orgMembershipDAL.find({
-              $in: {
-                id: operation.value.map((member) => member.value)
-              }
-            });
 
-            await addUsersToGroupByUserIds({
-              group,
-              userIds: orgMemberships.map((membership) => membership.userId as string),
-              userDAL,
-              userGroupMembershipDAL,
-              orgDAL,
-              groupProjectDAL,
-              projectKeyDAL,
-              projectDAL,
-              projectBotDAL
-            });
-          } catch {
-            logger.info("Repeat SCIM user-group add operation");
-          }
-
-          break;
-        }
-        case "remove": {
-          const orgMembershipId = extractScimValueFromPath(operation.path);
-          if (!orgMembershipId) throw new ScimRequestError({ detail: "Invalid path value", status: 400 });
-          const orgMembership = await orgMembershipDAL.findById(orgMembershipId);
-          if (!orgMembership) throw new ScimRequestError({ detail: "Org Membership Not Found", status: 400 });
-          await removeUsersFromGroupByUserIds({
+          await addUsersToGroupByUserIds({
             group,
-            userIds: [orgMembership.userId as string],
+            userIds: orgMemberships.map((membership) => membership.userId as string),
             userDAL,
             userGroupMembershipDAL,
+            orgDAL,
             groupProjectDAL,
-            projectKeyDAL
-          });
-          break;
-        }
-        default: {
-          throw new ScimRequestError({
-            detail: "Invalid Operation",
-            status: 400
+            projectKeyDAL,
+            projectDAL,
+            projectBotDAL
           });
+        } catch {
+          logger.info("Repeat SCIM user-group add operation");
         }
+      } else if (operation.op === "remove" || operation.op === "Remove") {
+        const orgMembershipId = extractScimValueFromPath(operation.path);
+        if (!orgMembershipId) throw new ScimRequestError({ detail: "Invalid path value", status: 400 });
+        const orgMembership = await orgMembershipDAL.findById(orgMembershipId);
+        if (!orgMembership) throw new ScimRequestError({ detail: "Org Membership Not Found", status: 400 });
+        await removeUsersFromGroupByUserIds({
+          group,
+          userIds: [orgMembership.userId as string],
+          userDAL,
+          userGroupMembershipDAL,
+          groupProjectDAL,
+          projectKeyDAL
+        });
+      } else {
+        throw new ScimRequestError({
+          detail: "Invalid Operation",
+          status: 400
+        });
       }
     }
 

backend/src/ee/services/scim/scim-types.ts

@@ -110,8 +110,10 @@ export type TUpdateScimGroupNamePatchDTO = {
   operations: (TRemoveOp | TReplaceOp | TAddOp)[];
 };
 
+// akhilmhdh: I know, this is done due to lack of time. Need to change later to support as normalized rather than like this
+// Forgive akhil blame tony
 type TReplaceOp = {
-  op: "replace";
+  op: "replace" | "Replace";
   value: {
     id: string;
     displayName: string;
@@ -119,12 +121,12 @@ type TReplaceOp = {
 };
 
 type TRemoveOp = {
-  op: "remove";
+  op: "remove" | "Remove";
   path: string;
 };
 
 type TAddOp = {
-  op: "add";
+  op: "add" | "Add";
   path: string;
   value: {
     value: string;