working auth filter rejection with authentication header and realm name

[mike-kfed]

This is for issue #62 based on pr #716 but mine actually works. However I understand it is a crazy hack how I pass the realm name along to create the additional header. If some maintainer could guide me how to make that clean please? I'll fix it as needed.

For cleaner implementation: I'd assume we need a new rejection type. Because a failed authentication (i.e. user/pass wrong) will require the same headers returned as this authentication header missing filter does, only now needed by another filter: the one checking the credentials.

[mike-kfed]

While getting rid of unit-error and looking at your attempt with the Authorizer trait, I realized my example is broken. Here's a quick attempt to work around the limitation:

use headers::authorization::Basic;
use std::collections::HashMap;
use std::sync::Arc;
use warp::Filter;

async fn is_authenticated(
    auth_header: Basic,
    pwdb: Arc<HashMap<&str, &str>>,
) -> Result<String, warp::Rejection> {
    let user = auth_header.username();
    if pwdb.get(user) == Some(&auth_header.password()) {
        Ok(user.to_string())
    } else {
        Err(warp::reject::unauthorized("Basic", "Realm")) // stupid hack making unauthorized public
    }
}

#[tokio::main]
async fn main() {
    let pwdb: HashMap<_, _> = vec![
        ("alice", "wonderland"),
        ("bob", "cat"),
        ("carl", "IePai4ph"),
    ]
    .into_iter()
    .collect();
    let pwdb = Arc::new(pwdb); // no lock-guard needed, it's read-only
    let pwdb = warp::any().map(move || pwdb.clone());
    let routes = warp::auth::basic("Realm name")
        .and(pwdb.clone())
        .and_then(is_authenticated)
        .map(|user: String| format!("Hello, {} you're authenticated", user));

    warp::serve(routes).run(([127, 0, 0, 1], 3030)).await;
}

Another important thing, the CONTENT_TYPE header is hardcoded to "text/plain; charset=utf-8" in everything we do. this is another stumbling block for securing REST-APIs (especially when Bearer scheme is used this is a no-go, people will use this for json).

In my application I hacked around all those limitations making a custom rejection handler and more boilerplate. Would be nice to also allow for custom-content-type too while we are at it.

I like your Authorizer trait idea, seems more flexible than my simple and_then filter approach.

[FreeMasen]

Another important thing, the CONTENT_TYPE header is hardcoded to "text/plain; charset=utf-8" in everything we do. this is another stumbling block for securing REST-APIs (especially when Bearer scheme is used this is a no-go, people will use this for json).

I am not sure I follow this, the 401 reply is sent back with an empty body so the Content-Type of that reply is going to be plain/text. Any subsequent handler would be able to reply with whatever Content-Type they would like. Are you saying that you'd like configure the reply's body when the header is not found? If so, then adding a new Rejection would probably be a requirement that a user could .recover from.

I like your Authorizer trait idea, seems more flexible than my simple and_then filter approach.

I would be curious where you end up with this, I ultimately decided that this wasn't the right abstraction but it could have been my implementation.

[mike-kfed]

the problem arises when the authentication fails, then the warp reject code we are building here is active, correct? If that is the case you'll have a content-type of plain/text instead of potentially wanted json. Plus no control of output on auth-error, you might want to send something like {"error": "blah", "msg": "password expired"}. Sure if auth succeeds then whatever the user serves is correctly handled, since our filter here was passed.

Basically we need to be able to handle

1. no request-header came in
2. header came in but password/token wrong
3. header came in but invalid
4. header came in auth ok.

cases 1, 3 and 4 are easily doable with crate only code, number 2 is where the user likely wants to customise.

[FreeMasen]

header came in but password/token wrong

This would have to follow down the rejection path. It is a bit more boilerplate but I think allowing for rejections to be handled outside of the primary handler is a nice thing about this pattern. Here is a gist with the basic pattern:

https://gist.github.com/FreeMasen/c0ec97e03bfe93d3d703ee31a605bcbf

[mike-kfed]

thanks for the help, that is basically what I built for my app - I think it is a lot of boilerplate necessary to "just" have authentication. If we can find a way to hide that for the users of warp it'd be cooler. I'll return trying to find a nice solution on monday, no computer accessible this weekend :)

[mike-kfed]

I made my code a bit more readable and played with your example code. Problem using the rejection handler is now

• not providing user/pass returns Error 500 because it does not handle UnauthorizedChallenge rejections
• if I were to add UnauthorizedChallenge handling, that struct would have to be public and not private.
  • Let's suppose that it was public: we'd write redundant boiler plate returning the www-authenticate header with realm/scheme which is already in src/reject.rs.
  • for browsers we need to return www-authenticate header and 401 if the password was wrong to make them popup the user/pass input, just 403 forbidden makes it impossible for the user to fix his error since the browser will keep sending the Authorization header. If this is done in the rejection handler, again redundant code that is already in src/reject.rs
• using rej.into_response() in the reject handler doesn't work either because warp::reject::sealed::IsReject trait is private
• problem I remember from my private project, if you add a websocket server to this you need to add e.g. MissingConnectionUpgrade handling, even more boilerplate just because authentication is of interest lots of other stuff needs to be handled too.

I think we need to figure out a better way for .recover() in general. I'd propose we polish up the filters::auth and reject modules, write a working example with all boilerplate needed, try to get that merged into master. And then open a new issue that discusses getting rid of the boilderplate? what do you think?

[mike-kfed]

quickhack for into_response() being private:

diff --git a/src/reject.rs b/src/reject.rs
index 05e4a86..e2cbb4c 100644
--- a/src/reject.rs
+++ b/src/reject.rs
@@ -337,6 +337,11 @@ impl Rejection {
             false
         }
     }
+
+    /// workaround for recover() handler
+    pub fn make_response(&self) -> hyper::Response<Body> {
+        self.into_response()
+    }
 }

 impl<T: Reject> From<T> for Rejection {

then your examples else branch is simply

let res = if let Some(inner) = rej.find::<AuthRejections>() {
// ...
} else {
    rej.make_response()
};

but this change goes way beyond this issues title I feel.

[FreeMasen]

* not providing user/pass returns Error 500 because it does not handle `UnauthorizedChallenge` rejections

My expectations is that the UnauthorizedChallenge wouldn't be a rejection but a reply that is already taken care of by the framework. If I instrument an endpoint with warp::auth::basic("MyRealm", |basic| { //... }) I want warp to send the 401 for me automatically. If that isn't something provided by the filter it really doesn't seem very useful since you can get the same via warp::header("Authorization")

[mike-kfed]

This is what I mean, as soon as you recover you have to deal with any rejection, the examples/rejections.rs even shows that you need to deal with 404 and everything. So yes right now you could build this yourself with warp::header("Authorization") no advantage using warp's library filters, which is the point I tried to make - a fundamental change of how this works is needed. (Or I am misunderstanding how . recover() is supposed to be used, my experience so far is it hands over any rejection in the route giving you responsibility to handle it yourself.)

[FreeMasen]

(Or I am misunderstanding how . recover() is supposed to be used

I think you are understanding how recover is supposed to be used. In the case that a route is defined using basic I would expect that an reply, not a rejection, would be sent by that filter to orchestrate the challenge. CORS preflight requests would be a similar example, an incoming OPTIONS request is not handled by the instrumented route, instead is replied to by warp under the hood.

Another important thing, the CONTENT_TYPE header is hardcoded to "text/plain; charset=utf-8" in everything we do. this is another stumbling block for securing REST-APIs

I believe this is still the point you are trying to make that I may be misunderstanding. Are you saying that you want to reply to an unauthorized request with some kind of custom body during the Unauthorized Challenge? If so, can you provide a little more detail about the use case you see as being a stumbling block? I would expect the body of the Unauthorized Challenge response to be empty making the content type not really important

[mike-kfed]

the issue is for browsers you need to send the 401 + www-authenticate header in 2 cases

1. the user has not provided any auth info
2. the user provided the wrong auth info, i.e. after a custom is_authenticated call

If the basic() filter returns a reply for case 1. that could be okay, but you need to then redundantly construct the reply for case 2. in your code since the basic() will not pass along the scheme+realm into the rejection handler (it's a reply now). Also, given that 404 and more is a rejection, as soon I want to custom handle the auth using rejections, you also have to custom handle 404, connectionupgrade and whatnot. When all you wanted to do is create and handle a certain type of rejections. We need a rejections "filter" to go before .recover() maybe, that auto-handles rejections into replies except the ones matching a certain pattern?

for the content-type issue, if you want to write a pure json only api, you should be able to serve only json without sometimes changing to text/plain. This can be done by copying what is being coded here and handling every rejection, so one could argue it's fine to write lots of boilerplate for this special usecase (even though it'd be a verbatim copy changing only the content-type). Something like nginx config types { } default_type "application/json"; to override would be cool to have (overriding content-types of whatever warp does internally).

For standard authentication without custom requirements beyond authentication/authorization I feel it is too much extra code to write and confusing. I wouldn't expect I have to handle 404 myself when I only want to handle auth myself and vice versa.

[mike-kfed]

I would expect the body of the Unauthorized Challenge response to be empty making the content type not really important

most webservers send a human readable string informing them auth is necessary, otherwise the browser window stays empty when the user clicks "cancel" in his auth-popup. But yeah, when the content is empty, you can also argue it is not necessary to send a content-type at all ;)

[FreeMasen]

the user provided the wrong auth info, i.e. after a custom is_authenticated call

I would expect this should lead to a 403 and not another 401 as defined here, also for reference

if you want to write a pure json only api,

I think this is a separate issue entirely. I could possibly see the inclusion of another Server level configuration or some kind of with(warp::default_content_type("json")) but in this case I would say worrying about the content-type is out of scope. Alternatively it may already be covered by the with_header fn

[mike-kfed]

I would expect this should lead to a 403 and not another 401 as defined here, also for reference

correct, I patched it to send empty body and no content-type now.

I think this is a separate issue entirely.

I agree.

Leaves us with how to best treat recover for rejections lowering the coding-load of auth implementations. My hack from above is basically the rejections-filter doing the default warp action if not handled myself. what do you think?

[mike-kfed]

CORS preflight requests would be a similar example, an incoming OPTIONS request is not handled by the instrumented route, instead is replied to by warp under the hood.

I just played with this, the cors filter also is implemented with rejections, difference is it's use of wrapsealed, however if your example code was extended with CORS this way, the handle_rejections also gets the CORS rejections to handle:

    let routes = warp::auth::basic("Realm name")
        .and(pwdb.clone())
        .and_then(is_authenticated)
        .map(|user: String| format!("Hello, {} you're authenticated", user))
        .with(cors)
        .recover(handle_rejections);

normally nobody notices that because they probably chain the .with() at the end of their route avoiding it by chance. So I guess the auth filter is implemented correctly, it's just a question of how to resolve issue #451

[FreeMasen]

normally nobody notices that because they probably chain the .with() at the end of their route avoiding it by chance. So I guess the auth filter is implemented correctly, it's just a question of how to resolve issue #451

I feel like auth should probably be a with style filter as apposed to the normal route style filter (these should have better names...). As it stands I haven't been able to figure out how to do async work from a with style filter, either using the wrap_fn filter or trying to setup my own auth filters.

[mike-kfed]

I agree a with style filter would help, and it might be worth the effort to get one going. But in the end it is probably easier to make a public interface to convert a rejection into a impl Reply which is needed anyway for other use-cases too.

[mike-kfed]

Hello @FreeMasen I started a new branch for a with-filter called http_basic_auth_cb, based on PR #819 but allowing for a callback function to authenticate, filter code here basic.rs how to use it is shown here tests/auth_basic.rs

It'd be great if we can work together on making that work with an async callback? or discuss a different API for building an auth filter? Anyways please let me know what you think