tower-rs / tower-rs/tower-http
`AsyncAuthorizeRequest` should consider being replaced with a bounded `tower::Service`
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 913
- Forks
- 231
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 8
Description
Bug Report
Version
tower-http v0.2.5
Crates
tower-http
Description
tower_http::auth::AsyncRequireAuthorization uses a trait AsyncAuthorizeRequest to define the authorization. This trait is very nearly a refinement of tower::Service except it has no equivalent to tower::Service::poll_ready(). This means I can't easily implement async authorization in terms of an underlying tower::Service without manually checking poll_ready() and blocking my future on that, despite the wrapping AsyncRequireAuthorization itself advertising that it is ready. This breaks the whole ready tracking of tower::Service.
I think AsyncAuthorizeRequest needs its own poll_ready() method. It can have a default implementation that just returns Poll::Ready(Ok(())) so as to preserve compatibility. AsyncRequireAuthorization can then poll that in its own poll_ready(). This way I can properly delay readiness until my wrapped service is ready.
Beyond that, it would be nice to actually just go ahead and allow tower::Service impls with appropriate bounds in AsyncRequireAuthorization, so I can use existing service combinators to construct something of the appropriate shape without having to define a custom type just for the AsyncAuthorizeRequest impl. This could work by adding a blanket impl of AsyncRequireAuthorization for any T: tower::Service with the right shape[^shape]. This would be a breaking change though as it would produce an error for any type that already implements both AsyncAuthorizeRequest and tower::Service. A nice benefit of this approach is I could then add arbitrary middleware to my auth implementation (such as tracing) or add additional validation with something like tower::ServiceExt::map_response() or populating auth error response bodies with tower::ServiceExt::map_err().
This could also be done in a backwards-compatible way by adding an adaptor that wraps a tower::Service and implements AsyncAuthorizeRequest (along with convenience methods on AsyncRequireAuthorization and AsyncRequireAuthorizationLayer), but this is a bit more awkward. This could be used as a transition plan until tower-http 0.3 though.
[^shape]: This would admittedly be mildly odd on the naming front since <T as Service>::Response would actually be a Request and <T as Service>::Error would be a Response but it should work just fine.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading AsyncAuthorizeRequest, AsyncRequireAuthorization, and AsyncRequireAuthorizationLayer alongside tower::Service and its poll_ready contract. Compare adding readiness to the trait, supporting bounded services directly, or providing an adaptor, while checking compatibility concerns. Done means the chosen API preserves correct readiness tracking and supports the requested authorization-service composition.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100