From 7d0574087c389afc765059ad05d41a5e4f487d1b Mon Sep 17 00:00:00 2001
From: carlosmonastyrski <charly.monastyrski@gmail.com>
Date: Fri, 2 May 2025 13:36:05 -0300
Subject: [PATCH] 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) => {
                       </div>
                       <Controller
                         control={control}
-                        name={`loginMappings.${i}.principals`}
+                        name={`loginMappings.${i}.allowedPrincipals`}
                         render={({ field: { value = [], onChange }, fieldState: { error } }) => (
                           <div className="flex flex-col space-y-2">
                             {value.map((principal, principalIndex) => (