dakrone / clj-http

An idiomatic clojure http client wrapping the apache client. Officially supported version.
http://clojars.org/clj-http
MIT License
1.78k stars 408 forks source link

NTLM Auth doesn't work #409

Open Eilie opened 6 years ago

Eilie commented 6 years ago

Hello, ntlm auth doesn't work:

(def login "login")
(def pass "pass")
(def host "localhost")
(def domain "domain")

;; Works and returns status 200.
(let [nt (NTCredentials. login pass host domain)
      creds (doto (BasicCredentialsProvider.) (.setCredentials AuthScope/ANY nt))
      client (-> (HttpClients/custom) (.setDefaultCredentialsProvider creds) (.build))
      request (HttpGet. "https://foo.com")
      response (.execute client request)]
  (.getStatusLine response))

;; Doesn't and return status 401
(client/get "https://foo.com" {:ntlm-auth [login pass host domain]})

Curious about why it doesn't work, since code in core.clj is similar to the working version above.

dakrone commented 6 years ago

Okay, do you know if there is a good way I can test NTLM auth? I don't have any services locally that I can stand up to try it and see if it works.

Otherwise, can you try increasing the log level for the apache HTTP client to see if it logs anything? At certain log levels it'll log the entire request, so that would help track down the differences in the two.

alex-k1 commented 6 years ago

Hi,

Looks like it is setting additional header "Connection: close" when constructing a request (connection manager is not reusable). But for ntlm it is supposed to be "Connection: keep-alive". When changing to "keep-alive" it works fine.

alex-k1 commented 6 years ago

It also works fine when using persistent connection (with-connection-pool). In that case "Connection" header is set to "keep-alive".

dakrone commented 6 years ago

Looks like it is setting additional header "Connection: close" when constructing a request (connection manager is not reusable). But for ntlm it is supposed to be "Connection: keep-alive". When changing to "keep-alive" it works fine.

Strange, this seems... odd. I wouldn't expect auth to completely fail if the connection were closed afterwards. I'm hestitant to add a workaround to add keep-alive because technically the connection isn't being kept alive.

dakrone commented 6 years ago

@alex-k1 can you expand on what sort of server you are trying to authenticate against? IIS? I'd like to try and set up a reproduction with whatever OS/software you're using.

d4hines commented 6 years ago

I'm having the same issue. I'll try to provide what details I can, but I admittedly don't know much about NTLM (nor do I have access to our corporate servers). I tried the code that @Eilie initially posted (...(NTCredentials. login pass host domain)...) and it works; however, it does print the following warning:

Apr 17, 2018 11:20:32 AM org.apache.http.impl.auth.HttpAuthenticator generateAuthResponse WARNING: NEGOTIATE authentication error: No valid credentials provided (Mechanism level: No valid credentials provided (Mechanism level: Failed to find any Kerberos tgt))

(client/get ...) doesn't work, nor does adding {:headers {"Connection" "keep-alive"}}

Here's the whole response

{:request-time 12,
 :repeatable? false,
 :protocol-version {:name "HTTP", :major 1, :minor 1},
 :streaming? true,
 :chunked? false,
 :cookies
 {"ReqClientId"
  {:discard false,
   :expires #inst "2068-04-17T15:24:42.000-00:00",
   :path "/",
   :secure true,
   :value "4d67659b-7f26-438e-a78b-fe84b1011e17",
   :version 0}},
 :reason-phrase "Unauthorized",
 :headers
 {"Content-Type" "text/plain",
  "Server" "Microsoft-IIS/8.5",
  "REQ_ID" "ea2c6d07-ec0b-4432-a8c7-32ac183265e6",
  "WWW-Authenticate" ["NTLM" "Negotiate"],
  "X-Powered-By" "ASP.NET",
  "Date" "Tue, 17 Apr 2018 15:24:42 GMT",
  "Content-Length" "51"},
 :orig-content-encoding nil,
 :status 401,
 :length 51,
 :body "HTTP Error 401.1 - Unauthorized: Access is denied\r\n",
 :trace-redirects []}

Any ideas?

alex-k1 commented 6 years ago

@dakrone I don't have access to underlying server, but from logs it is Microsoft-IIS/8.5 and connection is made from macOS High Sierra

alex-k1 commented 6 years ago

@d4hines try this snippet, it should work

(def ctx (HttpClientContext/create))
(c/with-connection-pool {:threads 1 :default-per-route 1}
  (c/get "url-1" {:ntlm-auth ["user" "password" nil nil] :debug true :http-client-context ctx})
  (c/get "url-2" {:debug true :http-client-context ctx}))
d4hines commented 6 years ago

Ah, thank you @alex-k1! Based on the docs, is that how clj-http intends us to use NTLM? The example the OP gave should work, even without the connection pool, right?

dakrone commented 6 years ago

The example the OP gave should work, even without the connection pool, right?

Yes that's the intent, you should not have to use a connection pool to use NTLM auth

dakrone commented 6 years ago

Actually, what I previously wrote is incorrect, I've done more reading on NTLM auth and it turns out to require a persistent connection, rather than a once-per-request authentication.

I've pushed a commit to master to bring the NTLM auth that clj-http does closer to the original (working) example, can someone with access to an IIS machine see if it makes any difference if you do:

(client/with-connection-pool {}
    (client/get "url"
                {:ntlm-auth ["user" "password" nil nil]}))

Also, can you enable debug logging if possible, and provide the wire output so we can see whether it attempted to do the authentication

d4hines commented 6 years ago

@dakrone, thank you for making that change, I don't want this thread to die, but I'm tied up with another project atm. I'll test this out as soon as I can to confirm it's working.

dakrone commented 6 years ago

Okay no problem, I’ll leave this open to until you get a chance to test ;; Lee

On Thu, Apr 26, 2018, at 9:00 AM, Daniel Hines wrote:

@dakrone, thank you for making that change, I don't want this thread to> die, but I'm tied up with another project atm. I'll test this out as soon as I can to confirm it's working.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/dakrone/clj-http/issues/409#issuecomment-384672819

d4hines commented 6 years ago

@dakrone, I've tested it and it's definitely working. I also have the debug logs for you. Dumb question: how do I scrub these in a way that's safe to post to the internet? I'm not sure how NTLM headers work, but I assume I should redact them from the log. Anything else?

d4hines commented 6 years ago

@dakrone, in addition to the question above, I'm also wondering how I should go about writing a higher -level API library that involves NTLM. For example, say someone wants to use the high-level functions do-something and do-something-else from my library. Should I make them call with-connection-pool like so:

(def config {:ntlm-auth ["user" "password" nil nil]})
(client/with-connection-pool {}
    (lib/do-something config)
    (lib/do-something-else config))

I supppose I could write a macro that called yours under the hood and eliminated the need to pass the options every time:

(lib/using-credentials {:ntlm-auth ["user" "password" nil nil]}
    (lib/do-something config)
    (lib/do-something-else config))

Or would it be better to take over with-connection-pool's job like so:

;; Inject opts somehow (stuartsierra/component, etc.)
(def nltm-conn (conn/make-reuseable-async-conn-manager opts))
(defn do-something []
  (binding [conn/*async-connection-manager* ntlm-conn]
 ;; Make the api calls, etc...

Thoughts? Many thanks.

Edit

After seeing this answer and comments on SO, I'm going to assume that conn/*async-connection-manager* was meant to be overridden by me as a user of clj-http, and thus the third option is probably best.