Skip to content
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

Issue with group-level access permissions and collection management for managers #3624

Closed
fdaone opened this issue Jun 28, 2023 · 2 comments · Fixed by #3754
Closed

Issue with group-level access permissions and collection management for managers #3624

fdaone opened this issue Jun 28, 2023 · 2 comments · Fixed by #3754
Labels
bug Something isn't working help wanted Extra attention is needed low priority Won't fix anytime soon, but will accept PR if provided troubleshooting There might be bug or it could be user error, more info needed

Comments

@fdaone
Copy link

fdaone commented Jun 28, 2023

Tested with the most recent testing docker image (Digest:sha256:78f4cf6c42004d70afb8673ef55bd88f25b62094b41275e935947e4ed6e8db17)

Subject of the issue

Group-level access permissions are not working as intended with regards to collection management (for members with the manager role).

Deployment environment (Generated via diagnostics page)

  • Vaultwarden version: v1.28.1-e7f083de
  • Web-vault version: v2023.5.0
  • OS/Arch: linux/x86_64
  • Running within Docker: true (Base: Debian)
  • Environment settings overridden: false
  • Uses a reverse proxy: true
  • IP Header check: true (X-Real-IP)
  • Internet access: false
  • Internet access via a proxy: false
  • DNS Check: true
  • Browser/Server Time Check: true
  • Server/NTP Time Check: n/a
  • Domain Configuration Check: true
  • HTTPS Check: true
  • Database type: PostgreSQL
  • Database version: PostgreSQL 14.2 (Ubuntu 14.2-1.pgdg20.04+1) on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0, 64-bit
  • Clients used: Issue is seen with the web-vault. Not using any other clients.
  • Reverse proxy and version: nginx/1.18.0 (Ubuntu)
  • Other relevant information:

Vaultwarden is started like this: docker run -d --env-file /opt/docker/vw-data-test/.env --name vw-test -v /opt/docker/vw-data-test:/data -p 0.0.0.0:8081:81 -p 0.0.0.0:3013:3013 --restart on-failure harbor/mirror/docker.io/vaultwarden/server@sha256:78f4cf6c42004d70afb8673ef55bd88f25b62094b41275e935947e4ed6e8db17

Steps to reproduce the issue

As an admin assign the manager role to Member.
Add Member to Group that has 'Can edit' on Collection.

Log in as Member. Go to Organizations (top menu). Collection cannot be edited/modified as one would expect with the manager role and 'Can edit'.

The small pop-up menu with 'Edit info','Access',Delete' is simply not accessible. Normally this small pop-up menu can be accessed by clicking the 3 small dots to the far right of a collection or by clicking the "arrow" (pointing down) right next to the collection name once you're already looking inside the collection in question. Neither the 3 dots, nor the arrow pointing down is shown in the web UI.

However, new collections can without problems be created. As such, create new NestedCollection with Collection as "parent" and give 'Can edit' to Group.

Now NestedCollection has been created, but Member also cannot edit/modify this one.

The ability to modify/edit collections only works, if Member gets 'Can edit' applied directly as a user-level access permission (which of course defeats the whole purpose of utilizing group-level access permissions which are highly convenient in many scenarios with several users).

Now comes the funny/puzzling part... If Member gets even just 'Can view' applied as a user-level access permission, the 'Can edit' access permission from the Group starts to work immediately.

@BlackDex BlackDex added the troubleshooting There might be bug or it could be user error, more info needed label Jul 4, 2023
@BlackDex BlackDex added the bug Something isn't working label Jul 12, 2023
@BlackDex
Copy link
Collaborator

Confirmed.
It probably has the same issue as #3413 in regards to a query which is probably not optimized.

We need to try and solve this in a good manner, and i did not had the time yet to take a very good look at all the group queries to find a sane way to solve this all.

So, if anybody has a good idea, normalize this data differently, or other sane ways, please help out by creating a PR or provide some good pointers :).

@BlackDex BlackDex added help wanted Extra attention is needed low priority Won't fix anytime soon, but will accept PR if provided labels Jul 12, 2023
matlink added a commit to matlink/vaultwarden that referenced this issue Aug 4, 2023
matlink added a commit to matlink/vaultwarden that referenced this issue Aug 4, 2023
matlink added a commit to matlink/vaultwarden that referenced this issue Aug 4, 2023
matlink added a commit to matlink/vaultwarden that referenced this issue Aug 4, 2023
matlink added a commit to matlink/vaultwarden that referenced this issue Aug 4, 2023
matlink added a commit to matlink/vaultwarden that referenced this issue Aug 4, 2023
matlink added a commit to matlink/vaultwarden that referenced this issue Aug 4, 2023
matlink added a commit to matlink/vaultwarden that referenced this issue Jan 1, 2024
matlink added a commit to matlink/vaultwarden that referenced this issue Feb 17, 2024
dani-garcia pushed a commit that referenced this issue Mar 17, 2024
* Fix #3624: fix manager permission within groups

* Query returns UUID only

* Fix issue when user is manager and in a group having access to all collections

* optimize condition check

* fix(groups): renaming and optimizations

* fix: wrong organization group membership detection

* Simplify group membership check

Co-authored-by: Stefan Melmuk <509385+stefan0xC@users.noreply.github.com>

* Remove unused statement

* improve check if the user has access via groups

instead of returning the two lists of member ids and later checking if
they contain the uuid of the current user, we really only care if
the current user has full access via a group or if they have
access to a given collection via a group

* improve comments for get_org_collections_details

* small refactor to make it easier to review

* fix(groups): query full access via group only when necessary

Co-authored-by: Mathijs van Veluw <black.dex@gmail.com>

* chore(fmt): apply rustfmt

---------

Co-authored-by: Stefan Melmuk <509385+stefan0xC@users.noreply.github.com>
Co-authored-by: Stefan Melmuk <stefan.melmuk@gmail.com>
Co-authored-by: Mathijs van Veluw <black.dex@gmail.com>
@fdaone
Copy link
Author

fdaone commented Mar 18, 2024

Thank you so much to everyone who made this happen. Really appreciate it. I have just tested the newest docker testing release. Works as intended now. 🥳 🙌

ZhReimu pushed a commit to ZhReimu/vaultwarden that referenced this issue Jul 9, 2024
…ia#3754)

* Fix dani-garcia#3624: fix manager permission within groups

* Query returns UUID only

* Fix issue when user is manager and in a group having access to all collections

* optimize condition check

* fix(groups): renaming and optimizations

* fix: wrong organization group membership detection

* Simplify group membership check

Co-authored-by: Stefan Melmuk <509385+stefan0xC@users.noreply.github.com>

* Remove unused statement

* improve check if the user has access via groups

instead of returning the two lists of member ids and later checking if
they contain the uuid of the current user, we really only care if
the current user has full access via a group or if they have
access to a given collection via a group

* improve comments for get_org_collections_details

* small refactor to make it easier to review

* fix(groups): query full access via group only when necessary

Co-authored-by: Mathijs van Veluw <black.dex@gmail.com>

* chore(fmt): apply rustfmt

---------

Co-authored-by: Stefan Melmuk <509385+stefan0xC@users.noreply.github.com>
Co-authored-by: Stefan Melmuk <stefan.melmuk@gmail.com>
Co-authored-by: Mathijs van Veluw <black.dex@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed low priority Won't fix anytime soon, but will accept PR if provided troubleshooting There might be bug or it could be user error, more info needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants