diff --git a/src/AccountDeleter/EmptyFeatureFlagService.cs b/src/AccountDeleter/EmptyFeatureFlagService.cs index 16a4b51fff..c16971728d 100644 --- a/src/AccountDeleter/EmptyFeatureFlagService.cs +++ b/src/AccountDeleter/EmptyFeatureFlagService.cs @@ -290,5 +290,10 @@ public bool ProxyGravatarEnSubdomain() { throw new NotImplementedException(); } + + public bool IsDisplayUploadWarningV2Enabled(User user) + { + throw new NotImplementedException(); + } } } diff --git a/src/GitHubVulnerabilities2Db/Fakes/FakeFeatureFlagService.cs b/src/GitHubVulnerabilities2Db/Fakes/FakeFeatureFlagService.cs index 328ec9ad31..a6a4822462 100644 --- a/src/GitHubVulnerabilities2Db/Fakes/FakeFeatureFlagService.cs +++ b/src/GitHubVulnerabilities2Db/Fakes/FakeFeatureFlagService.cs @@ -130,6 +130,11 @@ public bool IsMarkdigMdSyntaxHighlightEnabled() throw new NotImplementedException(); } + public bool IsDisplayUploadWarningV2Enabled(User user) + { + throw new NotImplementedException(); + } + public bool IsODataDatabaseReadOnlyEnabled() { throw new NotImplementedException(); diff --git a/src/GitHubVulnerabilities2Db/GitHubVulnerabilities2Db.csproj b/src/GitHubVulnerabilities2Db/GitHubVulnerabilities2Db.csproj index a6e893978f..8b030f61cb 100644 --- a/src/GitHubVulnerabilities2Db/GitHubVulnerabilities2Db.csproj +++ b/src/GitHubVulnerabilities2Db/GitHubVulnerabilities2Db.csproj @@ -94,9 +94,6 @@ 2.106.0 - - - {6262f4fc-29be-4226-b676-db391c89d396} diff --git a/src/NuGetGallery.Core/NuGetVersionExtensions.cs b/src/NuGetGallery.Core/NuGetVersionExtensions.cs index 8877b55a2c..af986e3c87 100644 --- a/src/NuGetGallery.Core/NuGetVersionExtensions.cs +++ b/src/NuGetGallery.Core/NuGetVersionExtensions.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Text.RegularExpressions; +using NuGet.Services.Entities; using NuGet.Versioning; namespace NuGetGallery @@ -31,6 +32,16 @@ public static string ToFullString(string version) return version; } } + + public static string GetNormalizedPackageVersion(Package package) + { + if (package == null) + { + return string.Empty; + } + + return string.IsNullOrEmpty(package.NormalizedVersion) ? Normalize(package.Version) : package.NormalizedVersion; + } } public static class NuGetVersionExtensions diff --git a/src/NuGetGallery.Core/Services/FIleNameHelper.cs b/src/NuGetGallery.Core/Services/FIleNameHelper.cs index 6b614d5cc7..be18f32092 100644 --- a/src/NuGetGallery.Core/Services/FIleNameHelper.cs +++ b/src/NuGetGallery.Core/Services/FIleNameHelper.cs @@ -29,9 +29,8 @@ public static string BuildFileName(Package package, string format, string exten return BuildFileName( package.PackageRegistration.Id, - string.IsNullOrEmpty(package.NormalizedVersion) ? - NuGetVersionFormatter.Normalize(package.Version) : - package.NormalizedVersion, format, extension); + NuGetVersionFormatter.GetNormalizedPackageVersion(package), + format, extension); } public static string BuildFileName(string id, string version, string pathTemplate, string extension) diff --git a/src/NuGetGallery.Services/Configuration/FeatureFlagService.cs b/src/NuGetGallery.Services/Configuration/FeatureFlagService.cs index a282991271..91aa9eaef9 100644 --- a/src/NuGetGallery.Services/Configuration/FeatureFlagService.cs +++ b/src/NuGetGallery.Services/Configuration/FeatureFlagService.cs @@ -47,6 +47,7 @@ public class FeatureFlagService : IFeatureFlagService private const string LicenseMdRenderingFlightName = GalleryPrefix + "LicenseMdRendering"; private const string MarkdigMdRenderingFlightName = GalleryPrefix + "MarkdigMdRendering"; private const string MarkdigMdSyntaxHighlightFlightName = GalleryPrefix + "MarkdigMdSyntaxHighlight"; + private const string DisplayUploadWarningV2FlightName = GalleryPrefix + "DisplayUploadWarningV2"; private const string DeletePackageApiFlightName = GalleryPrefix + "DeletePackageApi"; private const string ImageAllowlistFlightName = GalleryPrefix + "ImageAllowlist"; private const string DisplayBannerFlightName = GalleryPrefix + "Banner"; @@ -341,6 +342,11 @@ public bool IsMarkdigMdSyntaxHighlightEnabled() return _client.IsEnabled(MarkdigMdSyntaxHighlightFlightName, defaultValue: false); } + public bool IsDisplayUploadWarningV2Enabled(User user) + { + return _client.IsEnabled(DisplayUploadWarningV2FlightName, user, defaultValue: false); + } + public bool IsDeletePackageApiEnabled(User user) { return _client.IsEnabled(DeletePackageApiFlightName, user, defaultValue: false); diff --git a/src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs b/src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs index 89efc2d115..7fbca115ff 100644 --- a/src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs +++ b/src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs @@ -268,6 +268,11 @@ public interface IFeatureFlagService /// bool IsMarkdigMdSyntaxHighlightEnabled(); + /// + /// Whether the new warning of the verfiy metadata when upload package is enabled. + /// + bool IsDisplayUploadWarningV2Enabled(User user); + /// /// Whether or not the user can delete a package through the API. /// diff --git a/src/NuGetGallery.Services/SupportRequest/SupportRequestService.cs b/src/NuGetGallery.Services/SupportRequest/SupportRequestService.cs index 3fcb2662ec..591c8fe695 100644 --- a/src/NuGetGallery.Services/SupportRequest/SupportRequestService.cs +++ b/src/NuGetGallery.Services/SupportRequest/SupportRequestService.cs @@ -201,7 +201,7 @@ public async Task UpdateIssueAsync(int issueId, int? assignedToId, int issueStat newIssue.CreatedBy = loggedInUser; newIssue.OwnerEmail = requestorEmailAddress; newIssue.PackageId = package?.PackageRegistration.Id; - newIssue.PackageVersion = package?.Version; + newIssue.PackageVersion = NuGetVersionFormatter.GetNormalizedPackageVersion(package); newIssue.Reason = reason; newIssue.SiteRoot = _siteRoot; newIssue.UserKey = user?.Key; diff --git a/src/NuGetGallery/App_Data/Files/Content/flags.json b/src/NuGetGallery/App_Data/Files/Content/flags.json index 9da692110d..6295c2ccbe 100644 --- a/src/NuGetGallery/App_Data/Files/Content/flags.json +++ b/src/NuGetGallery/App_Data/Files/Content/flags.json @@ -108,6 +108,12 @@ "SiteAdmins": false, "Accounts": [], "Domains": [] + }, + "NuGetGallery.DisplayUploadWarningV2": { + "All": true, + "SiteAdmins": false, + "Accounts": [], + "Domains": [] } } } \ No newline at end of file diff --git a/src/NuGetGallery/Controllers/OrganizationsController.cs b/src/NuGetGallery/Controllers/OrganizationsController.cs index ae2ddc694b..979566ecfa 100644 --- a/src/NuGetGallery/Controllers/OrganizationsController.cs +++ b/src/NuGetGallery/Controllers/OrganizationsController.cs @@ -68,7 +68,7 @@ protected override Task SendNewAccountEmailAsync(User account) var message = new NewAccountMessage( MessageServiceConfiguration, account, - Url.ConfirmOrganizationEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false)); + Url.ConfirmOrganizationEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false, supportEmail: true)); return MessageService.SendMessageAsync(message); } @@ -78,7 +78,7 @@ protected override Task SendEmailChangedConfirmationNoticeAsync(User account) var message = new EmailChangeConfirmationMessage( MessageServiceConfiguration, account, - Url.ConfirmOrganizationEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false)); + Url.ConfirmOrganizationEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false, supportEmail: true)); return MessageService.SendMessageAsync(message); } diff --git a/src/NuGetGallery/Controllers/PackagesController.cs b/src/NuGetGallery/Controllers/PackagesController.cs index 730fca2f8e..08a0ac1015 100644 --- a/src/NuGetGallery/Controllers/PackagesController.cs +++ b/src/NuGetGallery/Controllers/PackagesController.cs @@ -375,6 +375,7 @@ public virtual async Task UploadPackage() verifyRequest.LicenseExpressionSegments = GetLicenseExpressionSegmentsOrNull(packageMetadata.LicenseMetadata); verifyRequest.ReadmeFileContents = await GetReadmeFileContentsOrNullAsync(packageMetadata, packageArchiveReader); verifyRequest.IsMarkdigMdSyntaxHighlightEnabled = _featureFlagService.IsMarkdigMdSyntaxHighlightEnabled(); + verifyRequest.IsDisplayUploadWarningV2Enabled = _featureFlagService.IsDisplayUploadWarningV2Enabled(currentUser); model.InProgressUpload = verifyRequest; return View(model); @@ -658,6 +659,7 @@ private async Task UploadPackageInternal(PackageArchiveReader packag model.IsSymbolsPackage = isSymbolsPackageUpload; model.HasExistingAvailableSymbols = hasExistingSymbolsPackageAvailable; model.IsMarkdigMdSyntaxHighlightEnabled = _featureFlagService.IsMarkdigMdSyntaxHighlightEnabled(); + model.IsDisplayUploadWarningV2Enabled = _featureFlagService.IsDisplayUploadWarningV2Enabled(currentUser); model.Warnings.AddRange(packageContentData.Warnings.Select(w => new JsonValidationMessage(w))); model.LicenseFileContents = packageContentData.LicenseFileContents; @@ -1172,6 +1174,11 @@ public virtual ActionResult AtomFeed(string id, bool prerel = true) [HttpGet] public virtual async Task License(string id, string version) { + if (version == null) + { + return HttpNotFound(); + } + var package = _packageService.FindPackageByIdAndVersionStrict(id, version); if (package == null || package.PackageStatusKey == PackageStatus.Deleted) { diff --git a/src/NuGetGallery/NuGetGallery.csproj b/src/NuGetGallery/NuGetGallery.csproj index 3df6b2ae92..2f11378fe9 100644 --- a/src/NuGetGallery/NuGetGallery.csproj +++ b/src/NuGetGallery/NuGetGallery.csproj @@ -333,6 +333,8 @@ + + @@ -635,6 +637,11 @@ + + True + True + Strings.resx + @@ -1483,11 +1490,6 @@ - - Strings.resx - True - True - @@ -2058,8 +2060,8 @@ PublicResXFileCodeGenerator - Strings.Designer.cs Designer + Strings.Designer.cs diff --git a/src/NuGetGallery/RequestModels/SubmitPackageRequest.cs b/src/NuGetGallery/RequestModels/SubmitPackageRequest.cs index 8d21c789c3..dfe1e537b1 100644 --- a/src/NuGetGallery/RequestModels/SubmitPackageRequest.cs +++ b/src/NuGetGallery/RequestModels/SubmitPackageRequest.cs @@ -14,5 +14,7 @@ public class SubmitPackageRequest public bool IsUserLocked { get; set; } public bool IsMarkdigMdSyntaxHighlightEnabled { get; set; } + + public bool IsDisplayUploadWarningV2Enabled { get; set; } } } \ No newline at end of file diff --git a/src/NuGetGallery/RequestModels/VerifyPackageRequest.cs b/src/NuGetGallery/RequestModels/VerifyPackageRequest.cs index af291499d8..e28938c776 100644 --- a/src/NuGetGallery/RequestModels/VerifyPackageRequest.cs +++ b/src/NuGetGallery/RequestModels/VerifyPackageRequest.cs @@ -119,6 +119,7 @@ public VerifyPackageRequest(PackageMetadata packageMetadata, IEnumerable p public bool IsSymbolsPackage { get; set; } public bool HasExistingAvailableSymbols { get; set; } public bool IsMarkdigMdSyntaxHighlightEnabled { get; set; } + public bool IsDisplayUploadWarningV2Enabled { get; set; } public List Warnings { get; set; } = new List(); diff --git a/src/NuGetGallery/Scripts/gallery/async-file-upload.js b/src/NuGetGallery/Scripts/gallery/async-file-upload.js index e1ef85c435..41f888c0c5 100644 --- a/src/NuGetGallery/Scripts/gallery/async-file-upload.js +++ b/src/NuGetGallery/Scripts/gallery/async-file-upload.js @@ -303,6 +303,11 @@ $("#verify-collapser-container").removeClass("hidden"); $("#submit-collapser-container").removeClass("hidden"); + if (model != null && model.IsDisplayUploadWarningV2Enabled) { + $('#upload-package-form').collapse('hide'); + $('#warning-container').addClass('hidden'); + } + window.nuget.configureExpanderHeading("verify-package-section"); window.nuget.configureExpanderHeading("submit-package-form"); } diff --git a/src/NuGetGallery/Services/MissingLicenseValidationMessageV2.cs b/src/NuGetGallery/Services/MissingLicenseValidationMessageV2.cs new file mode 100644 index 0000000000..5d94464508 --- /dev/null +++ b/src/NuGetGallery/Services/MissingLicenseValidationMessageV2.cs @@ -0,0 +1,38 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Globalization; +using System.Web; + +namespace NuGetGallery +{ + /// + /// A warning for packages that are missing licensing metadata. + /// + public class MissingLicenseValidationMessageV2 : IValidationMessage + { + private string DocumentationLink => $"{Strings.UploadPackage_LearnMore_PackagingLicenseV2}."; + + private readonly string _baseMessage; + + public MissingLicenseValidationMessageV2(string basePlainTextMessage) + { + if (string.IsNullOrWhiteSpace(basePlainTextMessage)) + { + throw new ArgumentException( + string.Format(CultureInfo.CurrentCulture, Strings.ParameterCannotBeNullOrEmpty, nameof(basePlainTextMessage)), nameof(basePlainTextMessage)); + } + + _baseMessage = basePlainTextMessage; + PlainTextMessage = $"{_baseMessage} {Strings.UploadPackage_LearnMore_PackagingLicenseV2}: https://aka.ms/nuget/authoring-best-practices#licensing."; + } + + public string PlainTextMessage { get; } + + public bool HasRawHtmlRepresentation => true; + + public string RawHtmlMessage + => Strings.UploadPackage_LicenseShouldBeSpecifiedHtmlV2 + " " + DocumentationLink; + } +} \ No newline at end of file diff --git a/src/NuGetGallery/Services/PackageMetadataValidationService.cs b/src/NuGetGallery/Services/PackageMetadataValidationService.cs index df018d9722..5f66faa866 100644 --- a/src/NuGetGallery/Services/PackageMetadataValidationService.cs +++ b/src/NuGetGallery/Services/PackageMetadataValidationService.cs @@ -152,7 +152,7 @@ public class PackageMetadataValidationService : IPackageMetadataValidationServic return result; } - result = await CheckReadmeMetadataAsync(nuGetPackage, currentUser); + result = await CheckReadmeMetadataAsync(nuGetPackage, warnings, currentUser); if (result != null) { return result; @@ -211,7 +211,14 @@ private async Task CheckLicenseMetadataAsync(PackageArc } else { - warnings.Add(new MissingLicenseValidationMessage(Strings.UploadPackage_LicenseShouldBeSpecified)); + if (_featureFlagService.IsDisplayUploadWarningV2Enabled(user)) + { + warnings.Add(new MissingLicenseValidationMessageV2(Strings.UploadPackage_LicenseShouldBeSpecifiedV2)); + } + else + { + warnings.Add(new MissingLicenseValidationMessage(Strings.UploadPackage_LicenseShouldBeSpecified)); + } } } @@ -415,13 +422,18 @@ private async Task CheckIconMetadataAsync(PackageArchiv return null; } - private async Task CheckReadmeMetadataAsync(PackageArchiveReader nuGetPackage, User user) + private async Task CheckReadmeMetadataAsync(PackageArchiveReader nuGetPackage, List warnings, User user) { var nuspecReader = GetNuspecReader(nuGetPackage); var readmeElement = nuspecReader.ReadmeElement; if (readmeElement == null) { + if (_featureFlagService.IsDisplayUploadWarningV2Enabled(user)) + { + warnings.Add(new UploadPackageMissingReadme()); + } + return null; } diff --git a/src/NuGetGallery/Services/UploadPackageMissingReadme.cs b/src/NuGetGallery/Services/UploadPackageMissingReadme.cs new file mode 100644 index 0000000000..906d1c0299 --- /dev/null +++ b/src/NuGetGallery/Services/UploadPackageMissingReadme.cs @@ -0,0 +1,12 @@ +namespace NuGetGallery +{ + /// + /// Represents a package ID reservation conflict + /// + public class UploadPackageMissingReadme : IValidationMessage + { + public bool HasRawHtmlRepresentation => true; + public string PlainTextMessage => Strings.UploadPackage_MissingReadme; + public string RawHtmlMessage => Strings.UploadPackage_MissingReadmeHtml; + } +} \ No newline at end of file diff --git a/src/NuGetGallery/Strings.Designer.cs b/src/NuGetGallery/Strings.Designer.cs index 3e56acf9b7..b2ce9caa57 100644 --- a/src/NuGetGallery/Strings.Designer.cs +++ b/src/NuGetGallery/Strings.Designer.cs @@ -2732,6 +2732,15 @@ public class Strings { } } + /// + /// Looks up a localized string similar to See how to include a license within the package. + /// + public static string UploadPackage_LearnMore_PackagingLicenseV2 { + get { + return ResourceManager.GetString("UploadPackage_LearnMore_PackagingLicenseV2", resourceCulture); + } + } + /// /// Looks up a localized string similar to Specifying <licenseUrl> in the package metadata is no longer allowed, please specify the license in the package.. /// @@ -2777,6 +2786,24 @@ public class Strings { } } + /// + /// Looks up a localized string similar to <strong>License</strong> missing.. + /// + public static string UploadPackage_LicenseShouldBeSpecifiedHtmlV2 { + get { + return ResourceManager.GetString("UploadPackage_LicenseShouldBeSpecifiedHtmlV2", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to License missing.. + /// + public static string UploadPackage_LicenseShouldBeSpecifiedV2 { + get { + return ResourceManager.GetString("UploadPackage_LicenseShouldBeSpecifiedV2", resourceCulture); + } + } + /// /// Looks up a localized string similar to The package contains a malformed license URL.. /// @@ -2804,6 +2831,24 @@ public class Strings { } } + /// + /// Looks up a localized string similar to Readme missing. Go to https://learn.microsoft.com/en-us/nuget/create-packages/package-authoring-best-practices#readme learn How to include a readme file within the package.. + /// + public static string UploadPackage_MissingReadme { + get { + return ResourceManager.GetString("UploadPackage_MissingReadme", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to <strong>Readme</strong> missing.<a href="https://learn.microsoft.com/en-us/nuget/create-packages/package-authoring-best-practices#readme"> See how to include a readme file within the package</a>, or add it as you upload.. + /// + public static string UploadPackage_MissingReadmeHtml { + get { + return ResourceManager.GetString("UploadPackage_MissingReadmeHtml", resourceCulture); + } + } + /// /// Looks up a localized string similar to User '{0}' does not have the rights to upload a package with a new ID on behalf of user '{1}'.. /// diff --git a/src/NuGetGallery/Strings.resx b/src/NuGetGallery/Strings.resx index f32f950789..4e4c4bdc0d 100644 --- a/src/NuGetGallery/Strings.resx +++ b/src/NuGetGallery/Strings.resx @@ -538,6 +538,12 @@ For more information, please contact '{2}'. The Documentation URL must be a raw Markdown file hosted on GitHub. + + <strong>Readme</strong> missing.<a href="https://learn.microsoft.com/en-us/nuget/create-packages/package-authoring-best-practices#readme"> See how to include a readme file within the package</a>, or add it as you upload. + + + Readme missing. Go to https://learn.microsoft.com/en-us/nuget/create-packages/package-authoring-best-practices#readme learn How to include a readme file within the package. + The user '{0}' is now an owner of the prefix '{1}'. @@ -1070,11 +1076,17 @@ The {1} Team Learn more + + + <strong>License</strong> missing. + + + License missing. All published packages should have license information specified. - + License expression must only contain licenses that are approved by Open Source Initiative or Free Software Foundation. Unsupported licenses: {0}. {0} is list of the package license ids that are neither OSI nor FSF approved @@ -1190,6 +1202,9 @@ The {1} Team The readme is not editable with the package has the embedded readme + + See how to include a license within the package + Learn more about including a license within the package diff --git a/src/NuGetGallery/Views/Packages/DisplayPackage.cshtml b/src/NuGetGallery/Views/Packages/DisplayPackage.cshtml index 52ad0e73c2..db61f93436 100644 --- a/src/NuGetGallery/Views/Packages/DisplayPackage.cshtml +++ b/src/NuGetGallery/Views/Packages/DisplayPackage.cshtml @@ -97,6 +97,13 @@ { packageManagers = new PackageManagerViewModel[] { + new PackageManagerViewModel(".NET CLI") + { + Id = "dotnet-cli", + CommandPrefix = "> ", + InstallPackageCommands = new [] { string.Format("dotnet add package {0} --version {1}", Model.Id, Model.Version) }, + }, + new PackageManagerViewModel("Package Manager") { Id = "package-manager", @@ -107,13 +114,6 @@ }, - new PackageManagerViewModel(".NET CLI") - { - Id = "dotnet-cli", - CommandPrefix = "> ", - InstallPackageCommands = new [] { string.Format("dotnet add package {0} --version {1}", Model.Id, Model.Version) }, - }, - new PackageManagerViewModel("PackageReference") { Id = "package-reference", diff --git a/src/NuGetGallery/Views/Packages/_VerifyMetadata.cshtml b/src/NuGetGallery/Views/Packages/_VerifyMetadata.cshtml index 514f802e8d..645afa9398 100644 --- a/src/NuGetGallery/Views/Packages/_VerifyMetadata.cshtml +++ b/src/NuGetGallery/Views/Packages/_VerifyMetadata.cshtml @@ -27,7 +27,7 @@ -

(none specified)

+

(none specified)

@@ -58,25 +58,58 @@
- @ViewHelpers.AlertWarning( - @ - We found the following issue(s): -
    - -
  • - - - -
  • - - -
  • - - -
- We recommend that you fix these issues and upload a new package. To learn more about authoring great packages, view our - Best Practices page.
-
) + + @ViewHelpers.AlertWarning( + @ + We found the following issue(s): +
    + +
  • + + + +
  • + + +
  • + + +
+ We recommend that you fix these issues and upload a new package. To learn more about authoring great packages, view our + Best Practices page.
+
) + + +
+ + We found the following issue(s) with the package. We recommend that you fix these issues and re-upload the package: +
    + +
  • + + +
  • + + + +
  • + + +
  • + + +
  • + + +
  • + + +
+ To learn more about authoring great packages, view our + Best Practices page.
+
+
+
@@ -112,25 +145,24 @@
+
-
+

- +

-
-
@@ -148,38 +180,38 @@
- - +
-
+

- +
-
+

- + +
-
+

+
- +
- -

(none specified)

-
+ -
+

+
@@ -187,10 +219,12 @@
+
(shown on package search page)
-
+

+
(for this version)
@@ -212,14 +246,13 @@
- +
-
+

-
@@ -228,11 +261,12 @@
- +
-
+

+
@@ -244,10 +278,10 @@
- +
-
+

diff --git a/src/NuGetGallery/Web.config b/src/NuGetGallery/Web.config index 02743e762a..99ad9340ca 100644 --- a/src/NuGetGallery/Web.config +++ b/src/NuGetGallery/Web.config @@ -85,7 +85,7 @@ - + diff --git a/src/VerifyGitHubVulnerabilities/Job.cs b/src/VerifyGitHubVulnerabilities/Job.cs index ce1a2d49ff..a80ca8498b 100644 --- a/src/VerifyGitHubVulnerabilities/Job.cs +++ b/src/VerifyGitHubVulnerabilities/Job.cs @@ -25,23 +25,30 @@ namespace VerifyGitHubVulnerabilities { class Job : JsonConfigurationJob { - private readonly HttpClient _client = new HttpClient(); - public override async Task Run() { - Console.WriteLine("Fetching vulnerabilities from GitHub..."); + var telemetryClient = _serviceProvider.GetRequiredService(); + + Logger.LogInformation("Fetching vulnerabilities from GitHub..."); var advisoryQueryService = _serviceProvider.GetRequiredService(); var advisories = await advisoryQueryService.GetAdvisoriesSinceAsync(DateTimeOffset.MinValue, CancellationToken.None); - Console.WriteLine($" FOUND {advisories.Count} advisories."); + Logger.LogInformation("Found {Count} advisories.", advisories.Count); - Console.WriteLine("Fetching vulnerabilities from DB..."); + Logger.LogInformation("Fetching vulnerabilities from DB..."); var ingestor = _serviceProvider.GetRequiredService(); await ingestor.IngestAsync(advisories.OrderBy(x => x.DatabaseId).ToList()); var verifier = _serviceProvider.GetRequiredService(); - Console.WriteLine(verifier.HasErrors ? - "DB does not match GitHub API - see stderr output for details" : - "DB/metadata matches GitHub API!"); + if (verifier.HasErrors) + { + Logger.LogError("DB/metadata does not match GitHub API - see error logs for details"); + telemetryClient.TrackMetric(nameof(VerifyGitHubVulnerabilities) + ".DataIsInconsistent", 1); + } + else + { + Logger.LogInformation("DB/metadata matches GitHub API!"); + telemetryClient.TrackMetric(nameof(VerifyGitHubVulnerabilities) + ".DataIsConsistent", 1); + } } protected override void ConfigureJobServices(IServiceCollection services, IConfigurationRoot configurationRoot) @@ -95,7 +102,7 @@ protected void ConfigureGalleryServices(ContainerBuilder containerBuilder) protected void ConfigureQueryServices(ContainerBuilder containerBuilder) { containerBuilder - .RegisterInstance(_client) + .RegisterInstance(new HttpClient()) .As(); containerBuilder diff --git a/src/VerifyGitHubVulnerabilities/Program.cs b/src/VerifyGitHubVulnerabilities/Program.cs index a3a88d5bb0..fdc886ce9a 100644 --- a/src/VerifyGitHubVulnerabilities/Program.cs +++ b/src/VerifyGitHubVulnerabilities/Program.cs @@ -10,7 +10,7 @@ class Program static void Main(string[] args) { var job = new Job(); - JobRunner.RunOnce(job, args).Wait(); + JobRunner.Run(job, args).Wait(); } } } diff --git a/src/VerifyGitHubVulnerabilities/README.md b/src/VerifyGitHubVulnerabilities/README.md index 2d1ad79c59..e4cf550a3e 100644 --- a/src/VerifyGitHubVulnerabilities/README.md +++ b/src/VerifyGitHubVulnerabilities/README.md @@ -1,28 +1,23 @@ ## Overview -This is a run-once job for taking a current snapshot of the whole of GitHub's security advisory collection (pertaining to the `NUGET` ecosystem) and comparing it with the vulnerability records in the gallery database. It's designed to be run on an ad hoc basis (not an ongoing monitoring job), from the command line, with all status message streamed to `stdout` and all vulnerability differences streamed to `stderr`. +This is a recurring, monitoring job for taking a current snapshot of the whole of GitHub's security advisory collection (pertaining to the `NUGET` ecosystem) and comparing it with the vulnerability records in the gallery database. It's designed to interact with GitHub, the NuGet Gallery DB, and V3 metadata endpoints in a read-only manner. It just reports problems when it finds them. -## Running the job +## Running the job locally A typical command line will look like this: ``` -verifygithubvulnerabilities.exe -Configuration appsettings.json -InstrumentationKey -HeartbeatIntervalSeconds 60 2>c:\errors.txt +VerifyGitHubVulnerabilities.exe -Configuration appsettings.json -InstrumentationKey -HeartbeatIntervalSeconds 60 ``` -- the `2>` will direct `stderr` to a text file (a `1>` would similarly direct `stdout` but it's probably useful to have that print to console) -- the contents of the `appsettings.json` file will be similar (identical is fine--there are just some extra unneeded settings in them) to the settings files used for `GitHubVulnerabilities2Db`. - ### Using DEV resources The easiest way to run the tool if you are on the nuget.org team is to use the DEV environment resources: 1. Install the certificate used to authenticate as our client AAD app registration into your `CurrentUser` certificate store. 1. Clone our internal [`NuGetDeployment`](https://nuget.visualstudio.com/DefaultCollection/NuGetMicrosoft/_git/NuGetDeploymentp) repository. -1. Take a copy of the [DEV GitHubVulnerabilities2Db appsettings.json](https://nuget.visualstudio.com/NuGetMicrosoft/_git/NuGetDeployment?path=%2Fsrc%2FJobs%2FNuGet.Jobs.Cloud%2FJobs%2FGitHubVulnerabilities2Db%2FDEV%2Fnorthcentralus%2Fappsettings.json) file and place it in the same directory as the `verifygithubvulnerabilities.exe`. This will use our secrets to authenticate to the SQL server (this file also contains a reference to the secret used for the access token to GitHub). -1. Set the following property, in the `appsettings.json`, under the `"Initialization"` object: `"NuGetV3Index": "https://apidev.nugettest.org/v3/index.json"`. -1. Overwrite the Gallery DB `"ConnectionString"` property to be the connection string from [DEV USSC (read-only) gallery config](https://nuget.visualstudio.com/NuGetMicrosoft/_git/NuGetDeployment?path=/src/Gallery/ExpressV2/ServiceSpecs/Parameters.AS/DEV/USSC/NuGet.Gallery.Parameters.json&version=GBmaster). This provides an additional protection to ensure the job runs as read-only. -2. Run as per above. +1. Take a copy of the [DEV VerifyGitHubVulnerabilities appsettings.json](https://nuget.visualstudio.com/NuGetMicrosoft/_git/NuGetDeployment?path=%2Fsrc%2FJobs%2FNuGet.Jobs.Cloud%2FJobs%VerifyGitHubVulnerabilities%2FDEV%2Fnorthcentralus%2Fappsettings.json) file and place it in the same directory as the `VerifyGitHubVulnerabilities.exe`. This will use our secrets to authenticate to the SQL server (this file also contains a reference to the secret used for the access token to GitHub). +1. Run as per above. ## Algorithm diff --git a/src/VerifyGitHubVulnerabilities/Scripts/Functions.ps1 b/src/VerifyGitHubVulnerabilities/Scripts/Functions.ps1 new file mode 100644 index 0000000000..b7e8663b2c --- /dev/null +++ b/src/VerifyGitHubVulnerabilities/Scripts/Functions.ps1 @@ -0,0 +1,41 @@ +Function Uninstall-NuGetService() { + Param ([string]$ServiceName) + + if (Get-Service $ServiceName -ErrorAction SilentlyContinue) + { + Write-Host Removing service $ServiceName... + Stop-Service $ServiceName -Force + sc.exe delete $ServiceName + Write-Host Removed service $ServiceName. + } else { + Write-Host Skipping removal of service $ServiceName - no such service exists. + } +} + +Function Install-NuGetService() { + Param ( + [string]$ServiceName, + [string]$ServiceTitle, + [string]$ScriptToRun, + [Parameter(Mandatory=$false)][string]$Username, + [Parameter(Mandatory=$false)][string]$Password + ) + + Write-Host Installing service $ServiceName... + + $installService = "nssm install $ServiceName $ScriptToRun" + cmd /C $installService + + Set-Service -Name $ServiceName -DisplayName "$ServiceTitle - $ServiceName" -Description "Runs $ServiceTitle." -StartupType Automatic + sc.exe failure $ServiceName reset= 30 actions= restart/5000 + + if ($Username) { + Write-Host Running service under specific credentials. + sc.exe config "$ServiceName" obj= "$Username" password= "$Password" + } + + # Run service + net start $ServiceName + + Write-Host Installed service $ServiceName. +} diff --git a/src/VerifyGitHubVulnerabilities/Scripts/PostDeploy.ps1 b/src/VerifyGitHubVulnerabilities/Scripts/PostDeploy.ps1 new file mode 100644 index 0000000000..d56a30b1ca --- /dev/null +++ b/src/VerifyGitHubVulnerabilities/Scripts/PostDeploy.ps1 @@ -0,0 +1,20 @@ +. .\Functions.ps1 + +$jobsToInstall = $OctopusParameters["Jobs.ServiceNames"].Split("{,}") + +Write-Host Installing services... + +$currentDirectory = [string](Get-Location) + +$jobsToInstall.Split("{;}") | %{ + $serviceName = $_ + $serviceTitle = $OctopusParameters["Jobs.$serviceName.Title"] + $serviceUsername = $OctopusParameters["Jobs.$serviceName.Username"] + $servicePassword = $OctopusParameters["Jobs.$serviceName.Password"] + $scriptToRun = $OctopusParameters["Jobs.$serviceName.Script"] + $scriptToRun = "$currentDirectory\$scriptToRun" + + Install-NuGetService -ServiceName $serviceName -ServiceTitle $serviceTitle -ScriptToRun $scriptToRun -Username $serviceUsername -Password $servicePassword +} + +Write-Host Installed services. \ No newline at end of file diff --git a/src/VerifyGitHubVulnerabilities/Scripts/PreDeploy.ps1 b/src/VerifyGitHubVulnerabilities/Scripts/PreDeploy.ps1 new file mode 100644 index 0000000000..ef711a9125 --- /dev/null +++ b/src/VerifyGitHubVulnerabilities/Scripts/PreDeploy.ps1 @@ -0,0 +1,11 @@ +. .\Functions.ps1 + +$jobsToInstall = $OctopusParameters["Jobs.ServiceNames"].Split("{,}") + +Write-Host Removing services... + +$jobsToInstall.Split("{;}") | %{ + Uninstall-NuGetService -ServiceName $_ +} + +Write-Host Removed services. \ No newline at end of file diff --git a/src/VerifyGitHubVulnerabilities/Scripts/nssm.exe b/src/VerifyGitHubVulnerabilities/Scripts/nssm.exe new file mode 100644 index 0000000000..6ccfe3cfb8 Binary files /dev/null and b/src/VerifyGitHubVulnerabilities/Scripts/nssm.exe differ diff --git a/src/VerifyGitHubVulnerabilities/Verify/PackageVulnerabilitiesVerifier.cs b/src/VerifyGitHubVulnerabilities/Verify/PackageVulnerabilitiesVerifier.cs index 3c63a70b82..5965bd826b 100644 --- a/src/VerifyGitHubVulnerabilities/Verify/PackageVulnerabilitiesVerifier.cs +++ b/src/VerifyGitHubVulnerabilities/Verify/PackageVulnerabilitiesVerifier.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Logging; using NuGet.Configuration; using NuGet.Protocol; using NuGet.Protocol.Core.Types; @@ -21,14 +22,17 @@ public class PackageVulnerabilitiesVerifier : IPackageVulnerabilitiesVerifier { private readonly VerifyGitHubVulnerabilitiesConfiguration _configuration; private readonly IEntitiesContext _entitiesContext; + private readonly ILogger _logger; private Lazy> _packageMetadataResource; private Dictionary> _packageMetadata; private static readonly SemaphoreSlim semaphoreSlim = new SemaphoreSlim(1); - public PackageVulnerabilitiesVerifier(VerifyGitHubVulnerabilitiesConfiguration configuration, - IEntitiesContext entitiesContext) + public PackageVulnerabilitiesVerifier( + VerifyGitHubVulnerabilitiesConfiguration configuration, + IEntitiesContext entitiesContext, + ILogger logger) { _configuration = configuration; if (_configuration.VerifyDatabase) @@ -38,6 +42,7 @@ public class PackageVulnerabilitiesVerifier : IPackageVulnerabilitiesVerifier _packageMetadata = new Dictionary>(); _packageMetadataResource = new Lazy>(InitializeMetadataResourceAsync); + _logger = logger; } public bool HasErrors { get; private set; } @@ -51,7 +56,7 @@ public Task UpdateVulnerabilityAsync(PackageVulnerability vulnerability, bool wi { if (vulnerability == null) { - Console.Error.WriteLine("Null vulnerability passed to verifier! Continuing..."); + _logger.LogWarning("Null vulnerability passed to verifier! Continuing..."); return Task.CompletedTask; } @@ -73,7 +78,10 @@ public Task UpdateVulnerabilityAsync(PackageVulnerability vulnerability, bool wi private void VerifyVulnerabilityInDatabase(PackageVulnerability vulnerability, bool withdrawn) { - Console.WriteLine($"[Database] Verifying vulnerability {vulnerability.GitHubDatabaseKey}."); + _logger.LogInformation( + "[Database] Verifying vulnerability {GitHubDatabaseKey} (advisory URL: {AdvisoryUrl}).", + vulnerability.GitHubDatabaseKey, + vulnerability.AdvisoryUrl); var existingVulnerability = _entitiesContext.Vulnerabilities .Include(v => v.AffectedRanges) @@ -83,9 +91,9 @@ private void VerifyVulnerabilityInDatabase(PackageVulnerability vulnerability, b { if (existingVulnerability != null) { - Console.Error.WriteLine(withdrawn ? - $@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey} was withdrawn and should not be in DB!" : - $@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey} affects no packages and should not be in DB!"); + _logger.LogError(withdrawn ? + "[Database] Vulnerability advisory {GitHubDatabaseKey} was withdrawn and should not be in DB!" : + "[Database] Vulnerability advisory {GitHubDatabaseKey} affects no packages and should not be in DB!", vulnerability.GitHubDatabaseKey); HasErrors = true; } @@ -94,48 +102,58 @@ private void VerifyVulnerabilityInDatabase(PackageVulnerability vulnerability, b if (existingVulnerability == null) { - Console.Error.WriteLine($"[Database] Cannot find vulnerability {vulnerability.GitHubDatabaseKey} in DB!"); + _logger.LogError("[Database] Cannot find vulnerability {GitHubDatabaseKey} in DB!", vulnerability.GitHubDatabaseKey); HasErrors = true; return; } if (existingVulnerability.Severity != vulnerability.Severity) { - Console.Error.WriteLine( - $@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey - }, severity does not match! GitHub: {vulnerability.Severity}, DB: {existingVulnerability.Severity}"); + _logger.LogError( + "[Database] Vulnerability advisory {GitHubDatabaseKey}, severity does not match! GitHub: {GitHubSeverity}, DB: {DbSeverity}", + vulnerability.GitHubDatabaseKey, + vulnerability.Severity, + existingVulnerability.Severity); HasErrors = true; } if (existingVulnerability.AdvisoryUrl != vulnerability.AdvisoryUrl) { - Console.Error.WriteLine( - $@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey - }, advisory URL does not match! GitHub: {vulnerability.AdvisoryUrl}, DB: { existingVulnerability.AdvisoryUrl}"); + _logger.LogError( + "[Database] Vulnerability advisory {GitHubDatabaseKey}, advisory URL does not match! GitHub: {GitHubAdvisoryUrl}, DB: {DbAdvisoryUrl}", + vulnerability.GitHubDatabaseKey, + vulnerability.AdvisoryUrl, + existingVulnerability.AdvisoryUrl); HasErrors = true; } foreach (var range in vulnerability.AffectedRanges) { - Console.WriteLine($"[Database] Verifying range affecting {range.PackageId} {range.PackageVersionRange}."); + _logger.LogInformation("[Database] Verifying range affecting {PackageId} {PackageVersionRange}.", range.PackageId, range.PackageVersionRange); var existingRange = existingVulnerability.AffectedRanges .SingleOrDefault(r => r.PackageId == range.PackageId && r.PackageVersionRange == range.PackageVersionRange); if (existingRange == null) { - Console.Error.WriteLine( - $@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey - }, cannot find range {range.PackageId} {range.PackageVersionRange} in DB!"); + _logger.LogError( + "[Database] Vulnerability advisory {GitHubDatabaseKey}, cannot find range {PackageId} {PackageVersionRange} in DB!", + vulnerability.GitHubDatabaseKey, + range.PackageId, + range.PackageVersionRange); HasErrors = true; continue; } if (existingRange.FirstPatchedPackageVersion != range.FirstPatchedPackageVersion) { - Console.Error.WriteLine( - $@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey - }, range {range.PackageId} {range.PackageVersionRange}, first patched version does not match! GitHub: { - range.FirstPatchedPackageVersion}, DB: {range.FirstPatchedPackageVersion}"); + _logger.LogError( + "[Database] Vulnerability advisory {GitHubDatabaseKey}, range {PackageId} {PackageVersionRange}, " + + "first patched version does not match! GitHub: {GitHubFirstPatchedPackageVersion}, DB: {DbFirstPatchedPackageVersion}", + vulnerability.GitHubDatabaseKey, + range.PackageVersionRange, + range.PackageVersionRange, + range.FirstPatchedPackageVersion, + existingRange.FirstPatchedPackageVersion); HasErrors = true; } @@ -150,10 +168,13 @@ private void VerifyVulnerabilityInDatabase(PackageVulnerability vulnerability, b var version = NuGetVersion.Parse(package.NormalizedVersion); if (versionRange.Satisfies(version) != package.VulnerablePackageRanges.Contains(existingRange)) { - Console.Error.WriteLine( - $@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey - }, range {range.PackageId} {range.PackageVersionRange}, package {package.NormalizedVersion - } is not properly marked vulnerable to vulnerability!"); + _logger.LogError( + "[Database] Vulnerability advisory {GitHubDatabaseKey}, " + + "range {PackageId} {PackageVersionRange}, package {NormalizedVersion} is not properly marked vulnerable to vulnerability!", + vulnerability.GitHubDatabaseKey, + range.PackageId, + range.PackageVersionRange, + package.NormalizedVersion); HasErrors = true; } } @@ -162,7 +183,10 @@ private void VerifyVulnerabilityInDatabase(PackageVulnerability vulnerability, b private Task VerifyVulnerabilityInMetadataAsync(PackageVulnerability gitHubAdvisory) { - Console.WriteLine($"[Metadata] Verifying vulnerability {gitHubAdvisory.GitHubDatabaseKey}."); + _logger.LogInformation( + "[Metadata] Verifying vulnerability {GitHubDatabaseKey} (advisory URL: {AdvisoryUrl}).", + gitHubAdvisory.GitHubDatabaseKey, + gitHubAdvisory.AdvisoryUrl); if (gitHubAdvisory.AffectedRanges == null || !gitHubAdvisory.AffectedRanges.Any()) { @@ -235,9 +259,12 @@ private Task VerifyVulnerabilityInMetadataAsync(PackageVulnerability gitHubAdvis { if (!hasTheVulnerability) { - Console.Error.WriteLine( - $@"[Metadata] Vulnerability advisory {advisoryDatabaseKey - }, version {versionMetadata.Identity.Version} of package {packageId} is not marked vulnerable and is in a vulnerable range!"); + _logger.LogError( + "[Metadata] Vulnerability advisory {AdvisoryDatabaseKey}, version {Version} of package {PackageId} " + + "is not marked vulnerable and is in a vulnerable range!", + advisoryDatabaseKey, + versionMetadata.Identity.Version, + packageId); HasErrors = true; } @@ -246,9 +273,12 @@ private Task VerifyVulnerabilityInMetadataAsync(PackageVulnerability gitHubAdvis .FirstOrDefault(v => v.Severity != (int)advisorySeverity); if (firstSeverityMismatch != null) { - Console.Error.WriteLine( - $@"[Metadata] Vulnerability advisory {advisoryDatabaseKey - }, severities has at least one mismatch! GitHub: {advisorySeverity}, Metadata: {firstSeverityMismatch.Severity}"); + _logger.LogError( + "[Metadata] Vulnerability advisory {AdvisoryDatabaseKey}, severities has at least one mismatch! " + + "GitHub: {GitHubAdvisorySeverity}, Metadata: {FirstSeverityMismatchSeverity}", + advisoryDatabaseKey, + advisorySeverity, + firstSeverityMismatch.Severity); HasErrors = true; } } @@ -256,9 +286,12 @@ private Task VerifyVulnerabilityInMetadataAsync(PackageVulnerability gitHubAdvis { if (hasTheVulnerability) { - Console.Error.WriteLine( - $@"[Metadata] Vulnerability advisory {advisoryDatabaseKey - }, version {versionMetadata.Identity.Version} of package {packageId} is marked vulnerable and is not in a vulnerable range!"); + _logger.LogError( + "[Metadata] Vulnerability advisory {AdvisoryDatabaseKey}, version {Version} of package {PackageId} " + + "is marked vulnerable and is not in a vulnerable range!", + advisoryDatabaseKey, + versionMetadata.Identity.Version, + packageId); HasErrors = true; } } diff --git a/src/VerifyGitHubVulnerabilities/VerifyGitHubVulnerabilities.csproj b/src/VerifyGitHubVulnerabilities/VerifyGitHubVulnerabilities.csproj index c9352dccb8..4a9f5bf80a 100644 --- a/src/VerifyGitHubVulnerabilities/VerifyGitHubVulnerabilities.csproj +++ b/src/VerifyGitHubVulnerabilities/VerifyGitHubVulnerabilities.csproj @@ -54,6 +54,11 @@ + + + + + @@ -78,6 +83,9 @@ NuGetGallery.Services + + + ..\..\build $(BUILD_SOURCESDIRECTORY)\build diff --git a/src/VerifyGitHubVulnerabilities/VerifyGitHubVulnerabilities.nuspec b/src/VerifyGitHubVulnerabilities/VerifyGitHubVulnerabilities.nuspec index e7fae0b674..d199d33795 100644 --- a/src/VerifyGitHubVulnerabilities/VerifyGitHubVulnerabilities.nuspec +++ b/src/VerifyGitHubVulnerabilities/VerifyGitHubVulnerabilities.nuspec @@ -11,5 +11,10 @@ + + + + + \ No newline at end of file diff --git a/src/VerifyMicrosoftPackage/Fakes/FakeFeatureFlagService.cs b/src/VerifyMicrosoftPackage/Fakes/FakeFeatureFlagService.cs index f4482e2ae1..dbbbb94e1c 100644 --- a/src/VerifyMicrosoftPackage/Fakes/FakeFeatureFlagService.cs +++ b/src/VerifyMicrosoftPackage/Fakes/FakeFeatureFlagService.cs @@ -124,5 +124,7 @@ public class FakeFeatureFlagService : IFeatureFlagService public bool IsNewAccount2FAEnforcementEnabled() => throw new NotImplementedException(); public bool IsNuGetAccountPasswordLoginEnabled() => throw new NotImplementedException(); + + public bool IsDisplayUploadWarningV2Enabled(User user) => throw new NotImplementedException(); } } diff --git a/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs index 44ea19c9c2..b293c22d4c 100644 --- a/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs @@ -10071,6 +10071,21 @@ public async Task GivenInvalidPackageReturns404() Assert.IsType(result); } + [Fact] + public async Task GivenNullVersionReturns404() + { + // arrange + var controller = CreateController( + GetConfigurationService(), + packageService: _packageService); + + // act + var result = await controller.License(_packageId, version: null); + + // assert + Assert.IsType(result); + } + [Fact] public async Task GivenDeletedPackageReturns404() { diff --git a/tests/NuGetGallery.Facts/Services/PackageMetadataValidationServiceFacts.cs b/tests/NuGetGallery.Facts/Services/PackageMetadataValidationServiceFacts.cs index 2e4c575ce6..85b2f208e0 100644 --- a/tests/NuGetGallery.Facts/Services/PackageMetadataValidationServiceFacts.cs +++ b/tests/NuGetGallery.Facts/Services/PackageMetadataValidationServiceFacts.cs @@ -442,6 +442,50 @@ public async Task HandlesMissingLicenseAccordingToSettings(bool allowLicenseless } } + [Theory] + [InlineData(false, false)] + [InlineData(true, true)] + public async Task HandlesMissingLicenseAccordingToSettingsWhenDisplayUploadWarningV2Enabled(bool allowLicenselessPackages, bool expectedSuccess) + { + _nuGetPackage = GeneratePackageWithUserContent( + licenseUrl: null, + licenseExpression: null, + licenseFilename: null, + readmeFilename:"readme.md", + readmeFileContents: "read me"); + + _config + .Setup(x => x.AllowLicenselessPackages) + .Returns(allowLicenselessPackages); + _featureFlagService + .Setup(ffs => ffs.IsDisplayUploadWarningV2Enabled(_currentUser)) + .Returns(true); + _featureFlagService + .Setup(ffs => ffs.AreEmbeddedReadmesEnabled(It.IsAny())) + .Returns(true); + + var result = await _target.ValidateMetadataBeforeUploadAsync( + _nuGetPackage.Object, + GetPackageMetadata(_nuGetPackage), + _currentUser); + + if (!expectedSuccess) + { + Assert.Equal(PackageValidationResultType.Invalid, result.Type); + Assert.StartsWith("The package has no license information specified.", result.Message.PlainTextMessage); + Assert.IsType(result.Message); + Assert.Empty(result.Warnings); + } + else + { + Assert.Equal(PackageValidationResultType.Accepted, result.Type); + Assert.Null(result.Message); + Assert.Single(result.Warnings); + Assert.IsType(result.Warnings[0]); + Assert.StartsWith("License missing. See how to include a license within the package: https://aka.ms/nuget/authoring-best-practices#licensing.", result.Warnings[0].PlainTextMessage); + } + } + const string LicenseDeprecationUrl = "https://aka.ms/deprecateLicenseUrl"; const string RegularLicenseUrl = "https://example.com/license"; @@ -1436,6 +1480,57 @@ public async Task RejectsPackagesWithMissingReadmeFile() Assert.Empty(result.Warnings); } + [Fact] + public async Task WarnsAboutPackagesWithoutReadmeWhenDisplayUploadWarningV2Enabled() + { + _nuGetPackage = GeneratePackageWithUserContent( + licenseExpression: "MIT", + licenseUrl: new Uri("https://licenses.nuget.org/MIT")); + + _featureFlagService + .Setup(ffs => ffs.IsDisplayUploadWarningV2Enabled(_currentUser)) + .Returns(true); + + var result = await _target.ValidateMetadataBeforeUploadAsync( + _nuGetPackage.Object, + GetPackageMetadata(_nuGetPackage), + _currentUser); + + Assert.Equal(PackageValidationResultType.Accepted, result.Type); + Assert.Null(result.Message); + var warning = Assert.Single(result.Warnings); + Assert.IsType(warning); + Assert.StartsWith("Readme missing. Go to https://learn.microsoft.com/en-us/nuget/create-packages/package-authoring-best-practices#readme learn How to include a readme file within the package.", warning.PlainTextMessage); + Assert.StartsWith("Readme missing. See how to include a readme file within the package, or add it as you upload.", warning.RawHtmlMessage); + } + + [Fact] + public async Task WarnsAboutPackagesWithoutWhenEmbeddedReadmeNotEnabledAndDisplayUploadWarningV2Enabled() + { + _nuGetPackage = GeneratePackageWithUserContent( + licenseExpression: "MIT", + licenseUrl: new Uri("https://licenses.nuget.org/MIT")); + + _featureFlagService + .Setup(ffs => ffs.IsDisplayUploadWarningV2Enabled(_currentUser)) + .Returns(true); + _featureFlagService + .Setup(ffs => ffs.AreEmbeddedReadmesEnabled(It.IsAny())) + .Returns(false); + + var result = await _target.ValidateMetadataBeforeUploadAsync( + _nuGetPackage.Object, + GetPackageMetadata(_nuGetPackage), + _currentUser); + + Assert.Equal(PackageValidationResultType.Accepted, result.Type); + Assert.Null(result.Message); + var warning = Assert.Single(result.Warnings); + Assert.IsType(warning); + Assert.StartsWith("Readme missing. Go to https://learn.microsoft.com/en-us/nuget/create-packages/package-authoring-best-practices#readme learn How to include a readme file within the package.", warning.PlainTextMessage); + Assert.StartsWith("Readme missing. See how to include a readme file within the package, or add it as you upload.", warning.RawHtmlMessage); + } + private async Task ValidatePackageWithReadme(string readmePath, byte[] readmeFileData) { _nuGetPackage = GeneratePackageWithUserContent( @@ -1943,7 +2038,7 @@ public async Task RejectIsTyposquattingNewVersion() Assert.Empty(result.Warnings); } } - + public abstract class FactsBase { protected const string PackageId = "theId";