[Fixes #14262] Avoid returning all permissions in dataset owner payload#14263
[Fixes #14262] Avoid returning all permissions in dataset owner payload#14263sijandh35 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request modifies geonode/security/registry.py to restrict global permissions to only those explicitly defined in the PERMISSIONS map, removing the fallback for custom permissions. The reviewer identifies a high-severity issue where this change could lead to regressions, as it excludes valid GeoNode-specific and extension-based permissions like add_service or delete_service, potentially disabling UI actions for authorized users. It is recommended to either expand the whitelist or implement a more inclusive filtering mechanism.
| if codename in PERMISSIONS: | ||
| # return constant names defined by GeoNode | ||
| perms.add(PERMISSIONS[codename]) |
There was a problem hiding this comment.
The removal of the else block restricts global permissions to only those explicitly defined in the PERMISSIONS map (which currently only contains add_resourcebase). This will cause other GeoNode-specific global permissions that do not require mapping, such as add_service, delete_service, or any custom permissions from extensions, to be excluded from the global permissions payload.
While this successfully reduces the payload size by filtering out Django/Guardian internal permissions, it may lead to regressions in the UI where authorized users (including admins) can no longer see buttons or perform actions that depend on these global permissions. Consider using a more inclusive filter (e.g., based on app labels) or expanding the PERMISSIONS whitelist in geonode/security/permissions.py to include all relevant GeoNode global permissions.
There was a problem hiding this comment.
/gemoni A solution could be to add a method to register new permissions inside the list of global permissions handled by the PermissionsRegistry.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #14263 +/- ##
==========================================
- Coverage 74.92% 74.92% -0.01%
==========================================
Files 975 975
Lines 59902 59901 -1
Branches 8157 8157
==========================================
- Hits 44884 44883 -1
Misses 13194 13194
Partials 1824 1824 🚀 New features to boost your workflow:
|
Fixes #14262
Checklist
For all pull requests:
The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):
Submitting the PR does not require you to check all items, but by the time it gets merged, they should be either satisfied or inapplicable.