Skip to content
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

bug: basic-auth does not pick up user-info #34

Closed
lread opened this issue Aug 16, 2022 · 0 comments · Fixed by #37
Closed

bug: basic-auth does not pick up user-info #34

lread opened this issue Aug 16, 2022 · 0 comments · Fixed by #37

Comments

@lread
Copy link
Collaborator

lread commented Aug 16, 2022

Observation

I was reviewing docstrings and code when I noticed that user-info is not applied to basic-auth.

Repro

(require '[clj-http.lite.client :as client])

(client/get "https://joe:blow@httpbin.org/basic-auth/joe/blow"
            {:throw-exceptions false})

Actual

We get a 401

;; => {:headers
;;     {"date" "Tue, 16 Aug 2022 01:38:23 GMT",
;;      "content-length" "0",
;;      "connection" "keep-alive",
;;      "server" "gunicorn/19.9.0",
;;      "www-authenticate" "Basic realm=\"Fake Realm\"",
;;      "access-control-allow-origin" "*",
;;      "access-control-allow-credentials" "true"},
;;     :status 401,
;;     :body nil}

Expected

If I specify :basic-auth a different way, I get the expected 200 response:

(client/get "https://httpbin.org/basic-auth/joe/blow"
            {:basic-auth "joe:blow"})
;; => {:headers
;;     {"date" "Tue, 16 Aug 2022 01:39:31 GMT",
;;      "content-type" "application/json",
;;      "content-length" "46",
;;      "connection" "keep-alive",
;;      "server" "gunicorn/19.9.0",
;;      "access-control-allow-origin" "*",
;;      "access-control-allow-credentials" "true"},
;;     :status 200,
;;     :body "{\n  \"authenticated\": true, \n  \"user\": \"joe\"\n}\n"}

Diagnosis

It might be not entirely obvious to the uninitiated that wrapper behaviours are executed from last to first.

(defn wrap-request
"Returns a battaries-included HTTP request function coresponding to the given
core client. See client/client."
[request]
(-> request
wrap-query-params
wrap-user-info
wrap-url
wrap-redirects
wrap-decompression
wrap-input-coercion
wrap-output-coercion
wrap-exceptions
wrap-basic-auth
wrap-accept
wrap-accept-encoding
wrap-content-type
wrap-form-params
wrap-method
wrap-links
wrap-unknown-host
wrap-oauth))

I'll add a comment to indicate such while fixing this one.

If we take a peek at clj-http (our grandpappy) we see it has wrap-basic-auth above (this means invoked after) wrap-user-info.

Next Steps

I'll follow up with a PR.

lread added a commit to lread/clj-http-lite that referenced this issue Aug 16, 2022
Also moved wrap-oauth to match clj-http wrapper ordering for easier ongoing comparison.

Fixes clj-commons#34
@lread lread closed this as completed in 697705f Aug 16, 2022
@lread lread closed this as completed in #37 Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant