Adding Manager Role support #1242
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dani-garcia
reviewed
Nov 29, 2020
Thank you for taking time to implement this need. I tried all my best, but my lack of knowledge of Rust made me rely on more experimented users. I'm building it to try it on my own, and will give you feedback. |
This has been requested a few times (dani-garcia#1136 & dani-garcia#246 & forum), and there already were two (1:1 duplicate) PR's (dani-garcia#1222 & dani-garcia#1223) which needed some changes and no followups or further comments unfortunally. This PR adds two auth headers. - ManagerHeaders Checks if the user-type is Manager or higher and if the manager is part of that collection or not. - ManagerHeadersLoose Check if the user-type is Manager or higher, but does not check if the user is part of the collection, needed for a few features like retreiving all the users of an org. I think this is the safest way to implement this instead of having to check this within every function which needs this manually. Also some extra checks if a manager has access to all collections or just a selection. fixes dani-garcia#1136
BlackDex
force-pushed
the
allow-manager-role
branch
from
c992bb5 to
7cf8809
Compare
No problem, i saw the need was there, and had some extra time left. |
|
@dani-garcia Thx for the note about that query already being done. Changes are applied. |
|
Thanks for the great support! |
Closed
jjlin
added a commit
to jjlin/vaultwarden
that referenced
this pull request
Jan 27, 2021
The implementation of the `Manager` user type (dani-garcia#1242) introduced a regression whereby owner/admin users are incorrectly denied access to certain collection APIs if their access control for collections isn't set to "access all". Owner/admin users should always have full access to collection APIs, per https://bitwarden.com/help/article/user-types-access-control/#access-control: > Assigning Admins and Owners to Collections via Access Control will only > impact which Collections appear readily in the Filters section of their > Vault. Admins and Owners will always be able to access "un-assigned" > Collections via the Organization view.
Koisell
pushed a commit
to Koisell/vaultwarden
that referenced
this pull request
Feb 17, 2021
The implementation of the `Manager` user type (dani-garcia#1242) introduced a regression whereby owner/admin users are incorrectly denied access to certain collection APIs if their access control for collections isn't set to "access all". Owner/admin users should always have full access to collection APIs, per https://bitwarden.com/help/article/user-types-access-control/#access-control: > Assigning Admins and Owners to Collections via Access Control will only > impact which Collections appear readily in the Filters section of their > Vault. Admins and Owners will always be able to access "un-assigned" > Collections via the Organization view.
thelittlefireman
pushed a commit
to thelittlefireman/bitwarden_rs
that referenced
this pull request
Mar 19, 2021
Adding Manager Role support
thelittlefireman
pushed a commit
to thelittlefireman/bitwarden_rs
that referenced
this pull request
Mar 19, 2021
The implementation of the `Manager` user type (dani-garcia#1242) introduced a regression whereby owner/admin users are incorrectly denied access to certain collection APIs if their access control for collections isn't set to "access all". Owner/admin users should always have full access to collection APIs, per https://bitwarden.com/help/article/user-types-access-control/#access-control: > Assigning Admins and Owners to Collections via Access Control will only > impact which Collections appear readily in the Filters section of their > Vault. Admins and Owners will always be able to access "un-assigned" > Collections via the Organization view. Signed-off-by: thelittlefireman <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This has been requested a few times (#1136 & #246 & forum), and there already were two
(1:1 duplicate) PR's (#1222 & #1223) which needed some changes and no
followups or further comments unfortunally.
This PR adds two auth headers.
Checks if the user-type is Manager or higher and if the manager is
part of that collection or not.
Check if the user-type is Manager or higher, but does not check if the
user is part of the collection, needed for a few features like
retreiving all the users of an org.
I think this is the safest way to implement this instead of having to
check this within every function which needs this manually.
Also some extra checks if a manager has access to all collections or
just a selection.
fixes #1136