diff --git a/server/src/handlers/http.rs b/server/src/handlers/http.rs index ab935a2a8..cbb3cba2e 100644 --- a/server/src/handlers/http.rs +++ b/server/src/handlers/http.rs @@ -20,7 +20,7 @@ use std::fs::File; use std::io::BufReader; use actix_cors::Cors; -use actix_web::{web, App, HttpServer, Route}; +use actix_web::{web, App, HttpServer}; use actix_web_prometheus::PrometheusMetrics; use actix_web_static_files::ResourceFiles; use rustls::{Certificate, PrivateKey, ServerConfig}; @@ -29,7 +29,7 @@ use rustls_pemfile::{certs, pkcs8_private_keys}; use crate::option::CONFIG; use crate::rbac::role::Action; -use self::middleware::{Auth, DisAllowRootUser}; +use self::middleware::{DisAllowRootUser, RouteExt}; mod about; mod health_check; @@ -201,17 +201,25 @@ pub fn configure_routes(cfg: &mut web::ServiceConfig) { web::delete() .to(rbac::delete_user) .authorize(Action::DeleteUser), - ), + ) + .wrap(DisAllowRootUser), ) .service( web::resource("/{username}/role") // PUT /user/{username}/roles => Put roles for user - .route(web::put().to(rbac::put_role).authorize(Action::PutRoles)) - .route(web::get().to(rbac::get_role).authorize(Action::GetRole)), - ) - // Deny request if username is same as the env variable P_USERNAME. - .wrap(DisAllowRootUser); - + .route( + web::put() + .to(rbac::put_role) + .authorize(Action::PutRoles) + .wrap(DisAllowRootUser), + ) + .route( + web::get() + .to(rbac::get_role) + .authorize_for_user(Action::GetRole), + ), + ); + // Deny request if username is same as the env variable P_USERNAME. cfg.service( // Base path "{url}/api/v1" web::scope(&base_path()) @@ -261,24 +269,3 @@ fn base_path() -> String { pub fn metrics_path() -> String { format!("{}/metrics", base_path()) } - -trait RouteExt { - fn authorize(self, action: Action) -> Self; - fn authorize_for_stream(self, action: Action) -> Self; -} - -impl RouteExt for Route { - fn authorize(self, action: Action) -> Self { - self.wrap(Auth { - action, - stream: false, - }) - } - - fn authorize_for_stream(self, action: Action) -> Self { - self.wrap(Auth { - action, - stream: true, - }) - } -} diff --git a/server/src/handlers/http/middleware.rs b/server/src/handlers/http/middleware.rs index d65699c89..fa1d2b1e1 100644 --- a/server/src/handlers/http/middleware.rs +++ b/server/src/handlers/http/middleware.rs @@ -22,16 +22,46 @@ use std::future::{ready, Ready}; use actix_web::{ dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform}, error::{ErrorBadRequest, ErrorUnauthorized}, - Error, + Error, Route, }; use actix_web_httpauth::extractors::basic::BasicAuth; use futures_util::future::LocalBoxFuture; use crate::{option::CONFIG, rbac::role::Action, rbac::Users}; +pub trait RouteExt { + fn authorize(self, action: Action) -> Self; + fn authorize_for_stream(self, action: Action) -> Self; + fn authorize_for_user(self, action: Action) -> Self; +} + +impl RouteExt for Route { + fn authorize(self, action: Action) -> Self { + self.wrap(Auth { + action, + method: auth_no_context, + }) + } + + fn authorize_for_stream(self, action: Action) -> Self { + self.wrap(Auth { + action, + method: auth_stream_context, + }) + } + + fn authorize_for_user(self, action: Action) -> Self { + self.wrap(Auth { + action, + method: auth_user_context, + }) + } +} + +// Authentication Layer with no context pub struct Auth { pub action: Action, - pub stream: bool, + pub method: fn(&mut ServiceRequest, Action) -> Result, } impl Transform for Auth @@ -49,15 +79,15 @@ where fn new_transform(&self, service: S) -> Self::Future { ready(Ok(AuthMiddleware { action: self.action, - match_stream: self.stream, service, + auth_method: self.method, })) } } pub struct AuthMiddleware { action: Action, - match_stream: bool, + auth_method: fn(&mut ServiceRequest, Action) -> Result, service: S, } @@ -74,38 +104,46 @@ where forward_ready!(service); fn call(&self, mut req: ServiceRequest) -> Self::Future { - // Extract username and password from the request using basic auth extractor. - let creds = req.extract::().into_inner(); - let creds = creds.map_err(Into::into).map(|creds| { - let username = creds.user_id().trim().to_owned(); - // password is not mandatory by basic auth standard. - // If not provided then treat as empty string - let password = creds.password().unwrap_or("").trim().to_owned(); - (username, password) - }); - - let stream = if self.match_stream { - req.match_info().get("logstream") - } else { - None - }; - - let auth_result: Result = creds.map(|(username, password)| { - Users.authenticate(username, password, self.action, stream) - }); - + let auth_result: Result = (self.auth_method)(&mut req, self.action); let fut = self.service.call(req); - Box::pin(async move { if !auth_result? { return Err(ErrorUnauthorized("Not authorized")); } - fut.await }) } } +pub fn auth_no_context(req: &mut ServiceRequest, action: Action) -> Result { + let creds = extract_basic_auth(req); + creds.map(|(username, password)| Users.authenticate(username, password, action, None, None)) +} + +pub fn auth_stream_context(req: &mut ServiceRequest, action: Action) -> Result { + let creds = extract_basic_auth(req); + let stream = req.match_info().get("logstream"); + creds.map(|(username, password)| Users.authenticate(username, password, action, stream, None)) +} + +pub fn auth_user_context(req: &mut ServiceRequest, action: Action) -> Result { + let creds = extract_basic_auth(req); + let user = req.match_info().get("username"); + creds.map(|(username, password)| Users.authenticate(username, password, action, None, user)) +} + +fn extract_basic_auth(req: &mut ServiceRequest) -> Result<(String, String), Error> { + // Extract username and password from the request using basic auth extractor. + let creds = req.extract::().into_inner(); + creds.map_err(Into::into).map(|creds| { + let username = creds.user_id().trim().to_owned(); + // password is not mandatory by basic auth standard. + // If not provided then treat as empty string + let password = creds.password().unwrap_or("").trim().to_owned(); + (username, password) + }) +} + // The credentials set in the env vars (P_USERNAME & P_PASSWORD) are treated // as root credentials. Any other user is not allowed to modify or delete // the root user. Deny request if username is same as username diff --git a/server/src/handlers/http/rbac.rs b/server/src/handlers/http/rbac.rs index 5bc1c4015..576c917b4 100644 --- a/server/src/handlers/http/rbac.rs +++ b/server/src/handlers/http/rbac.rs @@ -51,9 +51,6 @@ pub async fn put_user( ) -> Result { let username = username.into_inner(); validator::user_name(&username)?; - if username == CONFIG.parseable.username { - return Err(RBACError::BadUser); - } let _ = UPDATE_LOCK.lock().await; if Users.contains(&username) { reset_password(username).await @@ -90,9 +87,6 @@ pub async fn get_role(username: web::Path) -> Result) -> Result { let username = username.into_inner(); - if username == CONFIG.parseable.username { - return Err(RBACError::BadUser); - } let _ = UPDATE_LOCK.lock().await; // fail this request if the user does not exists if !Users.contains(&username) { @@ -137,9 +131,6 @@ pub async fn put_role( role: web::Json, ) -> Result { let username = username.into_inner(); - if username == CONFIG.parseable.username { - return Err(RBACError::BadUser); - } let role = role.into_inner(); let role: HashSet = serde_json::from_value(role)?; let role = role.into_iter().collect(); @@ -184,8 +175,6 @@ async fn put_metadata(metadata: &StorageMetadata) -> Result<(), ObjectStorageErr #[derive(Debug, thiserror::Error)] pub enum RBACError { - #[error("Request cannot be allowed for this user")] - BadUser, #[error("User exists already")] UserExists, #[error("User does not exist")] @@ -201,7 +190,6 @@ pub enum RBACError { impl actix_web::ResponseError for RBACError { fn status_code(&self) -> http::StatusCode { match self { - Self::BadUser => StatusCode::BAD_REQUEST, Self::UserExists => StatusCode::BAD_REQUEST, Self::UserDoesNotExist => StatusCode::NOT_FOUND, Self::SerdeError(_) => StatusCode::BAD_REQUEST, diff --git a/server/src/rbac.rs b/server/src/rbac.rs index 60517e4e2..05f179299 100644 --- a/server/src/rbac.rs +++ b/server/src/rbac.rs @@ -80,11 +80,12 @@ impl Users { username: String, password: String, action: Action, - stream: Option<&str>, + context_stream: Option<&str>, + context_user: Option<&str>, ) -> bool { let key = (username, password); // try fetch from auth map for faster auth flow - if let Some(res) = auth_map().check_auth(&key, action, stream) { + if let Some(res) = auth_map().check_auth(&key, action, context_stream, context_user) { return res; } @@ -99,7 +100,7 @@ impl Users { // verify from auth map and return let key = (username, password); return auth_map - .check_auth(&key, action, stream) + .check_auth(&key, action, context_stream, context_user) .expect("entry for this key just added"); } } diff --git a/server/src/rbac/map.rs b/server/src/rbac/map.rs index 0c8e0263e..d309a63be 100644 --- a/server/src/rbac/map.rs +++ b/server/src/rbac/map.rs @@ -117,7 +117,8 @@ impl AuthMap { &self, key: &(String, String), required_action: Action, - on_stream: Option<&str>, + context_stream: Option<&str>, + context_user: Option<&str>, ) -> Option { self.inner.get(key).map(|perms| { perms.iter().any(|user_perm| { @@ -126,14 +127,18 @@ impl AuthMap { Permission::Unit(action) => action == required_action || action == Action::All, Permission::Stream(action, ref stream) | Permission::StreamWithTag(action, ref stream, _) => { - let ok_stream = if let Some(on_stream) = on_stream { - stream == on_stream || stream == "*" + let ok_stream = if let Some(context_stream) = context_stream { + stream == context_stream || stream == "*" } else { // if no stream to match then stream check is not needed true }; (action == required_action || action == Action::All) && ok_stream } + Permission::SelfRole => { + context_user.map(|x| x == key.0).unwrap_or_default() + && required_action == Action::GetRole + } } }) }) diff --git a/server/src/rbac/role.rs b/server/src/rbac/role.rs index 7f03e040a..786df8eeb 100644 --- a/server/src/rbac/role.rs +++ b/server/src/rbac/role.rs @@ -44,6 +44,7 @@ pub enum Permission { Unit(Action), Stream(Action, String), StreamWithTag(Action, String, Option), + SelfRole, } // Currently Roles are tied to one stream @@ -94,6 +95,7 @@ impl RoleBuilder { }; perms.push(perm); } + perms.push(Permission::SelfRole); perms } }