[Bug] Group Permissions Collection Deletion Issue (Manager role) about vaultwarden HOT 10 OPEN

So I understand why Bitwarden decided to get rid of the current approach and use

You're probably right.

Over the last few days, we have been playing around with groups and collections (although most of the problems are likely to occur even without groups).
The goal was to create different teams within an organization and give them the possibility to share individual elements on demand. This should primarily be done via separate collections (e.g. "Share_GroupA_to_GroupB").

Group A shares something with group B. Group B can share it with group C without group A noticing.
If group A removes the sharing with group B, group C will no longer see anything (usually).

Group A shares something with group B by reading it.
The element is read-only for all users in group B :-)
The manager in group B can extend the right of group B to the collection to "Edit" and all users have full access :-(

As you have already noticed: If a group has read-only access to collections, the webui sometimes allows editing of functions that are actually not permitted. Sometimes this is acknowledged with an error message (...cipher....), sometimes the dialog simply closes and the setting has not been changed.

In my opinion, the balancing act is that a role has priority over the collection rights and at the same time the same users should be restricted via rights to the collections.

P.S. Thank you stefan0XC for the quick fix to the problem with the Directory Connector. I just got around to compiling it and will try to test it tomorrow.

from vaultwarden.

I would like to add an additional note: Promoting the user to Admin removes this problem. So it seems isolated to the Manager role.

from vaultwarden.

I see according to the bitwarden.com documentation, that the Manager role will no longer exist. So I wonder if the issue above is related to the new web vault changes.

https://bitwarden.com/help/user-types-access-control/

Snippet:

Starting on March 3, 2024, organizations that haven't turned on collection management will begin to be migrated in batches to an updated permissions structure. If not migrated yet, your organization will be within the next few weeks or if you manually turn on collection management.
During migration, all Managers are migrated to members with the User role and automatically provided with a new Can manage permission over assigned collections. They will retain the ability to fully manage those collections, including the ability to assign new members or groups access.

from vaultwarden.

The issue has nothing to do with the manager role, as that currently is under a feature flag and not enabled for Vaultwarden.
The old way still works for both Self Hosted and Bitwarden Cloud unless you are migrated already or opt-in for it your self.

We would have to check what the reason here is, or maybe fix the manager to user role right away.

from vaultwarden.

As far as I've looked into it, the removal will be ignored because Cipher::get_collections() does not have support for collections via groups, so it won't be part of the symmetric difference (if I understand the logic correctly).

I've not looked more into why there is no immediate visual indication of this change. (If it works with the bitwarden/server the web-vault might expect the function to return something instead of an EmptyResult?)

from vaultwarden.

Did some additional testing incase it helps. It looks like the extension (2024.4.2) is affected as well.

People with the User or manager rank will be able to see that they can uncheck a collection from a password entry using the Bitwarden extension. On the latest web vault version in testing, you will see those collections are greyed out and unable to be modified.
Upon saving the change via the extension, it'll show as change was updated. Although upon the next sync to server, the change is undone on the client side. So anything before the sync, gives a false sense of successful change.

from vaultwarden.

While trying to fix the issue I've run into couple of scenarios where it would either not work for Admins/Owners (deleting the collections which aren't visible for an Admin/Owner in the Password Manager, or not being able to delete collections even though they have been unselected) or there would still be a problem with Managers because for them it's also important whether or not they have write access to a collection (because read-only collections will still be visible/selectable in the Admin Console but hidden in the Password Manager). Also updating the collections in the Admin Console sometimes caused an exception for Managers or for Admins/Owners by switching to the Admin Console.

Testing the different cases is a bit tedious because I need to use at least two different accounts (with Manager and Admin or Owner role) assigned to different collections and not using the access_all permission.

So I understand why Bitwarden decided to get rid of the current approach and use their more flexible permission system which you can probably check for more easily. (E.g. only get those collections where you have the Can manage permission unless you are admin/owner, where you will always have access to all collections irregardless.)

But now I'm wondering if we should not get rid of the manager role and the access all flag as well and implement custom permissions instead.

from vaultwarden.

get rid of the manager role and the access all flag as well and implement custom permissions instead.

Preferably it'd probably be best to go forward with the custom permissions, if that's the way Bitwarden has already went.
Based on discussions from above, I assume this would mean a custom roles feature flag would need to be activated as a pre-requisite to using groups?

from vaultwarden.

It also needs flexible collections then and a migration code for it.

from vaultwarden.

I am still trying to fix the bug with the current approach, so not sure if it is required right now. But eventually we probably want/have to implement flexible collections and migrate to the new approach. 🙈

from vaultwarden.