From b2a976f3d4aa49129cccee3800fe90cba5265a6b Mon Sep 17 00:00:00 2001 From: Tuan Dang Date: Sun, 28 Apr 2024 21:58:24 -0700 Subject: [PATCH] Update groups CRUD SCIM to use orgMembershipId --- backend/src/ee/routes/v1/scim-router.ts | 18 +--- .../saml-config/saml-config-service.ts | 2 +- backend/src/ee/services/scim/scim-fns.ts | 6 +- backend/src/ee/services/scim/scim-service.ts | 98 +++++++++---------- backend/src/ee/services/scim/scim-types.ts | 2 +- backend/src/server/routes/index.ts | 1 + backend/src/services/org/org-fns.ts | 69 +++++++++---- backend/src/services/org/org-service.ts | 53 +++------- backend/src/services/user/user-fns.ts | 2 +- 9 files changed, 120 insertions(+), 131 deletions(-) diff --git a/backend/src/ee/routes/v1/scim-router.ts b/backend/src/ee/routes/v1/scim-router.ts index ed2ce92db..7283b8fe1 100644 --- a/backend/src/ee/routes/v1/scim-router.ts +++ b/backend/src/ee/routes/v1/scim-router.ts @@ -152,7 +152,6 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), handler: async (req) => { - console.log("GET /Users req.query: ", req.query); const users = await req.server.services.scim.listScimUsers({ startIndex: req.query.startIndex, limit: req.query.count, @@ -193,7 +192,6 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), handler: async (req) => { - console.log(`GET /Users/${req.params.orgMembershipId}`); const user = await req.server.services.scim.getScimUser({ orgMembershipId: req.params.orgMembershipId, orgId: req.permission.orgId @@ -248,8 +246,6 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), handler: async (req) => { - console.log("POST /Users req.body: ", req.body); - const primaryEmail = req.body.emails?.find((email) => email.primary)?.value; const user = await req.server.services.scim.createScimUser({ @@ -277,7 +273,6 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), handler: async (req) => { - console.log(`DELETE /Users/${req.params.orgMembershipId}`); const user = await req.server.services.scim.deleteScimUser({ orgMembershipId: req.params.orgMembershipId, orgId: req.permission.orgId @@ -324,7 +319,6 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), handler: async (req) => { - console.log("POST /Groups req.body: ", req.body); const group = await req.server.services.scim.createScimGroup({ orgId: req.permission.orgId, ...req.body @@ -365,10 +359,9 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), handler: async (req) => { - console.log("GET /Groups req.query: ", req.query); const groups = await req.server.services.scim.listScimGroups({ orgId: req.permission.orgId, - offset: req.query.startIndex, + startIndex: req.query.startIndex, limit: req.query.count }); @@ -402,7 +395,6 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), handler: async (req) => { - console.log(`GET /Groups/${req.params.groupId}`); const group = await req.server.services.scim.getScimGroup({ groupId: req.params.groupId, orgId: req.permission.orgId @@ -424,10 +416,10 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { displayName: z.string().trim(), members: z.array( z.object({ - value: z.string(), // infisical userId + value: z.string(), // infisical orgMembershipId display: z.string() }) - ) // note: is this where members are added to group? + ) }), response: { 200: z.object({ @@ -448,7 +440,6 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), handler: async (req) => { - console.log(`PUT /Groups/${req.params.groupId} req.body: `, req.body); const group = await req.server.services.scim.updateScimGroupNamePut({ groupId: req.params.groupId, orgId: req.permission.orgId, @@ -510,7 +501,6 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), handler: async (req) => { - console.log(`PATCH /Groups/${req.params.groupId} req.body: `, req.body); const group = await req.server.services.scim.updateScimGroupNamePatch({ groupId: req.params.groupId, orgId: req.permission.orgId, @@ -534,7 +524,6 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), handler: async (req) => { - console.log(`DELETE /Groups/${req.params.groupId}`); const group = await req.server.services.scim.deleteScimGroup({ groupId: req.params.groupId, orgId: req.permission.orgId @@ -585,7 +574,6 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }, onRequest: verifyAuth([AuthMode.SCIM_TOKEN]), handler: async (req) => { - console.log(`PUT /Users/${req.params.orgMembershipId} req.body: `, req.body); const user = await req.server.services.scim.replaceScimUser({ orgMembershipId: req.params.orgMembershipId, orgId: req.permission.orgId, diff --git a/backend/src/ee/services/saml-config/saml-config-service.ts b/backend/src/ee/services/saml-config/saml-config-service.ts index 2a7a6a60a..1dddffa29 100644 --- a/backend/src/ee/services/saml-config/saml-config-service.ts +++ b/backend/src/ee/services/saml-config/saml-config-service.ts @@ -369,7 +369,7 @@ export const samlConfigServiceFactory = ({ }); } else { user = await userDAL.transaction(async (tx) => { - const uniqueUsername = await normalizeUsername(externalId, userDAL); + const uniqueUsername = await normalizeUsername(`${firstName ?? ""}-${lastName ?? ""}`, userDAL); const newUser = await userDAL.create( { username: uniqueUsername, diff --git a/backend/src/ee/services/scim/scim-fns.ts b/backend/src/ee/services/scim/scim-fns.ts index 8668ed5e1..ec54a4d1f 100644 --- a/backend/src/ee/services/scim/scim-fns.ts +++ b/backend/src/ee/services/scim/scim-fns.ts @@ -65,18 +65,18 @@ export const buildScimUser = ({ export const buildScimGroupList = ({ scimGroups, - offset, + startIndex, limit }: { scimGroups: TScimGroup[]; - offset: number; + startIndex: number; limit: number; }): TListScimGroups => { return { Resources: scimGroups, itemsPerPage: limit, schemas: ["urn:ietf:params:scim:api:messages:2.0:ListResponse"], - startIndex: offset, + startIndex, totalResults: scimGroups.length }; }; diff --git a/backend/src/ee/services/scim/scim-service.ts b/backend/src/ee/services/scim/scim-service.ts index 6700e7f05..a9a78c886 100644 --- a/backend/src/ee/services/scim/scim-service.ts +++ b/backend/src/ee/services/scim/scim-service.ts @@ -14,7 +14,7 @@ import { TOrgPermission } from "@app/lib/types"; import { AuthTokenType } from "@app/services/auth/auth-type"; import { TGroupProjectDALFactory } from "@app/services/group-project/group-project-dal"; import { TOrgDALFactory } from "@app/services/org/org-dal"; -import { deleteOrgMembership } from "@app/services/org/org-fns"; +import { deleteOrgMembershipFn } from "@app/services/org/org-fns"; import { TOrgMembershipDALFactory } from "@app/services/org-membership/org-membership-dal"; import { TProjectDALFactory } from "@app/services/project/project-dal"; import { TProjectBotDALFactory } from "@app/services/project-bot/project-bot-dal"; @@ -62,7 +62,7 @@ type TScimServiceFactoryDep = { >; orgMembershipDAL: TOrgMembershipDALFactory; // TODO: Pick projectDAL: Pick; - projectMembershipDAL: Pick; + projectMembershipDAL: Pick; groupDAL: Pick< TGroupDALFactory, "create" | "findOne" | "findAllGroupMembers" | "update" | "delete" | "findGroups" | "transaction" @@ -71,7 +71,7 @@ type TScimServiceFactoryDep = { userGroupMembershipDAL: TUserGroupMembershipDALFactory; // TODO: Pick projectKeyDAL: Pick; projectBotDAL: Pick; - licenseService: Pick; + licenseService: Pick; permissionService: Pick; smtpService: Pick; }; @@ -174,12 +174,6 @@ export const scimServiceFactory = ({ // SCIM server endpoints const listScimUsers = async ({ startIndex, limit, filter, orgId }: TListScimUsersDTO): Promise => { - console.log("listScimUsers args: ", { - startIndex, - limit, - filter, - orgId - }); // done const org = await orgDAL.findById(orgId); if (!org.scimEnabled) @@ -232,10 +226,6 @@ export const scimServiceFactory = ({ }; const getScimUser = async ({ orgMembershipId, orgId }: TGetScimUserDTO) => { - console.log("getScimUser args: ", { - orgMembershipId, - orgId - }); // done const [membership] = await orgDAL .findMembership({ [`${TableName.OrgMembership}.id` as "id"]: orgMembershipId, @@ -260,8 +250,6 @@ export const scimServiceFactory = ({ status: 403 }); - console.log("getScimUser membership: ", membership); - return buildScimUser({ orgMembershipId: membership.id, username: membership.username, @@ -273,14 +261,6 @@ export const scimServiceFactory = ({ }; const createScimUser = async ({ username, email, firstName, lastName, orgId }: TCreateScimUserDTO) => { - // do we get external ID or not? - console.log("createScimUser args: ", { - username, - email, - firstName, - lastName, - orgId - }); const org = await orgDAL.findById(orgId); if (!org) @@ -337,7 +317,7 @@ export const scimServiceFactory = ({ ); } } else { - const uniqueUsername = await normalizeUsername(username, userDAL); + const uniqueUsername = await normalizeUsername(`${firstName}-${lastName}`, userDAL); user = await userDAL.create( { username: uniqueUsername, @@ -398,7 +378,6 @@ export const scimServiceFactory = ({ }; const updateScimUser = async ({ userId, orgId, operations }: TUpdateScimUserDTO) => { - console.log("updateScimUser"); // done const [membership] = await orgDAL .findMembership({ userId, @@ -438,12 +417,14 @@ export const scimServiceFactory = ({ }); if (!active) { - await deleteOrgMembership({ + await deleteOrgMembershipFn({ orgMembershipId: membership.id, orgId: membership.orgId, orgDAL, - projectDAL, - projectMembershipDAL + projectMembershipDAL, + projectKeyDAL, + userAliasDAL, + licenseService }); } @@ -458,11 +439,6 @@ export const scimServiceFactory = ({ }; const replaceScimUser = async ({ orgMembershipId, active, orgId }: TReplaceScimUserDTO) => { - console.log("replaceScimUser args: ", { - orgMembershipId, - orgId, - active - }); // done const [membership] = await orgDAL .findMembership({ [`${TableName.OrgMembership}.id` as "id"]: orgMembershipId, @@ -489,12 +465,14 @@ export const scimServiceFactory = ({ if (!active) { // tx - await deleteOrgMembership({ + await deleteOrgMembershipFn({ orgMembershipId: membership.id, orgId: membership.orgId, orgDAL, - projectDAL, - projectMembershipDAL + projectMembershipDAL, + projectKeyDAL, + userAliasDAL, + licenseService }); } @@ -509,10 +487,6 @@ export const scimServiceFactory = ({ }; const deleteScimUser = async ({ orgMembershipId, orgId }: TDeleteScimUserDTO) => { - console.log("deleteScimUser args: ", { - orgMembershipId, - orgId - }); // done const [membership] = await orgDAL.findMembership({ [`${TableName.OrgMembership}.id` as "id"]: orgMembershipId, [`${TableName.OrgMembership}.orgId` as "orgId"]: orgId @@ -531,18 +505,20 @@ export const scimServiceFactory = ({ }); } - await deleteOrgMembership({ + await deleteOrgMembershipFn({ orgMembershipId: membership.id, orgId: membership.orgId, orgDAL, - projectDAL, - projectMembershipDAL + projectMembershipDAL, + projectKeyDAL, + userAliasDAL, + licenseService }); return {}; // intentionally return empty object upon success }; - const listScimGroups = async ({ orgId, offset, limit }: TListScimGroupsDTO) => { + const listScimGroups = async ({ orgId, startIndex, limit }: TListScimGroupsDTO) => { const plan = await licenseService.getPlan(orgId); if (!plan.groups) throw new BadRequestError({ @@ -563,9 +539,15 @@ export const scimServiceFactory = ({ status: 403 }); - const groups = await groupDAL.findGroups({ - orgId - }); + const groups = await groupDAL.findGroups( + { + orgId + }, + { + offset: startIndex - 1, + limit + } + ); const scimGroups = groups.map((group) => buildScimGroup({ @@ -577,7 +559,7 @@ export const scimServiceFactory = ({ return buildScimGroupList({ scimGroups, - offset, + startIndex, limit }); }; @@ -616,9 +598,15 @@ export const scimServiceFactory = ({ ); if (members && members.length) { + const orgMemberships = await orgMembershipDAL.find({ + $in: { + id: members.map((member) => member.value) + } + }); + const newMembers = await addUsersToGroupByUserIds({ group, - userIds: members.map((member) => member.value), + userIds: orgMemberships.map((membership) => membership.userId as string), userDAL, userGroupMembershipDAL, orgDAL, @@ -733,7 +721,13 @@ export const scimServiceFactory = ({ } if (members) { - const membersIdsSet = new Set(members.map((member) => member.value)); + const orgMemberships = await orgMembershipDAL.find({ + $in: { + id: members.map((member) => member.value) + } + }); + + const membersIdsSet = new Set(orgMemberships.map((orgMembership) => orgMembership.userId)); const directMemberUserIds = ( await userGroupMembershipDAL.find({ @@ -752,13 +746,13 @@ export const scimServiceFactory = ({ const allMembersUserIds = directMemberUserIds.concat(pendingGroupAdditionsUserIds); const allMembersUserIdsSet = new Set(allMembersUserIds); - const toAddUserIds = members.filter((member) => !allMembersUserIdsSet.has(member.value)); + 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.value), + userIds: toAddUserIds.map((member) => member.userId as string), userDAL, userGroupMembershipDAL, orgDAL, diff --git a/backend/src/ee/services/scim/scim-types.ts b/backend/src/ee/services/scim/scim-types.ts index a82f08656..e804d51a5 100644 --- a/backend/src/ee/services/scim/scim-types.ts +++ b/backend/src/ee/services/scim/scim-types.ts @@ -65,7 +65,7 @@ export type TDeleteScimUserDTO = { }; export type TListScimGroupsDTO = { - offset: number; + startIndex: number; limit: number; orgId: string; }; diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index b944eb37e..31a777716 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -349,6 +349,7 @@ export const registerRoutes = async ( userDAL }); const orgService = orgServiceFactory({ + userAliasDAL, licenseService, samlConfigDAL, orgRoleDAL, diff --git a/backend/src/services/org/org-fns.ts b/backend/src/services/org/org-fns.ts index 69eac5e07..a63ffabee 100644 --- a/backend/src/services/org/org-fns.ts +++ b/backend/src/services/org/org-fns.ts @@ -1,43 +1,78 @@ +import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; import { TOrgDALFactory } from "@app/services/org/org-dal"; -import { TProjectDALFactory } from "@app/services/project/project-dal"; +import { TProjectKeyDALFactory } from "@app/services/project-key/project-key-dal"; import { TProjectMembershipDALFactory } from "@app/services/project-membership/project-membership-dal"; +import { TUserAliasDALFactory } from "@app/services/user-alias/user-alias-dal"; type TDeleteOrgMembership = { orgMembershipId: string; orgId: string; orgDAL: Pick; - projectDAL: Pick; - projectMembershipDAL: Pick; + projectMembershipDAL: Pick; + projectKeyDAL: Pick; + userAliasDAL: Pick; + licenseService: Pick; }; -export const deleteOrgMembership = async ({ +export const deleteOrgMembershipFn = async ({ orgMembershipId, orgId, orgDAL, - projectDAL, - projectMembershipDAL + projectMembershipDAL, + projectKeyDAL, + userAliasDAL, + licenseService }: TDeleteOrgMembership) => { - const membership = await orgDAL.transaction(async (tx) => { - // delete org membership + const deletedMembership = await orgDAL.transaction(async (tx) => { const orgMembership = await orgDAL.deleteMembershipById(orgMembershipId, orgId, tx); - const projects = await projectDAL.find({ orgId }, { tx }); + if (!orgMembership.userId) { + await licenseService.updateSubscriptionOrgMemberCount(orgId); + return orgMembership; + } - // delete associated project memberships - await projectMembershipDAL.delete( + await userAliasDAL.delete( { - $in: { - projectId: projects.map((project) => project.id) - }, - userId: orgMembership.userId as string + userId: orgMembership.userId, + orgId }, tx ); - // TODO: delete associated aliases + // Get all the project memberships of the user in the organization + const projectMemberships = await projectMembershipDAL.findProjectMembershipsByUserId(orgId, orgMembership.userId); + // Delete all the project memberships of the user in the organization + await projectMembershipDAL.delete( + { + $in: { + id: projectMemberships.map((membership) => membership.id) + } + }, + tx + ); + + // Get all the project keys of the user in the organization + const projectKeys = await projectKeyDAL.find({ + $in: { + projectId: projectMemberships.map((membership) => membership.projectId) + }, + receiverId: orgMembership.userId + }); + + // Delete all the project keys of the user in the organization + await projectKeyDAL.delete( + { + $in: { + id: projectKeys.map((key) => key.id) + } + }, + tx + ); + + await licenseService.updateSubscriptionOrgMemberCount(orgId); return orgMembership; }); - return membership; + return deletedMembership; }; diff --git a/backend/src/services/org/org-service.ts b/backend/src/services/org/org-service.ts index 996a08c4d..a666e6a03 100644 --- a/backend/src/services/org/org-service.ts +++ b/backend/src/services/org/org-service.ts @@ -18,6 +18,7 @@ import { generateUserSrpKeys } from "@app/lib/crypto/srp"; import { BadRequestError, UnauthorizedError } from "@app/lib/errors"; import { alphaNumericNanoId } from "@app/lib/nanoid"; import { isDisposableEmail } from "@app/lib/validator"; +import { TUserAliasDALFactory } from "@app/services/user-alias/user-alias-dal"; import { ActorAuthMethod, ActorType, AuthMethod, AuthTokenType } from "../auth/auth-type"; import { TAuthTokenServiceFactory } from "../auth-token/auth-token-service"; @@ -30,6 +31,7 @@ import { TUserDALFactory } from "../user/user-dal"; import { TIncidentContactsDALFactory } from "./incident-contacts-dal"; import { TOrgBotDALFactory } from "./org-bot-dal"; import { TOrgDALFactory } from "./org-dal"; +import { deleteOrgMembershipFn } from "./org-fns"; import { TOrgRoleDALFactory } from "./org-role-dal"; import { TDeleteOrgMembershipDTO, @@ -43,6 +45,7 @@ import { } from "./org-types"; type TOrgServiceFactoryDep = { + userAliasDAL: Pick; orgDAL: TOrgDALFactory; orgBotDAL: TOrgBotDALFactory; orgRoleDAL: TOrgRoleDALFactory; @@ -65,6 +68,7 @@ type TOrgServiceFactoryDep = { export type TOrgServiceFactory = ReturnType; export const orgServiceFactory = ({ + userAliasDAL, orgDAL, userDAL, groupDAL, @@ -572,47 +576,14 @@ export const orgServiceFactory = ({ const { permission } = await permissionService.getUserOrgPermission(userId, orgId, actorAuthMethod, actorOrgId); ForbiddenError.from(permission).throwUnlessCan(OrgPermissionActions.Delete, OrgPermissionSubjects.Member); - const deletedMembership = await orgDAL.transaction(async (tx) => { - const orgMembership = await orgDAL.deleteMembershipById(membershipId, orgId, tx); - - if (!orgMembership.userId) { - await licenseService.updateSubscriptionOrgMemberCount(orgId); - return orgMembership; - } - - // Get all the project memberships of the user in the organization - const projectMemberships = await projectMembershipDAL.findProjectMembershipsByUserId(orgId, orgMembership.userId); - - // Delete all the project memberships of the user in the organization - await projectMembershipDAL.delete( - { - $in: { - id: projectMemberships.map((membership) => membership.id) - } - }, - tx - ); - - // Get all the project keys of the user in the organization - const projectKeys = await projectKeyDAL.find({ - $in: { - projectId: projectMemberships.map((membership) => membership.projectId) - }, - receiverId: orgMembership.userId - }); - - // Delete all the project keys of the user in the organization - await projectKeyDAL.delete( - { - $in: { - id: projectKeys.map((key) => key.id) - } - }, - tx - ); - - await licenseService.updateSubscriptionOrgMemberCount(orgId); - return orgMembership; + const deletedMembership = await deleteOrgMembershipFn({ + orgMembershipId: membershipId, + orgId, + orgDAL, + projectMembershipDAL, + projectKeyDAL, + userAliasDAL, + licenseService }); return deletedMembership; diff --git a/backend/src/services/user/user-fns.ts b/backend/src/services/user/user-fns.ts index 23789df1b..639320e24 100644 --- a/backend/src/services/user/user-fns.ts +++ b/backend/src/services/user/user-fns.ts @@ -4,7 +4,7 @@ import { alphaNumericNanoId } from "@app/lib/nanoid"; import { TUserDALFactory } from "@app/services/user/user-dal"; export const normalizeUsername = async (username: string, userDAL: Pick) => { - let attempt = slugify(username); + let attempt = slugify(`${username}-${alphaNumericNanoId(4)}`); let user = await userDAL.findOne({ username: attempt }); if (!user) return attempt;