Allow users to fetch own role(s) (#473)

Author: trueleo

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,
-        })
-    }
-}

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<bool, Error>,
 }
 
 impl<S, B> Transform<S, ServiceRequest> 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<S> {
     action: Action,
-    match_stream: bool,
+    auth_method: fn(&mut ServiceRequest, Action) -> Result<bool, Error>,
     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::<BasicAuth>().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<bool, Error> = creds.map(|(username, password)| {
-            Users.authenticate(username, password, self.action, stream)
-        });
-
+        let auth_result: Result<bool, Error> = (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<bool, Error> {
+    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<bool, Error> {
+    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<bool, Error> {
+    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::<BasicAuth>().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

server/src/handlers/http/rbac.rs

@@ -51,9 +51,6 @@ pub async fn put_user(
 ) -> Result<impl Responder, RBACError> {
     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<String>) -> Result<impl Responder, RBA
 // Handler for DELETE /api/v1/user/delete/{username}
 pub async fn delete_user(username: web::Path<String>) -> Result<impl Responder, RBACError> {
     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<serde_json::Value>,
 ) -> Result<String, RBACError> {
     let username = username.into_inner();
-    if username == CONFIG.parseable.username {
-        return Err(RBACError::BadUser);
-    }
     let role = role.into_inner();
     let role: HashSet<DefaultPrivilege> = 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,

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");
             }
         }

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<bool> {
         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
+                    }
                 }
             })
         })

server/src/rbac/role.rs

@@ -44,6 +44,7 @@ pub enum Permission {
     Unit(Action),
     Stream(Action, String),
     StreamWithTag(Action, String, Option<String>),
+    SelfRole,
 }
 
 // Currently Roles are tied to one stream
@@ -94,6 +95,7 @@ impl RoleBuilder {
             };
             perms.push(perm);
         }
+        perms.push(Permission::SelfRole);
         perms
     }
 }