Skip to content

Commit e980283

Browse files
nikhilsinhaparseablenikhilsinhaparseable
authored
fix: use userid, username and method in a usergroup (#1422)
new structure for POST /usergroup ``` { "name": "parseable", "roles": [], "users": [ { "username": "nikhil", "method": "native" } ] } ``` new structure for add/remove user - PATCH /usergroup/{name}/add(remove) ``` { "users": [ { "username": "authentik Default Admin", "method": "oauth" } ] } ``` * code rabbit suggestions * shared lock among all rbac operations * update username to userid * update logic for user existence * fmt change * clippy changes * code rabbit suggestions * refactor * add guard before update * remove lock as lock held in caller * change string response to json with status 200 * status code in response * update method in sync with ingestors * rename to userid, update comment
1 parent a273e4f commit e980283

File tree

15 files changed

+402
-252
lines changed

15 files changed

+402
-252
lines changed

‎src/handlers/http/cluster/mod.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ pub async fn get_demo_data_from_ingestor(action: &str) -> Result<(), PostError>
222222

223223
// forward the role update request to all ingestors to keep them in sync
224224
pub async fn sync_users_with_roles_with_ingestors(
225-
username: &str,
225+
userid: &str,
226226
role: &HashSet<String>,
227227
operation: &str,
228228
) -> Result<(), RBACError> {
@@ -236,7 +236,7 @@ pub async fn sync_users_with_roles_with_ingestors(
236236
RBACError::SerdeError(err)
237237
})?;
238238

239-
let username = username.to_owned();
239+
let userid = userid.to_owned();
240240

241241
let op = operation.to_string();
242242

@@ -245,15 +245,15 @@ pub async fn sync_users_with_roles_with_ingestors(
245245
"{}{}/user/{}/role/sync/{}",
246246
ingestor.domain_name,
247247
base_path_without_preceding_slash(),
248-
username,
248+
userid,
249249
op
250250
);
251251

252252
let role_data = role_data.clone();
253253

254254
async move {
255255
let res = INTRA_CLUSTER_CLIENT
256-
.put(url)
256+
.patch(url)
257257
.header(header::AUTHORIZATION, &ingestor.token)
258258
.header(header::CONTENT_TYPE, "application/json")
259259
.body(role_data)
@@ -282,15 +282,15 @@ pub async fn sync_users_with_roles_with_ingestors(
282282
}
283283

284284
// forward the delete user request to all ingestors to keep them in sync
285-
pub async fn sync_user_deletion_with_ingestors(username: &str) -> Result<(), RBACError> {
286-
let username = username.to_owned();
285+
pub async fn sync_user_deletion_with_ingestors(userid: &str) -> Result<(), RBACError> {
286+
let userid = userid.to_owned();
287287

288288
for_each_live_ingestor(move |ingestor| {
289289
let url = format!(
290290
"{}{}/user/{}/sync",
291291
ingestor.domain_name,
292292
base_path_without_preceding_slash(),
293-
username
293+
userid
294294
);
295295

296296
async move {
@@ -331,21 +331,21 @@ pub async fn sync_user_creation_with_ingestors(
331331
if let Some(role) = role {
332332
user.roles.clone_from(role);
333333
}
334-
let username = user.username();
334+
let userid = user.userid();
335335

336336
let user_data = to_vec(&user).map_err(|err| {
337337
error!("Fatal: failed to serialize user: {:?}", err);
338338
RBACError::SerdeError(err)
339339
})?;
340340

341-
let username = username.to_string();
341+
let userid = userid.to_string();
342342

343343
for_each_live_ingestor(move |ingestor| {
344344
let url = format!(
345345
"{}{}/user/{}/sync",
346346
ingestor.domain_name,
347347
base_path_without_preceding_slash(),
348-
username
348+
userid
349349
);
350350

351351
let user_data = user_data.clone();

‎src/handlers/http/modal/ingest/ingestor_rbac.rs‎

Lines changed: 37 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,14 @@
1818

1919
use std::collections::HashSet;
2020

21-
use actix_web::{Responder, web};
22-
use tokio::sync::Mutex;
21+
use actix_web::{HttpResponse, web};
22+
use http::StatusCode;
2323

2424
use crate::{
25-
handlers::http::{modal::utils::rbac_utils::get_metadata, rbac::RBACError},
25+
handlers::http::{
26+
modal::utils::rbac_utils::get_metadata,
27+
rbac::{RBACError, UPDATE_LOCK},
28+
},
2629
rbac::{
2730
Users,
2831
map::roles,
@@ -31,18 +34,14 @@ use crate::{
3134
storage,
3235
};
3336

34-
// async aware lock for updating storage metadata and user map atomicically
35-
static UPDATE_LOCK: Mutex<()> = Mutex::const_new(());
36-
3737
// Handler for POST /api/v1/user/{username}
3838
// Creates a new user by username if it does not exists
3939
pub async fn post_user(
4040
username: web::Path<String>,
4141
body: Option<web::Json<serde_json::Value>>,
42-
) -> Result<impl Responder, RBACError> {
42+
) -> Result<HttpResponse, RBACError> {
4343
let username = username.into_inner();
4444

45-
let generated_password = String::default();
4645
let metadata = get_metadata().await?;
4746
if let Some(body) = body {
4847
let user: ParseableUser = serde_json::from_value(body.into_inner())?;
@@ -52,37 +51,38 @@ pub async fn post_user(
5251
Users.add_roles(&username, created_role.clone());
5352
}
5453

55-
Ok(generated_password)
54+
Ok(HttpResponse::Ok().status(StatusCode::OK).finish())
5655
}
5756

58-
// Handler for DELETE /api/v1/user/delete/{username}
59-
pub async fn delete_user(username: web::Path<String>) -> Result<impl Responder, RBACError> {
60-
let username = username.into_inner();
61-
let _ = UPDATE_LOCK.lock().await;
57+
// Handler for DELETE /api/v1/user/delete/{userid}
58+
pub async fn delete_user(userid: web::Path<String>) -> Result<HttpResponse, RBACError> {
59+
let userid = userid.into_inner();
60+
let _guard = UPDATE_LOCK.lock().await;
6261
// fail this request if the user does not exists
63-
if !Users.contains(&username) {
62+
if !Users.contains(&userid) {
6463
return Err(RBACError::UserDoesNotExist);
6564
};
65+
6666
// delete from parseable.json first
6767
let mut metadata = get_metadata().await?;
68-
metadata.users.retain(|user| user.username() != username);
68+
metadata.users.retain(|user| user.userid() != userid);
6969

7070
let _ = storage::put_staging_metadata(&metadata);
7171

7272
// update in mem table
73-
Users.delete_user(&username);
74-
Ok(format!("deleted user: {username}"))
73+
Users.delete_user(&userid);
74+
Ok(HttpResponse::Ok().status(StatusCode::OK).finish())
7575
}
7676

77-
// Handler PATCH /user/{username}/role/sync/add => Add roles to a user
77+
// Handler PATCH /user/{userid}/role/sync/add => Add roles to a user
7878
pub async fn add_roles_to_user(
79-
username: web::Path<String>,
79+
userid: web::Path<String>,
8080
roles_to_add: web::Json<HashSet<String>>,
81-
) -> Result<String, RBACError> {
82-
let username = username.into_inner();
81+
) -> Result<HttpResponse, RBACError> {
82+
let userid = userid.into_inner();
8383
let roles_to_add = roles_to_add.into_inner();
8484

85-
if !Users.contains(&username) {
85+
if !Users.contains(&userid) {
8686
return Err(RBACError::UserDoesNotExist);
8787
};
8888

@@ -103,7 +103,7 @@ pub async fn add_roles_to_user(
103103
if let Some(user) = metadata
104104
.users
105105
.iter_mut()
106-
.find(|user| user.username() == username)
106+
.find(|user| user.userid() == userid)
107107
{
108108
user.roles.extend(roles_to_add.clone());
109109
} else {
@@ -113,20 +113,19 @@ pub async fn add_roles_to_user(
113113

114114
let _ = storage::put_staging_metadata(&metadata);
115115
// update in mem table
116-
Users.add_roles(&username.clone(), roles_to_add.clone());
117-
118-
Ok(format!("Roles updated successfully for {username}"))
116+
Users.add_roles(&userid.clone(), roles_to_add.clone());
117+
Ok(HttpResponse::Ok().status(StatusCode::OK).finish())
119118
}
120119

121-
// Handler PATCH /user/{username}/role/sync/add => Add roles to a user
120+
// Handler PATCH /user/{userid}/role/sync/remove => Remove roles to a user
122121
pub async fn remove_roles_from_user(
123-
username: web::Path<String>,
122+
userid: web::Path<String>,
124123
roles_to_remove: web::Json<HashSet<String>>,
125-
) -> Result<String, RBACError> {
126-
let username = username.into_inner();
124+
) -> Result<HttpResponse, RBACError> {
125+
let userid = userid.into_inner();
127126
let roles_to_remove = roles_to_remove.into_inner();
128127

129-
if !Users.contains(&username) {
128+
if !Users.contains(&userid) {
130129
return Err(RBACError::UserDoesNotExist);
131130
};
132131

@@ -143,7 +142,7 @@ pub async fn remove_roles_from_user(
143142
}
144143

145144
// check that user actually has these roles
146-
let user_roles: HashSet<String> = HashSet::from_iter(Users.get_role(&username));
145+
let user_roles: HashSet<String> = HashSet::from_iter(Users.get_role(&userid));
147146
let roles_not_with_user: HashSet<String> =
148147
HashSet::from_iter(roles_to_remove.difference(&user_roles).cloned());
149148

@@ -153,12 +152,12 @@ pub async fn remove_roles_from_user(
153152
)));
154153
}
155154

156-
// update parseable.json first
155+
// update parseable.json in staging first
157156
let mut metadata = get_metadata().await?;
158157
if let Some(user) = metadata
159158
.users
160159
.iter_mut()
161-
.find(|user| user.username() == username)
160+
.find(|user| user.userid() == userid)
162161
{
163162
let diff: HashSet<String> =
164163
HashSet::from_iter(user.roles.difference(&roles_to_remove).cloned());
@@ -170,14 +169,14 @@ pub async fn remove_roles_from_user(
170169

171170
let _ = storage::put_staging_metadata(&metadata);
172171
// update in mem table
173-
Users.remove_roles(&username.clone(), roles_to_remove.clone());
172+
Users.remove_roles(&userid.clone(), roles_to_remove.clone());
174173

175-
Ok(format!("Roles updated successfully for {username}"))
174+
Ok(HttpResponse::Ok().status(StatusCode::OK).finish())
176175
}
177176

178177
// Handler for POST /api/v1/user/{username}/generate-new-password
179178
// Resets password for the user to a newly generated one and returns it
180-
pub async fn post_gen_password(username: web::Path<String>) -> Result<impl Responder, RBACError> {
179+
pub async fn post_gen_password(username: web::Path<String>) -> Result<HttpResponse, RBACError> {
181180
let username = username.into_inner();
182181
let mut new_hash = String::default();
183182
let mut metadata = get_metadata().await?;
@@ -197,6 +196,5 @@ pub async fn post_gen_password(username: web::Path<String>) -> Result<impl Respo
197196
return Err(RBACError::UserDoesNotExist);
198197
}
199198
Users.change_password_hash(&username, &new_hash);
200-
201-
Ok("Updated")
199+
Ok(HttpResponse::Ok().status(StatusCode::OK).finish())
202200
}

‎src/handlers/http/modal/ingest/ingestor_role.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,21 +48,21 @@ pub async fn put(
4848
// refresh the sessions of all users using this role
4949
// for this, iterate over all user_groups and users and create a hashset of users
5050
let mut session_refresh_users: HashSet<String> = HashSet::new();
51-
for user_group in read_user_groups().values().cloned() {
51+
for user_group in read_user_groups().values() {
5252
if user_group.roles.contains(&name) {
53-
session_refresh_users.extend(user_group.users);
53+
session_refresh_users.extend(user_group.users.iter().map(|u| u.userid().to_string()));
5454
}
5555
}
5656

5757
// iterate over all users to see if they have this role
58-
for user in users().values().cloned() {
58+
for user in users().values() {
5959
if user.roles.contains(&name) {
60-
session_refresh_users.insert(user.username().to_string());
60+
session_refresh_users.insert(user.userid().to_string());
6161
}
6262
}
6363

64-
for username in session_refresh_users {
65-
mut_sessions().remove_user(&username);
64+
for userid in session_refresh_users {
65+
mut_sessions().remove_user(&userid);
6666
}
6767

6868
Ok(HttpResponse::Ok().finish())

‎src/handlers/http/modal/ingest_server.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -184,13 +184,13 @@ impl IngestServer {
184184
web::scope("/user")
185185
.service(
186186
web::resource("/{username}/sync")
187-
// PUT /user/{username}/sync => Sync creation of a new user
187+
// POST /user/{username}/sync => Sync creation of a new user
188188
.route(
189189
web::post()
190190
.to(ingestor_rbac::post_user)
191191
.authorize(Action::PutUser),
192192
)
193-
// DELETE /user/{username} => Sync deletion of a user
193+
// DELETE /user/{userid} => Sync deletion of a user
194194
.route(
195195
web::delete()
196196
.to(ingestor_rbac::delete_user)
@@ -199,8 +199,8 @@ impl IngestServer {
199199
.wrap(DisAllowRootUser),
200200
)
201201
.service(
202-
web::resource("/{username}/role/sync/add")
203-
// PATCH /user/{username}/role/sync/add => Add roles to a user
202+
web::resource("/{userid}/role/sync/add")
203+
// PATCH /user/{userid}/role/sync/add => Add roles to a user
204204
.route(
205205
web::patch()
206206
.to(ingestor_rbac::add_roles_to_user)
@@ -209,8 +209,8 @@ impl IngestServer {
209209
),
210210
)
211211
.service(
212-
web::resource("/{username}/role/sync/remove")
213-
// PATCH /user/{username}/role/sync/remove => Remove roles from a user
212+
web::resource("/{userid}/role/sync/remove")
213+
// PATCH /user/{userid}/role/sync/remove => Remove roles from a user
214214
.route(
215215
web::patch()
216216
.to(ingestor_rbac::remove_roles_from_user)

0 commit comments

Comments
 (0)