From 929dc059c34bc2d6e55d902ca34699abaee738dc Mon Sep 17 00:00:00 2001 From: = Date: Sat, 31 Aug 2024 23:32:22 +0530 Subject: [PATCH 1/5] feat: updated scim user endpoint --- backend/package-lock.json | 38 ++++- backend/package.json | 2 + backend/src/ee/routes/v1/scim-router.ts | 150 +++++++++--------- backend/src/ee/services/scim/scim-fns.ts | 9 +- backend/src/ee/services/scim/scim-service.ts | 158 ++++++++++++------- backend/src/ee/services/scim/scim-types.ts | 22 ++- backend/src/lib/knex/scim.ts | 122 ++++++++++++++ backend/src/server/app.ts | 15 ++ backend/src/services/org/org-dal.ts | 63 ++++++++ 9 files changed, 426 insertions(+), 153 deletions(-) create mode 100644 backend/src/lib/knex/scim.ts diff --git a/backend/package-lock.json b/backend/package-lock.json index b3eef6bd1..d860616f1 100644 --- a/backend/package-lock.json +++ b/backend/package-lock.json @@ -79,6 +79,8 @@ "posthog-node": "^3.6.2", "probot": "^13.0.0", "safe-regex": "^2.1.1", + "scim-patch": "^0.8.3", + "scim2-parse-filter": "^0.2.10", "smee-client": "^2.0.0", "tedious": "^18.2.1", "tweetnacl": "^1.0.3", @@ -13040,12 +13042,12 @@ } }, "node_modules/micromatch": { - "version": "4.0.5", - "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.5.tgz", - "integrity": "sha512-DMy+ERcEW2q8Z2Po+WNXuw3c5YaUSFjAO5GsJqfEl7UjvtIuFKO6ZrKvcItdy98dwFI2N1tg3zNIdKaQT+aNdA==", + "version": "4.0.8", + "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.8.tgz", + "integrity": "sha512-PXwfBhYu0hBCPw8Dn0E+WDYb7af3dSLVWKi3HGv84IdF4TyFoC0ysxFd0Goxw7nSv4T/PzEJQxsYsEiFCKo2BA==", "dev": true, "dependencies": { - "braces": "^3.0.2", + "braces": "^3.0.3", "picomatch": "^2.3.1" }, "engines": { @@ -15495,6 +15497,34 @@ "resolved": "https://registry.npmjs.org/sax/-/sax-1.3.0.tgz", "integrity": "sha512-0s+oAmw9zLl1V1cS9BtZN7JAd0cW5e0QH4W3LWEK6a4LaLEA2OTpGYWDY+6XasBLtz6wkm3u1xRw95mRuJ59WA==" }, + "node_modules/scim-patch": { + "version": "0.8.3", + "resolved": "https://registry.npmjs.org/scim-patch/-/scim-patch-0.8.3.tgz", + "integrity": "sha512-3d0wD4THAt03Zgi08kCQwc+lvPJ2v4wwk41b0xViVa4gLYSgRUCmGkJNBsaE+yoKg0fufTOJCcSrufZaqYn/og==", + "dependencies": { + "@types/node": "^22.0.0", + "fast-deep-equal": "3.1.3", + "scim2-parse-filter": "0.2.10" + } + }, + "node_modules/scim-patch/node_modules/@types/node": { + "version": "22.5.1", + "resolved": "https://registry.npmjs.org/@types/node/-/node-22.5.1.tgz", + "integrity": "sha512-KkHsxej0j9IW1KKOOAA/XBA0z08UFSrRQHErzEfA3Vgq57eXIMYboIlHJuYIfd+lwCQjtKqUu3UnmKbtUc9yRw==", + "dependencies": { + "undici-types": "~6.19.2" + } + }, + "node_modules/scim-patch/node_modules/undici-types": { + "version": "6.19.8", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-6.19.8.tgz", + "integrity": "sha512-ve2KP6f/JnbPBFyobGHuerC9g1FYGn/F8n1LWTwNxCEzd6IfqTwUQcNXgEtmmQ6DlRrC1hrSrBnCZPokRrDHjw==" + }, + "node_modules/scim2-parse-filter": { + "version": "0.2.10", + "resolved": "https://registry.npmjs.org/scim2-parse-filter/-/scim2-parse-filter-0.2.10.tgz", + "integrity": "sha512-k5TgGSuQEbR4jXRgw/GPAYVL9fMp1pWA2abLF5z3q9IGWSuZTqbrZBOSUezvc+rtViXr+czSZjg3eAN4QSTvxQ==" + }, "node_modules/secure-json-parse": { "version": "2.7.0", "resolved": "https://registry.npmjs.org/secure-json-parse/-/secure-json-parse-2.7.0.tgz", diff --git a/backend/package.json b/backend/package.json index fadec14ee..967ccfb78 100644 --- a/backend/package.json +++ b/backend/package.json @@ -177,6 +177,8 @@ "posthog-node": "^3.6.2", "probot": "^13.0.0", "safe-regex": "^2.1.1", + "scim-patch": "^0.8.3", + "scim2-parse-filter": "^0.2.10", "smee-client": "^2.0.0", "tedious": "^18.2.1", "tweetnacl": "^1.0.3", diff --git a/backend/src/ee/routes/v1/scim-router.ts b/backend/src/ee/routes/v1/scim-router.ts index 45e89de4a..2c6d2e620 100644 --- a/backend/src/ee/routes/v1/scim-router.ts +++ b/backend/src/ee/routes/v1/scim-router.ts @@ -5,22 +5,30 @@ import { readLimit, writeLimit } from "@app/server/config/rateLimiter"; import { verifyAuth } from "@app/server/plugins/auth/verify-auth"; import { AuthMode } from "@app/services/auth/auth-type"; -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) { - const error = err as Error; - done(error, undefined); - } - }); +const ScimUserSchema = z.object({ + schemas: z.array(z.string()), + id: z.string().trim(), + userName: z.string().trim(), + name: z + .object({ + familyName: z.string().trim().optional(), + givenName: z.string().trim().optional() + }) + .optional(), + emails: z + .array( + z.object({ + primary: z.boolean(), + value: z.string().email(), + type: z.string().trim() + }) + ) + .optional(), + displayName: z.string().trim(), + active: z.boolean() +}); +export const registerScimRouter = async (server: FastifyZodProvider) => { server.route({ url: "/scim-tokens", method: "POST", @@ -127,25 +135,7 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }), response: { 200: z.object({ - Resources: z.array( - z.object({ - id: z.string().trim(), - userName: z.string().trim(), - name: z.object({ - familyName: z.string().trim(), - givenName: z.string().trim() - }), - emails: z.array( - z.object({ - primary: z.boolean(), - value: z.string(), - type: z.string().trim() - }) - ), - displayName: z.string().trim(), - active: z.boolean() - }) - ), + Resources: z.array(ScimUserSchema), itemsPerPage: z.number(), schemas: z.array(z.string()), startIndex: z.number(), @@ -173,23 +163,7 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { orgMembershipId: z.string().trim() }), response: { - 201: z.object({ - schemas: z.array(z.string()), - id: z.string().trim(), - userName: z.string().trim(), - name: z.object({ - familyName: z.string().trim(), - givenName: z.string().trim() - }), - emails: z.array( - z.object({ - primary: z.boolean(), - value: z.string(), - type: z.string().trim() - }) - ), - displayName: z.string().trim(), - active: z.boolean(), + 200: ScimUserSchema.extend({ groups: z.array( z.object({ value: z.string().trim(), @@ -216,10 +190,12 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { body: z.object({ schemas: z.array(z.string()), userName: z.string().trim(), - name: z.object({ - familyName: z.string().trim(), - givenName: z.string().trim() - }), + name: z + .object({ + familyName: z.string().trim().optional(), + givenName: z.string().trim().optional() + }) + .optional(), emails: z .array( z.object({ @@ -229,28 +205,10 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }) ) .optional(), - // displayName: z.string().trim(), - active: z.boolean() + active: z.boolean().default(true) }), response: { - 200: z.object({ - schemas: z.array(z.string()), - id: z.string().trim(), - userName: z.string().trim(), - name: z.object({ - familyName: z.string().trim(), - givenName: z.string().trim() - }), - emails: z.array( - z.object({ - primary: z.boolean(), - value: z.string().email(), - type: z.string().trim() - }) - ), - displayName: z.string().trim(), - active: z.boolean() - }) + 200: ScimUserSchema } }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), @@ -260,8 +218,8 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { const user = await req.server.services.scim.createScimUser({ externalId: req.body.userName, email: primaryEmail, - firstName: req.body.name.givenName, - lastName: req.body.name.familyName, + firstName: req.body?.name?.givenName, + lastName: req.body?.name?.familyName, orgId: req.permission.orgId }); @@ -291,6 +249,44 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { } }); + server.route({ + url: "/Users/:orgMembershipId", + method: "PATCH", + schema: { + params: z.object({ + orgMembershipId: z.string().trim() + }), + body: z.object({ + schemas: z.array(z.string()), + Operations: z.array( + z.union([ + z.object({ + op: z.union([z.literal("remove"), z.literal("Remove")]), + path: z.string().trim() + }), + z.object({ + op: z.union([z.literal("add"), z.literal("Add"), z.literal("replace"), z.literal("Replace")]), + path: z.string().trim().optional(), + value: z.any() + }) + ]) + ) + }), + response: { + 200: ScimUserSchema + } + }, + onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), + handler: async (req) => { + const user = await req.server.services.scim.updateScimUser({ + orgMembershipId: req.params.orgMembershipId, + orgId: req.permission.orgId, + operations: req.body.Operations + }); + + return user; + } + }); server.route({ url: "/Groups", method: "POST", diff --git a/backend/src/ee/services/scim/scim-fns.ts b/backend/src/ee/services/scim/scim-fns.ts index b7454dc90..ae03cace8 100644 --- a/backend/src/ee/services/scim/scim-fns.ts +++ b/backend/src/ee/services/scim/scim-fns.ts @@ -45,7 +45,9 @@ export const buildScimUser = ({ firstName, lastName, groups = [], - active + active, + createdAt, + updatedAt }: { orgMembershipId: string; username: string; @@ -57,6 +59,8 @@ export const buildScimUser = ({ display: string; }[]; active: boolean; + createdAt: Date; + updatedAt: Date; }): TScimUser => { const scimUser = { schemas: ["urn:ietf:params:scim:schemas:core:2.0:User"], @@ -81,7 +85,8 @@ export const buildScimUser = ({ groups, meta: { resourceType: "User", - location: null + created: createdAt, + lastModified: updatedAt } }; diff --git a/backend/src/ee/services/scim/scim-service.ts b/backend/src/ee/services/scim/scim-service.ts index 0cb9e3e82..2df75450a 100644 --- a/backend/src/ee/services/scim/scim-service.ts +++ b/backend/src/ee/services/scim/scim-service.ts @@ -1,6 +1,7 @@ import { ForbiddenError } from "@casl/ability"; import slugify from "@sindresorhus/slugify"; import jwt from "jsonwebtoken"; +import { scimPatch } from "scim-patch"; import { OrgMembershipRole, OrgMembershipStatus, TableName, TOrgMemberships, TUsers } from "@app/db/schemas"; import { TGroupDALFactory } from "@app/ee/services/group/group-dal"; @@ -64,12 +65,18 @@ type TScimServiceFactoryDep = { scimDAL: Pick; userDAL: Pick< TUserDALFactory, - "find" | "findOne" | "create" | "transaction" | "findUserEncKeyByUserIdsBatch" | "findById" + "find" | "findOne" | "create" | "transaction" | "findUserEncKeyByUserIdsBatch" | "findById" | "updateById" >; userAliasDAL: Pick; orgDAL: Pick< TOrgDALFactory, - "createMembership" | "findById" | "findMembership" | "deleteMembershipById" | "transaction" | "updateMembershipById" + | "createMembership" + | "findById" + | "findMembership" + | "findMembershipWithScimFilter" + | "deleteMembershipById" + | "transaction" + | "updateMembershipById" >; orgMembershipDAL: Pick; projectDAL: Pick; @@ -193,7 +200,12 @@ export const scimServiceFactory = ({ }; // SCIM server endpoints - const listScimUsers = async ({ startIndex, limit, filter, orgId }: TListScimUsersDTO): Promise => { + const listScimUsers = async ({ + startIndex = 0, + limit = 100, + filter, + orgId + }: TListScimUsersDTO): Promise => { const org = await orgDAL.findById(orgId); if (!org.scimEnabled) @@ -207,23 +219,20 @@ export const scimServiceFactory = ({ ...(limit && { limit }) }; - const users = await orgDAL.findMembership( - { - [`${TableName.OrgMembership}.orgId` as "id"]: orgId, - ...parseScimFilter(filter) - }, - findOpts - ); + const users = await orgDAL.findMembershipWithScimFilter(orgId, filter, findOpts); - const scimUsers = users.map(({ id, externalId, username, firstName, lastName, email, isActive }) => - buildScimUser({ - orgMembershipId: id ?? "", - username: externalId ?? username, - firstName: firstName ?? "", - lastName: lastName ?? "", - email, - active: isActive - }) + const scimUsers = users.map( + ({ id, externalId, username, firstName, lastName, email, isActive, createdAt, updatedAt }) => + buildScimUser({ + orgMembershipId: id ?? "", + username: externalId ?? username, + firstName: firstName ?? "", + lastName: lastName ?? "", + email, + active: isActive, + createdAt, + updatedAt + }) ); return buildScimUserList({ @@ -273,7 +282,9 @@ export const scimServiceFactory = ({ groups: groupMembershipsInOrg.map((group) => ({ value: group.groupId, display: group.groupName - })) + })), + createdAt: membership.createdAt, + updatedAt: membership.updatedAt }); }; @@ -349,7 +360,11 @@ export const scimServiceFactory = ({ } if (!user) { - const uniqueUsername = await normalizeUsername(`${firstName}-${lastName}`, userDAL); + const uniqueUsername = await normalizeUsername( + // external id is username + `${firstName}-${lastName}`, + userDAL + ); user = await userDAL.create( { username: serverCfg.trustSamlEmails ? email : uniqueUsername, @@ -430,10 +445,13 @@ export const scimServiceFactory = ({ firstName: createdUser.firstName, lastName: createdUser.lastName, email: createdUser.email ?? "", - active: createdOrgMembership.isActive + active: createdOrgMembership.isActive, + createdAt: createdOrgMembership.createdAt, + updatedAt: createdOrgMembership.updatedAt }); }; + // partial const updateScimUser = async ({ orgMembershipId, orgId, operations }: TUpdateScimUserDTO) => { const [membership] = await orgDAL .findMembership({ @@ -459,37 +477,51 @@ export const scimServiceFactory = ({ status: 403 }); - let active = true; - - operations.forEach((operation) => { - if (operation.op.toLowerCase() === "replace") { - if (operation.path === "active" && operation.value === "False") { - // azure scim op format - active = false; - } else if (typeof operation.value === "object" && operation.value.active === false) { - // okta scim op format - active = false; - } - } - }); - - if (!active) { - await orgMembershipDAL.updateById(membership.id, { - isActive: false - }); - } - - return buildScimUser({ + const scimUser = buildScimUser({ orgMembershipId: membership.id, - username: membership.externalId ?? membership.username, email: membership.email, - firstName: membership.firstName, lastName: membership.lastName, - active + firstName: membership.firstName, + active: membership.isActive, + username: membership.username, + createdAt: membership.createdAt, + updatedAt: membership.updatedAt }); + scimPatch(scimUser, operations); + + const serverCfg = await getServerCfg(); + await userDAL.transaction(async (tx) => { + await orgMembershipDAL.updateById( + membership.id, + { + isActive: scimUser.active + }, + tx + ); + const hasEmailChanged = scimUser.emails[0].value !== membership.email; + await userDAL.updateById( + membership.userId, + { + firstName: scimUser.name.givenName, + email: scimUser.emails[0].value, + lastName: scimUser.name.familyName, + isEmailVerified: hasEmailChanged ? serverCfg.trustSamlEmails : true + }, + tx + ); + }); + + return scimUser; }; - const replaceScimUser = async ({ orgMembershipId, active, orgId }: TReplaceScimUserDTO) => { + const replaceScimUser = async ({ + orgMembershipId, + active, + orgId, + lastName, + firstName, + email + }: TReplaceScimUserDTO) => { const [membership] = await orgDAL .findMembership({ [`${TableName.OrgMembership}.id` as "id"]: orgMembershipId, @@ -514,15 +546,27 @@ export const scimServiceFactory = ({ status: 403 }); - await orgMembershipDAL.updateById(membership.id, { - isActive: active + const serverCfg = await getServerCfg(); + await userDAL.transaction(async (tx) => { + await orgMembershipDAL.updateById( + membership.id, + { + isActive: active + }, + tx + ); + await userDAL.updateById( + membership.userId, + { + firstName, + email, + lastName, + isEmailVerified: serverCfg.trustSamlEmails + }, + tx + ); }); - const groupMembershipsInOrg = await userGroupMembershipDAL.findGroupMembershipsByUserIdInOrg( - membership.userId, - orgId - ); - return buildScimUser({ orgMembershipId: membership.id, username: membership.externalId ?? membership.username, @@ -530,10 +574,8 @@ export const scimServiceFactory = ({ firstName: membership.firstName, lastName: membership.lastName, active, - groups: groupMembershipsInOrg.map((group) => ({ - value: group.groupId, - display: group.groupName - })) + createdAt: membership.createdAt, + updatedAt: membership.updatedAt }); }; diff --git a/backend/src/ee/services/scim/scim-types.ts b/backend/src/ee/services/scim/scim-types.ts index 410b6557c..153d8a1bc 100644 --- a/backend/src/ee/services/scim/scim-types.ts +++ b/backend/src/ee/services/scim/scim-types.ts @@ -1,3 +1,5 @@ +import { ScimPatchOperation } from "scim-patch"; + import { TOrgPermission } from "@app/lib/types"; export type TCreateScimTokenDTO = { @@ -34,29 +36,24 @@ export type TGetScimUserDTO = { export type TCreateScimUserDTO = { externalId: string; email?: string; - firstName: string; - lastName: string; + firstName?: string; + lastName?: string; orgId: string; }; export type TUpdateScimUserDTO = { orgMembershipId: string; orgId: string; - operations: { - op: string; - path?: string; - value?: - | string - | { - active: boolean; - }; - }[]; + operations: ScimPatchOperation[]; }; export type TReplaceScimUserDTO = { orgMembershipId: string; active: boolean; orgId: string; + email?: string; + firstName?: string; + lastName?: string; }; export type TDeleteScimUserDTO = { @@ -166,7 +163,8 @@ export type TScimUser = { }[]; meta: { resourceType: string; - location: null; + created: Date; + lastModified: Date; }; }; diff --git a/backend/src/lib/knex/scim.ts b/backend/src/lib/knex/scim.ts new file mode 100644 index 000000000..81e87f656 --- /dev/null +++ b/backend/src/lib/knex/scim.ts @@ -0,0 +1,122 @@ +import { Knex } from "knex"; +import { Compare, Filter, parse } from "scim2-parse-filter"; + +const appendParentToGroupingOperator = (parentPath: string, filter: Filter) => { + if (filter.op !== "[]" && filter.op !== "and" && filter.op !== "or" && filter.op !== "not") { + return { ...filter, attrPath: `${parentPath}.${(filter as Compare).attrPath}` }; + } + return filter; +}; + +export const generateKnexQueryFromScim = ( + rootQuery: Knex.QueryBuilder, + rootScimFilter: string, + getAttributeField: (attr: string) => string | null +) => { + const scimRootFilterAst = parse(rootScimFilter); + const stack = [ + { + scimFilterAst: scimRootFilterAst, + query: rootQuery + } + ]; + + while (stack.length) { + const { scimFilterAst, query } = stack.pop()!; + switch (scimFilterAst.op) { + case "eq": { + const attrPath = getAttributeField(scimFilterAst.attrPath); + console.log(attrPath, scimFilterAst.compValue); + if (attrPath) void query.where(attrPath, scimFilterAst.compValue); + break; + } + case "pr": { + const attrPath = getAttributeField(scimFilterAst.attrPath); + if (attrPath) void query.whereNotNull(attrPath); + break; + } + case "gt": { + const attrPath = getAttributeField(scimFilterAst.attrPath); + if (attrPath) void query.where(attrPath, ">", scimFilterAst.compValue); + break; + } + case "ge": { + const attrPath = getAttributeField(scimFilterAst.attrPath); + if (attrPath) void query.where(attrPath, ">=", scimFilterAst.compValue); + break; + } + case "lt": { + const attrPath = getAttributeField(scimFilterAst.attrPath); + if (attrPath) void query.where(attrPath, "<", scimFilterAst.compValue); + break; + } + case "le": { + const attrPath = getAttributeField(scimFilterAst.attrPath); + if (attrPath) void query.where(attrPath, "<=", scimFilterAst.compValue); + break; + } + case "sw": { + const attrPath = getAttributeField(scimFilterAst.attrPath); + if (attrPath) void query.whereILike(attrPath, `${scimFilterAst.compValue}%`); + break; + } + case "ew": { + const attrPath = getAttributeField(scimFilterAst.attrPath); + if (attrPath) void query.whereILike(attrPath, `%${scimFilterAst.compValue}`); + break; + } + case "co": { + const attrPath = getAttributeField(scimFilterAst.attrPath); + if (attrPath) void query.whereILike(attrPath, `%${scimFilterAst.compValue}%`); + break; + } + case "ne": { + const attrPath = getAttributeField(scimFilterAst.attrPath); + if (attrPath) void query.whereNot(attrPath, "=", scimFilterAst.compValue); + break; + } + case "and": { + void query.andWhere((subQueryBuilder) => { + scimFilterAst.filters.forEach((el) => { + stack.push({ + query: subQueryBuilder, + scimFilterAst: el + }); + }); + }); + break; + } + case "or": { + void query.orWhere((subQueryBuilder) => { + scimFilterAst.filters.forEach((el) => { + stack.push({ + query: subQueryBuilder, + scimFilterAst: el + }); + }); + }); + break; + } + case "not": { + void query.whereNot((subQueryBuilder) => { + stack.push({ + query: subQueryBuilder, + scimFilterAst: scimFilterAst.filter + }); + }); + break; + } + case "[]": { + void query.whereNot((subQueryBuilder) => { + stack.push({ + query: subQueryBuilder, + scimFilterAst: appendParentToGroupingOperator(scimFilterAst.attrPath, scimFilterAst.valFilter) + }); + }); + break; + } + default: + break; + } + } +}; diff --git a/backend/src/server/app.ts b/backend/src/server/app.ts index ee8acec0f..1ab8bc3fd 100644 --- a/backend/src/server/app.ts +++ b/backend/src/server/app.ts @@ -49,6 +49,21 @@ export const main = async ({ db, smtp, logger, queue, keyStore }: TMain) => { server.setValidatorCompiler(validatorCompiler); server.setSerializerCompiler(serializerCompiler); + 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) { + const error = err as Error; + done(error, undefined); + } + }); + try { await server.register(cookie, { secret: appCfg.COOKIE_SECRET_SIGN_KEY diff --git a/backend/src/services/org/org-dal.ts b/backend/src/services/org/org-dal.ts index d7c6ba31a..d84dd1afd 100644 --- a/backend/src/services/org/org-dal.ts +++ b/backend/src/services/org/org-dal.ts @@ -12,6 +12,7 @@ import { } from "@app/db/schemas"; import { DatabaseError } from "@app/lib/errors"; import { buildFindFilter, ormify, selectAllTableCols, TFindFilter, TFindOpt, withTransaction } from "@app/lib/knex"; +import { generateKnexQueryFromScim } from "@app/lib/knex/scim"; export type TOrgDALFactory = ReturnType; @@ -280,6 +281,67 @@ export const orgDALFactory = (db: TDbClient) => { .select( selectAllTableCols(TableName.OrgMembership), db.ref("email").withSchema(TableName.Users), + db.ref("isEmailVerified").withSchema(TableName.Users), + db.ref("username").withSchema(TableName.Users), + db.ref("firstName").withSchema(TableName.Users), + db.ref("lastName").withSchema(TableName.Users), + db.ref("scimEnabled").withSchema(TableName.Organization), + db.ref("externalId").withSchema(TableName.UserAliases) + ) + .where({ isGhost: false }); + + if (limit) void query.limit(limit); + if (offset) void query.offset(offset); + if (sort) { + void query.orderBy(sort.map(([column, order, nulls]) => ({ column: column as string, order, nulls }))); + } + const res = await query; + return res; + } catch (error) { + throw new DatabaseError({ error, name: "Find one" }); + } + }; + + const findMembershipWithScimFilter = async ( + orgId: string, + scimFilter: string | undefined, + { offset, limit, sort, tx }: TFindOpt = {} + ) => { + try { + const query = (tx || db.replicaNode())(TableName.OrgMembership) + // eslint-disable-next-line + .where(`${TableName.OrgMembership}.orgId`, orgId) + .where((qb) => { + if (scimFilter) { + void generateKnexQueryFromScim(qb, scimFilter, (attrPath) => { + switch (attrPath) { + case "active": + return `${TableName.OrgMembership}.isActive`; + case "userName": + return `${TableName.UserAliases}.externalId`; + case "name.givenName": + return `${TableName.Users}.firstName`; + case "name.familyName": + return `${TableName.Users}.lastName`; + case "email.value": + return `${TableName.Users}.email`; + default: + return null; + } + }); + } + }) + .join(TableName.Users, `${TableName.Users}.id`, `${TableName.OrgMembership}.userId`) + .join(TableName.Organization, `${TableName.Organization}.id`, `${TableName.OrgMembership}.orgId`) + .leftJoin(TableName.UserAliases, function joinUserAlias() { + this.on(`${TableName.UserAliases}.userId`, "=", `${TableName.OrgMembership}.userId`) + .andOn(`${TableName.UserAliases}.orgId`, "=", `${TableName.OrgMembership}.orgId`) + .andOn(`${TableName.UserAliases}.aliasType`, "=", (tx || db).raw("?", ["saml"])); + }) + .select( + selectAllTableCols(TableName.OrgMembership), + db.ref("email").withSchema(TableName.Users), + db.ref("isEmailVerified").withSchema(TableName.Users), db.ref("username").withSchema(TableName.Users), db.ref("firstName").withSchema(TableName.Users), db.ref("lastName").withSchema(TableName.Users), @@ -314,6 +376,7 @@ export const orgDALFactory = (db: TDbClient) => { updateById, deleteById, findMembership, + findMembershipWithScimFilter, createMembership, updateMembershipById, deleteMembershipById, From 8f48a64fd658bd4897de28c9bf7126d1b59d4d88 Mon Sep 17 00:00:00 2001 From: = Date: Sun, 1 Sep 2024 18:55:17 +0530 Subject: [PATCH 2/5] feat: finished fixing scim group --- backend/src/ee/routes/v1/scim-router.ts | 557 +++++++++---------- backend/src/ee/services/scim/scim-fns.ts | 11 +- backend/src/ee/services/scim/scim-service.ts | 269 +++++---- backend/src/ee/services/scim/scim-types.ts | 32 +- backend/src/lib/knex/scim.ts | 1 - 5 files changed, 394 insertions(+), 476 deletions(-) diff --git a/backend/src/ee/routes/v1/scim-router.ts b/backend/src/ee/routes/v1/scim-router.ts index 2c6d2e620..c4e03094c 100644 --- a/backend/src/ee/routes/v1/scim-router.ts +++ b/backend/src/ee/routes/v1/scim-router.ts @@ -28,6 +28,23 @@ const ScimUserSchema = z.object({ active: z.boolean() }); +const ScimGroupSchema = z.object({ + schemas: z.array(z.string()), + id: z.string().trim(), + displayName: z.string().trim(), + members: z + .array( + z.object({ + value: z.string(), + display: z.string().optional() + }) + ) + .optional(), + meta: z.object({ + resourceType: z.string().trim() + }) +}); + export const registerScimRouter = async (server: FastifyZodProvider) => { server.route({ url: "/scim-tokens", @@ -249,304 +266,6 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { } }); - server.route({ - url: "/Users/:orgMembershipId", - method: "PATCH", - schema: { - params: z.object({ - orgMembershipId: z.string().trim() - }), - body: z.object({ - schemas: z.array(z.string()), - Operations: z.array( - z.union([ - z.object({ - op: z.union([z.literal("remove"), z.literal("Remove")]), - path: z.string().trim() - }), - z.object({ - op: z.union([z.literal("add"), z.literal("Add"), z.literal("replace"), z.literal("Replace")]), - path: z.string().trim().optional(), - value: z.any() - }) - ]) - ) - }), - response: { - 200: ScimUserSchema - } - }, - onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), - handler: async (req) => { - const user = await req.server.services.scim.updateScimUser({ - orgMembershipId: req.params.orgMembershipId, - orgId: req.permission.orgId, - operations: req.body.Operations - }); - - return user; - } - }); - server.route({ - url: "/Groups", - method: "POST", - schema: { - body: z.object({ - schemas: z.array(z.string()), - displayName: z.string().trim(), - members: z - .array( - z.object({ - value: z.string(), - display: z.string() - }) - ) - .optional() // okta-specific - }), - response: { - 200: z.object({ - schemas: z.array(z.string()), - id: z.string().trim(), - displayName: z.string().trim(), - members: z - .array( - z.object({ - value: z.string(), - display: z.string() - }) - ) - .optional(), - meta: z.object({ - resourceType: z.string().trim() - }) - }) - } - }, - onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), - handler: async (req) => { - const group = await req.server.services.scim.createScimGroup({ - orgId: req.permission.orgId, - ...req.body - }); - - return group; - } - }); - - server.route({ - url: "/Groups", - method: "GET", - schema: { - querystring: z.object({ - startIndex: z.coerce.number().default(1), - count: z.coerce.number().default(20), - filter: z.string().trim().optional() - }), - response: { - 200: z.object({ - Resources: z.array( - z.object({ - schemas: z.array(z.string()), - id: z.string().trim(), - displayName: z.string().trim(), - members: z.array( - z.object({ - value: z.string(), - display: z.string() - }) - ), - meta: z.object({ - resourceType: z.string().trim() - }) - }) - ), - itemsPerPage: z.number(), - schemas: z.array(z.string()), - startIndex: z.number(), - totalResults: z.number() - }) - } - }, - onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), - handler: async (req) => { - const groups = await req.server.services.scim.listScimGroups({ - orgId: req.permission.orgId, - startIndex: req.query.startIndex, - filter: req.query.filter, - limit: req.query.count - }); - - return groups; - } - }); - - server.route({ - url: "/Groups/:groupId", - method: "GET", - schema: { - params: z.object({ - groupId: z.string().trim() - }), - response: { - 200: z.object({ - schemas: z.array(z.string()), - id: z.string().trim(), - displayName: z.string().trim(), - members: z.array( - z.object({ - value: z.string(), - display: z.string() - }) - ), - meta: z.object({ - resourceType: z.string().trim() - }) - }) - } - }, - onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), - handler: async (req) => { - const group = await req.server.services.scim.getScimGroup({ - groupId: req.params.groupId, - orgId: req.permission.orgId - }); - return group; - } - }); - - server.route({ - url: "/Groups/:groupId", - method: "PUT", - schema: { - params: z.object({ - groupId: z.string().trim() - }), - body: z.object({ - schemas: z.array(z.string()), - id: z.string().trim(), - displayName: z.string().trim(), - members: z.array( - z.object({ - value: z.string(), - display: z.string() - }) - ) - }), - response: { - 200: z.object({ - schemas: z.array(z.string()), - id: z.string().trim(), - displayName: z.string().trim(), - members: z.array( - z.object({ - value: z.string(), - display: z.string() - }) - ), - meta: z.object({ - resourceType: z.string().trim() - }) - }) - } - }, - onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), - handler: async (req) => { - const group = await req.server.services.scim.updateScimGroupNamePut({ - groupId: req.params.groupId, - orgId: req.permission.orgId, - ...req.body - }); - - return group; - } - }); - - server.route({ - url: "/Groups/:groupId", - method: "PATCH", - schema: { - params: z.object({ - groupId: z.string().trim() - }), - body: z.object({ - schemas: z.array(z.string()), - Operations: z.array( - z.union([ - z.object({ - op: z.union([z.literal("replace"), z.literal("Replace")]), - value: z.object({ - id: z.string().trim(), - displayName: z.string().trim() - }) - }), - z.object({ - op: z.union([z.literal("remove"), z.literal("Remove")]), - path: z.string().trim() - }), - z.object({ - op: z.union([z.literal("add"), z.literal("Add")]), - path: z.string().trim(), - value: z.array( - z.object({ - value: z.string().trim(), - display: z.string().trim().optional() - }) - ) - }) - ]) - ) - }), - response: { - 200: z.object({ - schemas: z.array(z.string()), - id: z.string().trim(), - displayName: z.string().trim(), - members: z.array( - z.object({ - value: z.string(), - display: z.string() - }) - ), - meta: z.object({ - resourceType: z.string().trim() - }) - }) - } - }, - onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), - handler: async (req) => { - const group = await req.server.services.scim.updateScimGroupNamePatch({ - groupId: req.params.groupId, - orgId: req.permission.orgId, - operations: req.body.Operations - }); - - return group; - } - }); - - server.route({ - url: "/Groups/:groupId", - method: "DELETE", - schema: { - params: z.object({ - groupId: z.string().trim() - }), - response: { - 200: z.object({}) - } - }, - onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), - handler: async (req) => { - const group = await req.server.services.scim.deleteScimGroup({ - groupId: req.params.groupId, - orgId: req.permission.orgId - }); - - return group; - } - }); - server.route({ url: "/Users/:orgMembershipId", method: "PUT", @@ -558,10 +277,12 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { schemas: z.array(z.string()), id: z.string().trim(), userName: z.string().trim(), - name: z.object({ - familyName: z.string().trim(), - givenName: z.string().trim() - }), + name: z + .object({ + familyName: z.string().trim().optional(), + givenName: z.string().trim().optional() + }) + .optional(), displayName: z.string().trim(), active: z.boolean() }), @@ -602,4 +323,236 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { return user; } }); + + server.route({ + url: "/Users/:orgMembershipId", + method: "PATCH", + schema: { + params: z.object({ + orgMembershipId: z.string().trim() + }), + body: z.object({ + schemas: z.array(z.string()), + Operations: z.array( + z.union([ + z.object({ + op: z.union([z.literal("remove"), z.literal("Remove")]), + path: z.string().trim(), + value: z + .object({ + value: z.string() + }) + .array() + .optional() + }), + z.object({ + op: z.union([z.literal("add"), z.literal("Add"), z.literal("replace"), z.literal("Replace")]), + path: z.string().trim().optional(), + value: z.any().optional() + }) + ]) + ) + }), + response: { + 200: ScimUserSchema + } + }, + onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), + handler: async (req) => { + const user = await req.server.services.scim.updateScimUser({ + orgMembershipId: req.params.orgMembershipId, + orgId: req.permission.orgId, + operations: req.body.Operations + }); + + return user; + } + }); + server.route({ + url: "/Groups", + method: "POST", + schema: { + body: z.object({ + schemas: z.array(z.string()), + displayName: z.string().trim(), + members: z + .array( + z.object({ + value: z.string(), + display: z.string() + }) + ) + .optional() + }), + response: { + 200: ScimGroupSchema + } + }, + onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), + handler: async (req) => { + const group = await req.server.services.scim.createScimGroup({ + orgId: req.permission.orgId, + ...req.body + }); + + return group; + } + }); + + server.route({ + url: "/Groups", + method: "GET", + schema: { + querystring: z.object({ + startIndex: z.coerce.number().default(1), + count: z.coerce.number().default(20), + filter: z.string().trim().optional(), + excludedAttributes: z.string().trim().optional() + }), + response: { + 200: z.object({ + Resources: z.array(ScimGroupSchema), + itemsPerPage: z.number(), + schemas: z.array(z.string()), + startIndex: z.number(), + totalResults: z.number() + }) + } + }, + onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), + handler: async (req) => { + const groups = await req.server.services.scim.listScimGroups({ + orgId: req.permission.orgId, + startIndex: req.query.startIndex, + filter: req.query.filter, + limit: req.query.count, + isMembersExcluded: req.query.excludedAttributes === "members" + }); + + return groups; + } + }); + + server.route({ + url: "/Groups/:groupId", + method: "GET", + schema: { + params: z.object({ + groupId: z.string().trim() + }), + response: { + 200: ScimGroupSchema + } + }, + onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), + handler: async (req) => { + const group = await req.server.services.scim.getScimGroup({ + groupId: req.params.groupId, + orgId: req.permission.orgId + }); + return group; + } + }); + + server.route({ + url: "/Groups/:groupId", + method: "PUT", + schema: { + params: z.object({ + groupId: z.string().trim() + }), + body: z.object({ + schemas: z.array(z.string()), + id: z.string().trim(), + displayName: z.string().trim(), + members: z.array( + z.object({ + value: z.string(), + display: z.string() + }) + ) + }), + response: { + 200: ScimGroupSchema + } + }, + onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), + handler: async (req) => { + const group = await req.server.services.scim.replaceScimGroup({ + groupId: req.params.groupId, + orgId: req.permission.orgId, + ...req.body + }); + + return group; + } + }); + + server.route({ + url: "/Groups/:groupId", + method: "PATCH", + schema: { + params: z.object({ + groupId: z.string().trim() + }), + body: z.object({ + schemas: z.array(z.string()), + Operations: z.array( + z.union([ + z.object({ + op: z.union([z.literal("remove"), z.literal("Remove")]), + path: z.string().trim(), + value: z + .object({ + value: z.string() + }) + .array() + .optional() + }), + z.object({ + op: z.union([z.literal("add"), z.literal("Add"), z.literal("replace"), z.literal("Replace")]), + path: z.string().trim().optional(), + value: z.any() + }) + ]) + ) + }), + response: { + 200: ScimGroupSchema + } + }, + onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), + handler: async (req) => { + console.log(JSON.stringify(req.body, null, 4)); + const group = await req.server.services.scim.updateScimGroup({ + groupId: req.params.groupId, + orgId: req.permission.orgId, + operations: req.body.Operations + }); + console.log(group); + return group; + } + }); + + server.route({ + url: "/Groups/:groupId", + method: "DELETE", + schema: { + params: z.object({ + groupId: z.string().trim() + }), + response: { + 200: z.object({}) + } + }, + onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), + handler: async (req) => { + const group = await req.server.services.scim.deleteScimGroup({ + groupId: req.params.groupId, + orgId: req.permission.orgId + }); + + return group; + } + }); }; diff --git a/backend/src/ee/services/scim/scim-fns.ts b/backend/src/ee/services/scim/scim-fns.ts index ae03cace8..8de22ac54 100644 --- a/backend/src/ee/services/scim/scim-fns.ts +++ b/backend/src/ee/services/scim/scim-fns.ts @@ -114,14 +114,18 @@ export const buildScimGroupList = ({ export const buildScimGroup = ({ groupId, name, - members + members, + updatedAt, + createdAt }: { groupId: string; name: string; members: { value: string; - display: string; + display?: string; }[]; + createdAt: Date; + updatedAt: Date; }): TScimGroup => { const scimGroup = { schemas: ["urn:ietf:params:scim:schemas:core:2.0:Group"], @@ -130,7 +134,8 @@ export const buildScimGroup = ({ members, meta: { resourceType: "Group", - location: null + created: createdAt, + lastModified: updatedAt } }; diff --git a/backend/src/ee/services/scim/scim-service.ts b/backend/src/ee/services/scim/scim-service.ts index 2df75450a..956919cf1 100644 --- a/backend/src/ee/services/scim/scim-service.ts +++ b/backend/src/ee/services/scim/scim-service.ts @@ -10,7 +10,6 @@ import { TUserGroupMembershipDALFactory } from "@app/ee/services/group/user-grou import { TScimDALFactory } from "@app/ee/services/scim/scim-dal"; import { getConfig } from "@app/lib/config/env"; import { BadRequestError, ScimRequestError, UnauthorizedError } from "@app/lib/errors"; -import { logger } from "@app/lib/logger"; import { alphaNumericNanoId } from "@app/lib/nanoid"; import { TOrgPermission } from "@app/lib/types"; import { AuthTokenType } from "@app/services/auth/auth-type"; @@ -33,14 +32,7 @@ import { TLicenseServiceFactory } from "../license/license-service"; import { OrgPermissionActions, OrgPermissionSubjects } from "../permission/org-permission"; import { TPermissionServiceFactory } from "../permission/permission-service"; import { TProjectUserAdditionalPrivilegeDALFactory } from "../project-user-additional-privilege/project-user-additional-privilege-dal"; -import { - buildScimGroup, - buildScimGroupList, - buildScimUser, - buildScimUserList, - extractScimValueFromPath, - parseScimFilter -} from "./scim-fns"; +import { buildScimGroup, buildScimGroupList, buildScimUser, buildScimUserList, parseScimFilter } from "./scim-fns"; import { TCreateScimGroupDTO, TCreateScimTokenDTO, @@ -612,7 +604,7 @@ export const scimServiceFactory = ({ return {}; // intentionally return empty object upon success }; - const listScimGroups = async ({ orgId, startIndex, limit, filter }: TListScimGroupsDTO) => { + const listScimGroups = async ({ orgId, startIndex, limit, filter, isMembersExcluded }: TListScimGroupsDTO) => { const plan = await licenseService.getPlan(orgId); if (!plan.groups) throw new BadRequestError({ @@ -645,6 +637,21 @@ export const scimServiceFactory = ({ ); const scimGroups: TScimGroup[] = []; + if (isMembersExcluded) { + return buildScimGroupList({ + scimGroups: groups.map((group) => + buildScimGroup({ + groupId: group.id, + name: group.name, + members: [], + createdAt: group.createdAt, + updatedAt: group.updatedAt + }) + ), + startIndex, + limit + }); + } for await (const group of groups) { const members = await userGroupMembershipDAL.findGroupMembershipsByGroupIdInOrg(group.id, orgId); @@ -654,7 +661,9 @@ export const scimServiceFactory = ({ members: members.map((member) => ({ value: member.orgMembershipId, display: `${member.firstName ?? ""} ${member.lastName ?? ""}` - })) + })), + createdAt: group.createdAt, + updatedAt: group.updatedAt }); scimGroups.push(scimGroup); } @@ -738,7 +747,9 @@ export const scimServiceFactory = ({ members: orgMemberships.map(({ id, firstName, lastName }) => ({ value: id, display: `${firstName} ${lastName}` - })) + })), + createdAt: newGroup.group.createdAt, + updatedAt: newGroup.group.updatedAt }); }; @@ -781,31 +792,17 @@ export const scimServiceFactory = ({ members: orgMemberships.map(({ id, firstName, lastName }) => ({ value: id, display: `${firstName} ${lastName}` - })) + })), + createdAt: group.createdAt, + updatedAt: group.updatedAt }); }; - const updateScimGroupNamePut = async ({ groupId, orgId, displayName, members }: TUpdateScimGroupNamePutDTO) => { - const plan = await licenseService.getPlan(orgId); - if (!plan.groups) - throw new BadRequestError({ - message: "Failed to update SCIM group due to plan restriction. Upgrade plan to update SCIM group." - }); - - const org = await orgDAL.findById(orgId); - if (!org) { - throw new ScimRequestError({ - detail: "Organization Not Found", - status: 404 - }); - } - - if (!org.scimEnabled) - throw new ScimRequestError({ - detail: "SCIM is disabled for the organization", - status: 403 - }); - + const $replaceGroupDAL = async ( + groupId: string, + orgId: string, + { displayName, members = [] }: { displayName: string; members: { value: string }[] } + ) => { const updatedGroup = await groupDAL.transaction(async (tx) => { const [group] = await groupDAL.update( { @@ -824,74 +821,96 @@ export const scimServiceFactory = ({ }); } - if (members) { - const orgMemberships = await orgMembershipDAL.find({ - $in: { - id: members.map((member) => member.value) - } + const orgMemberships = members.length + ? await orgMembershipDAL.find({ + $in: { + id: members.map((member) => member.value) + } + }) + : []; + + const membersIdsSet = new Set(orgMemberships.map((orgMembership) => orgMembership.userId)); + const userGroupMembers = await userGroupMembershipDAL.find({ + groupId: group.id + }); + const directMemberUserIds = userGroupMembers.filter((el) => !el.isPending).map((membership) => membership.userId); + + const pendingGroupAdditionsUserIds = userGroupMembers + .filter((el) => el.isPending) + .map((pendingGroupAddition) => pendingGroupAddition.userId); + + const allMembersUserIds = directMemberUserIds.concat(pendingGroupAdditionsUserIds); + const allMembersUserIdsSet = new Set(allMembersUserIds); + + const toAddUserIds = orgMemberships.filter((member) => !allMembersUserIdsSet.has(member.userId as string)); + const toRemoveUserIds = allMembersUserIds.filter((userId) => !membersIdsSet.has(userId)); + + if (toAddUserIds.length) { + await addUsersToGroupByUserIds({ + group, + userIds: toAddUserIds.map((member) => member.userId as string), + userDAL, + userGroupMembershipDAL, + orgDAL, + groupProjectDAL, + projectKeyDAL, + projectDAL, + projectBotDAL, + tx }); + } - const membersIdsSet = new Set(orgMemberships.map((orgMembership) => orgMembership.userId)); - - const directMemberUserIds = ( - await userGroupMembershipDAL.find({ - groupId: group.id, - isPending: false - }) - ).map((membership) => membership.userId); - - const pendingGroupAdditionsUserIds = ( - await userGroupMembershipDAL.find({ - groupId: group.id, - isPending: true - }) - ).map((pendingGroupAddition) => pendingGroupAddition.userId); - - const allMembersUserIds = directMemberUserIds.concat(pendingGroupAdditionsUserIds); - const allMembersUserIdsSet = new Set(allMembersUserIds); - - const toAddUserIds = orgMemberships.filter((member) => !allMembersUserIdsSet.has(member.userId as string)); - const toRemoveUserIds = allMembersUserIds.filter((userId) => !membersIdsSet.has(userId)); - - if (toAddUserIds.length) { - await addUsersToGroupByUserIds({ - group, - userIds: toAddUserIds.map((member) => member.userId as string), - userDAL, - userGroupMembershipDAL, - orgDAL, - groupProjectDAL, - projectKeyDAL, - projectDAL, - projectBotDAL, - tx - }); - } - - if (toRemoveUserIds.length) { - await removeUsersFromGroupByUserIds({ - group, - userIds: toRemoveUserIds, - userDAL, - userGroupMembershipDAL, - groupProjectDAL, - projectKeyDAL, - tx - }); - } + if (toRemoveUserIds.length) { + await removeUsersFromGroupByUserIds({ + group, + userIds: toRemoveUserIds, + userDAL, + userGroupMembershipDAL, + groupProjectDAL, + projectKeyDAL, + tx + }); } return group; }); + return updatedGroup; + }; + + const replaceScimGroup = async ({ groupId, orgId, displayName, members }: TUpdateScimGroupNamePutDTO) => { + const plan = await licenseService.getPlan(orgId); + if (!plan.groups) + throw new BadRequestError({ + message: "Failed to update SCIM group due to plan restriction. Upgrade plan to update SCIM group." + }); + + const org = await orgDAL.findById(orgId); + if (!org) { + throw new ScimRequestError({ + detail: "Organization Not Found", + status: 404 + }); + } + + if (!org.scimEnabled) + throw new ScimRequestError({ + detail: "SCIM is disabled for the organization", + status: 403 + }); + + const updatedGroup = await $replaceGroupDAL(groupId, orgId, { displayName, members }); + return buildScimGroup({ groupId: updatedGroup.id, name: updatedGroup.name, - members + members, + updatedAt: updatedGroup.updatedAt, + createdAt: updatedGroup.createdAt }); }; - const updateScimGroupNamePatch = async ({ groupId, orgId, operations }: TUpdateScimGroupNamePatchDTO) => { + const updateScimGroup = async ({ groupId, orgId, operations }: TUpdateScimGroupNamePatchDTO) => { const plan = await licenseService.getPlan(orgId); if (!plan.groups) throw new BadRequestError({ @@ -913,7 +932,7 @@ export const scimServiceFactory = ({ status: 403 }); - let group = await groupDAL.findOne({ + const group = await groupDAL.findOne({ id: groupId, orgId }); @@ -925,64 +944,28 @@ export const scimServiceFactory = ({ }); } - for await (const operation of operations) { - 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) - } - }); - - 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"); - } - } 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 - }); - } - } - const members = await userGroupMembershipDAL.findGroupMembershipsByGroupIdInOrg(group.id, orgId); - - return buildScimGroup({ + const scimGroup = buildScimGroup({ groupId: group.id, name: group.name, members: members.map((member) => ({ + value: member.orgMembershipId + })), + createdAt: group.createdAt, + updatedAt: group.updatedAt + }); + scimPatch(scimGroup, operations); + // remove members is a weird case not following scim convention + await $replaceGroupDAL(groupId, orgId, { displayName: scimGroup.displayName, members: scimGroup.members }); + + const updatedScimMembers = await userGroupMembershipDAL.findGroupMembershipsByGroupIdInOrg(group.id, orgId); + return { + ...scimGroup, + members: updatedScimMembers.map((member) => ({ value: member.orgMembershipId, display: `${member.firstName ?? ""} ${member.lastName ?? ""}` })) - }); + }; }; const deleteScimGroup = async ({ groupId, orgId }: TDeleteScimGroupDTO) => { @@ -1058,8 +1041,8 @@ export const scimServiceFactory = ({ createScimGroup, getScimGroup, deleteScimGroup, - updateScimGroupNamePut, - updateScimGroupNamePatch, + replaceScimGroup, + updateScimGroup, fnValidateScimToken }; }; diff --git a/backend/src/ee/services/scim/scim-types.ts b/backend/src/ee/services/scim/scim-types.ts index 153d8a1bc..9caa44a03 100644 --- a/backend/src/ee/services/scim/scim-types.ts +++ b/backend/src/ee/services/scim/scim-types.ts @@ -66,6 +66,7 @@ export type TListScimGroupsDTO = { filter?: string; limit: number; orgId: string; + isMembersExcluded?: boolean; }; export type TListScimGroups = { @@ -104,31 +105,7 @@ export type TUpdateScimGroupNamePutDTO = { export type TUpdateScimGroupNamePatchDTO = { groupId: string; orgId: string; - 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" | "Replace"; - value: { - id: string; - displayName: string; - }; -}; - -type TRemoveOp = { - op: "remove" | "Remove"; - path: string; -}; - -type TAddOp = { - op: "add" | "Add"; - path: string; - value: { - value: string; - display?: string; - }[]; + operations: ScimPatchOperation[]; }; export type TDeleteScimGroupDTO = { @@ -174,10 +151,11 @@ export type TScimGroup = { displayName: string; members: { value: string; - display: string; + display?: string; }[]; meta: { resourceType: string; - location: null; + created: Date; + lastModified: Date; }; }; diff --git a/backend/src/lib/knex/scim.ts b/backend/src/lib/knex/scim.ts index 81e87f656..530a7d45a 100644 --- a/backend/src/lib/knex/scim.ts +++ b/backend/src/lib/knex/scim.ts @@ -26,7 +26,6 @@ export const generateKnexQueryFromScim = ( switch (scimFilterAst.op) { case "eq": { const attrPath = getAttributeField(scimFilterAst.attrPath); - console.log(attrPath, scimFilterAst.compValue); if (attrPath) void query.where(attrPath, scimFilterAst.compValue); break; } From 58bab4d163891cbbf23e3dafc8beee2b829ee06e Mon Sep 17 00:00:00 2001 From: = Date: Mon, 2 Sep 2024 13:42:38 +0530 Subject: [PATCH 3/5] feat: resolved some more missing corner case in scim --- backend/src/ee/routes/v1/scim-router.ts | 28 +++++++++++-------- backend/src/ee/services/scim/scim-service.ts | 29 +++++++++++--------- backend/src/ee/services/scim/scim-types.ts | 1 + 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/backend/src/ee/routes/v1/scim-router.ts b/backend/src/ee/routes/v1/scim-router.ts index c4e03094c..72d2b241e 100644 --- a/backend/src/ee/routes/v1/scim-router.ts +++ b/backend/src/ee/routes/v1/scim-router.ts @@ -180,14 +180,7 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { orgMembershipId: z.string().trim() }), response: { - 200: ScimUserSchema.extend({ - groups: z.array( - z.object({ - value: z.string().trim(), - display: z.string().trim() - }) - ) - }) + 200: ScimUserSchema } }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), @@ -284,6 +277,15 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }) .optional(), displayName: z.string().trim(), + emails: z + .array( + z.object({ + primary: z.boolean(), + value: z.string().email(), + type: z.string().trim() + }) + ) + .optional(), active: z.boolean() }), response: { @@ -315,10 +317,15 @@ 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.replaceScimUser({ orgMembershipId: req.params.orgMembershipId, orgId: req.permission.orgId, - active: req.body.active + firstName: req.body?.name?.givenName, + lastName: req.body?.name?.familyName, + active: req.body?.active, + email: primaryEmail, + externalId: req.body.userName }); return user; } @@ -450,6 +457,7 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { groupId: req.params.groupId, orgId: req.permission.orgId }); + return group; } }); @@ -523,13 +531,11 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), handler: async (req) => { - console.log(JSON.stringify(req.body, null, 4)); const group = await req.server.services.scim.updateScimGroup({ groupId: req.params.groupId, orgId: req.permission.orgId, operations: req.body.Operations }); - console.log(group); return group; } }); diff --git a/backend/src/ee/services/scim/scim-service.ts b/backend/src/ee/services/scim/scim-service.ts index 956919cf1..ec9ce5aa4 100644 --- a/backend/src/ee/services/scim/scim-service.ts +++ b/backend/src/ee/services/scim/scim-service.ts @@ -59,7 +59,7 @@ type TScimServiceFactoryDep = { TUserDALFactory, "find" | "findOne" | "create" | "transaction" | "findUserEncKeyByUserIdsBatch" | "findById" | "updateById" >; - userAliasDAL: Pick; + userAliasDAL: Pick; orgDAL: Pick< TOrgDALFactory, | "createMembership" @@ -259,11 +259,6 @@ export const scimServiceFactory = ({ status: 403 }); - const groupMembershipsInOrg = await userGroupMembershipDAL.findGroupMembershipsByUserIdInOrg( - membership.userId, - orgId - ); - return buildScimUser({ orgMembershipId: membership.id, username: membership.externalId ?? membership.username, @@ -271,10 +266,6 @@ export const scimServiceFactory = ({ firstName: membership.firstName, lastName: membership.lastName, active: membership.isActive, - groups: groupMembershipsInOrg.map((group) => ({ - value: group.groupId, - display: group.groupName - })), createdAt: membership.createdAt, updatedAt: membership.updatedAt }); @@ -475,7 +466,7 @@ export const scimServiceFactory = ({ lastName: membership.lastName, firstName: membership.firstName, active: membership.isActive, - username: membership.username, + username: membership.externalId ?? membership.username, createdAt: membership.createdAt, updatedAt: membership.updatedAt }); @@ -512,7 +503,8 @@ export const scimServiceFactory = ({ orgId, lastName, firstName, - email + email, + externalId }: TReplaceScimUserDTO) => { const [membership] = await orgDAL .findMembership({ @@ -540,6 +532,17 @@ export const scimServiceFactory = ({ const serverCfg = await getServerCfg(); await userDAL.transaction(async (tx) => { + await userAliasDAL.update( + { + orgId, + aliasType: UserAliasType.SAML, + userId: membership.userId + }, + { + externalId + }, + tx + ); await orgMembershipDAL.updateById( membership.id, { @@ -561,7 +564,7 @@ export const scimServiceFactory = ({ return buildScimUser({ orgMembershipId: membership.id, - username: membership.externalId ?? membership.username, + username: externalId, email: membership.email, firstName: membership.firstName, lastName: membership.lastName, diff --git a/backend/src/ee/services/scim/scim-types.ts b/backend/src/ee/services/scim/scim-types.ts index 9caa44a03..4d23b5f0b 100644 --- a/backend/src/ee/services/scim/scim-types.ts +++ b/backend/src/ee/services/scim/scim-types.ts @@ -54,6 +54,7 @@ export type TReplaceScimUserDTO = { email?: string; firstName?: string; lastName?: string; + externalId: string; }; export type TDeleteScimUserDTO = { From fe31d44d22d9f899b9aee018918889bb5858e017 Mon Sep 17 00:00:00 2001 From: = Date: Mon, 2 Sep 2024 13:50:26 +0530 Subject: [PATCH 4/5] feat: made scim user default permission as no access in org --- backend/src/ee/services/scim/scim-service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/ee/services/scim/scim-service.ts b/backend/src/ee/services/scim/scim-service.ts index ec9ce5aa4..9352d09cc 100644 --- a/backend/src/ee/services/scim/scim-service.ts +++ b/backend/src/ee/services/scim/scim-service.ts @@ -316,7 +316,7 @@ export const scimServiceFactory = ({ userId: userAlias.userId, inviteEmail: email, orgId, - role: OrgMembershipRole.Member, + role: OrgMembershipRole.NoAccess, status: user.isAccepted ? OrgMembershipStatus.Accepted : OrgMembershipStatus.Invited, // if user is fully completed, then set status to accepted, otherwise set it to invited so we can update it later isActive: true }, From 23483ab7e1cc889bbabb479ded1d92e3fab068f3 Mon Sep 17 00:00:00 2001 From: = Date: Mon, 2 Sep 2024 13:55:56 +0530 Subject: [PATCH 5/5] feat: removed non rfc related groups in user scim resource --- backend/src/ee/routes/v1/scim-router.ts | 8 +------- backend/src/ee/services/scim/scim-fns.ts | 6 ------ backend/src/ee/services/scim/scim-types.ts | 4 ---- 3 files changed, 1 insertion(+), 17 deletions(-) diff --git a/backend/src/ee/routes/v1/scim-router.ts b/backend/src/ee/routes/v1/scim-router.ts index 72d2b241e..427c77fa7 100644 --- a/backend/src/ee/routes/v1/scim-router.ts +++ b/backend/src/ee/routes/v1/scim-router.ts @@ -305,13 +305,7 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }) ), displayName: z.string().trim(), - active: z.boolean(), - groups: z.array( - z.object({ - value: z.string().trim(), - display: z.string().trim() - }) - ) + active: z.boolean() }) } }, diff --git a/backend/src/ee/services/scim/scim-fns.ts b/backend/src/ee/services/scim/scim-fns.ts index 8de22ac54..3ade1a117 100644 --- a/backend/src/ee/services/scim/scim-fns.ts +++ b/backend/src/ee/services/scim/scim-fns.ts @@ -44,7 +44,6 @@ export const buildScimUser = ({ email, firstName, lastName, - groups = [], active, createdAt, updatedAt @@ -54,10 +53,6 @@ export const buildScimUser = ({ email?: string | null; firstName: string | null | undefined; lastName: string | null | undefined; - groups?: { - value: string; - display: string; - }[]; active: boolean; createdAt: Date; updatedAt: Date; @@ -82,7 +77,6 @@ export const buildScimUser = ({ ] : [], active, - groups, meta: { resourceType: "User", created: createdAt, diff --git a/backend/src/ee/services/scim/scim-types.ts b/backend/src/ee/services/scim/scim-types.ts index 4d23b5f0b..5099e4ca0 100644 --- a/backend/src/ee/services/scim/scim-types.ts +++ b/backend/src/ee/services/scim/scim-types.ts @@ -135,10 +135,6 @@ export type TScimUser = { type: string; }[]; active: boolean; - groups: { - value: string; - display: string; - }[]; meta: { resourceType: string; created: Date;