From 49bbb400379c86d2c8c334e4cdd566575209dc0b Mon Sep 17 00:00:00 2001 From: spbsoluble <1661003+spbsoluble@users.noreply.github.com> Date: Thu, 13 Aug 2026 11:56:36 -0700 Subject: [PATCH] fix(enroll): renewals ignore template product code, correct AutoApprove UI text, log config presence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three independent fixes found during UCSD triage (issues #25, #26, #27): - RenewCertificateAsync built every renewal order from the connector's DefaultProductCode alone, ignoring the template's own ProductCode/ProfileId entirely. Threaded the template's code through RenewCertificateRequest.ProfileId, falling back to DefaultProductCode only when the template doesn't have one (using a blank-check, not ??, since EnrollmentParams.ProductCode never returns null — the same dead-fallback bug that made DefaultProductCode a no-op for new enrollments). - AutoApprove's UI text claimed the plugin attempts automatic approval of pending certificates; no such call exists anywhere in the code. Corrected to say so plainly. - OrganizationNumber, DefaultProductCode, and GroupNumber had zero log visibility, which is what made a stuck-pending-orders question undiagnosable from a support log. Added presence flags to the plugin-initialized log line. --- .../CERTInextCAPluginCoverageTests.cs | 54 +++++++++++++++++++ .../CERTInextClientRequestShapeTests.cs | 54 +++++++++++++++++++ CERTInext/API/CertificateRequest.cs | 9 ++++ CERTInext/CERTInextCAPlugin.cs | 8 +++ CERTInext/CERTInextCAPluginConfig.cs | 4 +- CERTInext/Client/CERTInextClient.cs | 12 +++-- 6 files changed, 136 insertions(+), 5 deletions(-) diff --git a/CERTInext.Tests/CERTInextCAPluginCoverageTests.cs b/CERTInext.Tests/CERTInextCAPluginCoverageTests.cs index f684f7d..1a9faa9 100644 --- a/CERTInext.Tests/CERTInextCAPluginCoverageTests.cs +++ b/CERTInext.Tests/CERTInextCAPluginCoverageTests.cs @@ -259,6 +259,60 @@ public async Task RenewOrReissue_CallsRenewApi_WhenCertWithinRenewalWindow() It.IsAny()), Times.Never); } + // --------------------------------------------------------------------------- + // A1d-2: renewal within window carries the template's product code onto the + // RenewCertificateRequest, not just the connector-level DefaultProductCode. + // Regression for issue #26 / local issues/0012. + // --------------------------------------------------------------------------- + + [Fact] + public async Task RenewOrReissue_CallsRenewApi_UsesTemplateProductCode() + { + var clientMock = NewMock(); + var readerMock = NewReaderMock(); + + // Expiry is 30 days in the future, renewal window is 90 days → within window + DateTime expiry = DateTime.UtcNow.AddDays(30); + + readerMock + .Setup(r => r.GetRequestIDBySerialNumber(It.IsAny())) + .ReturnsAsync(MockCertificateData.CertId1); + + readerMock + .Setup(r => r.GetExpirationDateByRequestId(MockCertificateData.CertId1)) + .Returns(expiry); + + clientMock + .Setup(c => c.RenewCertificateAsync( + MockCertificateData.CertId1, + It.Is(r => r.ProfileId == MockCertificateData.ProfileIdClient), + It.IsAny())) + .ReturnsAsync(MockCertificateData.IssuedEnrollResponse("cert-renewed-002")); + + var plugin = new CERTInextCAPlugin(clientMock.Object, readerMock.Object); + + // ProfileId is a non-default value distinct from the connector's DefaultProductCode. + var productInfo = MakeProductInfo(profileId: MockCertificateData.ProfileIdClient, extras: new Dictionary + { + ["PriorCertSN"] = "AABBCCDDEEFF", + ["RenewalWindowDays"] = "90" + }); + + var result = await plugin.Enroll( + csr: MockCertificateData.FakeCsrPem, + subject: "CN=test.example.com", + san: null, + productInfo: productInfo, + requestFormat: RequestFormat.PKCS10, + enrollmentType: EnrollmentType.RenewOrReissue); + + result.Status.Should().Be((int)EndEntityStatus.GENERATED); + clientMock.Verify(c => c.RenewCertificateAsync( + MockCertificateData.CertId1, + It.Is(r => r.ProfileId == MockCertificateData.ProfileIdClient), + It.IsAny()), Times.Once); + } + // --------------------------------------------------------------------------- // A1e: PriorCertSN present, cert already expired → new enroll // Semantics: useRenewalApi = expiry > now && expiry <= now + window. diff --git a/CERTInext.Tests/CERTInextClientRequestShapeTests.cs b/CERTInext.Tests/CERTInextClientRequestShapeTests.cs index 4e59495..fd61dfb 100644 --- a/CERTInext.Tests/CERTInextClientRequestShapeTests.cs +++ b/CERTInext.Tests/CERTInextClientRequestShapeTests.cs @@ -288,5 +288,59 @@ public async Task ValidityDays_OnRequest_OverridesConnectorDefault() CapturedOrderBody().GetProperty("subscriptionDetails") .GetProperty("validity").GetString().Should().Be("2"); } + + // ----------------------------------------------------------------------- + // RenewCertificateAsync — productCode resolution (issue #26 / local issues/0012) + // Renewals go out as a fresh GenerateOrderSSL order; the product code must + // come from the template (RenewCertificateRequest.ProfileId) when supplied, + // falling back to the connector's DefaultProductCode only when it is not. + // ----------------------------------------------------------------------- + + [Fact] + public async Task RenewCertificateAsync_ProfileIdSet_UsesTemplateProductCode() + { + StubHappyEnroll(); + var cfg = MinimalConfig(); + cfg.DefaultProductCode = "connector-default-code"; + + var renewReq = new RenewCertificateRequest + { + Csr = MockCertificateData.FakeCsrPem, + ProfileId = "template-product-code", + ValidityDays = 365, + Comment = "Renewal test" + }; + + await BuildClient(cfg).RenewCertificateAsync(MockCertificateData.OrderNumber1, renewReq); + + CapturedOrderBody().GetProperty("productCode").GetString() + .Should().Be("template-product-code", + "the template's own product code must win over the connector default"); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public async Task RenewCertificateAsync_ProfileIdBlank_FallsBackToConnectorDefault(string blankProfileId) + { + StubHappyEnroll(); + var cfg = MinimalConfig(); + cfg.DefaultProductCode = "connector-default-code"; + + var renewReq = new RenewCertificateRequest + { + Csr = MockCertificateData.FakeCsrPem, + ProfileId = blankProfileId, + ValidityDays = 365, + Comment = "Renewal test" + }; + + await BuildClient(cfg).RenewCertificateAsync(MockCertificateData.OrderNumber1, renewReq); + + CapturedOrderBody().GetProperty("productCode").GetString() + .Should().Be("connector-default-code", + "a blank ProfileId must fall back to the connector's DefaultProductCode, not an empty string"); + } } } diff --git a/CERTInext/API/CertificateRequest.cs b/CERTInext/API/CertificateRequest.cs index 043b4b5..c0da0d6 100644 --- a/CERTInext/API/CertificateRequest.cs +++ b/CERTInext/API/CertificateRequest.cs @@ -631,6 +631,15 @@ public class RenewCertificateRequest [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public string Subject { get; set; } + /// + /// Template/enrollment product code to submit the renewal order under. Without it, the + /// renewal falls back to the connector-level default product code, which is often unset — + /// leaving renewals to go out under an empty product code regardless of the template used. + /// + [JsonPropertyName("profileId")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public string ProfileId { get; set; } + /// /// SANs to carry onto the renewal order. Renewals previously submitted none, so a /// renewed UCC certificate came back holding only its primary domain. diff --git a/CERTInext/CERTInextCAPlugin.cs b/CERTInext/CERTInextCAPlugin.cs index a631606..76c3cb4 100644 --- a/CERTInext/CERTInextCAPlugin.cs +++ b/CERTInext/CERTInextCAPlugin.cs @@ -240,6 +240,9 @@ public void Initialize(IAnyCAPluginConfigProvider configProvider, ICertificateDa bool hasClientId = !string.IsNullOrWhiteSpace(_config.OAuth2ClientId); bool hasClientSecret= !string.IsNullOrWhiteSpace(_config.OAuth2ClientSecret); bool hasTokenUrl = !string.IsNullOrWhiteSpace(_config.OAuth2TokenUrl); + bool hasOrganizationNumber = !string.IsNullOrWhiteSpace(_config.OrganizationNumber); + bool hasDefaultProductCode = !string.IsNullOrWhiteSpace(_config.DefaultProductCode); + bool hasGroupNumber = !string.IsNullOrWhiteSpace(_config.GroupNumber); _logger.LogInformation( "CERTInext plugin initialized. " + @@ -247,6 +250,8 @@ public void Initialize(IAnyCAPluginConfigProvider configProvider, ICertificateDa "ApiKeyPresent={ApiKeyPresent}, UsernamePresent={UsernamePresent}, " + "PasswordPresent={PasswordPresent}, OAuth2ClientIdPresent={OAuth2ClientIdPresent}, " + "OAuth2ClientSecretPresent={OAuth2ClientSecretPresent}, OAuth2TokenUrlPresent={OAuth2TokenUrlPresent}, " + + "OrganizationNumberPresent={OrganizationNumberPresent}, DefaultProductCodePresent={DefaultProductCodePresent}, " + + "GroupNumberPresent={GroupNumberPresent}, " + "PageSize={PageSize}, IgnoreExpired={IgnoreExpired}, SubmitNonDnsSans={SubmitNonDnsSans}, " + "DcvEnabled={DcvEnabled}, DcvTxtRecordTemplate={DcvTxtRecordTemplate}, " + "DomainValidatorFactoryInjected={FactoryInjected}", @@ -254,6 +259,8 @@ public void Initialize(IAnyCAPluginConfigProvider configProvider, ICertificateDa hasApiKey, hasUsername, hasPassword, hasClientId, hasClientSecret, hasTokenUrl, + hasOrganizationNumber, hasDefaultProductCode, + hasGroupNumber, _config.PageSize, _config.IgnoreExpired, _config.SubmitNonDnsSans, _config.DcvEnabled, _config.DcvTxtRecordTemplate, _domainValidatorFactory != null); @@ -1320,6 +1327,7 @@ private async Task RenewOrReissueAsync( // holding only its primary domain. Subject = subject, Sans = BuildSanList(san, csr, subject), + ProfileId = ep.ProductCode, ValidityDays = ep.ValidityDays > 0 ? ep.ValidityDays : (int?)null, RequesterName = string.IsNullOrWhiteSpace(ep.RequesterName) ? null : ep.RequesterName, RequesterEmail = string.IsNullOrWhiteSpace(ep.RequesterEmail) ? null : ep.RequesterEmail, diff --git a/CERTInext/CERTInextCAPluginConfig.cs b/CERTInext/CERTInextCAPluginConfig.cs index 980a26a..93fb26a 100644 --- a/CERTInext/CERTInextCAPluginConfig.cs +++ b/CERTInext/CERTInextCAPluginConfig.cs @@ -444,8 +444,8 @@ public static Dictionary GetTemplateParameterAnnotat }, [Constants.EnrollmentParam.AutoApprove] = new PropertyConfigInfo { - Comments = "OPTIONAL: If true, the gateway will attempt automatic approval of certificates " + - "that are returned in a pending-approval state. Default: false.", + Comments = "Currently has no effect — reserved for future use. The plugin does not call " + + "any approval endpoint against CERTInext regardless of this setting.", Hidden = false, DefaultValue = false, Type = "Boolean" diff --git a/CERTInext/Client/CERTInextClient.cs b/CERTInext/Client/CERTInextClient.cs index 9ecde2b..2fdcc18 100644 --- a/CERTInext/Client/CERTInextClient.cs +++ b/CERTInext/Client/CERTInextClient.cs @@ -810,14 +810,20 @@ public async Task RenewCertificateAsync( certificateId, LogSanitizer.Strip(renewalDomainName)); } - // We don't have the product code from TrackOrder — build an order using - // the config defaults and the CSR from the renewal request. + // Prefer the template's own product code (threaded through via request.ProfileId); + // only fall back to the connector-level default when the caller didn't supply one. + // EnrollmentParams.ProductCode never returns null (it returns string.Empty when it + // can't resolve a code), so this must be a blank check, not a null-coalesce — a + // null-coalesce here would make the DefaultProductCode fallback unreachable, the + // same dead-fallback bug that made DefaultProductCode a no-op for new enrollments. var orderReq = new GenerateOrderSslRequest { Meta = await BuildMetaAsync(ct), OrderDetails = new SslOrderDetails { - ProductCode = _config.DefaultProductCode ?? string.Empty, + ProductCode = string.IsNullOrWhiteSpace(request.ProfileId) + ? (_config.DefaultProductCode ?? string.Empty) + : request.ProfileId, SaveAndHold = "0", RequestorInformation = new RequestorInformation {