Skip to content

Commit a890a7d

Browse files
authored
fix(enroll): renewal product code, AutoApprove UI text, config log visibility (#28)
fix(enroll): renewals ignore template product code, correct AutoApprove UI text, log config presence 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.
1 parent 5d2d154 commit a890a7d

6 files changed

Lines changed: 136 additions & 5 deletions

File tree

CERTInext.Tests/CERTInextCAPluginCoverageTests.cs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,60 @@ public async Task RenewOrReissue_CallsRenewApi_WhenCertWithinRenewalWindow()
259259
It.IsAny<CancellationToken>()), Times.Never);
260260
}
261261

262+
// ---------------------------------------------------------------------------
263+
// A1d-2: renewal within window carries the template's product code onto the
264+
// RenewCertificateRequest, not just the connector-level DefaultProductCode.
265+
// Regression for issue #26 / local issues/0012.
266+
// ---------------------------------------------------------------------------
267+
268+
[Fact]
269+
public async Task RenewOrReissue_CallsRenewApi_UsesTemplateProductCode()
270+
{
271+
var clientMock = NewMock();
272+
var readerMock = NewReaderMock();
273+
274+
// Expiry is 30 days in the future, renewal window is 90 days → within window
275+
DateTime expiry = DateTime.UtcNow.AddDays(30);
276+
277+
readerMock
278+
.Setup(r => r.GetRequestIDBySerialNumber(It.IsAny<string>()))
279+
.ReturnsAsync(MockCertificateData.CertId1);
280+
281+
readerMock
282+
.Setup(r => r.GetExpirationDateByRequestId(MockCertificateData.CertId1))
283+
.Returns(expiry);
284+
285+
clientMock
286+
.Setup(c => c.RenewCertificateAsync(
287+
MockCertificateData.CertId1,
288+
It.Is<RenewCertificateRequest>(r => r.ProfileId == MockCertificateData.ProfileIdClient),
289+
It.IsAny<CancellationToken>()))
290+
.ReturnsAsync(MockCertificateData.IssuedEnrollResponse("cert-renewed-002"));
291+
292+
var plugin = new CERTInextCAPlugin(clientMock.Object, readerMock.Object);
293+
294+
// ProfileId is a non-default value distinct from the connector's DefaultProductCode.
295+
var productInfo = MakeProductInfo(profileId: MockCertificateData.ProfileIdClient, extras: new Dictionary<string, string>
296+
{
297+
["PriorCertSN"] = "AABBCCDDEEFF",
298+
["RenewalWindowDays"] = "90"
299+
});
300+
301+
var result = await plugin.Enroll(
302+
csr: MockCertificateData.FakeCsrPem,
303+
subject: "CN=test.example.com",
304+
san: null,
305+
productInfo: productInfo,
306+
requestFormat: RequestFormat.PKCS10,
307+
enrollmentType: EnrollmentType.RenewOrReissue);
308+
309+
result.Status.Should().Be((int)EndEntityStatus.GENERATED);
310+
clientMock.Verify(c => c.RenewCertificateAsync(
311+
MockCertificateData.CertId1,
312+
It.Is<RenewCertificateRequest>(r => r.ProfileId == MockCertificateData.ProfileIdClient),
313+
It.IsAny<CancellationToken>()), Times.Once);
314+
}
315+
262316
// ---------------------------------------------------------------------------
263317
// A1e: PriorCertSN present, cert already expired → new enroll
264318
// Semantics: useRenewalApi = expiry > now && expiry <= now + window.

CERTInext.Tests/CERTInextClientRequestShapeTests.cs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,5 +288,59 @@ public async Task ValidityDays_OnRequest_OverridesConnectorDefault()
288288
CapturedOrderBody().GetProperty("subscriptionDetails")
289289
.GetProperty("validity").GetString().Should().Be("2");
290290
}
291+
292+
// -----------------------------------------------------------------------
293+
// RenewCertificateAsync — productCode resolution (issue #26 / local issues/0012)
294+
// Renewals go out as a fresh GenerateOrderSSL order; the product code must
295+
// come from the template (RenewCertificateRequest.ProfileId) when supplied,
296+
// falling back to the connector's DefaultProductCode only when it is not.
297+
// -----------------------------------------------------------------------
298+
299+
[Fact]
300+
public async Task RenewCertificateAsync_ProfileIdSet_UsesTemplateProductCode()
301+
{
302+
StubHappyEnroll();
303+
var cfg = MinimalConfig();
304+
cfg.DefaultProductCode = "connector-default-code";
305+
306+
var renewReq = new RenewCertificateRequest
307+
{
308+
Csr = MockCertificateData.FakeCsrPem,
309+
ProfileId = "template-product-code",
310+
ValidityDays = 365,
311+
Comment = "Renewal test"
312+
};
313+
314+
await BuildClient(cfg).RenewCertificateAsync(MockCertificateData.OrderNumber1, renewReq);
315+
316+
CapturedOrderBody().GetProperty("productCode").GetString()
317+
.Should().Be("template-product-code",
318+
"the template's own product code must win over the connector default");
319+
}
320+
321+
[Theory]
322+
[InlineData(null)]
323+
[InlineData("")]
324+
[InlineData(" ")]
325+
public async Task RenewCertificateAsync_ProfileIdBlank_FallsBackToConnectorDefault(string blankProfileId)
326+
{
327+
StubHappyEnroll();
328+
var cfg = MinimalConfig();
329+
cfg.DefaultProductCode = "connector-default-code";
330+
331+
var renewReq = new RenewCertificateRequest
332+
{
333+
Csr = MockCertificateData.FakeCsrPem,
334+
ProfileId = blankProfileId,
335+
ValidityDays = 365,
336+
Comment = "Renewal test"
337+
};
338+
339+
await BuildClient(cfg).RenewCertificateAsync(MockCertificateData.OrderNumber1, renewReq);
340+
341+
CapturedOrderBody().GetProperty("productCode").GetString()
342+
.Should().Be("connector-default-code",
343+
"a blank ProfileId must fall back to the connector's DefaultProductCode, not an empty string");
344+
}
291345
}
292346
}

CERTInext/API/CertificateRequest.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,15 @@ public class RenewCertificateRequest
631631
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
632632
public string Subject { get; set; }
633633

634+
/// <summary>
635+
/// Template/enrollment product code to submit the renewal order under. Without it, the
636+
/// renewal falls back to the connector-level default product code, which is often unset —
637+
/// leaving renewals to go out under an empty product code regardless of the template used.
638+
/// </summary>
639+
[JsonPropertyName("profileId")]
640+
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
641+
public string ProfileId { get; set; }
642+
634643
/// <summary>
635644
/// SANs to carry onto the renewal order. Renewals previously submitted none, so a
636645
/// renewed UCC certificate came back holding only its primary domain.

CERTInext/CERTInextCAPlugin.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,20 +240,27 @@ public void Initialize(IAnyCAPluginConfigProvider configProvider, ICertificateDa
240240
bool hasClientId = !string.IsNullOrWhiteSpace(_config.OAuth2ClientId);
241241
bool hasClientSecret= !string.IsNullOrWhiteSpace(_config.OAuth2ClientSecret);
242242
bool hasTokenUrl = !string.IsNullOrWhiteSpace(_config.OAuth2TokenUrl);
243+
bool hasOrganizationNumber = !string.IsNullOrWhiteSpace(_config.OrganizationNumber);
244+
bool hasDefaultProductCode = !string.IsNullOrWhiteSpace(_config.DefaultProductCode);
245+
bool hasGroupNumber = !string.IsNullOrWhiteSpace(_config.GroupNumber);
243246

244247
_logger.LogInformation(
245248
"CERTInext plugin initialized. " +
246249
"ApiUrl={ApiUrl}, AuthMode={AuthMode}, Enabled={Enabled}, " +
247250
"ApiKeyPresent={ApiKeyPresent}, UsernamePresent={UsernamePresent}, " +
248251
"PasswordPresent={PasswordPresent}, OAuth2ClientIdPresent={OAuth2ClientIdPresent}, " +
249252
"OAuth2ClientSecretPresent={OAuth2ClientSecretPresent}, OAuth2TokenUrlPresent={OAuth2TokenUrlPresent}, " +
253+
"OrganizationNumberPresent={OrganizationNumberPresent}, DefaultProductCodePresent={DefaultProductCodePresent}, " +
254+
"GroupNumberPresent={GroupNumberPresent}, " +
250255
"PageSize={PageSize}, IgnoreExpired={IgnoreExpired}, SubmitNonDnsSans={SubmitNonDnsSans}, " +
251256
"DcvEnabled={DcvEnabled}, DcvTxtRecordTemplate={DcvTxtRecordTemplate}, " +
252257
"DomainValidatorFactoryInjected={FactoryInjected}",
253258
_config.ApiUrl, _config.AuthMode, _config.Enabled,
254259
hasApiKey, hasUsername,
255260
hasPassword, hasClientId,
256261
hasClientSecret, hasTokenUrl,
262+
hasOrganizationNumber, hasDefaultProductCode,
263+
hasGroupNumber,
257264
_config.PageSize, _config.IgnoreExpired, _config.SubmitNonDnsSans,
258265
_config.DcvEnabled, _config.DcvTxtRecordTemplate,
259266
_domainValidatorFactory != null);
@@ -1320,6 +1327,7 @@ private async Task<EnrollmentResult> RenewOrReissueAsync(
13201327
// holding only its primary domain.
13211328
Subject = subject,
13221329
Sans = BuildSanList(san, csr, subject),
1330+
ProfileId = ep.ProductCode,
13231331
ValidityDays = ep.ValidityDays > 0 ? ep.ValidityDays : (int?)null,
13241332
RequesterName = string.IsNullOrWhiteSpace(ep.RequesterName) ? null : ep.RequesterName,
13251333
RequesterEmail = string.IsNullOrWhiteSpace(ep.RequesterEmail) ? null : ep.RequesterEmail,

CERTInext/CERTInextCAPluginConfig.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -444,8 +444,8 @@ public static Dictionary<string, PropertyConfigInfo> GetTemplateParameterAnnotat
444444
},
445445
[Constants.EnrollmentParam.AutoApprove] = new PropertyConfigInfo
446446
{
447-
Comments = "OPTIONAL: If true, the gateway will attempt automatic approval of certificates " +
448-
"that are returned in a pending-approval state. Default: false.",
447+
Comments = "Currently has no effect — reserved for future use. The plugin does not call " +
448+
"any approval endpoint against CERTInext regardless of this setting.",
449449
Hidden = false,
450450
DefaultValue = false,
451451
Type = "Boolean"

CERTInext/Client/CERTInextClient.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -810,14 +810,20 @@ public async Task<EnrollCertificateResponse> RenewCertificateAsync(
810810
certificateId, LogSanitizer.Strip(renewalDomainName));
811811
}
812812

813-
// We don't have the product code from TrackOrder — build an order using
814-
// the config defaults and the CSR from the renewal request.
813+
// Prefer the template's own product code (threaded through via request.ProfileId);
814+
// only fall back to the connector-level default when the caller didn't supply one.
815+
// EnrollmentParams.ProductCode never returns null (it returns string.Empty when it
816+
// can't resolve a code), so this must be a blank check, not a null-coalesce — a
817+
// null-coalesce here would make the DefaultProductCode fallback unreachable, the
818+
// same dead-fallback bug that made DefaultProductCode a no-op for new enrollments.
815819
var orderReq = new GenerateOrderSslRequest
816820
{
817821
Meta = await BuildMetaAsync(ct),
818822
OrderDetails = new SslOrderDetails
819823
{
820-
ProductCode = _config.DefaultProductCode ?? string.Empty,
824+
ProductCode = string.IsNullOrWhiteSpace(request.ProfileId)
825+
? (_config.DefaultProductCode ?? string.Empty)
826+
: request.ProfileId,
821827
SaveAndHold = "0",
822828
RequestorInformation = new RequestorInformation
823829
{

0 commit comments

Comments
 (0)