Closes the #2 acquisition-readiness blocker from the 2026-05-01 issuer
coverage audit. New() at ejbca.go:L79-L88 previously constructed an
http.Client with only Timeout set — no Transport, no TLSClientConfig.
When AuthMode=mtls (the default), the client never presented the
configured ClientCert/ClientKey. The OAuth2 path worked; mTLS always
failed authentication. Tests passed because they injected a pre-built
*http.Client via NewWithHTTPClient, a path the production factory never
took.
This commit:
- Rewrites New() to load ClientCertPath + ClientKeyPath via
tls.LoadX509KeyPair when AuthMode=mtls, configure
*http.Transport.TLSClientConfig with MinVersion: TLS 1.2 (compatibility
floor for on-prem EJBCA installs that may predate TLS 1.3), and return
(*Connector, error). Constructs a fresh *http.Transport — does NOT
clone http.DefaultTransport, which would leak mutation across the
package boundary.
- OAuth2 mode unchanged: returns a client with no transport
customization (the Bearer header path is wired in setAuthHeaders).
- Invalid auth_mode values return (nil, error) immediately rather than
falling through to the mtls default and erroring at cert load.
- Updates the factory call site at issuerfactory/factory.go for the
new signature; the factory's outer (issuer.Connector, error) shape
was already in place.
- Adds TestNew_MTLSWiresClientCert: calls production New() (NOT
NewWithHTTPClient) with real cert/key files generated via stdlib
crypto/x509, asserts httpClient.Transport.TLSClientConfig.Certificates
is non-empty. Includes an httptest TLS server with
ClientAuth: tls.RequireAndVerifyClientCert that proves the cert is
actually presented on the wire — not just stashed in a struct field.
- Adds TestNew_MTLSCertLoadFailure: missing-cert path returns an error
wrapping fs.ErrNotExist (verified via errors.Is).
- Adds TestNew_OAuth2NoTransportTuning: OAuth2 path leaves Transport
nil, ensuring no accidental mTLS bleedthrough.
- Adds TestNew_InvalidAuthMode: explicit guard that auth_mode values
other than "mtls"/"oauth2" return (nil, error) at New() time.
- Adds export_test.go with HTTPClientForTest helper so the external
ejbca_test package can inspect the connector's internal *http.Client
for the wiring assertions. Compile-only during `go test`; production
builds don't expose it.
- Adds mustNewForValidateConfig test helper (OAuth2 placeholder
connector) for the existing ValidateConfig-only tests; pre-fix they
used New(nil, ...) which is no longer valid because nil config falls
into the mTLS default branch that requires non-nil cert paths.
- Updates ejbca_stubs_test.go (internal package) for the new
(*Connector, error) signature; switches the dummy connector to
OAuth2 mode so Config{} doesn't error at New().
Out of scope (separate follow-ups, per the prompt's explicit fence):
- OAuth2 token refresh missing
- Config.Token plaintext at runtime (needs SecretRef abstraction)
- RevokeCertificate composite OrderID parsing (the issuerDN := "" line
at ejbca.go:L313)
Verified locally:
- gofmt clean
- go vet ./... clean
- staticcheck ./... clean
- golangci-lint run --timeout 5m ./... → 0 issues
- go test -short -count=1 ./internal/connector/issuer/ejbca/ green
- go test -short -count=1 ./internal/connector/issuerfactory/ green
- go test -short -count=1 ./internal/service/ green
- go build ./... success
Audit reference: cowork/issuer-coverage-audit-2026-05-01/RESULTS.md
Top-10 fix#2.
shankar0123