From b803c81cdd18b50ccf089d55a373a14e56b1f7cf Mon Sep 17 00:00:00 2001 From: Ana Bujan Date: Tue, 8 Mar 2022 17:24:53 +0100 Subject: [PATCH] Add custom deny handler --- Cargo.toml | 2 +- README.md | 7 ++- .../src/permissions.rs | 30 +++++++++-- .../role-based-authorization/src/routes.rs | 27 ++++++---- src/lib.rs | 53 +++++++++++++++---- src/service.rs | 23 +++++--- src/tests/test_service.rs | 42 ++++++++++++--- 7 files changed, 147 insertions(+), 37 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 27b3d74..3c652a0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-permissions" -version = "0.1.0" +version = "0.1.1" edition = "2018" authors = ["Ana Bujan "] readme = "README.md" diff --git a/README.md b/README.md index 1e31516..4384f27 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ impl Permission for IsAllowed { Dependencies: ```toml [dependencies] -actix-permissions = "0.1.0" +actix-permissions = "0.1.1" ``` Code: ```rust @@ -94,6 +94,11 @@ You could use actix-permissions for role based authorization check, like in *rol *hello-world* example is just a proof of concept, showing how you can compose a list of permissions, access service request, payload and injected services. +## Permission Deny +By default, 403 is returned for failed permission checks. You may want to toggle between `Unauthorized` and `Forbidden`, +maybe customize 403 forbidden messages. That's why `check_with_custom_deny` is for. +Take a look at [role based authorization example](./examples/role-based-authorization) for more info. + ## Contributing This project welcomes all kinds of contributions. No contribution is too small! diff --git a/examples/role-based-authorization/src/permissions.rs b/examples/role-based-authorization/src/permissions.rs index 8a36214..9bf4c7f 100644 --- a/examples/role-based-authorization/src/permissions.rs +++ b/examples/role-based-authorization/src/permissions.rs @@ -1,17 +1,32 @@ +use crate::models::Role; +use actix_permissions::builder::Builder; +use actix_permissions::check_with_custom_deny; use actix_permissions::permission::Permission; use actix_web::dev::Payload; -use actix_web::{HttpMessage, HttpRequest}; +use actix_web::{FromRequest, Handler, HttpMessage, HttpRequest, HttpResponse, Responder, Route}; use std::future::{ready, Ready}; -use crate::models::Role; #[derive(Clone)] pub struct RolePermissionCheck { role: Role, } +fn custom_deny_handler(req: &HttpRequest, _payload: &mut Payload) -> HttpResponse { + let role_exists = req.extensions().get::().is_some(); + if role_exists { + return HttpResponse::Unauthorized().body("You don't have access rights!"); + } else { + return HttpResponse::Forbidden().body("Forbidden!"); + } +} + impl Permission for RolePermissionCheck { fn call(&self, req: &HttpRequest, _payload: &mut Payload) -> Ready> { - let is_allowed = req.extensions().get::().map(|user_role| self.role >= *user_role).unwrap_or(false); + let is_allowed = req + .extensions() + .get::() + .map(|user_role| self.role >= *user_role) + .unwrap_or(false); let res: actix_web::Result = Ok(is_allowed); ready(res) } @@ -21,3 +36,12 @@ impl Permission for RolePermissionCheck { pub fn has_min_role(role: Role) -> RolePermissionCheck { RolePermissionCheck { role } } + +pub fn check(route: Route, builder: Builder, handler: F) -> Route +where + F: Handler, + Args: FromRequest + 'static, + F::Output: Responder, +{ + check_with_custom_deny(route, builder, handler, custom_deny_handler) +} diff --git a/examples/role-based-authorization/src/routes.rs b/examples/role-based-authorization/src/routes.rs index 7aec190..7a7f87b 100644 --- a/examples/role-based-authorization/src/routes.rs +++ b/examples/role-based-authorization/src/routes.rs @@ -1,6 +1,6 @@ -use actix_permissions::{check, with}; -use actix_web::*; +use actix_permissions::with; use actix_web::web::ServiceConfig; +use actix_web::*; use crate::models::Role; use crate::permissions::*; @@ -20,13 +20,22 @@ async fn index() -> Result { pub fn routes(cfg: &mut ServiceConfig) { cfg.route( "/", - check(web::get(), with(has_min_role(Role::User)), index, ), - ).route( + check(web::get(), with(has_min_role(Role::User)), index), + ) + .route( "/admin", - check(web::get(), with(has_min_role(Role::Administrator)), administrators_index, ), + check( + web::get(), + with(has_min_role(Role::Administrator)), + administrators_index, + ), ) - .route( - "/mod", - check(web::get(), with(has_min_role(Role::Moderator)), moderators_index, ), - ); + .route( + "/mod", + check( + web::get(), + with(has_min_role(Role::Moderator)), + moderators_index, + ), + ); } diff --git a/src/lib.rs b/src/lib.rs index 80608de..688b716 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,4 @@ -//! Actix Permissions are extensions for permission and input validation for actix-web +//! Actix Permissions are extensions for permission and input validation for actix-web. //! //! # Examples //! ```no_run @@ -44,19 +44,20 @@ use std::future::ready; use std::sync::Arc; -use actix_web::dev::fn_factory; -use actix_web::{FromRequest, Handler, Responder, Route}; +use actix_web::dev::{fn_factory, Payload}; +use actix_web::http::StatusCode; +use actix_web::{FromRequest, Handler, HttpRequest, HttpResponse, Responder, Route}; use crate::builder::Builder; use crate::permission::Permission; use crate::service::PermissionService; -mod builder; +pub mod builder; pub mod permission; pub(crate) mod service; mod tests; -/// Creates a permission builder, initiated with single permission +/// Creates a permission builder, initiated with single permission. /// /// # Arguments /// * `permission` - permission @@ -67,11 +68,14 @@ where Builder::new().and(permission) } -/// Creates a permission builder, initiated with variable number of permissions +fn default_deny_handler(_req: &HttpRequest, _payload: &mut Payload) -> HttpResponse { + HttpResponse::new(StatusCode::FORBIDDEN) +} /// Creates a route which: -/// - intercepts requests and validates inputs -/// - if permission checks are all true, passes through to handler +/// - intercepts requests and validates inputs. +/// - if permission checks are all true, passes through to handler. +/// - if any of the permissions is false, FORBIDDEN is returned. pub fn check(route: Route, builder: Builder, handler: F) -> Route where F: Handler, @@ -82,6 +86,37 @@ where route.service(fn_factory(move || { let new_perms_c = Arc::clone(&new_perms); let handler = handler.clone(); - ready(Ok(PermissionService::new(new_perms_c, handler))) + ready(Ok(PermissionService::new( + new_perms_c, + handler, + default_deny_handler, + ))) + })) +} + +/// Creates a more flexible route than `check`, which: +/// - intercepts requests and validates inputs. +/// - if permission checks are all true, passes through to handler. +/// - if any of the permissions is false, `deny_handler` is called. +pub fn check_with_custom_deny( + route: Route, + builder: Builder, + handler: F, + deny_handler: fn(&HttpRequest, &mut Payload) -> HttpResponse, +) -> Route +where + F: Handler, + Args: FromRequest + 'static, + F::Output: Responder, +{ + let new_perms = Arc::new(builder.permissions); + route.service(fn_factory(move || { + let new_perms_c = Arc::clone(&new_perms); + let handler = handler.clone(); + ready(Ok(PermissionService::new( + new_perms_c, + handler, + deny_handler, + ))) })) } diff --git a/src/service.rs b/src/service.rs index e8d8076..9c6d0ba 100644 --- a/src/service.rs +++ b/src/service.rs @@ -1,13 +1,13 @@ -use crate::Permission; -use actix_web::dev::{Service, ServiceRequest, ServiceResponse}; -use actix_web::http::StatusCode; -use actix_web::{dev, FromRequest, Handler, HttpResponse, Responder}; -use futures_core::future::LocalBoxFuture; use std::convert::Infallible; use std::marker::PhantomData; use std::sync::Arc; -/// +use actix_web::dev::{Payload, Service, ServiceRequest, ServiceResponse}; +use actix_web::{dev, FromRequest, Handler, HttpRequest, HttpResponse, Responder}; +use futures_core::future::LocalBoxFuture; + +use crate::Permission; + /// Service that intercepts request, validates it with a list of permissions. /// If any of the permissions fail, 403 forbidden is returned. /// If permissions succeed, request is proxied to handler @@ -25,6 +25,7 @@ where perms: Arc>>, handler: F, phantom_data: PhantomData, + deny_handler: fn(&HttpRequest, &mut Payload) -> HttpResponse, } impl PermissionService @@ -33,11 +34,16 @@ where Args: FromRequest, F::Output: Responder, { - pub fn new(perms: Arc>>, handler: F) -> Self { + pub fn new( + perms: Arc>>, + handler: F, + deny_handler: fn(&HttpRequest, &mut Payload) -> HttpResponse, + ) -> Self { Self { perms, handler, phantom_data: PhantomData::default(), + deny_handler, } } } @@ -58,13 +64,14 @@ where let (req, mut payload) = args.into_parts(); let perms = Arc::clone(&self.perms); let handler = self.handler.clone(); + let deny_handler = self.deny_handler; Box::pin(async move { for permission in perms.iter() { let result = permission.call(&req, &mut payload).await; match result { Ok(false) => { - let response = HttpResponse::new(StatusCode::FORBIDDEN); // TODO: Forbidden or Unauthorized? + let response = deny_handler(&req, &mut payload); return Ok(ServiceResponse::new(req, response)); } Err(err) => { diff --git a/src/tests/test_service.rs b/src/tests/test_service.rs index d9470f0..8ea5fd5 100644 --- a/src/tests/test_service.rs +++ b/src/tests/test_service.rs @@ -1,11 +1,13 @@ #[cfg(test)] mod tests { - use crate::PermissionService; - use actix_web::dev::{Payload, Service}; - use actix_web::{test, Error, HttpRequest}; use std::future::{ready, Ready}; use std::sync::Arc; + use actix_web::dev::{Payload, Service}; + use actix_web::{test, Error, HttpRequest, HttpResponse}; + + use crate::{default_deny_handler, PermissionService, StatusCode}; + async fn index() -> Result { Ok("Welcome!".to_string()) } @@ -13,7 +15,7 @@ mod tests { #[actix_web::test] async fn test_no_permission_checks_set() { let service_req = test::TestRequest::with_uri("/").to_srv_request(); - let service = PermissionService::new(Arc::new(vec![]), index); + let service = PermissionService::new(Arc::new(vec![]), index, default_deny_handler); let result = service.call(service_req).await; @@ -27,13 +29,41 @@ mod tests { ready(Ok(false)) } + fn custom_deny_handler(_req: &HttpRequest, _payload: &mut Payload) -> HttpResponse { + HttpResponse::new(StatusCode::UNAUTHORIZED) + } + #[actix_web::test] async fn test_deny_all() { let service_req = test::TestRequest::with_uri("/").to_srv_request(); - let service = PermissionService::new(Arc::new(vec![Box::new(deny_all)]), index); + let service = PermissionService::new( + Arc::new(vec![Box::new(deny_all)]), + index, + default_deny_handler, + ); let result = service.call(service_req).await; - assert!(result.is_ok()) + assert!(result.is_ok()); + + let result = result.unwrap(); + assert_eq!(result.status(), StatusCode::FORBIDDEN) + } + + #[actix_web::test] + async fn test_deny_all_custom_handler() { + let service_req = test::TestRequest::with_uri("/").to_srv_request(); + let service = PermissionService::new( + Arc::new(vec![Box::new(deny_all)]), + index, + custom_deny_handler, + ); + + let result = service.call(service_req).await; + + assert!(result.is_ok()); + + let result = result.unwrap(); + assert_eq!(result.status(), StatusCode::UNAUTHORIZED) } }