Allow signatures in Valid or InGracePeriod states (#358)

Author: loic-sharma

src/NuGet.Services.Validation.Orchestrator/PackageCertificates/PackageCertificatesValidator.cs

@@ -74,10 +74,10 @@ private async Task<ValidatorStatus> GetStatusAsync(IValidationRequest request)
                 return status;
             }
 
-            // All of the requested certificate validations have finished. Fail the validation if the
-            // signature has been invalidated. At this point, the signature MUST have a state of either
-            // "Unknown" or "Invalid" as the PackageSigningValidator sets signatures to an "Unknown" status
-            // and the ValidateCertificate job may set signatures to the "Invalid" state.
+            // All of the requested certificate validations have finished. At this point, the signature
+            // may have a status of "Unknown" if the package is at ingestion and its signature has passed
+            // all validations, "Invalid" if one or more of the signature's certificates has failed validations,
+            // or "InGracePeriod" or "Valid" if this is a revalidation request.
             var signature = await FindSignatureAsync(request);
 
             if (signature.Status == PackageSignatureStatus.Invalid)
@@ -91,18 +91,6 @@ private async Task<ValidatorStatus> GetStatusAsync(IValidationRequest request)
 
                 return await _validatorStateService.TryUpdateValidationStatusAsync(request, status, ValidationStatus.Failed);
             }
-            else if (signature.Status != PackageSignatureStatus.Unknown)
-            {
-                _logger.LogError(
-                    Error.PackageCertificateValidationInvalidSignatureState,
-                    "Failing validation {ValidationId} ({PackageId} {PackageVersion}) due to invalid signature status: {SignatureStatus}",
-                    request.ValidationId,
-                    request.PackageId,
-                    request.PackageVersion,
-                    signature.Status);
-
-                return await _validatorStateService.TryUpdateValidationStatusAsync(request, status, ValidationStatus.Failed);
-            }
             else
             {
                 _logger.LogInformation(
@@ -256,9 +244,10 @@ private void PromoteSignature(IValidationRequest request, PackageSignature signa
                                         : PackageSignatureStatus.InGracePeriod;
 
             _logger.LogInformation(
-                "Promoting package {PackageId} {PackageVersion} signature to status {SignatureStatus}",
+                "Promoting package {PackageId} {PackageVersion} signature from status {OldSignatureStatus} to status {NewSignatureStatus}",
                 request.PackageId,
                 request.PackageVersion,
+                signature.Status,
                 newSignatureStatus);
 
             signature.Status = newSignatureStatus;
@@ -288,7 +277,7 @@ bool IsCertificateStatusPastTime(EndCertificate certificate, DateTime time)
                 if (timestamp.EndCertificate.Status == EndCertificateStatus.Revoked)
                 {
                     _logger.LogError(
-                        0,
+                        Error.PackageCertificateValidationInvalidSignatureState,
                         "Valid signature cannot have a timestamp whose end certificate is revoked ({ValidationId}, {PackageId} {PackageVersion})",
                         request.ValidationId,
                         request.PackageId,

tests/NuGet.Services.Validation.Orchestrator.Tests/PackageCertificates/PackageCertificatesValidatorFacts.cs

@@ -203,22 +203,21 @@ public async Task ReturnsExpectedStatusForCertificateValidations(ValidationStatu
 
             public static IEnumerable<object[]> InvalidSignatureFailsValidationData()
             {
-                // Signatures SHOULD NOT have "Valid" and "InGracePeriod" Statuses before
-                // the CertificateValidator finishes. If the signatures somehow do, the
-                // validator should fail as this is an invalid state.
                 yield return new object[]
                 {
-                    ValidationStatus.Failed, PackageSignatureStatus.Valid
+                    ValidationStatus.Succeeded, PackageSignatureStatus.Unknown
                 };
 
+                // Signatures may have "Valid" and "InGracePeriod" Statuses before
+                // the CertificateValidator finishes due to revalidations.
                 yield return new object[]
                 {
-                    ValidationStatus.Failed, PackageSignatureStatus.InGracePeriod
+                    ValidationStatus.Succeeded, PackageSignatureStatus.Valid
                 };
 
                 yield return new object[]
                 {
-                    ValidationStatus.Succeeded, PackageSignatureStatus.Unknown
+                    ValidationStatus.Succeeded, PackageSignatureStatus.InGracePeriod
                 };
 
                 yield return new object[]