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

Support digest auth #205

Closed
swankjesse opened this issue Jun 5, 2013 · 29 comments
Closed

Support digest auth #205

swankjesse opened this issue Jun 5, 2013 · 29 comments
Labels
enhancement Feature not a bug

Comments

@swankjesse
Copy link
Member

Android issue 11140

@etiennemartin
Copy link

Do we have any way of doing this externally? Lets say we implement our own Digest authentication scheme. Does okhttp or HttpURLConnection support it?

@swankjesse
Copy link
Member Author

@etiennemartin yep. Just implement Authenticator.

@rburgst
Copy link

rburgst commented Oct 11, 2015

for anyone interested, I have made https://github.com/rburgst/okhttp-digest available. Turns out that "just" implementing Authenticator is not trivial for Digest authentication.

@rfc2822
Copy link
Contributor

rfc2822 commented Oct 16, 2015

As it happens, I have done an implementation of Digest (and Basic) auth including WWW-Authenticate parsing, too:

It depends only on okhttp and doesn't import third-party code.

Are you interested in taking this into okhttp (after further testing etc.)?

@swankjesse
Copy link
Member Author

@rfc2822 yeah! Are you the original author & willing to license as Apache 2?

@rfc2822
Copy link
Contributor

rfc2822 commented Oct 16, 2015

@swankjesse Yes. Some small changes would still have to be done, e.g. adapt logging and removing @NonNull.

@swankjesse
Copy link
Member Author

Excellent. And you know me, I'm a pretty thorough (read: pedantic) code reviewer. But your starting place looks healthy, especially with the wealth of tests.

@rburgst
Copy link

rburgst commented Oct 17, 2015

@rfc2822 I had something like your solution as well but unfortunately it doesn't support authenticated sessions. I.e. if you did multiple requests, then every request would cause a 401 cycle. I had to introduce an interceptor and extend the Authenticator API to support this (unless there is another solution that I am missing).

@rfc2822
Copy link
Contributor

rfc2822 commented Oct 17, 2015

@rburgst Do you mean MD5-sess?

I use this code above to talk to do actual Digest authentication, so it works, but I think the servers are talking MD5 and not MD5-sess (which should work, but I couldn't test it in wild for now).

Or do you mean preemptive authentication in a session (»The Authorization header may be included preemptively; doing so improves server efficiency and avoids extra round trips for authentication challenges.«)?

@rburgst
Copy link

rburgst commented Oct 17, 2015

No, I mean if you have an authenticated session where you perform multiple operations (say PROFIND, GET, PUT, ...) then usually this always incurs a 401 response from the server since every new request will be unauthenticated. Once you have the nonce from the server, you can already pre-authenticate the following requests to avoid those extra round trips.
At least this is what I was experiencing, don't you have this problem?

@rfc2822
Copy link
Contributor

rfc2822 commented Oct 17, 2015

No, I mean if you have an authenticated session where you perform multiple operations (say PROFIND, GET, PUT, ...) then usually this always incurs a 401 response from the server since every new request will be unauthenticated. Once you have the nonce from the server, you can already pre-authenticate the following requests to avoid those extra round trips.

I guess you mean preemptive authentication (sending the Authorization header even without a WWW-Authenticate). If I understand it correctly, RFC 2617 3.3 says that a client should (lower-case) »remember username, password, nonce, nonce count and opaque […] to construct the Authorization header in future requests«.

But:

The Authorization header may be included preemptively; doing so improves server efficiency and avoids extra round trips for authentication challenges.

Is this what you mean? As far as I understand it, it's

  1. totally optional
  2. implies more checks, e.g. whether the nonce is now stale.

One could argue that it's even more secure if the server provides a new nonce for every request (which I have seen), and in this case every request HAS to be sent twice, once without Authorization header and once with header.

I agree that it would be faster and better to avoid those unnecessary requests, but I didn't do it because

  1. I don't need Digest auth for high performance, but only to support some servers that don't speak Basic (which would make more sense to me, in times of TLS),
  2. as far as I understand it, the implementation is complete without preemptive authorization.

@rburgst
Copy link

rburgst commented Oct 19, 2015

In my use case (where I do multiple requests) it is essential. The additional roundtrips multiply the processing by a factor of 2. IMHO every mobile implementation should strive to minimise roundtrips. But in essence you are right, you wouldn't strictly need it.

@JPercivall
Copy link

Has there been any more progress to get Digest Auth merged into OkHttp master? I'm in the process of refactoring Apache NiFi's InvokeHttp processor to use OkHttp and would like to allow digest auth. @rfc2822's implementation would work best for my use case (handshake instead of preemptive).

@swankjesse
Copy link
Member Author

For OkHttp 3.0 I'm quite tempted to drop the Authenticator interface and instead have a template for BasicAuthInterceptor and perhaps DigestAuthInterceptor. The authenticator interface is a bit awkward, particularly in that gets its credential synchronously and only in response to an auth failure.

@swankjesse
Copy link
Member Author

.. which is really to say, if you can do digest auth as an interceptor, that might be fantastic

@JPercivall
Copy link

Having to deal with the AuthCache and Authenticators can be confusing to a new user.

A simple template for a digest interceptor would be great. For basic auth though, I'm just adding an Authorization header since it doesn't rely on a handshake.

@JPercivall
Copy link

Actually thinking about it some more, the only way for the digest handshake to work (at least on the first call) is synchronously since it needs the realm, qop, nonce and opaque values (assuming not preemptive).

I'm not sure what the best practice is for interceptors but in order to handle that use-case I'd imagine you'd have to catch the response, compute the auths and resend the request within the intercepted response.

@rburgst
Copy link

rburgst commented Nov 3, 2015

@swankjesse you can check how Apache does it, they do have a concept of an overarching HTTP context where data can be shared between multiple requests. This is then used by the authenticators who need it. However, I agree this is not entirely trivial to get right with Apache HC.
@JPercivall if you have to use digest auth and have to send multiple requests to the server (which IMHO are most use cases) then you won't get around doing some caching. I tried to make the API not overly complicated, after all its about 3 additional lines of code for setting everything up.

@rfc2822
Copy link
Contributor

rfc2822 commented Nov 5, 2015

Because authentication is a core feature of HTTP, my humble suggestion is that okhttp should parse and understand the WWW-Authenticate header (after all, it's defined in the HTTP RFCs) and thus provide a framework for authenticators. These authenticators should implement an interface and can register to okhttp. The interface allows them to be called whenever

  1. a WWW-Authenticate header is sent with the authentication method they have registered for (e.g. Basic, Digest, X-My-Authentication),
  2. a request is sent – preemptive authentication could be handled, for instance, with a method like supportsPreemptiveAuth() that the authenticators can implement. If it returns true, every request is sent to the authenticator (just like a filter), allowing it to add authorization headers.

@swankjesse
Copy link
Member Author

@rfc28222 yep! And we could build all of that behavior into an interceptor.

@mopsop
Copy link

mopsop commented Apr 21, 2016

Any news about the digest authentication in okhttp ?
I tried to use the solutions listed previously without success :(

@rfc2822
Copy link
Contributor

rfc2822 commented Apr 21, 2016

@mopsop Our implementation in dav4android (+ tests) is still not caching, but it's usable (and used in real life).

If you need another license, please let me know. Maybe you can contributing caching behavior …

@mopsop
Copy link

mopsop commented Apr 21, 2016

I tried to adapt the code to use it with a Proxy with digest authentication by changing the header names, but the authentication fails :( And I would like something for Android and Java

@ayzaan
Copy link

ayzaan commented Jun 18, 2016

Any update on whether one of these solutions will be merged into okhttp?

@swankjesse
Copy link
Member Author

@swankjesse
Copy link
Member Author

I’m closing this. The interceptor above should be good enough for anyone who wants digest auth.

@JPercivall
Copy link

For reference, currently okhttp-digest relies on the Android logger and causes errors for those who aren't running on an Android device. I raised an issue[1] and @rburgst is working to address it.

[1] rburgst/okhttp-digest#13

@rburgst
Copy link

rburgst commented Jul 31, 2016

okhttp-digest now uses slf4j

@swankjesse swankjesse removed this from the Icebox milestone Nov 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature not a bug
Projects
None yet
Development

No branches or pull requests

7 participants