WIP: Auth fixes for 1.x

src/chocolatey/infrastructure.app/commands/ChocolateyInfoCommand.cs

@@ -37,7 +37,7 @@ public override void configure_argument_parser(OptionSet optionSet, ChocolateyCo
                 .Add(
                     "s=|source=",
                     "Source - Source location for install. Can use special 'webpi' or 'windowsfeatures' sources. Defaults to sources.",
-                    option => configuration.Sources = option.remove_surrounding_quotes())
+                    option => configuration.Sources = configuration.ExplicitSources = option.remove_surrounding_quotes())
                 .Add(
                     "l|lo|localonly|local-only",
                     "LocalOnly - Only search against local machine items.",

src/chocolatey/infrastructure.app/commands/ChocolateyInstallCommand.cs

@@ -41,7 +41,7 @@ public virtual void configure_argument_parser(OptionSet optionSet, ChocolateyCon
             optionSet
                 .Add("s=|source=",
                      "Source - The source to find the package(s) to install. Special sources include: ruby, webpi, cygwin, windowsfeatures, and python. To specify more than one source, pass it with a semi-colon separating the values (e.g. \"'source1;source2'\"). Defaults to default feeds.",
-                     option => configuration.Sources = option.remove_surrounding_quotes())
+                     option => configuration.Sources = configuration.ExplicitSources = option.remove_surrounding_quotes())
                 .Add("version=",
                      "Version - A specific version to install. Defaults to unspecified.",
                      option => configuration.Version = option.remove_surrounding_quotes())

src/chocolatey/infrastructure.app/commands/ChocolateyListCommand.cs

@@ -51,7 +51,7 @@ public virtual void configure_argument_parser(OptionSet optionSet, ChocolateyCon
             optionSet
                 .Add("s=|source=",
                      "Source - Source location for install. Can use special 'webpi' or 'windowsfeatures' sources. Defaults to sources." + deprecationNotice,
-                     option => configuration.Sources = option.remove_surrounding_quotes())
+                     option => configuration.Sources = configuration.ExplicitSources = option.remove_surrounding_quotes())
                 .Add("l|lo|local|localonly|local-only",
                      localOnlyDescription,
                      option => configuration.ListCommand.LocalOnly = option != null)

src/chocolatey/infrastructure.app/commands/ChocolateyOutdatedCommand.cs

@@ -39,7 +39,7 @@ public virtual void configure_argument_parser(OptionSet optionSet, ChocolateyCon
             optionSet
                 .Add("s=|source=",
                      "Source - The source to find the package(s) to install. Special sources include: ruby, webpi, cygwin, windowsfeatures, and python. To specify more than one source, pass it with a semi-colon separating the values (e.g. \"'source1;source2'\"). Defaults to default feeds.",
-                     option => configuration.Sources = option.remove_surrounding_quotes())
+                     option => configuration.Sources = configuration.ExplicitSources = option.remove_surrounding_quotes())
                 .Add("u=|user=",
                      "User - used with authenticated feeds. Defaults to empty.",
                      option => configuration.SourceCommand.Username = option.remove_surrounding_quotes())

src/chocolatey/infrastructure.app/commands/ChocolateySourceCommand.cs

@@ -49,7 +49,7 @@ public virtual void configure_argument_parser(OptionSet optionSet, ChocolateyCon
                      option => configuration.SourceCommand.Name = option.remove_surrounding_quotes())
                 .Add("s=|source=",
                      "Source - The source. This can be a folder/file share or an http location. If it is a url, it will be a location you can go to in a browser and it returns OData with something that says Packages in the browser, similar to what you see when you go to https://community.chocolatey.org/api/v2/. Required with add action. Defaults to empty.",
-                     option => configuration.Sources = option.remove_surrounding_quotes())
+                     option => configuration.Sources = configuration.ExplicitSources = option.remove_surrounding_quotes())
                 .Add("u=|user=",
                      "User - used with authenticated feeds. Defaults to empty.",
                      option => configuration.SourceCommand.Username = option.remove_surrounding_quotes())

src/chocolatey/infrastructure.app/commands/ChocolateyUpgradeCommand.cs

@@ -41,7 +41,7 @@ public virtual void configure_argument_parser(OptionSet optionSet, ChocolateyCon
             optionSet
                 .Add("s=|source=",
                      "Source - The source to find the package(s) to install. Special sources include: ruby, webpi, cygwin, windowsfeatures, and python. To specify more than one source, pass it with a semi-colon separating the values (e.g. \"'source1;source2'\"). Defaults to default feeds.",
-                     option => configuration.Sources = option.remove_surrounding_quotes())
+                     option => configuration.Sources = configuration.ExplicitSources = option.remove_surrounding_quotes())
                 .Add("version=",
                      "Version - A specific version to install. Defaults to unspecified.",
                      option => configuration.Version = option.remove_surrounding_quotes())

src/chocolatey/infrastructure.app/configuration/ChocolateyConfiguration.cs

@@ -252,6 +252,12 @@ private void append_output(StringBuilder propertyValues, string append)
         /// </summary>
         public string Sources { get; set; }
 
+        /// <summary>
+        /// One or more source locations set by comamnd line only. Semi-colon delimited.
+        /// <strong>Do not set this anywhere other than parsing CLI arguments for commands.</strong>
+        /// </summary>
+        public string ExplicitSources { get; set; }
+
         public string SourceType { get; set; }
 
         // top level commands

src/chocolatey/infrastructure.app/nuget/ChocolateyNugetCredentialProvider.cs

@@ -79,59 +79,56 @@ public ICredentials GetCredentials(Uri uri, IWebProxy proxy, CredentialType cred
             }
 
             // credentials were not explicit
-            // discover based on closest match in sources
-            var candidateSources = _config.MachineSources.Where(
-                s =>
-                {
-                    var sourceUrl = s.Key.TrimEnd('/');
-
-                    try
-                    {
-                        var sourceUri = new Uri(sourceUrl);
-                        return sourceUri.Host.is_equal_to(uri.Host)
-                            && !string.IsNullOrWhiteSpace(s.Username)
-                            && !string.IsNullOrWhiteSpace(s.EncryptedPassword);
-                    }
-                    catch (Exception)
-                    {
-                        this.Log().Error("Source '{0}' is not a valid Uri".format_with(sourceUrl));
-                    }
-
-                    return false;
-                }).ToList();
-
+            // find matching source(s) in sources list
+            var trimmedTargetUri = new Uri(uri.AbsoluteUri.TrimEnd('/'));
             MachineSourceConfiguration source = null;
 
+            // If the user has specified --source with a *named* source and not a URL, try to find the matching one
+            // with the correct URL for this credential request.
+            var namedExplicitSources = _config.ExplicitSources?.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries)
+                .Where(s => !Uri.IsWellFormedUriString(s, UriKind.Absolute))
+                .ToList();
 
-            if (candidateSources.Count == 1)
+            if (namedExplicitSources?.Count > 0)
             {
-                // only one match, use it
-                source = candidateSources.FirstOrDefault();
+                // Uri.Equals() and == operator compare hostnames case-insensitively and the remainder of the url case-sensitively
+                // while ignoring #fragments on the URLs, but does care about trailing slashes, which we do not here.
+                source = _config.MachineSources
+                    .Where(s => namedExplicitSources.Contains(s.Name) && new Uri(s.Key.TrimEnd('/')) == trimmedTargetUri)
+                    .FirstOrDefault();
             }
-            else if (candidateSources.Count > 1)
+
+            if (source is null)
             {
-                // find the source that is the closest match
-                foreach (var candidateSource in candidateSources.or_empty_list_if_null())
+                // Could not find a valid source by name, or the source(s) specified were all URLs.
+                // Try to look up the target URL in the saved machine sources to attempt to match credentials.
+                //
+                // Note: This behaviour remains as removing it would be a breaking change, but we may want
+                // to remove this in a future version, as specifying an explicit URL should potentially
+                // not go looking in the configuration file for saved credentials anyway.
+                var candidateSources = _config.MachineSources
+                    .Where(s => !string.IsNullOrWhiteSpace(s.Username)
+                        && !string.IsNullOrWhiteSpace(s.EncryptedPassword)
+                        && Uri.TryCreate(s.Key.TrimEnd('/'), UriKind.Absolute, out var trimmedSourceUri)
+                        && trimmedSourceUri == trimmedTargetUri)
+                    .ToList();
+
+                if (candidateSources.Count == 1)
                 {
-                    var candidateRegEx = new Regex(Regex.Escape(candidateSource.Key.TrimEnd('/')),RegexOptions.CultureInvariant | RegexOptions.IgnoreCase);
-                    if (candidateRegEx.IsMatch(uri.OriginalString.TrimEnd('/')))
-                    {
-                        this.Log().Debug("Source selected will be '{0}'".format_with(candidateSource.Key.TrimEnd('/')));
-                        source = candidateSource;
-                        break;
-                    }
+                    // only one match, use it
+                    source = candidateSources.First();
                 }
-
-                if (source == null && !retrying)
+                else if (candidateSources.Count > 1 && !retrying)
                 {
-                    // use the first source. If it fails, fall back to grabbing credentials from the user
+                    // Use the credentials from the first found source, unless it's a retry (creds already tried and failed)
+                    // use the first source. If it fails, fall back to grabbing credentials from the user.
                     var candidateSource = candidateSources.First();
                     this.Log().Debug("Evaluated {0} candidate sources but was unable to find a match, using {1}".format_with(candidateSources.Count, candidateSource.Key.TrimEnd('/')));
                     source = candidateSource;
                 }
             }
 
-            if (source == null)
+            if (source is null)
             {
                 this.Log().Debug("Asking user for credentials for '{0}'".format_with(uri.OriginalString));
                 return get_credentials_from_user(uri, proxy, credentialType);

tests/helpers/common/Chocolatey/Disable-ChocolateySource.ps1

@@ -8,10 +8,8 @@ function Disable-ChocolateySource {
         [Parameter()]
         [switch]$All
     )
-    # Significantly weird behaviour with piping this source list by property name.
-    $CurrentSources = (Invoke-Choco source list -r).Lines | ConvertFrom-ChocolateyOutput -Command SourceList | Where-Object {
-        $_.Name -like $Name
-    }
+
+    $CurrentSources = Get-ChocolateySource -Name $Name
     foreach ($Source in $CurrentSources) {
         $null = Invoke-Choco source disable --name $Source.Name
     }

tests/helpers/common/Chocolatey/Enable-ChocolateySource.ps1

@@ -9,9 +9,7 @@ function Enable-ChocolateySource {
         [switch]$All
     )
     # Significantly weird behaviour with piping this source list by property name.
-    $CurrentSources = (Invoke-Choco source list -r).Lines | ConvertFrom-ChocolateyOutput -Command SourceList | Where-Object {
-        $_.Name -like $Name
-    }
+    $CurrentSources = Get-ChocolateySource -Name $Name
     foreach ($Source in $CurrentSources) {
         $null = Invoke-Choco source enable --name $Source.Name
     }

tests/helpers/common/Chocolatey/Get-ChocolateySource.ps1

@@ -0,0 +1,14 @@
+function Get-ChocolateySource {
+    [CmdletBinding()]
+    param(
+        [Parameter()]
+        [string]$Name = "*",
+
+        [Parameter()]
+        [switch]$All
+    )
+    # Significantly weird behaviour with piping this source list by property name.
+    (Invoke-Choco source list -r).Lines | ConvertFrom-ChocolateyOutput -Command SourceList | Where-Object {
+        $_.Name -like $Name
+    }
+}

tests/pester-tests/features/CredentialProvider.Tests.ps1

@@ -0,0 +1,62 @@
+Describe 'Ensuring credentials do not bleed from configured sources' -Tag CredentialProvider -ForEach @(
+    @{
+        Command = 'info'
+        ExitCode = 0
+}
+    @{
+        Command = 'install'
+        ExitCode = 1
+}
+    @{
+        Command = 'outdated'
+        ExitCode = 0
+}
+    @{
+        Command = 'search'
+        ExitCode = 1
+}
+    @{
+        Command = 'upgrade'
+        ExitCode = 1
+}
+    @{
+        Command = 'download'
+        ExitCode = 1
+}
+) {
+    BeforeDiscovery {
+        $HasLicensedExtension = Test-PackageIsEqualOrHigher -PackageName 'chocolatey.extension' -Version '6.0.0'
+    }
+
+    BeforeAll {
+        Initialize-ChocolateyTestInstall
+        Disable-ChocolateySource -All
+        Enable-ChocolateySource -Name 'hermes'
+        $SetupSource = Get-ChocolateySource -Name 'hermes-setup'
+        Remove-Item download -force -recurse
+    }
+
+    # Skip the download command if chocolatey.extension is not installed.
+    Context 'Command (<Command>)' -Skip:($Command -eq 'download' -and -not $HasLicensedExtension) {
+        BeforeAll {
+            # Picked a package that is on `hermes-setup` but not on `hermes`.
+            $PackageUnderTest = 'chocolatey-compatibility.extension'
+            Restore-ChocolateyInstallSnapshot
+            # Chocolatey will prompt for credentials, we need to force something in there, and this will do that.
+            $Output = 'n' | Invoke-Choco $Command $PackageUnderTest --confirm --source="'$($SetupSource.Url)'"
+        }
+
+        AfterAll {
+            Remove-ChocolateyInstallSnapshot
+        }
+
+        It 'Exits Correctly (<ExitCode>)' {
+            $Output.ExitCode | Should -Be $ExitCode -Because $Output.String
+        }
+
+        It 'Outputs error message' {
+            $FilteredOutput = $Output.Lines -match "Failed to fetch results from V2 feed at '$($SetupSource.Url.Trim('/'))"
+            $FilteredOutput.Count | Should -BeGreaterOrEqual 1 -Because $Output.String
+        }
+    }
+}