Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conflict between HttpAuthentication::bearer and actix-cors #127

Closed
salfonseca opened this issue Nov 10, 2020 · 11 comments · Fixed by #260
Closed

Conflict between HttpAuthentication::bearer and actix-cors #127

salfonseca opened this issue Nov 10, 2020 · 11 comments · Fixed by #260
Assignees
Labels
A-httpauth Project: actix-web-httpauth C-bug Something isn't working

Comments

@salfonseca
Copy link

Expected Behavior

Be able to use HttpAuthentication::bearer and actix-cors in the same project

Current Behavior

Hi, I am trying to use HttpAuthentication::bearer in a project that also uses actix-cors, if I use HttpAuthentication::bearer or actix-cors alone it works fine, but if I use both I get an error saying that cors has block the request due to No 'Access-Control-Allow-Origin' header is present on the requested resource.

Steps to Reproduce (for bugs)

My code is:

#[actix_rt::main]
pub async fn init() -> std::io::Result<()> {
 let port = env::var("PORT").unwrap_or_else(|_e| "8000".to_string());
 let mut socket = "127.0.0.1".to_string();
 socket.push(':');
 socket = socket + &port;
 let db_state = db_mongodb::DB::init().await;

 HttpServer::new(move || {
    App::new()
        .wrap(
            Cors::permissive()
        )
        .data(db_state.clone())
        .service(
            web::scope("/api")
            .wrap(HttpAuthentication::bearer(auth::validator))
            .configure(routes::init_secured)
        )
        .service(
            web::scope("/public")
            .configure(routes::init_public)
        )
 })
 .bind(socket).unwrap()
 .run()
 .await
}

Your Environment

  • Rust Version: rustc 1.47.0 (18bf6b4f0 2020-10-07)
  • actix-web = "3.2.0"
  • actix-rt = "1.1.1"
  • actix-web-httpauth = "0.5.0"
  • actix-cors = "0.5.1"
  • Operating System: Debian GNU/Linux 10 (buster)
  • Kernel: Linux 4.19.0-12-amd64
  • Architecture: x86-64
@robjtede robjtede added C-bug Something isn't working A-httpauth Project: actix-web-httpauth labels Nov 10, 2020
@robjtede
Copy link
Member

robjtede commented Nov 10, 2020

I suspect there's an error being thrown in httpauth middleware impl that should not be.

A reminder that the semantic meaning of an error in middleware is unrecoverable and outer middlewares should not attempt to handle them.

@Valentine-Mario
Copy link

Valentine-Mario commented Nov 11, 2020

I also experienced the same issues. Got the same browser CORS error when trying to consume API on the browser after setting the CORS and HttpAuthentication::bearer

Edit: Fixed it by removing the HttpAuthentication Middleware and handling http auth in the individual functions (which is obviously a more hectic approach).

@Pytal
Copy link

Pytal commented Nov 15, 2020

I initially thought that I was having the same issue as well with actix-cors and actix-web-httpauth not working together, but it turned out that the No 'Access-Control-Allow-Origin' header is present on the requested resource error on the client was the result of unhandled errors in my routes.

This probably caused the cors middleware to not run resulting in the expected headers not being attached.

@ctron
Copy link

ctron commented Nov 20, 2020

I noticed the merged PR, and a new release ('0.5.3`). Still, I see the same behavior.

Should that problem be fixed now?

@robjtede
Copy link
Member

Not yet. It will still skip downstream middleware if the validation function returns an error. That PR fixed a different case causing the same behaviour without changing the API.

@ctron
Copy link

ctron commented Nov 20, 2020

Not yet. It will still skip downstream middleware if the validation function returns an error. That PR fixed a different case causing the same behaviour without changing the API.

Thanks for the quick response. I overlooked that I need to update actix-web-httpauth as well. I patched my dependency to use the version from master now, and that seems to fix my issue.

I would appreciate a release with the current patch already.

@neo-clon
Copy link

I have the same issue

When my middleware return ErrorUnauthorized, cors no set the Allowed Origin header

image

I'm using this version
actix-web = "3.3.2"
actix-web-httpauth = "0.5.0"
actix-cors = "0.5.4"

image

image

image

@jordan-boyer
Copy link

jordan-boyer commented Mar 27, 2021

I have the same issue and i would like the help solving it.

Here my suggestion.

In httpauth middleware the signature for the return of the process_fn is this Future<Output = Result<ServiceRequest, Error>> where Error is an _actix_web::Error

I don't think that the process must yield an unrecoverable error so why not replace the signature by this Future<Output = Result<ServiceRequest, ServiceResponse>>

I know that it will be a breaking change but let me explain.

We can now replace this code:

// TODO: alter to remove ? operator; an error response is required for downstream
// middleware to do their thing (eg. cors adding headers)
let req = process_fn(req, credentials).await?;

service.call(req).await

By this:

match process_fn(req, credentials).await {
    Ok(req) => {
        let fut = service.borrow_mut().call(req);
        fut.await
    },
    Err(res) => {
        Ok(res)
     },
}

For me the user is in charge of what happen in the validation function and the user must take a decision when an error occur.

The middleware should not worry about an error occurring in this function and should in this case break the chain of middleware and send the response back directly but this will not break middleware call before httpauth.

If you guys want to see it in action i would be glad to put this in a PR with some test case.

P.S: english is not my native language so feel free to correct my mistake and let me know if you guys didn't understand something =)

@spensk8
Copy link

spensk8 commented Mar 9, 2022

I was experiencing a similar problem until I realized that the "wrap's" execute in reverse order and therefore the bearer authentication middleware was checking if the OPTIONS request had the 'Authorization' token (which it should not have)
In your case you have a "wrap" inside a "service" so I'm not sure how the order is handled in that scenario.
This worked for me:

const DEFAULT_SERVER_URL: &str = "localhost:8080";
const DEFAULT_FRONTEND_URL: &str = "http://localhost:8082";

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    env::set_var("RUST_LOG", "actix_web=debug,actix_server=info");
    env_logger::init();

    HttpServer::new(move || {
        let cors = Cors::default()
            .supports_credentials()
            .allowed_origin(env::var("FRONTEND_URL").unwrap_or(DEFAULT_FRONTEND_URL.to_string()).as_str())
            .allowed_methods(vec!["GET", "POST"])
            .allowed_headers(vec![
                http::header::AUTHORIZATION, http::header::ACCEPT, http::header::CONTENT_TYPE
            ]);
        App::new()
            // Middlewares execute in reverse order
            .wrap(HttpAuthentication::bearer(auth::validation::validator)) // validation defined in a separate file
            .wrap(cors)
            .wrap(middleware::Logger::default()) // enable logger - should be always last
            .service(web::scope("/api").configure(app::routes::scoped_config)) // routes defined in a separate file
    })
        .bind(env::var("SERVER_URL").unwrap_or(DEFAULT_SERVER_URL.to_string()))?
        .run()
        .await
}
# actix dependencies versions
actix-rt = "2.7.0"
actix-web = "4.0.1"
actix-cors = "0.6.1"
actix-web-httpauth = "0.6.0"

@robjtede
Copy link
Member

fixed in 0.7.0

@itmor
Copy link

itmor commented Feb 17, 2024

воиспроизводится

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-httpauth Project: actix-web-httpauth C-bug Something isn't working
Projects
None yet
9 participants