From ae48787834103050217da3b5e4331f2f1ac817a2 Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Mon, 17 Nov 2025 17:29:39 -0800 Subject: [PATCH 01/12] Return external account required --- backend/src/ee/services/pki-acme/pki-acme-schemas.ts | 10 +++++++++- backend/src/ee/services/pki-acme/pki-acme-service.ts | 5 ++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/backend/src/ee/services/pki-acme/pki-acme-schemas.ts b/backend/src/ee/services/pki-acme/pki-acme-schemas.ts index 58ca7e833..23b86d172 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-schemas.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-schemas.ts @@ -58,7 +58,15 @@ export const GetAcmeDirectoryResponseSchema = z.object({ newNonce: z.string(), newAccount: z.string(), newOrder: z.string(), - revokeCert: z.string().optional() + revokeCert: z.string().optional(), + meta: z + .object({ + termsOfService: z.string().optional(), + website: z.string().optional(), + caaIdentities: z.array(z.string()).optional(), + externalAccountRequired: z.boolean().optional() + }) + .optional() }); // New Account payload schema 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 43da08b1c..ccacf816e 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-service.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-service.ts @@ -353,7 +353,10 @@ export const pkiAcmeServiceFactory = ({ return { newNonce: buildUrl(profile.id, "/new-nonce"), newAccount: buildUrl(profile.id, "/new-account"), - newOrder: buildUrl(profile.id, "/new-order") + newOrder: buildUrl(profile.id, "/new-order"), + meta: { + externalAccountRequired: true + } }; }; From b251cf480206488658f1ae2cd505eead94832acb Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Mon, 17 Nov 2025 17:40:29 -0800 Subject: [PATCH 02/12] Update BDD test to ensure external account required value is returned --- .../pki/acme/{dicrectory.feature => directory.feature} | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) rename backend/bdd/features/pki/acme/{dicrectory.feature => directory.feature} (86%) diff --git a/backend/bdd/features/pki/acme/dicrectory.feature b/backend/bdd/features/pki/acme/directory.feature similarity index 86% rename from backend/bdd/features/pki/acme/dicrectory.feature rename to backend/bdd/features/pki/acme/directory.feature index 664ff7457..53084a681 100644 --- a/backend/bdd/features/pki/acme/dicrectory.feature +++ b/backend/bdd/features/pki/acme/directory.feature @@ -9,6 +9,9 @@ Feature: Directory { "newNonce": "{BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/new-nonce", "newAccount": "{BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/new-account", - "newOrder": "{BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/new-order" + "newOrder": "{BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/new-order", + "meta": { + "externalAccountRequired": true + } } """ From d190fb15c986362d6a09fdecc6352a1683538b56 Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Tue, 18 Nov 2025 09:20:22 -0800 Subject: [PATCH 03/12] Add test to reproduce the problem --- backend/bdd/features/pki/acme/account.feature | 23 ++++++++++++++-- backend/bdd/features/steps/pki_acme.py | 27 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/backend/bdd/features/pki/acme/account.feature b/backend/bdd/features/pki/acme/account.feature index 589c5ab24..04b1f29c6 100644 --- a/backend/bdd/features/pki/acme/account.feature +++ b/backend/bdd/features/pki/acme/account.feature @@ -11,8 +11,27 @@ Feature: Account When I have an ACME client connecting to "{BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/directory" 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 And I memorize acme_account.uri as account_uri - And I find the existing ACME account with email fangpen@infisical.com and EAB key id "{acme_profile.eab_kid}" with secret "{acme_profile.eab_secret}" as acme_account - And the value acme_account.uri should be equal to "{account_uri}" + And I find the existing ACME account with email fangpen@infisical.com and EAB key id "{acme_profile.eab_kid}" with secret "{acme_profile.eab_secret}" as retrieved_account + And the value retrieved_account.uri should be equal to "{account_uri}" + + # Note: This is a very special case for cert-manager. + # There's a bug in their ACME client implementation, they don't take the account KID value they have + # and relying on a '{"onlyReturnExisting": true}' new-account request to find out their KID value. + # But the problem is, that new-account request doesn't come with EAB. And while the get existing account operation + # fails, they just discard the error and proceed to request a new order. Since no KID provided, their ACME + # client will send JWK instead. As a result, we are seeing KID not provide in header error for the new-order + # endpoint. + # + # To solve the problem, we lose the check for EAB a bit for the onlyReturnExisting new account request + # ref: https://github.com/cert-manager/cert-manager/issues/7388#issuecomment-3535630925 + Scenario: Create a new account with EAB then retrieve it without EAB + 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" + 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 + And I memorize acme_account.uri as account_uri + And I find the existing ACME account without EAB as retrieved_account + And the value error with should be absent + And the value retrieved_account.uri should be equal to "{account_uri}" Scenario: Create a new account without EAB Given I have an ACME cert profile as "acme_profile" diff --git a/backend/bdd/features/steps/pki_acme.py b/backend/bdd/features/steps/pki_acme.py index 5043b0e7d..e194fdb84 100644 --- a/backend/bdd/features/steps/pki_acme.py +++ b/backend/bdd/features/steps/pki_acme.py @@ -434,6 +434,20 @@ def step_impl(context: Context, email: str, kid: str, secret: str, account_var: ) +@then("I find the existing ACME account without EAB as {account_var}") +def step_impl(context: Context, account_var: str): + acme_client = context.acme_client + registration = messages.NewRegistration.from_data( + only_return_existing=True, + ) + # Reset account so that it will send JWK instead of KID + acme_client.net.account = None + try: + context.vars[account_var] = acme_client.new_account(registration) + except Exception as exp: + context.vars["error"] = exp + + @then("I register a new ACME account with email {email} without EAB") def step_impl(context: Context, email: str): acme_client = context.acme_client @@ -600,6 +614,19 @@ def step_impl(context: Context, var_path: str, jq_query: str): ) +@then("the value {var_path} with should be absent") +def step_impl(context: Context, var_path: str): + try: + value = eval_var(context, var_path) + except Exception as exp: + if isinstance(exp, KeyError): + return + raise + assert False, ( + f"value at {var_path!r} should be absent, but we got this instead: {value!r}" + ) + + @then('the value {var_path} with jq "{jq_query}" should be equal to {expected}') def step_impl(context: Context, var_path: str, jq_query: str, expected: str): value, result = apply_value_with_jq( From a668822e192495c4163ab0701d18b55d5566a5fb Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Tue, 18 Nov 2025 09:28:10 -0800 Subject: [PATCH 04/12] Only check EAB when creating a new account --- backend/bdd/features/pki/acme/account.feature | 9 --- .../ee/services/pki-acme/pki-acme-service.ts | 63 +++++++++++-------- 2 files changed, 38 insertions(+), 34 deletions(-) diff --git a/backend/bdd/features/pki/acme/account.feature b/backend/bdd/features/pki/acme/account.feature index 04b1f29c6..5214464a3 100644 --- a/backend/bdd/features/pki/acme/account.feature +++ b/backend/bdd/features/pki/acme/account.feature @@ -15,15 +15,6 @@ Feature: Account And the value retrieved_account.uri should be equal to "{account_uri}" # Note: This is a very special case for cert-manager. - # There's a bug in their ACME client implementation, they don't take the account KID value they have - # and relying on a '{"onlyReturnExisting": true}' new-account request to find out their KID value. - # But the problem is, that new-account request doesn't come with EAB. And while the get existing account operation - # fails, they just discard the error and proceed to request a new order. Since no KID provided, their ACME - # client will send JWK instead. As a result, we are seeing KID not provide in header error for the new-order - # endpoint. - # - # To solve the problem, we lose the check for EAB a bit for the onlyReturnExisting new account request - # ref: https://github.com/cert-manager/cert-manager/issues/7388#issuecomment-3535630925 Scenario: Create a new account with EAB then retrieve it without EAB 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" 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 ccacf816e..d9c28b498 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-service.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-service.ts @@ -389,11 +389,48 @@ export const pkiAcmeServiceFactory = ({ payload: TCreateAcmeAccountPayload; }): Promise> => { const profile = await validateAcmeProfile(profileId); + const publicKeyThumbprint = await calculateJwkThumbprint(jwk, "sha256"); + + if (onlyReturnExisting) { + const existingAccount: TPkiAcmeAccounts | null = await acmeAccountDAL.findByProfileIdAndPublicKeyThumbprintAndAlg( + profileId, + alg, + publicKeyThumbprint + ); + if (!existingAccount) { + throw new AcmeAccountDoesNotExistError({ message: "ACME account not found" }); + } + // With the same public key, we found an existing account, just return it + return { + status: 200, + body: { + status: "valid", + contact: existingAccount.emails, + orders: buildUrl(profile.id, `/accounts/${existingAccount.id}/orders`) + }, + headers: { + Location: buildUrl(profile.id, `/accounts/${existingAccount.id}`), + Link: `<${buildUrl(profile.id, "/directory")}>;rel="index"` + } + }; + } + + // Note: We only check EAB for the new account request. This is a very special case for cert-manager. + // There's a bug in their ACME client implementation, they don't take the account KID value they have + // and relying on a '{"onlyReturnExisting": true}' new-account request to find out their KID value. + // But the problem is, that new-account request doesn't come with EAB. And while the get existing account operation + // fails, they just discard the error and proceed to request a new order. Since no KID provided, their ACME + // client will send JWK instead. As a result, we are seeing KID not provide in header error for the new-order + // endpoint. + // + // To solve the problem, we lose the check for EAB a bit for the onlyReturnExisting new account request. + // It should be fine as we've already checked EAB when they created the account. + // And the private key ownership indicating they are the same user. + // ref: https://github.com/cert-manager/cert-manager/issues/7388#issuecomment-3535630925 if (!externalAccountBinding) { throw new AcmeExternalAccountRequiredError({ message: "External account binding is required" }); } - const publicKeyThumbprint = await calculateJwkThumbprint(jwk, "sha256"); const certificateManagerKmsId = await getProjectKmsCertificateKeyId({ projectId: profile.projectId, projectDAL, @@ -444,30 +481,6 @@ export const pkiAcmeServiceFactory = ({ }); } - const existingAccount: TPkiAcmeAccounts | null = await acmeAccountDAL.findByProfileIdAndPublicKeyThumbprintAndAlg( - profileId, - alg, - publicKeyThumbprint - ); - if (onlyReturnExisting && !existingAccount) { - throw new AcmeAccountDoesNotExistError({ message: "ACME account not found" }); - } - if (existingAccount) { - // With the same public key, we found an existing account, just return it - return { - status: 200, - body: { - status: "valid", - contact: existingAccount.emails, - orders: buildUrl(profile.id, `/accounts/${existingAccount.id}/orders`) - }, - headers: { - Location: buildUrl(profile.id, `/accounts/${existingAccount.id}`), - Link: `<${buildUrl(profile.id, "/directory")}>;rel="index"` - } - }; - } - const newAccount = await acmeAccountDAL.create({ profileId: profile.id, alg, From 9ae862fc1fbd2e68ae67f885f250a8a0ce97159b Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Tue, 18 Nov 2025 09:46:40 -0800 Subject: [PATCH 05/12] Fix find existing account --- backend/bdd/features/steps/pki_acme.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/backend/bdd/features/steps/pki_acme.py b/backend/bdd/features/steps/pki_acme.py index e194fdb84..1390cab3f 100644 --- a/backend/bdd/features/steps/pki_acme.py +++ b/backend/bdd/features/steps/pki_acme.py @@ -437,13 +437,10 @@ def step_impl(context: Context, email: str, kid: str, secret: str, account_var: @then("I find the existing ACME account without EAB as {account_var}") def step_impl(context: Context, account_var: str): acme_client = context.acme_client - registration = messages.NewRegistration.from_data( - only_return_existing=True, - ) - # Reset account so that it will send JWK instead of KID - acme_client.net.account = None + # registration = messages.RegistrationResource.from_json(dict(uri="")) + registration = acme_client.net.account try: - context.vars[account_var] = acme_client.new_account(registration) + context.vars[account_var] = acme_client.query_registration(registration) except Exception as exp: context.vars["error"] = exp From b50f03d4fb961dec4b3ac9ff322280c9fc69fce5 Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Tue, 18 Nov 2025 15:20:30 -0800 Subject: [PATCH 06/12] Add test case for both jwk and kid provided --- .../features/pki/acme/access-control.feature | 50 ++++++++++++++++++- .../pki-acme/pki-acme-challenge-service.ts | 7 ++- .../ee/services/pki-acme/pki-acme-service.ts | 3 ++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/backend/bdd/features/pki/acme/access-control.feature b/backend/bdd/features/pki/acme/access-control.feature index 6615d00f8..50588be76 100644 --- a/backend/bdd/features/pki/acme/access-control.feature +++ b/backend/bdd/features/pki/acme/access-control.feature @@ -221,7 +221,6 @@ Feature: Access Control | order | .authorizations[0].uri | auth_uri | {auth_uri} | | | order | .authorizations[0].body.challenges[0].url | challenge_uri | {challenge_uri} | {} | - Scenario Outline: URL mismatch 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" @@ -271,3 +270,52 @@ Feature: Access Control | order | .authorizations[0].uri | auth_uri | {auth_uri} | https://example.com/acmes/auths/FOOBAR | URL mismatch in the protected header | | order | .authorizations[0].body.challenges[0].url | challenge_uri | {challenge_uri} | BAD | Invalid URL in the protected header | | order | .authorizations[0].body.challenges[0].url | challenge_uri | {challenge_uri} | https://example.com/acmes/challenges/FOOBAR | URL mismatch in the protected header | + + Scenario Outline: Send KID and JWK in the same time + 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" + 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 + And I memorize acme_account.uri with jq "capture("/(?[^/]+)$") | .id" as account_id + When I create certificate signing request as csr + Then I add names to certificate signing request csr + """ + { + "COMMON_NAME": "localhost" + } + """ + Then I create a RSA private key pair as cert_key + And I sign the certificate signing request csr with private key cert_key and output it as csr_pem in PEM format + And I submit the certificate signing request PEM csr_pem certificate order to the ACME server as order + And I peak and memorize the next nonce as nonce_value + And I memorize with jq "" as + When I send a raw ACME request to "" + """ + { + "protected": { + "alg": "RS256", + "nonce": "{nonce_value}", + "url": "", + "kid": "{acme_account.uri}", + "jwk": { + "n": "mmEWxUv2lUYDZe_M2FXJ_WDXgHoEG7PVvg-dfz1STzyMwx0qvM66KMenXSyVA0r-_Ssb6p8VexSWGOFKskM4ryKUihn2KNH5e8nXZBqzqYeKQ8vqaCdaWzTxFI1dg0xhk0CWptkZHxpRpLalztFJ1Pq7L2qvQOM2YT7wPYbwQhpaSiVNXAb1W4FwAPyC04v1mHehvST-esaDT7j_5-eU5cCcmyi4_g5nBawcinOjj5o3VCg4X8UjK--AjhAyYHx1nRMr-7xk4x-0VIpQ_OODjLB3WzN8s1YEb0Jx5Bv1JyeCw35zahqs3fAFyRje-p5ENk9NCxfz5x9ZGkszkkNt0Q", + "e": "AQAB", + "kty": "RSA" + } + }, + "payload": {} + } + """ + Then the value response.status_code should be equal to 400 + And the value response with jq ".status" should be equal to 400 + And the value response with jq ".type" should be equal to "urn:ietf:params:acme:error:malformed" + And the value response with jq ".detail" should be equal to "Both JWK and KID are provided in the protected header" + + Examples: Endpoints + | src_var | jq | dest_var | url | + | order | . | not_used | {BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/accounts/{account_id}/orders | + | order | . | not_used | {BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/new-order | + | order | . | not_used | {order.uri} | + | order | . | not_used | {order.uri}/finalize | + | order | . | not_used | {order.uri}/certificate | + | order | .authorizations[0].uri | auth_uri | {auth_uri} | + | order | .authorizations[0].body.challenges[0].url | challenge_uri | {challenge_uri} | diff --git a/backend/src/ee/services/pki-acme/pki-acme-challenge-service.ts b/backend/src/ee/services/pki-acme/pki-acme-challenge-service.ts index 9148b0336..7a3747fed 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-challenge-service.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-challenge-service.ts @@ -74,7 +74,12 @@ export const pkiAcmeChallengeServiceFactory = ({ // Notice: well, we are in a transaction, ideally we should not hold transaction and perform // a long running operation for long time. But assuming we are not performing a tons of // challenge validation at the same time, it should be fine. - const challengeResponse = await fetch(challengeUrl, { signal: AbortSignal.timeout(timeoutMs) }); + const challengeResponse = await fetch(challengeUrl, { + // In case if we override the host in the development mode, still provide the original host in the header + // to help the upstream server to validate the request + headers: { Host: host }, + signal: AbortSignal.timeout(timeoutMs) + }); if (challengeResponse.status !== 200) { throw new AcmeIncorrectResponseError({ message: `ACME challenge response is not 200: ${challengeResponse.status}` 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 d9c28b498..71a2383b6 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-service.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-service.ts @@ -206,6 +206,9 @@ export const pkiAcmeServiceFactory = ({ const { protectedHeader: rawProtectedHeader, payload: rawPayload } = result; try { const protectedHeader = ProtectedHeaderSchema.parse(rawProtectedHeader); + if (protectedHeader.jwk && protectedHeader.kid) { + throw new AcmeMalformedError({ message: "Both JWK and KID are provided in the protected header" }); + } const parsedUrl = (() => { try { return new URL(protectedHeader.url); From d341dd0fb75987dc0890b7ae68469fe9251aff80 Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Tue, 18 Nov 2025 16:59:06 -0800 Subject: [PATCH 07/12] Return existing even if `onlyReturnExisting` is not set to ture while creating the new one --- .../ee/services/pki-acme/pki-acme-service.ts | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) 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 71a2383b6..1d724495f 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-service.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-service.ts @@ -291,6 +291,7 @@ export const pkiAcmeServiceFactory = ({ url, rawJwsPayload, getJWK: async (protectedHeader) => { + // get jwk instead of kid if (!protectedHeader.kid) { throw new AcmeMalformedError({ message: "KID is required in the protected header" }); } @@ -394,16 +395,15 @@ export const pkiAcmeServiceFactory = ({ const profile = await validateAcmeProfile(profileId); const publicKeyThumbprint = await calculateJwkThumbprint(jwk, "sha256"); + const existingAccount: TPkiAcmeAccounts | null = await acmeAccountDAL.findByProfileIdAndPublicKeyThumbprintAndAlg( + profileId, + alg, + publicKeyThumbprint + ); if (onlyReturnExisting) { - const existingAccount: TPkiAcmeAccounts | null = await acmeAccountDAL.findByProfileIdAndPublicKeyThumbprintAndAlg( - profileId, - alg, - publicKeyThumbprint - ); if (!existingAccount) { throw new AcmeAccountDoesNotExistError({ message: "ACME account not found" }); } - // With the same public key, we found an existing account, just return it return { status: 200, body: { @@ -433,6 +433,20 @@ export const pkiAcmeServiceFactory = ({ if (!externalAccountBinding) { throw new AcmeExternalAccountRequiredError({ message: "External account binding is required" }); } + if (existingAccount) { + return { + status: 200, + body: { + status: "valid", + contact: existingAccount.emails, + orders: buildUrl(profile.id, `/accounts/${existingAccount.id}/orders`) + }, + headers: { + Location: buildUrl(profile.id, `/accounts/${existingAccount.id}`), + Link: `<${buildUrl(profile.id, "/directory")}>;rel="index"` + } + }; + } const certificateManagerKmsId = await getProjectKmsCertificateKeyId({ projectId: profile.projectId, From d9d6d3b3d985ccc3118cc6c29aff6279f061e543 Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Tue, 18 Nov 2025 18:48:42 -0800 Subject: [PATCH 08/12] Add test to ensure that we don't create duplicate account --- backend/bdd/features/pki/acme/account.feature | 9 +++++++++ backend/bdd/features/steps/pki_acme.py | 2 ++ 2 files changed, 11 insertions(+) diff --git a/backend/bdd/features/pki/acme/account.feature b/backend/bdd/features/pki/acme/account.feature index 5214464a3..14e304c6c 100644 --- a/backend/bdd/features/pki/acme/account.feature +++ b/backend/bdd/features/pki/acme/account.feature @@ -6,6 +6,15 @@ Feature: Account 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 And the value acme_account.uri with jq "." should match pattern {BASE_URL}/api/v1/pki/acme/profiles/{acme_profile.id}/accounts/(.+) + Scenario: Create a new account with the same key pair twice + 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" + 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 + And I memorize acme_account.uri as kid + And 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_account2 + And the value error.__class__.__name__ should be equal to "ConflictError" + And the value error.location should be equal to "{kid}" + Scenario: Find an existing account 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" diff --git a/backend/bdd/features/steps/pki_acme.py b/backend/bdd/features/steps/pki_acme.py index 1390cab3f..51ab4209e 100644 --- a/backend/bdd/features/steps/pki_acme.py +++ b/backend/bdd/features/steps/pki_acme.py @@ -387,6 +387,8 @@ def register_account_with_eab( ): acme_client = context.acme_client account_public_key = acme_client.net.key.public_key() + # clear the account in case if we want to register twice + acme_client.net.account = None if hasattr(context, "alt_eab_url"): eab_directory = messages.Directory.from_json( {"newAccount": context.alt_eab_url} From 022c3802e3a2124350be0274c492c1a832f498f1 Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Tue, 18 Nov 2025 19:00:10 -0800 Subject: [PATCH 09/12] Add unique constraint for public key under the same profile --- ...-acme-account-public-key-and-profile-id.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 backend/src/db/migrations/20251119025017_add-unique-constraint-for-pki-acme-account-public-key-and-profile-id.ts diff --git a/backend/src/db/migrations/20251119025017_add-unique-constraint-for-pki-acme-account-public-key-and-profile-id.ts b/backend/src/db/migrations/20251119025017_add-unique-constraint-for-pki-acme-account-public-key-and-profile-id.ts new file mode 100644 index 000000000..5bc4601e3 --- /dev/null +++ b/backend/src/db/migrations/20251119025017_add-unique-constraint-for-pki-acme-account-public-key-and-profile-id.ts @@ -0,0 +1,32 @@ +import { Knex } from "knex"; + +import { dropConstraintIfExists } from "@app/db/migrations/utils/dropConstraintIfExists"; +import { TableName } from "@app/db/schemas"; + +const CONSTRAINT_NAME = "unique_pki_acme_account_public_key_and_profile_id"; + +export async function up(knex: Knex): Promise { + if (await knex.schema.hasTable(TableName.PkiAcmeAccount)) { + const hasProfileId = await knex.schema.hasColumn(TableName.PkiAcmeAccount, "profileId"); + const hasPublicKeyThumbprint = await knex.schema.hasColumn(TableName.PkiAcmeAccount, "publicKeyThumbprint"); + + if (hasProfileId && hasPublicKeyThumbprint) { + await knex.schema.alterTable(TableName.PkiAcmeAccount, (table) => { + table.unique(["profileId", "publicKeyThumbprint"], { indexName: CONSTRAINT_NAME }); + }); + } + } +} + +export async function down(knex: Knex): Promise { + if (await knex.schema.hasTable(TableName.PkiAcmeAccount)) { + const hasProfileId = await knex.schema.hasColumn(TableName.PkiAcmeAccount, "profileId"); + const hasPublicKeyThumbprint = await knex.schema.hasColumn(TableName.PkiAcmeAccount, "publicKeyThumbprint"); + + await knex.schema.alterTable(TableName.PkiAcmeAccount, async () => { + if (hasProfileId && hasPublicKeyThumbprint) { + await dropConstraintIfExists(TableName.PkiAcmeAccount, CONSTRAINT_NAME, knex); + } + }); + } +} From 2026fcd628a48b55070d062fd3f79148113bec01 Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Tue, 18 Nov 2025 19:22:37 -0800 Subject: [PATCH 10/12] Isolate vars --- backend/bdd/features/environment.py | 17 +++++++++++------ backend/bdd/features/steps/pki_acme.py | 13 ++++++++++--- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/backend/bdd/features/environment.py b/backend/bdd/features/environment.py index 9a2e9f90b..dce0eb538 100644 --- a/backend/bdd/features/environment.py +++ b/backend/bdd/features/environment.py @@ -185,28 +185,33 @@ def bootstrap_infisical(context: Context): def before_all(context: Context): + base_vars = { + "BASE_URL": BASE_URL, + "PEBBLE_URL": PEBBLE_URL, + } if BOOTSTRAP_INFISICAL: details = bootstrap_infisical(context) - context.vars = { - "BASE_URL": BASE_URL, - "PEBBLE_URL": PEBBLE_URL, + vars = base_vars | { "PROJECT_ID": details["project"]["id"], "CERT_CA_ID": details["ca"]["id"], "CERT_TEMPLATE_ID": details["cert_template"]["id"], "AUTH_TOKEN": details["auth_token"], } else: - context.vars = { - "BASE_URL": BASE_URL, - "PEBBLE_URL": PEBBLE_URL, + vars = base_vars | { "PROJECT_ID": PROJECT_ID, "CERT_CA_ID": CERT_CA_ID, "CERT_TEMPLATE_ID": CERT_TEMPLATE_ID, "AUTH_TOKEN": AUTH_TOKEN, } + context._initial_vars = vars context.http_client = httpx.Client(base_url=BASE_URL) +def before_scenario(context: Context, scenario: typing.Any): + context.vars = deepcopy(context._initial_vars) + + def after_scenario(context: Context, scenario: typing.Any): if hasattr(context, "web_server"): context.web_server.shutdown_and_server_close() diff --git a/backend/bdd/features/steps/pki_acme.py b/backend/bdd/features/steps/pki_acme.py index 51ab4209e..353ec942d 100644 --- a/backend/bdd/features/steps/pki_acme.py +++ b/backend/bdd/features/steps/pki_acme.py @@ -387,8 +387,9 @@ def register_account_with_eab( ): acme_client = context.acme_client account_public_key = acme_client.net.key.public_key() - # clear the account in case if we want to register twice - acme_client.net.account = None + if not only_return_existing: + # clear the account in case if we want to register twice + acme_client.net.account = None if hasattr(context, "alt_eab_url"): eab_directory = messages.Directory.from_json( {"newAccount": context.alt_eab_url} @@ -408,8 +409,14 @@ def register_account_with_eab( only_return_existing=only_return_existing, ) try: - context.vars[account_var] = acme_client.new_account(registration) + if not only_return_existing: + context.vars[account_var] = acme_client.new_account(registration) + else: + context.vars[account_var] = acme_client.query_registration( + acme_client.net.account + ) except Exception as exp: + logger.error(f"Failed to register: {exp}", exc_info=True) context.vars["error"] = exp From ce418036294e30dfe3e92ece5f9c1e1dd66a6ebf Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Tue, 18 Nov 2025 19:24:20 -0800 Subject: [PATCH 11/12] deepcopy --- backend/bdd/features/environment.py | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/bdd/features/environment.py b/backend/bdd/features/environment.py index dce0eb538..976998c72 100644 --- a/backend/bdd/features/environment.py +++ b/backend/bdd/features/environment.py @@ -3,6 +3,7 @@ import os import pathlib import typing +from copy import deepcopy import httpx from behave.runner import Context From 2093dee2d1a5103eef43c439af4cf8357bdeb1a8 Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Tue, 18 Nov 2025 19:40:04 -0800 Subject: [PATCH 12/12] Added todo --- backend/src/ee/services/pki-acme/pki-acme-service.ts | 1 + 1 file changed, 1 insertion(+) 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 1d724495f..4f560ade7 100644 --- a/backend/src/ee/services/pki-acme/pki-acme-service.ts +++ b/backend/src/ee/services/pki-acme/pki-acme-service.ts @@ -498,6 +498,7 @@ export const pkiAcmeServiceFactory = ({ }); } + // TODO: handle unique constraint violation error, should be very very rare const newAccount = await acmeAccountDAL.create({ profileId: profile.id, alg,