Skip to content

Commit

Permalink
Deprecate http usage: delete operations, promote warning to error f…
Browse files Browse the repository at this point in the history 
…or http sources (#5703)
  • Loading branch information
@Nigusu-Allehu
Nigusu-Allehu committed Jul 2, 2024
1 parent d34deb6 commit 96ab115
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 19 deletions.
7 changes: 3 additions & 4 deletions src/NuGet.Core/NuGet.Commands/CommandRunners/DeleteRunner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ public static async Task Run(
{
source = CommandRunnerUtility.ResolveSource(sourceProvider, source);
PackageSource packageSource = CommandRunnerUtility.GetOrCreatePackageSource(sourceProvider, source);
// Only warn for V3 style sources because they have a service index which is different from the final push url.
if (packageSource.IsHttp && !packageSource.IsHttps && !packageSource.AllowInsecureConnections &&
(packageSource.ProtocolVersion == 3 || packageSource.Source.EndsWith("json", StringComparison.OrdinalIgnoreCase)))
// Throw an error if an http source is used without setting AllowInsecureConnections
if (packageSource.IsHttp && !packageSource.IsHttps && !packageSource.AllowInsecureConnections)
{
logger.LogWarning(string.Format(CultureInfo.CurrentCulture, Strings.Warning_HttpServerUsage, "delete", packageSource.Source));
throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Strings.Error_HttpSource_Single, "delete", packageSource.Source));
}
var packageUpdateResource = await CommandRunnerUtility.GetPackageUpdateResource(sourceProvider, packageSource, CancellationToken.None);

Expand Down
49 changes: 34 additions & 15 deletions test/NuGet.Clients.Tests/NuGet.CommandLine.Test/NuGetDeleteCommandTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Net;
using NuGet.Test.Utility;
Expand All @@ -15,6 +16,7 @@ public class NuGetDeleteCommandTest
{
private const string ApiKeyHeader = "X-NuGet-ApiKey";
private static readonly string NuGetExePath = Util.GetNuGetExePath();
private string _httpErrorSingle = "You are running the '{0}' operation with an 'HTTP' source: {1}. NuGet requires HTTPS sources. To use an HTTP source, you must explicitly set 'allowInsecureConnections' to true in your NuGet.Config file. Please refer to https://aka.ms/nuget-https-everywhere.";

// Tests deleting a package from a source that is a file system directory.
[Fact]
Expand Down Expand Up @@ -181,11 +183,15 @@ public void DeleteCommand_DeleteFromHttpSource()
deleteRequestIsCalled = true;
return HttpStatusCode.OK;
});
using SimpleTestPathContext pathContext = new SimpleTestPathContext();
pathContext.Settings.AddSource("http-feed", $"{server.Uri}nuget", allowInsecureConnectionsValue: "true");
var configFileName = "nuget.config";
var configFilePath = Path.Combine(pathContext.WorkingDirectory, configFileName);

// Act
string[] args = new string[] {
"delete", "testPackage1", "1.1.0",
"-Source", server.Uri + "nuget", "-NonInteractive" };
"-Source", server.Uri + "nuget", "-NonInteractive", "-ConfigFile " + configFilePath };

var r = CommandRunner.Run(
nugetexe,
Expand Down Expand Up @@ -218,6 +224,10 @@ public void DeleteCommand_WithApiKeyAsThirdArgument()
});

server.Start();
using SimpleTestPathContext pathContext = new SimpleTestPathContext();
pathContext.Settings.AddSource("http-feed", $"{server.Uri}nuget", allowInsecureConnectionsValue: "true");
var configFileName = "nuget.config";
var configFilePath = Path.Combine(pathContext.WorkingDirectory, configFileName);

// Act
var args = new[] {
Expand All @@ -227,7 +237,8 @@ public void DeleteCommand_WithApiKeyAsThirdArgument()
testApiKey,
"-Source",
server.Uri + "nuget",
"-NonInteractive"
"-NonInteractive",
"-ConfigFile " + configFilePath
};

var result = CommandRunner.Run(
Expand Down Expand Up @@ -263,6 +274,10 @@ public void DeleteCommand_WithApiKeyAsNamedArgument()
});

server.Start();
using SimpleTestPathContext pathContext = new SimpleTestPathContext();
pathContext.Settings.AddSource("http-feed", $"{server.Uri}nuget", allowInsecureConnectionsValue: "true");
var configFileName = "nuget.config";
var configFilePath = Path.Combine(pathContext.WorkingDirectory, configFileName);

// Act
var args = new[]
Expand All @@ -275,7 +290,8 @@ public void DeleteCommand_WithApiKeyAsNamedArgument()
testApiKey,
"-Source",
server.Uri + "nuget",
"-NonInteractive"
"-NonInteractive",
"-ConfigFIle " + configFilePath
};

var result = CommandRunner.Run(
Expand Down Expand Up @@ -336,7 +352,7 @@ public void DeleteCommand_WithApiKeyFromConfig(string configKeyFormatString)

var source = server.Uri + "index.json";
var packageSourcesSection = SimpleTestSettingsContext.GetOrAddSection(settings.XML, "packageSources");
SimpleTestSettingsContext.AddEntry(packageSourcesSection, $"MockServer", source);
SimpleTestSettingsContext.AddEntry(packageSourcesSection, $"MockServer", source, "AllowInsecureConnections", "true");

var configKey = string.Format(configKeyFormatString, server.Uri);
var configValue = Configuration.EncryptionUtility.EncryptString(testApiKey);
Expand Down Expand Up @@ -380,15 +396,18 @@ public void DeleteCommand_ShowsServerWarnings(string firstServerWarning, string
using (var server = new MockServer())
{
server.Start();

using SimpleTestPathContext pathContext = new SimpleTestPathContext();
pathContext.Settings.AddSource("http-feed", $"{server.Uri}nuget", allowInsecureConnectionsValue: "true");
var configFileName = "nuget.config";
var configFilePath = Path.Combine(pathContext.WorkingDirectory, configFileName);
server.Delete.Add("/nuget/testPackage1/1.1", request => HttpStatusCode.OK);

server.AddServerWarnings(serverWarnings);

// Act
string[] args = new string[] {
"delete", "testPackage1", "1.1.0",
"-Source", server.Uri + "nuget", "-NonInteractive" };
"-Source", server.Uri + "nuget", "-NonInteractive", "-ConfigFile " + configFilePath };

var r = CommandRunner.Run(
nugetexe,
Expand Down Expand Up @@ -417,7 +436,7 @@ public void DeleteCommand_Failure_InvalidArguments(string args)
[Theory]
[InlineData("true", false)]
[InlineData("false", true)]
public void DeleteCommand_WhenDeleteWithHttpSourceAndAllowInsecureConnections_WarnsCorrectly(string allowInsecureConnections, bool isHttpWarningExpected)
public void DeleteCommand_WhenDeleteWithHttpSourceAndAllowInsecureConnections_DisplaysErrorCorrectly(string allowInsecureConnections, bool shouldFail)
{
var nugetexe = Util.GetNuGetExePath();

Expand All @@ -444,30 +463,30 @@ public void DeleteCommand_WhenDeleteWithHttpSourceAndAllowInsecureConnections_Wa
</packageSources>
</configuration>";
File.WriteAllText(config.NuGetConfig, nugetConfigContent);
string expectedError = string.Format(CultureInfo.CurrentCulture, _httpErrorSingle, "delete", $"{server.Uri}nuget");

// Act
string[] args = new string[] {
"delete", "testPackage1", "1.1.0",
"-Source", server.Uri + "nuget",
"-ConfigFile", config.NuGetConfig, "-NonInteractive" };

var r = CommandRunner.Run(
var result = CommandRunner.Run(
nugetexe,
Directory.GetCurrentDirectory(),
string.Join(" ", args));

// Assert
Assert.Equal(0, r.ExitCode);
Assert.True(deleteRequestIsCalled);

string expectedWarning = "WARNING: You are running the 'delete' operation with an 'HTTP' source";
if (isHttpWarningExpected)
if (shouldFail)
{
Assert.Contains(expectedWarning, r.AllOutput);
Assert.Equal(1, result.ExitCode);
Assert.Contains(expectedError, result.AllOutput);
}
else
{
Assert.DoesNotContain(expectedWarning, r.AllOutput);
Assert.Equal(0, result.ExitCode);
Assert.True(deleteRequestIsCalled);
Assert.DoesNotContain(expectedError, result.AllOutput);
}
}
}
Expand Down

0 comments on commit 96ab115

Please sign in to comment.