From 417c06b00e0a97fc7163181805892fbf0f44bf79 Mon Sep 17 00:00:00 2001 From: Roland Date: Tue, 19 Jul 2022 08:40:01 +0800 Subject: [PATCH] fix: broken stream when authentication failed (#260) * fix: broken http stream when authentication failed Signed-off-by: Roland Ma * remove unchanged Signed-off-by: Roland Ma * Update CHANGES.md * Update CHANGES.md * Update CHANGES.md Co-authored-by: Rob Ede --- actix-web-httpauth/CHANGES.md | 3 ++ actix-web-httpauth/examples/middleware.rs | 5 ++- actix-web-httpauth/examples/with-cors.rs | 2 +- actix-web-httpauth/src/middleware.rs | 37 ++++++++++++----------- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/actix-web-httpauth/CHANGES.md b/actix-web-httpauth/CHANGES.md index 86a6b5c25..d7bdbcbbd 100644 --- a/actix-web-httpauth/CHANGES.md +++ b/actix-web-httpauth/CHANGES.md @@ -1,8 +1,11 @@ # Changes ## Unreleased - 2022-xx-xx +- Auth validator functions now need to return `(Error, ServiceRequest)` in error cases. [#260] - Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. +[#260]: https://github.com/actix/actix-extras/pull/260 + ## 0.6.0 - 2022-03-01 - Update `actix-web` dependency to `4`. diff --git a/actix-web-httpauth/examples/middleware.rs b/actix-web-httpauth/examples/middleware.rs index 4df4c0341..487f96346 100644 --- a/actix-web-httpauth/examples/middleware.rs +++ b/actix-web-httpauth/examples/middleware.rs @@ -3,7 +3,10 @@ use actix_web::{middleware, web, App, Error, HttpServer}; use actix_web_httpauth::extractors::basic::BasicAuth; use actix_web_httpauth::middleware::HttpAuthentication; -async fn validator(req: ServiceRequest, _credentials: BasicAuth) -> Result { +async fn validator( + req: ServiceRequest, + _credentials: BasicAuth, +) -> Result { Ok(req) } diff --git a/actix-web-httpauth/examples/with-cors.rs b/actix-web-httpauth/examples/with-cors.rs index eb2f890ca..4f034a295 100644 --- a/actix-web-httpauth/examples/with-cors.rs +++ b/actix-web-httpauth/examples/with-cors.rs @@ -5,7 +5,7 @@ use actix_web_httpauth::{extractors::bearer::BearerAuth, middleware::HttpAuthent async fn ok_validator( req: ServiceRequest, credentials: BearerAuth, -) -> Result { +) -> Result { eprintln!("{:?}", credentials); Ok(req) } diff --git a/actix-web-httpauth/src/middleware.rs b/actix-web-httpauth/src/middleware.rs index 16b0004f4..68a41ea8b 100644 --- a/actix-web-httpauth/src/middleware.rs +++ b/actix-web-httpauth/src/middleware.rs @@ -39,7 +39,7 @@ impl HttpAuthentication where T: AuthExtractor, F: Fn(ServiceRequest, T) -> O, - O: Future>, + O: Future>, { /// Construct `HttpAuthentication` middleware with the provided auth extractor `T` and /// validation callback `F`. @@ -54,7 +54,7 @@ where impl HttpAuthentication where F: Fn(ServiceRequest, basic::BasicAuth) -> O, - O: Future>, + O: Future>, { /// Construct `HttpAuthentication` middleware for the HTTP "Basic" authentication scheme. /// @@ -70,7 +70,7 @@ where /// async fn validator( /// req: ServiceRequest, /// credentials: BasicAuth, - /// ) -> Result { + /// ) -> Result { /// // All users are great and more than welcome! /// Ok(req) /// } @@ -85,7 +85,7 @@ where impl HttpAuthentication where F: Fn(ServiceRequest, bearer::BearerAuth) -> O, - O: Future>, + O: Future>, { /// Construct `HttpAuthentication` middleware for the HTTP "Bearer" authentication scheme. /// @@ -96,7 +96,7 @@ where /// # use actix_web_httpauth::middleware::HttpAuthentication; /// # use actix_web_httpauth::extractors::bearer::{Config, BearerAuth}; /// # use actix_web_httpauth::extractors::{AuthenticationError, AuthExtractorConfig}; - /// async fn validator(req: ServiceRequest, credentials: BearerAuth) -> Result { + /// async fn validator(req: ServiceRequest, credentials: BearerAuth) -> Result { /// if credentials.token() == "mF_9.B5f-4.1JqM" { /// Ok(req) /// } else { @@ -105,7 +105,7 @@ where /// .unwrap_or_else(Default::default) /// .scope("urn:example:channel=HBO&urn:example:rating=G,PG-13"); /// - /// Err(AuthenticationError::from(config).into()) + /// Err((AuthenticationError::from(config).into(), req)) /// } /// } /// @@ -121,7 +121,7 @@ where S: Service, Error = Error> + 'static, S::Future: 'static, F: Fn(ServiceRequest, T) -> O + 'static, - O: Future> + 'static, + O: Future> + 'static, T: AuthExtractor + 'static, B: MessageBody + 'static, { @@ -155,7 +155,7 @@ where S: Service, Error = Error> + 'static, S::Future: 'static, F: Fn(ServiceRequest, T) -> O + 'static, - O: Future> + 'static, + O: Future> + 'static, T: AuthExtractor + 'static, B: MessageBody + 'static, { @@ -178,9 +178,12 @@ where } }; - // 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?; + let req = match process_fn(req, credentials).await { + Ok(req) => req, + Err((err, req)) => { + return Ok(req.error_response(err).map_into_right_body()); + } + }; service.call(req).await.map(|res| res.map_into_left_body()) } @@ -362,10 +365,10 @@ mod tests { #[actix_web::test] async fn test_middleware_works_with_app() { async fn validator( - _req: ServiceRequest, + req: ServiceRequest, _credentials: BasicAuth, - ) -> Result { - Err(ErrorForbidden("You are not welcome!")) + ) -> Result { + Err((ErrorForbidden("You are not welcome!"), req)) } let middleware = HttpAuthentication::basic(validator); @@ -387,10 +390,10 @@ mod tests { #[actix_web::test] async fn test_middleware_works_with_scope() { async fn validator( - _req: ServiceRequest, + req: ServiceRequest, _credentials: BasicAuth, - ) -> Result { - Err(ErrorForbidden("You are not welcome!")) + ) -> Result { + Err((ErrorForbidden("You are not welcome!"), req)) } let middleware = actix_web::middleware::Compat::new(HttpAuthentication::basic(validator));