Skip to content

Commit 45cfe77

Browse files
authored
Refactor auth flow (#425)
Argon2 is CPU intensive and yet highly recommended for Password hashing. Before this PR, each API call required a hashing to validate the basic auth header. With this PR, we keep a copy of password in memory (only), lazily loaded as users make auth call. This allows much faster authentication. Other changes in this PR: - Authentication and Authorisation are merged into single middleware. - Added AuthMap for modular permission lookup Part of #250
1 parent c55be20 commit 45cfe77

File tree

7 files changed

+221
-190
lines changed

7 files changed

+221
-190
lines changed

server/src/handlers/http.rs

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,16 @@ use std::fs::File;
2020
use std::io::BufReader;
2121

2222
use actix_cors::Cors;
23-
use actix_web::dev::ServiceRequest;
24-
use actix_web::{web, App, HttpMessage, HttpServer, Route};
25-
use actix_web_httpauth::extractors::basic::BasicAuth;
26-
use actix_web_httpauth::middleware::HttpAuthentication;
23+
use actix_web::{web, App, HttpServer, Route};
2724
use actix_web_prometheus::PrometheusMetrics;
2825
use actix_web_static_files::ResourceFiles;
2926
use rustls::{Certificate, PrivateKey, ServerConfig};
3027
use rustls_pemfile::{certs, pkcs8_private_keys};
3128

3229
use crate::option::CONFIG;
3330
use crate::rbac::role::Action;
34-
use crate::rbac::Users;
3531

36-
use self::middleware::{Authorization, DisAllowRootUser};
32+
use self::middleware::{Auth, DisAllowRootUser};
3733

3834
mod health_check;
3935
mod ingest;
@@ -65,21 +61,6 @@ macro_rules! create_app {
6561
};
6662
}
6763

68-
async fn authenticate(
69-
req: ServiceRequest,
70-
credentials: BasicAuth,
71-
) -> Result<ServiceRequest, (actix_web::Error, ServiceRequest)> {
72-
let username = credentials.user_id().trim().to_owned();
73-
let password = credentials.password().unwrap().trim();
74-
75-
if Users.authenticate(&username, password) {
76-
req.extensions_mut().insert(username);
77-
Ok(req)
78-
} else {
79-
Err((actix_web::error::ErrorUnauthorized("Unauthorized"), req))
80-
}
81-
}
82-
8364
pub async fn run_http(prometheus: PrometheusMetrics) -> anyhow::Result<()> {
8465
let ssl_acceptor = match (
8566
&CONFIG.parseable.tls_cert_path,
@@ -224,7 +205,8 @@ pub fn configure_routes(cfg: &mut web::ServiceConfig) {
224205
.service(
225206
web::resource("/{username}/role")
226207
// PUT /user/{username}/roles => Put roles for user
227-
.route(web::put().to(rbac::put_role).authorize(Action::PutRoles)),
208+
.route(web::put().to(rbac::put_role).authorize(Action::PutRoles))
209+
.route(web::get().to(rbac::get_role).authorize(Action::GetRole)),
228210
)
229211
// Deny request if username is same as the env variable P_USERNAME.
230212
.wrap(DisAllowRootUser);
@@ -266,8 +248,7 @@ pub fn configure_routes(cfg: &mut web::ServiceConfig) {
266248
logstream_api,
267249
),
268250
)
269-
.service(user_api)
270-
.wrap(HttpAuthentication::basic(authenticate)),
251+
.service(user_api),
271252
)
272253
// GET "/" ==> Serve the static frontend directory
273254
.service(ResourceFiles::new("/", generated));
@@ -288,14 +269,14 @@ trait RouteExt {
288269

289270
impl RouteExt for Route {
290271
fn authorize(self, action: Action) -> Self {
291-
self.wrap(Authorization {
272+
self.wrap(Auth {
292273
action,
293274
stream: false,
294275
})
295276
}
296277

297278
fn authorize_for_stream(self, action: Action) -> Self {
298-
self.wrap(Authorization {
279+
self.wrap(Auth {
299280
action,
300281
stream: true,
301282
})

server/src/handlers/http/middleware.rs

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,21 +22,22 @@ use std::future::{ready, Ready};
2222
use actix_web::{
2323
dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform},
2424
error::{ErrorBadRequest, ErrorUnauthorized},
25-
Error, HttpMessage,
25+
Error,
2626
};
27+
use actix_web_httpauth::extractors::basic::BasicAuth;
2728
use futures_util::future::LocalBoxFuture;
2829

2930
use crate::{
3031
option::CONFIG,
3132
rbac::{role::Action, Users},
3233
};
3334

34-
pub struct Authorization {
35+
pub struct Auth {
3536
pub action: Action,
3637
pub stream: bool,
3738
}
3839

39-
impl<S, B> Transform<S, ServiceRequest> for Authorization
40+
impl<S, B> Transform<S, ServiceRequest> for Auth
4041
where
4142
S: Service<ServiceRequest, Response = ServiceResponse<B>, Error = Error>,
4243
S::Future: 'static,
@@ -45,25 +46,25 @@ where
4546
type Response = ServiceResponse<B>;
4647
type Error = Error;
4748
type InitError = ();
48-
type Transform = AuthorizationMiddleware<S>;
49+
type Transform = AuthMiddleware<S>;
4950
type Future = Ready<Result<Self::Transform, Self::InitError>>;
5051

5152
fn new_transform(&self, service: S) -> Self::Future {
52-
ready(Ok(AuthorizationMiddleware {
53+
ready(Ok(AuthMiddleware {
5354
action: self.action,
5455
match_stream: self.stream,
5556
service,
5657
}))
5758
}
5859
}
5960

60-
pub struct AuthorizationMiddleware<S> {
61+
pub struct AuthMiddleware<S> {
6162
action: Action,
6263
match_stream: bool,
6364
service: S,
6465
}
6566

66-
impl<S, B> Service<ServiceRequest> for AuthorizationMiddleware<S>
67+
impl<S, B> Service<ServiceRequest> for AuthMiddleware<S>
6768
where
6869
S: Service<ServiceRequest, Response = ServiceResponse<B>, Error = Error>,
6970
S::Future: 'static,
@@ -75,25 +76,34 @@ where
7576

7677
forward_ready!(service);
7778

78-
fn call(&self, req: ServiceRequest) -> Self::Future {
79+
fn call(&self, mut req: ServiceRequest) -> Self::Future {
80+
// Extract username and password using basic auth extractor.
81+
let creds = req.extract::<BasicAuth>().into_inner();
82+
let creds = creds.map_err(Into::into).map(|creds| {
83+
let username = creds.user_id().trim().to_owned();
84+
// password is not mandatory by basic auth standard.
85+
// If not provided then treat as empty string
86+
let password = creds.password().unwrap_or("").trim().to_owned();
87+
(username, password)
88+
});
89+
7990
let stream = if self.match_stream {
8091
req.match_info().get("logstream")
8192
} else {
8293
None
8394
};
84-
let extensions = req.extensions();
85-
let username = extensions
86-
.get::<String>()
87-
.expect("authentication layer verified username");
88-
let is_auth = Users.check_permission(username, self.action, stream);
89-
drop(extensions);
95+
96+
let auth_result: Result<bool, Error> = creds.map(|(username, password)| {
97+
Users.authenticate(username, password, self.action, stream)
98+
});
9099

91100
let fut = self.service.call(req);
92101

93102
Box::pin(async move {
94-
if !is_auth {
103+
if !auth_result? {
95104
return Err(ErrorUnauthorized("Not authorized"));
96105
}
106+
97107
fut.await
98108
})
99109
}

server/src/handlers/http/rbac.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,16 @@ pub async fn put_user(username: web::Path<String>) -> Result<impl Responder, RBA
6666
}
6767
}
6868

69+
// Handler for GET /api/v1/user/{username}/role
70+
// returns role for a user if that user exists
71+
pub async fn get_role(username: web::Path<String>) -> Result<impl Responder, RBACError> {
72+
if !Users.contains(&username) {
73+
return Err(RBACError::UserDoesNotExist);
74+
};
75+
76+
Ok(web::Json(Users.get_role(&username)))
77+
}
78+
6979
// Handler for DELETE /api/v1/user/delete/{username}
7080
pub async fn delete_user(username: web::Path<String>) -> Result<impl Responder, RBACError> {
7181
let username = username.into_inner();
@@ -116,7 +126,6 @@ pub async fn put_role(
116126
let role = role.into_inner();
117127
let role: Vec<DefaultPrivilege> = serde_json::from_value(role)?;
118128

119-
let permissions;
120129
if !Users.contains(&username) {
121130
return Err(RBACError::UserDoesNotExist);
122131
};
@@ -127,16 +136,15 @@ pub async fn put_role(
127136
.iter_mut()
128137
.find(|user| user.username == username)
129138
{
130-
user.role = role;
131-
permissions = user.permissions()
139+
user.role.clone_from(&role);
132140
} else {
133141
// should be unreachable given state is always consistent
134142
return Err(RBACError::UserDoesNotExist);
135143
}
136144

137145
put_metadata(&metadata).await?;
138146
// update in mem table
139-
Users.put_permissions(&username, &permissions);
147+
Users.put_role(&username, role);
140148
Ok(format!("Roles updated successfully for {}", username))
141149
}
142150

0 commit comments

Comments
 (0)