Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clone option in the list view is missing on Maps for non admin users #1308

Closed
marthamareal opened this issue Nov 16, 2022 · 9 comments
Closed
Assignees
Milestone

Comments

@marthamareal
Copy link

marthamareal commented Nov 16, 2022

Expected behavior

Users should be able to clone Maps from the list view.

Actual behavior

Only admins can see the clone option on maps in the list view

@DavidQuartz
Copy link
Member

DavidQuartz commented Nov 28, 2022

We have 2 requirements for the clone action, which are the is_copyable prop and download_resourcebase perm. But somehow for maps, only admins have the download_resourcebase perm on them so this check doesn't pass for normal users.

It seems the permissions for maps need to be checked for normal users on the server side, if the behaviour is not expected @marthamareal @giohappy

@giohappy
Copy link

@mattiagiupponi can you take a look at this?

@mattiagiupponi
Copy link

mattiagiupponi commented Nov 29, 2022

@giohappy
The permission is missing because map as a subtype is not considered as DOWNLOADABLE_RESOURCES saying that:

elif AdvancedSecurityWorkflowManager.assignable_perm_condition(perm, _resource_type):
    _safe_assign_perm(perm, anonymous_group, _resource.get_self_resource())

Internally the AdvancedSecurityWorkflowManager evaluate if the resource can have the "download_resourcebase" permissions if the resource is in the DOWNLOADABLE_RESOURCES list

_assignable_perm_policy_condition = (perm in DOWNLOAD_PERMISSIONS and resource_type in DOWNLOADABLE_RESOURCES) or \
            (perm in DATASET_EDIT_DATA_PERMISSIONS and resource_type in DATA_EDITABLE_RESOURCES_SUBTYPES) or \
            (perm not in (DOWNLOAD_PERMISSIONS + DATASET_EDIT_DATA_PERMISSIONS))

The map doesn't pass mainly because of the first if since map is not in the DOWNLOADABLE_RESOURCES so that the download_resourcebase permission is not assigned.

NOTE: we can add the map in the DOWNLOAD_PERMISSIONS but I guess this will enable other functionalities in the front end since the map cannot be downloaded. A suggestion could be to let a resource be cloned if the is_copyable is True and if the user has the add_resourcebase permission

@DavidQuartz
Copy link
Member

In that case, for admins, are all possible perms (which includes download_resourcebase) automatically returned for maps?
@mattiagiupponi

@giohappy
Copy link

@mattiagiupponi is_copyable and add_resourcebase are not enough to say if the user can clone the resource when it's a dataset or a documentsince for these types of resources we need to copy the underlying data, which can only be done only if you have grants to download it.
Of course, downloading is meaningless for the other resource types.

I suspect that a specific permission must be created to say if the user has permission to clone, which will be calculated differently based on the resource type. It might be copy_resourcebase?

By the way @DavidQuartz I see that the "Save as..." button is available inside a map. How is the visibility of this button calculated?

@mattiagiupponi
Copy link

mattiagiupponi commented Nov 29, 2022

In that case, for admins, are all possible perms (which includes download_resourcebase) automatically returned for maps? @mattiagiupponi

Yes, the admin will have all the permissions because it does not pass through that code

I suspect that specific permission must be created to say if the user has permission to clone, which will be calculated differently based on the resource type. Might it be copy_resourcebase?

@giohappy Can't it be easier to rely only on the is_copyable property? If we make the property the central point of reliability, is also easier to maintain (and test). Will be the property to calculate when the given resource is copyable or not

@giohappy
Copy link

giohappy commented Nov 29, 2022

@giohappy Can't it be easier to rely only on the is_copyable property? If we make the property the central point of reliability, is also easier to maintain (and test). Will be the property to calculate when the given resource is copyable or not

is_copyable is a property of the resource, not a permission. It only says that granted you have permission to do it, the resource can be copied. For example, it is false for remote datasets.

@DavidQuartz
Copy link
Member

By the way @DavidQuartz I see that the "Save as..." button is available inside a map. How is the visibility of this button calculated?

@giohappy the Save As plugin is allowed on all maps in map viewer. The permissions are only checked for datasets and documents as this issue required

@giohappy
Copy link

giohappy commented Dec 2, 2022

as agreed with @allyoucanmap for the moment we will let the client decide if download_resourcebase must be taken into account or not depending on the resource type.

  • it will be implemented on master according to the new catalog code
  • it will be fixed in 4.0.x

@allyoucanmap allyoucanmap reopened this Dec 2, 2022
@giohappy giohappy added this to the 4.0.2 milestone Dec 16, 2022
ridoo pushed a commit to Thuenen-GeoNode-Development/geonode-mapstore-client that referenced this issue Dec 21, 2022
ridoo pushed a commit to Thuenen-GeoNode-Development/geonode-mapstore-client that referenced this issue Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants