From a1d00f2c41a171451135375048b3f11d07e04dd4 Mon Sep 17 00:00:00 2001 From: Tuan Dang Date: Tue, 16 Jul 2024 20:19:27 +0700 Subject: [PATCH 1/2] Add SCIM user activation/deactivation --- ...0715113110_org-membership-active-status.ts | 25 ++++++ backend/src/db/schemas/org-memberships.ts | 3 +- backend/src/db/seeds/2-org.ts | 3 +- backend/src/ee/routes/v1/scim-router.ts | 16 +++- .../group/user-group-membership-dal.ts | 17 +++- .../ldap-config/ldap-config-service.ts | 6 +- .../ee/services/oidc/oidc-config-service.ts | 6 +- .../saml-config/saml-config-service.ts | 6 +- backend/src/ee/services/scim/scim-fns.ts | 13 ++- backend/src/ee/services/scim/scim-service.ts | 83 ++++++++++++------- backend/src/ee/services/scim/scim-types.ts | 5 +- backend/src/server/routes/index.ts | 2 +- .../services/auth-token/auth-token-service.ts | 16 +++- backend/src/services/org/org-dal.ts | 1 + backend/src/services/org/org-service.ts | 12 ++- frontend/src/hooks/api/users/types.ts | 1 + .../OrgMembersSection/OrgMembersTable.tsx | 22 +++-- 17 files changed, 182 insertions(+), 55 deletions(-) create mode 100644 backend/src/db/migrations/20240715113110_org-membership-active-status.ts diff --git a/backend/src/db/migrations/20240715113110_org-membership-active-status.ts b/backend/src/db/migrations/20240715113110_org-membership-active-status.ts new file mode 100644 index 000000000..72a33a6d6 --- /dev/null +++ b/backend/src/db/migrations/20240715113110_org-membership-active-status.ts @@ -0,0 +1,25 @@ +import { Knex } from "knex"; + +import { TableName } from "../schemas"; + +export async function up(knex: Knex): Promise { + if (await knex.schema.hasTable(TableName.OrgMembership)) { + await knex.schema.alterTable(TableName.OrgMembership, (t) => { + t.boolean("isActive").nullable(); + }); + + await knex(TableName.OrgMembership).update("isActive", true); + + await knex.schema.alterTable(TableName.OrgMembership, (t) => { + t.boolean("isActive").notNullable().alter(); + }); + } +} + +export async function down(knex: Knex): Promise { + if (await knex.schema.hasTable(TableName.OrgMembership)) { + await knex.schema.alterTable(TableName.OrgMembership, (t) => { + t.dropColumn("isActive"); + }); + } +} diff --git a/backend/src/db/schemas/org-memberships.ts b/backend/src/db/schemas/org-memberships.ts index b1858e5be..7fc6f46eb 100644 --- a/backend/src/db/schemas/org-memberships.ts +++ b/backend/src/db/schemas/org-memberships.ts @@ -17,7 +17,8 @@ export const OrgMembershipsSchema = z.object({ userId: z.string().uuid().nullable().optional(), orgId: z.string().uuid(), roleId: z.string().uuid().nullable().optional(), - projectFavorites: z.string().array().nullable().optional() + projectFavorites: z.string().array().nullable().optional(), + isActive: z.boolean() }); export type TOrgMemberships = z.infer; diff --git a/backend/src/db/seeds/2-org.ts b/backend/src/db/seeds/2-org.ts index ba2f65a36..a02224dbc 100644 --- a/backend/src/db/seeds/2-org.ts +++ b/backend/src/db/seeds/2-org.ts @@ -29,7 +29,8 @@ export async function seed(knex: Knex): Promise { role: OrgMembershipRole.Admin, orgId: org.id, status: OrgMembershipStatus.Accepted, - userId: user.id + userId: user.id, + isActive: true } ]); } diff --git a/backend/src/ee/routes/v1/scim-router.ts b/backend/src/ee/routes/v1/scim-router.ts index 3fdcabc8a..88a1df457 100644 --- a/backend/src/ee/routes/v1/scim-router.ts +++ b/backend/src/ee/routes/v1/scim-router.ts @@ -186,7 +186,13 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }) ), displayName: z.string().trim(), - active: z.boolean() + active: z.boolean(), + groups: z.array( + z.object({ + value: z.string().trim(), + display: z.string().trim() + }) + ) }) } }, @@ -572,7 +578,13 @@ export const registerScimRouter = async (server: FastifyZodProvider) => { }) ), displayName: z.string().trim(), - active: z.boolean() + active: z.boolean(), + groups: z.array( + z.object({ + value: z.string().trim(), + display: z.string().trim() + }) + ) }) } }, 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 e20cf317b..64983d24f 100644 --- a/backend/src/ee/services/group/user-group-membership-dal.ts +++ b/backend/src/ee/services/group/user-group-membership-dal.ts @@ -162,11 +162,26 @@ export const userGroupMembershipDALFactory = (db: TDbClient) => { } }; + const findUserGroupMembershipsInOrg = async (userId: string, orgId: string) => { + try { + const docs = await db + .replicaNode()(TableName.UserGroupMembership) + .join(TableName.Groups, `${TableName.UserGroupMembership}.groupId`, `${TableName.Groups}.id`) + .where(`${TableName.UserGroupMembership}.userId`, userId) + .where(`${TableName.Groups}.orgId`, orgId); + + return docs; + } catch (error) { + throw new DatabaseError({ error, name: "findTest" }); + } + }; + return { ...userGroupMembershipOrm, filterProjectsByUserMembership, findUserGroupMembershipsInProject, findGroupMembersNotInProject, - deletePendingUserGroupMembershipsByUserIds + deletePendingUserGroupMembershipsByUserIds, + findUserGroupMembershipsInOrg }; }; diff --git a/backend/src/ee/services/ldap-config/ldap-config-service.ts b/backend/src/ee/services/ldap-config/ldap-config-service.ts index e1b2e011a..6d09d5025 100644 --- a/backend/src/ee/services/ldap-config/ldap-config-service.ts +++ b/backend/src/ee/services/ldap-config/ldap-config-service.ts @@ -449,7 +449,8 @@ export const ldapConfigServiceFactory = ({ userId: userAlias.userId, orgId, role: OrgMembershipRole.Member, - status: OrgMembershipStatus.Accepted + status: OrgMembershipStatus.Accepted, + isActive: true }, tx ); @@ -534,7 +535,8 @@ export const ldapConfigServiceFactory = ({ inviteEmail: email, orgId, role: OrgMembershipRole.Member, - status: newUser.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 + status: newUser.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 }, tx ); diff --git a/backend/src/ee/services/oidc/oidc-config-service.ts b/backend/src/ee/services/oidc/oidc-config-service.ts index 55c929b9a..f86f29646 100644 --- a/backend/src/ee/services/oidc/oidc-config-service.ts +++ b/backend/src/ee/services/oidc/oidc-config-service.ts @@ -193,7 +193,8 @@ export const oidcConfigServiceFactory = ({ inviteEmail: email, orgId, role: OrgMembershipRole.Member, - status: foundUser.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 + status: foundUser.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 }, tx ); @@ -266,7 +267,8 @@ export const oidcConfigServiceFactory = ({ inviteEmail: email, orgId, role: OrgMembershipRole.Member, - status: newUser.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 + status: newUser.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 }, tx ); 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 c147b7e27..5779b73f6 100644 --- a/backend/src/ee/services/saml-config/saml-config-service.ts +++ b/backend/src/ee/services/saml-config/saml-config-service.ts @@ -370,7 +370,8 @@ export const samlConfigServiceFactory = ({ inviteEmail: email, orgId, role: OrgMembershipRole.Member, - status: foundUser.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 + status: foundUser.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 }, tx ); @@ -457,7 +458,8 @@ export const samlConfigServiceFactory = ({ inviteEmail: email, orgId, role: OrgMembershipRole.Member, - status: newUser.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 + status: newUser.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 }, tx ); diff --git a/backend/src/ee/services/scim/scim-fns.ts b/backend/src/ee/services/scim/scim-fns.ts index 08b652185..cdfe63f43 100644 --- a/backend/src/ee/services/scim/scim-fns.ts +++ b/backend/src/ee/services/scim/scim-fns.ts @@ -32,12 +32,19 @@ export const parseScimFilter = (filterToParse: string | undefined) => { return { [attributeName]: parsedValue.replace(/"/g, "") }; }; +export function extractScimValueFromPath(path: string): string | null { + const regex = /members\[value eq "([^"]+)"\]/; + const match = path.match(regex); + return match ? match[1] : null; +} + export const buildScimUser = ({ orgMembershipId, username, email, firstName, lastName, + groups = [], active }: { orgMembershipId: string; @@ -45,6 +52,10 @@ export const buildScimUser = ({ email?: string | null; firstName: string; lastName: string; + groups?: { + value: string; + display: string; + }[]; active: boolean; }): TScimUser => { const scimUser = { @@ -67,7 +78,7 @@ export const buildScimUser = ({ ] : [], active, - groups: [], + groups, meta: { resourceType: "User", location: null diff --git a/backend/src/ee/services/scim/scim-service.ts b/backend/src/ee/services/scim/scim-service.ts index 4d15676b8..2f39e9168 100644 --- a/backend/src/ee/services/scim/scim-service.ts +++ b/backend/src/ee/services/scim/scim-service.ts @@ -30,7 +30,14 @@ import { UserAliasType } from "@app/services/user-alias/user-alias-types"; import { TLicenseServiceFactory } from "../license/license-service"; import { OrgPermissionActions, OrgPermissionSubjects } from "../permission/org-permission"; import { TPermissionServiceFactory } from "../permission/permission-service"; -import { buildScimGroup, buildScimGroupList, buildScimUser, buildScimUserList, parseScimFilter } from "./scim-fns"; +import { + buildScimGroup, + buildScimGroupList, + buildScimUser, + buildScimUserList, + extractScimValueFromPath, + parseScimFilter +} from "./scim-fns"; import { TCreateScimGroupDTO, TCreateScimTokenDTO, @@ -61,7 +68,7 @@ type TScimServiceFactoryDep = { TOrgDALFactory, "createMembership" | "findById" | "findMembership" | "deleteMembershipById" | "transaction" | "updateMembershipById" >; - orgMembershipDAL: Pick; + orgMembershipDAL: Pick; projectDAL: Pick; projectMembershipDAL: Pick; groupDAL: Pick< @@ -71,7 +78,12 @@ type TScimServiceFactoryDep = { groupProjectDAL: Pick; userGroupMembershipDAL: Pick< TUserGroupMembershipDALFactory, - "find" | "transaction" | "insertMany" | "filterProjectsByUserMembership" | "delete" + | "find" + | "transaction" + | "insertMany" + | "filterProjectsByUserMembership" + | "delete" + | "findUserGroupMembershipsInOrg" >; projectKeyDAL: Pick; projectBotDAL: Pick; @@ -197,14 +209,14 @@ export const scimServiceFactory = ({ findOpts ); - const scimUsers = users.map(({ id, externalId, username, firstName, lastName, email }) => + const scimUsers = users.map(({ id, externalId, username, firstName, lastName, email, isActive }) => buildScimUser({ orgMembershipId: id ?? "", username: externalId ?? username, firstName: firstName ?? "", lastName: lastName ?? "", email, - active: true + active: isActive }) ); @@ -240,13 +252,19 @@ export const scimServiceFactory = ({ status: 403 }); + const groupMembershipsInOrg = await userGroupMembershipDAL.findUserGroupMembershipsInOrg(membership.userId, orgId); + return buildScimUser({ orgMembershipId: membership.id, username: membership.externalId ?? membership.username, email: membership.email ?? "", firstName: membership.firstName as string, lastName: membership.lastName as string, - active: true + active: membership.isActive, + groups: groupMembershipsInOrg.map((group) => ({ + value: group.groupId, + display: group.name + })) }); }; @@ -296,7 +314,8 @@ export const scimServiceFactory = ({ inviteEmail: email, orgId, role: OrgMembershipRole.Member, - 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 + 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 }, tx ); @@ -364,7 +383,8 @@ export const scimServiceFactory = ({ inviteEmail: email, orgId, role: OrgMembershipRole.Member, - 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 + 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 }, tx ); @@ -401,7 +421,7 @@ export const scimServiceFactory = ({ firstName: createdUser.firstName as string, lastName: createdUser.lastName as string, email: createdUser.email ?? "", - active: true + active: createdOrgMembership.isActive }); }; @@ -445,14 +465,8 @@ export const scimServiceFactory = ({ }); if (!active) { - await deleteOrgMembershipFn({ - orgMembershipId: membership.id, - orgId: membership.orgId, - orgDAL, - projectMembershipDAL, - projectKeyDAL, - userAliasDAL, - licenseService + await orgMembershipDAL.updateById(membership.id, { + isActive: false }); } @@ -491,17 +505,11 @@ export const scimServiceFactory = ({ status: 403 }); - if (!active) { - await deleteOrgMembershipFn({ - orgMembershipId: membership.id, - orgId: membership.orgId, - orgDAL, - projectMembershipDAL, - projectKeyDAL, - userAliasDAL, - licenseService - }); - } + await orgMembershipDAL.updateById(membership.id, { + isActive: active + }); + + const groupMembershipsInOrg = await userGroupMembershipDAL.findUserGroupMembershipsInOrg(membership.userId, orgId); return buildScimUser({ orgMembershipId: membership.id, @@ -509,7 +517,11 @@ export const scimServiceFactory = ({ email: membership.email, firstName: membership.firstName as string, lastName: membership.lastName as string, - active + active, + groups: groupMembershipsInOrg.map((group) => ({ + value: group.groupId, + display: group.name + })) }); }; @@ -881,7 +893,18 @@ export const scimServiceFactory = ({ break; } case "remove": { - // TODO + 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 + }); break; } default: { diff --git a/backend/src/ee/services/scim/scim-types.ts b/backend/src/ee/services/scim/scim-types.ts index f00336fd2..51caf585b 100644 --- a/backend/src/ee/services/scim/scim-types.ts +++ b/backend/src/ee/services/scim/scim-types.ts @@ -158,7 +158,10 @@ export type TScimUser = { type: string; }[]; active: boolean; - groups: string[]; + groups: { + value: string; + display: string; + }[]; meta: { resourceType: string; location: null; diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index c845de988..3f96106fe 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -331,7 +331,7 @@ export const registerRoutes = async ( permissionService, secretApprovalPolicyDAL }); - const tokenService = tokenServiceFactory({ tokenDAL: authTokenDAL, userDAL }); + const tokenService = tokenServiceFactory({ tokenDAL: authTokenDAL, userDAL, orgMembershipDAL }); const samlService = samlConfigServiceFactory({ permissionService, diff --git a/backend/src/services/auth-token/auth-token-service.ts b/backend/src/services/auth-token/auth-token-service.ts index b1f8aa2f6..3610bcded 100644 --- a/backend/src/services/auth-token/auth-token-service.ts +++ b/backend/src/services/auth-token/auth-token-service.ts @@ -4,7 +4,8 @@ import bcrypt from "bcrypt"; import { TAuthTokens, TAuthTokenSessions } from "@app/db/schemas"; import { getConfig } from "@app/lib/config/env"; -import { UnauthorizedError } from "@app/lib/errors"; +import { ForbiddenRequestError, UnauthorizedError } from "@app/lib/errors"; +import { TOrgMembershipDALFactory } from "@app/services/org-membership/org-membership-dal"; import { AuthModeJwtTokenPayload } from "../auth/auth-type"; import { TUserDALFactory } from "../user/user-dal"; @@ -14,6 +15,7 @@ import { TCreateTokenForUserDTO, TIssueAuthTokenDTO, TokenType, TValidateTokenFo type TAuthTokenServiceFactoryDep = { tokenDAL: TTokenDALFactory; userDAL: Pick; + orgMembershipDAL: Pick; }; export type TAuthTokenServiceFactory = ReturnType; @@ -67,7 +69,7 @@ export const getTokenConfig = (tokenType: TokenType) => { } }; -export const tokenServiceFactory = ({ tokenDAL, userDAL }: TAuthTokenServiceFactoryDep) => { +export const tokenServiceFactory = ({ tokenDAL, userDAL, orgMembershipDAL }: TAuthTokenServiceFactoryDep) => { const createTokenForUser = async ({ type, userId, orgId }: TCreateTokenForUserDTO) => { const { token, ...tkCfg } = getTokenConfig(type); const appCfg = getConfig(); @@ -154,6 +156,16 @@ export const tokenServiceFactory = ({ tokenDAL, userDAL }: TAuthTokenServiceFact const user = await userDAL.findById(session.userId); if (!user || !user.isAccepted) throw new UnauthorizedError({ name: "Token user not found" }); + if (token.organizationId) { + const orgMembership = await orgMembershipDAL.findOne({ + userId: user.id, + orgId: token.organizationId + }); + + if (!orgMembership) throw new ForbiddenRequestError({ message: "User not member of organization" }); + if (!orgMembership.isActive) throw new ForbiddenRequestError({ message: "User not active in organization" }); + } + return { user, tokenVersionId: token.tokenVersionId, orgId: token.organizationId }; }; diff --git a/backend/src/services/org/org-dal.ts b/backend/src/services/org/org-dal.ts index d518a698a..81dbc43bb 100644 --- a/backend/src/services/org/org-dal.ts +++ b/backend/src/services/org/org-dal.ts @@ -74,6 +74,7 @@ export const orgDALFactory = (db: TDbClient) => { db.ref("role").withSchema(TableName.OrgMembership), db.ref("roleId").withSchema(TableName.OrgMembership), db.ref("status").withSchema(TableName.OrgMembership), + db.ref("isActive").withSchema(TableName.OrgMembership), db.ref("email").withSchema(TableName.Users), db.ref("username").withSchema(TableName.Users), db.ref("firstName").withSchema(TableName.Users), diff --git a/backend/src/services/org/org-service.ts b/backend/src/services/org/org-service.ts index 248bab568..81be7ed8a 100644 --- a/backend/src/services/org/org-service.ts +++ b/backend/src/services/org/org-service.ts @@ -207,7 +207,8 @@ export const orgServiceFactory = ({ orgId, userId: user.id, role: OrgMembershipRole.Admin, - status: OrgMembershipStatus.Accepted + status: OrgMembershipStatus.Accepted, + isActive: true }; await orgDAL.createMembership(createMembershipData, tx); @@ -311,7 +312,8 @@ export const orgServiceFactory = ({ userId, orgId: org.id, role: OrgMembershipRole.Admin, - status: OrgMembershipStatus.Accepted + status: OrgMembershipStatus.Accepted, + isActive: true }, tx ); @@ -460,7 +462,8 @@ export const orgServiceFactory = ({ inviteEmail: inviteeEmail, orgId, role: OrgMembershipRole.Member, - status: OrgMembershipStatus.Invited + status: OrgMembershipStatus.Invited, + isActive: true }, tx ); @@ -491,7 +494,8 @@ export const orgServiceFactory = ({ orgId, userId: user.id, role: OrgMembershipRole.Member, - status: OrgMembershipStatus.Invited + status: OrgMembershipStatus.Invited, + isActive: true }, tx ); diff --git a/frontend/src/hooks/api/users/types.ts b/frontend/src/hooks/api/users/types.ts index 43e408571..a3793f942 100644 --- a/frontend/src/hooks/api/users/types.ts +++ b/frontend/src/hooks/api/users/types.ts @@ -60,6 +60,7 @@ export type OrgUser = { status: "invited" | "accepted" | "verified" | "completed"; deniedPermissions: any[]; roleId: string; + isActive: boolean; }; export type TProjectMembership = { diff --git a/frontend/src/views/Org/MembersPage/components/OrgMembersTab/components/OrgMembersSection/OrgMembersTable.tsx b/frontend/src/views/Org/MembersPage/components/OrgMembersTab/components/OrgMembersSection/OrgMembersTable.tsx index a14e09a67..c62b31592 100644 --- a/frontend/src/views/Org/MembersPage/components/OrgMembersTab/components/OrgMembersSection/OrgMembersTable.tsx +++ b/frontend/src/views/Org/MembersPage/components/OrgMembersTab/components/OrgMembersSection/OrgMembersTable.tsx @@ -171,14 +171,14 @@ export const OrgMembersTable = ({ handlePopUpOpen, setCompleteInviteLink }: Prop {isLoading && } {!isLoading && filterdUser?.map( - ({ user: u, inviteEmail, role, roleId, id: orgMembershipId, status }) => { + ({ user: u, inviteEmail, role, roleId, id: orgMembershipId, status, isActive }) => { const name = u && u.firstName ? `${u.firstName} ${u.lastName}` : "-"; const email = u?.email || inviteEmail; const username = u?.username ?? inviteEmail ?? "-"; return ( - {name} - {username} + {name} + {username} {(isAllowed) => ( <> - {status === "accepted" && ( + {!isActive && ( + + )} + {isActive && status === "accepted" && (