From 36916704be1a6a0eb54d811f6c4428ce3a2f85bd Mon Sep 17 00:00:00 2001 From: carlosmonastyrski Date: Fri, 2 May 2025 11:14:43 -0300 Subject: [PATCH 1/6] Add groups to ssh hosts allowed principals --- ...-groups-to-ssh-host-login-user-mappings.ts | 22 +++ .../schemas/ssh-host-login-user-mappings.ts | 3 +- backend/src/ee/services/group/group-dal.ts | 17 ++ .../group/user-group-membership-dal.ts | 3 +- .../ee/services/permission/permission-dal.ts | 7 +- .../services/permission/permission-service.ts | 31 +++- .../src/ee/services/ssh-host/ssh-host-dal.ts | 23 ++- .../ee/services/ssh-host/ssh-host-schema.ts | 23 ++- .../ee/services/ssh-host/ssh-host-service.ts | 93 +++++++++- .../ee/services/ssh-host/ssh-host-types.ts | 6 +- backend/src/lib/api-docs/constants.ts | 4 +- backend/src/server/routes/index.ts | 2 + .../secret-v2-bridge/secret-v2-bridge-dal.ts | 2 +- frontend/src/hooks/api/sshHost/types.ts | 3 + .../SshHostsPage/components/SshHostModal.tsx | 172 ++++++++++++------ .../SshHostsPage/components/SshHostsTable.tsx | 52 +++++- 16 files changed, 381 insertions(+), 82 deletions(-) create mode 100644 backend/src/db/migrations/20250501164905_add-groups-to-ssh-host-login-user-mappings.ts diff --git a/backend/src/db/migrations/20250501164905_add-groups-to-ssh-host-login-user-mappings.ts b/backend/src/db/migrations/20250501164905_add-groups-to-ssh-host-login-user-mappings.ts new file mode 100644 index 000000000..4f08146f9 --- /dev/null +++ b/backend/src/db/migrations/20250501164905_add-groups-to-ssh-host-login-user-mappings.ts @@ -0,0 +1,22 @@ +import { Knex } from "knex"; + +import { TableName } from "../schemas"; + +export async function up(knex: Knex): Promise { + if (!(await knex.schema.hasColumn(TableName.SshHostLoginUserMapping, "groupId"))) { + await knex.schema.alterTable(TableName.SshHostLoginUserMapping, (t) => { + t.uuid("groupId").nullable(); + t.foreign("groupId").references("id").inTable(TableName.Groups).onDelete("CASCADE"); + t.unique(["sshHostLoginUserId", "groupId"]); + }); + } +} + +export async function down(knex: Knex): Promise { + if (await knex.schema.hasColumn(TableName.SshHostLoginUserMapping, "groupId")) { + await knex.schema.alterTable(TableName.SshHostLoginUserMapping, (t) => { + t.dropUnique(["sshHostLoginUserId", "groupId"]); + t.dropColumn("groupId"); + }); + } +} diff --git a/backend/src/db/schemas/ssh-host-login-user-mappings.ts b/backend/src/db/schemas/ssh-host-login-user-mappings.ts index 6edb0d5a3..fd5fa460c 100644 --- a/backend/src/db/schemas/ssh-host-login-user-mappings.ts +++ b/backend/src/db/schemas/ssh-host-login-user-mappings.ts @@ -12,7 +12,8 @@ export const SshHostLoginUserMappingsSchema = z.object({ createdAt: z.date(), updatedAt: z.date(), sshHostLoginUserId: z.string().uuid(), - userId: z.string().uuid().nullable().optional() + userId: z.string().uuid().nullable().optional(), + groupId: z.string().uuid().nullable().optional() }); export type TSshHostLoginUserMappings = z.infer; diff --git a/backend/src/ee/services/group/group-dal.ts b/backend/src/ee/services/group/group-dal.ts index 59f82c05d..12a70a15e 100644 --- a/backend/src/ee/services/group/group-dal.ts +++ b/backend/src/ee/services/group/group-dal.ts @@ -157,10 +157,27 @@ export const groupDALFactory = (db: TDbClient) => { } }; + const findGroupsByProjectId = async (projectId: string, tx?: Knex) => { + try { + const docs = await (tx || db.replicaNode())(TableName.Groups) + .leftJoin( + TableName.GroupProjectMembership, + `${TableName.Groups}.id`, + `${TableName.GroupProjectMembership}.groupId` + ) + .where(`${TableName.GroupProjectMembership}.projectId`, projectId) + .select(selectAllTableCols(TableName.Groups)); + return docs; + } catch (error) { + throw new DatabaseError({ error, name: "Find groups by project id" }); + } + }; + return { findGroups, findByOrgId, findAllGroupPossibleMembers, + findGroupsByProjectId, ...groupOrm }; }; 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 be654b338..5ee97e457 100644 --- a/backend/src/ee/services/group/user-group-membership-dal.ts +++ b/backend/src/ee/services/group/user-group-membership-dal.ts @@ -176,7 +176,8 @@ export const userGroupMembershipDALFactory = (db: TDbClient) => { db.ref("name").withSchema(TableName.Groups).as("groupName"), db.ref("id").withSchema(TableName.OrgMembership).as("orgMembershipId"), db.ref("firstName").withSchema(TableName.Users).as("firstName"), - db.ref("lastName").withSchema(TableName.Users).as("lastName") + db.ref("lastName").withSchema(TableName.Users).as("lastName"), + db.ref("slug").withSchema(TableName.Groups).as("groupSlug") ); return docs; diff --git a/backend/src/ee/services/permission/permission-dal.ts b/backend/src/ee/services/permission/permission-dal.ts index 891d7193e..7a17108a2 100644 --- a/backend/src/ee/services/permission/permission-dal.ts +++ b/backend/src/ee/services/permission/permission-dal.ts @@ -132,7 +132,7 @@ export const permissionDALFactory = (db: TDbClient) => { } }; - const getProjectGroupPermissions = async (projectId: string) => { + const getProjectGroupPermissions = async (projectId: string, filterGroupId?: string) => { try { const docs = await db .replicaNode()(TableName.GroupProjectMembership) @@ -148,6 +148,11 @@ export const permissionDALFactory = (db: TDbClient) => { `groupCustomRoles.id` ) .where(`${TableName.GroupProjectMembership}.projectId`, "=", projectId) + .where((bd) => { + if (filterGroupId) { + void bd.where(`${TableName.GroupProjectMembership}.groupId`, "=", filterGroupId); + } + }) .select( db.ref("id").withSchema(TableName.GroupProjectMembership).as("membershipId"), db.ref("id").withSchema(TableName.Groups).as("groupId"), diff --git a/backend/src/ee/services/permission/permission-service.ts b/backend/src/ee/services/permission/permission-service.ts index 3d2f96f82..40988ac80 100644 --- a/backend/src/ee/services/permission/permission-service.ts +++ b/backend/src/ee/services/permission/permission-service.ts @@ -625,6 +625,34 @@ export const permissionServiceFactory = ({ return { permission }; }; + const checkGroupProjectPermission = async ({ + groupId, + projectId, + checkPermissions + }: { + groupId: string; + projectId: string; + checkPermissions: ProjectPermissionSet; + }) => { + const rawGroupProjectPermissions = await permissionDAL.getProjectGroupPermissions(projectId, groupId); + const groupPermissions = rawGroupProjectPermissions.map((groupProjectPermission) => { + const rolePermissions = + groupProjectPermission.roles?.map(({ role, permissions }) => ({ role, permissions })) || []; + const rules = buildProjectPermissionRules(rolePermissions); + const permission = createMongoAbility(rules, { + conditionsMatcher + }); + + return { + permission, + id: groupProjectPermission.groupId, + name: groupProjectPermission.username, + membershipId: groupProjectPermission.id + }; + }); + return groupPermissions.some((groupPermission) => groupPermission.permission.can(...checkPermissions)); + }; + return { getUserOrgPermission, getOrgPermission, @@ -634,6 +662,7 @@ export const permissionServiceFactory = ({ getOrgPermissionByRole, getProjectPermissionByRole, buildOrgPermission, - buildProjectPermissionRules + buildProjectPermissionRules, + checkGroupProjectPermission }; }; diff --git a/backend/src/ee/services/ssh-host/ssh-host-dal.ts b/backend/src/ee/services/ssh-host/ssh-host-dal.ts index 3c9755e65..347dac697 100644 --- a/backend/src/ee/services/ssh-host/ssh-host-dal.ts +++ b/backend/src/ee/services/ssh-host/ssh-host-dal.ts @@ -27,8 +27,17 @@ export const sshHostDALFactory = (db: TDbClient) => { `${TableName.SshHostLoginUserMapping}.sshHostLoginUserId` ) .leftJoin(TableName.Users, `${TableName.Users}.id`, `${TableName.SshHostLoginUserMapping}.userId`) + .leftJoin( + TableName.UserGroupMembership, + `${TableName.UserGroupMembership}.groupId`, + `${TableName.SshHostLoginUserMapping}.groupId` + ) .whereIn(`${TableName.SshHost}.projectId`, projectIds) - .andWhere(`${TableName.SshHostLoginUserMapping}.userId`, userId) + .andWhere((bd) => { + void bd + .where(`${TableName.SshHostLoginUserMapping}.userId`, userId) + .orWhere(`${TableName.UserGroupMembership}.userId`, userId); + }) .select( db.ref("id").withSchema(TableName.SshHost).as("sshHostId"), db.ref("projectId").withSchema(TableName.SshHost), @@ -85,6 +94,7 @@ export const sshHostDALFactory = (db: TDbClient) => { `${TableName.SshHostLoginUserMapping}.sshHostLoginUserId` ) .leftJoin(TableName.Users, `${TableName.SshHostLoginUserMapping}.userId`, `${TableName.Users}.id`) + .leftJoin(TableName.Groups, `${TableName.SshHostLoginUserMapping}.groupId`, `${TableName.Groups}.id`) .where(`${TableName.SshHost}.projectId`, projectId) .select( db.ref("id").withSchema(TableName.SshHost).as("sshHostId"), @@ -96,6 +106,7 @@ export const sshHostDALFactory = (db: TDbClient) => { db.ref("loginUser").withSchema(TableName.SshHostLoginUser), db.ref("username").withSchema(TableName.Users), db.ref("userId").withSchema(TableName.SshHostLoginUserMapping), + db.ref("slug").withSchema(TableName.Groups).as("groupSlug"), db.ref("userSshCaId").withSchema(TableName.SshHost), db.ref("hostSshCaId").withSchema(TableName.SshHost) ) @@ -113,7 +124,8 @@ export const sshHostDALFactory = (db: TDbClient) => { const loginMappings = Object.entries(loginMappingGrouped).map(([loginUser, entries]) => ({ loginUser, allowedPrincipals: { - usernames: unique(entries.map((e) => e.username)).filter(Boolean) + usernames: unique(entries.map((e) => e.username)).filter(Boolean), + groups: unique(entries.map((e) => e.groupSlug)).filter(Boolean) } })); @@ -144,6 +156,7 @@ export const sshHostDALFactory = (db: TDbClient) => { `${TableName.SshHostLoginUserMapping}.sshHostLoginUserId` ) .leftJoin(TableName.Users, `${TableName.SshHostLoginUserMapping}.userId`, `${TableName.Users}.id`) + .leftJoin(TableName.Groups, `${TableName.SshHostLoginUserMapping}.groupId`, `${TableName.Groups}.id`) .where(`${TableName.SshHost}.id`, sshHostId) .select( db.ref("id").withSchema(TableName.SshHost).as("sshHostId"), @@ -156,7 +169,8 @@ export const sshHostDALFactory = (db: TDbClient) => { db.ref("username").withSchema(TableName.Users), db.ref("userId").withSchema(TableName.SshHostLoginUserMapping), db.ref("userSshCaId").withSchema(TableName.SshHost), - db.ref("hostSshCaId").withSchema(TableName.SshHost) + db.ref("hostSshCaId").withSchema(TableName.SshHost), + db.ref("slug").withSchema(TableName.Groups).as("groupSlug") ); if (rows.length === 0) return null; @@ -171,7 +185,8 @@ export const sshHostDALFactory = (db: TDbClient) => { const loginMappings = Object.entries(loginMappingGrouped).map(([loginUser, entries]) => ({ loginUser, allowedPrincipals: { - usernames: unique(entries.map((e) => e.username)).filter(Boolean) + usernames: unique(entries.map((e) => e.username)).filter(Boolean), + groups: unique(entries.map((e) => e.groupSlug)).filter(Boolean) } })); diff --git a/backend/src/ee/services/ssh-host/ssh-host-schema.ts b/backend/src/ee/services/ssh-host/ssh-host-schema.ts index a9b674991..c8acb37bf 100644 --- a/backend/src/ee/services/ssh-host/ssh-host-schema.ts +++ b/backend/src/ee/services/ssh-host/ssh-host-schema.ts @@ -15,7 +15,24 @@ export const sanitizedSshHost = SshHostsSchema.pick({ export const loginMappingSchema = z.object({ loginUser: z.string().trim(), - allowedPrincipals: z.object({ - usernames: z.array(z.string().trim()).transform((usernames) => Array.from(new Set(usernames))) - }) + allowedPrincipals: z + .object({ + usernames: z + .array(z.string().trim()) + .transform((usernames) => Array.from(new Set(usernames))) + .optional(), + groups: z + .array(z.string().trim()) + .transform((groups) => Array.from(new Set(groups))) + .optional() + }) + .refine( + (data) => { + return (data.usernames && data.usernames.length > 0) || (data.groups && data.groups.length > 0); + }, + { + message: "At least one username or group must be provided", + path: ["allowedPrincipals"] + } + ) }); diff --git a/backend/src/ee/services/ssh-host/ssh-host-service.ts b/backend/src/ee/services/ssh-host/ssh-host-service.ts index 92f1f5236..fa8a453ca 100644 --- a/backend/src/ee/services/ssh-host/ssh-host-service.ts +++ b/backend/src/ee/services/ssh-host/ssh-host-service.ts @@ -1,6 +1,7 @@ import { ForbiddenError, subject } from "@casl/ability"; import { ActionProjectType, ProjectType } from "@app/db/schemas"; +import { TGroupDALFactory } from "@app/ee/services/group/group-dal"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { ProjectPermissionSshHostActions, ProjectPermissionSub } from "@app/ee/services/permission/project-permission"; import { TSshCertificateAuthorityDALFactory } from "@app/ee/services/ssh/ssh-certificate-authority-dal"; @@ -19,6 +20,7 @@ import { TProjectDALFactory } from "@app/services/project/project-dal"; import { TProjectSshConfigDALFactory } from "@app/services/project/project-ssh-config-dal"; import { TUserDALFactory } from "@app/services/user/user-dal"; +import { TUserGroupMembershipDALFactory } from "../group/user-group-membership-dal"; import { convertActorToPrincipals, createSshCert, @@ -38,12 +40,14 @@ import { type TSshHostServiceFactoryDep = { userDAL: Pick; + groupDAL: Pick; projectDAL: Pick; projectSshConfigDAL: Pick; sshCertificateAuthorityDAL: Pick; sshCertificateAuthoritySecretDAL: Pick; sshCertificateDAL: Pick; sshCertificateBodyDAL: Pick; + userGroupMembershipDAL: Pick; sshHostDAL: Pick< TSshHostDALFactory, | "transaction" @@ -57,7 +61,10 @@ type TSshHostServiceFactoryDep = { >; sshHostLoginUserDAL: TSshHostLoginUserDALFactory; sshHostLoginUserMappingDAL: TSshHostLoginUserMappingDALFactory; - permissionService: Pick; + permissionService: Pick< + TPermissionServiceFactory, + "getProjectPermission" | "getUserProjectPermission" | "checkGroupProjectPermission" + >; kmsService: Pick; }; @@ -65,6 +72,8 @@ export type TSshHostServiceFactory = ReturnType; export const sshHostServiceFactory = ({ userDAL, + userGroupMembershipDAL, + groupDAL, projectDAL, projectSshConfigDAL, sshCertificateAuthorityDAL, @@ -212,7 +221,7 @@ export const sshHostServiceFactory = ({ tx ); - if (allowedPrincipals.usernames.length > 0) { + if (allowedPrincipals.usernames && allowedPrincipals.usernames.length > 0) { const users = await userDAL.find( { $in: { @@ -251,6 +260,42 @@ export const sshHostServiceFactory = ({ tx ); } + + if (allowedPrincipals.groups && allowedPrincipals.groups.length > 0) { + const groups = await groupDAL.findGroupsByProjectId(projectId); + + const foundGroupSlugs = new Set(groups.map((g) => g.slug)); + + for (const slug of allowedPrincipals.groups) { + if (!foundGroupSlugs.has(slug)) { + throw new BadRequestError({ + message: `Invalid group slug: ${slug}` + }); + } + } + + for await (const group of groups) { + // check that each group has access to the SSH project and have read access to hosts + const hasPermission = await permissionService.checkGroupProjectPermission({ + groupId: group.id, + projectId, + checkPermissions: [ProjectPermissionSshHostActions.Read, ProjectPermissionSub.SshHosts] + }); + if (!hasPermission) { + throw new BadRequestError({ + message: `Group ${group.slug} does not have access to the SSH project` + }); + } + } + + await sshHostLoginUserMappingDAL.insertMany( + groups.map((group) => ({ + sshHostLoginUserId: sshHostLoginUser.id, + groupId: group.id + })), + tx + ); + } } const newSshHostWithLoginMappings = await sshHostDAL.findSshHostByIdWithLoginMappings(host.id, tx); @@ -319,7 +364,7 @@ export const sshHostServiceFactory = ({ tx ); - if (allowedPrincipals.usernames.length > 0) { + if (allowedPrincipals.usernames && allowedPrincipals.usernames.length > 0) { const users = await userDAL.find( { $in: { @@ -357,6 +402,42 @@ export const sshHostServiceFactory = ({ tx ); } + + if (allowedPrincipals.groups && allowedPrincipals.groups.length > 0) { + const groups = await groupDAL.findGroupsByProjectId(host.projectId); + + const foundGroupSlugs = new Set(groups.map((g) => g.slug)); + + for (const slug of allowedPrincipals.groups) { + if (!foundGroupSlugs.has(slug)) { + throw new BadRequestError({ + message: `Invalid group slug: ${slug}` + }); + } + } + + for await (const group of groups) { + // check that each group has access to the SSH project and have read access to hosts + const hasPermission = await permissionService.checkGroupProjectPermission({ + groupId: group.id, + projectId: host.projectId, + checkPermissions: [ProjectPermissionSshHostActions.Read, ProjectPermissionSub.SshHosts] + }); + if (!hasPermission) { + throw new BadRequestError({ + message: `Group ${group.slug} does not have access to the SSH project` + }); + } + } + + await sshHostLoginUserMappingDAL.insertMany( + groups.map((group) => ({ + sshHostLoginUserId: sshHostLoginUser.id, + groupId: group.id + })), + tx + ); + } } } } @@ -460,10 +541,14 @@ export const sshHostServiceFactory = ({ userDAL }); + const userGroups = await userGroupMembershipDAL.findGroupMembershipsByUserIdInOrg(actorId, actorOrgId); + const userGroupSlugs = userGroups.map((g) => g.groupSlug); + const mapping = host.loginMappings.find( (m) => m.loginUser === loginUser && - m.allowedPrincipals.usernames.some((allowed) => internalPrincipals.includes(allowed)) + (m.allowedPrincipals.usernames.some((allowed) => internalPrincipals.includes(allowed)) || + m.allowedPrincipals.groups.some((allowed) => userGroupSlugs.includes(allowed))) ); if (!mapping) { diff --git a/backend/src/ee/services/ssh-host/ssh-host-types.ts b/backend/src/ee/services/ssh-host/ssh-host-types.ts index a4826cd72..4586d66af 100644 --- a/backend/src/ee/services/ssh-host/ssh-host-types.ts +++ b/backend/src/ee/services/ssh-host/ssh-host-types.ts @@ -10,7 +10,8 @@ export type TCreateSshHostDTO = { loginMappings: { loginUser: string; allowedPrincipals: { - usernames: string[]; + usernames?: string[]; + groups?: string[]; }; }[]; userSshCaId?: string; @@ -26,7 +27,8 @@ export type TUpdateSshHostDTO = { loginMappings?: { loginUser: string; allowedPrincipals: { - usernames: string[]; + usernames?: string[]; + groups?: string[]; }; }[]; } & Omit; diff --git a/backend/src/lib/api-docs/constants.ts b/backend/src/lib/api-docs/constants.ts index 37a3d3679..e0e4de5df 100644 --- a/backend/src/lib/api-docs/constants.ts +++ b/backend/src/lib/api-docs/constants.ts @@ -1395,7 +1395,7 @@ export const SSH_HOSTS = { loginUser: "A login user on the remote machine (e.g. 'ec2-user', 'deploy', 'admin')", allowedPrincipals: "A list of allowed principals that can log in as the login user.", loginMappings: - "A list of login mappings for the SSH host. Each login mapping contains a login user and a list of corresponding allowed principals being usernames of users in the Infisical SSH project.", + "A list of login mappings for the SSH host. Each login mapping contains a login user and a list of corresponding allowed principals being usernames of users or groups slugs in the Infisical SSH project.", userSshCaId: "The ID of the SSH CA to use for user certificates. If not specified, the default user SSH CA will be used if it exists.", hostSshCaId: @@ -1410,7 +1410,7 @@ export const SSH_HOSTS = { loginUser: "A login user on the remote machine (e.g. 'ec2-user', 'deploy', 'admin')", allowedPrincipals: "A list of allowed principals that can log in as the login user.", loginMappings: - "A list of login mappings for the SSH host. Each login mapping contains a login user and a list of corresponding allowed principals being usernames of users in the Infisical SSH project." + "A list of login mappings for the SSH host. Each login mapping contains a login user and a list of corresponding allowed principals being usernames of users or groups slugs in the Infisical SSH project." }, DELETE: { sshHostId: "The ID of the SSH host to delete." diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index a71a69c20..0cabc0291 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -836,6 +836,8 @@ export const registerRoutes = async ( const sshHostService = sshHostServiceFactory({ userDAL, + groupDAL, + userGroupMembershipDAL, projectDAL, projectSshConfigDAL, sshCertificateAuthorityDAL, diff --git a/backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts b/backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts index 6ab348520..cd2773172 100644 --- a/backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts +++ b/backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts @@ -7,6 +7,7 @@ import { ProjectType, SecretsV2Schema, SecretType, TableName, TSecretsV2, TSecre import { TKeyStoreFactory } from "@app/keystore/keystore"; import { getConfig } from "@app/lib/config/env"; import { generateCacheKeyFromData } from "@app/lib/crypto/cache"; +import { applyJitter } from "@app/lib/dates"; import { BadRequestError, DatabaseError, NotFoundError } from "@app/lib/errors"; import { buildFindFilter, @@ -22,7 +23,6 @@ import type { TFindSecretsByFolderIdsFilter, TGetSecretsDTO } from "@app/services/secret-v2-bridge/secret-v2-bridge-types"; -import { applyJitter } from "@app/lib/dates"; export const SecretServiceCacheKeys = { get productKey() { diff --git a/frontend/src/hooks/api/sshHost/types.ts b/frontend/src/hooks/api/sshHost/types.ts index ebeb5130a..2170bf415 100644 --- a/frontend/src/hooks/api/sshHost/types.ts +++ b/frontend/src/hooks/api/sshHost/types.ts @@ -9,6 +9,7 @@ export type TSshHost = { loginUser: string; allowedPrincipals: { usernames: string[]; + groups: string[]; }; }[]; }; @@ -23,6 +24,7 @@ export type TCreateSshHostDTO = { loginUser: string; allowedPrincipals: { usernames: string[]; + groups: string[]; }; }[]; }; @@ -37,6 +39,7 @@ export type TUpdateSshHostDTO = { loginUser: string; allowedPrincipals: { usernames: string[]; + groups: string[]; }; }[]; }; diff --git a/frontend/src/pages/ssh/SshHostsPage/components/SshHostModal.tsx b/frontend/src/pages/ssh/SshHostsPage/components/SshHostModal.tsx index ede0a35a0..122234c92 100644 --- a/frontend/src/pages/ssh/SshHostsPage/components/SshHostModal.tsx +++ b/frontend/src/pages/ssh/SshHostsPage/components/SshHostModal.tsx @@ -23,6 +23,7 @@ import { useCreateSshHost, useGetSshHostById, useGetWorkspaceUsers, + useListWorkspaceGroups, useListWorkspaceSshHosts, useUpdateSshHost } from "@app/hooks/api"; @@ -48,7 +49,14 @@ const schema = z loginMappings: z .object({ loginUser: z.string().trim().min(1), - allowedPrincipals: z.array(z.string().trim()).default([]) + principals: z + .array( + z.object({ + type: z.enum(["user", "group"]), + value: z.string().trim() + }) + ) + .default([]) }) .array() .default([]) @@ -62,6 +70,7 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { const projectId = currentWorkspace?.id || ""; const { data: sshHosts } = useListWorkspaceSshHosts(currentWorkspace.id); const { data: members = [] } = useGetWorkspaceUsers(projectId); + const { data: groups = [] } = useListWorkspaceGroups(projectId); const [expandedMappings, setExpandedMappings] = useState>({}); const { data: sshHost } = useGetSshHostById( @@ -101,7 +110,16 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { userCertTtl: sshHost.userCertTtl, loginMappings: sshHost.loginMappings.map(({ loginUser, allowedPrincipals }) => ({ loginUser, - allowedPrincipals: allowedPrincipals.usernames + principals: [ + ...(allowedPrincipals.usernames || []).map((username) => ({ + type: "user" as const, + value: username + })), + ...(allowedPrincipals.groups || []).map((group) => ({ + type: "group" as const, + value: group + })) + ] })) }); @@ -151,18 +169,28 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { } } + const transformedLoginMappings = loginMappings.map(({ loginUser, principals }) => { + const usernames = principals.filter((p) => p.type === "user").map((p) => p.value); + + const groupNames = principals.filter((p) => p.type === "group").map((p) => p.value); + + return { + loginUser, + allowedPrincipals: { + usernames, + groups: groupNames + } + }; + }); + console.log(transformedLoginMappings); + if (sshHost) { await updateMutateAsync({ sshHostId: sshHost.id, hostname, alias: trimmedAlias, userCertTtl, - loginMappings: loginMappings.map(({ loginUser, allowedPrincipals }) => ({ - loginUser, - allowedPrincipals: { - usernames: allowedPrincipals - } - })) + loginMappings: transformedLoginMappings }); } else { await createMutateAsync({ @@ -170,12 +198,7 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { hostname, alias: trimmedAlias, userCertTtl, - loginMappings: loginMappings.map(({ loginUser, allowedPrincipals }) => ({ - loginUser, - allowedPrincipals: { - usernames: allowedPrincipals - } - })) + loginMappings: transformedLoginMappings }); } @@ -202,6 +225,15 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { })); }; + const isPrincipalDuplicate = ( + mappingIndex: number, + principalType: string, + principalValue: string + ) => { + const principals = getValues(`loginMappings.${mappingIndex}.principals`) || []; + return principals.some((p) => p.type === principalType && p.value === principalValue); + }; + return ( { variant="outline_bg" onClick={() => { const newIndex = loginMappingsFormFields.fields.length; - loginMappingsFormFields.append({ loginUser: "", allowedPrincipals: [""] }); + loginMappingsFormFields.append({ loginUser: "", principals: [] }); setExpandedMappings((prev) => ({ ...prev, [newIndex]: true @@ -360,8 +392,11 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { size="xs" variant="outline_bg" onClick={() => { - const current = getValues(`loginMappings.${i}.allowedPrincipals`) ?? []; - setValue(`loginMappings.${i}.allowedPrincipals`, [...current, ""]); + const current = getValues(`loginMappings.${i}.principals`) ?? []; + setValue(`loginMappings.${i}.principals`, [ + ...current, + { type: "user", value: "" } + ]); }} > Add Principal @@ -369,43 +404,72 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { (
- {(value.length === 0 ? [""] : value).map( - (principal: string, principalIndex: number) => ( -
-
- -
+ {value.map((principal, principalIndex) => ( +
+
+ +
+
+ +
+
{
- ) - )} +
+ ))} {error && {error.message}}
)} diff --git a/frontend/src/pages/ssh/SshHostsPage/components/SshHostsTable.tsx b/frontend/src/pages/ssh/SshHostsPage/components/SshHostsTable.tsx index 6efe40b36..02b7f3d9c 100644 --- a/frontend/src/pages/ssh/SshHostsPage/components/SshHostsTable.tsx +++ b/frontend/src/pages/ssh/SshHostsPage/components/SshHostsTable.tsx @@ -3,7 +3,9 @@ import { faEllipsis, faPencil, faServer, - faTrash + faTrash, + faUser, + faUsers } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import FileSaver from "file-saver"; @@ -12,6 +14,7 @@ import { twMerge } from "tailwind-merge"; import { createNotification } from "@app/components/notifications"; import { ProjectPermissionCan } from "@app/components/permissions"; import { + Badge, DropdownMenu, DropdownMenuContent, DropdownMenuItem, @@ -91,13 +94,46 @@ export const SshHostsTable = ({ handlePopUpOpen }: Props) => { None ) : ( host.loginMappings.map(({ loginUser, allowedPrincipals }) => ( -
-
{loginUser}
- {allowedPrincipals.usernames.map((username) => ( -
- └─ {username} -
- ))} +
+
{loginUser}
+
+ {allowedPrincipals.usernames.map((username) => ( +
+
+ └─ +
+
+ + {username} + user +
+
+ ))} + {allowedPrincipals.groups.map((group) => ( +
+
+ └─ +
+
+ + {group} + group +
+
+ ))} +
)) )} From 7d0574087c389afc765059ad05d41a5e4f487d1b Mon Sep 17 00:00:00 2001 From: carlosmonastyrski Date: Fri, 2 May 2025 13:36:05 -0300 Subject: [PATCH 2/6] Add groups to ssh hosts allowed principals bot improvements --- backend/src/ee/services/group/group-dal.ts | 6 +---- .../ee/services/ssh-host/ssh-host-service.ts | 17 ++++++------ .../SshHostsPage/components/SshHostModal.tsx | 27 ++++++++++--------- 3 files changed, 24 insertions(+), 26 deletions(-) diff --git a/backend/src/ee/services/group/group-dal.ts b/backend/src/ee/services/group/group-dal.ts index 12a70a15e..294ee019b 100644 --- a/backend/src/ee/services/group/group-dal.ts +++ b/backend/src/ee/services/group/group-dal.ts @@ -160,11 +160,7 @@ export const groupDALFactory = (db: TDbClient) => { const findGroupsByProjectId = async (projectId: string, tx?: Knex) => { try { const docs = await (tx || db.replicaNode())(TableName.Groups) - .leftJoin( - TableName.GroupProjectMembership, - `${TableName.Groups}.id`, - `${TableName.GroupProjectMembership}.groupId` - ) + .join(TableName.GroupProjectMembership, `${TableName.Groups}.id`, `${TableName.GroupProjectMembership}.groupId`) .where(`${TableName.GroupProjectMembership}.projectId`, projectId) .select(selectAllTableCols(TableName.Groups)); return docs; diff --git a/backend/src/ee/services/ssh-host/ssh-host-service.ts b/backend/src/ee/services/ssh-host/ssh-host-service.ts index fa8a453ca..348865b3a 100644 --- a/backend/src/ee/services/ssh-host/ssh-host-service.ts +++ b/backend/src/ee/services/ssh-host/ssh-host-service.ts @@ -262,16 +262,15 @@ export const sshHostServiceFactory = ({ } if (allowedPrincipals.groups && allowedPrincipals.groups.length > 0) { - const groups = await groupDAL.findGroupsByProjectId(projectId); + const projectGroups = await groupDAL.findGroupsByProjectId(projectId); + const groups = projectGroups.filter((g) => allowedPrincipals.groups?.includes(g.slug)); - const foundGroupSlugs = new Set(groups.map((g) => g.slug)); - - for (const slug of allowedPrincipals.groups) { - if (!foundGroupSlugs.has(slug)) { - throw new BadRequestError({ - message: `Invalid group slug: ${slug}` - }); - } + if (groups.length !== allowedPrincipals.groups?.length) { + throw new BadRequestError({ + message: `Invalid group slugs: ${allowedPrincipals.groups + .filter((g) => !projectGroups.some((pg) => pg.slug === g)) + .join(", ")}` + }); } for await (const group of groups) { diff --git a/frontend/src/pages/ssh/SshHostsPage/components/SshHostModal.tsx b/frontend/src/pages/ssh/SshHostsPage/components/SshHostModal.tsx index 122234c92..63f837606 100644 --- a/frontend/src/pages/ssh/SshHostsPage/components/SshHostModal.tsx +++ b/frontend/src/pages/ssh/SshHostsPage/components/SshHostModal.tsx @@ -49,11 +49,11 @@ const schema = z loginMappings: z .object({ loginUser: z.string().trim().min(1), - principals: z + allowedPrincipals: z .array( z.object({ type: z.enum(["user", "group"]), - value: z.string().trim() + value: z.string().trim().min(1) }) ) .default([]) @@ -110,7 +110,7 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { userCertTtl: sshHost.userCertTtl, loginMappings: sshHost.loginMappings.map(({ loginUser, allowedPrincipals }) => ({ loginUser, - principals: [ + allowedPrincipals: [ ...(allowedPrincipals.usernames || []).map((username) => ({ type: "user" as const, value: username @@ -169,10 +169,14 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { } } - const transformedLoginMappings = loginMappings.map(({ loginUser, principals }) => { - const usernames = principals.filter((p) => p.type === "user").map((p) => p.value); + const transformedLoginMappings = loginMappings.map(({ loginUser, allowedPrincipals }) => { + const usernames = allowedPrincipals + .filter((p) => p.type === "user" && p.value) + .map((p) => p.value); - const groupNames = principals.filter((p) => p.type === "group").map((p) => p.value); + const groupNames = allowedPrincipals + .filter((p) => p.type === "group" && p.value) + .map((p) => p.value); return { loginUser, @@ -182,7 +186,6 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { } }; }); - console.log(transformedLoginMappings); if (sshHost) { await updateMutateAsync({ @@ -230,7 +233,7 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { principalType: string, principalValue: string ) => { - const principals = getValues(`loginMappings.${mappingIndex}.principals`) || []; + const principals = getValues(`loginMappings.${mappingIndex}.allowedPrincipals`) || []; return principals.some((p) => p.type === principalType && p.value === principalValue); }; @@ -297,7 +300,7 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { variant="outline_bg" onClick={() => { const newIndex = loginMappingsFormFields.fields.length; - loginMappingsFormFields.append({ loginUser: "", principals: [] }); + loginMappingsFormFields.append({ loginUser: "", allowedPrincipals: [] }); setExpandedMappings((prev) => ({ ...prev, [newIndex]: true @@ -392,8 +395,8 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { size="xs" variant="outline_bg" onClick={() => { - const current = getValues(`loginMappings.${i}.principals`) ?? []; - setValue(`loginMappings.${i}.principals`, [ + const current = getValues(`loginMappings.${i}.allowedPrincipals`) ?? []; + setValue(`loginMappings.${i}.allowedPrincipals`, [ ...current, { type: "user", value: "" } ]); @@ -404,7 +407,7 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => {
(
{value.map((principal, principalIndex) => ( From 3ea450e94a1fdb81dc302103017e136ac2233c09 Mon Sep 17 00:00:00 2001 From: carlosmonastyrski Date: Fri, 2 May 2025 13:41:53 -0300 Subject: [PATCH 3/6] Add groups to ssh hosts allowed principals fix delete principal row issue --- .../src/pages/ssh/SshHostsPage/components/SshHostModal.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/pages/ssh/SshHostsPage/components/SshHostModal.tsx b/frontend/src/pages/ssh/SshHostsPage/components/SshHostModal.tsx index 63f837606..570633ce1 100644 --- a/frontend/src/pages/ssh/SshHostsPage/components/SshHostModal.tsx +++ b/frontend/src/pages/ssh/SshHostsPage/components/SshHostModal.tsx @@ -412,7 +412,7 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => {
{value.map((principal, principalIndex) => (
@@ -482,7 +482,7 @@ export const SshHostModal = ({ popUp, handlePopUpToggle }: Props) => { const newPrincipals = value.filter( (_, idx) => idx !== principalIndex ); - onChange(newPrincipals); + onChange([...newPrincipals]); }} > From bf85df7e3606ad8bdfa240c641d60a376e4b599f Mon Sep 17 00:00:00 2001 From: carlosmonastyrski Date: Tue, 6 May 2025 08:37:19 -0300 Subject: [PATCH 4/6] Fix SSH table UI user groups issues --- .../components/SshHostGroupModal.tsx | 160 +++++++++--------- .../SshHostsPage/components/SshHostsTable.tsx | 3 +- 2 files changed, 81 insertions(+), 82 deletions(-) diff --git a/frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupModal.tsx b/frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupModal.tsx index 6cd99c796..081af4cbf 100644 --- a/frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupModal.tsx +++ b/frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupModal.tsx @@ -351,88 +351,86 @@ export const SshHostGroupModal = ({ popUp, handlePopUpToggle }: Props) => { name={`loginMappings.${i}.allowedPrincipals`} render={({ field: { value = [], onChange }, fieldState: { error } }) => (
- {(value.length === 0 ? [{ type: "user", value: "" }] : value).map( - (principal, principalIndex) => ( -
-
- -
-
- -
-
- { - const newPrincipals = value.filter( - (_, idx) => idx !== principalIndex - ); - onChange(newPrincipals.length ? newPrincipals : []); - }} - > - - -
+ {value.map((principal, principalIndex) => ( +
+
+
- ) - )} +
+ +
+
+ { + const newPrincipals = value.filter( + (_, idx) => idx !== principalIndex + ); + onChange(newPrincipals.length ? newPrincipals : []); + }} + > + + +
+
+ ))} {error && {error.message}}
)} diff --git a/frontend/src/pages/ssh/SshHostsPage/components/SshHostsTable.tsx b/frontend/src/pages/ssh/SshHostsPage/components/SshHostsTable.tsx index fd43697ec..e3c53860e 100644 --- a/frontend/src/pages/ssh/SshHostsPage/components/SshHostsTable.tsx +++ b/frontend/src/pages/ssh/SshHostsPage/components/SshHostsTable.tsx @@ -131,7 +131,8 @@ export const SshHostsTable = ({ handlePopUpOpen }: Props) => { const filteredGroups = allowedPrincipals.groups?.filter( (g) => !existing.groups?.has(g) ); - return (filteredGroups?.length ?? 0) > 0 + return ((filteredGroups?.length || filteredUsernames?.length) ?? 0) > + 0 ? { loginUser, source: LoginMappingSource.HOST_GROUP, From a6b3be72a9582082e4b24195387c309a2372c3d5 Mon Sep 17 00:00:00 2001 From: Tuan Dang Date: Thu, 8 May 2025 14:02:25 -0700 Subject: [PATCH 5/6] Make minor PR adjustments --- .../components/SshHostGroupModal.tsx | 4 +- .../components/SshHostGroupsSection.tsx | 50 ++++++++++++------- .../components/SshHostsSection.tsx | 2 +- 3 files changed, 36 insertions(+), 20 deletions(-) diff --git a/frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupModal.tsx b/frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupModal.tsx index 081af4cbf..84be5a96d 100644 --- a/frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupModal.tsx +++ b/frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupModal.tsx @@ -57,8 +57,8 @@ export type FormData = z.infer; export const SshHostGroupModal = ({ popUp, handlePopUpToggle }: Props) => { const { currentWorkspace } = useWorkspace(); - const projectId = currentWorkspace?.id || ""; - const { data: sshHostGroups } = useListWorkspaceSshHostGroups(currentWorkspace.id); + const projectId = currentWorkspace.id; + const { data: sshHostGroups } = useListWorkspaceSshHostGroups(projectId); const { data: members = [] } = useGetWorkspaceUsers(projectId); const { data: groups = [] } = useListWorkspaceGroups(projectId); const [expandedMappings, setExpandedMappings] = useState>({}); diff --git a/frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupsSection.tsx b/frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupsSection.tsx index 70036d88a..e2c731f97 100644 --- a/frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupsSection.tsx +++ b/frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupsSection.tsx @@ -1,4 +1,4 @@ -import { faPlus } from "@fortawesome/free-solid-svg-icons"; +import { faArrowUpRightFromSquare, faPlus } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { UpgradePlanModal } from "@app/components/license/UpgradePlanModal"; @@ -56,22 +56,38 @@ export const SshHostGroupsSection = () => {

Host Groups

- - {(isAllowed) => ( - - )} - +
+ + + Documentation{" "} + + + + + {(isAllowed) => ( + + )} + +
diff --git a/frontend/src/pages/ssh/SshHostsPage/components/SshHostsSection.tsx b/frontend/src/pages/ssh/SshHostsPage/components/SshHostsSection.tsx index a3f9917ba..ed190db21 100644 --- a/frontend/src/pages/ssh/SshHostsPage/components/SshHostsSection.tsx +++ b/frontend/src/pages/ssh/SshHostsPage/components/SshHostsSection.tsx @@ -42,7 +42,7 @@ export const SshHostsSection = () => {

Hosts

-
+
Date: Fri, 9 May 2025 17:10:01 -0300 Subject: [PATCH 6/6] Trim hostname input on SSH Host permission form and fix getWorkspaceUsers key invalidation --- frontend/src/hooks/api/workspace/query-keys.tsx | 7 +++++-- .../components/SshHostPermissionConditions.tsx | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/frontend/src/hooks/api/workspace/query-keys.tsx b/frontend/src/hooks/api/workspace/query-keys.tsx index 91fe95a04..335248a80 100644 --- a/frontend/src/hooks/api/workspace/query-keys.tsx +++ b/frontend/src/hooks/api/workspace/query-keys.tsx @@ -16,8 +16,11 @@ export const workspaceKeys = { type ? ["workspaces", { type }] : (["workspaces"] as const), getWorkspaceAuditLogs: (workspaceId: string) => [{ workspaceId }, "workspace-audit-logs"] as const, - getWorkspaceUsers: (workspaceId: string, includeGroupMembers?: boolean, roles?: string[]) => - [{ workspaceId, includeGroupMembers, roles }, "workspace-users"] as const, + getWorkspaceUsers: ( + workspaceId: string, + includeGroupMembers: boolean = false, + roles: string[] = [] + ) => [{ workspaceId, includeGroupMembers, roles }, "workspace-users"] as const, getWorkspaceUserDetails: (workspaceId: string, membershipId: string) => [{ workspaceId, membershipId }, "workspace-user-details"] as const, getWorkspaceIdentityMemberships: (workspaceId: string) => diff --git a/frontend/src/pages/project/RoleDetailsBySlugPage/components/SshHostPermissionConditions.tsx b/frontend/src/pages/project/RoleDetailsBySlugPage/components/SshHostPermissionConditions.tsx index b82d86d81..85cba0d94 100644 --- a/frontend/src/pages/project/RoleDetailsBySlugPage/components/SshHostPermissionConditions.tsx +++ b/frontend/src/pages/project/RoleDetailsBySlugPage/components/SshHostPermissionConditions.tsx @@ -125,7 +125,7 @@ export const SshHostPermissionConditions = ({ position = 0, isDisabled }: Props) errorText={error?.message} className="mb-0 flex-grow" > - + field.onChange(e.target.value.trim())} /> )} />