Opt-out of "HTTPS everywhere" for other scenarios when allowInsecureConnections is set to true

[heng-liu]

Check if there is any other scenario we should consider:

• MSBuild NuGet SDK resolver scenario
• donet tool and dotnet workload scenario

[heng-liu]

• dotnet tool: It runs implicate restore, so it has the same behavior with dotnet restore Verify with setting up an http source, and have the following in NuGet.Config file:

  <packageSources>
  <clear /> 
  <add key="baget" value="http://localhost:5000/v3/index.json" allowInsecureConnections="true" />
  </packageSources>

  Then run following commands: dotnet tool install --local dotnetsay --version 2.1.7 ==> has the http warning C:\PatchedSDK\dotnet.exe tool install --local dotnetsay --version 2.1.7 ==> doesn't have the http warning

• dotnet workload: there is no http warning before and after, so no need to opt-out http warning in this scenario Verify with setting up an http source, and have the following in NuGet.Config file:

  <packageSources>
  <clear /> 
  <add key="baget" value="http://localhost:5000/v3/index.json" allowInsecureConnections="true" />
  </packageSources>

  Then run following commands: dotnet workload install tvos ==> no http warning C:\PatchedSDK\dotnet.exe workload install tvos ==> no http warning

• MSBuild NuGet SDK resolver: The Resolve method calls into RunWithoutCommit which calls the same method checking http source during restore in package reference. So it has the same behavior with package reference restore. Verify with writing a test as below and it passes successfully:

      [Theory]
      [InlineData("true", false)]
      [InlineData("false", true)]
      public async Task Resolve_WhenNuGetConfigWithAllowInsecureConnections_ReturnsSdkResultAndLogsHttpWarningCorrectlyAsync(string allowInsecureConnections, bool isHttpWarningExpected)
      {
          using (var pathContext = new SimpleTestPathContext())
          {
              var sdkReference = new SdkReference(PackageA, VersionOnePointZero, minimumVersion: null);
              var sdkResolverContext = new MockSdkResolverContext(pathContext.WorkingDirectory);
              var sdkResultFactory = new MockSdkResultFactory();
              var sdkResolver = new NuGetSdkResolver();
  
              var packageA = new SimpleTestPackageContext(PackageA, VersionOnePointZero);
              await SimpleTestPackageUtility.CreatePackagesAsync(
                  pathContext.PackageSource,
                  packageA);
  
              using var mockServer = new FileSystemBackedV3MockServer(pathContext.PackageSource);
              mockServer.Start();
              pathContext.Settings.AddSource("http-source", mockServer.ServiceIndexUri, allowInsecureConnections);
              await SimpleTestPackageUtility.CreateFolderFeedV3Async(
                  pathContext.PackageSource,
                  PackageSaveMode.Defaultv3,
                  new SimpleTestPackageContext(PackageA, VersionOnePointZero));
  
              pathContext.Settings.AddSource("http-source", mockServer.ServiceIndexUri, allowInsecureConnections);
  
              MockSdkResult result = sdkResolver.Resolve(sdkReference, sdkResolverContext, sdkResultFactory) as MockSdkResult;
              mockServer.Stop();
  
              Assert.True(result.Success);
              Assert.Empty(result.Errors);
  
              if (isHttpWarningExpected)
              {
                  Assert.Equal(1, result.Warnings.Count());
                  Assert.Contains("You are running the 'restore' operation with an 'HTTP' source", result.Warnings.Single());
              }
              else
              {
                  Assert.Empty(result.Warnings);
              }
          }
      }

  Thanks for @zivkan who helped identifying those scenarios!

[heng-liu]

This could be closed as fixed.

[baronfel]

@heng-liu can you verify this behavior with Annie and Daniel from the SDK team when you have a moment? They are working on making .NET tools install without an implicit restore and I don't want to lose the things you've verified here.

[heng-liu]

Thanks @baronfel ! When https://github.com/dotnet/sdk/pull/33835 is merged, there will be no http source warning for dotnet tool install scenario, so we don't have to provide opt-out options in this case.