Implement digest auth middleware behind a feature flag (on v3)

[jacksongoode]

See #468 for the original PR and discussion.

This supports the middleware digest authentication under a feature digest-auth.

Presently, in order to use this feature we have to catch the authentication status code (401) and respond to it. However, with v3 this means that we now need to pass http_status_as_error(false) in order to propagate the authentication header into the middleware.

See the TODOs in code for places of discussion.

@strohel @mbernat

[strohel]

Hi @algesten, we'd like to get your opinion on some design/implementation decisions, please see inline comments.

[strohel]

Here we would like to inspect the error, and if it is HTTP 401 (in case of http_status_as_error(true)), then extract the WWW_AUTHENTICATE header from the response and continue.

However, in ureq v3 the Error type is https://github.com/tonarino/ureq/blob/af13eef6e40a3366a5ccf15f2b7641228a5af565/src/error.rs#L9-L14
i.e. it isn't possible to extract the header from it.

There could be multiple solutions to this, including:

1. Move the https://github.com/tonarino/ureq/blob/af13eef6e40a3366a5ccf15f2b7641228a5af565/src/run.rs#L110-L112 check to a later point in execution, i.e. to https://github.com/tonarino/ureq/blob/af13eef6e40a3366a5ccf15f2b7641228a5af565/src/agent.rs#L231
2. Variant of (1): implement http_status_as_error(true) in terms of middleware (that would run as the outermost one -- if that's feasible).
3. Make Error::StatusCode contain the HTTP response (API-breaking change)
4. Make a separate HEAD request for every request when using DigestAuthMiddleware -- but we'd still need circumvent http_status_as_error(true) in that request somehow to get the header.
5. Any other..?

[algesten]

Since it's a concern for this middleware that it needs the http response in case of a 401, I think setting http_status_as_error(false) on the request (not the agent) is the right way.

However here is currently a bit of an oversight in the middleware API there is no good way to reach the config from inside middleware. There is a RequestExt that allows to configure with a specific Agent, however in the context for middleware this is not appropriate since there is a specific Agent that has invoked the middleware chain already.

I think I'll do a PR adding a new function RequestExt::middleware_configure() that only works from inside a middleware. Sounds ok?

[algesten]

See #1076

[strohel]

Thanks for #1076 @algesten and sorry for a slow response - we'll try to base this on #1076 (if only to validate that we can achieve the desired functionality).

[strohel]

This was a showstopper in the original PR for ureq v2, where we couldn't easily save the original body.

But in v3 we could do it using SendBody::into_reader(). However, there are still some corner-cases, i.e. the body could even be infinite stream (we don't want to eat all the memory in this case). How to handle that?

1. Do a HEAD request first without the body
   • either if the body isn't empty or small
   • or every time
2. Cache the body, but only till some limit? (this was suggested on the original PR)
3. Anything else?

[algesten]

Sounds like 2 leading to a HEAD if it reaches some limit might be a good way?

[algesten]

I've added some comments. I'm all for this initiative, and I'm happy to build out ureq in the ways necessary to support it, however I mentioned briefly in #392

...my starting point is that I think this middleware could live in a separate crate.

I don't think ureq should provide any MiddleWare in the base crate. If digest auth is something that is bundled, the question is why it comes in the shape of middleware? In some respects, the whole point of the middleware API is to avoid having code for less used functions in the ureq main crate.

[algesten]

Since it's a concern for this middleware that it needs the http response in case of a 401, I think setting http_status_as_error(false) on the request (not the agent) is the right way.

However here is currently a bit of an oversight in the middleware API there is no good way to reach the config from inside middleware. There is a RequestExt that allows to configure with a specific Agent, however in the context for middleware this is not appropriate since there is a specific Agent that has invoked the middleware chain already.

I think I'll do a PR adding a new function RequestExt::middleware_configure() that only works from inside a middleware. Sounds ok?

[algesten]

Sounds like 2 leading to a HEAD if it reaches some limit might be a good way?