Skip to content

Commit

Permalink
Fix collection management and match some json output (dani-garcia#5095)
Browse files Browse the repository at this point in the history
- Fixed collection management to be usable from the Password Manager UI
- Checked and brought in-to-sync with upstream several json responses
- Fixed a small issue with the `fields` response when it was empty

Signed-off-by: BlackDex <black.dex@gmail.com>
  • Loading branch information
BlackDex authored Oct 18, 2024
1 parent b7c254e commit ae6ed0e
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 27 deletions.
1 change: 0 additions & 1 deletion src/api/core/ciphers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ async fn sync(data: SyncData, headers: Headers, mut conn: DbConn) -> Json<Value>
"ciphers": ciphers_json,
"domains": domains_json,
"sends": sends_json,
"unofficialServer": true,
"object": "sync"
}))
}
Expand Down
1 change: 1 addition & 0 deletions src/api/core/organizations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ async fn get_org_collections_details(org_id: &str, headers: ManagerHeadersLoose,
json_object["users"] = json!(users);
json_object["groups"] = json!(groups);
json_object["object"] = json!("collectionAccessDetails");
json_object["unmanaged"] = json!(false);
data.push(json_object)
}

Expand Down
13 changes: 1 addition & 12 deletions src/api/identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,8 @@ async fn _refresh_login(data: ConnectData, conn: &mut DbConn) -> JsonResult {
"expires_in": expires_in,
"token_type": "Bearer",
"refresh_token": device.refresh_token,
"Key": user.akey,
"PrivateKey": user.private_key,

"Kdf": user.client_kdf_type,
"KdfIterations": user.client_kdf_iter,
"KdfMemory": user.client_kdf_memory,
"KdfParallelism": user.client_kdf_parallelism,
"ResetMasterPassword": false, // TODO: according to official server seems something like: user.password_hash.is_empty(), but would need testing
"scope": scope,
"unofficialServer": true,
});

Ok(Json(result))
Expand Down Expand Up @@ -342,7 +334,6 @@ async fn _password_login(
"MasterPasswordPolicy": master_password_policy,

"scope": scope,
"unofficialServer": true,
"UserDecryptionOptions": {
"HasMasterPassword": !user.password_hash.is_empty(),
"Object": "userDecryptionOptions"
Expand Down Expand Up @@ -461,9 +452,8 @@ async fn _user_api_key_login(
"KdfIterations": user.client_kdf_iter,
"KdfMemory": user.client_kdf_memory,
"KdfParallelism": user.client_kdf_parallelism,
"ResetMasterPassword": false, // TODO: Same as above
"ResetMasterPassword": false, // TODO: according to official server seems something like: user.password_hash.is_empty(), but would need testing
"scope": "api",
"unofficialServer": true,
});

Ok(Json(result))
Expand Down Expand Up @@ -495,7 +485,6 @@ async fn _organization_api_key_login(data: ConnectData, conn: &mut DbConn, ip: &
"expires_in": 3600,
"token_type": "Bearer",
"scope": "api.organization",
"unofficialServer": true,
})))
}

Expand Down
2 changes: 1 addition & 1 deletion src/db/models/cipher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ impl Cipher {

// NOTE: This was marked as *Backwards Compatibility Code*, but as of January 2021 this is still being used by upstream
// data_json should always contain the following keys with every atype
data_json["fields"] = json!([fields_json]);
data_json["fields"] = json!(fields_json);
data_json["name"] = json!(self.name);
data_json["notes"] = json!(self.notes);
data_json["passwordHistory"] = Value::Array(password_history_json.clone());
Expand Down
6 changes: 3 additions & 3 deletions src/db/models/collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ impl Collection {
let (read_only, hide_passwords, can_manage) = if let Some(cipher_sync_data) = cipher_sync_data {
match cipher_sync_data.user_organizations.get(&self.org_uuid) {
// Only for Manager types Bitwarden returns true for the can_manage option
// Owners and Admins always have false, but they can manage all collections anyway
Some(uo) if uo.has_full_access() => (false, false, uo.atype == UserOrgType::Manager),
// Owners and Admins always have true
Some(uo) if uo.has_full_access() => (false, false, uo.atype >= UserOrgType::Manager),
Some(uo) => {
// Only let a manager manage collections when the have full read/write access
let is_manager = uo.atype == UserOrgType::Manager;
Expand All @@ -98,7 +98,7 @@ impl Collection {
}
} else {
match UserOrganization::find_confirmed_by_user_and_org(user_uuid, &self.org_uuid, conn).await {
Some(ou) if ou.has_full_access() => (false, false, ou.atype == UserOrgType::Manager),
Some(ou) if ou.has_full_access() => (false, false, ou.atype >= UserOrgType::Manager),
Some(ou) => {
let is_manager = ou.atype == UserOrgType::Manager;
let read_only = !self.is_writable_by_user(user_uuid, conn).await;
Expand Down
2 changes: 1 addition & 1 deletion src/db/models/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl Group {
"id": entry.collections_uuid,
"readOnly": entry.read_only,
"hidePasswords": entry.hide_passwords,
"manage": *user_org_type == UserOrgType::Manager && !entry.read_only && !entry.hide_passwords
"manage": *user_org_type >= UserOrgType::Admin || (*user_org_type == UserOrgType::Manager && !entry.read_only && !entry.hide_passwords)
})
})
.collect();
Expand Down
6 changes: 2 additions & 4 deletions src/db/models/organization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ impl Organization {
"identifier": null, // not supported by us
"name": self.name,
"seats": null,
"maxAutoscaleSeats": null,
"maxCollections": null,
"maxStorageGb": i16::MAX, // The value doesn't matter, we don't check server-side
"use2fa": true,
Expand Down Expand Up @@ -374,7 +373,6 @@ impl UserOrganization {
"identifier": null, // Not supported
"name": org.name,
"seats": null,
"maxAutoscaleSeats": null,
"maxCollections": null,
"usersGetPremium": true,
"use2fa": true,
Expand Down Expand Up @@ -411,7 +409,7 @@ impl UserOrganization {
"familySponsorshipValidUntil": null,
"familySponsorshipToDelete": null,
"accessSecretsManager": false,
"limitCollectionCreationDeletion": true,
"limitCollectionCreationDeletion": false, // This should be set to true only when we can handle roles like createNewCollections
"allowAdminAccessToAllCollectionItems": true,
"flexibleCollections": false,

Expand Down Expand Up @@ -477,7 +475,7 @@ impl UserOrganization {
.into_iter()
.filter_map(|c| {
let (read_only, hide_passwords, can_manage) = if self.has_full_access() {
(false, false, self.atype == UserOrgType::Manager)
(false, false, self.atype >= UserOrgType::Manager)
} else if let Some(cu) = cu.get(&c.uuid) {
(
cu.read_only,
Expand Down
12 changes: 7 additions & 5 deletions src/db/models/user.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::util::{format_date, get_uuid, retry};
use chrono::{NaiveDateTime, TimeDelta, Utc};
use serde_json::Value;

Expand Down Expand Up @@ -90,7 +91,7 @@ impl User {
let email = email.to_lowercase();

Self {
uuid: crate::util::get_uuid(),
uuid: get_uuid(),
enabled: true,
created_at: now,
updated_at: now,
Expand All @@ -107,7 +108,7 @@ impl User {
salt: crypto::get_random_bytes::<64>().to_vec(),
password_iterations: CONFIG.password_iterations(),

security_stamp: crate::util::get_uuid(),
security_stamp: get_uuid(),
stamp_exception: None,

password_hint: None,
Expand Down Expand Up @@ -188,7 +189,7 @@ impl User {
}

pub fn reset_security_stamp(&mut self) {
self.security_stamp = crate::util::get_uuid();
self.security_stamp = get_uuid();
}

/// Set the stamp_exception to only allow a subsequent request matching a specific route using the current security-stamp.
Expand Down Expand Up @@ -259,6 +260,7 @@ impl User {
"forcePasswordReset": false,
"avatarColor": self.avatar_color,
"usesKeyConnector": false,
"creationDate": format_date(&self.created_at),
"object": "profile",
})
}
Expand Down Expand Up @@ -340,7 +342,7 @@ impl User {
let updated_at = Utc::now().naive_utc();

db_run! {conn: {
crate::util::retry(|| {
retry(|| {
diesel::update(users::table)
.set(users::updated_at.eq(updated_at))
.execute(conn)
Expand All @@ -357,7 +359,7 @@ impl User {

async fn _update_revision(uuid: &str, date: &NaiveDateTime, conn: &mut DbConn) -> EmptyResult {
db_run! {conn: {
crate::util::retry(|| {
retry(|| {
diesel::update(users::table.filter(users::uuid.eq(uuid)))
.set(users::updated_at.eq(date))
.execute(conn)
Expand Down

0 comments on commit ae6ed0e

Please sign in to comment.