From 8edfa9ad0b24169c32a560c22f7ee2f78d8bc3bc Mon Sep 17 00:00:00 2001
From: Tuan Dang <dangtony98@gmail.com>
Date: Tue, 3 Dec 2024 23:22:04 -0800
Subject: [PATCH] Improve requested user/host validation for ssh certificate
 template

---
 .../ssh-certificate-template-service.ts       |  2 +-
 .../ssh/ssh-certificate-authority-fns.ts      | 70 ++++++++++++++-----
 2 files changed, 54 insertions(+), 18 deletions(-)

diff --git a/backend/src/ee/services/ssh-certificate-template/ssh-certificate-template-service.ts b/backend/src/ee/services/ssh-certificate-template/ssh-certificate-template-service.ts
index 357a9a0db..88e3259b2 100644
--- a/backend/src/ee/services/ssh-certificate-template/ssh-certificate-template-service.ts
+++ b/backend/src/ee/services/ssh-certificate-template/ssh-certificate-template-service.ts
@@ -130,7 +130,7 @@ export const sshCertificateTemplateServiceFactory = ({
 
     if (name) {
       const existingTemplate = await sshCertificateTemplateDAL.getByName(name, actorOrgId);
-      if (existingTemplate) {
+      if (existingTemplate && existingTemplate.id !== id) {
         throw new BadRequestError({
           message: `SSH certificate template with name ${name} already exists`
         });
diff --git a/backend/src/ee/services/ssh/ssh-certificate-authority-fns.ts b/backend/src/ee/services/ssh/ssh-certificate-authority-fns.ts
index a299b8120..7491141e4 100644
--- a/backend/src/ee/services/ssh/ssh-certificate-authority-fns.ts
+++ b/backend/src/ee/services/ssh/ssh-certificate-authority-fns.ts
@@ -122,34 +122,70 @@ export const validateSshCertificatePrincipals = (
 ) => {
   switch (certType) {
     case SshCertType.USER: {
-      const allowsAllUsers = template.allowedUsers?.includes("*") ?? false;
-      return principals.every((principal) => {
-        if (principal === "*") return false;
-        if (allowsAllUsers) return isValidUserPattern(principal);
-        return template.allowedUsers?.includes(principal);
+      if (template.allowedUsers.length === 0) {
+        throw new BadRequestError({
+          message: "No allowed users are configured in the SSH certificate template."
+        });
+      }
+
+      const allowsAllUsers = template.allowedUsers.includes("*") ?? false;
+
+      principals.forEach((principal) => {
+        if (principal === "*") {
+          throw new BadRequestError({
+            message: `Principal '*' is not allowed for user certificates.`
+          });
+        }
+        if (allowsAllUsers && !isValidUserPattern(principal)) {
+          throw new BadRequestError({
+            message: `Principal '${principal}' does not match a valid user pattern.`
+          });
+        }
+        if (!allowsAllUsers && !template.allowedUsers.includes(principal)) {
+          throw new BadRequestError({
+            message: `Principal '${principal}' is not in the list of allowed users.`
+          });
+        }
       });
+      break;
     }
     case SshCertType.HOST: {
-      const allowsAllHosts = template.allowedHosts?.includes("*") ?? false;
-      return principals.every((principal) => {
-        if (principal.includes("*")) return false;
-        if (allowsAllHosts) return isValidHostPattern(principal);
+      if (template.allowedHosts.length === 0) {
+        throw new BadRequestError({
+          message: "No allowed hosts are configured in the SSH certificate template."
+        });
+      }
 
-        // Validate against allowed domains
-        return (
-          isValidHostPattern(principal) &&
-          template.allowedHosts?.some((allowedHost) => {
+      const allowsAllHosts = template.allowedHosts.includes("*") ?? false;
+
+      principals.forEach((principal) => {
+        if (principal.includes("*")) {
+          throw new BadRequestError({
+            message: `Principal '${principal}' with wildcards is not allowed for host certificates.`
+          });
+        }
+        if (allowsAllHosts && !isValidHostPattern(principal)) {
+          throw new BadRequestError({
+            message: `Principal '${principal}' does not match a valid host pattern.`
+          });
+        }
+
+        if (
+          !allowsAllHosts &&
+          !template.allowedHosts.some((allowedHost) => {
             if (allowedHost.startsWith("*.")) {
-              // Match subdomains of a wildcard domain
               const baseDomain = allowedHost.slice(2); // Remove the leading "*."
               return principal.endsWith(`.${baseDomain}`);
             }
-
-            // Exact match for non-wildcard domains
             return principal === allowedHost;
           })
-        );
+        ) {
+          throw new BadRequestError({
+            message: `Principal '${principal}' is not in the list of allowed hosts or domains.`
+          });
+        }
       });
+      break;
     }
     default:
       throw new BadRequestError({