Files
certctl/internal/service/shortlived_test.go
T
shankar0123 fefa5a5fd7 acme: support serial-only revocation via local cert-version lookup
Closes the #7 acquisition-readiness blocker from the 2026-05-01 issuer
coverage audit. Pre-fix, ACME RevokeCertificate at acme.go:L519-L529
returned the literal error "ACME revocation by serial not supported in
V1; provide certificate DER". RFC 8555 §7.6 genuinely requires the
cert DER bytes (not just the serial), but a CLM platform's job is to
abstract over that limitation. Operators routinely have only the
serial in hand: lost PEM, rotated key, GUI revoke action driven by a
row in the certs list.

This commit:

- Adds CertificateLookupRepo interface at the ACME connector boundary
  (connector boundary, NOT a service/repository import — the connector
  accepts whatever satisfies the shape). Production wiring in
  cmd/server/main.go injects the postgres CertificateRepository; tests
  inject a fake.

- Adds CertificateRepository.GetVersionBySerial(ctx, issuerID, serial)
  + interface declaration in repository/interfaces.go, returning the
  certificate_versions row whose SerialNumber matches, scoped to the
  issuer via JOIN on managed_certificates. Mirrors the existing
  GetByIssuerAndSerial shape but returns the version (where PEMChain
  lives). Per RFC 5280 §5.2.3 the issuer scope is required for
  determinism.

- Adds SetCertificateLookup + SetIssuerID setters on *acme.Connector.
  Mirror the pattern local.Connector already uses for OCSP responder
  wiring. Both must be wired before serial-only revoke works;
  unwired state falls back to a more actionable error pointing at the
  wiring requirement (the historical "not supported" wording is
  retired).

- Rewrites RevokeCertificate end-to-end: lookup → empty-PEM check →
  pem.Decode → block.Type == "CERTIFICATE" check → ensureClient →
  golang.org/x/crypto/acme.Client.RevokeCert(ctx, accountKey, der,
  reasonCode). RFC 8555 §7.6 case 1 (revocation request signed with
  account key) — the same account key issued the cert, so authority
  is intrinsic. The not-found path returns an actionable operator-
  facing error pointing at the local-store requirement.

- Adds mapRevocationReason translating RFC 5280 §5.3.1 reason strings
  (unspecified, keyCompromise, cACompromise, affiliationChanged,
  superseded, cessationOfOperation, certificateHold, removeFromCRL,
  privilegeWithdrawn, aACompromise) into golang.org/x/crypto/acme.
  CRLReasonCode. Accepts canonical camelCase + underscore_lower +
  ALL_CAPS_UNDERSCORE. Nil reason → 0 (unspecified). Unknown reason
  errors rather than silently demoting (operators rely on the reason
  for compliance reporting).

- Wiring update in service/issuer_registry.go: SetACMECertLookup
  setter on the registry; Rebuild type-asserts *acme.Connector and
  calls SetCertificateLookup + SetIssuerID, mirroring the existing
  *local.Connector branch. cmd/server/main.go calls
  issuerRegistry.SetACMECertLookup(certificateRepo) immediately after
  SetIssuanceMetrics — the postgres repo satisfies the interface via
  GetVersionBySerial.

- Tests:
  * acme_revoke_test.go (new): TestRevokeCertificate_NoCertLookupWired,
    TestRevokeCertificate_NoIssuerIDWired,
    TestRevokeCertificate_LookupReturnsNotFound (operator-facing
    "may not have been issued through certctl" hint pinned),
    TestRevokeCertificate_LookupArbitraryError,
    TestRevokeCertificate_VersionPEMEmpty (corrupt-row guard),
    TestRevokeCertificate_PEMMalformed_NoBlock,
    TestRevokeCertificate_PEMMalformed_WrongType (PRIVATE KEY block
    rejected as not a CERTIFICATE).
  * TestMapRevocationReason_TableDriven: full RFC 5280 reason set
    plus camelCase / underscore / ALL-CAPS variants plus
    nil-reason and unknown-reason cases.
  * acme_failure_test.go: renamed TestRevokeCertificate_AlwaysError
    → TestRevokeCertificate_UnwiredCertLookupFallback; the test
    still exercises the same backward-compat branch but now
    asserts the new "CertificateLookup wiring" error wording.

- Mock-repo updates (3 sites): mockCertificateRepository in
  internal/integration/lifecycle_test.go, mockCertRepo in
  internal/service/testutil_test.go, mockCertRepoWithGetError in
  internal/service/shortlived_test.go each gain a GetVersionBySerial
  implementation that mirrors the GetByIssuerAndSerial logic but
  returns the version row.

- docs/connectors.md ACME section: new "Revocation by serial number"
  subsection covering the workflow, the local-store requirement
  (cert was issued through certctl, not imported), the reason-code
  mapping with the three accepted spelling variants, and a pointer
  to the audit reference.

Out of scope (intentional, per spec):

- Recovering the DER from outside the local cert store (CT logs,
  CSR + signature reconstruction). If the cert wasn't issued through
  certctl, revoke-by-serial via certctl isn't possible.
- Revocation via the cert's private key (RFC 8555 §7.6 case 2). The
  account-key path covers all certctl-issued certs because the same
  account key issued them.
- Pebble-backed integration test for the happy path. Pebble integration
  is the right home for that — the unit tests in this commit pin all
  failure-mode branches before the network call, and the wiring
  branch in Rebuild is exercised by the existing
  TestIssuerRegistryRebuild paths.

Verified locally:
- gofmt -l . clean
- go vet ./... clean
- staticcheck ./... clean
- go test -short -count=1 across connector, service, repository,
  integration, api/middleware, api/handler: green

Audit reference: cowork/issuer-coverage-audit-2026-05-01/RESULTS.md
Top-10 fix #7.
2026-05-02 13:09:30 +00:00

427 lines
14 KiB
Go

package service
import (
"context"
"errors"
"log/slog"
"testing"
"time"
"github.com/shankar0123/certctl/internal/domain"
"github.com/shankar0123/certctl/internal/repository"
)
// setupShortLivedTestService creates a RenewalService with mock dependencies for short-lived cert tests
func setupShortLivedTestService(
certRepo *mockCertRepo,
profileRepo *mockProfileRepo,
auditRepo *mockAuditRepo,
) *RenewalService {
auditSvc := NewAuditService(auditRepo)
issuerRegistry := NewIssuerRegistry(slog.Default())
issuerRegistry.Set("iss-test", &mockIssuerConnector{})
svc := NewRenewalService(
certRepo,
newMockJobRepository(),
newMockRenewalPolicyRepository(),
profileRepo,
auditSvc,
NewNotificationService(newMockNotificationRepository(), map[string]Notifier{}),
issuerRegistry,
"agent",
)
return svc
}
// TestExpireShortLivedCertificates_Success verifies that active certificates with
// expired short-lived profiles are transitioned to Expired status
func TestExpireShortLivedCertificates_Success(t *testing.T) {
ctx := context.Background()
now := time.Now()
certRepo := newMockCertificateRepository()
profileRepo := newMockProfileRepository()
auditRepo := newMockAuditRepository()
// Create a short-lived profile (TTL < 1 hour = 3600 seconds)
shortLivedProfile := &domain.CertificateProfile{
ID: "prof-short",
Name: "Short-Lived",
MaxTTLSeconds: 300, // 5 minutes
AllowShortLived: true,
Enabled: true,
AllowedKeyAlgorithms: domain.DefaultKeyAlgorithms(),
AllowedEKUs: domain.DefaultEKUs(),
CreatedAt: now,
UpdatedAt: now,
}
profileRepo.AddProfile(shortLivedProfile)
// Create an active certificate that has already expired
expiredCert := &domain.ManagedCertificate{
ID: "mc-expired-short",
Name: "Expired Short-Lived Cert",
CommonName: "short.example.com",
SANs: []string{},
IssuerID: "iss-test",
CertificateProfileID: "prof-short",
Status: domain.CertificateStatusActive,
ExpiresAt: now.Add(-5 * time.Minute), // Already expired
CreatedAt: now.Add(-15 * time.Minute),
UpdatedAt: now.Add(-5 * time.Minute),
Tags: make(map[string]string),
}
certRepo.AddCert(expiredCert)
svc := setupShortLivedTestService(certRepo, profileRepo, auditRepo)
// Run the expiry check
err := svc.ExpireShortLivedCertificates(ctx)
if err != nil {
t.Fatalf("ExpireShortLivedCertificates failed: %v", err)
}
// Verify the cert status was updated to Expired
updated, err := certRepo.Get(ctx, "mc-expired-short")
if err != nil {
t.Fatalf("failed to get updated cert: %v", err)
}
if updated.Status != domain.CertificateStatusExpired {
t.Errorf("expected cert status to be Expired, got %s", updated.Status)
}
// Verify an audit event was recorded
if len(auditRepo.Events) == 0 {
t.Errorf("expected audit event to be recorded, got none")
}
}
// TestExpireShortLivedCertificates_NoCertsToExpire verifies the function handles
// empty certificate lists gracefully
func TestExpireShortLivedCertificates_NoCertsToExpire(t *testing.T) {
ctx := context.Background()
certRepo := newMockCertificateRepository()
profileRepo := newMockProfileRepository()
auditRepo := newMockAuditRepository()
svc := setupShortLivedTestService(certRepo, profileRepo, auditRepo)
// Run the expiry check on empty certificate list
err := svc.ExpireShortLivedCertificates(ctx)
if err != nil {
t.Fatalf("ExpireShortLivedCertificates failed: %v", err)
}
// Verify no audit events were recorded
if len(auditRepo.Events) != 0 {
t.Errorf("expected no audit events, got %d", len(auditRepo.Events))
}
}
// TestExpireShortLivedCertificates_ListError verifies that repository errors
// are properly propagated
func TestExpireShortLivedCertificates_ListError(t *testing.T) {
ctx := context.Background()
// Create a custom mock that returns an error from GetExpiringCertificates
customCertRepo := &mockCertRepoWithGetError{
GetExpiringCertificatesErr: errors.New("database connection failed"),
}
profileRepo := newMockProfileRepository()
auditRepo := newMockAuditRepository()
// Create the service manually to use our custom cert repo
auditSvc := NewAuditService(auditRepo)
issuerRegistry := NewIssuerRegistry(slog.Default())
issuerRegistry.Set("iss-test", &mockIssuerConnector{})
svc := NewRenewalService(
customCertRepo,
newMockJobRepository(),
newMockRenewalPolicyRepository(),
profileRepo,
auditSvc,
NewNotificationService(newMockNotificationRepository(), map[string]Notifier{}),
issuerRegistry,
"agent",
)
// Run the expiry check, expecting an error
err := svc.ExpireShortLivedCertificates(ctx)
if err == nil {
t.Fatalf("expected ExpireShortLivedCertificates to return an error, got nil")
}
if !errors.Is(err, customCertRepo.GetExpiringCertificatesErr) {
t.Errorf("expected error containing 'database connection failed', got %v", err)
}
}
// mockCertRepoWithGetError is a minimal custom mock for testing GetExpiringCertificates error handling
type mockCertRepoWithGetError struct {
GetExpiringCertificatesErr error
}
func (m *mockCertRepoWithGetError) List(ctx context.Context, filter *repository.CertificateFilter) ([]*domain.ManagedCertificate, int, error) {
return nil, 0, nil
}
func (m *mockCertRepoWithGetError) Get(ctx context.Context, id string) (*domain.ManagedCertificate, error) {
return nil, nil
}
func (m *mockCertRepoWithGetError) Create(ctx context.Context, cert *domain.ManagedCertificate) error {
return nil
}
func (m *mockCertRepoWithGetError) CreateWithTx(ctx context.Context, q repository.Querier, cert *domain.ManagedCertificate) error {
return nil
}
func (m *mockCertRepoWithGetError) Update(ctx context.Context, cert *domain.ManagedCertificate) error {
return nil
}
func (m *mockCertRepoWithGetError) UpdateWithTx(ctx context.Context, q repository.Querier, cert *domain.ManagedCertificate) error {
return nil
}
func (m *mockCertRepoWithGetError) Archive(ctx context.Context, id string) error {
return nil
}
func (m *mockCertRepoWithGetError) ListVersions(ctx context.Context, certID string) ([]*domain.CertificateVersion, error) {
return nil, nil
}
func (m *mockCertRepoWithGetError) CreateVersion(ctx context.Context, version *domain.CertificateVersion) error {
return nil
}
func (m *mockCertRepoWithGetError) CreateVersionWithTx(ctx context.Context, q repository.Querier, version *domain.CertificateVersion) error {
return nil
}
func (m *mockCertRepoWithGetError) GetLatestVersion(ctx context.Context, certID string) (*domain.CertificateVersion, error) {
return nil, nil
}
func (m *mockCertRepoWithGetError) GetByIssuerAndSerial(ctx context.Context, issuerID, serial string) (*domain.ManagedCertificate, error) {
return nil, nil
}
func (m *mockCertRepoWithGetError) GetVersionBySerial(ctx context.Context, issuerID, serial string) (*domain.CertificateVersion, error) {
return nil, nil
}
func (m *mockCertRepoWithGetError) GetExpiringCertificates(ctx context.Context, before time.Time) ([]*domain.ManagedCertificate, error) {
return nil, m.GetExpiringCertificatesErr
}
// TestExpireShortLivedCertificates_PartialUpdateError verifies that update errors
// on individual certs are logged but don't fail the entire operation
func TestExpireShortLivedCertificates_PartialUpdateError(t *testing.T) {
ctx := context.Background()
now := time.Now()
certRepo := newMockCertificateRepository()
profileRepo := newMockProfileRepository()
auditRepo := newMockAuditRepository()
// Create a short-lived profile
shortLivedProfile := &domain.CertificateProfile{
ID: "prof-short",
Name: "Short-Lived",
MaxTTLSeconds: 300,
AllowShortLived: true,
Enabled: true,
AllowedKeyAlgorithms: domain.DefaultKeyAlgorithms(),
AllowedEKUs: domain.DefaultEKUs(),
CreatedAt: now,
UpdatedAt: now,
}
profileRepo.AddProfile(shortLivedProfile)
// Create a certificate with a failing update
expiredCert := &domain.ManagedCertificate{
ID: "mc-expired-fail",
Name: "Expired Cert That Will Fail",
CommonName: "fail.example.com",
SANs: []string{},
IssuerID: "iss-test",
CertificateProfileID: "prof-short",
Status: domain.CertificateStatusActive,
ExpiresAt: now.Add(-5 * time.Minute),
CreatedAt: now.Add(-15 * time.Minute),
UpdatedAt: now.Add(-5 * time.Minute),
Tags: make(map[string]string),
}
certRepo.AddCert(expiredCert)
// Set up the repo to fail on update
certRepo.UpdateErr = errors.New("update failed")
svc := setupShortLivedTestService(certRepo, profileRepo, auditRepo)
// Run the expiry check - should not return an error even though update failed
err := svc.ExpireShortLivedCertificates(ctx)
if err != nil {
t.Fatalf("ExpireShortLivedCertificates should not fail on partial update errors, got %v", err)
}
// Verify no audit events were recorded (update failure skips audit recording)
if len(auditRepo.Events) != 0 {
t.Errorf("expected no audit events on update failure, got %d", len(auditRepo.Events))
}
}
// TestExpireShortLivedCertificates_AlreadyExpired verifies that certificates
// already in Expired status are not re-processed
func TestExpireShortLivedCertificates_AlreadyExpired(t *testing.T) {
ctx := context.Background()
now := time.Now()
certRepo := newMockCertificateRepository()
profileRepo := newMockProfileRepository()
auditRepo := newMockAuditRepository()
// Create a short-lived profile
shortLivedProfile := &domain.CertificateProfile{
ID: "prof-short",
Name: "Short-Lived",
MaxTTLSeconds: 300,
AllowShortLived: true,
Enabled: true,
AllowedKeyAlgorithms: domain.DefaultKeyAlgorithms(),
AllowedEKUs: domain.DefaultEKUs(),
CreatedAt: now,
UpdatedAt: now,
}
profileRepo.AddProfile(shortLivedProfile)
// Create a certificate that's already in Expired status
alreadyExpiredCert := &domain.ManagedCertificate{
ID: "mc-already-expired",
Name: "Already Expired Cert",
CommonName: "already-expired.example.com",
SANs: []string{},
IssuerID: "iss-test",
CertificateProfileID: "prof-short",
Status: domain.CertificateStatusExpired, // Already expired
ExpiresAt: now.Add(-30 * time.Minute),
CreatedAt: now.Add(-45 * time.Minute),
UpdatedAt: now.Add(-10 * time.Minute),
Tags: make(map[string]string),
}
certRepo.AddCert(alreadyExpiredCert)
svc := setupShortLivedTestService(certRepo, profileRepo, auditRepo)
// Run the expiry check
err := svc.ExpireShortLivedCertificates(ctx)
if err != nil {
t.Fatalf("ExpireShortLivedCertificates failed: %v", err)
}
// Verify no new audit events were recorded (cert was skipped)
if len(auditRepo.Events) != 0 {
t.Errorf("expected no audit events for already-expired cert, got %d", len(auditRepo.Events))
}
}
// TestExpireShortLivedCertificates_ProfileNotShortLived verifies that certificates
// with non-short-lived profiles are not expired by this function
func TestExpireShortLivedCertificates_ProfileNotShortLived(t *testing.T) {
ctx := context.Background()
now := time.Now()
certRepo := newMockCertificateRepository()
profileRepo := newMockProfileRepository()
auditRepo := newMockAuditRepository()
// Create a regular (not short-lived) profile with TTL > 1 hour
regularProfile := &domain.CertificateProfile{
ID: "prof-regular",
Name: "Regular",
MaxTTLSeconds: 86400, // 24 hours
AllowShortLived: false,
Enabled: true,
AllowedKeyAlgorithms: domain.DefaultKeyAlgorithms(),
AllowedEKUs: domain.DefaultEKUs(),
CreatedAt: now,
UpdatedAt: now,
}
profileRepo.AddProfile(regularProfile)
// Create an expired certificate with the regular profile
expiredCert := &domain.ManagedCertificate{
ID: "mc-expired-regular",
Name: "Expired Regular Cert",
CommonName: "regular.example.com",
SANs: []string{},
IssuerID: "iss-test",
CertificateProfileID: "prof-regular",
Status: domain.CertificateStatusActive,
ExpiresAt: now.Add(-1 * time.Hour),
CreatedAt: now.Add(-25 * time.Hour),
UpdatedAt: now.Add(-1 * time.Hour),
Tags: make(map[string]string),
}
certRepo.AddCert(expiredCert)
svc := setupShortLivedTestService(certRepo, profileRepo, auditRepo)
// Run the expiry check
err := svc.ExpireShortLivedCertificates(ctx)
if err != nil {
t.Fatalf("ExpireShortLivedCertificates failed: %v", err)
}
// Verify the cert status was NOT changed (because profile is not short-lived)
cert, _ := certRepo.Get(ctx, "mc-expired-regular")
if cert.Status != domain.CertificateStatusActive {
t.Errorf("cert should not have been expired (profile not short-lived), got status %s", cert.Status)
}
// Verify no audit events were recorded
if len(auditRepo.Events) != 0 {
t.Errorf("expected no audit events for non-short-lived profile, got %d", len(auditRepo.Events))
}
}
// TestExpireShortLivedCertificates_NoProfileRepository verifies the function
// handles nil profileRepo gracefully
func TestExpireShortLivedCertificates_NoProfileRepository(t *testing.T) {
ctx := context.Background()
certRepo := newMockCertificateRepository()
auditRepo := &mockAuditRepo{
Events: make([]*domain.AuditEvent, 0),
}
auditSvc := NewAuditService(auditRepo)
issuerRegistry := NewIssuerRegistry(slog.Default())
issuerRegistry.Set("iss-test", &mockIssuerConnector{})
svc := NewRenewalService(
certRepo,
newMockJobRepository(),
newMockRenewalPolicyRepository(),
nil, // nil profileRepo
auditSvc,
NewNotificationService(newMockNotificationRepository(), map[string]Notifier{}),
issuerRegistry,
"agent",
)
// Run the expiry check with nil profileRepo
err := svc.ExpireShortLivedCertificates(ctx)
if err != nil {
t.Fatalf("ExpireShortLivedCertificates should handle nil profileRepo gracefully, got error: %v", err)
}
}