Repository navigation
Port product rename, new cert types, and bug fixes from ev-ov-dv-multiname-certs - #20
Merged
bhillkeyfactor merged 4 commits intoSep 21, 2026
Conversation
…e/ev-ov-dv-multiname-certs
Ports the following from feature/ev-ov-dv-multiname-certs, adapted to
this branch's existing patterns (ProductIdToCodeMap/CodeToProductIdMap,
structured logging style) rather than overwriting them:
- Renamed all certificate product IDs to CSC's current certificate
type names, with pre-1.2.0 legacy names still accepted (added as
additional entries in the existing product-id maps, not a separate
alias layer) so existing Certificate Templates in Command keep
working.
- Added the 3 new certificate products: CSC TrustedSecure EV, Multiple
Names; OV Wildcard, Multiple Names; DV Wildcard, Multiple Names
(types 7/8/9).
- Replaced the hardcoded certificateType == "2"/"3" checks with
MultiNameCertificateTypes/EvCertificateTypes sets covering all
applicable types.
- ValidateProductInfo now calls RequestManager.IsKnownProductId
instead of checking a separate list, so accepted names can't drift
out of sync with what GetCertificateType actually resolves.
- Fixed the "Addtl Sans Comma Separated DCV Emails" field never being
read (typo'd lookup key: "DVC" instead of "DCV"), and added a
fallback to the primary CN's DCV email when no per-domain SAN email
override matches (CSC Global rejects requests with a SAN missing
domainControlValidation).
- Fixed a case-sensitivity bug ("priorcertsn" vs "PriorCertSN") that
silently prevented PriorCertSN from ever being read during
Renew/Reissue.
- Made Price.Total nullable to fix a JSON deserialization crash when
CSC Global returns "price.total": null.
- Updated integration-manifest.json and docsource/configuration.md to
match.
Explicitly NOT ported (per discussion - these don't fit this branch):
- FlowLogger changes/redesign - this branch's FlowLogger has an
incompatible tree-based design already wired into DNS-01 CNAME
auto-publish call sites; left untouched.
- The EnrollmentContext "Flow Summary" UX feature - this branch's
TryPublishCnameDcvAsync treats EnrollmentContext entries as real DNS
records to auto-publish; adding non-DNS entries there would be
actively harmful.
- .NET 6/8 multi-targeting - this branch already moved to net10.0-only
with newer package versions; not reintroducing the older targets.
- The Renew/Reissue GENERATED-vs-EXTERNALVALIDATION fix - already
present independently on this branch.
Also adds a new xUnit test project (this branch had none), with fresh
tests written against this branch's actual code shape rather than
adapted from the other branch's now-incompatible test suite: 40 tests
covering certificate type/SAN/EV routing for all 10 canonical + 7
legacy product names, IsKnownProductId, the DCV email fallback fix,
Price.Total null deserialization, and Renew/Reissue status codes.
Adds testability seams (matching the pattern already used for HTTP
mocking in feature/ev-ov-dv-multiname-certs):
- CSCGlobalCAPlugin.CscGlobalClient property is now internal instead
of private, so tests can inject a mock ICscGlobalClient.
- CscGlobalClient gets an internal HttpMessageHandler-accepting
constructor overload, so tests can supply a fake handler instead of
making real HTTP calls.
- AssemblyInfo.cs adds InternalsVisibleTo("CSCGlobalCAPlugin.Tests").
Expands the test suite from 40 to 233 tests, covering:
- RequestManager: all certificate-type/SAN/EV routing (canonical +
legacy names), IsKnownProductId, DCV email fallback, GetRevokeResult,
MapReturnStatus/MapCertificateTypeToProductId, custom fields,
Price.Total null deserialization.
- CscGlobalClient: all 7 API methods (success/400/error paths) via a
fake HttpMessageHandler, plus constructor validation.
- FlowLogger: Step/StepAsync/Fail/Skip/Branch/Dispose.
- CSCGlobalCAPlugin: Initialize, GetSingleRecord, Synchronize/
SyncCertificates, Revoke, Ping, ValidateCAConnectionInfo/
ValidateProductInfo, Enroll (New and RenewOrReissue - renewal vs.
reissue decision, DNS-01 CNAME auto-publish via IDomainValidatorFactory,
synchronous issuance polling via DcvPollTimeoutSeconds, and the
various failure branches), and the PEM/leaf-certificate parsing
helpers (GetEndEntityCertificate/ExtractCertificates/FindLeaf).
Line coverage: 15.57% -> 95.62%. Remaining gaps are dead code
(ExportCollectionToPem, RetryCountExceededException - both already
unused before this change) and a few defensive catch blocks for
exception types (AggregateException, arbitrary X509 export failures)
that aren't reachable through normal async/mock-based testing.
Added targeted tests for previously-uncovered conditional branches across RequestManager, CSCGlobalCAPlugin, and CscGlobalClient - optional-field mapping in the registration/renewal/reissue request builders, null-valued config keys, null-object constructor inputs, and a few edge cases in the renewal/reissue decision and DNS auto-publish paths. Left purely diagnostic logging ternaries and practically-unreachable defensive branches alone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports the product rename + new certificate types + several bug fixes from
feature/ev-ov-dv-multiname-certs, adapted to this branch's existing patterns rather than overwritten:CSC TrustedSecure EV, Multiple Names;OV Wildcard, Multiple Names;DV Wildcard, Multiple Names(types 7/8/9).ValidateProductInfonow uses a single source of truth (RequestManager.IsKnownProductId) instead of a separate list that could drift.Addtl Sans Comma Separated DCV Emailsfield never being read (typo'd lookup key), with a fallback to the primary CN's DCV email when no per-domain override matches.priorcertsnvsPriorCertSN) that silently broke Renew/Reissue.Price.Totalnullable to fix a deserialization crash when CSC returns"price.total": null.Explicitly NOT ported (see commit message for full reasoning):
EnrollmentContext"Flow Summary" UX feature - this branch'sTryPublishCnameDcvAsynctreatsEnrollmentContextentries as real DNS records to auto-publish; adding non-DNS text entries there would be actively harmful.Test plan
dotnet build cscglobal-caplugin.sln -c Release- 0 errorsdotnet test- 40/40 passing