microsoft / azure-devops-node-api

Azure DevOps Client for Node.js
Other
465 stars 231 forks source link

NTLM Authentication Fails with TFS 2015 #172

Closed ankitr42 closed 4 years ago

ankitr42 commented 6 years ago

Environment

Node version: 8.11.1 Npm version: 5.6.0 OS and version: Windows 10, Build 14393.2007 vsts-node-api version: 6.5.0

Issue Description

Trying to use this API to connect to a TFS 2015 instance with NTLM authentication always fails.

Expected behaviour

NTLM authentication should succeed and WebAPI.create() should return the connection data.

Actual behaviour

I have some test code in a function as:

let serverUrl = "http://myserver:8080/tfs/DefaultCollection"
    consts client = require('vso-node-api')
    let authHandler = client.getNtlmHandler("myusername", "mypassword")
    let webApi = new client.WebApi(serverUrl, authHandler)

   webApi.connect()

The connect() call fails after the server sends a 401 Unauthorized response with the WWW-Authenticate header. I dug into the source code and, please correct me if I'm wrong, I think the typed-rest-client and the http-client libraries embedded in the vso-node-api package do not take into account the authentication handler being used for a request. I traced the code flow for a connect() call, and I found that there is only 1 call to the handler.prepareRequest() method in httpclient.js. None of the other methods of the handler such as canHandleAuthentication(), handleAuthentication() get called. Furthermore, the NTLM authentication handler ntlm.ts does not make any additions to the request object in the prepareRequest() method - which I think is correct as it's supposed to handle the authentication flow in the handleAuthentication() method - only that never gets called.

stephenmichaelf commented 6 years ago

Thanks @ankitr42 I will take a look! We may need to update the dependency version for typed-rest-client. There were changes made in that library to make NTLM work and I believe they happened just before the 1.0.4 release.

ankitr42 commented 6 years ago

Thanks, @stephenmichaelf.

ArtGangsta commented 6 years ago

@stephenmichaelf I think the problem you're going to run into is that typed-rest-client, uses node-http-ntlm which works for NTLM, but is not truly integrated since it requires the user password. it does not use SSPI to do transparent auth (Integrated Windows Authentication / Negotiate)

I have looked around just a little bit, and the only module I found that works well out of the box for this is node-libcurl. They key there is that when you set curl options (setOpt), USERNAME needs to be set to empty-string and 'HTTPAUTH' to 'Curl.auth.NEGOTIATE'

With node-libcurl you can do anything that REST requires, including custom verbs such (via CUSTOMREQUEST).

The only problem I found with node-libcurl is that it doesn't use the certificate store on the client's machine, which means if you want to access https, you have to tell it to ignore bad certificate. Setting SSL_VERIFYPEER to false. That works almost all the time. One place it doesn't work is when you try to access "visualstudio.com" VSTS and you use a "Personal Access Token". It works fine if you use alternate cred, but if you use a "Personal Access Token", VSTS web site sends this thing in the response header that tells the client, "no, you cannot ignore the certificate" (Strict Transport Security). Actually, I haven't checked if it's always sent -- what I do know is that, curl will complain about it with PAT and not with alternate credentials.

Unfortunately this means node-libcurl cannot be used as the one-stop-shop for solving this problem, and no other solution seems to exist on node for single-sign-on.

It would be cool if node-http-ntlm supported Integrated Windows Authentication. The problem is that it uses node-httpreq internally and exposes that fact to clients so that they can pass-through configuration.

nadavsinai commented 6 years ago

It fails also with TFS 2017, even the latest vso-node-api doesn't have working NTLM support. see #175

namankanakiya commented 6 years ago

Still failing as of 6.6.0

damccorm commented 6 years ago

Currently investigating. It looks instead of getting a 401 response (which we would expect), we're getting a 302 redirect message, which redirects us to a 203 with an empty response message. I think the only reason we're not getting issues with using PAT here is that it uses preauth - it may be that only preauth is effectively supported on the server side (in which case NTLM would not work). I'll continue to look into it and communicate with the team that owns this.

damccorm commented 6 years ago

It looks like currently, even for a call that returns a 401, like WebApi.getCoreApi() NTLM is not working. This looks like another issue on the server, as the www-authenticate header is not returning NTLM as an option. Not sure if this is because it is not supported at all or they are just missing that in the header response. Regardless, I'll log this with them and continue to communicate any updates here.

UPDATE - not sure if this is actually the issue, digging further.

damccorm commented 6 years ago

@ankitr42 @nadavsinai @namankanakiya we are actively investigating this right now. It would be helpful if you could answer these questions:

  1. Was this ever working for you?
  2. If it was working, what version of node-api/TFS were you using?
namankanakiya commented 6 years ago

It was never working for me

On Thu, Oct 4, 2018, 10:53 AM damccorm notifications@github.com wrote:

@ankitr42 https://github.com/ankitr42 @nadavsinai https://github.com/nadavsinai @namankanakiya https://github.com/namankanakiya we are actively investigating this right now. It would be helpful if you could answer these questions:

  1. Was this ever working for you?
  2. If it was working, what version of node-api/TFS were you using?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Microsoft/azure-devops-node-api/issues/172#issuecomment-427111397, or mute the thread https://github.com/notifications/unsubscribe-auth/AQewj7Q5Etd4wrb5NL-RuKgvFFSn_dKIks5uhksQgaJpZM4TRHFi .

ankitr42 commented 6 years ago

@damccorm It was never working for me either.

nadavsinai commented 6 years ago

@damccorm never worked for me either. thanks for your work on the issue.

damccorm commented 6 years ago

Do you have the domain and workstation options specified when you get the ntlmHandler? It looks like these should actually be required, and not having them may be the source of the issues. Passing in an empty string should be enough for this. @ankitr42 it looks like they're at least not specified in the sample code you provided.

If that's what is causing the issues, that's definitely something typed-rest-client should take care of and we will react appropriately.

damccorm commented 6 years ago

Any update on this? Did this fix the problem for anyone?

nadavsinai commented 6 years ago

This is not the cause in my case, I have definitely supplied the workstation and domain details

On Wed, Oct 17, 2018 at 4:27 PM damccorm notifications@github.com wrote:

Any update on this? Did this fix the problem for anyone?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Microsoft/azure-devops-node-api/issues/172#issuecomment-430627789, or mute the thread https://github.com/notifications/unsubscribe-auth/ACvOIKM8QVOlie5RIKfsfA_T6Ym8G9Iwks5ulzBVgaJpZM4TRHFi .

vKarter commented 5 years ago

We have still the issue. :/ Anyone knows something? TFS 2015 vsts-node-api version: 8.1.1

vKarter commented 5 years ago

@damccorm Do you have any info about it?

damccorm commented 5 years ago

No, unfortunately. As of my last message in this thread I was unable to repro (assuming you specify domain/workspace). I'm guessing there's a decent chance that these issues are popping up because of configuration issues, though I'm not really sure what in particular would be causing them - NTLM is particularly fragile and challenging to debug which makes diagnosing them hard.

To be honest, my advice here is probably to move away from NTLM if possible. Since creating the protocol, Microsoft has since recommended dropping NTLMv1 (which is the version we're talking about here), and again its pretty tricky/fragile.

If that doesn't work for your, I don't have a ton I can offer you. One thing you could try is disabling security software and firewall temporarily for a test - that would help eliminate some options. I don't have a great path forward beyond that, sadly

sylvanaar commented 5 years ago

Hi I recently had an issue with NTLM that was related to the typed-rest-client.

I opened up a ticket there with my finding. We have worked around it here, but maybe this is also your issue.

https://github.com/microsoft/typed-rest-client/issues/151

EDIT: Oh I see you have seen it already. Did you try to patch your local copy in node-modules to see if it fixes your issue

vKarter commented 5 years ago

@sylvanaar Thanks a lot. I removed readBody() and I leave only resolve(res) and now it works :)

nadavsinai commented 5 years ago

@damccorm will this issue be fixed as per @vKarter findings in https://github.com/microsoft/typed-rest-client/issues/151#issuecomment-514403286 patching node_modules on post-install is not a nice option...

github-actions[bot] commented 4 years ago

This issue has had no activity in 90 days. Please comment if it is not actually stale

bnbarak commented 3 years ago

For me the problem is with typed-rest-client. After digging around, I found that typed-rest-client is making an OPTIONS request (preflight) prior to making the actual API call that results in a 401 (I added a break point in node_modules/typed-rest-client/handlers/personalaccesstoken.js).

Then I tested an API with postman, and indeed the OPTIONS request fail with a 401...

I did not find a way to disable the preflight call in typed-rest-client. I wish you used use a library that expose better options (axios).

Any idea how to disable the preflight request?