Problem with error handler when using fallible tower middleware #1908

lcmgh · 2023-04-04T09:05:05Z

lcmgh
Apr 4, 2023

Hi!

I built my middleware upon this example to authorize users: https://github.com/tower-rs/tower/blob/master/guides/building-a-middleware-from-scratch.md. But when adding it to my Axum Router it fails, even I am using an error handler.

error[E0277]: the trait bound `HandleError<RequireAuthAndRole<Route<_>>, fn(Box<(dyn StdError + Send + Sync + 'static)>) -> impl Future<Output = (StatusCode, std::string::String)> {handle_auth_error}, _>: Service<Request<_>>` is not satisfied
   --> simple-rest/src/main.rs:102:17
    |
100 |               get(root_handler).layer(
    |                                 ----- required by a bound introduced by this call
101 |                   
102 | /                 ServiceBuilder::new()
103 | |                 .layer(HandleErrorLayer::new(handle_auth_error))
104 | |                 .layer(mrp.require_role("default"))
    | |___________________________________________________^ the trait `Service<Request<_>>` is not implemented for `HandleError<RequireAuthAndRole<Route<_>>, fn(Box<(dyn StdError + Send + Sync + 'static)>) -> impl Future<Output = (StatusCode, std::string::String)> {handle_auth_error}, _>`

let app = Router::new()
    .route(
        "/",
        get(root_handler).layer(
            ServiceBuilder::new()
            .layer(HandleErrorLayer::new(handle_auth_error))
            .layer(require_role("default")) // Creates RequireAuthAndRoleLayer
        )
    )

// From the docs

pub async fn handle_auth_error(err: BoxError) -> (StatusCode, String) {
    if err.is::<tower::timeout::error::Elapsed>() {
        (
            StatusCode::REQUEST_TIMEOUT,
            "Request took too long".to_string(),
        )
    } else {
        (
            StatusCode::INTERNAL_SERVER_ERROR,
            format!("Unhandled internal error: {}", err),
        )
    }
}

use axum::response::Response;
use axum::BoxError;
use http::{header, Request, StatusCode};
use http_auth_basic::Credentials;
use hyper::Body;
use std::collections::HashMap;
use std::error::Error;
use std::fmt;
use std::future::Future;
use std::pin::Pin;
use std::sync::Arc;
use std::task::{Context, Poll};
use tower_layer::Layer;
use tower_service::Service;

use super::auth_layer::User;
use super::Authenticated;

type Users = Arc<HashMap<String, User>>;

#[derive(Clone)]
pub struct RequireAuthAndRole<T> {
    inner: T,
    users: Users,
    required_role: String,
}

impl<T> RequireAuthAndRole<T> {
    pub fn new(inner: T, users: Users, required_role: String) -> RequireAuthAndRole<T> {
        RequireAuthAndRole {
            inner,
            users,
            required_role,
        }
    }
}

// The error returned if processing a request timed out
#[derive(Debug, Clone)]
pub struct AccessDenied;

impl fmt::Display for AccessDenied {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "Access Denied")
    }
}

impl Error for AccessDenied {}

// We can implement `Service` for `RequireAuthAndRole<T>` if `T` is a `Service`
impl<T> Service<Request<Body>> for RequireAuthAndRole<T>
where
    T: Service<Request<Body>, Response = Response> + Send + 'static,
    T::Future: 'static,
    T::Error: Into<BoxError> + 'static,
    T::Response: 'static,
{
    // `RequireAuthAndRole` doesn't modify the response type, so we use `T`'s response type
    type Response = T::Response;
    // Errors may be either `AccessDenied` if access was denied, or the inner service's
    // `Error` type. Therefore, we return a boxed `dyn Error + Send + Sync` trait object to erase
    // the error's type.
    type Error = BoxError;
    type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>>>>;

    fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
        self.inner.poll_ready(cx).map_err(Into::into)
    }

    fn call(&mut self, mut req: Request<Body>) -> Self::Future {

        // Extract basic auth
        // Check against credentials
        // Return 401 in case of any error
        let user_id = match validate_auth(&req, &self.users, self.required_role.clone()) {
            Ok(v) => v,
            Err(_) => return Box::pin(async { Ok(err_resp()) }),
        };

        // Share authenticated user to downstream
        let authenticated = Authenticated { user_id };
        req.extensions_mut().insert(authenticated.clone());

        // Call the inner service and get a future that resolves to the response
        let fut = self.inner.call(req);

        // We have to box the errors so the types match
        let f = async move {
            fut.await.map_err(|err| err.into()).map(|mut res| {
                res.extensions_mut().insert(authenticated.clone());
                res
            })
        };

        Box::pin(f)
    }
}

fn validate_auth(
    req: &Request<Body>,
    users: &Users,
    required_role: String,
) -> eyre::Result<String> {
    let auth_header_value = req
        .headers()
        .get("Authorization")
        .ok_or_else(|| eyre::eyre!("User is not authorized"))?
        .to_str()?
        .to_string();

    let credentials = Credentials::from_header(auth_header_value).unwrap();

    // Get expected password if user exists, otherwise return error
    let expected_pw = users
        .get(&credentials.user_id)
        .ok_or_else(|| eyre::eyre!("User does not exist"))?
        .password
        .clone();

    // Compare expected vs. given password
    if expected_pw != credentials.password {
        Err(eyre::eyre!("Password does not match"))?
    }

    // Get permissions of user
    let user_permissions = &users
        .get(&credentials.user_id)
        .ok_or_else(|| eyre::eyre!("User is not authorized"))?
        .roles;

    // Ensure user has the required permission
    if !user_permissions.contains(&required_role) {
        Err(eyre::eyre!("User is not authorized"))?
    }

    Ok(credentials.user_id)
}

// A layer for wrapping services in `RequireAuthAndRole`
#[derive(Clone)]
pub struct RequireAuthAndRoleLayer {
    pub users: Users,
    pub required_role: String,
}

impl RequireAuthAndRoleLayer {
    pub fn new(users: Users, required_role: String) -> Self {
        RequireAuthAndRoleLayer {
            users,
            required_role,
        }
    }
}

impl<S> Layer<S> for RequireAuthAndRoleLayer {
    type Service = RequireAuthAndRole<S>;

    fn layer(&self, service: S) -> RequireAuthAndRole<S> {
        RequireAuthAndRole::new(service, self.users.clone(), self.required_role.clone())
    }
}

pub fn err_resp() -> Response<axum::body::BoxBody> {
    let mut res = Response::new(axum::body::BoxBody::default());
    *res.status_mut() = StatusCode::UNAUTHORIZED;
    res.headers_mut()
        .insert(header::WWW_AUTHENTICATE, "Basic".parse().unwrap());
    res
}

pub async fn handle_auth_error(err: BoxError) -> (StatusCode, String) {
    if err.is::<tower::timeout::error::Elapsed>() {
        (
            StatusCode::REQUEST_TIMEOUT,
            "Request took too long".to_string(),
        )
    } else {
        (
            StatusCode::INTERNAL_SERVER_ERROR,
            format!("Unhandled internal error: {}", err),
        )
    }
}

Answered by davidpdrsn

Apr 4, 2023

Doesn't look like your middleware has any errors of its own. It always just propagates the error from the wrapped service. In that case you should use type Error = T::Error;. Returning a response is not an error.

View full answer

davidpdrsn · 2023-04-04T09:13:24Z

davidpdrsn
Apr 4, 2023
Collaborator

Doesn't look like your middleware has any errors of its own. It always just propagates the error from the wrapped service. In that case you should use type Error = T::Error;. Returning a response is not an error.

7 replies

lcmgh Apr 4, 2023
Author

hm because then downstream users would need to define their own UNAUTHORIZED response. I agree with you.

lcmgh Apr 4, 2023
Author

I updated the layer accordingly but facing issues.

99  |         .route("/", get(root_handler).layer(require_role("default")))
    |                                       ----- ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Future<Output = Result<axum::http::Response<http_body::combinators::box_body::UnsyncBoxBody<axum::body::Bytes, axum::Error>>, Infallible>> + 'static)` cannot be sent between threads safely

use axum::response::Response;
use http::{header, Request, StatusCode};
use http_auth_basic::Credentials;
use hyper::Body;
use std::collections::HashMap;
use std::future::Future;
use std::pin::Pin;
use std::sync::Arc;
use std::task::{Context, Poll};
use tower_layer::Layer;
use tower_service::Service;

use super::auth_layer::User;
use super::Authenticated;

type Users = Arc<HashMap<String, User>>;

#[derive(Clone)]
pub struct RequireAuthAndRole<T> {
    inner: T,
    users: Users,
    required_role: String,
}

impl<T> RequireAuthAndRole<T> {
    pub fn new(inner: T, users: Users, required_role: String) -> RequireAuthAndRole<T> {
        RequireAuthAndRole {
            inner,
            users,
            required_role,
        }
    }
}

// We can implement `Service` for `RequireAuthAndRole<T>` if `T` is a `Service`
impl<T> Service<Request<Body>> for RequireAuthAndRole<T>
where
    T: Service<Request<Body>, Response = Response> + Send + 'static,
    T::Future: 'static,
    T::Response: 'static,
{
    // We don't modify the response type, so we use `T`'s response type
    type Response = T::Response;
    // We don't modify the error type, so we use `T`'s response type
    type Error = T::Error;
    type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>>>>;

    fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
        self.inner.poll_ready(cx).map_err(Into::into)
    }

    fn call(&mut self, mut req: Request<Body>) -> Self::Future {
        // Extract basic auth
        // Check against credentials
        // Return 401 in case of any error
        let user_id = match validate_auth(&req, &self.users, self.required_role.clone()) {
            Ok(v) => v,
            Err(_) => return Box::pin(async { Ok(err_resp()) }),
        };

        // Share authenticated user to downstream
        let authenticated = Authenticated { user_id };
        req.extensions_mut().insert(authenticated.clone());

        // Call the inner service and get a future that resolves to the response
        let fut = self.inner.call(req);

        // We have to box the errors so the types match
        let f = async move {
            fut.await.map_err(|err| err.into()).map(|mut res| {
                res.extensions_mut().insert(authenticated.clone());
                res
            })
        };

        Box::pin(f)
    }
}

fn validate_auth(
    req: &Request<Body>,
    users: &Users,
    required_role: String,
) -> eyre::Result<String> {
    let auth_header_value = req
        .headers()
        .get("Authorization")
        .ok_or_else(|| eyre::eyre!("User is not authorized"))?
        .to_str()?
        .to_string();

    let credentials = Credentials::from_header(auth_header_value).unwrap();

    // Get expected password if user exists, otherwise return error
    let expected_pw = users
        .get(&credentials.user_id)
        .ok_or_else(|| eyre::eyre!("User does not exist"))?
        .password
        .clone();

    // Compare expected vs. given password
    if expected_pw != credentials.password {
        Err(eyre::eyre!("Password does not match"))?
    }

    // Get permissions of user
    let user_permissions = &users
        .get(&credentials.user_id)
        .ok_or_else(|| eyre::eyre!("User is not authorized"))?
        .roles;

    // Ensure user has the required permission
    if !user_permissions.contains(&required_role) {
        Err(eyre::eyre!("User is not authorized"))?
    }

    Ok(credentials.user_id)
}

// A layer for wrapping services in `RequireAuthAndRole`
#[derive(Clone)]
pub struct RequireAuthAndRoleLayer {
    pub users: Users,
    pub required_role: String,
}

impl RequireAuthAndRoleLayer {
    pub fn new(users: Users, required_role: String) -> Self {
        RequireAuthAndRoleLayer {
            users,
            required_role,
        }
    }
}

impl<S> Layer<S> for RequireAuthAndRoleLayer {
    type Service = RequireAuthAndRole<S>;

    fn layer(&self, service: S) -> RequireAuthAndRole<S> {
        RequireAuthAndRole::new(service, self.users.clone(), self.required_role.clone())
    }
}

pub fn err_resp() -> Response<axum::body::BoxBody> {
    let mut res = Response::new(axum::body::BoxBody::default());
    *res.status_mut() = StatusCode::UNAUTHORIZED;
    res.headers_mut()
        .insert(header::WWW_AUTHENTICATE, "Basic".parse().unwrap());
    res
}

davidpdrsn Apr 4, 2023
Collaborator

You need to use

Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error> + Send>>>
                                                                ^^^^^^ add this

lcmgh Apr 4, 2023
Author

Thanks for your support. I updated it to

    type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>> + Send>>;

error: future cannot be sent between threads safely
  --> /Users/user/Code/abc/core/src/auth/auth_layer_v2.rs:76:9
   |
76 |         Box::pin(f)
   |         ^^^^^^^^^^^ future created by async block is not `Send`

Code in question

// We have to box the errors so the types match
let f = async move {
    fut.await.map_err(|err| err.into()).map(|mut res| {
        res.extensions_mut().insert(authenticated.clone());
        res
    })
};

Box::pin(f)

lcmgh Apr 4, 2023
Author

Found the problem. Had to use T::Future: Send + 'static,. Thx for your help!

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Problem with error handler when using fallible tower middleware #1908

{{title}}

Replies: 1 comment 7 replies

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

Select a reply

Problem with error handler when using fallible tower middleware #1908

lcmgh Apr 4, 2023

Replies: 1 comment · 7 replies

davidpdrsn Apr 4, 2023 Collaborator

lcmgh Apr 4, 2023 Author

lcmgh Apr 4, 2023 Author

davidpdrsn Apr 4, 2023 Collaborator

lcmgh Apr 4, 2023 Author

lcmgh Apr 4, 2023 Author

lcmgh
Apr 4, 2023

Replies: 1 comment 7 replies

davidpdrsn
Apr 4, 2023
Collaborator

lcmgh Apr 4, 2023
Author

lcmgh Apr 4, 2023
Author

davidpdrsn Apr 4, 2023
Collaborator

lcmgh Apr 4, 2023
Author

lcmgh Apr 4, 2023
Author