From 032c5b56207484ef6b5f4176bad768f08a83554f Mon Sep 17 00:00:00 2001 From: Tuan Dang Date: Mon, 22 Apr 2024 10:37:24 -0700 Subject: [PATCH] Convert pending group addition table into isPending field --- backend/src/@types/knex.d.ts | 8 - .../20240417032913_pending-group-addition.ts | 22 +- backend/src/db/schemas/index.ts | 1 - backend/src/db/schemas/models.ts | 1 - .../src/db/schemas/pending-group-additions.ts | 20 - .../src/db/schemas/user-group-membership.ts | 3 +- backend/src/ee/services/group/group-dal.ts | 44 +- backend/src/ee/services/group/group-fns.ts | 758 ++++++------------ .../src/ee/services/group/group-service.ts | 7 +- backend/src/ee/services/group/group-types.ts | 66 +- .../group/pending-group-addition-dal.ts | 55 -- .../group/user-group-membership-dal.ts | 41 +- backend/src/ee/services/scim/scim-service.ts | 17 +- backend/src/server/routes/index.ts | 5 - .../src/services/auth/auth-signup-service.ts | 12 +- backend/src/services/user/user-dal.ts | 8 +- 16 files changed, 333 insertions(+), 735 deletions(-) delete mode 100644 backend/src/db/schemas/pending-group-additions.ts delete mode 100644 backend/src/ee/services/group/pending-group-addition-dal.ts diff --git a/backend/src/@types/knex.d.ts b/backend/src/@types/knex.d.ts index 93c8cb1eb..2c8b8be5a 100644 --- a/backend/src/@types/knex.d.ts +++ b/backend/src/@types/knex.d.ts @@ -86,9 +86,6 @@ import { TOrgRoles, TOrgRolesInsert, TOrgRolesUpdate, - TPendingGroupAdditions, - TPendingGroupAdditionsInsert, - TPendingGroupAdditionsUpdate, TProjectBots, TProjectBotsInsert, TProjectBotsUpdate, @@ -215,11 +212,6 @@ declare module "knex/types/tables" { interface Tables { [TableName.Users]: Knex.CompositeTableType; [TableName.Groups]: Knex.CompositeTableType; - [TableName.PendingGroupAddition]: Knex.CompositeTableType< - TPendingGroupAdditions, - TPendingGroupAdditionsInsert, - TPendingGroupAdditionsUpdate - >; [TableName.UserGroupMembership]: Knex.CompositeTableType< TUserGroupMembership, TUserGroupMembershipInsert, diff --git a/backend/src/db/migrations/20240417032913_pending-group-addition.ts b/backend/src/db/migrations/20240417032913_pending-group-addition.ts index e720abd2f..70fe22727 100644 --- a/backend/src/db/migrations/20240417032913_pending-group-addition.ts +++ b/backend/src/db/migrations/20240417032913_pending-group-addition.ts @@ -1,25 +1,15 @@ import { Knex } from "knex"; import { TableName } from "../schemas"; -import { createOnUpdateTrigger, dropOnUpdateTrigger } from "../utils"; export async function up(knex: Knex): Promise { - if (!(await knex.schema.hasTable(TableName.PendingGroupAddition))) { - await knex.schema.createTable(TableName.PendingGroupAddition, (t) => { - t.uuid("id", { primaryKey: true }).defaultTo(knex.fn.uuid()); - t.uuid("userId").notNullable(); - t.foreign("userId").references("id").inTable(TableName.Users).onDelete("CASCADE"); - t.uuid("groupId").notNullable(); - t.foreign("groupId").references("id").inTable(TableName.Groups).onDelete("CASCADE"); - t.unique(["userId", "groupId"]); - t.timestamps(true, true, true); - }); - } - - await createOnUpdateTrigger(knex, TableName.PendingGroupAddition); + await knex.schema.alterTable(TableName.UserGroupMembership, (t) => { + t.boolean("isPending").notNullable().defaultTo(false); + }); } export async function down(knex: Knex): Promise { - await knex.schema.dropTableIfExists(TableName.PendingGroupAddition); - await dropOnUpdateTrigger(knex, TableName.PendingGroupAddition); + await knex.schema.alterTable(TableName.UserGroupMembership, (t) => { + t.dropColumn("isPending"); + }); } diff --git a/backend/src/db/schemas/index.ts b/backend/src/db/schemas/index.ts index 9a7772d5d..b9dab06ba 100644 --- a/backend/src/db/schemas/index.ts +++ b/backend/src/db/schemas/index.ts @@ -27,7 +27,6 @@ export * from "./org-bots"; export * from "./org-memberships"; export * from "./org-roles"; export * from "./organizations"; -export * from "./pending-group-additions"; export * from "./project-bots"; export * from "./project-environments"; export * from "./project-keys"; diff --git a/backend/src/db/schemas/models.ts b/backend/src/db/schemas/models.ts index ea1925eb2..d5cf1b886 100644 --- a/backend/src/db/schemas/models.ts +++ b/backend/src/db/schemas/models.ts @@ -3,7 +3,6 @@ import { z } from "zod"; export enum TableName { Users = "users", Groups = "groups", - PendingGroupAddition = "pending_group_additions", GroupProjectMembership = "group_project_memberships", GroupProjectMembershipRole = "group_project_membership_roles", UserGroupMembership = "user_group_membership", diff --git a/backend/src/db/schemas/pending-group-additions.ts b/backend/src/db/schemas/pending-group-additions.ts deleted file mode 100644 index b665f8841..000000000 --- a/backend/src/db/schemas/pending-group-additions.ts +++ /dev/null @@ -1,20 +0,0 @@ -// Code generated by automation script, DO NOT EDIT. -// Automated by pulling database and generating zod schema -// To update. Just run npm run generate:schema -// Written by akhilmhdh. - -import { z } from "zod"; - -import { TImmutableDBKeys } from "./models"; - -export const PendingGroupAdditionsSchema = z.object({ - id: z.string().uuid(), - userId: z.string().uuid(), - groupId: z.string().uuid(), - createdAt: z.date(), - updatedAt: z.date() -}); - -export type TPendingGroupAdditions = z.infer; -export type TPendingGroupAdditionsInsert = Omit, TImmutableDBKeys>; -export type TPendingGroupAdditionsUpdate = Partial, TImmutableDBKeys>>; diff --git a/backend/src/db/schemas/user-group-membership.ts b/backend/src/db/schemas/user-group-membership.ts index b6345d85a..6b5fccd46 100644 --- a/backend/src/db/schemas/user-group-membership.ts +++ b/backend/src/db/schemas/user-group-membership.ts @@ -12,7 +12,8 @@ export const UserGroupMembershipSchema = z.object({ userId: z.string().uuid(), groupId: z.string().uuid(), createdAt: z.date(), - updatedAt: z.date() + updatedAt: z.date(), + isPending: z.boolean().default(false) }); export type TUserGroupMembership = z.infer; diff --git a/backend/src/ee/services/group/group-dal.ts b/backend/src/ee/services/group/group-dal.ts index 26a5d688e..3da1f242c 100644 --- a/backend/src/ee/services/group/group-dal.ts +++ b/backend/src/ee/services/group/group-dal.ts @@ -84,36 +84,14 @@ export const groupDALFactory = (db: TDbClient) => { db.raw("?", [groupId]) ); }) - .leftJoin(TableName.PendingGroupAddition, function () { - this.on(`${TableName.PendingGroupAddition}.userId`, "=", `${TableName.Users}.id`).andOn( - `${TableName.PendingGroupAddition}.groupId`, - "=", - db.raw("?", [groupId]) - ); - }) - .select< - { - id: string; - groupId: string; - email: string; - username: string; - firstName: string; - lastName: string; - userId: string; - isPartOfGroup: boolean; - }[] - >( + .select( db.ref("id").withSchema(TableName.OrgMembership), db.ref("groupId").withSchema(TableName.UserGroupMembership), db.ref("email").withSchema(TableName.Users), db.ref("username").withSchema(TableName.Users), db.ref("firstName").withSchema(TableName.Users), db.ref("lastName").withSchema(TableName.Users), - db.ref("id").withSchema(TableName.Users).as("userId"), - db.raw('CASE WHEN ?? IS NOT NULL OR ?? IS NOT NULL THEN TRUE ELSE FALSE END AS "isPartOfGroup"', [ - `${TableName.UserGroupMembership}.groupId`, - `${TableName.PendingGroupAddition}.groupId` - ]) + db.ref("id").withSchema(TableName.Users).as("userId") ) .where({ isGhost: false }) .offset(offset); @@ -128,14 +106,16 @@ export const groupDALFactory = (db: TDbClient) => { const members = await query; - return members.map(({ email, username: memberUsername, firstName, lastName, userId, isPartOfGroup }) => ({ - id: userId, - email, - username: memberUsername, - firstName, - lastName, - isPartOfGroup - })); + return members.map( + ({ email, username: memberUsername, firstName, lastName, userId, groupId: memberGroupId }) => ({ + id: userId, + email, + username: memberUsername, + firstName, + lastName, + isPartOfGroup: !!memberGroupId + }) + ); } catch (error) { throw new DatabaseError({ error, name: "Find all org members" }); } diff --git a/backend/src/ee/services/group/group-fns.ts b/backend/src/ee/services/group/group-fns.ts index 383b44a6e..8b37de300 100644 --- a/backend/src/ee/services/group/group-fns.ts +++ b/backend/src/ee/services/group/group-fns.ts @@ -5,24 +5,143 @@ import { decryptAsymmetric, encryptAsymmetric, infisicalSymmetricDecrypt } from import { BadRequestError, ScimRequestError } from "@app/lib/errors"; import { + TAddUsersToGroup, TAddUsersToGroupByUserIds, - TAddUsersToGroupDirectly, - TAddUsersToPendingGroupAdditions, TConvertPendingGroupAdditionsToGroupMemberships, - TRemoveUsersFromGroupByUserIds, - TRemoveUsersFromGroupDirectly, - TRemoveUsersFromPendingGroupAdditions + TRemoveUsersFromGroupByUserIds } from "./group-types"; -/** - * Add users with usernames [usernames] to group [group] directly. - * - Users must have finished completing their account and have private key(s). - * @param {group} group - group to add user(s) to - * @param {string[]} usernames - username(s) of user(s) to add to group - */ -export const addUsersToGroupDirectly = async ({ +const addAcceptedUsersToGroup = async ({ + userIds, group, - usernames, + userGroupMembershipDAL, + userDAL, + groupProjectDAL, + projectKeyDAL, + projectDAL, + projectBotDAL, + tx +}: TAddUsersToGroup) => { + console.log("addAcceptedUsersToGroup args: ", { + userIds, + group + }); + const users = await userDAL.findUserEncKeyByUserIdsBatch( + { + userIds + }, + tx + ); + + await userGroupMembershipDAL.insertMany( + users.map((user) => ({ + userId: user.userId, + groupId: group.id, + isPending: false + })), + tx + ); + + // check which projects the group is part of + const projectIds = Array.from( + new Set( + ( + await groupProjectDAL.find( + { + groupId: group.id + }, + { tx } + ) + ).map((gp) => gp.projectId) + ) + ); + + const keys = await projectKeyDAL.find( + { + $in: { + projectId: projectIds, + receiverId: users.map((u) => u.id) + } + }, + { tx } + ); + + const userKeysSet = new Set(keys.map((k) => `${k.projectId}-${k.receiverId}`)); + + for await (const projectId of projectIds) { + const usersToAddProjectKeyFor = users.filter((u) => !userKeysSet.has(`${projectId}-${u.userId}`)); + + if (usersToAddProjectKeyFor.length) { + // there are users who need to be shared keys + // process adding bulk users to projects for each project individually + const ghostUser = await projectDAL.findProjectGhostUser(projectId, tx); + + if (!ghostUser) { + throw new BadRequestError({ + message: "Failed to find sudo user" + }); + } + + const ghostUserLatestKey = await projectKeyDAL.findLatestProjectKey(ghostUser.id, projectId, tx); + + if (!ghostUserLatestKey) { + throw new BadRequestError({ + message: "Failed to find sudo user latest key" + }); + } + + const bot = await projectBotDAL.findOne({ projectId }, tx); + + if (!bot) { + throw new BadRequestError({ + message: "Failed to find bot" + }); + } + + const botPrivateKey = infisicalSymmetricDecrypt({ + keyEncoding: bot.keyEncoding as SecretKeyEncoding, + iv: bot.iv, + tag: bot.tag, + ciphertext: bot.encryptedPrivateKey + }); + + const plaintextProjectKey = decryptAsymmetric({ + ciphertext: ghostUserLatestKey.encryptedKey, + nonce: ghostUserLatestKey.nonce, + publicKey: ghostUserLatestKey.sender.publicKey, + privateKey: botPrivateKey + }); + + const projectKeysToAdd = usersToAddProjectKeyFor.map((user) => { + const { ciphertext: encryptedKey, nonce } = encryptAsymmetric( + plaintextProjectKey, + user.publicKey, + botPrivateKey + ); + return { + encryptedKey, + nonce, + senderId: ghostUser.id, + receiverId: user.userId, + projectId + }; + }); + + await projectKeyDAL.insertMany(projectKeysToAdd, tx); + } + } +}; + +/** + * Add users with user ids [userIds] to group [group]. + * - Users may or may not have finished completing their accounts; this function will + * handle both adding users to groups directly and via pending group additions. + * @param {group} group - group to add user(s) to + * @param {string[]} userIds - id(s) of user(s) to add to group + */ +export const addUsersToGroupByUserIds = async ({ + group, + userIds, userDAL, userGroupMembershipDAL, orgDAL, @@ -31,33 +150,27 @@ export const addUsersToGroupDirectly = async ({ projectDAL, projectBotDAL, tx: outerTx -}: TAddUsersToGroupDirectly) => { +}: TAddUsersToGroupByUserIds) => { const processAddition = async (tx: Knex) => { - const users = await userDAL.findUserEncKeyByUsernameBatch( + const foundMembers = await userDAL.find( { - usernames + $in: { + id: userIds + } }, - tx + { tx } ); - const usersUsernamesSet = new Set(users.map((u) => u.username)); - usernames.forEach((username) => { - if (!usersUsernamesSet.has(username)) { - throw new BadRequestError({ - message: `Failed to find user with username ${username}` - }); - } - }); + const foundMembersIdsSet = new Set(foundMembers.map((member) => member.id)); - const userIds = users.map((u) => { - if (!u.isAccepted) { - throw new BadRequestError({ - message: `User ${u.username} cannot be added to group because they have not confirmed their account` - }); - } + const isCompleteMatch = userIds.every((userId) => foundMembersIdsSet.has(userId)); - return u.userId; - }); + if (!isCompleteMatch) { + throw new ScimRequestError({ + detail: "Members not found", + status: 404 + }); + } // check if user(s) group membership(s) already exists const existingUserGroupMemberships = await userGroupMembershipDAL.find( @@ -87,194 +200,6 @@ export const addUsersToGroupDirectly = async ({ { tx } ); - const existingUserOrgMembershipsUsernamesSet = new Set(existingUserOrgMemberships.map((u) => u.username)); - - usernames.forEach((username) => { - if (!existingUserOrgMembershipsUsernamesSet.has(username)) - throw new BadRequestError({ - message: `User ${username} is not part of the organization` - }); - }); - - await userGroupMembershipDAL.insertMany( - userIds.map((userId) => ({ - userId, - groupId: group.id - })), - tx - ); - - // check which projects the group is part of - const projectIds = Array.from( - new Set( - ( - await groupProjectDAL.find( - { - groupId: group.id - }, - { tx } - ) - ).map((gp) => gp.projectId) - ) - ); - - const keys = await projectKeyDAL.find( - { - $in: { - projectId: projectIds, - receiverId: userIds - } - }, - { tx } - ); - - const userKeysSet = new Set(keys.map((k) => `${k.projectId}-${k.receiverId}`)); - - for await (const projectId of projectIds) { - const usersToAddProjectKeyFor = users.filter((u) => !userKeysSet.has(`${projectId}-${u.userId}`)); - - if (usersToAddProjectKeyFor.length) { - // there are users who need to be shared keys - // process adding bulk users to projects for each project individually - const ghostUser = await projectDAL.findProjectGhostUser(projectId, tx); - - if (!ghostUser) { - throw new BadRequestError({ - message: "Failed to find sudo user" - }); - } - - const ghostUserLatestKey = await projectKeyDAL.findLatestProjectKey(ghostUser.id, projectId, tx); - - if (!ghostUserLatestKey) { - throw new BadRequestError({ - message: "Failed to find sudo user latest key" - }); - } - - const bot = await projectBotDAL.findOne({ projectId }, tx); - - if (!bot) { - throw new BadRequestError({ - message: "Failed to find bot" - }); - } - - const botPrivateKey = infisicalSymmetricDecrypt({ - keyEncoding: bot.keyEncoding as SecretKeyEncoding, - iv: bot.iv, - tag: bot.tag, - ciphertext: bot.encryptedPrivateKey - }); - - const plaintextProjectKey = decryptAsymmetric({ - ciphertext: ghostUserLatestKey.encryptedKey, - nonce: ghostUserLatestKey.nonce, - publicKey: ghostUserLatestKey.sender.publicKey, - privateKey: botPrivateKey - }); - - const projectKeysToAdd = usersToAddProjectKeyFor.map((user) => { - const { ciphertext: encryptedKey, nonce } = encryptAsymmetric( - plaintextProjectKey, - user.publicKey, - botPrivateKey - ); - return { - encryptedKey, - nonce, - senderId: ghostUser.id, - receiverId: user.userId, - projectId - }; - }); - - await projectKeyDAL.insertMany(projectKeysToAdd, tx); - } - } - - return users; - }; - - if (outerTx) { - return processAddition(outerTx); - } - return userDAL.transaction(async (tx) => { - return processAddition(tx); - }); -}; - -/** - * Add users with user ids [userIds] to group [group] via pending group additions. - * - Users must have not finished completing their accounts (i.e. they don't have private key(s) yet). - * @param {group} group - group to add user(s) to - * @param {string[]} userIds - id(s) of user(s) to add to group - */ -export const addUsersToPendingGroupAdditions = async ({ - group, - userIds, - pendingGroupAdditionDAL, - userDAL, - orgDAL, - tx: outerTx -}: TAddUsersToPendingGroupAdditions) => { - const processAddition = async (tx: Knex) => { - const users = await userDAL.find( - { - $in: { - id: userIds - } - }, - { tx } - ); - - const usersUserIdsSet = new Set(users.map((u) => u.id)); - userIds.forEach((userId) => { - if (!usersUserIdsSet.has(userId)) { - throw new BadRequestError({ - message: `Failed to find user with id ${userId}` - }); - } - }); - - users.map((u) => { - if (u.isAccepted) { - throw new BadRequestError({ - message: `User ${u.username} cannot be added to a pending group addition because they have confirmed their account` - }); - } - - return u.id; - }); - - // check if user(s) pending group addition(s) already exist - const existingPendingGroupAdditions = await pendingGroupAdditionDAL.find( - { - groupId: group.id, - $in: { - userId: userIds - } - }, - { tx } - ); - - if (existingPendingGroupAdditions.length) { - throw new BadRequestError({ - message: `User(s) are already part of the group ${group.slug}` - }); - } - - // check if all user(s) are part of the organization - const existingUserOrgMemberships = await orgDAL.findMembership( - { - orgId: group.orgId, - $in: { - userId: userIds - } - }, - { tx } - ); - const existingUserOrgMembershipsUserIdsSet = new Set(existingUserOrgMemberships.map((u) => u.userId)); userIds.forEach((userId) => { @@ -284,109 +209,45 @@ export const addUsersToPendingGroupAdditions = async ({ }); }); - await pendingGroupAdditionDAL.insertMany( - users.map((user) => ({ - userId: user.id, - groupId: group.id - })), - tx - ); - - return users; - }; - - if (outerTx) { - return processAddition(outerTx); - } - return userDAL.transaction(async (tx) => { - return processAddition(tx); - }); -}; - -/** - * Add users with user ids [userIds] to group [group]. - * - Users may or may not have finished completing their accounts; this function will - * handle both adding users to groups directly and via pending group additions. - * @param {group} group - group to add user(s) to - * @param {string[]} userIds - id(s) of user(s) to add to group - */ -export const addUsersToGroupByUserIds = async ({ - group, - userIds, - userDAL, - userGroupMembershipDAL, - orgDAL, - groupProjectDAL, - pendingGroupAdditionDAL, - projectKeyDAL, - projectDAL, - projectBotDAL, - tx: outerTx -}: TAddUsersToGroupByUserIds) => { - const processAddition = async (tx: Knex) => { - const foundMembers = await userDAL.find({ - $in: { - id: userIds - } - }); - - const foundMembersIdsSet = new Set(foundMembers.map((member) => member.id)); - - const isCompleteMatch = userIds.every((userId) => foundMembersIdsSet.has(userId)); - - if (!isCompleteMatch) { - throw new ScimRequestError({ - detail: "Members not found", - status: 404 - }); - } - - const membersToAddToGroupDirectly: TUsers[] = []; + const membersToAddToGroupNonPending: TUsers[] = []; const membersToAddToGroupPending: TUsers[] = []; foundMembers.forEach((member) => { if (member.isAccepted) { // add accepted member to group - membersToAddToGroupDirectly.push(member); + membersToAddToGroupNonPending.push(member); } else { // add incomplete member to pending group addition membersToAddToGroupPending.push(member); } }); - let addedUsers: TUsers[] = []; - - if (membersToAddToGroupDirectly.length) { - addedUsers = addedUsers.concat( - await addUsersToGroupDirectly({ - group, - usernames: membersToAddToGroupDirectly.map((member) => member.username), - userDAL, - userGroupMembershipDAL, - orgDAL, - groupProjectDAL, - projectKeyDAL, - projectDAL, - projectBotDAL, - tx - }) - ); + if (membersToAddToGroupNonPending.length) { + await addAcceptedUsersToGroup({ + userIds: membersToAddToGroupNonPending.map((member) => member.id), + group, + userDAL, + userGroupMembershipDAL, + groupProjectDAL, + projectKeyDAL, + projectDAL, + projectBotDAL, + tx + }); } if (membersToAddToGroupPending.length) { - addedUsers = addedUsers.concat( - await addUsersToPendingGroupAdditions({ - group, - userIds: membersToAddToGroupPending.map((member) => member.id), - pendingGroupAdditionDAL, - userDAL, - orgDAL, - tx - }) + await userGroupMembershipDAL.insertMany( + membersToAddToGroupPending.map((member) => ({ + userId: member.id, + groupId: group.id, + isPending: true + })), + tx ); } - return addedUsers; + return membersToAddToGroupNonPending.concat(membersToAddToGroupPending); }; if (outerTx) { @@ -399,185 +260,7 @@ export const addUsersToGroupByUserIds = async ({ /** * Remove users with user ids [userIds] from group [group]. - * - Users must be directly added to the group. - * @param {group} group - group to remove user(s) from - * @param {string[]} userIds - id(s) of user(s) to remove from group - */ -export const removeUsersFromGroupDirectly = async ({ - group, - userIds, - userDAL, - userGroupMembershipDAL, - groupProjectDAL, - projectKeyDAL, - tx: outerTx -}: TRemoveUsersFromGroupDirectly) => { - const processRemoval = async (tx: Knex) => { - const users = await userDAL.find( - { - $in: { - id: userIds - } - }, - { tx } - ); - - const usersUserIdsSet = new Set(users.map((u) => u.id)); - userIds.forEach((userId) => { - if (!usersUserIdsSet.has(userId)) { - throw new BadRequestError({ - message: `Failed to find user with id ${userId}` - }); - } - }); - - // check if user group membership already exists - const existingUserGroupMemberships = await userGroupMembershipDAL.find( - { - groupId: group.id, - $in: { - userId: userIds - } - }, - { tx } - ); - - const existingUserGroupMembershipsUserIdsSet = new Set(existingUserGroupMemberships.map((u) => u.userId)); - - userIds.forEach((userId) => { - if (!existingUserGroupMembershipsUserIdsSet.has(userId)) - throw new BadRequestError({ - message: `User(s) are not part of the group ${group.slug}` - }); - }); - - // check which projects the group is part of - const projectIds = Array.from( - new Set( - ( - await groupProjectDAL.find( - { - groupId: group.id - }, - { tx } - ) - ).map((gp) => gp.projectId) - ) - ); - - // TODO: this part can be optimized - for await (const userId of userIds) { - const t = await userGroupMembershipDAL.filterProjectsByUserMembership(userId, group.id, projectIds, tx); - const projectsToDeleteKeyFor = projectIds.filter((p) => !t.has(p)); - - if (projectsToDeleteKeyFor.length) { - await projectKeyDAL.delete( - { - receiverId: userId, - $in: { - projectId: projectsToDeleteKeyFor - } - }, - tx - ); - } - - await userGroupMembershipDAL.delete( - { - groupId: group.id, - userId - }, - tx - ); - } - - return users; - }; - - if (outerTx) { - return processRemoval(outerTx); - } - return userDAL.transaction(async (tx) => { - return processRemoval(tx); - }); -}; - -/** - * Remove users with user ids [userIds] from group [group] via pending group additions. - * - Users must have pending group additions to the group. - * @param {group} group - group to remove user(s) from - * @param {string[]} userIds - id(s) of user(s) to remove from group - */ -export const removeUsersFromPendingGroupAdditions = async ({ - group, - userIds, - userDAL, - pendingGroupAdditionDAL, - tx: outerTx -}: TRemoveUsersFromPendingGroupAdditions) => { - const processRemoval = async (tx: Knex) => { - const users = await userDAL.find( - { - $in: { - id: userIds - } - }, - { tx } - ); - - const usersUserIdsSet = new Set(users.map((u) => u.id)); - userIds.forEach((userId) => { - if (!usersUserIdsSet.has(userId)) { - throw new BadRequestError({ - message: `Failed to find user with id ${userId}` - }); - } - }); - - // check if user pending group addition already exists - const existingPendingGroupAdditions = await pendingGroupAdditionDAL.find( - { - groupId: group.id, - $in: { - userId: userIds - } - }, - { tx } - ); - - const existingPendingGroupAdditionsUserIdsSet = new Set(existingPendingGroupAdditions.map((u) => u.userId)); - - userIds.forEach((userId) => { - if (!existingPendingGroupAdditionsUserIdsSet.has(userId)) - throw new BadRequestError({ - message: `User(s) are not part of the group ${group.slug}` - }); - }); - - await pendingGroupAdditionDAL.delete( - { - groupId: group.id, - $in: { - userId: userIds - } - }, - tx - ); - - return users; - }; - - if (outerTx) { - return processRemoval(outerTx); - } - return userDAL.transaction(async (tx) => { - return processRemoval(tx); - }); -}; - -/** - * Remove users with user ids [userIds] from group [group]. - * - Users may be part of the group directly or via pending group additions; + * - Users may be part of the group (non-pending + pending); * this function will handle both cases. * @param {group} group - group to remove user(s) from * @param {string[]} userIds - id(s) of user(s) to remove from group @@ -588,7 +271,6 @@ export const removeUsersFromGroupByUserIds = async ({ userDAL, userGroupMembershipDAL, groupProjectDAL, - pendingGroupAdditionDAL, projectKeyDAL, tx: outerTx }: TRemoveUsersFromGroupByUserIds) => { @@ -610,48 +292,91 @@ export const removeUsersFromGroupByUserIds = async ({ }); } - const membersToRemoveFromGroupDirectly: TUsers[] = []; + // check if user group membership already exists + const existingUserGroupMemberships = await userGroupMembershipDAL.find( + { + groupId: group.id, + $in: { + userId: userIds + } + }, + { tx } + ); + + const existingUserGroupMembershipsUserIdsSet = new Set(existingUserGroupMemberships.map((u) => u.userId)); + + userIds.forEach((userId) => { + if (!existingUserGroupMembershipsUserIdsSet.has(userId)) + throw new BadRequestError({ + message: `User(s) are not part of the group ${group.slug}` + }); + }); + + const membersToRemoveFromGroupNonPending: TUsers[] = []; const membersToRemoveFromGroupPending: TUsers[] = []; foundMembers.forEach((member) => { if (member.isAccepted) { // remove accepted member from group - membersToRemoveFromGroupDirectly.push(member); + membersToRemoveFromGroupNonPending.push(member); } else { // remove incomplete member from pending group addition membersToRemoveFromGroupPending.push(member); } }); - let removedUsers: TUsers[] = []; - - if (membersToRemoveFromGroupDirectly.length) { - removedUsers = removedUsers.concat( - await removeUsersFromGroupDirectly({ - group, - userIds: membersToRemoveFromGroupDirectly.map((member) => member.id), - userDAL, - userGroupMembershipDAL, - groupProjectDAL, - projectKeyDAL, - tx - }) + if (membersToRemoveFromGroupNonPending.length) { + // check which projects the group is part of + const projectIds = Array.from( + new Set( + ( + await groupProjectDAL.find( + { + groupId: group.id + }, + { tx } + ) + ).map((gp) => gp.projectId) + ) ); + + // TODO: this part can be optimized + for await (const userId of userIds) { + const t = await userGroupMembershipDAL.filterProjectsByUserMembership(userId, group.id, projectIds, tx); + const projectsToDeleteKeyFor = projectIds.filter((p) => !t.has(p)); + + if (projectsToDeleteKeyFor.length) { + await projectKeyDAL.delete( + { + receiverId: userId, + $in: { + projectId: projectsToDeleteKeyFor + } + }, + tx + ); + } + + await userGroupMembershipDAL.delete( + { + groupId: group.id, + userId + }, + tx + ); + } } if (membersToRemoveFromGroupPending.length) { - removedUsers = removedUsers.concat( - await removeUsersFromPendingGroupAdditions({ - group, - userIds: membersToRemoveFromGroupPending.map((member) => member.id), - pendingGroupAdditionDAL, - userDAL, - tx - }) - ); + await userGroupMembershipDAL.delete({ + groupId: group.id, + $in: { + userId: membersToRemoveFromGroupPending.map((member) => member.id) + } + }); } - return removedUsers; + return membersToRemoveFromGroupNonPending.concat(membersToRemoveFromGroupPending); }; if (outerTx) { @@ -669,9 +394,7 @@ export const removeUsersFromGroupByUserIds = async ({ export const convertPendingGroupAdditionsToGroupMemberships = async ({ userIds, userDAL, - pendingGroupAdditionDAL, userGroupMembershipDAL, - orgDAL, groupProjectDAL, projectKeyDAL, projectDAL, @@ -705,15 +428,14 @@ export const convertPendingGroupAdditionsToGroupMemberships = async ({ } }); - const pendingGroupAdditions = await pendingGroupAdditionDAL.deletePendingGroupAdditionsByUserIds(userIds, tx); + const pendingGroupAdditions = await userGroupMembershipDAL.deletePendingUserGroupMembershipsByUserIds(userIds, tx); for await (const pendingGroupAddition of pendingGroupAdditions) { - await addUsersToGroupDirectly({ + await addAcceptedUsersToGroup({ + userIds: [pendingGroupAddition.user.id], group: pendingGroupAddition.group, - usernames: [pendingGroupAddition.user.username], userDAL, userGroupMembershipDAL, - orgDAL, groupProjectDAL, projectKeyDAL, projectDAL, diff --git a/backend/src/ee/services/group/group-service.ts b/backend/src/ee/services/group/group-service.ts index b9ec68013..e6a151bf7 100644 --- a/backend/src/ee/services/group/group-service.ts +++ b/backend/src/ee/services/group/group-service.ts @@ -2,7 +2,6 @@ import { ForbiddenError } from "@casl/ability"; import slugify from "@sindresorhus/slugify"; import { OrgMembershipRole, TOrgRoles } from "@app/db/schemas"; -import { TPendingGroupAdditionDALFactory } from "@app/ee/services/group/pending-group-addition-dal"; import { isAtLeastAsPrivileged } from "@app/lib/casl"; import { BadRequestError, ForbiddenRequestError } from "@app/lib/errors"; import { alphaNumericNanoId } from "@app/lib/nanoid"; @@ -29,7 +28,7 @@ import { import { TUserGroupMembershipDALFactory } from "./user-group-membership-dal"; type TGroupServiceFactoryDep = { - userDAL: Pick; + userDAL: Pick; groupDAL: Pick; groupProjectDAL: Pick; orgDAL: Pick; @@ -40,7 +39,6 @@ type TGroupServiceFactoryDep = { projectDAL: Pick; projectBotDAL: Pick; projectKeyDAL: Pick; - pendingGroupAdditionDAL: TPendingGroupAdditionDALFactory; // remove? permissionService: Pick; licenseService: Pick; }; @@ -56,7 +54,6 @@ export const groupServiceFactory = ({ projectDAL, projectBotDAL, projectKeyDAL, - pendingGroupAdditionDAL, permissionService, licenseService }: TGroupServiceFactoryDep) => { @@ -279,7 +276,6 @@ export const groupServiceFactory = ({ userGroupMembershipDAL, orgDAL, groupProjectDAL, - pendingGroupAdditionDAL, projectKeyDAL, projectDAL, projectBotDAL @@ -333,7 +329,6 @@ export const groupServiceFactory = ({ userIds: [user.id], userDAL, userGroupMembershipDAL, - pendingGroupAdditionDAL, groupProjectDAL, projectKeyDAL }); diff --git a/backend/src/ee/services/group/group-types.ts b/backend/src/ee/services/group/group-types.ts index 301e12cdc..ca9831ffb 100644 --- a/backend/src/ee/services/group/group-types.ts +++ b/backend/src/ee/services/group/group-types.ts @@ -1,7 +1,6 @@ import { Knex } from "knex"; import { TGroups } from "@app/db/schemas"; -import { TPendingGroupAdditionDALFactory } from "@app/ee/services/group/pending-group-addition-dal"; import { TUserGroupMembershipDALFactory } from "@app/ee/services/group/user-group-membership-dal"; import { TGenericPermission } from "@app/lib/types"; import { TGroupProjectDALFactory } from "@app/services/group-project/group-project-dal"; @@ -49,24 +48,22 @@ export type TRemoveUserFromGroupDTO = { // group fns types +export type TAddUsersToGroup = { + userIds: string[]; + group: TGroups; + userDAL: Pick; + userGroupMembershipDAL: Pick; + groupProjectDAL: Pick; + projectKeyDAL: Pick; + projectDAL: Pick; + projectBotDAL: Pick; + tx: Knex; +}; + export type TAddUsersToGroupByUserIds = { group: TGroups; userIds: string[]; - userDAL: Pick; - userGroupMembershipDAL: Pick; - orgDAL: Pick; - groupProjectDAL: Pick; - pendingGroupAdditionDAL: Pick; - projectKeyDAL: Pick; - projectDAL: Pick; - projectBotDAL: Pick; - tx?: Knex; -}; - -export type TAddUsersToGroupDirectly = { - group: TGroups; - usernames: string[]; - userDAL: Pick; + userDAL: Pick; userGroupMembershipDAL: Pick; orgDAL: Pick; groupProjectDAL: Pick; @@ -76,50 +73,23 @@ export type TAddUsersToGroupDirectly = { tx?: Knex; }; -export type TAddUsersToPendingGroupAdditions = { - userIds: string[]; - group: TGroups; - pendingGroupAdditionDAL: Pick; - userDAL: Pick; - orgDAL: Pick; - tx?: Knex; -}; - export type TRemoveUsersFromGroupByUserIds = { group: TGroups; userIds: string[]; userDAL: Pick; userGroupMembershipDAL: Pick; - pendingGroupAdditionDAL: Pick; groupProjectDAL: Pick; projectKeyDAL: Pick; tx?: Knex; }; -export type TRemoveUsersFromGroupDirectly = { - group: TGroups; - userIds: string[]; - userDAL: Pick; - userGroupMembershipDAL: Pick; - groupProjectDAL: Pick; - projectKeyDAL: Pick; - tx?: Knex; -}; - -export type TRemoveUsersFromPendingGroupAdditions = { - group: TGroups; - userIds: string[]; - pendingGroupAdditionDAL: Pick; - userDAL: Pick; - tx?: Knex; -}; - export type TConvertPendingGroupAdditionsToGroupMemberships = { userIds: string[]; - pendingGroupAdditionDAL: Pick; - userDAL: Pick; - userGroupMembershipDAL: Pick; - orgDAL: Pick; + userDAL: Pick; + userGroupMembershipDAL: Pick< + TUserGroupMembershipDALFactory, + "find" | "transaction" | "insertMany" | "deletePendingUserGroupMembershipsByUserIds" + >; groupProjectDAL: Pick; projectKeyDAL: Pick; projectDAL: Pick; diff --git a/backend/src/ee/services/group/pending-group-addition-dal.ts b/backend/src/ee/services/group/pending-group-addition-dal.ts deleted file mode 100644 index 579cbb6e5..000000000 --- a/backend/src/ee/services/group/pending-group-addition-dal.ts +++ /dev/null @@ -1,55 +0,0 @@ -import { Knex } from "knex"; - -import { TDbClient } from "@app/db"; -import { TableName } from "@app/db/schemas"; -import { DatabaseError } from "@app/lib/errors"; -import { ormify } from "@app/lib/knex"; - -export type TPendingGroupAdditionDALFactory = ReturnType; - -export const pendingGroupAdditionDALFactory = (db: TDbClient) => { - const pendingGroupAdditionOrm = ormify(db, TableName.PendingGroupAddition); - - // special query - const deletePendingGroupAdditionsByUserIds = async (userIds: string[], tx?: Knex) => { - try { - const pendingGroupAdditions = await (tx || db)(TableName.PendingGroupAddition) - .whereIn(`${TableName.PendingGroupAddition}.userId`, userIds) - .join(TableName.Groups, `${TableName.PendingGroupAddition}.groupId`, `${TableName.Groups}.id`) - .join(TableName.Users, `${TableName.PendingGroupAddition}.userId`, `${TableName.Users}.id`); - - await pendingGroupAdditionOrm.delete( - { - $in: { - userId: userIds - } - }, - tx - ); - - return pendingGroupAdditions.map(({ userId, username, groupId, orgId, name, slug, role, roleId }) => ({ - user: { - id: userId, - username - }, - group: { - id: groupId, - orgId, - name, - slug, - role, - roleId, - createdAt: new Date(), - updatedAt: new Date() - } - })); - } catch (error) { - throw new DatabaseError({ error, name: "Filter projects by user membership" }); - } - }; - - return { - ...pendingGroupAdditionOrm, - deletePendingGroupAdditionsByUserIds - }; -}; diff --git a/backend/src/ee/services/group/user-group-membership-dal.ts b/backend/src/ee/services/group/user-group-membership-dal.ts index d4f8fecd7..122c58561 100644 --- a/backend/src/ee/services/group/user-group-membership-dal.ts +++ b/backend/src/ee/services/group/user-group-membership-dal.ts @@ -122,10 +122,49 @@ export const userGroupMembershipDALFactory = (db: TDbClient) => { } }; + const deletePendingUserGroupMembershipsByUserIds = async (userIds: string[], tx?: Knex) => { + try { + const members = await (tx || db)(TableName.UserGroupMembership) + .whereIn(`${TableName.UserGroupMembership}.userId`, userIds) + .where(`${TableName.UserGroupMembership}.isPending`, true) + .join(TableName.Groups, `${TableName.UserGroupMembership}.groupId`, `${TableName.Groups}.id`) + .join(TableName.Users, `${TableName.UserGroupMembership}.userId`, `${TableName.Users}.id`); + + await userGroupMembershipOrm.delete( + { + $in: { + userId: userIds + } + }, + tx + ); + + return members.map(({ userId, username, groupId, orgId, name, slug, role, roleId }) => ({ + user: { + id: userId, + username + }, + group: { + id: groupId, + orgId, + name, + slug, + role, + roleId, + createdAt: new Date(), + updatedAt: new Date() + } + })); + } catch (error) { + throw new DatabaseError({ error, name: "Delete pending user group memberships by user ids" }); + } + }; + return { ...userGroupMembershipOrm, filterProjectsByUserMembership, findUserGroupMembershipsInProject, - findGroupMembersNotInProject + findGroupMembersNotInProject, + deletePendingUserGroupMembershipsByUserIds }; }; diff --git a/backend/src/ee/services/scim/scim-service.ts b/backend/src/ee/services/scim/scim-service.ts index bc7fd2528..d56c00a0c 100644 --- a/backend/src/ee/services/scim/scim-service.ts +++ b/backend/src/ee/services/scim/scim-service.ts @@ -5,7 +5,6 @@ import jwt from "jsonwebtoken"; import { OrgMembershipRole, OrgMembershipStatus, TableName, TGroups } from "@app/db/schemas"; import { TGroupDALFactory } from "@app/ee/services/group/group-dal"; import { addUsersToGroupByUserIds, removeUsersFromGroupByUserIds } from "@app/ee/services/group/group-fns"; -import { TPendingGroupAdditionDALFactory } from "@app/ee/services/group/pending-group-addition-dal"; import { TUserGroupMembershipDALFactory } from "@app/ee/services/group/user-group-membership-dal"; import { TScimDALFactory } from "@app/ee/services/scim/scim-dal"; import { getConfig } from "@app/lib/config/env"; @@ -48,7 +47,7 @@ import { type TScimServiceFactoryDep = { scimDAL: Pick; - userDAL: Pick; + userDAL: Pick; orgDAL: Pick< TOrgDALFactory, "createMembership" | "findById" | "findMembership" | "deleteMembershipById" | "transaction" @@ -63,7 +62,6 @@ type TScimServiceFactoryDep = { userGroupMembershipDAL: TUserGroupMembershipDALFactory; // TODO: Pick projectKeyDAL: Pick; projectBotDAL: Pick; - pendingGroupAdditionDAL: TPendingGroupAdditionDALFactory; // TODO: Pick licenseService: Pick; permissionService: Pick; smtpService: TSmtpService; @@ -83,7 +81,6 @@ export const scimServiceFactory = ({ userGroupMembershipDAL, projectKeyDAL, projectBotDAL, - pendingGroupAdditionDAL, permissionService, smtpService }: TScimServiceFactoryDep) => { @@ -572,7 +569,6 @@ export const scimServiceFactory = ({ userGroupMembershipDAL, orgDAL, groupProjectDAL, - pendingGroupAdditionDAL, projectKeyDAL, projectDAL, projectBotDAL, @@ -614,7 +610,6 @@ export const scimServiceFactory = ({ }); } - // TODO: update to include pending group additions const users = await groupDAL.findAllGroupMembers({ orgId: group.orgId, groupId: group.id @@ -676,13 +671,15 @@ export const scimServiceFactory = ({ const directMemberUserIds = ( await userGroupMembershipDAL.find({ - groupId: group.id + groupId: group.id, + isPending: false }) ).map((membership) => membership.userId); const pendingGroupAdditionsUserIds = ( - await pendingGroupAdditionDAL.find({ - groupId: group.id + await userGroupMembershipDAL.find({ + groupId: group.id, + isPending: true }) ).map((pendingGroupAddition) => pendingGroupAddition.userId); @@ -700,7 +697,6 @@ export const scimServiceFactory = ({ userGroupMembershipDAL, orgDAL, groupProjectDAL, - pendingGroupAdditionDAL, projectKeyDAL, projectDAL, projectBotDAL, @@ -715,7 +711,6 @@ export const scimServiceFactory = ({ userDAL, userGroupMembershipDAL, groupProjectDAL, - pendingGroupAdditionDAL, projectKeyDAL, tx }); diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index 7d2a8fd93..5ed2589f6 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -13,7 +13,6 @@ import { dynamicSecretLeaseQueueServiceFactory } from "@app/ee/services/dynamic- import { dynamicSecretLeaseServiceFactory } from "@app/ee/services/dynamic-secret-lease/dynamic-secret-lease-service"; import { groupDALFactory } from "@app/ee/services/group/group-dal"; import { groupServiceFactory } from "@app/ee/services/group/group-service"; -import { pendingGroupAdditionDALFactory } from "@app/ee/services/group/pending-group-addition-dal"; import { userGroupMembershipDALFactory } from "@app/ee/services/group/user-group-membership-dal"; import { identityProjectAdditionalPrivilegeDALFactory } from "@app/ee/services/identity-project-additional-privilege/identity-project-additional-privilege-dal"; import { identityProjectAdditionalPrivilegeServiceFactory } from "@app/ee/services/identity-project-additional-privilege/identity-project-additional-privilege-service"; @@ -218,7 +217,6 @@ export const registerRoutes = async ( const groupProjectDAL = groupProjectDALFactory(db); const groupProjectMembershipRoleDAL = groupProjectMembershipRoleDALFactory(db); const userGroupMembershipDAL = userGroupMembershipDALFactory(db); - const pendingGroupAdditionDAL = pendingGroupAdditionDALFactory(db); const secretScanningDAL = secretScanningDALFactory(db); const licenseDAL = licenseDALFactory(db); const dynamicSecretDAL = dynamicSecretDALFactory(db); @@ -270,7 +268,6 @@ export const registerRoutes = async ( projectDAL, projectBotDAL, projectKeyDAL, - pendingGroupAdditionDAL, permissionService, licenseService }); @@ -297,7 +294,6 @@ export const registerRoutes = async ( userGroupMembershipDAL, projectKeyDAL, projectBotDAL, - pendingGroupAdditionDAL, permissionService, smtpService }); @@ -352,7 +348,6 @@ export const registerRoutes = async ( smtpService, authDAL, userDAL, - pendingGroupAdditionDAL, userGroupMembershipDAL, projectKeyDAL, projectDAL, diff --git a/backend/src/services/auth/auth-signup-service.ts b/backend/src/services/auth/auth-signup-service.ts index be80c4c40..9bd8db002 100644 --- a/backend/src/services/auth/auth-signup-service.ts +++ b/backend/src/services/auth/auth-signup-service.ts @@ -2,7 +2,6 @@ import jwt from "jsonwebtoken"; import { OrgMembershipStatus } from "@app/db/schemas"; import { convertPendingGroupAdditionsToGroupMemberships } from "@app/ee/services/group/group-fns"; -import { TPendingGroupAdditionDALFactory } from "@app/ee/services/group/pending-group-addition-dal"; import { TUserGroupMembershipDALFactory } from "@app/ee/services/group/user-group-membership-dal"; import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; import { getConfig } from "@app/lib/config/env"; @@ -27,8 +26,10 @@ import { AuthMethod, AuthTokenType } from "./auth-type"; type TAuthSignupDep = { authDAL: TAuthDALFactory; userDAL: TUserDALFactory; - pendingGroupAdditionDAL: Pick; - userGroupMembershipDAL: Pick; + userGroupMembershipDAL: Pick< + TUserGroupMembershipDALFactory, + "find" | "transaction" | "insertMany" | "deletePendingUserGroupMembershipsByUserIds" + >; projectKeyDAL: Pick; projectDAL: Pick; projectBotDAL: Pick; @@ -44,7 +45,6 @@ export type TAuthSignupFactory = ReturnType; export const authSignupServiceFactory = ({ authDAL, userDAL, - pendingGroupAdditionDAL, userGroupMembershipDAL, projectKeyDAL, projectDAL, @@ -190,9 +190,7 @@ export const authSignupServiceFactory = ({ await convertPendingGroupAdditionsToGroupMemberships({ userIds: [user.id], userDAL, - pendingGroupAdditionDAL, userGroupMembershipDAL, - orgDAL, groupProjectDAL, projectKeyDAL, projectDAL, @@ -304,9 +302,7 @@ export const authSignupServiceFactory = ({ await convertPendingGroupAdditionsToGroupMemberships({ userIds: [user.id], userDAL, - pendingGroupAdditionDAL, userGroupMembershipDAL, - orgDAL, groupProjectDAL, projectKeyDAL, projectDAL, diff --git a/backend/src/services/user/user-dal.ts b/backend/src/services/user/user-dal.ts index 4c61c3174..530ca3ad1 100644 --- a/backend/src/services/user/user-dal.ts +++ b/backend/src/services/user/user-dal.ts @@ -34,16 +34,16 @@ export const userDALFactory = (db: TDbClient) => { } }; - const findUserEncKeyByUsernameBatch = async ({ usernames }: { usernames: string[] }, tx?: Knex) => { + const findUserEncKeyByUserIdsBatch = async ({ userIds }: { userIds: string[] }, tx?: Knex) => { try { return await (tx || db)(TableName.Users) .where({ isGhost: false }) - .whereIn("username", usernames) + .whereIn(`${TableName.Users}.id`, userIds) .join(TableName.UserEncryptionKey, `${TableName.Users}.id`, `${TableName.UserEncryptionKey}.userId`); } catch (error) { - throw new DatabaseError({ error, name: "Find user enc by email batch" }); + throw new DatabaseError({ error, name: "Find user enc by user ids batch" }); } }; @@ -136,7 +136,7 @@ export const userDALFactory = (db: TDbClient) => { ...userOrm, findUserByUsername, findUserEncKeyByUsername, - findUserEncKeyByUsernameBatch, // TODO: if successful, replace findUserEncKeyByUsername with this + findUserEncKeyByUserIdsBatch, findUserEncKeyByUserId, updateUserEncryptionByUserId, findUserByProjectMembershipId,