-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Add partial role support for manager only using web-vault v2024.12.0 #5219
base: main
Are you sure you want to change the base?
Conversation
This PR needs a web-vault built from this PR dani-garcia/bw_web_builds#186 But it would be nice if some people are able to test this PR using a web-vault built from the PR linked above. |
78ab1c1
to
4b283b2
Compare
May I suggest to add more comments to the code, where your changes from this PR have to be reverted when full role support is implemented. Unless you you'll keep the compat layer even then. It is quite possible to forget to remove/change current code/behavior when the "future" changes are not close to this PR. Please ignore, if I am way off. |
ae62f5b
to
6e337c6
Compare
@tessus done :) @stefan0xC I have fixed the icon in the collapsed menu. |
de0b54c
to
0a4ffc7
Compare
As mentioned before, this PR needs a new web-vault version for which a PR can be found here: CI builds of this web-vault can be downloaded here:
|
71d90ed
to
31957cc
Compare
31957cc
to
1e71195
Compare
If this is the only PR required for the new web vault, is it possible to merge this and then release the new web vault - maybe this weekend? |
I'd rather first fix a native client issue and release a version and after that merge this. I also think it is better to first release the web-vault and then add that release to this PR. |
Thanks! I fixed it just now. |
Now that I think about it, shouldn't the create account line also be hidden? I have signups disabled. |
Also fixed now :) |
0c4fede
to
d17dc0d
Compare
I'm checking to see if we can update to v2024.12.0. There seem to be some minor changes and some endpoints added. |
Code is adjusted to support v2024.12.0 now. Not too much changed. |
a16668a
to
748a242
Compare
The native client issue has been fixed and a new version has been released. Is it possible to merge this now and release the new web-vault 2024.12.0 (or the other way around as you suggested)? |
What's the matter with you and the new web vault version? Isn't the current one working for you? If not then just try to build and run it yourself |
I already do. If you had read my earlier comments, you would have known. |
I'm just trying to understand why you're so keen on this being merged for the second time already. |
@BlackDex the endpoint |
No it's not. It is there. |
@stefan0xC seems to work for me:
Be sure to pull in the latest commit and are on:
|
- Add the custom role which replaces the manager role - Added mini-details endpoint used by v2024.11.1 These changes try to add the custom role in such a way that it stays compatible with the older manager role. It will convert a manager role into a custom role, and if a manager has `access-all` rights, it will enable the correct custom roles. Upon saving it will convert these back to the old format. What this does is making sure you are able to revert back to an older version of Vaultwarden without issues. This way we can support newer web-vault's and still be compatible with a previous Vaultwarden version if needed. In the future this needs to be changed to full role support though. Fixed the 2FA hide CSS since the order of options has changed Signed-off-by: BlackDex <black.dex@gmail.com>
Signed-off-by: BlackDex <black.dex@gmail.com>
Signed-off-by: BlackDex <black.dex@gmail.com>
Signed-off-by: BlackDex <black.dex@gmail.com>
748a242
to
b0920a6
Compare
@BlackDex if the group has {
"accessAll": true,
"collections": [],
"externalId": null,
"id": "41917ddc-7cf7-47d0-bd7f-7b24d21c2b71",
"name": "manager group",
"object": "groupDetails",
"organizationId": "152bd996-19f9-40ee-b991-e52333a7e719"
} but it fails when the group has permissions for individual collections: {
"accessAll": false,
"collections": [
{
"hidePasswords": true,
"id": "f39e653f-3191-4312-887d-931889be5d20",
"manage": false,
"readOnly": false
},
{
"hidePasswords": true,
"id": "ca4b77ab-c849-47f4-a6f3-c4ebde35d5f0",
"manage": false,
"readOnly": true
}
],
"externalId": null,
"id": "41917ddc-7cf7-47d0-bd7f-7b24d21c2b71",
"name": "manager group",
"object": "groupDetails",
"organizationId": "152bd996-19f9-40ee-b991-e52333a7e719"
} |
@stefan0xC What happens then exactly? |
The exception happened when I tried to open the Custom user member (which used to be a Manager role without edit: I'm unable to reproduce the issue. It might have been caused by using an in-between version of the web-vault... |
Signed-off-by: BlackDex <black.dex@gmail.com>
These changes try to add the custom role in such a way that it stays compatible with the older manager role. It will convert a manager role into a custom role, and if a manager has
access-all
rights, it will enable the correct custom roles. Upon saving it will convert these back to the old format.What this does is making sure you are able to revert back to an older version of Vaultwarden without issues. This way we can support newer web-vault's and still be compatible with a previous Vaultwarden version if needed.
In the future this needs to be changed to full role support though.