Add IsVulnerable to Manage Packages view model (#8435)

Expand view models for vulnerabilities in manage packages page

Author: drewgillies

src/NuGetGallery.Services/PackageManagement/IPackageService.cs

@@ -87,9 +87,9 @@ Package FilterLatestPackage(
         Package FilterLatestPackageBySuffix(IReadOnlyCollection<Package> packages,
             string version, bool prerelease);
 
-        IEnumerable<Package> FindPackagesByOwner(User user, bool includeUnlisted, bool includeVersions = false);
+        IEnumerable<Package> FindPackagesByOwner(User user, bool includeUnlisted, bool includeVersions = false, bool includeVulnerabilities = false);
 
-        IEnumerable<Package> FindPackagesByAnyMatchingOwner(User user, bool includeUnlisted, bool includeVersions = false);
+        IEnumerable<Package> FindPackagesByAnyMatchingOwner(User user, bool includeUnlisted, bool includeVersions = false, bool includeVulnerabilities = false);
 
         IQueryable<PackageRegistration> FindPackageRegistrationsByOwner(User user);
 

src/NuGetGallery.Services/PackageManagement/PackageService.cs

@@ -406,9 +406,12 @@ private static Package FilterLatestPackageHelper(
             return package;
         }
 
-        public IEnumerable<Package> FindPackagesByOwner(User user, bool includeUnlisted, bool includeVersions = false)
+        public IEnumerable<Package> FindPackagesByOwner(User user, 
+            bool includeUnlisted, 
+            bool includeVersions = false,
+            bool includeVulnerabilities = false)
         {
-            return GetPackagesForOwners(new[] { user.Key }, includeUnlisted, includeVersions);
+            return GetPackagesForOwners(new[] { user.Key }, includeUnlisted, includeVersions, includeVulnerabilities);
         }
 
         /// <summary>
@@ -417,15 +420,17 @@ public IEnumerable<Package> FindPackagesByOwner(User user, bool includeUnlisted,
         public IEnumerable<Package> FindPackagesByAnyMatchingOwner(
             User user,
             bool includeUnlisted,
-            bool includeVersions = false)
+            bool includeVersions = false,
+            bool includeVulnerabilities = false)
         {
             var ownerKeys = user.Organizations.Select(org => org.OrganizationKey).ToList();
             ownerKeys.Insert(0, user.Key);
 
-            return GetPackagesForOwners(ownerKeys, includeUnlisted, includeVersions);
+            return GetPackagesForOwners(ownerKeys, includeUnlisted, includeVersions, includeVulnerabilities);
         }
 
-        private IEnumerable<Package> GetPackagesForOwners(IEnumerable<int> ownerKeys, bool includeUnlisted, bool includeVersions)
+        private IEnumerable<Package> GetPackagesForOwners(IEnumerable<int> ownerKeys, bool includeUnlisted,
+            bool includeVersions, bool includeVulnerabilities)
         {
             IQueryable<Package> packages = _packageRepository.GetAll()
                 .Where(p => p.PackageRegistration.Owners.Any(o => ownerKeys.Contains(o.Key)));
@@ -437,7 +442,7 @@ private IEnumerable<Package> GetPackagesForOwners(IEnumerable<int> ownerKeys, bo
 
             if (!includeVersions)
             {
-                // Do a best effort of retrieving the latest version. Note that UpdateIsLatest has had concurrency issues
+                // Do a best effort of retrieving the latest versions. Note that UpdateIsLatest has had concurrency issues
                 // where sometimes packages no rows with IsLatest set. In this case, we'll just select the last inserted
                 // row (descending [Key]) as opposed to reading all rows into memory and sorting on NuGetVersion.
                 packages = packages
@@ -453,11 +458,17 @@ private IEnumerable<Package> GetPackagesForOwners(IEnumerable<int> ownerKeys, bo
                         .FirstOrDefault());
             }
 
-            return packages
+            var result = packages
                 .Include(p => p.PackageRegistration)
                 .Include(p => p.PackageRegistration.Owners)
-                .Include(p => p.PackageRegistration.RequiredSigners)
-                .ToList();
+                .Include(p => p.PackageRegistration.RequiredSigners);
+
+            if (includeVulnerabilities)
+            {
+                result = result.Include($"{nameof(Package.VulnerablePackageRanges)}.{nameof(VulnerablePackageVersionRange.Vulnerability)}");
+            }
+
+            return result.ToList();
         }
 
         public IQueryable<PackageRegistration> FindPackageRegistrationsByOwner(User user)

src/NuGetGallery/Controllers/UsersController.cs

@@ -35,6 +35,7 @@ public partial class UsersController
         private readonly ListPackageItemViewModelFactory _listPackageItemViewModelFactory;
         private readonly ISupportRequestService _supportRequestService;
         private readonly IFeatureFlagService _featureFlagService;
+        private readonly IPackageVulnerabilitiesService _packageVulnerabilitiesService;
 
         public UsersController(
             IUserService userService,
@@ -51,6 +52,7 @@ public UsersController(
             ICertificateService certificateService,
             IContentObjectService contentObjectService,
             IFeatureFlagService featureFlagService,
+            IPackageVulnerabilitiesService packageVulnerabilitiesService,
             IMessageServiceConfiguration messageServiceConfiguration,
             IIconUrlProvider iconUrlProvider,
             IGravatarProxyService gravatarProxy)
@@ -73,8 +75,10 @@ public UsersController(
             _credentialBuilder = credentialBuilder ?? throw new ArgumentNullException(nameof(credentialBuilder));
             _supportRequestService = supportRequestService ?? throw new ArgumentNullException(nameof(supportRequestService));
             _featureFlagService = featureFlagService ?? throw new ArgumentNullException(nameof(featureFlagService));
+            _packageVulnerabilitiesService = packageVulnerabilitiesService ?? throw new ArgumentNullException(nameof(packageVulnerabilitiesService));
 
-            _listPackageItemRequiredSignerViewModelFactory = new ListPackageItemRequiredSignerViewModelFactory(securityPolicyService, iconUrlProvider);
+            _listPackageItemRequiredSignerViewModelFactory = new ListPackageItemRequiredSignerViewModelFactory(
+                securityPolicyService, iconUrlProvider, packageVulnerabilitiesService);
             _listPackageItemViewModelFactory = new ListPackageItemViewModelFactory(iconUrlProvider);
         }
 
@@ -521,7 +525,8 @@ public virtual ActionResult Packages()
 
             var wasAADLoginOrMultiFactorAuthenticated = User.WasMultiFactorAuthenticated() || User.WasAzureActiveDirectoryAccountUsedForSignin();
 
-            var packages = PackageService.FindPackagesByAnyMatchingOwner(currentUser, includeUnlisted: true);
+            var packages = PackageService.FindPackagesByAnyMatchingOwner(
+                currentUser, includeUnlisted: true, includeVulnerabilities: true);
 
             var listedPackages = GetPackages(packages, currentUser, wasAADLoginOrMultiFactorAuthenticated,
                 p => p.Listed && p.PackageStatusKey == PackageStatus.Available);
@@ -559,8 +564,10 @@ public virtual ActionResult Packages()
                 OwnerRequests = ownerRequests,
                 ReservedNamespaces = reservedPrefixes,
                 WasMultiFactorAuthenticated = User.WasMultiFactorAuthenticated(),
-                IsCertificatesUIEnabled = ContentObjectService.CertificatesConfiguration?.IsUIEnabledForUser(currentUser) ?? false
+                IsCertificatesUIEnabled = ContentObjectService.CertificatesConfiguration?.IsUIEnabledForUser(currentUser) ?? false,
+                IsPackageVulnerabilitiesEnabled = _featureFlagService.IsDisplayVulnerabilitiesEnabled()
             };
+
             return View(model);
         }
 

src/NuGetGallery/Helpers/ViewModelExtensions/ListPackageItemRequiredSignerViewModelFactory.cs

@@ -13,11 +13,16 @@ public class ListPackageItemRequiredSignerViewModelFactory
     {
         private readonly ListPackageItemViewModelFactory _listPackageItemViewModelFactory;
         private readonly ISecurityPolicyService _securityPolicyService;
+        private readonly IPackageVulnerabilitiesService _packageVulnerabilitiesService;
 
-        public ListPackageItemRequiredSignerViewModelFactory(ISecurityPolicyService securityPolicyService, IIconUrlProvider iconUrlProvider)
+        public ListPackageItemRequiredSignerViewModelFactory(
+            ISecurityPolicyService securityPolicyService, 
+            IIconUrlProvider iconUrlProvider,
+            IPackageVulnerabilitiesService packageVulnerabilitiesService)
         {
             _listPackageItemViewModelFactory = new ListPackageItemViewModelFactory(iconUrlProvider);
             _securityPolicyService = securityPolicyService ?? throw new ArgumentNullException(nameof(securityPolicyService));
+            _packageVulnerabilitiesService = packageVulnerabilitiesService ?? throw new ArgumentNullException(nameof(packageVulnerabilitiesService));
         }
 
         // username must be an empty string because <select /> option values are based on username
@@ -125,6 +130,8 @@ private ListPackageItemRequiredSignerViewModel SetupInternal(
                 viewModel.CanEditRequiredSigner &= wasAADLoginOrMultiFactorAuthenticated;
             }
 
+            viewModel.IsVulnerable = _packageVulnerabilitiesService.IsPackageVulnerable(package);
+
             return viewModel;
         }
 

src/NuGetGallery/Services/IPackageVulnerabilitiesService.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System.Collections.Generic;
-using System.Threading.Tasks;
 using NuGet.Services.Entities;
 
 namespace NuGetGallery
@@ -14,5 +13,11 @@ public interface IPackageVulnerabilitiesService
         /// </summary>
         /// <param name="id">id of the package for this query</param>
         IReadOnlyDictionary<int, IReadOnlyList<PackageVulnerability>> GetVulnerabilitiesById(string id);
+
+        /// <summary>
+        /// Returns true if the package has a vulnerability
+        /// </summary>
+        /// <param name="package">package to examine</param>
+        bool IsPackageVulnerable(Package package);
     }
 }

src/NuGetGallery/Services/PackageVulnerabilitiesService.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Data.Entity;
+using System.IO;
 using System.Linq;
 using NuGet.Services.Entities;
 
@@ -41,5 +42,20 @@ public IReadOnlyDictionary<int, IReadOnlyList<PackageVulnerability>> GetVulnerab
             return !result.Any() ? null :
                 result.ToDictionary(kv => kv.Key, kv => kv.Value as IReadOnlyList<PackageVulnerability>);
         }
+
+        public bool IsPackageVulnerable(Package package)
+        {
+            if (package == null)
+            {
+                throw new ArgumentNullException(nameof(package));
+            }
+
+            if (package.VulnerablePackageRanges == null)
+            {
+                throw new ArgumentException($"{nameof(package.VulnerablePackageRanges)} should be included in package query");
+            }
+
+            return package.VulnerablePackageRanges.FirstOrDefault(vpr => vpr.Vulnerability != null) != null;
+        }
     }
 }

src/NuGetGallery/ViewModels/ManagePackagesViewModel.cs

@@ -23,5 +23,7 @@ public class ManagePackagesViewModel
         public bool WasMultiFactorAuthenticated { get; set; }
 
         public bool IsCertificatesUIEnabled { get; set; }
+
+        public bool IsPackageVulnerabilitiesEnabled { get; set; }
     }
 }

src/NuGetGallery/ViewModels/PackageViewModel.cs

@@ -28,6 +28,7 @@ public class PackageViewModel : IPackageVersionModel
         public string Version { get; set; }
         public string FullVersion { get; set; }
         public PackageStatusSummary PackageStatusSummary { get; set; }
+        public bool IsVulnerable { get; set; }
 
         public bool IsCurrent(IPackageVersionModel current)
         {

tests/AccountDeleter.Facts/EvaluatorFacts.cs

@@ -31,7 +31,7 @@ public EvaluatorFacts(ITestOutputHelper output)
         public void NuGetDeleteevaluatorReturnsCorrectValueForUnconfirmed()
         {
             // Setup
-            _packageService.Setup(ps => ps.FindPackagesByOwner(It.IsAny<User>(), It.IsAny<bool>(), It.IsAny<bool>()))
+            _packageService.Setup(ps => ps.FindPackagesByOwner(It.IsAny<User>(), It.IsAny<bool>(), It.IsAny<bool>(), It.IsAny<bool>()))
                 .Returns(() =>
                 {
                     return new List<Package>();
@@ -58,7 +58,7 @@ public void NuGetDeleteevaluatorReturnsCorrectValueForUnconfirmed()
         public void NuGetDeleteevaluatorReturnsCorrectValueForPackages(bool userHasPackages, bool expectedResult)
         {
             // Setup
-            _packageService.Setup(ps => ps.FindPackagesByOwner(It.IsAny<User>(), It.IsAny<bool>(), It.IsAny<bool>()))
+            _packageService.Setup(ps => ps.FindPackagesByOwner(It.IsAny<User>(), It.IsAny<bool>(), It.IsAny<bool>(), It.IsAny<bool>()))
                 .Returns(() =>
                 {
                     if (userHasPackages)
@@ -94,7 +94,7 @@ public void NuGetDeleteevaluatorReturnsCorrectValueForPackages(bool userHasPacka
         public void NuGetDeleteevaluatorReturnsCorrectValueForOrganizations(bool userHasOrgs, bool userIsAdmin, bool expectedResult)
         {
             // Setup
-            _packageService.Setup(ps => ps.FindPackagesByOwner(It.IsAny<User>(), It.IsAny<bool>(), It.IsAny<bool>()))
+            _packageService.Setup(ps => ps.FindPackagesByOwner(It.IsAny<User>(), It.IsAny<bool>(), It.IsAny<bool>(), It.IsAny<bool>()))
                 .Returns(() =>
                 {
                     return new List<Package>();

tests/NuGetGallery.Facts/Controllers/OrganizationsControllerFacts.cs

@@ -1665,7 +1665,7 @@ public async Task IfAdministrator_ShowsViewWithCorrectData(bool isPackageOrphane
                     .Setup(stub => stub.FindByUsername(testOrganization.Username, false))
                     .Returns(testOrganization);
                 GetMock<IPackageService>()
-                    .Setup(stub => stub.FindPackagesByAnyMatchingOwner(testOrganization, It.IsAny<bool>(), false))
+                    .Setup(stub => stub.FindPackagesByAnyMatchingOwner(testOrganization, It.IsAny<bool>(), false, false))
                     .Returns(userPackages);
                 GetMock<IPackageService>()
                     .Setup(stub => stub.WillPackageBeOrphanedIfOwnerRemoved(packageRegistration, testOrganization))
@@ -1700,7 +1700,7 @@ public async Task IfOrphanedPackages_RedirectsToDeleteRequest()
                 controller.SetCurrentUser(fakes.OrganizationOwnerAdmin);
 
                 GetMock<IPackageService>()
-                    .Setup(x => x.FindPackagesByAnyMatchingOwner(testOrganization, true, false))
+                    .Setup(x => x.FindPackagesByAnyMatchingOwner(testOrganization, true, false, false))
                     .Returns(new[] { new Package { Version = "1.0.0", PackageRegistration = new PackageRegistration { Owners = new[] { testOrganization } } } });
                 GetMock<IPackageService>()
                     .Setup(x => x.WillPackageBeOrphanedIfOwnerRemoved(It.IsAny<PackageRegistration>(), testOrganization))
@@ -2484,7 +2484,7 @@ public void DeleteHappyAccount(bool withPendingIssues)
                     .Setup(stub => stub.FindByUsername(username, false))
                     .Returns(testUser);
                 GetMock<IPackageService>()
-                    .Setup(stub => stub.FindPackagesByAnyMatchingOwner(testUser, It.IsAny<bool>(), false))
+                    .Setup(stub => stub.FindPackagesByAnyMatchingOwner(testUser, It.IsAny<bool>(), false, false))
                     .Returns(userPackages);
                 const string iconUrl = "https://icon.test/url";
                 GetMock<IIconUrlProvider>()