Initialize NuGet's Credential Service #868

ransagy · 2019-08-28T09:48:41Z

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Somewhere between feature and bug fix.

⤵️ What is the current behavior?

When performing NuGet operations against private/authenticated feeds, Current behavior assumes that auth is done solely via basic http authentication. This is not always true in some cases (Azure DevOps Artifacts in my case) and NuGet has a plugin framework for credentials management.

🆕 What is the new behavior (if this is a feature change)?

We initialize NuGet's credentials service before performing operations, allowing the usage of NuGet's own providers for that. In my case, Microsoft's Azure Artifacts credentials provider.

💥 Does this PR introduce a breaking change?

Possibly, If a user has credentials providers installed that will override the old auth system.

🐛 Recommendations for testing

The auth tests i saw seem to cover these cases, But we might consider adding more tests that specifically use the credential providers and/or Azure Artifacts authentication. It might be too much of an overhead though.

📝 Links to relevant issues/docs

https://docs.microsoft.com/en-us/nuget/reference/extensibility/nuget-exe-credential-providers
https://github.com/microsoft/artifacts-credprovider

#542

🤔 Checklist before submitting

All projects build
Relevant documentation was updated (Not sure what constitutes relevant in this case - I'll be happy to do so with some pointers)

ransagy · 2019-08-28T10:05:35Z

NuKeeper/Local/LocalEngine.cs

@@ -43,6 +44,8 @@ public class LocalEngine : ILocalEngine

        public async Task Run(SettingsContainer settings, bool write)
        {
+            DefaultCredentialServiceUtility.SetupDefaultCredentialService(new NuGet.Common.NullLogger(), true);


I'm definitely open to suggestions here; Wasn't sure where the best place is to put this.
It's also possible we want to use a different logger and/or to do something to integrate NuKeeper's logger with the basic NuGet one?

If you'd add a dependency of NuGet.Common.ILogger to the constructor, you'll get one injected that will forward the events to the NuKeeper logging.

Other than that, I think this looks good (other than the fact it's awkward static setup, but that's how this code works in NuGet itself and not much we can do about it).

I'll fix that ASAP then, Thanks.

I'm not sure about this but should this Setup-method also be called in CollaborationEngine?

@skolima Thanks to @Greybird this is now fixed.
As per @MartinDemberger 's comment, I'd need another pair of eyes on that.

@ransagy I think I need you're fix in the CollaborationEngine. When I call runkeeper repo on a Azure devops nuget feed it can't find any packages. If my repo doesn't have a NuGet.config and I set credentials in my profile NuGet.config it works. But if my repository does have a NuGet.config without credentials it fails.
To bypass this I call nukeeper with --source, but this too fails because of credentials.

I tried to update GitRepositoryEngine with the line
DefaultCredentialServiceUtility.SetupDefaultCredentialService(_nugetLogger, true);
but when executing nukeeper I get the error:
Problem starting the plugin 'C:\Users%username%.nuget\plugins\netcore\CredentialProvider.Microsoft\CredentialProvider.Microsoft.dll'. Cannot start process because a file name has not been provided.

Can you help?

@johntyrrell The latter error sounds like the missing dotnet env path var: NuGet/Home#7438

@ransagy when I set the DOTNET_HOST_PATH like this:
[System.Environment]::SetEnvironmentVariable("DOTNET_HOST_PATH", "C:\Program Files\dotnet")

The error changes to this:
Problem starting the plugin 'C:\Users%USERNAME%.nuget\plugins\netcore\CredentialProvider.Microsoft\CredentialProvider.Microsoft.dll'. Access is denied.
System.ComponentModel.Win32Exception (5): Access is denied.
at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
at System.Diagnostics.Process.Start()
at NuGet.Protocol.Plugins.PluginProcess.Start()
at NuGet.Protocol.Plugins.PluginFactory.CreatePluginAsync(String filePath, IEnumerable1 arguments, IRequestHandlers requestHandlers, ConnectionOptions options, CancellationToken sessionCancellationToken) at NuGet.Protocol.Plugins.PluginFactory.GetOrCreateAsync(String filePath, IEnumerable1 arguments, IRequestHandlers requestHandlers, ConnectionOptions options, CancellationToken sessionCancellationToken)
at NuGet.Protocol.Plugins.PluginManager.TryCreatePluginAsync(PluginDiscoveryResult result, OperationClaim requestedOperationClaim, PluginRequestKey requestKey, String packageSourceRepository, JObject serviceIndex, CancellationToken cancellationToken)

Any ideas?

I think that env var needs to be the full path to the actual dotnet exe - In my (local machine) case i just set it to 'dotnet' so it will use whatever is installed in the path.

About permissions - No idea, I haven't used the pipeline scenario at all.

@ransagy , changing it do "dotnet" did the trick. After that I needed to also set NUGET_PLUGIN_HANDSHAKE_TIMEOUT_IN_SECONDS and NUGET_PLUGIN_REQUEST_TIMEOUT_IN_SECONDS to "30" and then I was able to read the Azure DevOps feed. Thnx!

ransagy · 2019-10-27T18:41:57Z

Tests are broken due to a non-related issue to this PR; see my PR to fix that in #894 .

* Initialize NuGet's Credential Service * Rebased and switched to the expected Logger (#2) * 📦 Automatic update of NUnit3TestAdapter to 3.15.1 (NuKeeperDotNet#869) * 📦 Automatic update of SimpleInjector to 4.7.1 (NuKeeperDotNet#870) * 📦 Automatic update of LibGit2Sharp to 0.26.1 (NuKeeperDotNet#874) * Automatic update of NuGet.CommandLine to 5.2.0 (NuKeeperDotNet#875) * 📦 Automatic update of NuGet.CommandLine to 5.2.0 * Bump the packaged NuGet version * 📦 Automatic update of Octokit to 0.34.0 (NuKeeperDotNet#880) * FxCopAnalyzers already pulls in other analyzers (NuKeeperDotNet#883) * Automatic update of Microsoft.NET.Test.Sdk to 16.3.0 (NuKeeperDotNet#884) * 📦 Automatic update of Microsoft.NET.Test.Sdk to 16.3.0 * Fix mismatched test project target framework * Automatic update of coverlet.msbuild to 2.7.0 (NuKeeperDotNet#885) * 📦 Automatic update of coverlet.msbuild to 2.7.0 * Remove duplicate coverlet privateassets node * 📦 Automatic update of NuGet.Protocol to 5.3.0 (NuKeeperDotNet#886) * 📦 Automatic update of System.Text.Encoding.CodePages to 4.6.0 (NuKeeperDotNet#887) * 📦 Automatic update of McMaster.Extensions.CommandLineUtils to 2.4.2 (NuKeeperDotNet#888) * 📦 Automatic update of Octokit to 0.36.0 (NuKeeperDotNet#890) * Initialize NuGet's Credential Service * Switch to nuget logger

MarcBruins · 2019-12-03T16:44:42Z

Is this ready to be reviewed again @ransagy?

ransagy · 2019-12-03T17:16:27Z

@MarcBruins Pending any additional comment in the thread about LocalEngine.cs but generally, yes indeed.

nullabletype · 2020-02-11T14:11:19Z

Any movement on this? It's something I've just run into as well 🙃

ransagy · 2020-02-11T14:12:45Z

Still waiting for approval/further review by the maintainers, as specified.
@MarcBruins can someone take a look? IIRC @skolima no longer reviews these as per some of comments on other open issues here.

pcghusit · 2020-03-20T18:53:27Z

Does anybody know when this might be released to a version (usable by the Azure pipeline extension)? Running into this on our Pipeline builds and it's a show-stopper.

@AnthonySteele ?

scp-mb · 2020-04-22T16:47:43Z

Also running up against this. Getting nukeeper to work via a devops pipeline either through the extension or a powershell script is headache inducing.

@AnthonySteele Any chance we can get an update on when to expect a new release?

josundt · 2020-06-03T12:45:59Z

I am also awaiting this to be released. When can I expect a release that includes this?

ransagy · 2020-06-03T13:03:01Z

I'm quite sure it was already included in the 0.28 and 0.29 NuGet releases; Just wasn't updated on the GitHub releases due to a bug, i think.
I'm not aware of the semantics needed for the pipeline scenario, Is there anything else required besides the tool being updated on NuGet?

AnthonySteele · 2020-06-03T14:24:36Z

That's correct, it's on the last NuGet releases, but there was an issue with release notes.
I will look into updating the ADO Extensions with the next release.

ransagy marked this pull request as ready for review August 28, 2019 10:01

ransagy mentioned this pull request Aug 28, 2019

Error on Private Source (AzureDevOps) #542

Closed

ransagy commented Aug 28, 2019

View reviewed changes

Initialize NuGet's Credential Service

31134eb

ransagy force-pushed the initCredentials branch from 673af7f to 31134eb Compare October 27, 2019 21:34

ransagy added 2 commits October 27, 2019 23:35

Merge branch 'master' into initCredentials

66acc21

ransagy force-pushed the initCredentials branch from edfe9e5 to 4bf74db Compare October 27, 2019 21:44

AnthonySteele self-assigned this Feb 11, 2020

AnthonySteele merged commit 3af96ea into NuKeeperDotNet:master Feb 11, 2020

johntyrrell mentioned this pull request Jun 9, 2020

Also run SetupDefaultCredentialService in GitRepositoryEngine so that… #993

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initialize NuGet's Credential Service #868

Initialize NuGet's Credential Service #868

ransagy commented Aug 28, 2019 •

edited

Loading

ransagy Aug 28, 2019

skolima Oct 16, 2019

ransagy Oct 16, 2019

MartinDemberger Oct 25, 2019

ransagy Oct 27, 2019

johntyrrell Jun 8, 2020

ransagy Jun 8, 2020 •

edited

Loading

johntyrrell Jun 8, 2020

ransagy Jun 8, 2020

johntyrrell Jun 9, 2020

ransagy commented Oct 27, 2019

MarcBruins commented Dec 3, 2019

ransagy commented Dec 3, 2019 •

edited

Loading

nullabletype commented Feb 11, 2020

ransagy commented Feb 11, 2020

pcghusit commented Mar 20, 2020 •

edited

Loading

scp-mb commented Apr 22, 2020

josundt commented Jun 3, 2020

ransagy commented Jun 3, 2020

AnthonySteele commented Jun 3, 2020

Initialize NuGet's Credential Service #868

Initialize NuGet's Credential Service #868

Conversation

ransagy commented Aug 28, 2019 • edited Loading

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

⤵️ What is the current behavior?

🆕 What is the new behavior (if this is a feature change)?

💥 Does this PR introduce a breaking change?

🐛 Recommendations for testing

📝 Links to relevant issues/docs

🤔 Checklist before submitting

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ransagy Jun 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ransagy commented Oct 27, 2019

MarcBruins commented Dec 3, 2019

ransagy commented Dec 3, 2019 • edited Loading

nullabletype commented Feb 11, 2020

ransagy commented Feb 11, 2020

pcghusit commented Mar 20, 2020 • edited Loading

scp-mb commented Apr 22, 2020

josundt commented Jun 3, 2020

ransagy commented Jun 3, 2020

AnthonySteele commented Jun 3, 2020

ransagy commented Aug 28, 2019 •

edited

Loading

ransagy Jun 8, 2020 •

edited

Loading

ransagy commented Dec 3, 2019 •

edited

Loading

pcghusit commented Mar 20, 2020 •

edited

Loading