Skip to content

Commit

Permalink
Address PR comment by Damon.
Browse files Browse the repository at this point in the history
  • Loading branch information
@erdembayar
erdembayar committed Apr 7, 2021
1 parent 6a3c789 commit fb609f6
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 237 deletions.
35 changes: 13 additions & 22 deletions src/NuGet.Core/NuGet.Packaging/PackageArchiveReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,26 +48,24 @@ private PackageArchiveReader(IFrameworkNameProvider frameworkProvider, IFramewor
_environmentVariableReader = EnvironmentVariableWrapper.Instance;
}

/// <summary>
/// Nupkg package reader
/// </summary>
/// <param name="stream">Nupkg data stream.</param>
/// <param name="environmentVariableReader">Environmental variable reader.</param>
// Nupkg package reader only for unit test
internal PackageArchiveReader(Stream stream, IEnvironmentVariableReader environmentVariableReader)
: this(stream)
{
_environmentVariableReader = environmentVariableReader ?? throw new ArgumentNullException(nameof(environmentVariableReader));
if (environmentVariableReader != null)
{
_environmentVariableReader = environmentVariableReader;
}
}

/// <summary>
/// Nupkg package reader
/// </summary>
/// <param name="filePath">File path for Nupkg data stream.</param>
/// <param name="environmentVariableReader">Environmental variable reader.</param>
// Nupkg package reader only for unit test
internal PackageArchiveReader(string filePath, IEnvironmentVariableReader environmentVariableReader)
: this(filePath)
{
_environmentVariableReader = environmentVariableReader ?? throw new ArgumentNullException(nameof(environmentVariableReader));
if (environmentVariableReader != null)
{
_environmentVariableReader = environmentVariableReader;
}
}

/// <summary>
Expand Down Expand Up @@ -488,18 +486,11 @@ public override bool CanVerifySignedPackages(SignedPackageVerifierSettings verif
}
else if (RuntimeEnvironmentHelper.IsLinux || RuntimeEnvironmentHelper.IsMacOSX)
{
// Conditionally enable back package sign verification temporary disabled due to Mozilla drop Symantec as CA on Linux/MAC.
// Please note: Linux/MAC case sensitive for env var.
// Please note: Linux/MAC case sensitive for env var name.
string signVerifyEnvVariable = _environmentVariableReader.GetEnvironmentVariable("DOTNET_OPT_IN_SECURE_PACKAGE_VERIFICATION");

if (!string.IsNullOrEmpty(signVerifyEnvVariable) && signVerifyEnvVariable.Equals(bool.TrueString, StringComparison.OrdinalIgnoreCase))
{
return true;
}
else
{
return false;
}
// Not opt-out option, only opt-in feature.
return !string.IsNullOrEmpty(signVerifyEnvVariable);
}
else
{
Expand Down
97 changes: 11 additions & 86 deletions test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetRestoreTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ namespace Dotnet.Integration.Test
[Collection("Dotnet Integration Tests")]
public class DotnetRestoreTests
{
private const string OptInPackageVerification = "DOTNET_OPT_IN_SECURE_PACKAGE_VERIFICATION";
private const string OptInPackageVerificationTypo = "DOTNET_OPT_IN_SECURE_PACKAGE_VERIFICATIOn";

private MsbuildIntegrationTestFixture _msbuildFixture;

public DotnetRestoreTests(MsbuildIntegrationTestFixture fixture)
Expand Down Expand Up @@ -269,12 +272,16 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async(
[InlineData("true")]
[InlineData("TRUE")]
[InlineData("TRUe")]
[InlineData("FALSE")]
[InlineData("xyz")]
[InlineData("")]
[InlineData(null)]
public async Task DotnetRestore_WithUnSignedPackageAndSignatureValidationModeAsRequired_OptInEnvVar_True_FailsAsync(string envVar)
{
using (var pathContext = _msbuildFixture.CreateSimpleTestPathContext())
{
//Arrange
var envVarName = "DOTNET_OPT_IN_SECURE_PACKAGE_VERIFICATION";
var envVarName = OptInPackageVerification;
var envVarValue = envVar;
//Setup packages and feed
var packageX = new SimpleTestPackageContext()
Expand Down Expand Up @@ -356,90 +363,8 @@ public async Task DotnetRestore_WithUnSignedPackageAndSignatureValidationModeAsR
using (var pathContext = _msbuildFixture.CreateSimpleTestPathContext())
{
//Arrange
var envVarName = "DOTNET_OPT_IN_SECURE_PACKAGE_VERIFICATION";
var envVarValue = "false";
//Setup packages and feed
var packageX = new SimpleTestPackageContext()
{
Id = "x",
Version = "1.0.0"
};
packageX.Files.Clear();
packageX.AddFile("lib/netcoreapp2.0/x.dll");
packageX.AddFile("ref/netcoreapp2.0/x.dll");
packageX.AddFile("lib/net472/x.dll");
packageX.AddFile("ref/net472/x.dll");

await SimpleTestPackageUtility.CreateFolderFeedV3Async(
pathContext.PackageSource,
PackageSaveMode.Defaultv3,
packageX);

// Set up solution, and project
var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot);

var projectName = "ClassLibrary1";
var workingDirectory = Path.Combine(pathContext.SolutionRoot, projectName);
var projectFile = Path.Combine(workingDirectory, $"{projectName}.csproj");

_msbuildFixture.CreateDotnetNewProject(pathContext.SolutionRoot, projectName, "classlib");

using (FileStream stream = File.Open(projectFile, FileMode.Open, FileAccess.ReadWrite))
{
XDocument xml = XDocument.Load(stream);

var attributes = new Dictionary<string, string>() { { "Version", "1.0.0" } };

ProjectFileUtils.AddItem(
xml,
"PackageReference",
packageX.Id,
string.Empty,
new Dictionary<string, string>(),
attributes);

ProjectFileUtils.WriteXmlToFile(xml, stream);
}

//set nuget.config properties
var doc = new XDocument();
var configuration = new XElement(XName.Get("configuration"));
doc.Add(configuration);

var config = new XElement(XName.Get("config"));
configuration.Add(config);

var signatureValidationMode = new XElement(XName.Get("add"));
signatureValidationMode.Add(new XAttribute(XName.Get("key"), "signatureValidationMode"));
signatureValidationMode.Add(new XAttribute(XName.Get("value"), "require"));
config.Add(signatureValidationMode);

File.WriteAllText(Path.Combine(workingDirectory, "NuGet.Config"), doc.ToString());

// Act
CommandRunnerResult result = _msbuildFixture.RunDotnet(
workingDirectory, "restore",
ignoreExitCode: true,
additionalEnvVars: new Dictionary<string, string>()
{
{ envVarName, envVarValue }
}
);

result.AllOutput.Should().NotContain($"error NU3004");
result.Success.Should().BeTrue();
result.ExitCode.Should().Be(0);
}
}

[PlatformFact(Platform.Linux, Platform.Darwin)]
public async Task DotnetRestore_WithUnSignedPackageAndSignatureValidationModeAsRequired_OptInEnvVar_Null_SucceedAsync()
{
using (var pathContext = _msbuildFixture.CreateSimpleTestPathContext())
{
//Arrange
var envVarName = "DOTNET_OPT_IN_SECURE_PACKAGE_VERIFICATION";
string envVarValue = null;
var envVarName = OptInPackageVerificationTypo;
var envVarValue = "xyz";
//Setup packages and feed
var packageX = new SimpleTestPackageContext()
{
Expand Down Expand Up @@ -520,7 +445,7 @@ public async Task DotnetRestore_WithUnSignedPackageAndSignatureValidationModeAsR
using (var pathContext = _msbuildFixture.CreateSimpleTestPathContext())
{
//Arrange
var envVarName = "dOTNET_OPT_IN_SECURE_PACKAGE_VERIFICATIOn";
var envVarName = OptInPackageVerificationTypo;
var envVarValue = "true";
//Setup packages and feed
var packageX = new SimpleTestPackageContext()
Expand Down
48 changes: 9 additions & 39 deletions test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageArchiveReaderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using System.Threading.Tasks;
using Moq;
using NuGet.Common;
using NuGet.Configuration;
using NuGet.Frameworks;
using NuGet.Packaging.Core;
using NuGet.Packaging.Signing;
Expand All @@ -23,6 +22,9 @@ namespace NuGet.Packaging.Test
{
public class PackageArchiveReaderTests
{
private const string OptInPackageVerification = "DOTNET_OPT_IN_SECURE_PACKAGE_VERIFICATION";
private const string OptInPackageVerificationTypo = "DOTNET_OPT_IN_SECURE_PACKAGE_VERIFICATIOn";

[Fact]
public void Constructor_WithStringPathParameter_DisposesInvalidStream()
{
Expand Down Expand Up @@ -1985,11 +1987,15 @@ public void CanVerifySignedPackages_ReturnsValueBasedOnOperatingSystemAndFramewo
[InlineData("true")]
[InlineData("TRUE")]
[InlineData("TRUe")]
[InlineData("FALSE")]
[InlineData("xyz")]
[InlineData("")]
[InlineData(null)]
public void CanVerifySignedPackages_ReturnsValueBasedOnOperatingSystemAndFramework_OptInEnvVar(string envVar)
{
// Arrange
var environment = new Mock<IEnvironmentVariableReader>(MockBehavior.Strict);
environment.Setup(s => s.GetEnvironmentVariable("DOTNET_OPT_IN_SECURE_PACKAGE_VERIFICATION")).Returns(envVar);
environment.Setup(s => s.GetEnvironmentVariable(OptInPackageVerification)).Returns(envVar);

using (var test = TestPackagesCore.GetPackageContentReaderTestPackage())
using (var packageArchiveReader = new PackageArchiveReader(test, environmentVariableReader: environment.Object))
Expand All @@ -2007,47 +2013,11 @@ public void CanVerifySignedPackages_ReturnsValueBasedOnOperatingSystemAndFramewo
}
}

[PlatformFact(Platform.Linux, Platform.Darwin)]
public void CanVerifySignedPackages_ReturnsValueBasedOnOperatingSystemAndFramework_OptInEnvVar_False_Fails()
{
// Arrange
string envVar = "false";
var environment = new Mock<IEnvironmentVariableReader>(MockBehavior.Strict);
environment.Setup(s => s.GetEnvironmentVariable("DOTNET_OPT_IN_SECURE_PACKAGE_VERIFICATION")).Returns(envVar);

using (var test = TestPackagesCore.GetPackageContentReaderTestPackage())
using (var packageArchiveReader = new PackageArchiveReader(test, environmentVariableReader: environment.Object))
{
// Act
bool result = packageArchiveReader.CanVerifySignedPackages(null);
// Assert
Assert.False(result);
}
}

[PlatformFact(Platform.Linux, Platform.Darwin)]
public void CanVerifySignedPackages_ReturnsValueBasedOnOperatingSystemAndFramework_OptInEnvVar_Null_Fails()
{
// Arrange
string envVar = null;
var environment = new Mock<IEnvironmentVariableReader>(MockBehavior.Strict);
environment.Setup(s => s.GetEnvironmentVariable("DOTNET_OPT_IN_SECURE_PACKAGE_VERIFICATION")).Returns(envVar);

using (var test = TestPackagesCore.GetPackageContentReaderTestPackage())
using (var packageArchiveReader = new PackageArchiveReader(test, environmentVariableReader: environment.Object))
{
// Act
bool result = packageArchiveReader.CanVerifySignedPackages(null);
// Assert
Assert.False(result);
}
}

[PlatformFact(Platform.Linux, Platform.Darwin)]
public void CanVerifySignedPackages_ReturnsValueBasedOnOperatingSystemAndFramework_WrongName_OptInEnvVar_Fails()
{
// Arrange
string envVarName = "dOTNET_OPT_IN_SECURE_PACKAGE_VERIFICATIOn";
string envVarName = OptInPackageVerificationTypo;
string envVarValue = "true";
var environment = new Mock<IEnvironmentVariableReader>(MockBehavior.Loose);
environment.Setup(s => s.GetEnvironmentVariable(envVarName)).Returns(envVarValue);
Expand Down
Loading

0 comments on commit fb609f6

Please sign in to comment.