mirror of
https://github.com/dani-garcia/vaultwarden.wiki.git
synced 2026-10-23 20:58:09 +03:00
Fix revoked org members retaining access to org ciphers (#7554)
The cipher access restriction queries (direct collection access, group collection access, and full access via groups) never checked the organization membership status. Since users_collections and groups_users rows are kept when a membership is revoked, revoked (and not yet confirmed) members kept read, write, delete, attachment and collection-move access to org ciphers via the direct-by-UUID endpoints. Require a confirmed membership in the cipher's organization in all three queries, matching the behavior of the sync queries. Co-authored-by: Daniel García <dani-garcia@users.noreply.github.com>
This commit is contained in:
1 parent
cc67d644f6
commit
d2660324e6
2 files changed
+19
-2
No files matched your search
+15
-1
@@ -657,16 +657,27 @@ impl Cipher {
|
||||
}
|
||||
|
||||
async fn get_user_collections_access_flags(&self, user_uuid: &UserId, conn: &DbConn) -> Vec<(bool, bool, bool)> {
|
||||
let user_uuid = user_uuid.to_string();
|
||||
conn.run(move |conn| {
|
||||
// Check whether this cipher is in any collections accessible to the
|
||||
// user. If so, retrieve the access flags for each collection.
|
||||
// The user must have a confirmed membership in the cipher's
|
||||
// organization, since users_collections rows are kept when a
|
||||
// membership is revoked or not confirmed yet.
|
||||
ciphers::table
|
||||
.filter(ciphers::uuid.eq(&self.uuid))
|
||||
.inner_join(ciphers_collections::table.on(ciphers::uuid.eq(ciphers_collections::cipher_uuid)))
|
||||
.inner_join(
|
||||
users_organizations::table.on(ciphers::organization_uuid
|
||||
.eq(users_organizations::org_uuid.nullable())
|
||||
.and(users_organizations::user_uuid.eq(user_uuid))
|
||||
.and(users_organizations::status.eq(MembershipStatus::Confirmed as i32))),
|
||||
)
|
||||
.inner_join(
|
||||
users_collections::table.on(ciphers_collections::collection_uuid
|
||||
.eq(users_collections::collection_uuid)
|
||||
.and(users_collections::user_uuid.eq(user_uuid))),
|
||||
// Only allow collection access via the confirmed membership.
|
||||
.and(users_organizations::user_uuid.eq(users_collections::user_uuid))),
|
||||
)
|
||||
.select((users_collections::read_only, users_collections::hide_passwords, users_collections::manage))
|
||||
.load::<(bool, bool, bool)>(conn)
|
||||
@@ -697,6 +708,9 @@ impl Cipher {
|
||||
.and(groups::organizations_uuid.eq(users_organizations::org_uuid))),
|
||||
)
|
||||
.filter(users_organizations::user_uuid.eq(user_uuid))
|
||||
// Only allow group access via a confirmed membership, since
|
||||
// groups_users rows are kept when a membership is revoked.
|
||||
.filter(users_organizations::status.eq(MembershipStatus::Confirmed as i32))
|
||||
.select((collections_groups::read_only, collections_groups::hide_passwords, collections_groups::manage))
|
||||
.load::<(bool, bool, bool)>(conn)
|
||||
.expect("Error getting group access restrictions")
|
||||
|
||||
@@ -13,7 +13,7 @@ use crate::{
|
||||
};
|
||||
use macros::UuidFromParam;
|
||||
|
||||
use super::{CollectionId, Membership, MembershipId, OrganizationId, User, UserId};
|
||||
use super::{CollectionId, Membership, MembershipId, MembershipStatus, OrganizationId, User, UserId};
|
||||
|
||||
#[derive(Identifiable, Queryable, Insertable, AsChangeset)]
|
||||
#[diesel(table_name = groups)]
|
||||
@@ -268,6 +268,9 @@ impl Group {
|
||||
.and(users_organizations::org_uuid.eq(groups::organizations_uuid))),
|
||||
)
|
||||
.filter(users_organizations::user_uuid.eq(user_uuid))
|
||||
// Only allow full access via a confirmed membership, since
|
||||
// groups_users rows are kept when a membership is revoked.
|
||||
.filter(users_organizations::status.eq(MembershipStatus::Confirmed as i32))
|
||||
.filter(groups::organizations_uuid.eq(org_uuid))
|
||||
.filter(groups::access_all.eq(true))
|
||||
.select(groups::access_all)
|
||||
|
||||
Reference in new issue
Block a user