Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename vulnerability management service, add feature flag #8356

Merged
merged 1 commit into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/GitHubVulnerabilities2Db/Ingest/AdvisoryIngestor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ namespace GitHubVulnerabilities2Db.Ingest
{
public class AdvisoryIngestor : IAdvisoryIngestor
{
private readonly IPackageVulnerabilityService _packageVulnerabilityService;
private readonly IPackageVulnerabilitiesManagementService _packageVulnerabilityService;
private readonly IGitHubVersionRangeParser _gitHubVersionRangeParser;

public AdvisoryIngestor(
IPackageVulnerabilityService packageVulnerabilityService,
IPackageVulnerabilitiesManagementService packageVulnerabilityService,
IGitHubVersionRangeParser gitHubVersionRangeParser)
{
_packageVulnerabilityService = packageVulnerabilityService ?? throw new ArgumentNullException(nameof(packageVulnerabilityService));
Expand Down
4 changes: 2 additions & 2 deletions src/GitHubVulnerabilities2Db/Job.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ protected void ConfigureIngestionServices(ContainerBuilder containerBuilder)
ConfigureGalleryServices(containerBuilder);

containerBuilder
.RegisterType<PackageVulnerabilityService>()
.As<IPackageVulnerabilityService>();
.RegisterType<PackageVulnerabilitiesManagementService>()
.As<IPackageVulnerabilitiesManagementService>();

containerBuilder
.RegisterType<GitHubVersionRangeParser>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public class FeatureFlagService : IFeatureFlagService
private const string ManageDeprecationFeatureName = GalleryPrefix + "ManageDeprecation";
private const string ManageDeprecationForManyVersionsFeatureName = GalleryPrefix + "ManageDeprecationMany";
private const string ManageDeprecationApiFeatureName = GalleryPrefix + "ManageDeprecationApi";
private const string DisplayVulnerabilitiesFeatureName = GalleryPrefix + "DisplayVulnerabilities";
private const string ODataReadOnlyDatabaseFeatureName = GalleryPrefix + "ODataReadOnlyDatabase";
private const string PackagesAtomFeedFeatureName = GalleryPrefix + "PackagesAtomFeed";
private const string SearchSideBySideFlightName = GalleryPrefix + "SearchSideBySide";
Expand Down Expand Up @@ -127,6 +128,11 @@ public bool IsManageDeprecationApiEnabled(User user)
return _client.IsEnabled(ManageDeprecationApiFeatureName, user, defaultValue: false);
}

public bool IsDisplayVulnerabilitiesEnabled()
{
return _client.IsEnabled(DisplayVulnerabilitiesFeatureName, defaultValue: false);
}

public bool AreEmbeddedIconsEnabled(User user)
{
return _client.IsEnabled(EmbeddedIconFlightName, user, defaultValue: false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ public interface IFeatureFlagService
/// </summary>
bool IsManageDeprecationApiEnabled(User user);

/// <summary>
/// Whether or not a package owner can view vulnerability advisory information on their package.
/// </summary>
bool IsDisplayVulnerabilitiesEnabled();

/// <summary>
/// Whether the user is allowed to publish packages with an embedded icon.
/// </summary>
Expand Down
4 changes: 2 additions & 2 deletions src/NuGetGallery.Services/NuGetGallery.Services.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@
<Compile Include="PackageManagement\IPackageOwnershipManagementService.cs" />
<Compile Include="PackageManagement\IPackageService.cs" />
<Compile Include="PackageManagement\IPackageUpdateService.cs" />
<Compile Include="PackageManagement\IPackageVulnerabilityService.cs" />
<Compile Include="PackageManagement\IPackageVulnerabilitiesManagementService.cs" />
<Compile Include="PackageManagement\IReservedNamespaceService.cs" />
<Compile Include="PackageManagement\PackageDeleteDecision.cs" />
<Compile Include="PackageManagement\PackageFilter.cs" />
Expand All @@ -170,7 +170,7 @@
<Compile Include="PackageManagement\PackageOwnerRequestService.cs" />
<Compile Include="PackageManagement\PackageOwnershipManagementService.cs" />
<Compile Include="PackageManagement\PackageService.cs" />
<Compile Include="PackageManagement\PackageVulnerabilityService.cs" />
<Compile Include="PackageManagement\PackageVulnerabilitiesManagementService.cs" />
<Compile Include="PackageManagement\ReservedNamespaceService.cs" />
<Compile Include="Permissions\ActionRequiringAccountPermissions.cs" />
<Compile Include="Permissions\ActionRequiringEntityPermissions.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@

namespace NuGetGallery
{
public interface IPackageVulnerabilityService
public interface IPackageVulnerabilitiesManagementService
{
/// <summary>
/// Adds any <see cref="VulnerablePackageVersionRange"/>s to <see cref="Package.Vulnerabilities"/> that it is a part of.
/// Adds any <see cref="VulnerablePackageVersionRange"/>s to <see cref="Package.VulnerableVersionRanges"/> that it is a part of.
/// </summary>
/// <remarks>
/// Does not commit changes. The caller is expected to commit any changes separately.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@

namespace NuGetGallery
{
public class PackageVulnerabilityService : IPackageVulnerabilityService
public class PackageVulnerabilitiesManagementService : IPackageVulnerabilitiesManagementService
{
private readonly IEntitiesContext _entitiesContext;
private readonly IPackageUpdateService _packageUpdateService;
private readonly ILogger<PackageVulnerabilityService> _logger;
private readonly ILogger<PackageVulnerabilitiesManagementService> _logger;

public PackageVulnerabilityService(
public PackageVulnerabilitiesManagementService(
IEntitiesContext entitiesContext,
IPackageUpdateService packageUpdateService,
ILogger<PackageVulnerabilityService> logger)
ILogger<PackageVulnerabilitiesManagementService> logger)
{
_entitiesContext = entitiesContext ?? throw new ArgumentNullException(nameof(entitiesContext));
_packageUpdateService = packageUpdateService ?? throw new ArgumentNullException(nameof(packageUpdateService));
Expand Down
3 changes: 2 additions & 1 deletion src/NuGetGallery/App_Data/Files/Content/flags.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
"NuGetGallery.ODataV2FindPackagesByIdNonHijacked": "Enabled",
"NuGetGallery.ODataV2FindPackagesByIdCountNonHijacked": "Enabled",
"NuGetGallery.ODataV2SearchNonHijacked": "Enabled",
"NuGetGallery.ODataV2SearchCountNonHijacked": "Enabled"
"NuGetGallery.ODataV2SearchCountNonHijacked": "Enabled",
"NuGetGallery.DisplayVulnerabilities": "Enabled"
},
"Flights": {
"NuGetGallery.TyposquattingFlight": {
Expand Down
4 changes: 2 additions & 2 deletions src/NuGetGallery/App_Start/DefaultDependenciesModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -442,8 +442,8 @@ protected override void Load(ContainerBuilder builder)
.As<IIconUrlTemplateProcessor>()
.InstancePerLifetimeScope();

builder.RegisterType<PackageVulnerabilityService>()
.As<IPackageVulnerabilityService>()
builder.RegisterType<PackageVulnerabilitiesManagementService>()
.As<IPackageVulnerabilitiesManagementService>()
.InstancePerLifetimeScope();

services.AddHttpClient();
Expand Down
4 changes: 2 additions & 2 deletions src/NuGetGallery/Services/PackageUploadService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class PackageUploadService : IPackageUploadService
private readonly IReservedNamespaceService _reservedNamespaceService;
private readonly IValidationService _validationService;
private readonly ICoreLicenseFileService _coreLicenseFileService;
private readonly IPackageVulnerabilityService _vulnerabilityService;
private readonly IPackageVulnerabilitiesManagementService _vulnerabilityService;
private readonly IPackageMetadataValidationService _metadataValidationService;

public PackageUploadService(
Expand All @@ -33,7 +33,7 @@ public PackageUploadService(
IValidationService validationService,
ICoreLicenseFileService coreLicenseFileService,
IDiagnosticsService diagnosticsService,
IPackageVulnerabilityService vulnerabilityService,
IPackageVulnerabilitiesManagementService vulnerabilityService,
IPackageMetadataValidationService metadataValidationService)
{
_packageService = packageService ?? throw new ArgumentNullException(nameof(packageService));
Expand Down
2 changes: 1 addition & 1 deletion src/VerifyGitHubVulnerabilities/Job.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public override async Task Run()
Console.WriteLine($" FOUND {advisories.Count} advisories.");

Console.WriteLine("Fetching vulnerabilities from DB...");
var verifier = new PackageVulnerabilityServiceVerifier(_serviceProvider.GetRequiredService<IEntitiesContext>());
var verifier = new PackageVulnerabilitiesVerifier(_serviceProvider.GetRequiredService<IEntitiesContext>());
var ingestor = new AdvisoryIngestor(verifier, new GitHubVersionRangeParser());
await ingestor.IngestAsync(advisories);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@

namespace VerifyGitHubVulnerabilities.Verify
{
public class PackageVulnerabilityServiceVerifier : IPackageVulnerabilityService
public class PackageVulnerabilitiesVerifier : IPackageVulnerabilitiesManagementService
{
private readonly IEntitiesContext _entitiesContext;

public PackageVulnerabilityServiceVerifier(
public PackageVulnerabilitiesVerifier(
IEntitiesContext entitiesContext)
{
_entitiesContext = entitiesContext ?? throw new ArgumentNullException(nameof(entitiesContext));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
<Compile Include="Job.cs" />
<Compile Include="Program.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
<Compile Include="Verify\PackageVulnerabilityServiceVerifier.cs" />
<Compile Include="Verify\PackageVulnerabilitiesVerifier.cs" />
</ItemGroup>
<ItemGroup>
<None Include="App.config" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,14 @@ public class MethodFacts
{
public MethodFacts()
{
PackageVulnerabilityServiceMock = new Mock<IPackageVulnerabilityService>();
PackageVulnerabilityServiceMock = new Mock<IPackageVulnerabilitiesManagementService>();
GitHubVersionRangeParserMock = new Mock<IGitHubVersionRangeParser>();
Ingestor = new AdvisoryIngestor(
PackageVulnerabilityServiceMock.Object,
GitHubVersionRangeParserMock.Object);
}

public Mock<IPackageVulnerabilityService> PackageVulnerabilityServiceMock { get; }
public Mock<IPackageVulnerabilitiesManagementService> PackageVulnerabilityServiceMock { get; }
public Mock<IGitHubVersionRangeParser> GitHubVersionRangeParserMock { get; }
public AdvisoryIngestor Ingestor { get; }
}
Expand Down
2 changes: 1 addition & 1 deletion tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@
<Compile Include="Services\PackageDeprecationManagementServiceFacts.cs" />
<Compile Include="Services\PackageDeprecationServiceFacts.cs" />
<Compile Include="Services\PackageRenameServiceFacts.cs" />
<Compile Include="Services\PackageVulnerabilityServiceFacts.cs" />
<Compile Include="Services\PackageVulnerabilitiesManagementServiceFacts.cs" />
<Compile Include="Services\SearchSideBySideServiceFacts.cs" />
<Compile Include="Services\PackageUpdateServiceFacts.cs" />
<Compile Include="Services\StatusServiceFacts.cs" />
Expand Down
14 changes: 7 additions & 7 deletions tests/NuGetGallery.Facts/Services/PackageUploadServiceFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ private static PackageUploadService CreateService(
Mock<IPackageService> packageService = null,
Mock<IReservedNamespaceService> reservedNamespaceService = null,
Mock<IValidationService> validationService = null,
Mock<IPackageVulnerabilityService> vulnerabilityService = null)
Mock<IPackageVulnerabilitiesManagementService> vulnerabilityService = null)
{
packageService = packageService ?? new Mock<IPackageService>();

Expand Down Expand Up @@ -64,7 +64,7 @@ private static PackageUploadService CreateService(

if (vulnerabilityService == null)
{
vulnerabilityService = new Mock<IPackageVulnerabilityService>();
vulnerabilityService = new Mock<IPackageVulnerabilitiesManagementService>();
}

validationService = validationService ?? new Mock<IValidationService>();
Expand Down Expand Up @@ -96,7 +96,7 @@ public async Task WillCallCreatePackageAsyncCorrectly()
var key = 0;
var packageService = new Mock<IPackageService>();
packageService.Setup(x => x.FindPackageRegistrationById(It.IsAny<string>())).Returns((PackageRegistration)null);
var vulnerabilityService = new Mock<IPackageVulnerabilityService>();
var vulnerabilityService = new Mock<IPackageVulnerabilitiesManagementService>();

var id = "Microsoft.Aspnet.Mvc";
var packageUploadService = CreateService(packageService, vulnerabilityService: vulnerabilityService);
Expand Down Expand Up @@ -151,7 +151,7 @@ public async Task WillMarkPackageRegistrationVerifiedFlagCorrectly(bool shouldMa
.Setup(r => r.GetReservedNamespacesForId(It.IsAny<string>()))
.Returns(testNamespaces.ToList().AsReadOnly());

var vulnerabilityService = new Mock<IPackageVulnerabilityService>();
var vulnerabilityService = new Mock<IPackageVulnerabilitiesManagementService>();

var packageUploadService = CreateService(
reservedNamespaceService: reservedNamespaceService,
Expand Down Expand Up @@ -193,7 +193,7 @@ public async Task WillMarkPackageRegistrationNotVerifiedIfIdMatchesNonOwnedShare
.Setup(r => r.GetReservedNamespacesForId(It.IsAny<string>()))
.Returns(testNamespaces.ToList().AsReadOnly());

var vulnerabilityService = new Mock<IPackageVulnerabilityService>();
var vulnerabilityService = new Mock<IPackageVulnerabilitiesManagementService>();

var packageUploadService = CreateService(
reservedNamespaceService: reservedNamespaceService,
Expand Down Expand Up @@ -741,7 +741,7 @@ public abstract class FactsBase
protected readonly Mock<ITelemetryService> _telemetryService;
protected readonly Mock<ICoreLicenseFileService> _licenseFileService;
protected readonly Mock<IDiagnosticsService> _diagnosticsService;
protected readonly Mock<IPackageVulnerabilityService> _vulnerabilityService;
protected readonly Mock<IPackageVulnerabilitiesManagementService> _vulnerabilityService;
protected readonly Mock<IPackageMetadataValidationService> _metadataValidationService;
protected Package _package;
protected Stream _packageFile;
Expand Down Expand Up @@ -784,7 +784,7 @@ public FactsBase()
.Setup(ds => ds.GetSource(It.IsAny<string>()))
.Returns(Mock.Of<IDiagnosticsSource>());

_vulnerabilityService = new Mock<IPackageVulnerabilityService>();
_vulnerabilityService = new Mock<IPackageVulnerabilitiesManagementService>();

_metadataValidationService = new Mock<IPackageMetadataValidationService>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace NuGetGallery.Services
{
public class PackageVulnerabilityServiceFacts
public class PackageVulnerabilitiesManagementServiceFacts
{
public class TheApplyExistingVulnerabilitiesToPackageMethod : MethodFacts
{
Expand Down Expand Up @@ -211,7 +211,7 @@ public async Task WithExistingVulnerability_Withdrawn_RemovesAndUnmarks(bool has
.Verifiable();
}

var service = GetService<PackageVulnerabilityService>();
var service = GetService<PackageVulnerabilitiesManagementService>();

// Act
await service.UpdateVulnerabilityAsync(vulnerability, true);
Expand Down Expand Up @@ -258,7 +258,7 @@ public async Task WithExistingVulnerability_NotWithdrawn_NoRanges_RemovesAndUnma
.Verifiable();
}

var service = GetService<PackageVulnerabilityService>();
var service = GetService<PackageVulnerabilitiesManagementService>();

// Act
await service.UpdateVulnerabilityAsync(vulnerability, false);
Expand Down Expand Up @@ -440,7 +440,7 @@ public async Task WithExistingVulnerability_NotWithdrawn_UpdatesPackages(
.Verifiable();
}

var service = GetService<PackageVulnerabilityService>();
var service = GetService<PackageVulnerabilitiesManagementService>();

// Act
await service.UpdateVulnerabilityAsync(vulnerability, false);
Expand Down Expand Up @@ -469,7 +469,7 @@ public MethodFacts()
_databaseMock = new Mock<IDatabase>();
Context = GetFakeContext();
UpdateServiceMock = GetMock<IPackageUpdateService>();
Service = GetService<PackageVulnerabilityService>();
Service = GetService<PackageVulnerabilitiesManagementService>();

_transactionMock
.Setup(x => x.Commit())
Expand All @@ -487,7 +487,7 @@ public MethodFacts()
private Mock<IDatabase> _databaseMock { get; }
protected FakeEntitiesContext Context { get; }
protected Mock<IPackageUpdateService> UpdateServiceMock { get; }
protected PackageVulnerabilityService Service { get; }
protected PackageVulnerabilitiesManagementService Service { get; }

protected void VerifyTransaction()
{
Expand Down