IQSS
Optimize permission lookups for a user
What this PR does / why we need it: The need to query "what dataverses does User x have Permission y on"
Which issue(s) this PR closes: https://github.com/IQSS/dataverse/issues/6467
- Closes #6467
Special notes for your reviewer: I'm not crazy about the api being called /allowedcollections/. Any suggestions for a better name would be appreciated.
Suggestions on how to test this: See IT test. Also test a group within a group for nested group permissions. I have tested it manually and it was working.
Does this PR introduce a user interface change? If mockups are available, please link/include them here: No
Is there a release notes update needed for this change?: Included
Additional documentation:
coverage: 22.738% (-0.01%) from 22.751% when pulling c290e403138cee223077fa93b01256eeda2672d6 on 6467-optimize-permission-lookups-for-a-user into 69ebed233a475820c0e1d6a1df65e2ba869cd678 on develop.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
Hi, finally am managing to review - looks good overall, a few questions, easier to ask as a general comment imo:
- Does the sql query deal with nested groups (I think so, with the "With" but wanted to confrim?
- Does the last union (ip groups) need the exists / permissions bit clause?
- Do we have any sense on performace of this query yet?
- Yes it handles groups within groups. I did test that
- Sorry it's a bit confusing. The permission bit part is added in the"AND @IPRANGESQL" by calling findPermittedCollections
- There hasn't been any formal performance testing but local tests as well as testing on the perf server were pretty good.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
Hi, finally am managing to review - looks good overall, a few questions, easier to ask as a general comment imo:
- Does the sql query deal with nested groups (I think so, with the "With" but wanted to confrim?
- Does the last union (ip groups) need the exists / permissions bit clause?
- Do we have any sense on performace of this query yet?
- Yes it handles groups within groups. I did test that
- Sorry it's a bit confusing. The permission bit part is added in the"AND @IPRANGESQL" by calling findPermittedCollections
- There hasn't been any formal performance testing but local tests as well as testing on the perf server were pretty good.
OK on 1 and 3. For 2, maybe I'm still missing it - for the query part added in lines 143-152 for the ip groups, I don't see any reference to permisison bits).
OK on 1 and 3. For 2, maybe I'm still missing it - for the query part added in lines 143-152 for the ip groups, I don't see any reference to permisison bits).
Ok. You are correct. there is no permission bits in IP groups. This is a bit confusing but here goes
in regular groups: "_roleAlias": "contributor", where contributor has permissions where each bit denotes a permission (for file download bit 4 is set) sql: (dataverserole.permissionbits & 16 !=0)
in IP Groups the role is the specific permission so there is no need to check the bit sql: look for 'fileDownloader' in WHERE roleassignment.assigneeidentifier IN (SELECT CONCAT('&ip/', persistedglobalgroup.persistedgroupalias) as assignee...) { "status": "OK", "data": { "assignee": "&ip/ipGroupebc7aa00", "roleId": 2, "_roleAlias": "fileDownloader", "definitionPointId": 4 } }
in IP Groups the role is the specific permission so there is no need to check the bit
I don't think that's accurate (or at least now how it's designed. Sure, IP groups are mostly meant to allow read access (and usually file download), BUT it could be for the role "viewer".
And technically, if you're logged in and from a certain ip address, then contributor role should work too. (as examples)
I added the PERMISSIONBIT to the IpGroup sql and changed the test to test with "CURATOR" role
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.
:package: Pushed preview images as
ghcr.io/gdcc/dataverse:6467-optimize-permission-lookups-for-a-user
ghcr.io/gdcc/configbaker:6467-optimize-permission-lookups-for-a-user
:ship: See on GHCR. Use by referencing with full name as printed above, mind the registry name.