Add vulnerability messaging to package details page (#8358)

Surface vulnerabilities in package detail page
NuGet · Jan 8, 2021 · 5ce2f98 · 5ce2f98
1 parent 86af6b6
commit 5ce2f98
Show file tree

Hide file tree

Showing 10 changed files with 493 additions and 150 deletions.
diff --git a/src/Bootstrap/dist/css/bootstrap-theme.css b/src/Bootstrap/dist/css/bootstrap-theme.css
diff --git a/src/Bootstrap/less/theme/page-display-package.less b/src/Bootstrap/less/theme/page-display-package.less
@@ -63,6 +63,50 @@
     }
   }
 
+  .vulnerabilities-container {
+    .vulnerabilities-expander {
+      display: flex;
+      justify-content: space-between;
+      vertical-align: middle;
+      width: 100%;
+
+      .vulnerabilities-expander-container {
+        display: flex;
+
+        .vulnerabilities-expander-icon {
+          position: unset;
+          top: unset;
+        }
+
+        .vulnerabilities-expander-info-right {
+          padding-left: 15px;
+        }
+      }
+    }
+
+    .vulnerabilities-content-container {
+      margin-top: 15px;
+      padding-top: 15px;
+      border-top: 1px solid lightgray;
+
+      .vulnerabilities-list {
+        border-collapse: unset;
+      }
+    }
+
+    .vulnerabilities-severity-critical {
+      color: red
+    }
+
+    .vulnerabilities-severity-high {
+      color: red
+    }
+
+    .vulnerabilities-severity-moderate {
+      color: blue
+    }
+  }
+
   .failed-validation-alert-list {
     margin-top: 15px;
     margin-bottom: 15px;

diff --git a/src/NuGetGallery/Controllers/PackagesController.cs b/src/NuGetGallery/Controllers/PackagesController.cs
@@ -873,12 +873,18 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
             }
 
             var readme = await _readMeService.GetReadMeHtmlAsync(package);
-            var deprecations = _deprecationService.GetDeprecationsById(id);
-            var packageKeyToDeprecation = deprecations
-                .GroupBy(d => d.PackageKey)
-                .ToDictionary(g => g.Key, g => g.First());
 
-            var packageKeyToVulnerabilities = _vulnerabilitiesService.GetVulnerabilitiesById(id);
+            var isPackageDeprecationEnabled = _featureFlagService.IsManageDeprecationEnabled(currentUser, allVersions);
+            var packageKeyToDeprecation = isPackageDeprecationEnabled
+                ? _deprecationService.GetDeprecationsById(id)
+                    .GroupBy(d => d.PackageKey)
+                    .ToDictionary(g => g.Key, g => g.First())
+                : null;
+
+            var isPackageVulnerabilitiesEnabled = _featureFlagService.IsDisplayVulnerabilitiesEnabled();
+            var packageKeyToVulnerabilities = isPackageVulnerabilitiesEnabled
+                ? _vulnerabilitiesService.GetVulnerabilitiesById(id)
+                : null;
 
             IReadOnlyList<PackageRename> packageRenames = null;
             if (_featureFlagService.IsPackageRenamesEnabled(currentUser))
@@ -900,8 +906,8 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
             model.SymbolsPackageValidationIssues = _validationService.GetLatestPackageValidationIssues(model.LatestSymbolsPackage);
             model.IsCertificatesUIEnabled = _contentObjectService.CertificatesConfiguration?.IsUIEnabledForUser(currentUser) ?? false;
             model.IsAtomFeedEnabled = _featureFlagService.IsPackagesAtomFeedEnabled();
-            model.IsPackageDeprecationEnabled = _featureFlagService.IsManageDeprecationEnabled(currentUser, allVersions);
-            model.IsPackageVulnerabilitiesEnabled = _featureFlagService.IsDisplayVulnerabilitiesEnabled();
+            model.IsPackageDeprecationEnabled = isPackageDeprecationEnabled;
+            model.IsPackageVulnerabilitiesEnabled = isPackageVulnerabilitiesEnabled;
             model.IsFuGetLinksEnabled = _featureFlagService.IsDisplayFuGetLinksEnabled();
             model.IsPackageRenamesEnabled = _featureFlagService.IsPackageRenamesEnabled(currentUser);
             model.IsPackageDependentsEnabled = _featureFlagService.IsPackageDependentsEnabled(currentUser);

diff --git a/src/NuGetGallery/Helpers/ViewModelExtensions/DisplayPackageViewModelFactory.cs b/src/NuGetGallery/Helpers/ViewModelExtensions/DisplayPackageViewModelFactory.cs
@@ -183,7 +183,8 @@ private DisplayPackageViewModel SetupCommon(
                 }
             }
 
-            if (packageKeyToDeprecation != null && packageKeyToDeprecation.TryGetValue(package.Key, out var deprecation))
+            PackageDeprecation deprecation = null;
+            if (packageKeyToDeprecation != null && packageKeyToDeprecation.TryGetValue(package.Key, out deprecation))
             {
                 viewModel.DeprecationStatus = deprecation.Status;
             }
@@ -192,15 +193,75 @@ private DisplayPackageViewModel SetupCommon(
                 viewModel.DeprecationStatus = PackageDeprecationStatus.NotDeprecated;
             }
 
-            if (packageKeyToVulnerabilities != null && packageKeyToVulnerabilities.TryGetValue(package.Key, out var vulnerabilities))
+            PackageVulnerabilitySeverity? maxVulnerabilitySeverity = null;
+            if (packageKeyToVulnerabilities != null 
+                && packageKeyToVulnerabilities.TryGetValue(package.Key, out var vulnerabilities)
+                && vulnerabilities != null && vulnerabilities.Any())
             {
                 viewModel.Vulnerabilities = vulnerabilities;
-                viewModel.MaxVulnerabilitySeverity = vulnerabilities.Max(v => v.Severity);
+                maxVulnerabilitySeverity = viewModel.Vulnerabilities.Max(v => v.Severity); // cache for messaging
+                viewModel.MaxVulnerabilitySeverity = maxVulnerabilitySeverity.Value;
+            }
+            else
+            {
+                viewModel.Vulnerabilities = null;
+                viewModel.MaxVulnerabilitySeverity = default;
             }
 
+            viewModel.PackageWarningIconTitle =
+                GetWarningIconTitle(viewModel.Version, deprecation, maxVulnerabilitySeverity);
+
             return viewModel;
         }
 
+        private static string GetWarningIconTitle(
+            string version, 
+            PackageDeprecation deprecation,
+            PackageVulnerabilitySeverity? maxVulnerabilitySeverity)
+        {
+            // We want a tooltip title for the warning icon, which concatenates deprecation and vulnerability information cleanly
+            var deprecationTitle = "";
+            if (deprecation != null)
+            {
+                deprecationTitle = version;
+                var isLegacy = deprecation.Status.HasFlag(PackageDeprecationStatus.Legacy);
+                var hasCriticalBugs = deprecation.Status.HasFlag(PackageDeprecationStatus.CriticalBugs);
+                if (hasCriticalBugs)
+                {
+                    if (isLegacy)
+                    {
+                        deprecationTitle += " is deprecated because it's legacy and has critical bugs";
+                    }
+                    else
+                    {
+                        deprecationTitle += " is deprecated because it has critical bugs";
+                    }
+                }
+                else if (isLegacy)
+                {
+                    deprecationTitle += " is deprecated because it's legacy and no longer maintained";
+                }
+                else
+                {
+                    deprecationTitle += " is deprecated";
+                }
+            }
+
+            if (maxVulnerabilitySeverity.HasValue)
+            {
+                var severity = Enum.GetName(typeof(PackageVulnerabilitySeverity), maxVulnerabilitySeverity)?.ToLowerInvariant() ?? "unknown";
+                var vulnerabilitiesTitle = $"{version} has at least one vulnerability with {severity} severity.";
+
+                return string.IsNullOrEmpty(deprecationTitle)
+                    ? vulnerabilitiesTitle
+                    : $"{deprecationTitle}; {vulnerabilitiesTitle}";
+            }
+
+            return string.IsNullOrEmpty(deprecationTitle)
+                ? string.Empty
+                : $"{deprecationTitle}.";
+        }
+
         private static string GetPushedBy(Package package, User currentUser, Dictionary<User, string> pushedByCache)
         {
             var userPushedBy = package.User;

diff --git a/src/NuGetGallery/NuGetGallery.csproj b/src/NuGetGallery/NuGetGallery.csproj
@@ -2305,6 +2305,9 @@
   <ItemGroup>
     <Content Include="App_Data\Files\Content\Query-Hint-Configuration.json" />
   </ItemGroup>
+  <ItemGroup>
+    <Content Include="Views\Packages\_DisplayPackageVulnerabilities.cshtml" />
+  </ItemGroup>
   <PropertyGroup>
     <VisualStudioVersion Condition="'$(VisualStudioVersion)' == ''">10.0</VisualStudioVersion>
     <VSToolsPath Condition="'$(VSToolsPath)' == ''">$(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion)</VSToolsPath>

diff --git a/src/NuGetGallery/Scripts/gallery/page-display-package.js b/src/NuGetGallery/Scripts/gallery/page-display-package.js
@@ -4,23 +4,37 @@ $(function () {
     // Configure the rename information container
     window.nuget.configureExpander("rename-content-container", "ChevronDown", null, "ChevronUp");
     configureExpanderWithEnterKeydown($('#show-rename-content-container'));
+    var expanderAttributes = ['data-toggle', 'data-target', 'aria-expanded', 'aria-controls', 'tabindex'];
+
+    // Configure the vulnerability information container
+    var vulnerabilitiesContainer = $('#show-vulnerabilities-content-container');
+    if ($('#vulnerabilities-content-container').children().length) {
+        // If the vulnerability information container has content, configure it as an expander.
+        window.nuget.configureExpander("vulnerabilities-content-container", "ChevronDown", null, "ChevronUp");
+        configureExpanderWithEnterKeydown(vulnerabilitiesContainer);
+    } else {
+        // If the container does not have content, remove its expander attributes
+        expanderAttributes.forEach(attribute => vulnerabilitiesContainer.removeAttr(attribute));
+
+        // The expander should not be clickable when it doesn't have content
+        vulnerabilitiesContainer.find('.vulnerabilities-expander').removeAttr('role');
+
+        $('#vulnerabilities-expander-icon-right').hide();
+    }
 
     // Configure the deprecation information container
-    var container = $('#show-deprecation-content-container');
+    var deprecationContainer = $('#show-deprecation-content-container');
     if ($('#deprecation-content-container').children().length) {
         // If the deprecation information container has content, configure it as an expander.
         window.nuget.configureExpander("deprecation-content-container", "ChevronDown", null, "ChevronUp");
-        configureExpanderWithEnterKeydown(container)
+        configureExpanderWithEnterKeydown(deprecationContainer);
     }
     else {
         // If the container does not have content, remove its expander attributes
-        var expanderAttributes = ['data-toggle', 'data-target', 'aria-expanded', 'aria-controls', 'tabindex'];
-        for (var i in expanderAttributes) {
-            container.removeAttr(expanderAttributes[i]);
-        }
+        expanderAttributes.forEach(attribute => deprecationContainer.removeAttr(attribute));
 
         // The expander should not be clickable when it doesn't have content
-        container.find('.deprecation-expander').removeAttr('role');
+        deprecationContainer.find('.deprecation-expander').removeAttr('role');
 
         $('#deprecation-expander-icon-right').hide();
     }

diff --git a/src/NuGetGallery/ViewModels/DisplayPackageViewModel.cs b/src/NuGetGallery/ViewModels/DisplayPackageViewModel.cs
@@ -87,6 +87,7 @@ public bool HasNewerRelease
         public PackageDeprecationStatus DeprecationStatus { get; set; }
         public IReadOnlyCollection<PackageVulnerability> Vulnerabilities { get; set; }
         public PackageVulnerabilitySeverity MaxVulnerabilitySeverity { get; set; }
+        public string PackageWarningIconTitle { get; set; }
         public string AlternatePackageId { get; set; }
         public string AlternatePackageVersion { get; set; }
         public string CustomMessage { get; set; }

diff --git a/src/NuGetGallery/Views/Packages/DisplayPackage.cshtml b/src/NuGetGallery/Views/Packages/DisplayPackage.cshtml
@@ -283,6 +283,11 @@
                 )
             }
 
+            @if (Model.IsPackageVulnerabilitiesEnabled && Model.Vulnerabilities != null)
+            {
+                @Html.Partial("_DisplayPackageVulnerabilities")
+            }
+
             @if (Model.IsPackageDeprecationEnabled && Model.DeprecationStatus != PackageDeprecationStatus.NotDeprecated)
             {
                 @Html.Partial("_DisplayPackageDeprecation")
@@ -359,7 +364,7 @@
 
                         @AppendAvailableSymbolsMessage(hasSymbolsPackageAvailable)
                         </text>
-                     )
+                    )
                 }
                 else if (Model.LatestSymbolsPackage.StatusKey == PackageStatus.FailedValidation)
                 {
@@ -404,7 +409,7 @@
                     @ViewHelpers.AlertWarning(
                         @<text>
                             The owner has unlisted this package.
-                            This could mean that the package is deprecated or shouldn't be used anymore.
+                            This could mean that the package is deprecated, has security vulnerabilities or shouldn't be used anymore.
                         </text>
                     )
                 }
@@ -684,9 +689,9 @@
                             {
                                 <th aria-hidden="true" abbr="Signature Information"></th>
                             }
-                            @if (Model.IsPackageDeprecationEnabled)
+                            @if (Model.IsPackageDeprecationEnabled || Model.IsPackageVulnerabilitiesEnabled)
                             {
-                                <th aria-hidden="true" abbr="Deprecation Information"></th>
+                                <th aria-hidden="true" abbr="Package Warnings"></th>
                             }
                         </tr>
                     </thead>
@@ -753,41 +758,16 @@
                                             </td>
                                         }
                                     }
-                                    @if (Model.IsPackageDeprecationEnabled)
-                                    {
-                                        if (packageVersion.DeprecationStatus == PackageDeprecationStatus.NotDeprecated)
-                                        {
-                                            <td class="package-icon-cell" aria-hidden="true"></td>
-                                        }
-                                        else
-                                        {
-                                            var deprecationTitle = packageVersion.Version;
-                                            var isLegacy = packageVersion.DeprecationStatus.HasFlag(PackageDeprecationStatus.Legacy);
-                                            var hasCriticalBugs = packageVersion.DeprecationStatus.HasFlag(PackageDeprecationStatus.CriticalBugs);
-                                            if (hasCriticalBugs)
-                                            {
-                                                if (isLegacy)
-                                                {
-                                                    deprecationTitle += " is deprecated because it's legacy and has critical bugs.";
-                                                }
-                                                else
-                                                {
-                                                    deprecationTitle += " is deprecated because it has critical bugs.";
-                                                }
-                                            }
-                                            else if (isLegacy)
-                                            {
-                                                deprecationTitle += " is deprecated because it's legacy and no longer maintained.";
-                                            }
-                                            else
-                                            {
-                                                deprecationTitle += " is deprecated.";
-                                            }
 
-                                            <td class="package-icon-cell">
-                                                <i class="ms-Icon ms-Icon--Warning package-icon" title="@deprecationTitle"></i>
-                                            </td>
-                                        }
+                                    @if (string.IsNullOrEmpty(packageVersion.PackageWarningIconTitle))
+                                    {
+                                        <td class="package-icon-cell" aria-hidden="true"></td>
+                                    }
+                                    else
+                                    {
+                                        <td class="package-icon-cell">
+                                            <i class="ms-Icon ms-Icon--Warning package-icon" title="@packageVersion.PackageWarningIconTitle"></i>
+                                        </td>
                                     }
                                 </tr>
                             }