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

Add support for ntlm authentication #2658

Merged
merged 1 commit into from Aug 25, 2017

Conversation

Projects
None yet
4 participants
@samhanes
Contributor

samhanes commented Aug 24, 2017

No description provided.

@matthid

Wow nice!
As long as CI says OK and the encoding stuff is fixed :)

I have some concerns about netcore and mono compat but in theory when CI is green it should be OK.
Regarding a x-plat paket-files NTLM is a bit of a problem, isn't it? So basically authtype: "ntlm" would render the file unusable on non-windows?

@samhanes

This comment has been minimized.

Show comment
Hide comment
@samhanes

samhanes Aug 24, 2017

Contributor

Yes, I think you're right - specifying NTLM in a file would mean it can't be used on non-windows.

I had considered limiting the auth type specification to config files for that reason, but I ultimately decided that if your nuget feed requires windows auth, you're already in trouble if you're not on windows. Do you think that makes sense?

Contributor

samhanes commented Aug 24, 2017

Yes, I think you're right - specifying NTLM in a file would mean it can't be used on non-windows.

I had considered limiting the auth type specification to config files for that reason, but I ultimately decided that if your nuget feed requires windows auth, you're already in trouble if you're not on windows. Do you think that makes sense?

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Aug 24, 2017

Member

Yep, just asking for confirmation I guess :)

Member

matthid commented Aug 24, 2017

Yep, just asking for confirmation I guess :)

```
If you don't want to check your username and password into source control, you
can use environment variables instead:
```paket
source http://myserver/nuget/api/v2 username: "%PRIVATE_FEED_USER%" password: "%PRIVATE_FEED_PASS%"
source http://myserver/nuget/api/v2 username: "%PRIVATE_FEED_USER%" password: "%PRIVATE_FEED_PASS%" authtype: "ntlm"

This comment has been minimized.

@0x53A

0x53A Aug 24, 2017

Contributor

You need either username/password OR ntlm, right?

@0x53A

0x53A Aug 24, 2017

Contributor

You need either username/password OR ntlm, right?

This comment has been minimized.

@0x53A

0x53A Aug 24, 2017

Contributor

Or do you need to enter your windows account and password? It should be able to use the current windows account.

@0x53A

0x53A Aug 24, 2017

Contributor

Or do you need to enter your windows account and password? It should be able to use the current windows account.

This comment has been minimized.

@samhanes

samhanes Aug 24, 2017

Contributor

No, the default credentials aren't used - just a different authentication scheme. See my reply to your other comment. :)

@samhanes

samhanes Aug 24, 2017

Contributor

No, the default credentials aren't used - just a different authentication scheme. See my reply to your other comment. :)

@0x53A

This comment has been minimized.

Show comment
Hide comment
@0x53A

0x53A Aug 24, 2017

Contributor

So i may be misunderstanding something, I haven't looked at this too deep to be honest.

But normally I understand WindowsAuthentication to mean that the current ambient user account will be used, and I DON'T need to enter my password anywhere.

Maybe this could help? https://stackoverflow.com/a/11414695/1872399

Contributor

0x53A commented Aug 24, 2017

So i may be misunderstanding something, I haven't looked at this too deep to be honest.

But normally I understand WindowsAuthentication to mean that the current ambient user account will be used, and I DON'T need to enter my password anywhere.

Maybe this could help? https://stackoverflow.com/a/11414695/1872399

@samhanes

This comment has been minimized.

Show comment
Hide comment
@samhanes

samhanes Aug 24, 2017

Contributor

@0x53A Using the default credentials is certainly one use case - but in this case we'd like to use Windows credentials to a separate domain for authentication. Locally we can store them in the windows credentials manager, but this still presents a problem on the build server. The changes I've proposed would allow you to provide a username in the form of domain\user and then paket will those credentials to authenticate the request via NTLM.

Contributor

samhanes commented Aug 24, 2017

@0x53A Using the default credentials is certainly one use case - but in this case we'd like to use Windows credentials to a separate domain for authentication. Locally we can store them in the windows credentials manager, but this still presents a problem on the build server. The changes I've proposed would allow you to provide a username in the form of domain\user and then paket will those credentials to authenticate the request via NTLM.

@0x53A

This comment has been minimized.

Show comment
Hide comment
@0x53A

0x53A Aug 24, 2017

Contributor

ok, makes sense.

I guess if someone wants default credentials they should implement it themselves ;D

Contributor

0x53A commented Aug 24, 2017

ok, makes sense.

I guess if someone wants default credentials they should implement it themselves ;D

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Aug 25, 2017

Member

WOW! This looks great!

Member

forki commented Aug 25, 2017

WOW! This looks great!

@forki forki merged commit e012329 into fsprojects:master Aug 25, 2017

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@samhanes

This comment has been minimized.

Show comment
Hide comment
@samhanes

samhanes Aug 25, 2017

Contributor

Thanks - you guys move fast!

Contributor

samhanes commented Aug 25, 2017

Thanks - you guys move fast!

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