Unable to handle 404 when some routes require authorization

[malobre]

Bug Report

Version

axum v0.2.5

Description

I have an app that has public and private routes. Private routes are guarded by a RequireAuthorizationLayer from tower-http like so:

let public = Router::new().route("/public", get(|| async { StatusCode::OK }));

let authorized = Router::new()
    .route("/authorized", get(|| async { StatusCode::OK }))
    .layer(RequireAuthorizationLayer::bearer("token"));

let not_found = (|| async { StatusCode::NOT_FOUND }).into_service();

public.or(authorized).or(not_found).boxed()

When I request a non-existing route the server returns a 401 Unauthorized response (from the RequireAuthorizationLayer) instead of a 404. I have tried changing the order of the routes without success.

Here is a minimal reproduction repo. Run cargo test.

[davidpdrsn]

This issue has to due with how or interacts with middleware that return early.

or needs to know that no handler matched the incoming request (note this is different from returning 404). axum uses some tricks to mark a response such that or can recognize that it came from EmptyRouter (which is always the bottom route) however since RequireAuthorization returns early, without ever calling your route, or has no way of catching that. RequireAuthorization is called before calling your handler and never looks at the URI, it effectively accepts all requests.

The solution is to re-write it using nest("/", ...):

let public = Router::new()
    .route("/public", get(|| async { StatusCode::OK }))
    .nest(
        "/authorized",
        Router::new()
            .route("/", get(|| async { StatusCode::OK }))
            .layer(RequireAuthorizationLayer::bearer("token")),
    );

let not_found = (|| async { StatusCode::NOT_FOUND }).into_service();

let app = public.or(not_found).boxed();

With the current design of or I don't think this is fixable in another way.

[malobre]

I have multiple routes that need authorization at the root level and this:

let public = Router::new()
    .route("/public", get(|| async { StatusCode::OK }))
    .nest(
        "/",
        Router::new()
            .route("/first", get(|| async { StatusCode::OK }))
            .route("/second", get(|| async { StatusCode::OK }))
            .layer(RequireAuthorizationLayer::bearer("token")),
    );

let not_found = (|| async { StatusCode::NOT_FOUND }).into_service();

let app = public.or(not_found).boxed();

doesn't seem to work.

I guess my only choice is to add the layer to handler individually. Thinking of it, is there any reason for the layer to be applied to the router instead of being recursively applied to existing routes (by Router::layer) ?

[davidpdrsn]

I guess my only choice is to add the layer to handler individually

When you need such fine grained control over which endpoints require auth I would recommend writing an extractor that handles it, and then add that as an argument to your handlers that require auth. That would make things easier to read and you wouldn't have to use nest("/", ...) which can be hard to use since it captures all requests.

Thinking of it, is there any reason for the layer to be applied to the router instead of being recursively applied to existing routes (by Router::layer) ?

Yes, its how tower works and given axum's ability to route to arbitrary tower services it would be hard to add.