From f8ecc1ae5df2ee8f84ed1c019f589b35a907f2eb Mon Sep 17 00:00:00 2001 From: kenpaicat <133065911+kenpaicat@users.noreply.github.com> Date: Sat, 23 Sep 2023 21:25:51 +0200 Subject: [PATCH] simplfy role-auth middleware --- crates/laguna-backend-middleware/src/auth.rs | 74 ++++------- .../src/auth_helper.rs | 23 ---- .../src/exclusive.rs | 117 ------------------ crates/laguna-backend-middleware/src/lib.rs | 2 - crates/laguna-backend-setup/src/lib.rs | 19 ++- 5 files changed, 33 insertions(+), 202 deletions(-) delete mode 100644 crates/laguna-backend-middleware/src/auth_helper.rs delete mode 100644 crates/laguna-backend-middleware/src/exclusive.rs diff --git a/crates/laguna-backend-middleware/src/auth.rs b/crates/laguna-backend-middleware/src/auth.rs index 39e24313..6df38b69 100644 --- a/crates/laguna-backend-middleware/src/auth.rs +++ b/crates/laguna-backend-middleware/src/auth.rs @@ -9,23 +9,17 @@ use actix_web::{Error, HttpMessage, HttpResponse, ResponseError}; use std::fmt; use futures_util::future::LocalBoxFuture; -use jwt_compact::alg::{Hs256, Hs256Key}; -use jwt_compact::{AlgorithmExt, UntrustedToken, ValidationError}; + use laguna_backend_dto::user::UserDTO; use laguna_backend_model::role::Role; use std::future::ready; use std::future::Ready; -use crate::consts::ACCESS_TOKEN_HEADER_NAME; - -pub struct AuthorizationMiddlewareFactory { - min_role: Role, - key: Hs256Key, -} +pub struct AuthorizationMiddlewareFactory(Role); impl AuthorizationMiddlewareFactory { - pub fn new(key: Hs256Key, min_role: Role) -> Self { - Self { key, min_role } + pub fn new(min_role: Role) -> Self { + Self(min_role) } } @@ -43,8 +37,7 @@ where fn new_transform(&self, service: S) -> Self::Future { ready(Ok(AuthorizationMiddleware { - min_role: self.min_role, - key: self.key.clone(), + min_role: self.0, service, })) } @@ -52,7 +45,6 @@ where pub struct AuthorizationMiddleware { min_role: Role, - key: Hs256Key, service: S, } @@ -60,7 +52,6 @@ pub struct AuthorizationMiddleware { pub enum AuthorizationError { UnauthorizedRole { min_role: Role, actual_role: Role }, NoToken, - Invalid(ValidationError), } impl fmt::Display for AuthorizationError { @@ -79,9 +70,6 @@ impl fmt::Display for AuthorizationError { Self::NoToken => { write!(f, "No token") }, - Self::Invalid(err) => { - write!(f, "Invalid token: {}", err) - }, } } } @@ -91,7 +79,6 @@ impl ResponseError for AuthorizationError { match self { Self::UnauthorizedRole { .. } => StatusCode::UNAUTHORIZED, Self::NoToken => StatusCode::UNAUTHORIZED, - Self::Invalid(_) => StatusCode::UNAUTHORIZED, } } @@ -99,7 +86,6 @@ impl ResponseError for AuthorizationError { match self { Self::UnauthorizedRole { .. } => HttpResponse::Unauthorized().body(format!("{}", self)), Self::NoToken => HttpResponse::Unauthorized().body(format!("{}", self)), - Self::Invalid(_) => HttpResponse::Unauthorized().body(format!("{}", self)), } } } @@ -117,37 +103,27 @@ where forward_ready!(service); fn call(&self, req: ServiceRequest) -> Self::Future { - log::info!("{:?}", req.extensions()); - // Token has already been validated & verified by AuthenticationService by the time it reaches this middleware. - let access_token_header = req.headers().get(ACCESS_TOKEN_HEADER_NAME); - if let Some(access_token_header) = access_token_header { - // SECURITY: Token is trusted at this point but additional verification is however still performed. - // NOTE: This is probably not a huge bottleneck and is a consequence of using external libraries for authentication (not authorization). - let access_token = UntrustedToken::new(access_token_header.to_str().unwrap()).unwrap(); - let signed_access_token = Hs256 - .validate_for_signed_token::(&access_token, &self.key) - .map_err(AuthorizationError::Invalid); - return match signed_access_token { - Ok(signed_access_token) => { - let min_role = self.min_role; - let role = signed_access_token.token.claims().custom.role; - if role < min_role { - return Box::pin(async move { - Result::::Err( - AuthorizationError::UnauthorizedRole { - min_role, - actual_role: role, - } - .into(), - ) - }); + let user_role = if let Some(user) = req.extensions().get::() { + user.role + } else { + return Box::pin(async move { + Result::::Err(AuthorizationError::NoToken.into()) + }); + }; + let min_role = self.min_role; + if user_role < min_role { + Box::pin(async move { + Result::::Err( + AuthorizationError::UnauthorizedRole { + min_role, + actual_role: user_role, } - let fut = self.service.call(req); - Box::pin(fut) - }, - Err(err) => Box::pin(async move { Result::::Err(err.into()) }), - }; + .into(), + ) + }) + } else { + let fut = self.service.call(req); + Box::pin(fut) } - Box::pin(async move { Err(AuthorizationError::NoToken.into()) }) } } diff --git a/crates/laguna-backend-middleware/src/auth_helper.rs b/crates/laguna-backend-middleware/src/auth_helper.rs deleted file mode 100644 index be55f1d6..00000000 --- a/crates/laguna-backend-middleware/src/auth_helper.rs +++ /dev/null @@ -1,23 +0,0 @@ -use actix_web::{ - dev::{ServiceFactory, ServiceRequest}, - Error, Scope, -}; -use jwt_compact::alg::Hs256Key; -use laguna_backend_model::role::Role; - -use crate::auth::AuthorizationMiddlewareFactory; - -// This is helper but we probably don't need this. -// This is also kinda stupid. -pub trait UseAuthorizationOnScope { - fn use_authorization(self, key: Hs256Key, min_role: Role, scope: Scope) -> Self; -} - -impl UseAuthorizationOnScope for Scope -where - T: ServiceFactory, -{ - fn use_authorization(self, key: Hs256Key, min_role: Role, scope: Scope) -> Self { - self.service(scope.wrap(AuthorizationMiddlewareFactory::new(key, min_role))) - } -} diff --git a/crates/laguna-backend-middleware/src/exclusive.rs b/crates/laguna-backend-middleware/src/exclusive.rs deleted file mode 100644 index 79a38896..00000000 --- a/crates/laguna-backend-middleware/src/exclusive.rs +++ /dev/null @@ -1,117 +0,0 @@ -use actix_web::body::{BoxBody, MessageBody}; -use actix_web::dev::ServiceResponse; -use actix_web::dev::{forward_ready, Service, ServiceRequest, Transform}; -use actix_web::error::Error; -use actix_web::http::StatusCode; -use actix_web::{HttpResponse, ResponseError}; -use futures_util::future::LocalBoxFuture; -use jwt_compact::alg::{Hs256, Hs256Key}; -use jwt_compact::{AlgorithmExt, UntrustedToken}; -use laguna_backend_dto::user::UserDTO; -use std::fmt; -use std::future::ready; -use std::future::Ready; -use uuid::Uuid; - -use crate::auth::AuthorizationError; -use crate::consts::ACCESS_TOKEN_HEADER_NAME; - -pub struct ExclusiveMiddlewareFactory { - key: Hs256Key, -} - -impl ExclusiveMiddlewareFactory { - pub fn new(key: Hs256Key) -> Self { - Self { key } - } -} - -impl Transform for ExclusiveMiddlewareFactory -where - S: Service, Error = Error>, - S::Future: 'static, - B: 'static + MessageBody, -{ - type Response = ServiceResponse; - type Error = Error; - type InitError = (); - type Transform = ExclusiveMiddleware; - type Future = Ready>; - - fn new_transform(&self, service: S) -> Self::Future { - ready(Ok(ExclusiveMiddleware { - service, - key: self.key.clone(), - })) - } -} - -pub struct ExclusiveMiddleware { - key: Hs256Key, - service: S, -} - -#[derive(Debug)] -pub enum ExclusiveError { - Exclusive, -} - -impl fmt::Display for ExclusiveError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Exclusive => f.write_str("This endpoint is exclusive to the user."), - } - } -} - -impl ResponseError for ExclusiveError { - fn status_code(&self) -> StatusCode { - match self { - Self::Exclusive => StatusCode::FORBIDDEN, - } - } - - fn error_response(&self) -> HttpResponse { - HttpResponse::new(self.status_code()).set_body(BoxBody::new(self.to_string())) - } -} - -impl Service for ExclusiveMiddleware -where - S: Service, Error = Error>, - S::Future: 'static, - B: 'static + MessageBody, -{ - type Response = ServiceResponse; - type Error = Error; - type Future = LocalBoxFuture<'static, Result>; - - forward_ready!(service); - - fn call(&self, req: ServiceRequest) -> Self::Future { - let access_token_header = req.headers().get(ACCESS_TOKEN_HEADER_NAME); - if let Some(access_token_header) = access_token_header { - // SECURITY: Token is trusted at this point but additional verification is however still performed. - // NOTE: This is probably not a huge bottleneck and is a consequence of using external libraries for authentication (not authorization). - let access_token = UntrustedToken::new(access_token_header.to_str().unwrap()).unwrap(); - let signed_access_token = Hs256 - .validate_for_signed_token::(&access_token, &self.key) - .map_err(AuthorizationError::Invalid); - return match signed_access_token { - Ok(signed_access_token) => { - if signed_access_token.token.claims().custom.id - != Uuid::parse_str(req.match_info().get("id").unwrap()).unwrap() - { - return Box::pin(async move { - Result::::Err(ExclusiveError::Exclusive.into()) - }); - } - let fut = self.service.call(req); - Box::pin(fut) - }, - Err(err) => Box::pin(async move { Result::::Err(err.into()) }), - }; - } - Box::pin(async move { Err(AuthorizationError::NoToken.into()) }) - } -} diff --git a/crates/laguna-backend-middleware/src/lib.rs b/crates/laguna-backend-middleware/src/lib.rs index a65d5649..33163e53 100644 --- a/crates/laguna-backend-middleware/src/lib.rs +++ b/crates/laguna-backend-middleware/src/lib.rs @@ -3,8 +3,6 @@ #![doc(html_favicon_url = "https://sloveniaengineering.github.io/laguna-backend/favicon.ico")] #![doc(issue_tracker_base_url = "https://github.com/SloveniaEngineering/laguna-backend")] pub mod auth; -pub mod auth_helper; pub mod consts; -pub mod exclusive; pub mod hexify; pub mod mime; diff --git a/crates/laguna-backend-setup/src/lib.rs b/crates/laguna-backend-setup/src/lib.rs index d83790c3..72c6a850 100644 --- a/crates/laguna-backend-setup/src/lib.rs +++ b/crates/laguna-backend-setup/src/lib.rs @@ -172,10 +172,7 @@ pub fn get_config_fn(settings: Settings) -> impl FnOnce(&mut ServiceConfig) { "/{id}", web::patch() .to(user_patch) - .wrap(AuthorizationMiddlewareFactory::new( - secret_key.clone(), - Role::Mod, - )), + .wrap(AuthorizationMiddlewareFactory::new(Role::Mod)), ) .route("/{id}/role_change", web::patch().to(user_role_change)) .route("/me", web::get().to(user_me_get)) @@ -188,9 +185,9 @@ pub fn get_config_fn(settings: Settings) -> impl FnOnce(&mut ServiceConfig) { .route("/{info_hash}", web::get().to(torrent_get::)) .route( "/", - web::put().to(torrent_put::).wrap( - AuthorizationMiddlewareFactory::new(secret_key.clone(), Role::Verified), - ), + web::put() + .to(torrent_put::) + .wrap(AuthorizationMiddlewareFactory::new(Role::Verified)), ) .route("/rating", web::post().to(rating_create::)) .route( @@ -203,15 +200,15 @@ pub fn get_config_fn(settings: Settings) -> impl FnOnce(&mut ServiceConfig) { ) .route( "/{info_hash}", - web::patch().to(torrent_patch::).wrap( - AuthorizationMiddlewareFactory::new(secret_key.clone(), Role::Mod), - ), + web::patch() + .to(torrent_patch::) + .wrap(AuthorizationMiddlewareFactory::new(Role::Mod)), ) .route( "/{info_hash}", web::delete() .to(torrent_delete::) - .wrap(AuthorizationMiddlewareFactory::new(secret_key, Role::Mod)), + .wrap(AuthorizationMiddlewareFactory::new(Role::Mod)), ) .route( "/{info_hash}/swarm",