Add back Map.edit_status

Revert "Fix existing permissions related tests"

This reverts commit 36d7d87301c54a1a40bc6bbc164120768b258fae.

WIP
This commit is contained in:
Yohan Boniface 2023-09-18 20:05:17 +02:00
parent 3dc4efe7b1
commit 3cbd6cca40
10 changed files with 146 additions and 36 deletions

View file

@ -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

View file

@ -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):

View file

@ -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

View file

@ -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)

View file

@ -5,7 +5,7 @@
<tr>
<th>{% blocktrans %}Map{% endblocktrans %}</th>
<th>{% blocktrans %}Name{% endblocktrans %}</th>
<th>{% blocktrans %}Who can see{% endblocktrans %}</th>
<th>{% blocktrans %}Who can see / edit{% endblocktrans %}</th>
<th>{% blocktrans %}Last save{% endblocktrans %}</th>
<th>{% blocktrans %}Owner{% endblocktrans %}</th>
<th>{% blocktrans %}Actions{% endblocktrans %}</th>
@ -19,7 +19,7 @@
<td>
<a href="{{ map_inst.get_absolute_url }}">{{ map_inst.name }}</a>
</td>
<td>{{ map_inst.get_share_status_display }}</td>
<td>{{ map_inst.get_share_status_display }} / {{ map_inst.get_edit_status_display }}</td>
<td>{{ map_inst.modified_at }}</td>
<td>
<a href="{{ map_inst.owner.get_url }}">{{ map_inst.owner }}</a>

View file

@ -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)

View file

@ -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):

View file

@ -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))

View file

@ -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

View file

@ -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