New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial support for credential providers. #3069

Merged
merged 12 commits into from Feb 26, 2018

Conversation

Projects
None yet
3 participants
@matthid
Copy link
Member

matthid commented Feb 24, 2018

closes #3021, closes #1509

paket-credential-providers

Paket support for Credential Providers

Paket supports Credential Providers through the same interface as NuGet.

Paket extends the definition and allows to run FDD-NetCore applications to run as well (*.dll files!), so Paket additionally searches for CredentialProvider*.dll files in the given paths. For this to work Paket needs to be able to resolve a dotnet executable from the PATH variable.

Development

For regular paket users installing Credential Providers works the same as for the NuGet client. If you have already installed Credential Providers in %LOCALAPPDATA%\NuGet\CredentialProviders paket should pick them up immediatly.

Example VSTS:

  1. Download the credential providers from your VSTS Instance.
    credential-providers-vsts

  2. Extract the CredentialProvider.VSS.exe file into the above path (%LOCALAPPDATA%\NuGet\CredentialProviders)

  3. use paket normally (without password in paket.dependencies and config) and enter the password in the provided dialog.

CI

There are two options to use Credential Providers in your build agent:

  1. Either install a global Credential Provider on your agent
  2. Use Tasks to provide Credential Providers as part of your build.

Example VSTS:

Install https://github.com/matthid/Paket.TeamBuildCredentials/releases/tag/0.1.1 on your TFS
Add the "Setup Paket credential manager" build step before calling Paket.
See more infos here

Note: Disable failing on standard error for your build step calling paket.
paket-vsts-disable-stderr

| Encrypt, Credentials(username, password, authType) ->
ConfigAuthentication(username, password, authType)
| Plaintext, Credentials(username, password, authType) ->
PlainTextAuthentication(username, password, authType)

This comment has been minimized.

@matthid

matthid Feb 24, 2018

Author Member

This change is a bit fishy. I have no idea where this information was used (or how it was supposed to be used). Apparently this information was not used at all or I introduced a bug.

| true -> ConfigAuthentication(username, password, authType)
| false -> PlainTextAuthentication(username, password, authType)
| true -> Credentials userPass
| false -> Credentials userPass

This comment has been minimized.

@matthid

matthid Feb 24, 2018

Author Member

What was the effect of this?

@matthid

This comment has been minimized.

Copy link
Member Author

matthid commented Feb 24, 2018

This PR pretty much makes it possible to use existing NuGet credential providers (https://docs.microsoft.com/en-us/nuget/reference/extensibility/nuget-exe-credential-providers).

This works already quite nicely on the development PC, but I have no idea how we get a working story for VSTS Team Build. Any idea @isaacabraham ?

@isaacabraham

This comment has been minimized.

Copy link
Contributor

isaacabraham commented Feb 24, 2018

@matthid working story? It should just work now I guess. Perhaps @foxrough can test it out to confirm though?

@matthid

This comment has been minimized.

Copy link
Member Author

matthid commented Feb 24, 2018

@isaacabraham I tested it and it doesn't just work. The nuget vsts task is doing a lot of magic internally and brings it's own CredentialProvider to make it work.

@matthid

This comment has been minimized.

Copy link
Member Author

matthid commented Feb 24, 2018

Problem is that doesn't quite fit in our model of bootstrapping paket within the build as well as how FAKE5 is working.

@matthid

This comment has been minimized.

Copy link
Member Author

matthid commented Feb 24, 2018

My current thinking is:

  • provide our own VSTS Extension with two tasks
  • One task sets up the credential provider (and needs to be before running the buildscript/paket) and saves the access token somewhere
  • One task cleans the credential provider

This would work for Paket and FAKE5

@isaacabraham

This comment has been minimized.

Copy link
Contributor

isaacabraham commented Feb 24, 2018

@matthid I already did a (very simple) Paket and Fake extension in VSTS through the F# Helpers extension (https://marketplace.visualstudio.com/items?itemName=isaacabraham.fsharp-helpers-extension). However it's very simple and probably needs a rewrite if it's going to do anything serious. I'm happy to make you an admin of that repo if you want.

Alternatively, perhaps we can look / speak to the NuGet team to see how they're doing it in VSTS?

@matthid

This comment has been minimized.

Copy link
Member Author

matthid commented Feb 24, 2018

@isaacabraham Yes I can try to extend there, however the question is more fundamental. I'm pretty sure I know what they are doing, however our workflow looks a bit different.

But yes maybe extending your tasks to provide an credential manager (just like nuget does it) makes a lot more use-cases viable. Currently I'm trying around various possibilities and creating new tasks. But I can add them to your suite after they start working.

@matthid

This comment has been minimized.

Copy link
Member Author

matthid commented Feb 24, 2018

@isaacabraham I guess that is not best practice but I added https://github.com/matthid/Paket.TeamBuildCredentials and some documentation around it.
It is not best practice because we store the OAuth token temporarily such that it is available for Paket. But I like that workflow a bit better.

We can add the tasks to your extension if you are OK with how they work. I tested a bit and everything seems to work just fine.

@forki forki force-pushed the master branch from 899efc6 to 453a238 Feb 26, 2018

@matthid matthid force-pushed the credential_providers branch from 94db308 to 3b0a316 Feb 26, 2018

matthid added some commits Feb 26, 2018

@matthid matthid merged commit 6f60131 into master Feb 26, 2018

2 of 4 checks passed

continuous-integration/appveyor/branch Waiting for AppVeyor build to complete
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@@ -75,6 +75,9 @@ let buildMergedDir = buildDir @@ "merged"
let paketFile = buildMergedDir @@ "paket.exe"

Environment.CurrentDirectory <- __SOURCE_DIRECTORY__

System.Net.ServicePointManager.SecurityProtocol <- unbox 192 ||| unbox 768 ||| unbox 3072 ||| unbox 48

This comment has been minimized.

@haf

haf Feb 26, 2018

Member

You should avoid magic number and assign them to constants before ORing them.

This comment has been minimized.

@matthid

matthid Feb 26, 2018

Author Member

@haf Yes good eyes ;)
I quickly copy&pasted it from Utils.fs. Basically, I think we need to fix this in either Octokit or FAKE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment