-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add partial role support for manager only using web-vault v2024.12.0 #5219
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -48,6 +48,7 @@ pub fn routes() -> Vec<Route> { | |||||||||||||||||||||||||||||||||||||||||
confirm_invite, | ||||||||||||||||||||||||||||||||||||||||||
bulk_confirm_invite, | ||||||||||||||||||||||||||||||||||||||||||
accept_invite, | ||||||||||||||||||||||||||||||||||||||||||
get_org_user_mini_details, | ||||||||||||||||||||||||||||||||||||||||||
get_user, | ||||||||||||||||||||||||||||||||||||||||||
edit_user, | ||||||||||||||||||||||||||||||||||||||||||
put_organization_user, | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -77,6 +78,7 @@ pub fn routes() -> Vec<Route> { | |||||||||||||||||||||||||||||||||||||||||
restore_organization_user, | ||||||||||||||||||||||||||||||||||||||||||
bulk_restore_organization_user, | ||||||||||||||||||||||||||||||||||||||||||
get_groups, | ||||||||||||||||||||||||||||||||||||||||||
get_groups_details, | ||||||||||||||||||||||||||||||||||||||||||
post_groups, | ||||||||||||||||||||||||||||||||||||||||||
get_group, | ||||||||||||||||||||||||||||||||||||||||||
put_group, | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -98,6 +100,7 @@ pub fn routes() -> Vec<Route> { | |||||||||||||||||||||||||||||||||||||||||
get_org_export, | ||||||||||||||||||||||||||||||||||||||||||
api_key, | ||||||||||||||||||||||||||||||||||||||||||
rotate_api_key, | ||||||||||||||||||||||||||||||||||||||||||
get_billing_metadata, | ||||||||||||||||||||||||||||||||||||||||||
] | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
@@ -323,6 +326,13 @@ async fn get_org_collections_details(org_id: &str, headers: ManagerHeadersLoose, | |||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// get all collection memberships for the current organization | ||||||||||||||||||||||||||||||||||||||||||
let coll_users = CollectionUser::find_by_organization(org_id, &mut conn).await; | ||||||||||||||||||||||||||||||||||||||||||
// Generate a HashMap to get the correct UserOrgType per user to determine the manage permission | ||||||||||||||||||||||||||||||||||||||||||
// We use the uuid instead of the user_uuid here, since that is what is used in CollectionUser | ||||||||||||||||||||||||||||||||||||||||||
let users_org_type: HashMap<String, i32> = UserOrganization::find_confirmed_by_org(org_id, &mut conn) | ||||||||||||||||||||||||||||||||||||||||||
.await | ||||||||||||||||||||||||||||||||||||||||||
.into_iter() | ||||||||||||||||||||||||||||||||||||||||||
.map(|uo| (uo.uuid, uo.atype)) | ||||||||||||||||||||||||||||||||||||||||||
.collect(); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// check if current user has full access to the organization (either directly or via any group) | ||||||||||||||||||||||||||||||||||||||||||
let has_full_access_to_org = user_org.access_all | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -336,11 +346,22 @@ async fn get_org_collections_details(org_id: &str, headers: ManagerHeadersLoose, | |||||||||||||||||||||||||||||||||||||||||
|| (CONFIG.org_groups_enabled() | ||||||||||||||||||||||||||||||||||||||||||
&& GroupUser::has_access_to_collection_by_member(&col.uuid, &user_org.uuid, &mut conn).await); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// Not assigned collections should not be returned | ||||||||||||||||||||||||||||||||||||||||||
if !assigned { | ||||||||||||||||||||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// get the users assigned directly to the given collection | ||||||||||||||||||||||||||||||||||||||||||
let users: Vec<Value> = coll_users | ||||||||||||||||||||||||||||||||||||||||||
.iter() | ||||||||||||||||||||||||||||||||||||||||||
.filter(|collection_user| collection_user.collection_uuid == col.uuid) | ||||||||||||||||||||||||||||||||||||||||||
.map(|collection_user| SelectionReadOnly::to_collection_user_details_read_only(collection_user).to_json()) | ||||||||||||||||||||||||||||||||||||||||||
.map(|collection_user| { | ||||||||||||||||||||||||||||||||||||||||||
SelectionReadOnly::to_collection_user_details_read_only( | ||||||||||||||||||||||||||||||||||||||||||
collection_user, | ||||||||||||||||||||||||||||||||||||||||||
*users_org_type.get(&collection_user.user_uuid).unwrap_or(&(UserOrgType::User as i32)), | ||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here you are passing the vaultwarden/src/db/models/collection.rs Lines 20 to 21 in ed4ad67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay I looked further into it and I think part of what makes it tricky is that we have two functions that retrieve a vaultwarden/src/db/models/collection.rs Lines 514 to 521 in ed4ad67
The second one at least is named accordingly vaultwarden/src/db/models/collection.rs Lines 610 to 619 in ed4ad67
|
||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||
.to_json() | ||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||
.collect(); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// get the group details for the given collection | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -645,12 +666,24 @@ async fn get_org_collection_detail( | |||||||||||||||||||||||||||||||||||||||||
Vec::with_capacity(0) | ||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// Generate a HashMap to get the correct UserOrgType per user to determine the manage permission | ||||||||||||||||||||||||||||||||||||||||||
// We use the uuid instead of the user_uuid here, since that is what is used in CollectionUser | ||||||||||||||||||||||||||||||||||||||||||
let users_org_type: HashMap<String, i32> = UserOrganization::find_confirmed_by_org(org_id, &mut conn) | ||||||||||||||||||||||||||||||||||||||||||
.await | ||||||||||||||||||||||||||||||||||||||||||
.into_iter() | ||||||||||||||||||||||||||||||||||||||||||
.map(|uo| (uo.uuid, uo.atype)) | ||||||||||||||||||||||||||||||||||||||||||
.collect(); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
let users: Vec<Value> = | ||||||||||||||||||||||||||||||||||||||||||
CollectionUser::find_by_collection_swap_user_uuid_with_org_user_uuid(&collection.uuid, &mut conn) | ||||||||||||||||||||||||||||||||||||||||||
.await | ||||||||||||||||||||||||||||||||||||||||||
.iter() | ||||||||||||||||||||||||||||||||||||||||||
.map(|collection_user| { | ||||||||||||||||||||||||||||||||||||||||||
SelectionReadOnly::to_collection_user_details_read_only(collection_user).to_json() | ||||||||||||||||||||||||||||||||||||||||||
SelectionReadOnly::to_collection_user_details_read_only( | ||||||||||||||||||||||||||||||||||||||||||
collection_user, | ||||||||||||||||||||||||||||||||||||||||||
*users_org_type.get(&collection_user.user_uuid).unwrap_or(&(UserOrgType::User as i32)), | ||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||
.to_json() | ||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||
.collect(); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
@@ -830,13 +863,19 @@ struct InviteData { | |||||||||||||||||||||||||||||||||||||||||
collections: Option<Vec<CollectionData>>, | ||||||||||||||||||||||||||||||||||||||||||
#[serde(default)] | ||||||||||||||||||||||||||||||||||||||||||
access_all: bool, | ||||||||||||||||||||||||||||||||||||||||||
#[serde(default)] | ||||||||||||||||||||||||||||||||||||||||||
permissions: HashMap<String, Value>, | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
#[post("/organizations/<org_id>/users/invite", data = "<data>")] | ||||||||||||||||||||||||||||||||||||||||||
async fn send_invite(org_id: &str, data: Json<InviteData>, headers: AdminHeaders, mut conn: DbConn) -> EmptyResult { | ||||||||||||||||||||||||||||||||||||||||||
let data: InviteData = data.into_inner(); | ||||||||||||||||||||||||||||||||||||||||||
let mut data: InviteData = data.into_inner(); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
let new_type = match UserOrgType::from_str(&data.r#type.into_string()) { | ||||||||||||||||||||||||||||||||||||||||||
// HACK: We need the raw user-type be be sure custom role is selected to determine the access_all permission | ||||||||||||||||||||||||||||||||||||||||||
// The from_str() will convert the custom role type into a manager role type | ||||||||||||||||||||||||||||||||||||||||||
let raw_type = &data.r#type.into_string(); | ||||||||||||||||||||||||||||||||||||||||||
// UserOrgType::from_str will convert custom (4) to manager (3) | ||||||||||||||||||||||||||||||||||||||||||
let new_type = match UserOrgType::from_str(raw_type) { | ||||||||||||||||||||||||||||||||||||||||||
Some(new_type) => new_type as i32, | ||||||||||||||||||||||||||||||||||||||||||
None => err!("Invalid type"), | ||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -845,6 +884,17 @@ async fn send_invite(org_id: &str, data: Json<InviteData>, headers: AdminHeaders | |||||||||||||||||||||||||||||||||||||||||
err!("Only Owners can invite Managers, Admins or Owners") | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// HACK: This converts the Custom role which has the `Manage all collections` box checked into an access_all flag | ||||||||||||||||||||||||||||||||||||||||||
// Since the parent checkbox is not send to the server we need to check and verify the child checkboxes | ||||||||||||||||||||||||||||||||||||||||||
// If the box is not checked, the user will still be a manager, but not with the access_all permission | ||||||||||||||||||||||||||||||||||||||||||
if raw_type.eq("4") | ||||||||||||||||||||||||||||||||||||||||||
&& data.permissions.get("editAnyCollection") == Some(&json!(true)) | ||||||||||||||||||||||||||||||||||||||||||
&& data.permissions.get("deleteAnyCollection") == Some(&json!(true)) | ||||||||||||||||||||||||||||||||||||||||||
&& data.permissions.get("createNewCollections") == Some(&json!(true)) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
data.access_all = true; | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
for email in data.emails.iter() { | ||||||||||||||||||||||||||||||||||||||||||
let mut user_org_status = UserOrgStatus::Invited as i32; | ||||||||||||||||||||||||||||||||||||||||||
let user = match User::find_by_mail(email, &mut conn).await { | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -1254,7 +1304,21 @@ async fn _confirm_invite( | |||||||||||||||||||||||||||||||||||||||||
save_result | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
#[get("/organizations/<org_id>/users/<org_user_id>?<data..>")] | ||||||||||||||||||||||||||||||||||||||||||
#[get("/organizations/<org_id>/users/mini-details", rank = 1)] | ||||||||||||||||||||||||||||||||||||||||||
async fn get_org_user_mini_details(org_id: &str, _headers: ManagerHeadersLoose, mut conn: DbConn) -> Json<Value> { | ||||||||||||||||||||||||||||||||||||||||||
let mut users_json = Vec::new(); | ||||||||||||||||||||||||||||||||||||||||||
for u in UserOrganization::find_by_org(org_id, &mut conn).await { | ||||||||||||||||||||||||||||||||||||||||||
users_json.push(u.to_json_mini_details(&mut conn).await); | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
Json(json!({ | ||||||||||||||||||||||||||||||||||||||||||
"data": users_json, | ||||||||||||||||||||||||||||||||||||||||||
"object": "list", | ||||||||||||||||||||||||||||||||||||||||||
"continuationToken": null, | ||||||||||||||||||||||||||||||||||||||||||
})) | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
#[get("/organizations/<org_id>/users/<org_user_id>?<data..>", rank = 2)] | ||||||||||||||||||||||||||||||||||||||||||
async fn get_user( | ||||||||||||||||||||||||||||||||||||||||||
org_id: &str, | ||||||||||||||||||||||||||||||||||||||||||
org_user_id: &str, | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -1282,6 +1346,8 @@ struct EditUserData { | |||||||||||||||||||||||||||||||||||||||||
groups: Option<Vec<String>>, | ||||||||||||||||||||||||||||||||||||||||||
#[serde(default)] | ||||||||||||||||||||||||||||||||||||||||||
access_all: bool, | ||||||||||||||||||||||||||||||||||||||||||
#[serde(default)] | ||||||||||||||||||||||||||||||||||||||||||
permissions: HashMap<String, Value>, | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
#[put("/organizations/<org_id>/users/<org_user_id>", data = "<data>", rank = 1)] | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -1303,14 +1369,30 @@ async fn edit_user( | |||||||||||||||||||||||||||||||||||||||||
headers: AdminHeaders, | ||||||||||||||||||||||||||||||||||||||||||
mut conn: DbConn, | ||||||||||||||||||||||||||||||||||||||||||
) -> EmptyResult { | ||||||||||||||||||||||||||||||||||||||||||
let data: EditUserData = data.into_inner(); | ||||||||||||||||||||||||||||||||||||||||||
let mut data: EditUserData = data.into_inner(); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
let Some(new_type) = UserOrgType::from_str(&data.r#type.into_string()) else { | ||||||||||||||||||||||||||||||||||||||||||
// HACK: We need the raw user-type be be sure custom role is selected to determine the access_all permission | ||||||||||||||||||||||||||||||||||||||||||
// The from_str() will convert the custom role type into a manager role type | ||||||||||||||||||||||||||||||||||||||||||
let raw_type = &data.r#type.into_string(); | ||||||||||||||||||||||||||||||||||||||||||
// UserOrgType::from_str will convert custom (4) to manager (3) | ||||||||||||||||||||||||||||||||||||||||||
let Some(new_type) = UserOrgType::from_str(raw_type) else { | ||||||||||||||||||||||||||||||||||||||||||
err!("Invalid type") | ||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
let Some(mut user_to_edit) = UserOrganization::find_by_uuid_and_org(org_user_id, org_id, &mut conn).await else { | ||||||||||||||||||||||||||||||||||||||||||
err!("The specified user isn't member of the organization") | ||||||||||||||||||||||||||||||||||||||||||
// HACK: This converts the Custom role which has the `Manage all collections` box checked into an access_all flag | ||||||||||||||||||||||||||||||||||||||||||
// Since the parent checkbox is not send to the server we need to check and verify the child checkboxes | ||||||||||||||||||||||||||||||||||||||||||
// If the box is not checked, the user will still be a manager, but not with the access_all permission | ||||||||||||||||||||||||||||||||||||||||||
if raw_type.eq("4") | ||||||||||||||||||||||||||||||||||||||||||
&& data.permissions.get("editAnyCollection") == Some(&json!(true)) | ||||||||||||||||||||||||||||||||||||||||||
&& data.permissions.get("deleteAnyCollection") == Some(&json!(true)) | ||||||||||||||||||||||||||||||||||||||||||
&& data.permissions.get("createNewCollections") == Some(&json!(true)) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
data.access_all = true; | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
let mut user_to_edit = match UserOrganization::find_by_uuid_and_org(org_user_id, org_id, &mut conn).await { | ||||||||||||||||||||||||||||||||||||||||||
Some(user) => user, | ||||||||||||||||||||||||||||||||||||||||||
None => err!("The specified user isn't member of the organization"), | ||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
if new_type != user_to_edit.atype | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -1901,6 +1983,12 @@ fn get_plans_tax_rates(_headers: Headers) -> Json<Value> { | |||||||||||||||||||||||||||||||||||||||||
Json(_empty_data_json()) | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
#[get("/organizations/<_org_id>/billing/metadata")] | ||||||||||||||||||||||||||||||||||||||||||
fn get_billing_metadata(_org_id: &str, _headers: Headers) -> Json<Value> { | ||||||||||||||||||||||||||||||||||||||||||
// Prevent a 404 error, which also causes Javascript errors. | ||||||||||||||||||||||||||||||||||||||||||
Json(_empty_data_json()) | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
fn _empty_data_json() -> Value { | ||||||||||||||||||||||||||||||||||||||||||
json!({ | ||||||||||||||||||||||||||||||||||||||||||
"object": "list", | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -2299,6 +2387,11 @@ async fn get_groups(org_id: &str, _headers: ManagerHeadersLoose, mut conn: DbCon | |||||||||||||||||||||||||||||||||||||||||
}))) | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
#[get("/organizations/<org_id>/groups/details", rank = 1)] | ||||||||||||||||||||||||||||||||||||||||||
async fn get_groups_details(org_id: &str, headers: ManagerHeadersLoose, conn: DbConn) -> JsonResult { | ||||||||||||||||||||||||||||||||||||||||||
get_groups(org_id, headers, conn).await | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
#[derive(Deserialize)] | ||||||||||||||||||||||||||||||||||||||||||
#[serde(rename_all = "camelCase")] | ||||||||||||||||||||||||||||||||||||||||||
struct GroupRequest { | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -2331,6 +2424,7 @@ struct SelectionReadOnly { | |||||||||||||||||||||||||||||||||||||||||
id: String, | ||||||||||||||||||||||||||||||||||||||||||
read_only: bool, | ||||||||||||||||||||||||||||||||||||||||||
hide_passwords: bool, | ||||||||||||||||||||||||||||||||||||||||||
manage: bool, | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
impl SelectionReadOnly { | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -2339,18 +2433,31 @@ impl SelectionReadOnly { | |||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
pub fn to_collection_group_details_read_only(collection_group: &CollectionGroup) -> SelectionReadOnly { | ||||||||||||||||||||||||||||||||||||||||||
// If both read_only and hide_passwords are false, then manage should be true | ||||||||||||||||||||||||||||||||||||||||||
// You can't have an entry with read_only and manage, or hide_passwords and manage | ||||||||||||||||||||||||||||||||||||||||||
// Or an entry with everything to false | ||||||||||||||||||||||||||||||||||||||||||
SelectionReadOnly { | ||||||||||||||||||||||||||||||||||||||||||
id: collection_group.groups_uuid.clone(), | ||||||||||||||||||||||||||||||||||||||||||
read_only: collection_group.read_only, | ||||||||||||||||||||||||||||||||||||||||||
hide_passwords: collection_group.hide_passwords, | ||||||||||||||||||||||||||||||||||||||||||
manage: !collection_group.read_only && !collection_group.hide_passwords, | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
pub fn to_collection_user_details_read_only(collection_user: &CollectionUser) -> SelectionReadOnly { | ||||||||||||||||||||||||||||||||||||||||||
pub fn to_collection_user_details_read_only( | ||||||||||||||||||||||||||||||||||||||||||
collection_user: &CollectionUser, | ||||||||||||||||||||||||||||||||||||||||||
user_org_type: i32, | ||||||||||||||||||||||||||||||||||||||||||
) -> SelectionReadOnly { | ||||||||||||||||||||||||||||||||||||||||||
// Vaultwarden allows manage access for Admins and Owners by default | ||||||||||||||||||||||||||||||||||||||||||
// For managers (Or custom role) it depends if they have read_ony or hide_passwords set to true or not | ||||||||||||||||||||||||||||||||||||||||||
SelectionReadOnly { | ||||||||||||||||||||||||||||||||||||||||||
id: collection_user.user_uuid.clone(), | ||||||||||||||||||||||||||||||||||||||||||
read_only: collection_user.read_only, | ||||||||||||||||||||||||||||||||||||||||||
hide_passwords: collection_user.hide_passwords, | ||||||||||||||||||||||||||||||||||||||||||
manage: user_org_type >= UserOrgType::Admin | ||||||||||||||||||||||||||||||||||||||||||
|| (user_org_type == UserOrgType::Manager | ||||||||||||||||||||||||||||||||||||||||||
&& !collection_user.read_only | ||||||||||||||||||||||||||||||||||||||||||
&& !collection_user.hide_passwords), | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
@@ -2534,7 +2641,7 @@ async fn bulk_delete_groups( | |||||||||||||||||||||||||||||||||||||||||
Ok(()) | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
#[get("/organizations/<org_id>/groups/<group_id>")] | ||||||||||||||||||||||||||||||||||||||||||
#[get("/organizations/<org_id>/groups/<group_id>", rank = 2)] | ||||||||||||||||||||||||||||||||||||||||||
async fn get_group(org_id: &str, group_id: &str, _headers: AdminHeaders, mut conn: DbConn) -> JsonResult { | ||||||||||||||||||||||||||||||||||||||||||
if !CONFIG.org_groups_enabled() { | ||||||||||||||||||||||||||||||||||||||||||
err!("Group support is disabled"); | ||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the comment states that it uses the
uuid
(of theUserOrganization
) instead of theuser_uuid
later theuser_uuid
is passed to get theusers_org_type
probably becauseCollectionUser
only knows aboutuser_uuid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been reading the code a bit and I think more comments would help.
e.g. in the struct
UserOrganization
, what's theuuid
anduser_uuid
for? Someone can only understand that, if they read and understand the entire code.And as you have shown here, the usage of these members is apparently not always that clear.
Don't take this the wrong way, I'd add the comments myself, but I doubt I understand all the different abstraction layers and levels in their entirety. I am also not saying that you should add those comments, but rather that I'd like to see more comments that explain certain connections and layers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm almost certain i changed this to have it working at all because it wasn't. I'll have to revisit it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I did not test if it still works. I just noticed the discrepancy when trying to apply your changes upon my newtype branch (#5320) because it would not even compile with the wrong id.
update: I have not tested all possible combinations but it seems to work just fine.
@tessus I'm not sure if comments would help. As I've shown they can be wrong or misleading. But to answer your question: Generally the
uuid
refers to an entity itself (e.g. theUserOrganization
relation or whatever is stored in theusers_organizations
table) while it does not with a prefix (e.g.user_uuid
refers to aUser
entity). I think the database schema is more consistent about this than the rest of the code base.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is an visualization of the relevant database relations
Created with visualize-sqlite and dot...
...after dropping the other tables from a fresh
db.sqlite3
:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok. Thanks for that. I was wondering why an n:m table would have its own uuid. However looking at the diagram, the naming is rather inconsistent and a bit confusing.