[Fixes #14117] Implement the option to configure the default permissions for registered members and add for anonymous default#14122
[Fixes #14117] Implement the option to configure the default permissions for registered members and add for anonymous default#14122
Conversation
…ered members and add for annonymous default
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a compact permissions system for default groups, deprecating the legacy boolean settings for anonymous view and download permissions. It implements the DEFAULT_ANONYMOUS_PERMISSIONS and DEFAULT_REGISTERED_MEMBERS_PERMISSIONS settings and adds a new DefaultSpecialGroupsPermissionsHandler to automate permission assignment during resource creation. The changes span security handlers, models, and the Geoserver manager to ensure integration and backward compatibility, supported by new unit tests. I have no feedback to provide.
giohappy
changed the title
giohappy
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #14122 +/- ##
==========================================
+ Coverage 74.55% 74.58% +0.03%
==========================================
Files 958 958
Lines 58043 58151 +108
Branches 7920 7934 +14
==========================================
+ Hits 43276 43374 +98
- Misses 13000 13005 +5
- Partials 1767 1772 +5 🚀 New features to boost your workflow:
|
| # - DEFAULT_ANONYMOUS_PERMISSIONS: view | download | none | ||
| # - DEFAULT_REGISTERED_MEMBERS_PERMISSIONS: view | download | edit | manage | none | ||
| DEFAULT_ANONYMOUS_PERMISSIONS = os.getenv("DEFAULT_ANONYMOUS_PERMISSIONS", None) | ||
| DEFAULT_REGISTERED_MEMBERS_PERMISSIONS = os.getenv("DEFAULT_REGISTERED_MEMBERS_PERMISSIONS", None) |
There was a problem hiding this comment.
If those two variables are the new default, i think the default value should not be None, but what we want to be the new default like download
|
|
||
| # Anonymous | ||
| if settings.DEFAULT_ANONYMOUS_VIEW_PERMISSION: | ||
| anonymous_compact = get_default_anonymous_compact_permission() |
There was a problem hiding this comment.
I think this part should not be longer required. Since we have an handler in the permissions_registry, the permissions should be already internally managed without the need to re-write again the check. This is valid also in other parts of the code
| exist_geolimits = exist_geolimits or has_geolimits(_resource, None, None) | ||
|
|
||
| if settings.DEFAULT_ANONYMOUS_DOWNLOAD_PERMISSION: | ||
| if anonymous_compact == DOWNLOAD_RIGHTS: |
There was a problem hiding this comment.
I think this part should not be longer required. Since we have an handler in the permissions_registry, the permissions should be already internally managed without the need to re-write again the check. This is valid also in other parts of the code
| # Anonymous | ||
| anonymous_can_view = settings.DEFAULT_ANONYMOUS_VIEW_PERMISSION | ||
| anonymous_compact = get_default_anonymous_compact_permission() | ||
| anonymous_can_view = anonymous_compact == VIEW_RIGHTS |
There was a problem hiding this comment.
I think this part should not be longer required. Since we have an handler in the permissions_registry, the permissions should be already internally managed without the need to re-write again the check. This is valid also in other parts of the code
| perm_spec["groups"][user_group] = ["view_resourcebase"] | ||
|
|
||
| anonymous_can_download = settings.DEFAULT_ANONYMOUS_DOWNLOAD_PERMISSION | ||
| anonymous_can_download = anonymous_compact == DOWNLOAD_RIGHTS |
There was a problem hiding this comment.
I think this part should not be longer required. Since we have an handler in the permissions_registry, the permissions should be already internally managed without the need to re-write again the check. This is valid also in other parts of the code
| @staticmethod | ||
| def is_anonymous_can_view(): | ||
| return settings.DEFAULT_ANONYMOUS_VIEW_PERMISSION | ||
| return get_default_anonymous_compact_permission() in (VIEW_RIGHTS, DOWNLOAD_RIGHTS) |
There was a problem hiding this comment.
maybe here we should use the permissions registry? the permisisons for anonymous should be mananaged already there
| @staticmethod | ||
| def is_anonymous_can_download(): | ||
| return settings.DEFAULT_ANONYMOUS_DOWNLOAD_PERMISSION | ||
| return get_default_anonymous_compact_permission() == DOWNLOAD_RIGHTS |
There was a problem hiding this comment.
maybe here we should use the permissions registry? the permisisons for anonymous should be mananaged already there
3 participants
Fixes #14117
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.