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

fix: broken stream when authentication failed #260

Merged
merged 6 commits into from
Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions actix-web-httpauth/CHANGES.md
Original file line number Diff line number Diff line change
@@ -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`.
Expand Down
5 changes: 4 additions & 1 deletion actix-web-httpauth/examples/middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ServiceRequest, Error> {
async fn validator(
req: ServiceRequest,
_credentials: BasicAuth,
) -> Result<ServiceRequest, (Error, ServiceRequest)> {
Ok(req)
}

Expand Down
2 changes: 1 addition & 1 deletion actix-web-httpauth/examples/with-cors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use actix_web_httpauth::{extractors::bearer::BearerAuth, middleware::HttpAuthent
async fn ok_validator(
req: ServiceRequest,
credentials: BearerAuth,
) -> Result<ServiceRequest, Error> {
) -> Result<ServiceRequest, (Error, ServiceRequest)> {
eprintln!("{:?}", credentials);
Ok(req)
}
Expand Down
37 changes: 20 additions & 17 deletions actix-web-httpauth/src/middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl<T, F, O> HttpAuthentication<T, F>
where
T: AuthExtractor,
F: Fn(ServiceRequest, T) -> O,
O: Future<Output = Result<ServiceRequest, Error>>,
O: Future<Output = Result<ServiceRequest, (Error, ServiceRequest)>>,
{
/// Construct `HttpAuthentication` middleware with the provided auth extractor `T` and
/// validation callback `F`.
Expand All @@ -54,7 +54,7 @@ where
impl<F, O> HttpAuthentication<basic::BasicAuth, F>
where
F: Fn(ServiceRequest, basic::BasicAuth) -> O,
O: Future<Output = Result<ServiceRequest, Error>>,
O: Future<Output = Result<ServiceRequest, (Error, ServiceRequest)>>,
{
/// Construct `HttpAuthentication` middleware for the HTTP "Basic" authentication scheme.
///
Expand All @@ -70,7 +70,7 @@ where
/// async fn validator(
/// req: ServiceRequest,
/// credentials: BasicAuth,
/// ) -> Result<ServiceRequest, Error> {
/// ) -> Result<ServiceRequest, (Error, ServiceRequest)> {
/// // All users are great and more than welcome!
/// Ok(req)
/// }
Expand All @@ -85,7 +85,7 @@ where
impl<F, O> HttpAuthentication<bearer::BearerAuth, F>
where
F: Fn(ServiceRequest, bearer::BearerAuth) -> O,
O: Future<Output = Result<ServiceRequest, Error>>,
O: Future<Output = Result<ServiceRequest, (Error, ServiceRequest)>>,
{
/// Construct `HttpAuthentication` middleware for the HTTP "Bearer" authentication scheme.
///
Expand All @@ -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<ServiceRequest, Error> {
/// async fn validator(req: ServiceRequest, credentials: BearerAuth) -> Result<ServiceRequest, (Error, ServiceRequest)> {
/// if credentials.token() == "mF_9.B5f-4.1JqM" {
/// Ok(req)
/// } else {
Expand All @@ -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))
/// }
/// }
///
Expand All @@ -121,7 +121,7 @@ where
S: Service<ServiceRequest, Response = ServiceResponse<B>, Error = Error> + 'static,
S::Future: 'static,
F: Fn(ServiceRequest, T) -> O + 'static,
O: Future<Output = Result<ServiceRequest, Error>> + 'static,
O: Future<Output = Result<ServiceRequest, (Error, ServiceRequest)>> + 'static,
T: AuthExtractor + 'static,
B: MessageBody + 'static,
{
Expand Down Expand Up @@ -155,7 +155,7 @@ where
S: Service<ServiceRequest, Response = ServiceResponse<B>, Error = Error> + 'static,
S::Future: 'static,
F: Fn(ServiceRequest, T) -> O + 'static,
O: Future<Output = Result<ServiceRequest, Error>> + 'static,
O: Future<Output = Result<ServiceRequest, (Error, ServiceRequest)>> + 'static,
T: AuthExtractor + 'static,
B: MessageBody + 'static,
{
Expand All @@ -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())
}
Expand Down Expand Up @@ -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<ServiceRequest, actix_web::Error> {
Err(ErrorForbidden("You are not welcome!"))
) -> Result<ServiceRequest, (actix_web::Error, ServiceRequest)> {
Err((ErrorForbidden("You are not welcome!"), req))
}
let middleware = HttpAuthentication::basic(validator);

Expand All @@ -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<ServiceRequest, actix_web::Error> {
Err(ErrorForbidden("You are not welcome!"))
) -> Result<ServiceRequest, (actix_web::Error, ServiceRequest)> {
Err((ErrorForbidden("You are not welcome!"), req))
}
let middleware = actix_web::middleware::Compat::new(HttpAuthentication::basic(validator));

Expand Down