Refactor auth flow

server/src/handlers/http.rs

@@ -20,20 +20,16 @@ use std::fs::File;
 use std::io::BufReader;
 
 use actix_cors::Cors;
-use actix_web::dev::ServiceRequest;
-use actix_web::{web, App, HttpMessage, HttpServer, Route};
-use actix_web_httpauth::extractors::basic::BasicAuth;
-use actix_web_httpauth::middleware::HttpAuthentication;
+use actix_web::{web, App, HttpServer, Route};
 use actix_web_prometheus::PrometheusMetrics;
 use actix_web_static_files::ResourceFiles;
 use rustls::{Certificate, PrivateKey, ServerConfig};
 use rustls_pemfile::{certs, pkcs8_private_keys};
 
 use crate::option::CONFIG;
 use crate::rbac::role::Action;
-use crate::rbac::Users;
 
-use self::middleware::{Authorization, DisAllowRootUser};
+use self::middleware::{Auth, DisAllowRootUser};
 
 mod health_check;
 mod ingest;
@@ -65,21 +61,6 @@ macro_rules! create_app {
     };
 }
 
-async fn authenticate(
-    req: ServiceRequest,
-    credentials: BasicAuth,
-) -> Result<ServiceRequest, (actix_web::Error, ServiceRequest)> {
-    let username = credentials.user_id().trim().to_owned();
-    let password = credentials.password().unwrap().trim();
-
-    if Users.authenticate(&username, password) {
-        req.extensions_mut().insert(username);
-        Ok(req)
-    } else {
-        Err((actix_web::error::ErrorUnauthorized("Unauthorized"), req))
-    }
-}
-
 pub async fn run_http(prometheus: PrometheusMetrics) -> anyhow::Result<()> {
     let ssl_acceptor = match (
         &CONFIG.parseable.tls_cert_path,
@@ -224,7 +205,8 @@ pub fn configure_routes(cfg: &mut web::ServiceConfig) {
         .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::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);
@@ -266,8 +248,7 @@ pub fn configure_routes(cfg: &mut web::ServiceConfig) {
                         logstream_api,
                     ),
             )
-            .service(user_api)
-            .wrap(HttpAuthentication::basic(authenticate)),
+            .service(user_api),
     )
     // GET "/" ==> Serve the static frontend directory
     .service(ResourceFiles::new("/", generated));
@@ -288,14 +269,14 @@ trait RouteExt {
 
 impl RouteExt for Route {
     fn authorize(self, action: Action) -> Self {
-        self.wrap(Authorization {
+        self.wrap(Auth {
             action,
             stream: false,
         })
     }
 
     fn authorize_for_stream(self, action: Action) -> Self {
-        self.wrap(Authorization {
+        self.wrap(Auth {
             action,
             stream: true,
         })

server/src/handlers/http/middleware.rs

@@ -22,21 +22,22 @@ use std::future::{ready, Ready};
 use actix_web::{
     dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform},
     error::{ErrorBadRequest, ErrorUnauthorized},
-    Error, HttpMessage,
+    Error,
 };
+use actix_web_httpauth::extractors::basic::BasicAuth;
 use futures_util::future::LocalBoxFuture;
 
 use crate::{
     option::CONFIG,
     rbac::{role::Action, Users},
 };
 
-pub struct Authorization {
+pub struct Auth {
     pub action: Action,
     pub stream: bool,
 }
 
-impl<S, B> Transform<S, ServiceRequest> for Authorization
+impl<S, B> Transform<S, ServiceRequest> for Auth
 where
     S: Service<ServiceRequest, Response = ServiceResponse<B>, Error = Error>,
     S::Future: 'static,
@@ -45,25 +46,25 @@ where
     type Response = ServiceResponse<B>;
     type Error = Error;
     type InitError = ();
-    type Transform = AuthorizationMiddleware<S>;
+    type Transform = AuthMiddleware<S>;
     type Future = Ready<Result<Self::Transform, Self::InitError>>;
 
     fn new_transform(&self, service: S) -> Self::Future {
-        ready(Ok(AuthorizationMiddleware {
+        ready(Ok(AuthMiddleware {
             action: self.action,
             match_stream: self.stream,
             service,
         }))
     }
 }
 
-pub struct AuthorizationMiddleware<S> {
+pub struct AuthMiddleware<S> {
     action: Action,
     match_stream: bool,
     service: S,
 }
 
-impl<S, B> Service<ServiceRequest> for AuthorizationMiddleware<S>
+impl<S, B> Service<ServiceRequest> for AuthMiddleware<S>
 where
     S: Service<ServiceRequest, Response = ServiceResponse<B>, Error = Error>,
     S::Future: 'static,
@@ -75,25 +76,34 @@ where
 
     forward_ready!(service);
 
-    fn call(&self, req: ServiceRequest) -> Self::Future {
+    fn call(&self, mut req: ServiceRequest) -> Self::Future {
+        // Extract username and password 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 extensions = req.extensions();
-        let username = extensions
-            .get::<String>()
-            .expect("authentication layer verified username");
-        let is_auth = Users.check_permission(username, self.action, stream);
-        drop(extensions);
+
+        let auth_result: Result<bool, Error> = creds.map(|(username, password)| {
+            Users.authenticate(username, password, self.action, stream)
+        });
 
         let fut = self.service.call(req);
 
         Box::pin(async move {
-            if !is_auth {
+            if !auth_result? {
                 return Err(ErrorUnauthorized("Not authorized"));
             }
+
             fut.await
         })
     }

server/src/handlers/http/rbac.rs

@@ -66,6 +66,16 @@ pub async fn put_user(username: web::Path<String>) -> Result<impl Responder, RBA
     }
 }
 
+// Handler for GET /api/v1/user/{username}/role
+// returns role for a user if that user exists
+pub async fn get_role(username: web::Path<String>) -> Result<impl Responder, RBACError> {
+    if !Users.contains(&username) {
+        return Err(RBACError::UserDoesNotExist);
+    };
+
+    Ok(web::Json(Users.get_role(&username)))
+}
+
 // 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();
@@ -116,7 +126,6 @@ pub async fn put_role(
     let role = role.into_inner();
     let role: Vec<DefaultPrivilege> = serde_json::from_value(role)?;
 
-    let permissions;
     if !Users.contains(&username) {
         return Err(RBACError::UserDoesNotExist);
     };
@@ -127,16 +136,15 @@ pub async fn put_role(
         .iter_mut()
         .find(|user| user.username == username)
     {
-        user.role = role;
-        permissions = user.permissions()
+        user.role.clone_from(&role);
     } else {
         // should be unreachable given state is always consistent
         return Err(RBACError::UserDoesNotExist);
     }
 
     put_metadata(&metadata).await?;
     // update in mem table
-    Users.put_permissions(&username, &permissions);
+    Users.put_role(&username, role);
     Ok(format!("Roles updated successfully for {}", username))
 }