From 744b02ee005dba58febfb4ce77a3198e2c736dd9 Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Thu, 30 Oct 2025 17:33:42 -0700 Subject: [PATCH] Refactor the code a bit --- backend/bdd/features/pki/acme/auth.feature | 77 +++++++++++++++++++ backend/src/ee/routes/v1/pki-acme-router.ts | 76 +++++++++--------- .../ee/services/pki-acme/pki-acme-service.ts | 17 ++-- .../ee/services/pki-acme/pki-acme-types.ts | 10 +-- backend/src/server/plugins/error-handler.ts | 1 + 5 files changed, 136 insertions(+), 45 deletions(-) create mode 100644 backend/bdd/features/pki/acme/auth.feature diff --git a/backend/bdd/features/pki/acme/auth.feature b/backend/bdd/features/pki/acme/auth.feature new file mode 100644 index 000000000..d9cf5c268 --- /dev/null +++ b/backend/bdd/features/pki/acme/auth.feature @@ -0,0 +1,77 @@ +Feature: Order + + Scenario: Create a new order + Given I have an ACME cert profile as "acme_profile" + When I have an ACME client connecting to {BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/directory +# # TODO: make it I have an account already instead? + Then I register a new ACME account with email fangpen@infisical.com and EAB key id {acme_profile.eab_kid} with secret {acme_profile.eab_secret} as acme_account + When I create certificate signing request as csr + Then I add names to certificate signing request csr + """ + { + "ORGANIZATION_NAME": "Infisical Inc", + "COMMON_NAME": "localhost" + } + """ + Then I create a RSA private key pair as cert_key + Then I sign the certificate signing request csr with private key cert_key and output it as csr_pem in PEM format + Then I submit the certificate signing request PEM csr_pem certificate order to the ACME server as order + Then the value order.uri with jq . should match pattern {BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/orders/(.+) + Then the value order.body with jq .status should be equal to "pending" + Then the value order.body with jq .identifiers should be equal to [{"type": "dns", "value": "localhost"}] + Then the value order.body with jq .finalize should match pattern {BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/orders/(.+)/finalize + Then the value order.body with jq all(.authorizations[]; startswith("{BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/authorizations/")) should be equal to true + + Scenario: Create a new order with SANs + Given I have an ACME cert profile as "acme_profile" + When I have an ACME client connecting to {BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/directory +# # TODO: make it I have an account already instead? + Then I register a new ACME account with email fangpen@infisical.com and EAB key id {acme_profile.eab_kid} with secret {acme_profile.eab_secret} as acme_account + When I create certificate signing request as csr + Then I add names to certificate signing request csr + """ + { + "ORGANIZATION_NAME": "Infisical Inc", + "COMMON_NAME": "localhost" + } + """ + Then I add subject alternative name to certificate signing request csr + """ + [ + "example.com", + "infisical.com" + ] + """ + Then I create a RSA private key pair as cert_key + Then I sign the certificate signing request csr with private key cert_key and output it as csr_pem in PEM format + Then I submit the certificate signing request PEM csr_pem certificate order to the ACME server as order + Then the value order.body with jq .identifiers | sort_by(.value) should be equal to json + """ + [ + {"type": "dns", "value": "example.com"}, + {"type": "dns", "value": "infisical.com"}, + {"type": "dns", "value": "localhost"} + ] + """ + + Scenario: Fetch an order + Given I have an ACME cert profile as "acme_profile" + When I have an ACME client connecting to {BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/directory +# # TODO: make it I have an account already instead? + Then I register a new ACME account with email fangpen@infisical.com and EAB key id {acme_profile.eab_kid} with secret {acme_profile.eab_secret} as acme_account + When I create certificate signing request as csr + Then I add names to certificate signing request csr + """ + { + "ORGANIZATION_NAME": "Infisical Inc", + "COMMON_NAME": "localhost" + } + """ + Then I create a RSA private key pair as cert_key + Then I sign the certificate signing request csr with private key cert_key and output it as csr_pem in PEM format + Then I submit the certificate signing request PEM csr_pem certificate order to the ACME server as order + Then I send an ACME post-as-get to order.uri as fetched_order + Then the value fetched_order with jq .status should be equal to "pending" + Then the value fetched_order with jq .identifiers should be equal to [{"type": "dns", "value": "localhost"}] + Then the value fetched_order with jq .finalize should match pattern {BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/orders/(.+)/finalize + Then the value fetched_order with jq all(.authorizations[]; startswith("{BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/authorizations/")) should be equal to true diff --git a/backend/src/ee/routes/v1/pki-acme-router.ts b/backend/src/ee/routes/v1/pki-acme-router.ts index a35e54cc1..37df5aa36 100644 --- a/backend/src/ee/routes/v1/pki-acme-router.ts +++ b/backend/src/ee/routes/v1/pki-acme-router.ts @@ -1,8 +1,9 @@ /* eslint-disable @typescript-eslint/no-floating-promises */ -import type { TAcmeResponse } from "@app/ee/services/pki-acme/pki-acme-types"; -import { FastifyReply } from "fastify"; +import type { TAcmeResponse, TAuthenciatedJwsPayload, TRawJwsPayload } from "@app/ee/services/pki-acme/pki-acme-types"; +import { FastifyReply, FastifyRequest } from "fastify"; import { z } from "zod"; +import { AcmeMalformedError } from "@app/ee/services/pki-acme/pki-acme-errors"; import { AcmeOrderResourceSchema, CreateAcmeAccountResponseSchema, @@ -19,9 +20,27 @@ import { } from "@app/ee/services/pki-acme/pki-acme-schemas"; import { ApiDocsTags } from "@app/lib/api-docs"; import { readLimit, writeLimit } from "@app/server/config/rateLimiter"; -import { AcmeAccountDoesNotExistError, AcmeMalformedError } from "@app/ee/services/pki-acme/pki-acme-errors"; export const registerPkiAcmeRouter = async (server: FastifyZodProvider) => { + const validateExistingAccount = async < + TSchema extends z.ZodSchema | undefined = undefined, + T = TSchema extends z.ZodSchema ? R : string + >({ + req, + schema + }: { + req: FastifyRequest<{ Params: { profileId: string; accountId?: string }; Body: TRawJwsPayload }>; + schema?: TSchema; + }): Promise> => { + return await server.services.pkiAcme.validateExistingAccountJwsPayload({ + url: new URL(req.url, `${req.protocol}://${req.hostname}`), + profileId: req.params.profileId, + rawJwsPayload: req.body, + schema, + expectedAccountId: req.params.accountId + }); + }; + const sendAcmeResponse = async (res: FastifyReply, profileId: string, response: TAcmeResponse): Promise => { res.code(response.status); for (const [key, value] of Object.entries(response.headers)) { @@ -163,12 +182,9 @@ export const registerPkiAcmeRouter = async (server: FastifyZodProvider) => { // TODO: replace with verify ACME signature here instead // onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req, res) => { - const { payload, profileId, accountId } = await server.services.pkiAcme.validateExistingAccountJwsPayload({ - url: req.url, - profileId: req.params.profileId, - rawJwsPayload: req.body, - schema: DeactivateAcmeAccountBodySchema, - expectedAccountId: req.params.accountId + const { payload, profileId, accountId } = await validateExistingAccount({ + req, + schema: DeactivateAcmeAccountBodySchema }); return sendAcmeResponse( res, @@ -205,10 +221,8 @@ export const registerPkiAcmeRouter = async (server: FastifyZodProvider) => { // TODO: replace with verify ACME signature here instead // onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req, res) => { - const { profileId, accountId, payload } = await server.services.pkiAcme.validateExistingAccountJwsPayload({ - url: req.url, - profileId: req.params.profileId, - rawJwsPayload: req.body, + const { profileId, accountId, payload } = await validateExistingAccount({ + req, schema: CreateAcmeOrderBodySchema }); return sendAcmeResponse( @@ -247,10 +261,9 @@ export const registerPkiAcmeRouter = async (server: FastifyZodProvider) => { // TODO: replace with verify ACME signature here instead // onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req, res) => { - const { profileId, accountId } = await server.services.pkiAcme.validateExistingAccountJwsPayload({ - url: req.url, - profileId: req.params.profileId, - rawJwsPayload: req.body + const { profileId, accountId } = await validateExistingAccount({ + req, + schema: FinalizeAcmeOrderBodySchema }); return sendAcmeResponse( res, @@ -288,10 +301,8 @@ export const registerPkiAcmeRouter = async (server: FastifyZodProvider) => { // TODO: replace with verify ACME signature here instead // onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req, res) => { - const { profileId, accountId, payload } = await server.services.pkiAcme.validateExistingAccountJwsPayload({ - url: req.url, - profileId: req.params.profileId, - rawJwsPayload: req.body, + const { profileId, accountId, payload } = await validateExistingAccount({ + req, schema: FinalizeAcmeOrderBodySchema }); return sendAcmeResponse( @@ -330,12 +341,9 @@ export const registerPkiAcmeRouter = async (server: FastifyZodProvider) => { // TODO: replace with verify ACME signature here instead // onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req, res) => { - const { profileId, accountId } = await server.services.pkiAcme.validateExistingAccountJwsPayload({ - url: req.url, - profileId: req.params.profileId, - rawJwsPayload: req.body, - schema: ListAcmeOrdersPayloadSchema, - expectedAccountId: req.params.accountId + const { profileId, accountId } = await validateExistingAccount({ + req, + schema: ListAcmeOrdersPayloadSchema }); return sendAcmeResponse( res, @@ -372,10 +380,9 @@ export const registerPkiAcmeRouter = async (server: FastifyZodProvider) => { // TODO: replace with verify ACME signature here instead // onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req, res) => { - const { profileId, accountId } = await server.services.pkiAcme.validateExistingAccountJwsPayload({ - url: req.url, - profileId: req.params.profileId, - rawJwsPayload: req.body + const { profileId, accountId } = await validateExistingAccount({ + req, + schema: FinalizeAcmeOrderBodySchema }); return sendAcmeResponse( res, @@ -409,10 +416,9 @@ export const registerPkiAcmeRouter = async (server: FastifyZodProvider) => { // TODO: replace with verify ACME signature here instead // onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), handler: async (req, res) => { - const { profileId, accountId, payload } = await server.services.pkiAcme.validateExistingAccountJwsPayload({ - url: req.url, - profileId: req.params.profileId, - rawJwsPayload: req.body + const { profileId, accountId, payload } = await validateExistingAccount({ + req, + schema: GetAcmeAuthorizationBodySchema }); if (payload !== "") { throw new AcmeMalformedError({ detail: "Payload should be empty" }); diff --git a/backend/src/ee/services/pki-acme/pki-acme-service.ts b/backend/src/ee/services/pki-acme/pki-acme-service.ts index 0658886a4..3508d8c93 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-service.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-service.ts @@ -6,6 +6,7 @@ import { NotFoundError } from "@app/lib/errors"; import { logger } from "@app/lib/logger"; import { TCertificateProfileDALFactory } from "@app/services/certificate-profile/certificate-profile-dal"; +import { TPkiAcmeChallenges } from "@app/db/schemas"; import { EnrollmentType, TCertificateProfileWithConfigs @@ -17,6 +18,7 @@ import { TPkiAcmeAuthDALFactory } from "./pki-acme-auth-dal"; import { AcmeAccountDoesNotExistError, AcmeBadPublicKeyError, + AcmeError, AcmeMalformedError, AcmeServerInternalError, AcmeUnauthorizedError, @@ -49,7 +51,6 @@ import { TRawJwsPayload, TRespondToAcmeChallengeResponse } from "./pki-acme-types"; -import { TPkiAcmeChallenges } from "@app/db/schemas"; type TPkiAcmeServiceFactoryDep = { certificateProfileDAL: Pick; @@ -100,7 +101,7 @@ export const pkiAcmeServiceFactory = ({ getJWK, schema }: { - url: string; + url: URL; rawJwsPayload: TRawJwsPayload; getJWK: (protectedHeader: JWSHeaderParameters) => Promise; schema?: TSchema; @@ -115,6 +116,9 @@ export const pkiAcmeServiceFactory = ({ return await importJWK(jwk, protectedHeader.alg); }); } catch (error) { + if (error instanceof AcmeError) { + throw error; + } if (error instanceof ZodError) { throw new AcmeMalformedError({ detail: `Invalid JWS payload: ${error.message}` }); } @@ -127,7 +131,7 @@ export const pkiAcmeServiceFactory = ({ const { protectedHeader: rawProtectedHeader, payload: rawPayload } = result; try { const protectedHeader = ProtectedHeaderSchema.parse(rawProtectedHeader); - if (protectedHeader.url !== url) { + if (new URL(protectedHeader.url).href !== url.href) { throw new AcmeUnauthorizedError({ detail: "URL mismatch in the protected header" }); } // TODO: consume the nonce here @@ -139,6 +143,9 @@ export const pkiAcmeServiceFactory = ({ payload }; } catch (error) { + if (error instanceof AcmeError) { + throw error; + } if (error instanceof ZodError) { throw new AcmeMalformedError({ detail: `Invalid JWS payload: ${error.message}` }); } @@ -151,7 +158,7 @@ export const pkiAcmeServiceFactory = ({ url, rawJwsPayload }: { - url: string; + url: URL; rawJwsPayload: TRawJwsPayload; }): Promise> => { return await validateJwsPayload({ @@ -177,7 +184,7 @@ export const pkiAcmeServiceFactory = ({ schema, expectedAccountId }: { - url: string; + url: URL; profileId: string; rawJwsPayload: TRawJwsPayload; schema?: TSchema; diff --git a/backend/src/ee/services/pki-acme/pki-acme-types.ts b/backend/src/ee/services/pki-acme/pki-acme-types.ts index 1f962a220..082078f2a 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-types.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-types.ts @@ -58,16 +58,16 @@ export type TPkiAcmeServiceFactory = { getJWK, schema }: { - url: string; + url: URL; rawJwsPayload: TRawJwsPayload; getJWK: (protectedHeader: JWSHeaderParameters) => Promise; - schema?: z.ZodSchema; - }) => Promise>; + schema?: TSchema; + }) => Promise>; validateNewAccountJwsPayload: ({ url, rawJwsPayload }: { - url: string; + url: URL; rawJwsPayload: TRawJwsPayload; }) => Promise>; validateExistingAccountJwsPayload: < @@ -80,7 +80,7 @@ export type TPkiAcmeServiceFactory = { schema, expectedAccountId }: { - url: string; + url: URL; profileId: string; rawJwsPayload: TRawJwsPayload; schema?: TSchema; diff --git a/backend/src/server/plugins/error-handler.ts b/backend/src/server/plugins/error-handler.ts index 50251377d..8f6b63141 100644 --- a/backend/src/server/plugins/error-handler.ts +++ b/backend/src/server/plugins/error-handler.ts @@ -248,6 +248,7 @@ export const fastifyErrHandler = fastifyPlugin(async (server: FastifyZodProvider .type("application/problem+json") .status(error.status) .send({ + reqId: req.id, status: error.status, type: `urn:ietf:params:acme:error:${error.type}`, detail: error.detail