diff --git a/backend/src/ee/routes/v1/group-router.ts b/backend/src/ee/routes/v1/group-router.ts index 355a259d53..9d5859dd5b 100644 --- a/backend/src/ee/routes/v1/group-router.ts +++ b/backend/src/ee/routes/v1/group-router.ts @@ -458,7 +458,9 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { machineIdentityId: z.string().trim().describe(GROUPS.ADD_MACHINE_IDENTITY.machineIdentityId) }), response: { - 200: GroupIdentityResponseSchema + 200: z.object({ + id: z.string() + }) } }, handler: async (req) => { @@ -528,7 +530,9 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { machineIdentityId: z.string().trim().describe(GROUPS.DELETE_MACHINE_IDENTITY.machineIdentityId) }), response: { - 200: GroupIdentityResponseSchema + 200: z.object({ + id: z.string() + }) } }, handler: async (req) => { diff --git a/backend/src/ee/services/group/group-fns.ts b/backend/src/ee/services/group/group-fns.ts index 13cede3867..4f988ca70d 100644 --- a/backend/src/ee/services/group/group-fns.ts +++ b/backend/src/ee/services/group/group-fns.ts @@ -291,21 +291,34 @@ export const addUsersToGroupByUserIds = async ({ * Add identities with identity ids [identityIds] to group [group]. * @param {group} group - group to add identity(s) to * @param {string[]} identityIds - id(s) of organization scoped identity(s) to add to group + * @returns {Promise<{ id: string }[]>} - id(s) of added identity(s) */ export const addIdentitiesToGroup = async ({ group, identityIds, identityDAL, - identityOrgMembershipDAL, - identityGroupMembershipDAL + identityGroupMembershipDAL, + membershipDAL }: TAddIdentitiesToGroup) => { return identityDAL.transaction(async (tx) => { const identityIdsSet = new Set(identityIds); const identityIdsArray = Array.from(identityIdsSet); - const foundIdentities = await identityOrgMembershipDAL.findByIds(identityIdsArray, group.orgId, tx); + // ensure all identities exist and belong to the org via org scoped membership + const foundIdentitiesMemberships = await membershipDAL.find( + { + scope: AccessScope.Organization, + scopeOrgId: group.orgId, + $in: { + actorIdentityId: identityIdsArray + } + }, + { tx } + ); - const existingIdentityOrgMembershipsIdentityIdsSet = new Set(foundIdentities.map((u) => u.id)); + const existingIdentityOrgMembershipsIdentityIdsSet = new Set( + foundIdentitiesMemberships.map((u) => u.actorIdentityId as string) + ); identityIdsArray.forEach((identityId) => { if (!existingIdentityOrgMembershipsIdentityIdsSet.has(identityId)) { @@ -333,14 +346,14 @@ export const addIdentitiesToGroup = async ({ } await identityGroupMembershipDAL.insertMany( - foundIdentities.map((identity) => ({ - identityId: identity.id, + foundIdentitiesMemberships.map((membership) => ({ + identityId: membership.actorIdentityId as string, groupId: group.id })), tx ); - return foundIdentities; + return identityIdsArray.map((identityId) => ({ id: identityId })); }); }; @@ -484,26 +497,42 @@ export const removeUsersFromGroupByUserIds = async ({ * Remove identities with identity ids [identityIds] from group [group]. * @param {group} group - group to remove identity(s) from * @param {string[]} identityIds - id(s) of identity(s) to remove from group + * @returns {Promise<{ id: string }[]>} - id(s) of removed identity(s) */ export const removeIdentitiesFromGroup = async ({ group, identityIds, identityDAL, - identityOrgMembershipDAL, + membershipDAL, identityGroupMembershipDAL }: TRemoveIdentitiesFromGroup) => { return identityDAL.transaction(async (tx) => { const identityIdsSet = new Set(identityIds); const identityIdsArray = Array.from(identityIdsSet); - const foundIdentities = await identityOrgMembershipDAL.findByIds(identityIdsArray, group.orgId, tx); + // ensure all identities exist and belong to the org via org scoped membership + const foundIdentitiesMemberships = await membershipDAL.find( + { + scope: AccessScope.Organization, + scopeOrgId: group.orgId, + $in: { + actorIdentityId: identityIdsArray + } + }, + { tx } + ); - if (foundIdentities.length !== identityIdsArray.length) { + const foundIdentitiesMembershipsIdentityIdsSet = new Set( + foundIdentitiesMemberships.map((u) => u.actorIdentityId as string) + ); + + if (foundIdentitiesMembershipsIdentityIdsSet.size !== identityIdsArray.length) { throw new NotFoundError({ message: `Machine identities not found` }); } + // check if identity group membership already exists const existingIdentityGroupMemberships = await identityGroupMembershipDAL.find( { groupId: group.id, @@ -536,7 +565,7 @@ export const removeIdentitiesFromGroup = async ({ tx ); - return foundIdentities; + return identityIdsArray.map((identityId) => ({ id: identityId })); }); }; diff --git a/backend/src/ee/services/group/group-service.ts b/backend/src/ee/services/group/group-service.ts index da9756046f..30d047a776 100644 --- a/backend/src/ee/services/group/group-service.ts +++ b/backend/src/ee/services/group/group-service.ts @@ -6,7 +6,7 @@ import { TOidcConfigDALFactory } from "@app/ee/services/oidc/oidc-config-dal"; import { BadRequestError, NotFoundError, PermissionBoundaryError, UnauthorizedError } from "@app/lib/errors"; import { alphaNumericNanoId } from "@app/lib/nanoid"; import { TIdentityDALFactory } from "@app/services/identity/identity-dal"; -import { TIdentityOrgDALFactory } from "@app/services/identity/identity-org-dal"; +import { TMembershipDALFactory } from "@app/services/membership/membership-dal"; import { TMembershipRoleDALFactory } from "@app/services/membership/membership-role-dal"; import { TMembershipGroupDALFactory } from "@app/services/membership-group/membership-group-dal"; import { TOrgDALFactory } from "@app/services/org/org-dal"; @@ -46,7 +46,6 @@ import { TUserGroupMembershipDALFactory } from "./user-group-membership-dal"; type TGroupServiceFactoryDep = { userDAL: Pick; identityDAL: Pick; - identityOrgMembershipDAL: Pick; identityGroupMembershipDAL: Pick; groupDAL: Pick< TGroupDALFactory, @@ -62,6 +61,7 @@ type TGroupServiceFactoryDep = { | "findAllGroupProjects" >; membershipGroupDAL: Pick; + membershipDAL: Pick; membershipRoleDAL: Pick; orgDAL: Pick; userGroupMembershipDAL: Pick< @@ -83,7 +83,7 @@ export type TGroupServiceFactory = ReturnType; export const groupServiceFactory = ({ identityDAL, - identityOrgMembershipDAL, + membershipDAL, identityGroupMembershipDAL, userDAL, groupDAL, @@ -672,17 +672,21 @@ export const groupServiceFactory = ({ details: { missingPermissions: permissionBoundary.missingPermissions } }); - const [identity] = await identityOrgMembershipDAL.findByIds([identityId], group.orgId); + const identityMembership = await membershipDAL.findOne({ + scope: AccessScope.Organization, + scopeOrgId: group.orgId, + actorIdentityId: identityId + }); - if (!identity) { + if (!identityMembership) { throw new NotFoundError({ message: `Identity with id ${identityId} is not part of the organization` }); } const identities = await addIdentitiesToGroup({ group, - identityIds: [identity.id], + identityIds: [identityId], identityDAL, - identityOrgMembershipDAL, + membershipDAL, identityGroupMembershipDAL }); @@ -824,16 +828,21 @@ export const groupServiceFactory = ({ details: { missingPermissions: permissionBoundary.missingPermissions } }); - const [identity] = await identityOrgMembershipDAL.findByIds([identityId], group.orgId); + const identityMembership = await membershipDAL.findOne({ + scope: AccessScope.Organization, + scopeOrgId: group.orgId, + actorIdentityId: identityId + }); - if (!identity) + if (!identityMembership) { throw new NotFoundError({ message: `Identity with id ${identityId} is not part of the organization` }); + } const identities = await removeIdentitiesFromGroup({ group, - identityIds: [identity.id], + identityIds: [identityId], identityDAL, - identityOrgMembershipDAL, + membershipDAL, identityGroupMembershipDAL }); diff --git a/backend/src/ee/services/group/group-types.ts b/backend/src/ee/services/group/group-types.ts index e1f2d030dc..044dc4bca1 100644 --- a/backend/src/ee/services/group/group-types.ts +++ b/backend/src/ee/services/group/group-types.ts @@ -4,7 +4,7 @@ import { TGroups } from "@app/db/schemas"; import { TUserGroupMembershipDALFactory } from "@app/ee/services/group/user-group-membership-dal"; import { OrderByDirection, TGenericPermission } from "@app/lib/types"; import { TIdentityDALFactory } from "@app/services/identity/identity-dal"; -import { TIdentityOrgDALFactory } from "@app/services/identity/identity-org-dal"; +import { TMembershipDALFactory } from "@app/services/membership/membership-dal"; import { TMembershipGroupDALFactory } from "@app/services/membership-group/membership-group-dal"; import { TOrgDALFactory } from "@app/services/org/org-dal"; import { TProjectDALFactory } from "@app/services/project/project-dal"; @@ -129,8 +129,8 @@ export type TAddIdentitiesToGroup = { group: TGroups; identityIds: string[]; identityDAL: Pick; - identityOrgMembershipDAL: Pick; identityGroupMembershipDAL: Pick; + membershipDAL: Pick; }; export type TRemoveUsersFromGroupByUserIds = { @@ -147,7 +147,7 @@ export type TRemoveIdentitiesFromGroup = { group: TGroups; identityIds: string[]; identityDAL: Pick; - identityOrgMembershipDAL: Pick; + membershipDAL: Pick; identityGroupMembershipDAL: Pick; }; diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index 76c038bdd2..640c4e9a49 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -757,7 +757,7 @@ export const registerRoutes = async ( }); const groupService = groupServiceFactory({ identityDAL, - identityOrgMembershipDAL, + membershipDAL, identityGroupMembershipDAL, userDAL, groupDAL, diff --git a/backend/src/services/identity/identity-org-dal.ts b/backend/src/services/identity/identity-org-dal.ts index 5dac91ad23..117195ca71 100644 --- a/backend/src/services/identity/identity-org-dal.ts +++ b/backend/src/services/identity/identity-org-dal.ts @@ -674,24 +674,5 @@ export const identityOrgDALFactory = (db: TDbClient) => { } }; - const findByIds = async (identityIds: string[], orgId: string, tx?: Knex) => { - try { - const identities = await (tx || db.replicaNode())(TableName.Identity) - .join(TableName.Membership, `${TableName.Membership}.actorIdentityId`, `${TableName.Identity}.id`) - .where(`${TableName.Membership}.scope`, AccessScope.Organization) - .where(`${TableName.Membership}.scopeOrgId`, orgId) - .whereNotNull(`${TableName.Membership}.actorIdentityId`) - .whereNull(`${TableName.Identity}.projectId`) - .whereIn(`${TableName.Identity}.id`, identityIds) - .distinctOn(`${TableName.Identity}.id`) - .orderBy(`${TableName.Identity}.id`) - .select(selectAllTableCols(TableName.Identity)); - - return identities; - } catch (error) { - throw new DatabaseError({ error, name: "findByIds" }); - } - }; - - return { find, findOne, countAllOrgIdentities, searchIdentities, findByIds }; + return { find, findOne, countAllOrgIdentities, searchIdentities }; };