Skip to content

feature: add a new option on PKI ACME enrollment method to skip EAB validation#5227

Merged
carlosmonastyrski merged 3 commits intomainfrom
feat/PKI-88
Jan 28, 2026
Merged

feature: add a new option on PKI ACME enrollment method to skip EAB validation#5227
carlosmonastyrski merged 3 commits intomainfrom
feat/PKI-88

Conversation

@carlosmonastyrski
Copy link
Contributor

Context

This PR introduces a new flag on ACME enrollment profiles that allows EAB validation to be skipped. When enabled, all certificate requests made against that ACME profile will be automatically issued without performing External Account Binding checks.

For security reasons, skipping EAB validation cannot be combined with skipping domain validation. Allowing both would introduce significant risk and potential vulnerabilities, so this configuration is explicitly disallowed.

Screenshots

Steps to verify the change

Type

  • Fix
  • Feature
  • Improvement
  • Breaking
  • Docs
  • Chore

Checklist

  • Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
  • Tested locally
  • Updated docs (if needed)
  • Read the contributing guide

@maidul98
Copy link
Collaborator

maidul98 commented Jan 21, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 21, 2026

Greptile Summary

  • Adds a new skipEabBinding flag to PKI ACME enrollment profiles that allows External Account Binding (EAB) validation to be bypassed for certificate requests
  • Implements security validation to prevent both EAB and DNS ownership verification from being skipped simultaneously, ensuring at least one validation mechanism remains active
  • Updates database schema, TypeScript types, UI forms, API endpoints, and ACME service logic to support the new configuration option with proper defaults and backward compatibility

Important Files Changed

Filename Overview
backend/src/db/migrations/20260121192801_add-skip-eab-binding-to-acme-config.ts New migration adds skipEabBinding column to PkiAcmeEnrollmentConfig table with proper idempotent checks
backend/src/services/certificate-profile/certificate-profile-service.ts Critical security validation logic prevents both EAB and DNS verification from being disabled simultaneously
backend/src/ee/services/pki-acme/pki-acme-service.ts Core ACME service logic modified to conditionally skip EAB validation based on the new flag
frontend/src/pages/cert-manager/PoliciesPage/components/CertificateProfilesTab/CreateProfileModal.tsx UI form validation and disabled state management for mutually exclusive skip options
backend/src/services/certificate-profile/certificate-profile-schemas.ts Missing schema-level validation for mutual exclusion constraint mentioned in PR description

Confidence score: 3/5

  • This PR introduces a security-sensitive feature with proper validation in most places, but has a critical gap in schema-level validation
  • Score lowered due to missing mutual exclusion constraint in Zod schemas, inconsistent validation coverage, and potential security risk if business logic validation is bypassed
  • Pay close attention to certificate-profile-schemas.ts which lacks the security constraint validation present in the service layer

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments (1)

  1. backend/src/server/routes/v1/certificate-profiles-router.ts, line 479-496 (link)

    logic: Missing validation in update route for the EAB+DNS safety check that exists in create route

11 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

@carlosmonastyrski carlosmonastyrski merged commit 26624c0 into main Jan 28, 2026
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants