diff --git a/umap/decorators.py b/umap/decorators.py index 52faa127..b9b5232a 100644 --- a/umap/decorators.py +++ b/umap/decorators.py @@ -36,11 +36,12 @@ def can_edit_map(view_func): map_inst = get_object_or_404(Map, pk=kwargs["map_id"]) user = request.user kwargs["map_inst"] = map_inst # Avoid rerequesting the map in the view - can_edit = map_inst.can_edit(user=user, request=request) - if not can_edit: - if map_inst.owner and not user.is_authenticated: - return simple_json_response(login_required=str(LOGIN_URL)) - return HttpResponseForbidden() + if map_inst.edit_status >= map_inst.EDITORS: + can_edit = map_inst.can_edit(user=user, request=request) + if not can_edit: + if map_inst.owner and not user.is_authenticated: + return simple_json_response(login_required=str(LOGIN_URL)) + return HttpResponseForbidden() return view_func(request, *args, **kwargs) return wrapper diff --git a/umap/forms.py b/umap/forms.py index e6893690..1b024d18 100644 --- a/umap/forms.py +++ b/umap/forms.py @@ -36,7 +36,20 @@ class SendLinkForm(forms.Form): class UpdateMapPermissionsForm(forms.ModelForm): class Meta: model = Map - fields = ("editors", "share_status", "owner") + fields = ("edit_status", "editors", "share_status", "owner") + + +class AnonymousMapPermissionsForm(forms.ModelForm): + STATUS = ( + (Map.OWNER, _("Only editable with secret edit link")), + (Map.ANONYMOUS, _("Everyone can edit")), + ) + + edit_status = forms.ChoiceField(choices=STATUS) + + class Meta: + model = Map + fields = ("edit_status",) class DataLayerForm(forms.ModelForm): diff --git a/umap/models.py b/umap/models.py index 8de2105d..3f8848fa 100644 --- a/umap/models.py +++ b/umap/models.py @@ -221,12 +221,16 @@ class Map(NamedModel): In anononymous mode: only "anonymous owners" (having edit cookie set) """ can = False - if not self.owner: + if request and not self.owner: if settings.UMAP_ALLOW_ANONYMOUS and self.is_anonymous_owner(request): can = True + if self.edit_status == self.ANONYMOUS: + can = True + elif user is None: + can = False elif user == self.owner: can = True - elif user in self.editors.all(): + elif self.edit_status == self.EDITORS and user in self.editors.all(): can = True return can @@ -430,12 +434,16 @@ class DataLayer(NamedModel): Define if a user can edit or not the instance, according to his account or the request. """ - can = self.map.can_edit(user, request) - if can: - # Owner or editor, no need for further checks. - return can + can = False + if not self.map.owner: + if settings.UMAP_ALLOW_ANONYMOUS and self.map.is_anonymous_owner(request): + can = True if self.edit_status == self.ANONYMOUS: can = True + elif user is not None and user == self.map.owner: + can = True + elif self.edit_status == self.EDITORS and user in self.map.editors.all(): + can = True return can diff --git a/umap/static/umap/js/umap.permissions.js b/umap/static/umap/js/umap.permissions.js index a34b8167..45321db4 100644 --- a/umap/static/umap/js/umap.permissions.js +++ b/umap/static/umap/js/umap.permissions.js @@ -5,6 +5,7 @@ L.U.MapPermissions = L.Class.extend({ owner: null, editors: [], share_status: null, + edit_status: null, }, initialize: function (map) { @@ -64,9 +65,26 @@ L.U.MapPermissions = L.Class.extend({ this.options.anonymous_edit_url }` L.DomUtil.add('p', 'help-text', container, helpText) + fields.push([ + 'options.edit_status', + { + handler: 'IntSelect', + label: L._('Who can edit'), + selectOptions: this.map.options.edit_statuses, + helpText: helpText, + }, + ]) } } else { if (this.isOwner()) { + fields.push([ + 'options.edit_status', + { + handler: 'IntSelect', + label: L._('Who can edit'), + selectOptions: this.map.options.edit_statuses, + }, + ]) fields.push([ 'options.share_status', { @@ -136,6 +154,8 @@ L.U.MapPermissions = L.Class.extend({ for (let i = 0; i < this.options.editors.length; i++) formData.append('editors', this.options.editors[i].id) } + if (this.isOwner() || this.isAnonymousMap()) + formData.append('edit_status', this.options.edit_status) if (this.isOwner()) { formData.append('owner', this.options.owner && this.options.owner.id) formData.append('share_status', this.options.share_status) diff --git a/umap/templates/umap/map_table.html b/umap/templates/umap/map_table.html index 2150838b..eb8cef5a 100644 --- a/umap/templates/umap/map_table.html +++ b/umap/templates/umap/map_table.html @@ -5,7 +5,7 @@ {% blocktrans %}Map{% endblocktrans %} {% blocktrans %}Name{% endblocktrans %} - {% blocktrans %}Who can see{% endblocktrans %} + {% blocktrans %}Who can see / edit{% endblocktrans %} {% blocktrans %}Last save{% endblocktrans %} {% blocktrans %}Owner{% endblocktrans %} {% blocktrans %}Actions{% endblocktrans %} @@ -19,7 +19,7 @@ {{ map_inst.name }} - {{ map_inst.get_share_status_display }} + {{ map_inst.get_share_status_display }} / {{ map_inst.get_edit_status_display }} {{ map_inst.modified_at }} {{ map_inst.owner }} diff --git a/umap/tests/integration/test_owned_map.py b/umap/tests/integration/test_owned_map.py index 061676f8..171b8ca8 100644 --- a/umap/tests/integration/test_owned_map.py +++ b/umap/tests/integration/test_owned_map.py @@ -3,7 +3,7 @@ from time import sleep import pytest from playwright.sync_api import expect -from umap.models import DataLayer +from umap.models import DataLayer, Map pytestmark = pytest.mark.django_db @@ -91,6 +91,7 @@ def test_owner_permissions_form(map, datalayer, live_server, login): def test_map_update_with_editor(map, live_server, login, user): + map.edit_status = Map.EDITORS map.editors.add(user) map.save() page = login(user) @@ -113,6 +114,7 @@ def test_map_update_with_editor(map, live_server, login, user): def test_permissions_form_with_editor(map, datalayer, live_server, login, user): + map.edit_status = Map.EDITORS map.editors.add(user) map.save() page = login(user) @@ -145,6 +147,7 @@ def test_owner_has_delete_map_button(map, live_server, login): def test_editor_do_not_have_delete_map_button(map, live_server, login, user): + map.edit_status = Map.EDITORS map.editors.add(user) map.save() page = login(user) diff --git a/umap/tests/test_map.py b/umap/tests/test_map.py index bc1283c3..d0bb52e7 100644 --- a/umap/tests/test_map.py +++ b/umap/tests/test_map.py @@ -9,33 +9,62 @@ from .base import MapFactory pytestmark = pytest.mark.django_db -def test_anonymous_cannot_edit(map): +def test_anonymous_can_edit_if_status_anonymous(map): anonymous = AnonymousUser() + map.edit_status = map.ANONYMOUS + map.save() + assert map.can_edit(anonymous) + + +def test_anonymous_cannot_edit_if_not_status_anonymous(map): + anonymous = AnonymousUser() + map.edit_status = map.OWNER + map.save() assert not map.can_edit(anonymous) -def test_non_editors_cannot_edit(map, user): +def test_non_editors_can_edit_if_status_anonymous(map, user): assert map.owner != user + map.edit_status = map.ANONYMOUS + map.save() + assert map.can_edit(user) + + +def test_non_editors_cannot_edit_if_not_status_anonymous(map, user): + map.edit_status = map.OWNER + map.save() assert not map.can_edit(user) -def test_editors_can_edit(map, user): +def test_editors_cannot_edit_if_status_owner(map, user): + map.edit_status = map.OWNER + map.editors.add(user) + map.save() + assert not map.can_edit(user) + + +def test_editors_can_edit_if_status_editors(map, user): + map.edit_status = map.EDITORS map.editors.add(user) map.save() assert map.can_edit(user) -def test_owner_can_edit(map): - assert map.can_edit(map.owner) - - -def test_logged_in_user_should_not_be_allowed_for_anonymous_map(map, user, rf): +def test_logged_in_user_should_be_allowed_for_anonymous_map_with_anonymous_edit_status(map, user, rf): # noqa map.owner = None + map.edit_status = map.ANONYMOUS map.save() url = reverse('map_update', kwargs={'map_id': map.pk}) request = rf.get(url) request.user = user - assert not map.can_edit(user, request) + assert map.can_edit(user, request) + + +def test_anonymous_user_should_not_be_allowed_for_anonymous_map(map, user, rf): # noqa + map.owner = None + map.edit_status = map.OWNER + map.save() + assert not map.can_edit() def test_clone_should_return_new_instance(map, user): diff --git a/umap/tests/test_map_views.py b/umap/tests/test_map_views.py index a0f4bdcf..01e78c59 100644 --- a/umap/tests/test_map_views.py +++ b/umap/tests/test_map_views.py @@ -38,6 +38,7 @@ def test_create(client, user, post_data): assert created_map.center.x == 13.447265624999998 assert created_map.center.y == 48.94415123418794 assert j["permissions"] == { + "edit_status": 3, "share_status": 1, "owner": {"id": user.pk, "name": "Joe", "url": "/en/user/Joe/"}, "editors": [], @@ -166,20 +167,24 @@ def test_user_not_allowed_should_not_clone_map(client, map, user, settings): settings.UMAP_ALLOW_ANONYMOUS = False assert Map.objects.count() == 1 url = reverse("map_clone", kwargs={"map_id": map.pk}) + map.edit_status = map.OWNER + map.save() response = client.post(url) assert login_required(response) client.login(username=user.username, password="123123") response = client.post(url) assert response.status_code == 403 + map.edit_status = map.ANONYMOUS + map.save() client.logout() response = client.post(url) - assert response.status_code == 200 - assert "login_required" in json.loads(response.content) + assert response.status_code == 403 assert Map.objects.count() == 1 def test_clone_should_set_cloner_as_owner(client, map, user): url = reverse("map_clone", kwargs={"map_id": map.pk}) + map.edit_status = map.EDITORS map.editors.add(user) map.save() client.login(username=user.username, password="123123") @@ -297,19 +302,19 @@ def test_only_owner_can_delete(client, map, user): assert response.status_code == 403 -def test_map_editors_cannot_change_owner(client, map, user): - owner = map.owner +def test_map_editors_do_not_see_owner_change_input(client, map, user): map.editors.add(user) + map.edit_status = map.EDITORS map.save() url = reverse("map_update_permissions", kwargs={"map_id": map.pk}) client.login(username=user.username, password="123123") - response = client.post(url, data={"owner": user.pk}) - assert response.status_code == 200 - assert map.owner == owner + response = client.get(url) + assert "id_owner" not in response -def test_logged_in_user_cannot_edit_map(client, map, user): +def test_logged_in_user_can_edit_map_editable_by_anonymous(client, map, user): map.owner = None + map.edit_status = map.ANONYMOUS map.save() client.login(username=user.username, password="123123") url = reverse("map_update", kwargs={"map_id": map.pk}) @@ -319,7 +324,8 @@ def test_logged_in_user_cannot_edit_map(client, map, user): "name": new_name, } response = client.post(url, data) - assert response.status_code == 403 + assert response.status_code == 200 + assert Map.objects.get(pk=map.pk).name == new_name @pytest.mark.usefixtures("allow_anonymous") @@ -416,9 +422,13 @@ def test_bad_anonymous_edit_url_should_return_403(cookieclient, anonymap): @pytest.mark.usefixtures("allow_anonymous") -def test_clone_anonymous_map_should_not_be_possible(client, anonymap, user): # noqa +def test_clone_anonymous_map_should_not_be_possible_if_user_is_not_allowed( + client, anonymap, user +): # noqa assert Map.objects.count() == 1 url = reverse("map_clone", kwargs={"map_id": anonymap.pk}) + anonymap.edit_status = anonymap.OWNER + anonymap.save() response = client.post(url) assert response.status_code == 403 client.login(username=user.username, password="123123") @@ -427,6 +437,23 @@ def test_clone_anonymous_map_should_not_be_possible(client, anonymap, user): # assert Map.objects.count() == 1 +@pytest.mark.usefixtures("allow_anonymous") +def test_clone_map_should_be_possible_if_edit_status_is_anonymous( + client, anonymap +): # noqa + assert Map.objects.count() == 1 + url = reverse("map_clone", kwargs={"map_id": anonymap.pk}) + anonymap.edit_status = anonymap.ANONYMOUS + anonymap.save() + response = client.post(url) + assert response.status_code == 200 + assert Map.objects.count() == 2 + clone = Map.objects.latest("pk") + assert clone.pk != anonymap.pk + assert clone.name == "Clone of " + anonymap.name + assert clone.owner is None + + @pytest.mark.usefixtures("allow_anonymous") def test_anyone_can_access_anonymous_map(cookieclient, anonymap): url = reverse("map", args=(anonymap.slug, anonymap.pk)) diff --git a/umap/tests/test_views.py b/umap/tests/test_views.py index df96042a..e53d2230 100644 --- a/umap/tests/test_views.py +++ b/umap/tests/test_views.py @@ -282,6 +282,7 @@ def test_user_dashboard_display_user_maps(client, map): assert f"{map.get_absolute_url()}?share" in body assert f"{map.get_absolute_url()}?download" in body assert "Everyone (public)" in body + assert "Owner only" in body @pytest.mark.django_db diff --git a/umap/views.py b/umap/views.py index e58d75ee..f7fe3327 100644 --- a/umap/views.py +++ b/umap/views.py @@ -48,6 +48,7 @@ from .forms import ( DataLayerForm, DataLayerPermissionsForm, AnonymousDataLayerPermissionsForm, + AnonymousMapPermissionsForm, FlatErrorList, MapSettingsForm, SendLinkForm, @@ -526,6 +527,7 @@ class MapDetailMixin: class PermissionsMixin: def get_permissions(self): permissions = {} + permissions["edit_status"] = self.object.edit_status permissions["share_status"] = self.object.share_status if self.object.owner: permissions["owner"] = { @@ -563,7 +565,7 @@ class MapView(MapDetailMixin, PermissionsMixin, DetailView): @property def edit_mode(self): - edit_mode = 'disabled' + edit_mode = "disabled" if self.object.can_edit(self.request.user, self.request): edit_mode = "advanced" elif any( @@ -655,12 +657,18 @@ class MapUpdate(FormLessEditMixin, PermissionsMixin, UpdateView): class UpdateMapPermissions(FormLessEditMixin, UpdateView): model = Map pk_url_kwarg = "map_id" - form_class = UpdateMapPermissionsForm + + def get_form_class(self): + if self.object.owner: + return UpdateMapPermissionsForm + else: + return AnonymousMapPermissionsForm def get_form(self, form_class=None): form = super().get_form(form_class) user = self.request.user if self.object.owner and not user == self.object.owner: + del form.fields["edit_status"] del form.fields["share_status"] del form.fields["owner"] return form