Allow all manager to create collections again #5488
Conversation
|
@stefan0xC, if you are able, please check and verify. |
|
Also, we can't allow it for users too, as that will currently break because we check for Manager Headers. |
|
Tested it a bit and found the following issues:
Here are the logs from the 3. issue first request to {
"groups":[],
"users": [
{"id":"62ac3290-2ab3-4e58-9a1c-45c83570b6e6","readOnly":false,"hidePasswords":false,"manage":false},
{"id":"b0089b1c-960f-408b-a36c-8ed5ad54aa9e","readOnly":false,"hidePasswords":false,"manage":false},
{"id":"4d574091-85a8-4dc7-8c21-c96e0eded250","readOnly":false,"hidePasswords":false,"manage":false},
{"id":"510e2949-e433-4d3f-9bdc-35c9f9e7347a","readOnly":false,"hidePasswords":false,"manage":true},
{"id":"9ca6e1d0-c334-4e09-b0d4-9251c04eb5bb","readOnly":false,"hidePasswords":false,"manage":false},
{"id":"22ecb6d3-1c48-43c8-94f6-9380a4ac71dd","readOnly":false,"hidePasswords":false,"manage":false}
],
"externalId":null,
"name":"2.7OHlTYWwJYK4uBnke11IOQ==|ukizJ9GF8zHZrDphVAdN8A==|t9MMTfYTjSvi+JV08JDK4sJ+ou+4qNnrL4JC+SZi544="}which is weird because according to my database those are my users different memberships
I have not yet checked further into why this happens. |
|
Thanks for the in depth test The rest i would need to check too. And i also see a warning regarding not using a |
Yeah, I mean it's just weird that the web-vault doesn't prevent you from doing that (or warn you about it) but I think that might also have been the case previously so it's not something I'd fix either. (edit: also removing your own access seems to work as it did before.) |
|
I think I have found the issue. When vaultwarden/src/db/models/collection.rs Lines 688 to 696 in a3dccee This seems to join the tables incorrectly and returns all memberships from all members that are assigned to the given collection regardless of the organization instead of only the members for that organization that are assigned to the collection. So I think it should be more like select uo.uuid from collections as c
inner join users_collections as uc on c.uuid=uc.collection_uuid
inner join users_organizations as uo on c.org_uuid = uo.org_uuid
where uo.user_uuid = uc.user_uuid and c.uuid = '8613531a-e984-4a70-aa41-52d1eab0b332';Or to make it easier, since we already know the |
|
I found the issue of not having direct manage access to a collection after creation, we need to return the detailed collection json, that solved the issue. |
This commit checks if the member is a manager or better, and if so allows it to createCollections. We actually check if it is less then a Manager, since the `limitCollectionCreation` should be set to false to allow it and true to prevent. This should fix an issue discussed in dani-garcia#5484 Signed-off-by: BlackDex <[email protected]>
BlackDex
force-pushed
the
allow-managers-to-create-collections
branch
from
6af2b80 to
980b0d9
Compare
|
@stefan0xC if you can recheck the same scenario, that would be cool. |
BlackDex
force-pushed
the
allow-managers-to-create-collections
branch
from
980b0d9 to
2fb337d
Compare
|
Yeah, I think I found another issue though in vaultwarden/src/db/models/collection.rs Lines 583 to 592 in 663f88e The filter should probably be applied to |
Signed-off-by: BlackDex <[email protected]>
BlackDex
force-pushed
the
allow-managers-to-create-collections
branch
from
2fb337d to
692967f
Compare
|
@stefan0xC, i added an extra filter there too. That should help. |
BlackDex
requested review from
dani-garcia
and removed request for
dani-garcia
This commit checks if the member is a manager or better, and if so allows it to createCollections. We actually check if it is less then a Manager, since the
limitCollectionCreationshould be set to false to allow it and true to prevent.This should fix an issue discussed in #5484
Fixes #5489