Return package signing requirement issues to the user and fail the va…

…lidation (#1092)

Progress on NuGet/Engineering#1739

src/NuGet.Services.Validation.Orchestrator/PackageSigning/ProcessSignature/PackageSignatureValidator.cs

@@ -92,6 +92,23 @@ private INuGetValidationResponse Validate(INuGetValidationRequest request, INuGe
                     return NuGetValidationResponse.Succeeded;
                 }
 
+                if (response.Issues.Count == 1)
+                {
+                    var singleIssueCode = response.Issues[0].IssueCode;
+                    if (singleIssueCode == ValidationIssueCode.PackageIsNotSigned
+                        || singleIssueCode == ValidationIssueCode.PackageIsSignedWithUnauthorizedCertificate)
+                    {
+                        /// This indicates that the package owner changed their certificate requirements on the package
+                        /// between the time that <see cref="PackageSignatureProcessor"/> and
+                        /// <see cref="PackageSignatureValidator"/> ran. In most cases, the processor catches this
+                        /// problem so we never reach this state, but it is possible for a race condition between the
+                        /// validation flow and the user's updates to their signing certificate requirements.
+                        /// 
+                        /// In this case we want to surface the issues to the user so that they can try again.
+                        return response;
+                    }
+                }
+
                 _logger.LogCritical(
                     "Unexpected validation response in package signature validator. This may be caused by an invalid repository " +
                     "signature. Throwing an exception to force this validation to dead-letter. " +

tests/NuGet.Services.Validation.Orchestrator.Tests/PackageSigning/ProcessSignature/PackageSignatureValidatorFacts.cs

@@ -64,7 +64,7 @@ public async Task ReturnsPersistedStatus(ValidationStatus status)
             }
 
             [Fact]
-            public async Task DoesNotReturnValidatorIssues()
+            public async Task WhenStateIsSuccess_DoesNotReturnValidatorIssues()
             {
                 // Arrange
                 _validatorStateService
@@ -98,6 +98,70 @@ public async Task DoesNotReturnValidatorIssues()
                 Assert.Equal(0, actual.Issues.Count);
             }
 
+            [Fact]
+            public async Task WhenStateIsFailedAndIssuesAreNotExpected_FailsWithException()
+            {
+                // Arrange
+                _config.RepositorySigningEnabled = true;
+
+                _validatorStateService
+                    .Setup(x => x.GetStatusAsync(It.IsAny<INuGetValidationRequest>()))
+                    .ReturnsAsync(new ValidatorStatus
+                    {
+                        ValidationId = ValidationId,
+                        PackageKey = PackageKey,
+                        ValidatorName = ValidatorName.PackageSignatureProcessor,
+                        State = ValidationStatus.Failed,
+                        ValidatorIssues = new List<ValidatorIssue>
+                        {
+                            new ValidatorIssue
+                            {
+                                IssueCode = ValidationIssueCode.PackageIsZip64,
+                                Data = "{}",
+                            },
+                        },
+                    });
+
+                // Act & Assert
+                var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => _target.GetResponseAsync(_validationRequest.Object));
+                Assert.Equal("Package signature validator has an unexpected validation response", ex.Message);
+            }
+
+            [Theory]
+            [InlineData(ValidationIssueCode.PackageIsNotSigned)]
+            [InlineData(ValidationIssueCode.PackageIsSignedWithUnauthorizedCertificate)]
+            public async Task WhenStateIsFailedAndIssueCanBeCausedByOwner_ReturnsFailedValidation(ValidationIssueCode issueCode)
+            {
+                // Arrange
+                _config.RepositorySigningEnabled = true;
+
+                _validatorStateService
+                    .Setup(x => x.GetStatusAsync(It.IsAny<INuGetValidationRequest>()))
+                    .ReturnsAsync(new ValidatorStatus
+                    {
+                        ValidationId = ValidationId,
+                        PackageKey = PackageKey,
+                        ValidatorName = ValidatorName.PackageSignatureProcessor,
+                        State = ValidationStatus.Failed,
+                        ValidatorIssues = new List<ValidatorIssue>
+                        {
+                            new ValidatorIssue
+                            {
+                                IssueCode = issueCode,
+                                Data = "{}",
+                            },
+                        },
+                    });
+
+                // Act
+                var actual = await _target.GetResponseAsync(_validationRequest.Object);
+
+                // Assert
+                Assert.Equal(ValidationStatus.Failed, actual.Status);
+                Assert.Equal(1, actual.Issues.Count);
+                Assert.Equal(issueCode, actual.Issues.Single().IssueCode);
+            }
+
             [Fact]
             public async Task WhenRepositorySigningEnabled_ThrowsExceptionIfValidationFails()
             {