Implement digest authentication middleware behind a feature flag

[PabloMansanet]

In #392 , @algesten mentioned that it could be possible to consider default middleware for common features like Digest authentication. Here's a simple implementation, in case it's general enough to add to the main crate.

Some thoughts:

• I realized the structure of the project is pretty nice and flat, so it felt wrong to create a subfolder for particular middlewares. I ended up adding this as a feature-flagged embedded module under digest. Is this a good approach?
• I'm not sure about using external services like httpbin for unit tests, but I saw other tests doing a similar thing so I went with what seemed to be the simplest approach. I'm happy to take directions on how to mock this instead to fully test it offline.
• The recursive solution helps transparently navigate the "back and forth" between server and client without needing any Arc<Mutex<>> state in the middleware itself, but this may be erring on the "too smart" side. I don't directly see any problems it might cause but I could definitely be missing some.

[PabloMansanet]

Thanks for the review @algesten. All the proposed changes make sense so I'll ping when I'm done with them :slightly_smiling_face:

[PabloMansanet]

While testing this middleware on the field, we've noticed an issue that might be difficult to solve with the current Middleware API.

Basically, while this works well for GET requests, anything with a payload is tricky because the middleware has no access to the payload from what I can see. Starting a request with send_form will send the payload without authorization, and then this middleware's recursive call to send will not have the payload available at all.

Is it conceivable to solve this kind of problem through a change in the Middleware API? I imagine the limitations around the current payload are due to Payload::Reader, but I wonder if, for all other Payload enum variants, it would be possible for middlewares to observe the payload.

There was a similar issue around 307/308 responses, and the conclusion seems to be that PUT/POST/DELETE "retries" have to be handled at application level. Is this still the expectation, or can we possibly solve this at the middleware level for non-stream payloads?

[algesten]

Hey! Thanks for fixing those things!

There was a similar issue around 307/308 responses, and the conclusion seems to be that PUT/POST/DELETE "retries" have to be handled at application level. Is this still the expectation, or can we possibly solve this at the middleware level for non-stream payloads?

As you correctly pointed out, this is very similar to handling 307/308 with body. I'm trying to think if there is some quick win here without handling streaming bodies.

let response = next.handle(request.clone())?;

Here we would need something else, maybe something along the lines of:

// Buffer request bodies up to 10k
request.resend_buffer(10_000);

let response = next.handle(request)?;

...

if let Some((mut request, body)) = response.take_resend() {
    request.set("Authorization", "Bar");
    request.resend(body);
}

The idea is that take_resend will only return something if resend_buffer was set on the request and the request body is smaller than that buffer. body above would be some opaque type ResendBody that can only be used as an argument to request.resend().

[algesten]

Oh for the failed lint, do you mind running cargo fmt?

[PabloMansanet]

Done! (running fmt, I mean). Thanks for the response @algesten . I see #473 is in the works, which will likely unblock me from working on this one, so I will wait for that to progress :)

[algesten]

No activity. Feel free to reopen or start new PR.

[algesten]

Didn't mean to close this.