Merge pull request #4993 from Infisical/fix/pki-routes-cleanup

fix: improvements on certificate request endpoint

backend/src/server/routes/v1/certificate-router.ts

@@ -316,13 +316,11 @@ export const registerCertificateRouter = async (server: FastifyZodProvider) => {
       params: z.object({
         requestId: z.string().uuid()
       }),
-      query: z.object({
-        projectId: z.string().uuid()
-      }),
       response: {
         200: z.object({
           status: z.nativeEnum(CertificateRequestStatus),
           certificate: z.string().nullable(),
+          certificateId: z.string().nullable(),
           privateKey: z.string().nullable(),
           serialNumber: z.string().nullable(),
           errorMessage: z.string().nullable(),
@@ -333,18 +331,17 @@ export const registerCertificateRouter = async (server: FastifyZodProvider) => {
     },
     onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]),
     handler: async (req) => {
-      const data = await server.services.certificateRequest.getCertificateFromRequest({
+      const { certificateRequest, projectId } = await server.services.certificateRequest.getCertificateFromRequest({
         actor: req.permission.type,
         actorId: req.permission.id,
         actorAuthMethod: req.permission.authMethod,
         actorOrgId: req.permission.orgId,
-        projectId: (req.query as { projectId: string }).projectId,
         certificateRequestId: req.params.requestId
       });
 
       await server.services.auditLog.createAuditLog({
         ...req.auditLogInfo,
-        projectId: (req.query as { projectId: string }).projectId,
+        projectId,
         event: {
           type: EventType.GET_CERTIFICATE_REQUEST,
           metadata: {
@@ -352,7 +349,7 @@ export const registerCertificateRouter = async (server: FastifyZodProvider) => {
           }
         }
       });
-      return data;
+      return certificateRequest;
     }
   });
 

backend/src/services/certificate-request/certificate-request-service.test.ts

@@ -258,7 +258,7 @@ describe("CertificateRequestService", () => {
       (mockCertificateService.getCertBody as any).mockResolvedValue(mockCertBody);
       (mockCertificateService.getCertPrivateKey as any).mockResolvedValue(mockPrivateKey);
 
-      const result = await service.getCertificateFromRequest(mockGetData);
+      const { certificateRequest, projectId } = await service.getCertificateFromRequest(mockGetData);
 
       expect(mockCertificateRequestDAL.findByIdWithCertificate).toHaveBeenCalledWith(
         "550e8400-e29b-41d4-a716-446655440005"
@@ -277,8 +277,9 @@ describe("CertificateRequestService", () => {
         actorAuthMethod: AuthMethod.EMAIL,
         actorOrgId: "550e8400-e29b-41d4-a716-446655440002"
       });
-      expect(result).toEqual({
+      expect(certificateRequest).toEqual({
         status: CertificateRequestStatus.ISSUED,
+        certificateId: "550e8400-e29b-41d4-a716-446655440006",
         certificate: "-----BEGIN CERTIFICATE-----\nMOCK_CERT_PEM\n-----END CERTIFICATE-----",
         privateKey: "-----BEGIN PRIVATE KEY-----\nMOCK_KEY_PEM\n-----END PRIVATE KEY-----",
         serialNumber: "123456",
@@ -286,6 +287,7 @@ describe("CertificateRequestService", () => {
         createdAt: mockRequestWithCert.createdAt,
         updatedAt: mockRequestWithCert.updatedAt
       });
+      expect(projectId).toEqual("550e8400-e29b-41d4-a716-446655440003");
     });
 
     it("should get certificate from request successfully when no certificate is attached", async () => {
@@ -310,10 +312,11 @@ describe("CertificateRequestService", () => {
       (mockPermissionService.getProjectPermission as any).mockResolvedValue(mockPermission);
       (mockCertificateRequestDAL.findByIdWithCertificate as any).mockResolvedValue(mockRequestWithoutCert);
 
-      const result = await service.getCertificateFromRequest(mockGetData);
+      const { certificateRequest, projectId } = await service.getCertificateFromRequest(mockGetData);
 
-      expect(result).toEqual({
+      expect(certificateRequest).toEqual({
         status: CertificateRequestStatus.PENDING,
+        certificateId: null,
         certificate: null,
         privateKey: null,
         serialNumber: null,
@@ -321,6 +324,7 @@ describe("CertificateRequestService", () => {
         createdAt: mockRequestWithoutCert.createdAt,
         updatedAt: mockRequestWithoutCert.updatedAt
       });
+      expect(projectId).toEqual("550e8400-e29b-41d4-a716-446655440003");
     });
 
     it("should get certificate from request successfully when user lacks private key permission", async () => {
@@ -354,7 +358,7 @@ describe("CertificateRequestService", () => {
       (mockCertificateRequestDAL.findByIdWithCertificate as any).mockResolvedValue(mockRequestWithCert);
       (mockCertificateService.getCertBody as any).mockResolvedValue(mockCertBody);
 
-      const result = await service.getCertificateFromRequest(mockGetData);
+      const { certificateRequest, projectId } = await service.getCertificateFromRequest(mockGetData);
 
       expect(mockCertificateRequestDAL.findByIdWithCertificate).toHaveBeenCalledWith(
         "550e8400-e29b-41d4-a716-446655440005"
@@ -367,8 +371,9 @@ describe("CertificateRequestService", () => {
         actorOrgId: "550e8400-e29b-41d4-a716-446655440002"
       });
       expect(mockCertificateService.getCertPrivateKey).not.toHaveBeenCalled();
-      expect(result).toEqual({
+      expect(certificateRequest).toEqual({
         status: CertificateRequestStatus.ISSUED,
+        certificateId: "550e8400-e29b-41d4-a716-446655440008",
         certificate: "-----BEGIN CERTIFICATE-----\nMOCK_CERT_PEM\n-----END CERTIFICATE-----",
         privateKey: null,
         serialNumber: "123456",
@@ -376,6 +381,7 @@ describe("CertificateRequestService", () => {
         createdAt: mockRequestWithCert.createdAt,
         updatedAt: mockRequestWithCert.updatedAt
       });
+      expect(projectId).toEqual("550e8400-e29b-41d4-a716-446655440003");
     });
 
     it("should get certificate from request successfully when user has private key permission but key retrieval fails", async () => {
@@ -414,7 +420,7 @@ describe("CertificateRequestService", () => {
       (mockCertificateService.getCertBody as any).mockResolvedValue(mockCertBody);
       (mockCertificateService.getCertPrivateKey as any).mockRejectedValue(new Error("Private key not found"));
 
-      const result = await service.getCertificateFromRequest(mockGetData);
+      const { certificateRequest, projectId } = await service.getCertificateFromRequest(mockGetData);
 
       expect(mockCertificateRequestDAL.findByIdWithCertificate).toHaveBeenCalledWith(
         "550e8400-e29b-41d4-a716-446655440005"
@@ -433,8 +439,9 @@ describe("CertificateRequestService", () => {
         actorAuthMethod: AuthMethod.EMAIL,
         actorOrgId: "550e8400-e29b-41d4-a716-446655440002"
       });
-      expect(result).toEqual({
+      expect(certificateRequest).toEqual({
         status: CertificateRequestStatus.ISSUED,
+        certificateId: "550e8400-e29b-41d4-a716-446655440009",
         certificate: "-----BEGIN CERTIFICATE-----\nMOCK_CERT_PEM\n-----END CERTIFICATE-----",
         privateKey: null,
         serialNumber: "123456",
@@ -442,6 +449,7 @@ describe("CertificateRequestService", () => {
         createdAt: mockRequestWithCert.createdAt,
         updatedAt: mockRequestWithCert.updatedAt
       });
+      expect(projectId).toEqual("550e8400-e29b-41d4-a716-446655440003");
     });
 
     it("should get certificate from request with error message when failed", async () => {
@@ -466,17 +474,19 @@ describe("CertificateRequestService", () => {
       (mockPermissionService.getProjectPermission as any).mockResolvedValue(mockPermission);
       (mockCertificateRequestDAL.findByIdWithCertificate as any).mockResolvedValue(mockFailedRequest);
 
-      const result = await service.getCertificateFromRequest(mockGetData);
+      const { certificateRequest, projectId } = await service.getCertificateFromRequest(mockGetData);
 
-      expect(result).toEqual({
+      expect(certificateRequest).toEqual({
         status: CertificateRequestStatus.FAILED,
         certificate: null,
+        certificateId: null,
         privateKey: null,
         serialNumber: null,
         errorMessage: "Certificate issuance failed",
         createdAt: mockFailedRequest.createdAt,
         updatedAt: mockFailedRequest.updatedAt
       });
+      expect(projectId).toEqual("550e8400-e29b-41d4-a716-446655440003");
     });
 
     it("should throw NotFoundError when certificate request does not exist", async () => {

backend/src/services/certificate-request/certificate-request-service.ts

@@ -170,13 +170,17 @@ export const certificateRequestServiceFactory = ({
     actorId,
     actorAuthMethod,
     actorOrgId,
-    projectId,
     certificateRequestId
   }: TGetCertificateFromRequestDTO) => {
+    const certificateRequest = await certificateRequestDAL.findByIdWithCertificate(certificateRequestId);
+    if (!certificateRequest) {
+      throw new NotFoundError({ message: "Certificate request not found" });
+    }
+
     const { permission } = await permissionService.getProjectPermission({
       actor,
       actorId,
-      projectId,
+      projectId: certificateRequest.projectId,
       actorAuthMethod,
       actorOrgId,
       actionProjectType: ActionProjectType.CertificateManager
@@ -187,25 +191,20 @@ export const certificateRequestServiceFactory = ({
       ProjectPermissionSub.Certificates
     );
 
-    const certificateRequest = await certificateRequestDAL.findByIdWithCertificate(certificateRequestId);
-    if (!certificateRequest) {
-      throw new NotFoundError({ message: "Certificate request not found" });
-    }
-
-    if (certificateRequest.projectId !== projectId) {
-      throw new NotFoundError({ message: "Certificate request not found" });
-    }
-
     // If no certificate is attached, return basic info
     if (!certificateRequest.certificate) {
       return {
-        status: certificateRequest.status as CertificateRequestStatus,
+        certificateRequest: {
-        certificate: null,
+          status: certificateRequest.status as CertificateRequestStatus,
-        privateKey: null,
+          certificate: null,
-        serialNumber: null,
+          certificateId: null,
-        errorMessage: certificateRequest.errorMessage || null,
+          privateKey: null,
-        createdAt: certificateRequest.createdAt,
+          serialNumber: null,
-        updatedAt: certificateRequest.updatedAt
+          errorMessage: certificateRequest.errorMessage || null,
+          createdAt: certificateRequest.createdAt,
+          updatedAt: certificateRequest.updatedAt
+        },
+        projectId: certificateRequest.projectId
       };
     }
 
@@ -240,13 +239,17 @@ export const certificateRequestServiceFactory = ({
     }
 
     return {
-      status: certificateRequest.status as CertificateRequestStatus,
+      certificateRequest: {
-      certificate: certBody.certificate,
+        status: certificateRequest.status as CertificateRequestStatus,
-      privateKey,
+        certificate: certBody.certificate,
-      serialNumber: certificateRequest.certificate.serialNumber,
+        certificateId: certificateRequest.certificate.id,
-      errorMessage: certificateRequest.errorMessage || null,
+        privateKey,
-      createdAt: certificateRequest.createdAt,
+        serialNumber: certificateRequest.certificate.serialNumber,
-      updatedAt: certificateRequest.updatedAt
+        errorMessage: certificateRequest.errorMessage || null,
+        createdAt: certificateRequest.createdAt,
+        updatedAt: certificateRequest.updatedAt
+      },
+      projectId: certificateRequest.projectId
     };
   };
 

backend/src/services/certificate-request/certificate-request-types.ts

@@ -27,7 +27,7 @@ export type TGetCertificateRequestDTO = TProjectPermission & {
   certificateRequestId: string;
 };
 
-export type TGetCertificateFromRequestDTO = TProjectPermission & {
+export type TGetCertificateFromRequestDTO = Omit<TProjectPermission, "projectId"> & {
   certificateRequestId: string;
 };