[Fixes #7718] Permissions assignments on Remote Services

[mattiagiupponi]

Basic implementation of permissions on remote services

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

For all pull requests:

• Confirm you have read the contribution guidelines
• You have sent a Contribution Licence Agreement (CLA) as necessary (not required for small changes, e.g., fixing typos in the documentation)
• Make sure the first PR targets the master branch, eventual backports will be managed later. This can be ignored if the PR is fixing an issue that only happens in a specific branch, but not in newer ones.

The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):

• There is a ticket in https://github.com/GeoNode/geonode/issues describing the issue/improvement/feature (a notable exemption is, changes not visible to end-users)
• The issue connected to the PR must have Labels and Milestone assigned
• PR for bug fixes and small new features are presented as a single commit
• Commit message must be in the form "[Fixes #<issue_number>] Title of the Issue"
• New unit tests have been added covering the changes, unless there is an explanation on why the tests are not necessary/implemented
• This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
• This PR passes the QA checks: flake8 geonode
• Commits changing the settings, UI, existing user workflows, or adding new functionality, need to include documentation updates
• Commits adding new texts do use gettext and have updated .po / .mo files (without location infos)

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.

[giohappy]

@mattiagiupponi

• we should remove the visibily to Anonymous users when a new Service is created, otherwise any user can see other's services.
• "Add Remote Service" button is not available anymore. It ust be restored for any registered user (contributor).

[afabiani]

There are PEP-8 issues

^@^@geonode/services/views.py:104:1: W293 blank line contains whitespace

[giohappy]

@mattiagiupponi unfortunately it looks there's still something to be fixed.
My normal user doesn't see the "Add Remote Service" button yet. I guess it's because the contributors group doesn't have the requested permission assigned. I guess we need a fixture that assign the permission to the group if it doesn't exist.

[mattiagiupponi]

@giohappy The issue was caused by the permissions used to understand if an user can add a resource or not.
I used the base.add_resourcebase permission, while by default the contributors has the auth.addresourcebase permission (the custom created by us). The PR now contains the check with the auth.addresourcebase, please let me know if we need to use the base.add_resourcebase and add it to the group by default or use the custom permission auth.addresourcebase

@afabiani pep8 issue fixed

[giohappy]

@mattiagiupponi as agreed we're going to revert the creation of the custom base_addresourcebase permission done in #7364.
We can simply use the default add_resourcebase permission created automatically by Django.

[mattiagiupponi]

Unfortunately the docker containers do not start anymore. Most probably this is caused by wrong migrations. We will need to fix this before merging.

Migration fixed, I thought that the Django default permissions were applied before the other app migrations. Now should work

[afabiani]

@giohappy @mattiagiupponi I was testing the PR... I'm not sure if this could be a regression or it is something still not implemented, but:

• By creating a new user and removing it from the "contributors" group, shouldn't be supposed to be unable to create new resources at all?
  

I currently see it not being able to create new Remote Services only.

Can you please provide more details on that? About the rest it looks good to me.

[afabiani]

What's exactly the meaning of the perm add_resource_from_service? As a contributor I cannot see the existing Remote Services

Moreover, if I try to create a Remote Service with the same URL I'll get and error

[mattiagiupponi]

Well as far as I see, looks like more something that is not implemented yet.
Looking at the code, the Upload Layer button is enabled if the user is logged in. There are no checks regarding any kind of group.

geonode/geonode/templates/base.html

Lines 300 to 307 in 1fdfb47

	{% if user.is_authenticated and not READ_ONLY_MODE %}
	<li role="separator" class="divider"></li>
	<li><a href="{% url "layer_upload" %}">{% trans "Upload Layer" %}</a></li>
	{% if USE_GEOSERVER and CREATE_LAYER %}
	<li><a href="{% url "layer_create" %}">{% trans "Create Layer" %}</a></li>
	{% endif %}
	<li><a href="{% url "document_upload" %}">{% trans "Upload Document" %}</a></li>
	{% endif %}

geonode/geonode/layers/templates/layers/layer_list_default.html

Lines 11 to 13 in 1fdfb47

	{% if user.is_authenticated and not READ_ONLY_MODE %}
	<a href="{% url "layer_upload" %}" class="btn btn-primary pull-right">{% trans "Upload Layers" %}</a>
	{% endif %}

You do not see the remote service only because you are not logged in as a superuser or staff member:

geonode/geonode/templates/base.html

Lines 308 to 312 in 1fdfb47

	{% if user.is_superuser or user.is_staff %}
	{% if not READ_ONLY_MODE %}
	<li><a href="{% url "register_service" %}">{% trans "Add Remote Service" %}</a></li>
	{% endif %}
	{% endif %}

@giohappy Maybe it is worth taking a look at all the templates and finding a common way to handle them.

About add_resource_from_service is meant to let the user import resources once the service is registered (you have to click on it and you will see a button named "import resources" (or similar)

And regarding that as a contributor you are not seeing the services, was requested that only the owner of the resource should be able to see them (as admin you can see all the resources).
Ad there was already a check that will raise an error if you try to register the same service 2 times:

geonode/geonode/services/forms.py

Lines 67 to 74 in 1fdfb47

	proposed_url = self.cleaned_data["url"]
	existing = Service.objects.filter(base_url=proposed_url).exists()
	if existing:
	raise ValidationError(
	_("Service %(url)s is already registered"),
	params={"url": proposed_url}
	)
	return proposed_url

[afabiani]

But what's the meaning of creating a Remote Service that is visible only to the owner and than prevent him to add resources?

[afabiani]

@mattiagiupponi can you please check if this error https://app.circleci.com/pipelines/github/GeoNode/geonode/3426/workflows/01b1c1e1-7c1f-4076-9940-f0ea7f827a8d/jobs/10259 is related to the PR?

[mattiagiupponi]

@mattiagiupponi can you please check if this error https://app.circleci.com/pipelines/github/GeoNode/geonode/3426/workflows/01b1c1e1-7c1f-4076-9940-f0ea7f827a8d/jobs/10259 is related to the PR?

i don't know, but I'm gonna check it

[mattiagiupponi]

@mattiagiupponi can you please check if this error https://app.circleci.com/pipelines/github/GeoNode/geonode/3426/workflows/01b1c1e1-7c1f-4076-9940-f0ea7f827a8d/jobs/10259 is related to the PR?

I made some researches and looks like that was a bug in Django:
https://code.djangoproject.com/ticket/10827

Looks like that the solution is:

If I move 'django.contrib.contenttypes' before 'django.contrib.auth' in INSTALLED_APPS, it works.

Or drop the actual DB and create a new one.
@afabiani I think that we can try by moving the installed apps before dropping the DB, do you agree?

[giohappy]

https://github.com/geosolutions-it/C183-ALMAVIVA-2020-GeoNode-DEV/issues/251#issuecomment-872849224

[afabiani]

@mattiagiupponi can you please check if this error https://app.circleci.com/pipelines/github/GeoNode/geonode/3426/workflows/01b1c1e1-7c1f-4076-9940-f0ea7f827a8d/jobs/10259 is related to the PR?

I made some researches and looks like that was a bug in Django:
https://code.djangoproject.com/ticket/10827

Looks like that the solution is:

If I move 'django.contrib.contenttypes' before 'django.contrib.auth' in INSTALLED_APPS, it works.

Or drop the actual DB and create a new one.
@afabiani I think that we can try by moving the installed apps before dropping the DB, do you agree?

Oh wow!
Ok... let's move the 'django.contrib.contenttypes' before 'django.contrib.auth' in INSTALLED_APPS then!

[afabiani]

Almost good, just a small issue.
The counter of the ingested resources still counts all of them

[mattiagiupponi]

Almost good, just a small issue.
The counter of the ingested resources still counts all of them

Fixed :)