Conversation
* added logging and update .net10 * Update generated docs * fixed package vulns * updated documentation * Update generated docs --------- Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io>
Idnomic's revoke SOAP API expects the serial in canonical form (no leading zeros, lowercase hex). X509Certificate2.SerialNumber returns padded uppercase hex (e.g. "05" for a 1-byte serial), which the API rejects. Normalize to "5" before submitting, with a guard so an all-zero serial doesn't trim to an empty string. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This automated merge PR rolls release-2.0 changes into main, primarily adding an IssuerDnFilter configuration option to scope certificate synchronization per Logical CA in multi-CA Idnomic environments, along with substantial logging/diagnostics refactoring and an added net10.0 target.
Changes:
- Added
IssuerDnFilterconfig/manifest/docs support and applied it during certificate sync/download operations. - Refactored plugin/client code for structured logging, flow-step tracing (new
FlowLogger), and stronger input/error handling. - Updated build targets to include
net10.0and adjusted certificate loading/TLS handling via conditional compilation.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents Issuer DN filtering and adds a new manual test case for multi-CA scoping. |
| integration-manifest.json | Exposes IssuerDnFilter in the integration manifest parameters list. |
| Idnomic/IdomicCAPlugin.cs | Adds flow logging, input validation, and passes IssuerDnFilter into client creation. |
| Idnomic/IdnomicPluginConfig.cs | Adds IssuerDnFilter to connection config constants, model, and annotations. |
| Idnomic/IdnomicClient.cs | Implements issuer DN filtering, expands diagnostics, adds conditional net10 cert/TLS handling. |
| Idnomic/Idnomic.csproj | Adds net10.0 target and new package references. |
| Idnomic/FlowLogger.cs | Introduces a flow/step tracing helper that renders trace-level ASCII diagrams. |
| docsource/idnomic.md | Mirrors README updates for IssuerDnFilter and the new test case. |
| docsource/configuration.md | Mirrors configuration/test-case documentation for IssuerDnFilter. |
| CHANGELOG.md | Adds a 2.x entry describing new filter, diagnostics, error handling, and net10 targeting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+143
to
+151
| // Ignore server-side certificate validation entirely (hostname + trust) | ||
| _soapClient.ClientCredentials.ServiceCertificate.SslCertificateAuthentication = | ||
| new System.ServiceModel.Security.X509ServiceCertificateAuthentication | ||
| { | ||
| CertificateValidationMode = System.ServiceModel.Security.X509CertificateValidationMode.None, | ||
| RevocationMode = X509RevocationMode.NoCheck | ||
| }; | ||
|
|
||
| _logger.LogTrace("SOAP client created with client certificate and server validation disabled"); |
Comment on lines
+249
to
+279
| public async Task<int> DownloadAllIssuedCertificates(BlockingCollection<AnyCAPluginCertificate> certificatesBuffer, CancellationToken cancelToken, DateTime? issuedAfter = null) | ||
| { | ||
| using var flow = new FlowLogger(_logger, "DownloadAllIssuedCertificates"); | ||
| _logger.MethodEntry(LogLevel.Debug); | ||
| EnsureClientIsEnabled(); | ||
|
|
||
| _logger.LogTrace("DownloadAllIssuedCertificates called. certificatesBuffer is {BufferState}, issuedAfter={IssuedAfter}", | ||
| certificatesBuffer == null ? "NULL" : "present", | ||
| issuedAfter?.ToString("o") ?? "(null)"); | ||
|
|
||
| flow.Step("ValidateInputs", () => | ||
| { | ||
| if (certificatesBuffer == null) | ||
| throw new ArgumentNullException(nameof(certificatesBuffer), "certificatesBuffer cannot be null."); | ||
| }); | ||
|
|
||
| _logger.LogDebug("Downloading all issued certificates from Idnomic service {Client}", this.ToString()); | ||
|
|
||
| int numberOfCertificates = 0; | ||
| int skippedCount = 0; | ||
| int filteredCount = 0; | ||
| int errorCount = 0; | ||
|
|
||
| try | ||
| { | ||
| List<Item> certItems = null; | ||
|
|
||
| await flow.StepAsync("FetchCertificateItems", async () => | ||
| { | ||
| certItems = await Task.Run(() => GetCertificateItems(), cancelToken); | ||
| }); |
Comment on lines
+296
to
+320
| var certProperties = item.Item1 as Item[]; | ||
| AnyCAPluginCertificate certificate = null; | ||
|
|
||
| try | ||
| { | ||
| certificate = ParseCertificateFromProperties(certProperties); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "DownloadAllIssuedCertificates: ParseCertificateFromProperties failed: {Message}", ex.Message); | ||
| errorCount++; | ||
| continue; | ||
| } | ||
|
|
||
| if (certificate == null) | ||
| { | ||
| skippedCount++; | ||
| continue; | ||
| } | ||
|
|
||
| if (!PassesIssuerFilter(certificate.Certificate)) | ||
| { | ||
| filteredCount++; | ||
| continue; | ||
| } |
Comment on lines
+169
to
+176
| public void Dispose() | ||
| { | ||
| if (_disposed) return; | ||
| _disposed = true; | ||
|
|
||
| _totalTimer.Stop(); | ||
| RenderFlow(); | ||
| } |
| <PackageReference Include="Keyfactor.AnyGateway.IAnyCAPlugin" Version="3.0.0" /> | ||
| <PackageReference Include="Keyfactor.Logging" Version="1.1.1" /> | ||
| <PackageReference Include="Keyfactor.PKI" Version="5.5.0" /> | ||
| <PackageReference Include="System.Drawing.Common" Version="10.0.5" /> |
Comment on lines
43
to
+50
| public IdnomicCAPlugin(IIdnomicClient client) | ||
| { | ||
| _logger.MethodEntry(); | ||
| _logger = LogHandler.GetClassLogger<IdnomicCAPlugin>(); | ||
| _logger.MethodEntry(LogLevel.Debug); | ||
| Client = client; | ||
| _idnomicClientWasInjected = true; | ||
| _logger.MethodExit(); | ||
| _logger.LogTrace("IdnomicCAPlugin constructed with injected client. Client is {ClientState}", | ||
| client == null ? "NULL" : "present"); |
Comment on lines
538
to
543
| Client.Enable(); | ||
| } | ||
| else | ||
| { | ||
| _logger.LogDebug("Disabling IdnomicClient"); | ||
| Client.Disable(); |
Comment on lines
+1
to
+3
| - 2.0.0 | ||
| - Added Issuer DN Filter (IssuerDnFilter) configuration parameter to restrict certificate synchronization per Logical CA | ||
| - Filter supports case-insensitive substring matching against the certificate's Issuer Distinguished Name |
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.
Merge release-2.0 to main - Automated PR