fix(dcv): run post-DCV issuance wait when DCV is already validated

Closes the cached-DCV gap surfaced during live testing on the new sandbox:
when CERTInext has cached a prior DCV validation for the parent domain (e.g.
scrup.org used across many test runs), the plugin's TrackOrder check sees
domainVerification already in a validated state and PerformDcvIfNeededAsync
used to return false ("skipped"). That left WaitForIssuanceAfterDcvAsync
unreachable, so Enroll() returned a pending result even though CERTInext
was about to generate the cert within seconds — forcing the gateway to wait
for the next sync cycle.

Changes:
- PerformDcvIfNeededAsync now returns true when DCV is functionally done,
  defined as either (a) the aggregate domainVerification.Status == "1"
  OR (b) every per-domain dcvStatus == "1" (the per-domain field has been
  observed flipping to validated slightly before the parent aggregate).
- WaitForIssuanceAfterDcvAsync short-circuits when its budget <= 0,
  avoiding a wasted GetCertificate call and keeping Strict-mock unit tests
  that intentionally disable the wait passing without re-wiring mocks.
- Rename dcvRan -> dcvDone at the three call sites (EnrollNewAsync,
  Synchronize, GetSingleRecord) to reflect the new "DCV is done, by us or
  CERTInext" semantics.

Tests:
- Rename Dcv_Skipped_WhenAllDomainsAlreadyValidated to
  Dcv_SkipsStaging_AndDoesNotIssuancePoll_WhenAllDomainsAlreadyValidated_AndIssuanceBudgetZero
  and assert GetCertificateAsync is never called in that path.
- Add Dcv_RunsIssuanceWait_WhenDcvAlreadyValidated_AndIssuanceBudgetPositive
  pinning the new cached-DCV happy path (pending then issued via the
  issuance poll).
- Bump issuance budget on Dcv_HappyPath_* and
  Dcv_WaitsForChallenge_WhenDomainVerificationAppearsLate so they continue
  to exercise the post-DCV fetch.

137/137 unit tests pass.

Author: spbsoluble

CERTInext.Tests/CERTInextCAPluginDcvTests.cs

@@ -116,7 +116,10 @@ private static Task<EnrollmentResult> Enroll(CERTInextCAPlugin plugin) =>
         public async Task Dcv_HappyPath_StagesVerifiesAndCleansUp()
         {
             var (mock, validator) = HappyPathMocks();
-            var plugin = BuildPlugin(mock.Object, new FakeDomainValidatorFactory(validator));
+            // Issuance budget > 0 so the post-DCV GetCertificate poll runs and lifts the
+            // issued cert out of the mock back into the EnrollmentResult.
+            var plugin = BuildPlugin(mock.Object, new FakeDomainValidatorFactory(validator),
+                DcvConfig(dcvWaitForIssuanceSeconds: 10));
 
             var result = await Enroll(plugin);
 
@@ -144,7 +147,8 @@ public async Task Dcv_HappyPath_StagesVerifiesAndCleansUp()
         public async Task Dcv_HappyPath_UsesCustomTxtTemplate()
         {
             var (mock, validator) = HappyPathMocks();
-            var config = DcvConfig();
+            // Issuance budget > 0 so the post-DCV GetCertificate poll runs.
+            var config = DcvConfig(dcvWaitForIssuanceSeconds: 10);
             config.DcvTxtRecordTemplate = "dcv-proof.{0}.acme-corp.com";
             var plugin = BuildPlugin(mock.Object, new FakeDomainValidatorFactory(validator), config);
 
@@ -211,8 +215,11 @@ public async Task Dcv_Skipped_WhenNoDomainVerificationBlock()
         }
 
         [Fact]
-        public async Task Dcv_Skipped_WhenAllDomainsAlreadyValidated()
+        public async Task Dcv_SkipsStaging_AndDoesNotIssuancePoll_WhenAllDomainsAlreadyValidated_AndIssuanceBudgetZero()
         {
+            // With DcvWaitForIssuanceSeconds=0 (the test fixture's DcvConfig default), an
+            // order with DCV already validated short-circuits: no TXT records staged AND
+            // no post-DCV GetCertificate poll. Lets sync pick up the cert on its own.
             var mock = NewMock();
             mock.Setup(c => c.EnrollCertificateAsync(It.IsAny<EnrollCertificateRequest>(), It.IsAny<CancellationToken>()))
                 .ReturnsAsync(new EnrollCertificateResponse { Id = MockCertificateData.DcvOrderId, Status = "pending" });
@@ -239,6 +246,54 @@ public async Task Dcv_Skipped_WhenAllDomainsAlreadyValidated()
 
             validator.StagedRecords.Should().BeEmpty();
             mock.Verify(c => c.GetDcvAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()), Times.Never);
+            // Issuance budget = 0 means the post-DCV poll short-circuits and GetCertificate
+            // is never called from this Enroll() path.
+            mock.Verify(c => c.GetCertificateAsync(It.IsAny<string>(), It.IsAny<CancellationToken>()), Times.Never);
+        }
+
+        [Fact]
+        public async Task Dcv_RunsIssuanceWait_WhenDcvAlreadyValidated_AndIssuanceBudgetPositive()
+        {
+            // The cached-DCV gap fix: when CERTInext shows DCV already validated (no work
+            // for the plugin's DNS-TXT staging) AND the admin has set a positive issuance
+            // budget, the plugin should poll GetCertificate until the cert is generated
+            // and return the issued result directly from Enroll() — not leave it for sync.
+            var mock = NewMock();
+            mock.Setup(c => c.EnrollCertificateAsync(It.IsAny<EnrollCertificateRequest>(), It.IsAny<CancellationToken>()))
+                .ReturnsAsync(new EnrollCertificateResponse { Id = MockCertificateData.DcvOrderId, Status = "pending" });
+
+            mock.Setup(c => c.TrackOrderAsync(MockCertificateData.DcvOrderId, It.IsAny<CancellationToken>()))
+                .ReturnsAsync(new TrackOrderResponse
+                {
+                    OrderDetails = new TrackOrderResponseDetails
+                    {
+                        OrderStatusId       = "1",
+                        CertificateStatusId = "1",
+                        DomainVerification  = new TrackOrderDomainVerification
+                        {
+                            Status = Constants.Dcv.StatusValidated
+                        }
+                    }
+                });
+
+            // First post-DCV fetch is still pending; second returns issued.
+            mock.SetupSequence(c => c.GetCertificateAsync(MockCertificateData.DcvOrderId, It.IsAny<CancellationToken>()))
+                .ReturnsAsync(MockCertificateData.PendingCertRecord(MockCertificateData.DcvOrderId))
+                .ReturnsAsync(MockCertificateData.IssuedCertRecord(MockCertificateData.DcvOrderId));
+
+            var validator = new FakeDomainValidator();
+            var plugin = BuildPlugin(mock.Object, new FakeDomainValidatorFactory(validator),
+                DcvConfig(dcvWaitForIssuanceSeconds: 10));
+
+            var result = await Enroll(plugin);
+
+            result.Status.Should().Be((int)EndEntityStatus.GENERATED,
+                "the issuance poll must lift the issued cert into the EnrollmentResult, " +
+                "not let the order fall through to a pending-then-sync round-trip");
+            validator.StagedRecords.Should().BeEmpty("no TXT staging is needed when DCV is already validated");
+            mock.Verify(c => c.GetDcvAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()), Times.Never);
+            mock.Verify(c => c.GetCertificateAsync(MockCertificateData.DcvOrderId, It.IsAny<CancellationToken>()),
+                Times.AtLeast(2), "plugin should have polled at least twice to see the cert transition to issued");
         }
 
         [Fact]
@@ -484,10 +539,11 @@ public async Task Dcv_WaitsForChallenge_WhenDomainVerificationAppearsLate()
                 .ReturnsAsync(MockCertificateData.IssuedCertRecord(MockCertificateData.DcvOrderId));
 
             var validator = new FakeDomainValidator();
+            // Both budgets positive so the polling paths exercise end-to-end.
             var plugin = BuildPlugin(
                 mock.Object,
                 new FakeDomainValidatorFactory(validator),
-                DcvConfig(dcvWaitForChallengeSeconds: 10, dcvWaitForIssuanceSeconds: 0));
+                DcvConfig(dcvWaitForChallengeSeconds: 10, dcvWaitForIssuanceSeconds: 10));
 
             var result = await Enroll(plugin);
 

CERTInext/CERTInextCAPlugin.cs

@@ -520,8 +520,8 @@ public async Task<AnyCAPluginCertificate> GetSingleRecord(string caRequestID)
                 int status = StatusMapper.ToRequestDisposition(cert.Status);
                 if (status == (int)EndEntityStatus.EXTERNALVALIDATION)
                 {
-                    bool dcvRan = await TryRunDcvDuringSyncAsync(caRequestID, CancellationToken.None);
-                    if (dcvRan)
+                    bool dcvDone = await TryRunDcvDuringSyncAsync(caRequestID, CancellationToken.None);
+                    if (dcvDone)
                     {
                         try
                         {
@@ -693,8 +693,8 @@ public async Task Synchronize(
                         // already done or not yet exposed.
                         if (status == (int)EndEntityStatus.EXTERNALVALIDATION)
                         {
-                            bool dcvRan = await TryRunDcvDuringSyncAsync(current.Id, cancelToken);
-                            if (dcvRan)
+                            bool dcvDone = await TryRunDcvDuringSyncAsync(current.Id, cancelToken);
+                            if (dcvDone)
                             {
                                 try
                                 {
@@ -823,8 +823,8 @@ private async Task<EnrollmentResult> EnrollNewAsync(
                 {
                     try
                     {
-                        bool dcvRan = await PerformDcvIfNeededAsync(orderNumber, dcvCts.Token);
-                        if (dcvRan)
+                        bool dcvDone = await PerformDcvIfNeededAsync(orderNumber, dcvCts.Token);
+                        if (dcvDone)
                         {
                             // Poll GetCertificate until CERTInext finishes generating the cert OR the
                             // issuance budget expires.  CERTInext issuance is async — DCV may verify
@@ -1149,9 +1149,30 @@ private async Task<bool> PerformDcvIfNeededAsync(string orderNumber, Cancellatio
                 }
             }
 
-            // If the overall DCV status is already validated, nothing to do
-            if (string.Equals(domainVerification.Status, Constants.Dcv.StatusValidated, StringComparison.Ordinal))
-                return false;
+            // If DCV is already validated CERTInext-side, the plugin has no DCV work to
+            // do — but CERTInext's certificate generation may still be in flight (this
+            // happens when CERTInext has cached a prior DCV validation for the parent
+            // domain).  Return true so the caller can run the issuance poll and pick up
+            // the cert directly from Enroll() instead of leaving it for the next sync.
+            //
+            // Treat "DCV done" as EITHER the overall aggregate Status flipping to "1"
+            // OR every individual per-domain dcvStatus being "1" — observed in the wild
+            // that the per-domain field can flip before the parent aggregate.
+            var allDomainEntries = domainVerification.GetDomainEntries();
+            bool aggregateValidated = string.Equals(
+                domainVerification.Status, Constants.Dcv.StatusValidated, StringComparison.Ordinal);
+            bool everyDomainValidated = allDomainEntries.Count > 0
+                && allDomainEntries.All(kvp => string.Equals(
+                    kvp.Value?.DcvStatus, Constants.Dcv.StatusValidated, StringComparison.Ordinal));
+            if (aggregateValidated || everyDomainValidated)
+            {
+                _logger.LogInformation(
+                    "DCV is already validated for order {OrderNumber} " +
+                    "(aggregateStatus={Aggregate}, perDomainAllValidated={PerDomain}). " +
+                    "Skipping DNS-TXT staging; caller may run the issuance poll.",
+                    orderNumber, aggregateValidated, everyDomainValidated);
+                return true;
+            }
 
             // Include domains that are pending DCV and either have no method set yet,
             // or are already assigned to DNS TXT (numeric "1" from API or label from TrackOrder).
@@ -1332,6 +1353,19 @@ private async Task<LegacyGetCertificateResponse> WaitForIssuanceAfterDcvAsync(
             DateTime deadline = DateTime.UtcNow.AddSeconds(Math.Max(0, waitBudgetSeconds));
             LegacyGetCertificateResponse last = null;
 
+            // Admin opt-out: budget <= 0 means "don't wait, let sync pick the cert up".
+            // Short-circuit before any API call so the gateway doesn't pay a TrackOrder +
+            // optional DownloadCertificate round trip per Enroll when the admin has
+            // explicitly disabled the wait.
+            if (waitBudgetSeconds <= 0)
+            {
+                _logger.LogDebug(
+                    "Post-DCV issuance wait disabled (DcvWaitForIssuanceSeconds<=0). " +
+                    "Order {OrderNumber} will be picked up on the next sync cycle.",
+                    orderNumber);
+                return null;
+            }
+
             int attempt = 0;
             while (true)
             {