geonode
geonode copied to clipboard
Create moderators group and permissions for publishing and approval
Expected Behavior
We want to introduce the moderators
group, which will be used as a "role" like we have done for contributors.
Two new permissions approve_resourcebase and publish_resorcebase will be created and assigned to the moderators
group.
Users assigned to this group will have the permissions to publish and/or approve any resource.
Documentation for the advanced workflow must be updated accordingly.
Actual Behavior
The current advanced workflow system rely on admins and group managers to grant permissions for the approval and publishing of resources (approve
and publish
metadata flags).
Steps to Reproduce the Problem
Specifications
- GeoNode version: master
- Installation method (manual, GeoNode Docker, SPCGeoNode Docker):
- Platform:
- Additional details:
I report below the description from our own internal task.
The idea is to centralize the permission checks for the advanced workflow under the security module, and check wether a user has publishing / moderating permissions.
This method to check the moderator permissions will return true if the user:
- is and admin
- has metadata edit permissions on the specific resource (this happens for admins and group managers)
- has the
approve_resources
permission
Something similar for the check of permissions to publish. User can publish if:
- is an admin
- has
publish_resources
permissions - he has the (already implemented)
publish_resourcebase
permission on the resource. This resource specific permission could probably be dropped since, AFAIK, only an admin can publish ifADMIN_MODERATE_UPLOADS
is active.
Long story short, we want to continue the work of basing the security logic on groups and permissions, rather then specific users'attributes (group manager, memeber of a group, etc.).
Some highlights of the critical points where the logic has been applied:
At client side there are few places where the legacy views
disable the form checkboxes
accordingly, e.g.:
NOTE: a new issue will be opened afterwards to make the REST API v2 aware of this logic, and drive the client functionalities accordingly. The client shouldn't be aweare of the underlying workflows. The backend will send back the user permissions based on the underlying logic. So, for example, id the resource has been approved (and the advanced workflow is active) the returned permissions on the resource for any other user shouldn't include the edit permissions.
@giohappy, Given the advanced workflow does this mean, that on creating the moderators
group, should we just assign them only the approve_resources
perm?
Some clarifications. The steps are
- create the new
moderators
group, create the newapprove_resources
andpublish_resources
and assign these permissions to the new group. - create a new
approve_resourcebase
permissions, along with the already existingpublish_resourcebase
permission - Adapt the security.utils methods to keep into account moderators when returning the uer permissions on a resource (see below)
- Adapt GeoNode views where the advanced workflows settings are checked to enable / disable the
approves
andis published
fields (see below)
"Virtual" permissions
When permissions are retrieved for a resource (get_visible_resources, get_resources_with_perms, get_users_with_perms, resolve_object, and so forth), the final permissions will be returned either if the user has those permissions directly assigned or if the user has global permissions that apply to the case.
Currently the group manager of the group assigned to the resource has change_resourcebase_metadata
assigned. This assignment is triggered by changes to the group's manager or the group assigned to the resource.
So, permissions for this user are returned automatically when we check the user permissions on a resource.
On the other side, the new moderators won't be assigned direct permissions on resources. They will get "virtual" permissions from being moderators. This means that the methods that rely on permissions (spread all over GeoNode!) should determine and return not only the "real" permissions of a user but also "virtual" permissions. This is one of the reasons we want to remove security code duplication the most as possible, and move it under the security module.
As an example, when we retriev the permissions for a resource we should add the following case:
If user has approve_resources
or user has publish_resources
, for any resource the following permissions will be returned:
-
view_resourcebase
-
download_resourcebase
-
change_resoucebase_metadata
-
approve_resourcebase
-
publish_resourcebase
Notice that if we implement this approach we gen get ridd of the assignment of real permissions for the group managers.
@afabiani please give your opinion on this, since it goes hand in hand of our recent discussion on a similar topic for compact permissions.
Can approve and can publish conditions
In the end the following conditions must be implemented:
if the user is "group manager for the resources" or user.perms.get
("approve_resources") or is admin:
-> return approve_resourcebase
for the user
user.perms.get
("publish_resources") or is admin:
-> return publish_resourcebase
for the user
User can_approve and can_publish
We want to have a new people.Profile.can_approve
and people.Profile.can_publish
methods (which will rely on the security utils) to check if the user
{resourcetype}_metadata views check wether to make the is_published and approved checkboxes disabled or not, dependeing on the advanced workflow settings and the user permissions. We want to remove this duplication of code and move the checks inside two new people.Profile.can_approve
and people.Profile.can_publish
methods, that will rely on the security utils.
For example, the following lines https://github.com/GeoNode/geonode/blob/9a1784071ffc65a98ab19019b172564dff8a5b2e/geonode/layers/views.py?plain=1#L1116-L1131
if settings.ADMIN_MODERATE_UPLOADS:
if not request.user.is_superuser:
can_change_metadata = request.user.has_perm(
'change_resourcebase_metadata',
layer.get_self_resource())
try:
is_manager = request.user.groupmember_set.all().filter(role='manager').exists()
except Exception:
is_manager = False
if not is_manager or not can_change_metadata:
if settings.RESOURCE_PUBLISHING:
dataset_form.fields['is_published'].widget.attrs.update(
{'disabled': 'true'})
dataset_form.fields['is_approved'].widget.attrs.update(
{'disabled': 'true'})
shoild become something like this:
if not user.can_appprove(resource):
if not user.can_publish(resource):
dataset_form.fields['is_published'].widget.attrs.update(
{'disabled': 'true'})
dataset_form.fields['is_approved'].widget.attrs.update(
{'disabled': 'true'})
@giohappy I agree with your analysis.
I also want to highlight that part of this task will be to improve and update the set_workflow_perms
paths accordingly, in order to recognize the Group Managers
and assign them to the correct Authority Groups
. We should also remove them whenever they are demoted
.
ResourceBase
inherits the set_workflow_perms
method from the Security model
https://github.com/GeoNode/geonode/blob/837da7c218d22251e1ba30ddbcc5b0160ef1fbbf/geonode/security/models.py#L144
The latter completely relies on the ResourceManager
which:
-
Checks for the managers and assigns the permissions accordingly https://github.com/GeoNode/geonode/blob/837da7c218d22251e1ba30ddbcc5b0160ef1fbbf/geonode/resource/manager.py#L700
-
Invokes the
Concrete Resource Manager
in order to finalize the process (in this case sets the GeoServer rules) https://github.com/GeoNode/geonode/blob/837da7c218d22251e1ba30ddbcc5b0160ef1fbbf/geonode/geoserver/manager.py#L507
@afabiani on second thouth I have a doubt regarding "moderators". In my plan users having this role (Django Group assigned) will should have the following permissions:
-
view_resourcebase
-
download_resourcebase
-
change_resoucebase_metadata
-
approve_resourcebase
-
publish_resourcebase
The idea was to make these permissions virtual, or dynamic, instead of actually assigning the permissions on the resources to the single users. By the way this implies that Geofence rules for those Django groups should be managed accordingly. Questions:
- does GeoNode return Django Groups (not GeoNode GroupProfiles!) to the Geoserver Role service?
- how complex would it be to manage (GeoNode side) the rules for Django Groups?
@giohappy currently the GeoFence rules relies already on Django Group slugs, not on GeoNode GroupProfiles.