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,
@@ -266,8 +247,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 +268,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,33 @@ where
 
     forward_ready!(service);
 
-    fn call(&self, req: ServiceRequest) -> Self::Future {
+    fn call(&self, mut req: ServiceRequest) -> Self::Future {
+        let creds = req.extract::<BasicAuth>().into_inner();
+        let creds: Result<(String, String), Error> = creds.map_err(Into::into).and_then(|creds| {
+            let username = creds.user_id().trim().to_owned();
+            if let Some(password) = creds.password() {
+                Ok((username, password.trim().to_owned()))
+            } else {
+                Err(ErrorBadRequest("Password is required in basic auth"))
+            }
+        });
         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 is_auth = creds.map(|creds| {
+            let (username, password) = creds;
+            Users.authenticate(username, password, self.action, stream)
+        });
 
         let fut = self.service.call(req);
 
         Box::pin(async move {
-            if !is_auth {
+            if !is_auth? {
                 return Err(ErrorUnauthorized("Not authorized"));
             }
+
             fut.await
         })
     }

server/src/handlers/http/rbac.rs

@@ -116,7 +116,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 +126,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))
 }