Feature/dns plugins - #22
Open
bhillkeyfactor wants to merge 52 commits into
Open
bhillkeyfactor wants to merge 52 commits into
bhillkeyfactor wants to merge 52 commits into
Conversation
…cglobal-caplugin into feature/dns-plugins
CSC DCV requires a CNAME record, so resolve a DNS provider advertising the 'cname' validation type (e.g. GoDaddyCnameDomainValidator) rather than the ACME 'dns-01'/TXT variant. Docs updated to call out the CNAME validator and warn against selecting the TXT validator for CSC domains.
Adds the standard Enabled boolean field to the CA Connection settings, matching the pattern used by every other Keyfactor CA plugin (SSL Store, Digicert, HydrantId, GCP CAS, Idnomic, etc.). Purpose: allow ops to create the CA record before all API credentials are available. When Enabled=false, the plugin short-circuits: - Initialize: skips CscGlobalClient construction (no valid creds needed) - Ping: no-op, logs a warning - ValidateCAConnectionInfo / ValidateProductInfo: skip validation - Synchronize: completes the buffer immediately - Enroll: returns FAILED with a clear message - Revoke: throws InvalidOperationException with a clear message Default is true so existing deployments that don't set the key continue to function without change. Reads the value from incoming connectionInfo in the Validate* methods so an operator editing the CA sees consistent behavior with the current form state.
* Add custom field support * changelog * support cname return from enrollment * Update generated docs * feat: release 1.1.1 * Update generated docs * Fix for issues with * Test * Added template parameter configuration via REST gateway. Fixed bug with email used for verification. Changed docs and enrollment field/template parameter names. See changelog. * Update generated docs * Fixed broken logging. * Incremental sync support added using csc date filter so sync timing can run faster that default full sync periods * Update generated docs * Fixes for Incremental Sync * Update CHANGELOG.md --------- Co-authored-by: Mikey Henderson <4452096+fiddlermikey@users.noreply.github.com> Co-authored-by: Sean <1661003+spbsoluble@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> Co-authored-by: Brian Hill <bhill@keyfactor.com> Co-authored-by: Brian Hill <76450501+bhillkeyfactor@users.noreply.github.com> * Fix NullReferenceException in GetEnrollmentResult for null DCV email (#9) * Fix NullReferenceException in GetEnrollmentResult for null DCV email The condition for adding DCV email entries to the cnames dictionary was inverted (string.IsNullOrEmpty instead of !string.IsNullOrEmpty), causing cnames.Add(null, null) and an ArgumentNullException on every enrollment where CSC returned a DcvDetail with email=null (typical for EMAIL DCV orders that have actionNeeded=N, and for CNAME-only DCV). Inverts the condition and adds a ContainsKey guard to mirror the existing CName branch. * Update generated docs --------- Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> * Feature/dns plugins (#12) * 200 day renewal fixes * Update generated docs * Improved logging .net 10 support * Removed Template Sync Logic * Update generated docs * fixed template mapping issue * product fixes * Update generated docs * fixed renewal issue * documentation fixes * Update generated docs * DNS Changes * Update generated docs * dns code updates * Update generated docs * change type * Update generated docs * fixed mismatch * Use 'cname' validation type for CSC CNAME DCV CSC DCV requires a CNAME record, so resolve a DNS provider advertising the 'cname' validation type (e.g. GoDaddyCnameDomainValidator) rather than the ACME 'dns-01'/TXT variant. Docs updated to call out the CNAME validator and warn against selecting the TXT validator for CSC domains. * Update generated docs * added polling to grab cert * Update generated docs * Update integration-manifest.json * Update CSCGlobalCAPlugin.csproj * Update CSCGlobalCAPlugin.csproj * Update keyfactor-bootstrap-workflow-v3.yml * docs: auto-generate README and documentation [skip ci] * Add Enabled CA connection flag Adds the standard Enabled boolean field to the CA Connection settings, matching the pattern used by every other Keyfactor CA plugin (SSL Store, Digicert, HydrantId, GCP CAS, Idnomic, etc.). Purpose: allow ops to create the CA record before all API credentials are available. When Enabled=false, the plugin short-circuits: - Initialize: skips CscGlobalClient construction (no valid creds needed) - Ping: no-op, logs a warning - ValidateCAConnectionInfo / ValidateProductInfo: skip validation - Synchronize: completes the buffer immediately - Enroll: returns FAILED with a clear message - Revoke: throws InvalidOperationException with a clear message Default is true so existing deployments that don't set the key continue to function without change. Reads the value from incoming connectionInfo in the Validate* methods so an operator editing the CA sees consistent behavior with the current form state. * docs: auto-generate README and documentation [skip ci] --------- Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Update integration-manifest.json (#14) --------- Co-authored-by: David Galey <dgaley@keyfactor.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> Co-authored-by: Mark Kachkaev <37276742+mkachk@users.noreply.github.com> Co-authored-by: Mikey Henderson <4452096+fiddlermikey@users.noreply.github.com> Co-authored-by: Sean <1661003+spbsoluble@users.noreply.github.com> Co-authored-by: Brian Hill <bhill@keyfactor.com> Co-authored-by: Brian Hill <76450501+bhillkeyfactor@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Rolls main's content back to the 1.1.2 release; the 2.0.0 work remains available on the release-2.0 branch and the 2.0.0 tag.
* Add custom field support * changelog * support cname return from enrollment * Update generated docs * feat: release 1.1.1 * Update generated docs * Fix for issues with * Test * Added template parameter configuration via REST gateway. Fixed bug with email used for verification. Changed docs and enrollment field/template parameter names. See changelog. * Update generated docs * Fixed broken logging. * Incremental sync support added using csc date filter so sync timing can run faster that default full sync periods * Update generated docs * Fixes for Incremental Sync * Update CHANGELOG.md --------- Co-authored-by: Mikey Henderson <4452096+fiddlermikey@users.noreply.github.com> Co-authored-by: Sean <1661003+spbsoluble@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> Co-authored-by: Brian Hill <bhill@keyfactor.com> Co-authored-by: Brian Hill <76450501+bhillkeyfactor@users.noreply.github.com> * Fix NullReferenceException in GetEnrollmentResult for null DCV email (#9) * Fix NullReferenceException in GetEnrollmentResult for null DCV email The condition for adding DCV email entries to the cnames dictionary was inverted (string.IsNullOrEmpty instead of !string.IsNullOrEmpty), causing cnames.Add(null, null) and an ArgumentNullException on every enrollment where CSC returned a DcvDetail with email=null (typical for EMAIL DCV orders that have actionNeeded=N, and for CNAME-only DCV). Inverts the condition and adds a ContainsKey guard to mirror the existing CName branch. * Update generated docs --------- Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> * Fix/san email dictionary key (#17) * Fix KeyNotFoundException when Addtl Sans Comma Separated DVC Emails is unset Only look up the optional additional-SAN-emails field when the domain control validation method is EMAIL, and use TryGetValue instead of the indexer so a missing/blank value no longer throws. * Add changelog entries for 1.1.2 and 1.1.3 * Update keyfactor-bootstrap-workflow-v3.yml * docs: auto-generate README and documentation [skip ci] * Fix KeyNotFoundException in GetSubjectAlternativeNames when sans has no 'dnsname' key UC certificate enrollments with zero SANs supplied threw KeyNotFoundException from the sans["dnsname"] indexer. Use TryGetValue and treat a missing key as no SANs instead of throwing. * Note second dnsname KeyNotFoundException fix in 1.1.3 changelog entry --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> --------- Co-authored-by: David Galey <dgaley@keyfactor.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> Co-authored-by: Mark Kachkaev <37276742+mkachk@users.noreply.github.com> Co-authored-by: Mikey Henderson <4452096+fiddlermikey@users.noreply.github.com> Co-authored-by: Sean <1661003+spbsoluble@users.noreply.github.com> Co-authored-by: Brian Hill <bhill@keyfactor.com> Co-authored-by: Brian Hill <76450501+bhillkeyfactor@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…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.
…erts Port product rename, new cert types, and bug fixes from ev-ov-dv-multiname-certs
Resolves conflicts by keeping feature/dns-plugins' side throughout: release-2.0 only carries an older snapshot of this code (legacy-only product names, the still-present PriorCertSN case-sensitivity bug, the typo'd DCV email SAN key, and the pre-DI-seam client constructor), all of which are already fixed and superseded on this branch.
…ing table Synced DV/EV/UC/Premium certs weren't mapping to their Certificate Profile in Command because CodeToProductIdMap resolved them back to the old pre-1.2.0 legacy product names, which no longer match the canonical Template Short Names configured in Command. Follow the same fix already proven on feature/ev-ov-dv-multiname-certs: pass CSC's certificateType straight through as ProductID during sync instead of remapping it, since CSC's API already returns the current/canonical product name directly.
…erts Sync ProductID directly from CSC's certificateType, drop reverse-mapp…
…ow summary in StatusMessage Every early-return failure path in Enroll (plus the catch-all exception handler) set Status = 30 (INPROCESS) instead of FAILED, so Command treated these as still-pending rather than errors and never surfaced them in the UI. Follow the same fix already proven on feature/ev-ov-dv-multiname-certs: use EndEntityStatus.FAILED throughout, and add FlowLogger.GetSummary() (a concise step list, distinct from the ASCII-tree RenderFlow() used for Trace logs) to prepend what the plugin actually attempted ahead of the terse error text. Also fixes the reissue "one click not available" message incorrectly saying "Renew" instead of "Reissue".
…erts Fix Enroll failures reporting INPROCESS instead of FAILED, surface fl…
…essage Command's enrollment UI doesn't surface StatusMessage on a successful/pending result at all - only EnrollmentContext is shown, so the flow summary added for failures was invisible on the success path. Add AttachFlowSummary (following the same fix already proven on feature/ev-ov-dv-multiname-certs) to also attach a "Flow Summary" entry to EnrollmentContext for success/pending results, alongside whatever DCV instructions came back, and use it for the FAILED results returned from GetEnrollmentResult/GetRenewResponse/ GetReIssueResult, which the earlier Status=30 fix didn't cover since they're already FAILED coming out of RequestManager. Called after TryPublishCnameDcvAsync in the New enrollment path so the "Flow Summary" entry is never present yet when DNS auto-publish walks EnrollmentContext looking for real CNAME records to publish.
…erts Attach flow summary to EnrollmentContext on success, not just StatusM…
Matches the same fix already proven on feature/ev-ov-dv-multiname-certs: add FlowLogger.GetSummaryEntries(), which returns one dictionary entry per step (plus a header entry) instead of a single multi-line block. AttachFlowSummary now merges these into EnrollmentContext directly so Command's bulleted rendering shows a readable line per step, rather than one run-on entry with embedded newlines that don't render as separate bullets.
…erts Render flow summary as one bullet per step instead of one blob
…E methods The DcvAutoPublish step (formerly PublishCnameDcv) ran unconditionally on every New enrollment and, for EMAIL-validated certs, silently no-op'd while still showing a bare [OK] under a CNAME-sounding step name in the flow summary - confusing when read back in Command's UI. Add a FlowLogger.StepAsync overload that uses the action's own return value as the step detail (the existing overload's detail parameter is evaluated before the action runs, so it can't reflect what the action decided), and have TryPublishCnameDcvAsync return why it skipped or how many records it published.
…erts Rename PublishCnameDcv step and explain why it's a no-op for non-CNAM…
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
2 participants
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.
No description provided.