actix / actix-web-httpauth

Moved to actix-extras repo.
https://github.com/actix/actix-extras/tree/HEAD/actix-web-httpauth
Apache License 2.0
82 stars 37 forks source link

Tighten HttpAuthentication::with_fn bound from FnMut to Fn. #11

Closed qnighy closed 5 years ago

qnighy commented 5 years ago

Three constructor functions for HttpAuthentication only requires F: FnMut. However, the constructed instances are only usable when F: Fn. This is unfortunate for closures, since the passed closure will be inferred to be only FnMut when it only captures immutably.

use actix_web::{middleware, web, App, HttpServer};

use futures::future;

use actix_web_httpauth::middleware::HttpAuthentication;

fn main() -> std::io::Result<()> {
    HttpServer::new(|| {
        let auth = HttpAuthentication::basic(|req, _credentials| future::ok(req));
        App::new()
            .wrap(middleware::Logger::default())
            .wrap(auth) // Compile fails here
            .service(web::resource("/").to(|| "Test\r\n"))
    })
    .bind("127.0.0.1:8080")?
    .workers(1)
    .run()
}

Technically it's a breaking change, but as it's actually unusable without F: Fn, I'd think it's considered a bugfix. If this is unsuitable, I'll prepare another PR that introduces HttpAuthentication::with_fn2 etc.

otavio commented 5 years ago

Agreed! I'd like it to go in as well :-)

svartalf commented 5 years ago

Hey, @qnighy, thank you for a contribution!

I'm a little busy today, so I'll review the changes tomorrow, but so far it looks okay to me.

svartalf commented 5 years ago

Published as a 0.3.2 version, thanks once again!