Deprecate http usage: source commands, promote warning to error for…

… http sources (#5702)

src/NuGet.Core/NuGet.Commands/NuGet.Commands.csproj

@@ -65,5 +65,6 @@
     <InternalsVisibleTo Include="NuGet.SolutionRestoreManager.Test" />
     <InternalsVisibleTo Include="Test.Utility" />
     <InternalsVisibleTo Include="NuGet.Commands.FuncTest"/>
+    <InternalsVisibleTo Include="Dotnet.Integration.Test" />
   </ItemGroup>
 </Project>

src/NuGet.Core/NuGet.Commands/SourcesCommands/SourceRunners.cs

@@ -66,9 +66,9 @@ public static void Run(AddSourceArgs args, Func<ILogger> getLogger)
 
             if (newPackageSource.IsHttp && !newPackageSource.IsHttps && !newPackageSource.AllowInsecureConnections)
             {
-                getLogger().LogWarning(
+                throw new ArgumentException(
                     string.Format(CultureInfo.CurrentCulture,
-                        Strings.Warning_HttpServerUsage,
+                        Strings.Error_HttpSource_Single_Short,
                         "add source",
                         args.Source));
             }
@@ -188,8 +188,6 @@ public static void Run(ListSourceArgs args, Func<ILogger> getLogger)
                             legend += " ";
                             getLogger().LogMinimal(legend + source.Source);
                         }
-
-                        WarnForHttpSources(sourcesList, getLogger);
                     }
                     break;
                 case SourcesListFormat.None:
@@ -219,16 +217,14 @@ private static void WarnForHttpSources(IEnumerable<PackageSource> sources, Func<
                 {
                     getLogger().LogWarning(
                     string.Format(CultureInfo.CurrentCulture,
-                        Strings.Warning_HttpServerUsage,
-                        "list source",
+                        Strings.Warning_List_HttpSource,
                         httpPackageSources[0]));
                 }
                 else
                 {
                     getLogger().LogWarning(
                             string.Format(CultureInfo.CurrentCulture,
-                            Strings.Warning_HttpServerUsage_MultipleSources,
-                            "list source",
+                            Strings.Warning_List_HttpSources,
                             Environment.NewLine + string.Join(Environment.NewLine, httpPackageSources.Select(e => e.Name))));
                 }
             }
@@ -286,10 +282,10 @@ public static void Run(UpdateSourceArgs args, Func<ILogger> getLogger)
                 existingSource = new Configuration.PackageSource(args.Source, existingSource.Name);
                 existingSource.AllowInsecureConnections = args.AllowInsecureConnections;
 
-                // If the existing source is not http, warn the user
+                // If the new source is not http, throw an error
                 if (existingSource.IsHttp && !existingSource.IsHttps && !existingSource.AllowInsecureConnections)
                 {
-                    getLogger().LogWarning(string.Format(CultureInfo.CurrentCulture, Strings.Warning_HttpServerUsage, "update source", args.Source));
+                    throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Strings.Error_HttpSource_Single, "update source", args.Source));
                 }
             }
 
@@ -383,7 +379,7 @@ public static void EnableOrDisableSource(PackageSourceProvider sourceProvider, s
                     Strings.SourcesCommandSourceEnabledSuccessfully, name));
                 if (packageSource.IsHttp && !packageSource.IsHttps && !packageSource.AllowInsecureConnections)
                 {
-                    getLogger().LogWarning(string.Format(CultureInfo.CurrentCulture, Strings.Warning_HttpServerUsage, "enable source", packageSource.Source));
+                    throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Strings.Error_HttpSource_Single, "enable source", packageSource.Source));
                 }
             }
             else

src/NuGet.Core/NuGet.Commands/Strings.resx

@@ -1100,11 +1100,25 @@ Non-HTTPS access will be removed in a future version. Consider migrating to 'HTT
     <value>The protocol version specified is invalid. Valid protocol versions are from {0} to {1}.</value>
   </data>
   <data name="Error_HttpSources_Multiple" xml:space="preserve">
-    <value>You are running the '{0}' operation with 'HTTP' sources: {1}. NuGet requires HTTPS sources. To use HTTP sources, you must explicitly set 'allowInsecureConnections' to true in your NuGet.Config file. Please refer to https://aka.ms/nuget-https-everywhere.</value>
+    <value>You are running the '{0}' operation with 'HTTP' sources: {1}. NuGet requires HTTPS sources. To use HTTP sources, you must explicitly set 'allowInsecureConnections' to true in your NuGet.Config file. Please refer to https://aka.ms/nuget-https-everywhere for more information.</value>
     <comment>0 - The command name. Ex. Push/Delete. 1 - list of server URIs</comment>
   </data>
   <data name="Error_HttpSource_Single" xml:space="preserve">
-    <value>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.</value>
+    <value>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 for more information.</value>
     <comment>0 - The command name. Ex. Push/Delete. 1 - server URI.</comment>
   </data>
+  <data name="Error_HttpSource_Single_Short" xml:space="preserve">
+    <value>You are running the '{0}' operation with an 'HTTP' source: {1}. NuGet requires HTTPS sources. Please refer to https://aka.ms/nuget-https-everywhere for more information.</value>
+    <comment>0 - The command name. Ex. Push/Delete. 1 - server URI.</comment>
+  </data>
+  <data name="Warning_List_HttpSource" xml:space="preserve">
+    <value>The following is a 'Non-HTTPS' source: {0}
+NuGet requires HTTPS sources. Please refer to https://aka.ms/nuget-https-everywhere for more information.</value>
+    <comment>{0} - list of server URIs</comment>
+  </data>
+  <data name="Warning_List_HttpSources" xml:space="preserve">
+    <value>The following are 'Non-HTTPS' sources: {0}
+NuGet requires HTTPS sources. Please refer to https://aka.ms/nuget-https-everywhere for more information.</value>
+    <comment>{0} - list of server URIs</comment>
+  </data>
 </root>

test/NuGet.Clients.FuncTests/NuGet.CommandLine.FuncTest/Commands/PushCommandTest.cs

@@ -3,7 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Globalization;
 using System.IO;
 using System.Net;
 using System.Threading;
@@ -26,7 +25,7 @@ public class PushCommandTest
         private const string ADVERTISE_SKIPDUPLICATE_OPTION = "To skip already published packages, use the option -SkipDuplicate"; //PushCommandSkipDuplicateAdvertiseNuGetExe
         private const string WITHOUT_FILENAME_MESSAGE_FILE_DOES_NOT_EXIST = "File does not exist";
         private const string MESSAGE_FILE_DOES_NOT_EXIST = WITHOUT_FILENAME_MESSAGE_FILE_DOES_NOT_EXIST + " ({0})";
-        private readonly ITestOutputHelper _testOutputHelper;	        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.";
+        private readonly ITestOutputHelper _testOutputHelper;
 
         public PushCommandTest(ITestOutputHelper testOutputHelper)
         {
@@ -861,7 +860,7 @@ public void PushCommand_Server_Snupkg_ByFilename_SnupkgExists_Conflict_ServerMes
         }
 
         [Fact]
-        public void PushCommand_WhenPushingToAnHttpServerV3_WithSymbols_Warns()
+        public void PushCommand_WhenPushingToAnHttpServerV3_WithSymbols_Errors()
         {
             // Arrange
             using var packageDirectory = TestDirectory.Create();
@@ -873,7 +872,6 @@ public void PushCommand_WhenPushingToAnHttpServerV3_WithSymbols_Warns()
 
             CommandRunnerResult result = null;
             using var server = CreateAndStartMockV3Server(packageDirectory, out string sourceName);
-            string expectedError = string.Format(CultureInfo.CurrentCulture, _httpErrorSingle, "push", sourceName);
             SetupMockServerAlwaysCreate(server);
             // Act
             result = CommandRunner.Run(
@@ -885,7 +883,7 @@ public void PushCommand_WhenPushingToAnHttpServerV3_WithSymbols_Warns()
 
             // Assert
             Assert.False(result.Success, result.AllOutput);
-            Assert.Contains(expectedError, result.AllOutput);
+            Assert.Contains(sourceName, result.Errors);
         }
 
         #region Helpers

test/NuGet.Clients.FuncTests/NuGet.CommandLine.FuncTest/Commands/SearchCommandTests.cs

@@ -934,12 +934,7 @@ public void SearchCommand_WhenSearchWithHttpSource_DisplaysAnErrorMessage()
             using MockServer server = new MockServer();
             PackageSource source = new PackageSource(server.Uri + "v3/index.json", "mockSource");
             using SimpleTestPathContext config = new SimpleTestPathContext();
-            CommandRunner.Run(
-                nugetexe,
-                config.WorkingDirectory,
-                $"source add -name mockSource -source {server.Uri}v3/index.json -configfile {config.NuGetConfig}",
-                    testOutputHelper: _testOutputHelper);
-
+            config.Settings.AddSource("mockSource", $"{server.Uri}v3/index.json");
             string index = $@"
                 {{
                     ""version"": ""3.0.0"",

test/NuGet.Clients.Tests/NuGet.CommandLine.Test/NuGetDeleteCommandTest.cs

@@ -3,7 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Globalization;
 using System.IO;
 using System.Net;
 using NuGet.Test.Utility;
@@ -16,7 +15,6 @@ 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]
@@ -433,10 +431,52 @@ public void DeleteCommand_Failure_InvalidArguments(string args)
             Util.TestCommandInvalidArguments(args);
         }
 
-        [Theory]
-        [InlineData("true", false)]
-        [InlineData("false", true)]
-        public void DeleteCommand_WhenDeleteWithHttpSourceAndAllowInsecureConnections_DisplaysErrorCorrectly(string allowInsecureConnections, bool shouldFail)
+        [Fact]
+        public void DeleteCommand_WhenDeleteWithHttpSourceAndAllowInsecureConnectionsFalse_Errors()
+        {
+            var nugetexe = Util.GetNuGetExePath();
+
+            // Arrange
+            using (var server = new MockServer())
+            {
+                server.Start();
+
+                server.Delete.Add("/nuget/testPackage1/1.1", request =>
+                {
+                    return HttpStatusCode.OK;
+                });
+
+                using SimpleTestPathContext config = new SimpleTestPathContext();
+
+                // Arrange the NuGet.Config file
+                string nugetConfigContent =
+    $@"<configuration>
+    <packageSources>
+        <clear />
+        <add key='http-feed' value='{server.Uri}nuget' protocalVersion=""3"" allowInsecureConnections=""False"" />
+    </packageSources>
+</configuration>";
+                File.WriteAllText(config.NuGetConfig, nugetConfigContent);
+
+                // Act
+                string[] args = new string[] {
+                    "delete", "testPackage1", "1.1.0",
+                    "-Source", server.Uri + "nuget",
+                    "-ConfigFile", config.NuGetConfig, "-NonInteractive" };
+
+                var result = CommandRunner.Run(
+                    nugetexe,
+                    Directory.GetCurrentDirectory(),
+                    string.Join(" ", args));
+
+                // Assert
+                Assert.Equal(1, result.ExitCode);
+                Assert.Contains($"{server.Uri}nuget", result.Errors);
+            }
+        }
+
+        [Fact]
+        public void DeleteCommand_WhenDeleteWithHttpSourceAndAllowInsecureConnectionsTrue_Succeeds()
         {
             var nugetexe = Util.GetNuGetExePath();
 
@@ -459,11 +499,10 @@ public void DeleteCommand_WhenDeleteWithHttpSourceAndAllowInsecureConnections_Di
     $@"<configuration>
     <packageSources>
         <clear />
-        <add key='http-feed' value='{server.Uri}nuget' protocalVersion=""3"" allowInsecureConnections=""{allowInsecureConnections}"" />
+        <add key='http-feed' value='{server.Uri}nuget' protocalVersion=""3"" allowInsecureConnections=""True"" />
     </packageSources>
 </configuration>";
                 File.WriteAllText(config.NuGetConfig, nugetConfigContent);
-                string expectedError = string.Format(CultureInfo.CurrentCulture, _httpErrorSingle, "delete", $"{server.Uri}nuget");
 
                 // Act
                 string[] args = new string[] {
@@ -477,17 +516,9 @@ public void DeleteCommand_WhenDeleteWithHttpSourceAndAllowInsecureConnections_Di
                     string.Join(" ", args));
 
                 // Assert
-                if (shouldFail)
-                {
-                    Assert.Equal(1, result.ExitCode);
-                    Assert.Contains(expectedError, result.AllOutput);
-                }
-                else
-                {
-                    Assert.Equal(0, result.ExitCode);
-                    Assert.True(deleteRequestIsCalled);
-                    Assert.DoesNotContain(expectedError, result.AllOutput);
-                }
+                Assert.Equal(0, result.ExitCode);
+                Assert.True(deleteRequestIsCalled);
+                Assert.DoesNotContain($"{server.Uri}nuget", result.Errors);
             }
         }