storeaccess: clear the legacy owner bit when revoking in bulk - #98
Open
r4nc0r wants to merge 1 commit into
Open
Conversation
Both bulk paths write STOREACCESS_SET for the requested users, which setFolderMembers removes from everyone else. A grant written up to 1.20 carries GROMOXSTOREOWNER instead, so the removal is a no-op for it: the grantee stays a store owner and stays listed, and the admin-web entry comes back on the next reload. The API path cleared the bit only from the users it was granting to, and the CLI path did not clear it at all. Clearing it from every member with an empty setFolderMembers call covers grantees and revokees alike, which is what deleteUserStoreAccess already does for a single user.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What happens
Revoking an additional store owner in bulk returns success and changes nothing when the grant was written by admin-api 1.20 or earlier. The grantee keeps store ownership,
GET storeAccesskeeps listing them, and the admin-web entry is back on the next reload. Both bulk paths are affected:PUT /domains/{d}/users/{u}/storeAccesswith the user left out of the list, andgrommunio-admin user <mailbox> storeowner remove.This is the part of #93 point 2 that 7deb809 did not reach.
DELETE storeAccess/{username}is already correct there. Follow-up to #95, which I closed in favour of 7deb809.Why
setFolderMembersadds the mask for the users in the list and removes it from every other member, so bulk revocation happens by omission withSTOREACCESS_SET(0x700). A grant written up to 1.20 carriesGROMOXSTOREOWNER(0x2000) instead, and0x2000 & ~0x700is0x2000, so nothing is written. gromox grants store ownership from that bit on its own —get_mbox_perminexch/exmdb/store.cppORs the stored rights in verbatim — andSTOREACCESS_GETmatches it, so the user stays both an owner and listed.7deb809 does clear the legacy bit in
setUserStoreAccessMulti, but only for the users named in the request, which are exactly the ones keeping their access.cli/user.pydoes not clear it at all.The change
One
setFolderMembers(eid, [], Permissions.GROMOXSTOREOWNER)call per bulk path. An empty list clears the mask from every member, so grantees and revokees are both covered in a single round trip, which is whatdeleteUserStoreAccessalready does for a single user. A row left with no rights is removed bysetFolderMembersitself.Verified
0x20000x2000, listed, still owner0x38000x3800, listed, still owner0x1800, not listed0x7000x7fb0x4fb, not listed0x4fb, not listedDELETE storeAccess0x2000/0x38000x1800Modern and client-granted rows are unaffected: the delta only ever clears
0x2000. The two rows that disagree on master are the legacy ones.grommunio/gromox@8e0dd961 replaces
0x2000with0x100on folder 9 in a one-time private-store schema upgrade (schema 29), after which these rows no longer exist. This patch is what makes the bulk paths correct on an installation whose gromox has not reached that schema yet, and a no-op afterwards.