Login required layer builder

[sinder38]

This is a quick fix

This solution does not fully support dependency injection config.

• Added support for specifying login_url as a static string in the login_required! macro.
• Added a test to cover this use case.

Example

let test_login_url: &'static str = "/login";
let app = Router::new()
    .route("/", axum::routing::get(|| async {}))
    .route_layer(login_required!(Backend, login_url = test_login_url));

Why?

In my app, the login_url comes from configuration injected via dependency injection:

pub fn routes(cfg: &HttpServerConfig) -> Router<AppState> {
    let mut router = Router::new()
        .route("/logout", get(log_out))
        .route("/account", get(get_user).post(update_user))
        .route("/account/password", post(update_user_password))
        .route_layer(login_required!(
            AuthnBackend,
            login_url = &cfg.login.login_page
        ));

Currently this fix would allow this. It still requires creating a static reference 😢

let login_url: &'static str = Box::leak(cfg.login.login_page.to_string().into_boxed_str());

While a OnceLock-based static config is technically an option, I don’t think libraries should force consumers to adopt that pattern:

use std::sync::OnceLock;

#[derive(Debug)]
pub struct AuthConfig {
    pub login_url: String,
}

static CONFIG: OnceLock<AuthConfig> = OnceLock::new();

pub fn auth_config() -> &'static AuthConfig {
    CONFIG.get_or_init(|| AuthConfig {
        login_url: "/login".to_string(),
    })
}

pub fn routes() -> Router<AppState> {
    Router::new()
        .route("/logout", get(log_out))
        .route("/account", get(get_user).post(update_user))
        .route("/account/password", post(update_user_password))
        .route_layer(login_required!(
            AuthnBackend,
            login_url = &auth_config().login_url
        ))
}

Notes

• This may also be solvable by switching from from_fn to from_fn_with_state, though I didn’t explore that path in this PR.
• The same issue and solution pattern likely apply to the require_permission! macro.

[sinder38]

I am leaning towards implementing a proper solution with state, but not sure how to go about it since it requires a lot of changes.

[maxcountryman]

I am leaning towards implementing a proper solution with state, but not sure how to go about it since it requires a lot of changes.

Is it possible this should be its own crate or a crate that lives within this crate but which can be opted into?

These macros are a little bit of an afterthought (provided mostly as a convenience since library consumers are free to design more sophisticated or domain specific solutions using the same parts) so I'm okay to reconsider them more broadly.

[sinder38]

Is it possible this should be its own crate or a crate that lives within this crate but which can be opted into?

I think moving them into an opt-in crate is definitely possible; similar to how strum separates traits into strum and macro implementations into strum_macros. That said, I believe it's unnecessary in this case and that keeping them as an optional feature within the same crate is okay.

I can write some macros variant that support state. I’ve realised this feature is particularly useful for permissions, since those can sometimes be modified by users.

These macros are a little bit of an afterthought (provided mostly as a convenience since library consumers are free to design more sophisticated or domain specific solutions using the same parts) so I'm okay to reconsider them more broadly.

I think having these macros even in their current form significantly lowers the barrier to entry, and is great overall

[maxcountryman]

Okay great. I'm happy to support further development in this crate.

[sinder38]

I was thinking about how this should be implemented and I cant really decide.

// Approach with path from req
// it maybe redundant if users prefer to get required perms from identifier or just use their app config
login_required_with_state!(
    Backend,
    state: AppState,
    |req: &Request<_>, state: &State| -> Option<RequiredPermissions> {
        state.get_required_for_path(req.uri().path())
    }
);
// Approach with only state
// This seems to be the minimum
login_required_with_state!(
    Backend,
    state: AppState,
    |state: &State| -> Option<RequiredPermissions> {
        state.get_required_for_path("this_path_layer_identifier").path())
    }
);
// Approach with req and user
// This one will be very "boilerplaty" If you need so much control you probably shouldn't use these macros
login_required_with_state!(
    Backend,
    state: AppState,
    |req: &Request<_>, state: &State, user: User| -> bool {
        let requierd = state.get_required_for_path(req.uri().path());
        required.iter().all(|perm| perms.contains(&perm.as_str()));
    }
);

[maxcountryman]

My thought is having the request makes it easier to support more use cases without making it too burdensome.

Also as an aside, it seems confusing there are two "states" so something to consider maybe.

[maxcountryman]

What if we took a different approach entirely and moved away from macros?

The existing macros are a convenient wrapper around from_fn, which is great for application-level use, but generally not ideal for libraries. Library authors typically define their own middleware by implementing tower::Layer and tower::Service directly. While this is more verbose, it offers greater flexibility and enables a cleaner, more extensible public API.

As a concrete example, we could define a Require middleware like this:

pub struct Require<B, P, F> {
    backend: B,
    predicate: P,
    fallback: F,
}

impl<S, B, P, F> tower::Layer<S> for Require<B, P, F>
where
    P: Clone,
    F: Clone,
{
    type Service = RequireService<S, B, P, F>;

    fn layer(&self, inner: S) -> Self::Service {
        RequireService {
            inner,
            backend: self.backend.clone(),
            predicate: self.predicate.clone(),
            fallback: self.fallback.clone(),
        }
    }
}

And pair it with a fluent builder API that enables expressive, declarative configuration:

let require_login = Require::login::<MyBackend>()
    .login_url("/signin")
    .redirect_field("next")
    .on_failure(AuthFailure::Unauthorized);

let require_perms = Require::permissions::<MyBackend>()
    .permissions(["test.read", "test.write"])
    .login_url("/login")
    .on_failure(AuthFailure::RedirectDynamic(|req| build_login_uri(req)));

This style gives users the benefits of composability and type safety, while offering a cleaner foundation for future enhancements like dynamic state access or more granular fallback behavior.

[sinder38]

I have to agree. I couldn’t find a way to specify only a single State either way, so I’ve been exploring other approaches as well

[sinder38]

I stumbled into a bit of a problem. I want to remove currently mandatory state(()). There is no way to use () like the default the ordinary way. After thinking about it I couldn't find a good way to do this, other than these:

1. make state and predicate recreate a new struct with different types. This seems to work but is awkward

Looks like this:

impl<B: AuthnBackend, T> RequireBuilder<B, (), T> {
    pub fn new() -> Self {
        Self {
            predicate: None,
            fallback: None,
            login_url: None,
            redirect_field: None,
            state: None,
        }
    }
    pub fn state<ST>(mut self, state: ST) -> RequireBuilder<B, ST, T> {
        RequireBuilder{
            predicate: None, // image recreation here
            fallback:  self.fallback,
            state: Some(state),
            redirect_field: self.redirect_field,
            login_url: self.login_url
        }
    }
}

2. Have 2 variations of new, with state and without.

I don't like this option at all

[maxcountryman]

I'm not able to look more closely at the moment, but is there any guidance via the way Axum handles this?

[sinder38]

I haven’t found a clear pattern how to handles this. While it’s an important quality of life feature, I think it can be done later. For now, I’ll start with implementing custom futures to avoid using Box, and I’ll focus on writing more comprehensive tests, as the current ones are almost nonexistent.

Edit: I realised, I implemented the fallback handler in a wrong place, It was supposed to be fallback for permissions. This is a good opportunity to add simple parameters functionality like in your example.

[sinder38]

The current implementation is mostly functional, but several improvements are still needed:

1. User Experience:
   The .state(()) call is still there and everywhere, which is very sad. The builder feels larger than I wanted. Some names are confusing .
   Add a parameterized variant to Rstr (supporting both Forbidden and RestrictFn cases). Currently it has i_dunno place holder.

2. Optimizations:
   There are redundant clones and unnecessary Box::pin usages that should be eliminated for better performance and clarity.

3. Testing:
   Tests for the state functionality are missing and need to be implemented to ensure robustness.

4. No backward compatability:
   Test results differ from macro implementations: when a user is unauthorised and lacks permissions, the middleware responds with FallbackFn (Unauthorized) instead of RestrictFn (Forbidden), unlike the macros. These cases are marked with // WARN: comments.

[sinder38]

Even though this actually works, I'm totally doing it the wrong way. After digging through the tower-rs implementation, I realised I should've gone with a layer stacking/composition approach instead. And the way I'm accepting closures from users is pretty messy - doing it through traits would be way cleaner. 😞

[maxcountryman]

I think there's some pretty good examples in tower and especially tower-http, which I seem to recall has a couple examples of middleware that take closures or callbacks.

[sinder38]

Summary

• I added custom futures, though they’re still subject to some changes.
• The builder is a bit obscure, but overall it seems to work as intended.
• I added the trait implementation only for the fallback, which should also be faster in terms of performance because it avoids dynamic dispatch.

What do you think of the builder, specifically?

Example without state

let require = RequireBuilder::<Backend>::new()
    .fallback(|_| async { StatusCode::GONE.into_response() }) // New format with traits (Closure)
    .fallback(RedirectFallback::new().login_url("/login"))    // New format with traits (Builder)
    .on_restrict(Rstr::from_closure(|_| async {               // Old format with enums (shown on restrict fn)
        StatusCode::UNAUTHORIZED.into_response()
    }))
    .build();

Example with state

let require_login = RequireBuilder::new_with_state(state.clone())
    .fallback(RedirectFallback { // New format implemented with traits (Raw)
        redirect_field: None,
        login_url: Some("/login".to_string()),
    })
    .predicate(Predicate::from_closure(f)) // Old format with enums 
    .on_restrict(Rstr::from_closure(|_| async { // Old format with enums 
        StatusCode::UNAUTHORIZED.into_response()
    }))
    .build();

[maxcountryman]

From my view this is much nicer way of doing things, especially if we intend to continue building support into this crate.