Addressed greptlie comments and suggestions

2026-09-22 13:39:35 +00:00 · 2025-08-27 04:04:39 -03:00
parent 83479a091e
commit 7b52d60036
22 changed files with 80 additions and 54 deletions
--- a/backend/src/ee/services/audit-log/audit-log-types.ts
+++ b/backend/src/ee/services/audit-log/audit-log-types.ts
@@ -281,6 +281,7 @@ export enum EventType {
  UPDATE_SSH_CERTIFICATE_TEMPLATE = "update-ssh-certificate-template",
  DELETE_SSH_CERTIFICATE_TEMPLATE = "delete-ssh-certificate-template",
  GET_SSH_CERTIFICATE_TEMPLATE = "get-ssh-certificate-template",
+  GET_AZURE_AD_TEMPLATES = "get-azure-ad-templates",
  GET_SSH_HOST = "get-ssh-host",
  CREATE_SSH_HOST = "create-ssh-host",
  UPDATE_SSH_HOST = "update-ssh-host",
@@ -2497,6 +2498,14 @@ interface CreateCertificateTemplateEstConfig {
  };
 }

+interface GetAzureAdCsTemplatesEvent {
+  type: EventType.GET_AZURE_AD_TEMPLATES;
+  metadata: {
+    caId: string;
+    amount: number;
+  };
+}
+
 interface UpdateCertificateTemplateEstConfig {
  type: EventType.UPDATE_CERTIFICATE_TEMPLATE_EST_CONFIG;
  metadata: {
@@ -3636,6 +3645,7 @@ export type Event =
  | CreateCertificateTemplateEstConfig
  | UpdateCertificateTemplateEstConfig
  | GetCertificateTemplateEstConfig
+  | GetAzureAdCsTemplatesEvent
  | AttemptCreateSlackIntegration
  | AttemptReinstallSlackIntegration
  | UpdateSlackIntegration
--- a/backend/src/server/routes/index.ts
+++ b/backend/src/server/routes/index.ts
@@ -948,7 +948,6 @@ export const registerRoutes = async (
    certificateAuthorityCrlDAL,
    certificateAuthoritySecretDAL,
    projectDAL,
-    appConnectionDAL,
    kmsService,
    permissionService,
    pkiCollectionDAL,
--- a/backend/src/server/routes/v1/certificate-authority-routers/azure-ad-cs-certificate-authority-router.ts
+++ b/backend/src/server/routes/v1/certificate-authority-routers/azure-ad-cs-certificate-authority-router.ts
@@ -1,5 +1,6 @@
 import { z } from "zod";

+import { EventType } from "@app/ee/services/audit-log/audit-log-types";
 import { readLimit } from "@app/server/config/rateLimiter";
 import { verifyAuth } from "@app/server/plugins/auth/verify-auth";
 import { AuthMode } from "@app/services/auth/auth-type";
@@ -59,6 +60,18 @@ export const registerAzureAdCsCertificateAuthorityRouter = async (server: Fastif
        actorOrgId: req.permission.orgId
      });

+      await server.services.auditLog.createAuditLog({
+        ...req.auditLogInfo,
+        projectId: req.query.projectId,
+        event: {
+          type: EventType.GET_AZURE_AD_TEMPLATES,
+          metadata: {
+            caId: req.params.caId,
+            amount: templates.length
+          }
+        }
+      });
+
      return { templates };
    }
  });
--- a/backend/src/services/app-connection/azure-adcs/azure-adcs-connection-fns.ts
+++ b/backend/src/services/app-connection/azure-adcs/azure-adcs-connection-fns.ts
@@ -1,5 +1,7 @@
 /* eslint-disable no-case-declarations, @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-var-requires, no-await-in-loop, no-continue */
+import { getConfig } from "@app/lib/config/env";
 import { BadRequestError, NotFoundError } from "@app/lib/errors";
+import { blockLocalAndPrivateIpAddresses } from "@app/lib/validator/validate-url";
 import { decryptAppConnectionCredentials } from "@app/services/app-connection/app-connection-fns";
 import { TKmsServiceFactory } from "@app/services/kms/kms-service";

@@ -145,7 +147,7 @@ const testAdcsConnection = async (
        password,
        domain: credentials.domain,
        workstation: "",
-        rejectUnauthorized: false
+        rejectUnauthorized: !getConfig().isDevelopmentMode // Only allow unauthorized SSL in development
      });

      // Check if we got a successful response
@@ -232,7 +234,7 @@ const createNtlmClient = (username: string, password: string, baseUrl: string) =
        password,
        domain: parsedCredentials.domain,
        workstation: "",
-        rejectUnauthorized: false,
+        rejectUnauthorized: !getConfig().isDevelopmentMode, // Only allow unauthorized SSL in development,
        ...additionalOptions
      });
    },
@@ -244,7 +246,7 @@ const createNtlmClient = (username: string, password: string, baseUrl: string) =
        password,
        domain: parsedCredentials.domain,
        workstation: "",
-        rejectUnauthorized: false,
+        rejectUnauthorized: !getConfig().isDevelopmentMode, // Only allow unauthorized SSL in development,
        body,
        headers: {
          "Content-Type": "application/x-www-form-urlencoded",
@@ -302,6 +304,9 @@ export const validateAzureADCSConnectionCredentials = async (appConnection: TAzu
    const parsedCredentials = parseCredentials(credentials.username);
    const normalizedUrl = normalizeAdcsUrl(credentials.adcsUrl);

+    // Validate URL to prevent DNS manipulation attacks and SSRF
+    await blockLocalAndPrivateIpAddresses(normalizedUrl);
+
    // Test the connection using NTLM
    await testAdcsConnection(parsedCredentials, credentials.password, normalizedUrl);

--- a/backend/src/services/certificate-authority/azure-ad-cs/azure-ad-cs-certificate-authority-fns.ts
+++ b/backend/src/services/certificate-authority/azure-ad-cs/azure-ad-cs-certificate-authority-fns.ts
@@ -110,7 +110,7 @@ const buildSubjectDN = (commonName: string, properties?: TPkiSubscriberPropertie
  const emailAddress = sanitizeComponent(properties?.emailAddress);
  if (emailAddress) {
    // Enhanced email validation for DN usage
-    const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;
+    const emailRegex = new RE2(/^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/);
    if (emailRegex.test(emailAddress) && emailAddress.length > 5 && emailAddress.length < 64) {
      subject += `,E=${emailAddress}`;
    }
@@ -247,7 +247,7 @@ const submitCertificateRequest = async (
    }

    // Check for immediate certificate issuance
-    const certMatch = responseText.match(/-----BEGIN CERTIFICATE-----[\s\S]*?-----END CERTIFICATE-----/);
+    const certMatch = responseText.match(new RE2("-----BEGIN CERTIFICATE-----[\\s\\S]*?-----END CERTIFICATE-----"));
    if (certMatch) {
      // Clean up the certificate format
      certificate = certMatch[0].replace(new RE2("\\\\r\\\\n", "g"), "\n").replace(new RE2("\\\\r", "g"), "\n").trim();
@@ -325,15 +325,15 @@ const submitCertificateRequest = async (
      // General error extraction
      else {
        const errorPatterns = [
-          /The disposition message is "([^"]*)/i,
-          /Denied by Policy Module[^"]*"([^"]*)/i,
-          /<p[^>]*class[^>]*error[^>]*>(.*?)<\/p>/i,
-          /<div[^>]*class[^>]*error[^>]*>(.*?)<\/div>/i,
-          /<span[^>]*class[^>]*error[^>]*>(.*?)<\/span>/i,
-          /error[^<]*:([^<]*)/i,
-          /denied[^<]*:([^<]*)/i,
-          /The\s+request\s+contains\s+no\s+certificate\s+template\s+information/i,
-          /The\s+template\s+is\s+missing/i
+          new RE2('The disposition message is "([^"]*)"', "i"),
+          new RE2('Denied by Policy Module[^"]*"([^"]*)"', "i"),
+          new RE2("<p[^>]*class[^>]*error[^>]*>(.*?)<\\/p>", "i"),
+          new RE2("<div[^>]*class[^>]*error[^>]*>(.*?)<\\/div>", "i"),
+          new RE2("<span[^>]*class[^>]*error[^>]*>(.*?)<\\/span>", "i"),
+          new RE2("error[^<]*:([^<]*)", "i"),
+          new RE2("denied[^<]*:([^<]*)", "i"),
+          new RE2("The\\s+request\\s+contains\\s+no\\s+certificate\\s+template\\s+information", "i"),
+          new RE2("The\\s+template\\s+is\\s+missing", "i")
        ];

        // Try each pattern to find the error message
--- a/backend/src/services/certificate-authority/azure-ad-cs/azure-ad-cs-certificate-authority-schemas.ts
+++ b/backend/src/services/certificate-authority/azure-ad-cs/azure-ad-cs-certificate-authority-schemas.ts
@@ -11,11 +11,16 @@ export const AzureAdCsCertificateAuthorityConfigurationSchema = z.object({
  azureAdcsConnectionId: z.string().uuid().trim().describe("Azure ADCS Connection ID")
 });

-export const AzureAdCsCertificateAuthorityCredentialsSchema = z.object({
-  clientId: z.string(),
-  clientSecret: z.string().optional(),
-  certificateThumbprint: z.string().optional()
-});
+export const AzureAdCsCertificateAuthorityCredentialsSchema = z
+  .object({
+    clientId: z.string(),
+    clientSecret: z.string().optional(),
+    certificateThumbprint: z.string().optional()
+  })
+  .refine((data) => data.clientSecret || data.certificateThumbprint, {
+    message: "At least one authentication method (clientSecret or certificateThumbprint) must be provided",
+    path: ["clientSecret"]
+  });

 export const AzureAdCsCertificateAuthoritySchema = BaseCertificateAuthoritySchema.extend({
  type: z.literal(CaType.AZURE_AD_CS),
--- a/backend/src/services/certificate-authority/azure-ad-cs/azure-ad-cs-certificate-authority-types.ts
+++ b/backend/src/services/certificate-authority/azure-ad-cs/azure-ad-cs-certificate-authority-types.ts
@@ -8,8 +8,6 @@ import {

 export type TAzureAdCsCertificateAuthority = z.infer<typeof AzureAdCsCertificateAuthoritySchema>;

-export type TAzureAdCsCertificateAuthorityInput = z.infer<typeof CreateAzureAdCsCertificateAuthoritySchema>;
-
 export type TCreateAzureAdCsCertificateAuthorityDTO = z.infer<typeof CreateAzureAdCsCertificateAuthoritySchema>;

 export type TUpdateAzureAdCsCertificateAuthorityDTO = z.infer<typeof UpdateAzureAdCsCertificateAuthoritySchema>;
--- a/backend/src/services/certificate-authority/certificate-authority-maps.ts
+++ b/backend/src/services/certificate-authority/certificate-authority-maps.ts
@@ -12,11 +12,7 @@ export const CERTIFICATE_AUTHORITIES_CAPABILITIES_MAP: Record<CaType, CaCapabili
    CaCapability.REVOKE_CERTIFICATES,
    CaCapability.RENEW_CERTIFICATES
  ],
-  [CaType.ACME]: [
-    CaCapability.ISSUE_CERTIFICATES,
-    CaCapability.REVOKE_CERTIFICATES,
-    CaCapability.RENEW_CERTIFICATES
-  ],
+  [CaType.ACME]: [CaCapability.ISSUE_CERTIFICATES, CaCapability.REVOKE_CERTIFICATES, CaCapability.RENEW_CERTIFICATES],
  [CaType.AZURE_AD_CS]: [
    CaCapability.ISSUE_CERTIFICATES,
    CaCapability.RENEW_CERTIFICATES
--- a/backend/src/services/certificate-authority/certificate-authority-types.ts
+++ b/backend/src/services/certificate-authority/certificate-authority-types.ts
@@ -1,7 +1,7 @@
 import { TAcmeCertificateAuthority, TAcmeCertificateAuthorityInput } from "./acme/acme-certificate-authority-types";
 import {
  TAzureAdCsCertificateAuthority,
-  TAzureAdCsCertificateAuthorityInput
+  TCreateAzureAdCsCertificateAuthorityDTO
 } from "./azure-ad-cs/azure-ad-cs-certificate-authority-types";
 import { CaType } from "./certificate-authority-enums";
 import {
@@ -17,7 +17,7 @@ export type TCertificateAuthority =
 export type TCertificateAuthorityInput =
  | TInternalCertificateAuthorityInput
  | TAcmeCertificateAuthorityInput
-  | TAzureAdCsCertificateAuthorityInput;
+  | TCreateAzureAdCsCertificateAuthorityDTO;

 export type TCreateCertificateAuthorityDTO = Omit<TCertificateAuthority, "id">;

--- a/backend/src/services/certificate-authority/internal/internal-certificate-authority-fns.ts
+++ b/backend/src/services/certificate-authority/internal/internal-certificate-authority-fns.ts
@@ -98,7 +98,7 @@ const buildSubjectDN = (commonName: string, properties?: TPkiSubscriberPropertie
  const emailAddress = sanitizeComponent(properties?.emailAddress);
  if (emailAddress) {
    // Enhanced email validation for DN usage
-    const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;
+    const emailRegex = new RE2(/^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/);
    if (emailRegex.test(emailAddress) && emailAddress.length > 5 && emailAddress.length < 64) {
      subject += `,E=${emailAddress}`;
    }
--- a/backend/src/services/certificate/certificate-service.ts
+++ b/backend/src/services/certificate/certificate-service.ts
@@ -10,7 +10,6 @@ import {
 } from "@app/ee/services/permission/project-permission";
 import { crypto } from "@app/lib/crypto/cryptography";
 import { BadRequestError, NotFoundError } from "@app/lib/errors";
-import { TAppConnectionDALFactory } from "@app/services/app-connection/app-connection-dal";
 import { TCertificateBodyDALFactory } from "@app/services/certificate/certificate-body-dal";
 import { TCertificateDALFactory } from "@app/services/certificate/certificate-dal";
 import { TCertificateAuthorityCertDALFactory } from "@app/services/certificate-authority/certificate-authority-cert-dal";
@@ -52,7 +51,6 @@ type TCertificateServiceFactoryDep = {
  pkiCollectionDAL: Pick<TPkiCollectionDALFactory, "findById">;
  pkiCollectionItemDAL: Pick<TPkiCollectionItemDALFactory, "create">;
  projectDAL: Pick<TProjectDALFactory, "findProjectBySlug" | "findOne" | "updateById" | "findById" | "transaction">;
-  appConnectionDAL: Pick<TAppConnectionDALFactory, "findById">;
  kmsService: Pick<TKmsServiceFactory, "generateKmsKey" | "encryptWithKmsKey" | "decryptWithKmsKey">;
  permissionService: Pick<TPermissionServiceFactory, "getProjectPermission">;
 };
@@ -70,7 +68,6 @@ export const certificateServiceFactory = ({
  pkiCollectionDAL,
  pkiCollectionItemDAL,
  projectDAL,
-  appConnectionDAL,
  kmsService,
  permissionService
 }: TCertificateServiceFactoryDep) => {
--- a/backend/src/services/pki-subscriber/pki-subscriber-types.ts
+++ b/backend/src/services/pki-subscriber/pki-subscriber-types.ts
@@ -80,4 +80,4 @@ export type TPkiSubscriberProperties = {
  state?: string;
  locality?: string;
  emailAddress?: string;
-}
+};
--- a/docs/integrations/app-connections/azure-adcs.mdx
+++ b/docs/integrations/app-connections/azure-adcs.mdx
@@ -24,7 +24,7 @@ Connect Infisical to Microsoft Active Directory Certificate Services (ADCS) for
      ![Select Azure ADCS Connection](/images/app-connections/azure-adcs/azure-adcs-select-connection.png)
    </Step>
    <Step title="Configure Connection Details">
-    Fill in the following information:
+      Fill in the following information:
    - **Name**: Friendly name for this ADCS connection (e.g., "Production ADCS")
    - **ADCS URL**: Your ADCS web enrollment URL (e.g., `https://adcs.yourdomain.com/certsrv`)
    - **Username**: Domain administrator username (format: `DOMAIN\username` or `username@domain.com`)
@@ -34,9 +34,7 @@ Connect Infisical to Microsoft Active Directory Certificate Services (ADCS) for
    ![Connect to ADCS](/images/app-connections/azure-adcs/azure-adcs-app-connection-form.png)
  </Step>
  <Step title="Connection Created">
-    Your **Azure ADCS Connection** is now available for use in your Infisical
-    projects. ![Azure ADCS Connection
-    Created](/images/app-connections/azure-adcs/azure-adcs-app-connection-created.png)
+    Your **Azure ADCS Connection** is now available for use in your Infisical projects. ![Azure ADCS Connection Created](/images/app-connections/azure-adcs/azure-adcs-app-connection-created.png)
  </Step>
 </Steps>

--- a/frontend/src/hooks/api/appConnections/types/app-options.ts
+++ b/frontend/src/hooks/api/appConnections/types/app-options.ts
@@ -205,7 +205,8 @@ export type TAppConnectionOption =
  | TSupabaseConnectionOption
  | TDigitalOceanConnectionOption
  | TNetlifyConnectionOption
-  | TOktaConnectionOption;
+  | TOktaConnectionOption
+  | TAzureAdCsConnectionOption;

 export type TAppConnectionOptionMap = {
  [AppConnection.AWS]: TAwsConnectionOption;
--- a/frontend/src/hooks/api/appConnections/types/azure-adcs-connection.ts
+++ b/frontend/src/hooks/api/appConnections/types/azure-adcs-connection.ts
@@ -17,9 +17,5 @@ export type TCreateAzureADCSConnection = z.infer<typeof CreateAzureADCSConnectio

 export type TAzureADCSConnection = TRootAppConnection & { app: AppConnection.AzureADCS } & {
  method: AzureADCSConnectionMethod.UsernamePassword;
-  credentials: {
-    username: string;
-    password: string;
-    adcsUrl: string;
-  };
+  credentials: TCreateAzureADCSConnection;
 };
--- a/frontend/src/hooks/api/ca/mutations.tsx
+++ b/frontend/src/hooks/api/ca/mutations.tsx
@@ -41,7 +41,7 @@ export const useUpdateCa = () => {
      });
      // Invalidate external CAs list
      queryClient.invalidateQueries({
-        queryKey: [`external-cas-${projectId}`]
+        queryKey: caKeys.listExternalCasByProjectId(projectId)
      });
    }
  });
@@ -63,7 +63,7 @@ export const useCreateCa = () => {
      });
      // Invalidate external CAs list
      queryClient.invalidateQueries({
-        queryKey: [`external-cas-${projectId}`]
+        queryKey: caKeys.listExternalCasByProjectId(projectId)
      });
    }
  });
@@ -89,7 +89,7 @@ export const useDeleteCa = () => {
      });
      // Invalidate external CAs list
      queryClient.invalidateQueries({
-        queryKey: [`external-cas-${projectId}`]
+        queryKey: caKeys.listExternalCasByProjectId(projectId)
      });
    }
  });
--- a/frontend/src/hooks/api/ca/queries.tsx
+++ b/frontend/src/hooks/api/ca/queries.tsx
@@ -4,13 +4,14 @@ import { apiRequest } from "@app/config/request";

 import { TCertificateTemplate } from "../certificateTemplates/types";
 import { CaType } from "./enums";
-import { TCertificateAuthority, TUnifiedCertificateAuthority } from "./types";
+import { TAzureAdCsTemplate, TCertificateAuthority, TUnifiedCertificateAuthority } from "./types";

 export const caKeys = {
  getCaById: (caId: string) => [{ caId }, "ca"],
  getCaByNameAndProjectId: (caName: string, projectId: string) => [{ caName, projectId }, "ca"],
  listCasByTypeAndProjectId: (type: CaType, projectId: string) => [{ type, projectId }, "cas"],
  listCasByProjectId: (projectId: string) => [{ projectId }, "cas"],
+  listExternalCasByProjectId: (projectId: string) => [{ projectId }, "external-cas"],
  getCaCerts: (caId: string) => [{ caId }, "ca-cert"],
  getCaCrls: (caId: string) => [{ caId }, "ca-crls"],
  getCaCert: (caId: string) => [{ caId }, "ca-cert"],
@@ -73,7 +74,7 @@ export const useListCasByProjectId = (projectId: string) => {

 export const useListExternalCasByProjectId = (projectId: string) => {
  return useQuery({
-    queryKey: [`external-cas-${projectId}`],
+    queryKey: caKeys.listExternalCasByProjectId(projectId),
    queryFn: async () => {
      const [acmeResponse, azureAdCsResponse] = await Promise.allSettled([
        apiRequest.get<TUnifiedCertificateAuthority[]>(
@@ -200,7 +201,7 @@ export const useGetAzureAdcsTemplates = ({
    queryKey: caKeys.getAzureAdcsTemplates(caId, projectId),
    queryFn: async () => {
      const { data } = await apiRequest.get<{
-        templates: { id: string; name: string; description?: string }[];
+        templates: TAzureAdCsTemplate[];
      }>(`/api/v1/pki/ca/azure-ad-cs/${caId}/templates?projectId=${projectId}`);
      return data;
    },
--- a/frontend/src/hooks/api/ca/types.ts
+++ b/frontend/src/hooks/api/ca/types.ts
@@ -137,6 +137,12 @@ export type TSignIntermediateResponse = {
  serialNumber: string;
 };

+export type TAzureAdCsTemplate = {
+  id: string;
+  name: string;
+  description?: string;
+};
+
 export type TImportCaCertificateDTO = {
  caId: string;
  projectSlug: string;
--- a/frontend/src/pages/cert-manager/CertificateAuthoritiesPage/components/ExternalCaModal.tsx
+++ b/frontend/src/pages/cert-manager/CertificateAuthoritiesPage/components/ExternalCaModal.tsx
@@ -274,6 +274,8 @@ export const ExternalCaModal = ({ popUp, handlePopUpToggle }: Props) => {
        configPayload = {
          azureAdcsConnectionId: formConfiguration.azureAdcsConnection.id
        };
+      } else {
+        throw new Error("Invalid certificate authority configuration");
      }

      if (ca) {
--- a/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesTable.tsx
+++ b/frontend/src/pages/cert-manager/CertificatesPage/components/CertificatesTable.tsx
@@ -175,8 +175,9 @@ export const CertificatesTable = ({ handlePopUpOpen }: Props) => {
                        {/* Only show revoke button if CA supports revocation */}
                        {(() => {
                          const caType = caCapabilityMap[certificate.caId];
+                          // If caId not found in map, assume CA supports revocation to avoid hiding revoke option
                          const supportsRevocation =
-                            caType &&
+                            !caType ||
                            caSupportsCapability(caType, CaCapability.REVOKE_CERTIFICATES);

                          if (!supportsRevocation) {
--- a/frontend/src/pages/cert-manager/PkiSubscriberDetailsByIDPage/components/PkiSubscriberCertificatesTable.tsx
+++ b/frontend/src/pages/cert-manager/PkiSubscriberDetailsByIDPage/components/PkiSubscriberCertificatesTable.tsx
@@ -64,7 +64,7 @@ export const PkiSubscriberCertificatesTable = ({ subscriberName, handlePopUpOpen
  );

  // Fetch CA data to determine capabilities
-  const { data: caData } = useListCasByProjectId(currentWorkspace?.id ?? "");
+  const { data: caData } = useListCasByProjectId(currentWorkspace.id);

  // Create mapping from caId to CA type for capability checking
  const caCapabilityMap = useMemo(() => {
--- a/frontend/src/pages/cert-manager/PkiSubscribersPage/components/PkiSubscriberModal.tsx
+++ b/frontend/src/pages/cert-manager/PkiSubscribersPage/components/PkiSubscriberModal.tsx
@@ -151,8 +151,6 @@ export const PkiSubscriberModal = ({ popUp, handlePopUpToggle }: Props) => {
    projectId
  });

-  console.log(pkiSubscriber);
-
  // Initialize form with ALL subscriber data including template
  useEffect(() => {
    if (pkiSubscriber) {