diff --git a/umap/decorators.py b/umap/decorators.py index 42b08d5f..5e808d12 100644 --- a/umap/decorators.py +++ b/umap/decorators.py @@ -33,12 +33,11 @@ def can_edit_map(view_func): @wraps(view_func) def wrapper(request, *args, **kwargs): 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 if map_inst.edit_status >= map_inst.COLLABORATORS: - can_edit = map_inst.can_edit(user=user, request=request) + can_edit = map_inst.can_edit(request=request) if not can_edit: - if map_inst.owner and not user.is_authenticated: + if map_inst.owner and not request.user.is_authenticated: return simple_json_response(login_required=str(LOGIN_URL)) return HttpResponseForbidden() return view_func(request, *args, **kwargs) diff --git a/umap/models.py b/umap/models.py index f3b03d56..93caacec 100644 --- a/umap/models.py +++ b/umap/models.py @@ -281,8 +281,10 @@ class Map(NamedModel): def get_author(self): return self.team or self.owner - def is_owner(self, user=None, request=None): - if user and self.owner == user: + def is_owner(self, request=None): + if not request: + return False + if request.user and self.owner == request.user: return True return self.is_anonymous_owner(request) @@ -297,14 +299,16 @@ class Map(NamedModel): has_anonymous_cookie = False return has_anonymous_cookie - def can_delete(self, user=None, request=None): - if self.owner and user != self.owner: + def can_delete(self, request=None): + if not request: + return False + if self.owner and request.user != self.owner: return False if not self.owner and not self.is_anonymous_owner(request): return False return True - def can_edit(self, user=None, request=None): + def can_edit(self, request=None): """ Define if a user can edit or not the instance, according to his account or the request. @@ -318,12 +322,18 @@ class Map(NamedModel): - anyone otherwise (ANONYMOUS) """ can = False - 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: + if not request: + return False + user = request.user + if ( + not self.owner + and settings.UMAP_ALLOW_ANONYMOUS + and self.is_anonymous_owner(request) + ): can = True - elif user is None: + elif self.edit_status == self.ANONYMOUS: + can = True + elif not user.is_authenticated: can = False elif user == self.owner: can = True @@ -477,7 +487,7 @@ class DataLayer(NamedModel): path.append(str(self.map.pk)) return os.path.join(*path) - def metadata(self, user=None, request=None): + def metadata(self, request=None): # Retrocompat: minimal settings for maps not saved after settings property # has been introduced obj = self.settings or { @@ -488,7 +498,7 @@ class DataLayer(NamedModel): obj["old_id"] = self.old_id obj["id"] = self.pk obj["permissions"] = {"edit_status": self.edit_status} - obj["editMode"] = "advanced" if self.can_edit(user, request) else "disabled" + obj["editMode"] = "advanced" if self.can_edit(request) else "disabled" return obj def clone(self, map_inst=None): @@ -557,22 +567,25 @@ class DataLayer(NamedModel): if name.startswith(prefixes) and name.endswith(".gz"): self.geojson.storage.delete(os.path.join(root, name)) - def can_edit(self, user=None, request=None): + def can_edit(self, request=None): """ Define if a user can edit or not the instance, according to his account or the request. """ if self.edit_status == self.INHERIT: - return self.map.can_edit(user, request) + return self.map.can_edit(request) can = False + if not request: + return False + user = request.user 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: + elif user.is_authenticated and user == self.map.owner: can = True - elif user is not None and self.edit_status == self.COLLABORATORS: + elif user.is_authenticated and self.edit_status == self.COLLABORATORS: if user in self.map.editors.all() or self.map.team in user.teams.all(): can = True return can diff --git a/umap/templatetags/umap_tags.py b/umap/templatetags/umap_tags.py index 9e776637..367eef47 100644 --- a/umap/templatetags/umap_tags.py +++ b/umap/templatetags/umap_tags.py @@ -45,7 +45,7 @@ def tilelayer_preview(tilelayer): @register.filter def can_delete_map(map, request): - return map.can_delete(request.user, request) + return map.can_delete(request) @register.filter diff --git a/umap/tests/conftest.py b/umap/tests/conftest.py index bbf36927..f99327b4 100644 --- a/umap/tests/conftest.py +++ b/umap/tests/conftest.py @@ -90,3 +90,8 @@ def datalayer(map): @pytest.fixture def tilelayer(): return TileLayerFactory() + + +@pytest.fixture +def fake_request(rf): + return rf.get("/") diff --git a/umap/tests/test_datalayer.py b/umap/tests/test_datalayer.py index 99b0f5bb..bee1bcd8 100644 --- a/umap/tests/test_datalayer.py +++ b/umap/tests/test_datalayer.py @@ -1,7 +1,7 @@ -import os from pathlib import Path import pytest +from django.contrib.auth.models import AnonymousUser from django.core.files.base import ContentFile from umap.models import DataLayer, Map @@ -100,28 +100,33 @@ def test_should_remove_old_versions_on_save(map, settings): assert names == [Path(datalayer.geojson.name).name, newer, medium] -def test_anonymous_cannot_edit_in_editors_mode(datalayer): +def test_anonymous_cannot_edit_in_editors_mode(datalayer, fake_request): datalayer.edit_status = DataLayer.COLLABORATORS datalayer.save() - assert not datalayer.can_edit() + fake_request.user = AnonymousUser() + assert not datalayer.can_edit(fake_request) -def test_owner_can_edit_in_editors_mode(datalayer, user): +def test_owner_can_edit_in_editors_mode(datalayer, user, fake_request): datalayer.edit_status = DataLayer.COLLABORATORS datalayer.save() - assert datalayer.can_edit(datalayer.map.owner) + fake_request.user = datalayer.map.owner + assert datalayer.can_edit(fake_request) -def test_editor_can_edit_in_collaborators_mode(datalayer, user): +def test_editor_can_edit_in_collaborators_mode(datalayer, user, fake_request): map = datalayer.map map.editors.add(user) map.save() datalayer.edit_status = DataLayer.COLLABORATORS datalayer.save() - assert datalayer.can_edit(user) + fake_request.user = user + assert datalayer.can_edit(fake_request) -def test_team_members_can_edit_in_collaborators_mode(datalayer, user, team): +def test_team_members_can_edit_in_collaborators_mode( + datalayer, user, team, fake_request +): user.teams.add(team) user.save() map = datalayer.map @@ -129,60 +134,69 @@ def test_team_members_can_edit_in_collaborators_mode(datalayer, user, team): map.save() datalayer.edit_status = DataLayer.COLLABORATORS datalayer.save() - assert datalayer.can_edit(user) + fake_request.user = user + assert datalayer.can_edit(fake_request) -def test_anonymous_can_edit_in_public_mode(datalayer): +def test_anonymous_can_edit_in_public_mode(datalayer, fake_request): datalayer.edit_status = DataLayer.ANONYMOUS datalayer.save() - assert datalayer.can_edit() + fake_request.user = AnonymousUser() + assert datalayer.can_edit(fake_request) -def test_owner_can_edit_in_public_mode(datalayer, user): +def test_owner_can_edit_in_public_mode(datalayer, user, fake_request): datalayer.edit_status = DataLayer.ANONYMOUS datalayer.save() - assert datalayer.can_edit(datalayer.map.owner) + fake_request.user = datalayer.map.owner + assert datalayer.can_edit(fake_request) -def test_editor_can_edit_in_public_mode(datalayer, user): +def test_editor_can_edit_in_public_mode(datalayer, user, fake_request): map = datalayer.map map.editors.add(user) map.save() datalayer.edit_status = DataLayer.ANONYMOUS datalayer.save() - assert datalayer.can_edit(user) + fake_request.user = user + assert datalayer.can_edit(fake_request) -def test_anonymous_cannot_edit_in_anonymous_owner_mode(datalayer): +def test_anonymous_cannot_edit_in_anonymous_owner_mode(datalayer, fake_request): datalayer.edit_status = DataLayer.OWNER datalayer.save() map = datalayer.map map.owner = None map.save() - assert not datalayer.can_edit() + fake_request.user = AnonymousUser() + assert not datalayer.can_edit(fake_request) -def test_owner_can_edit_in_inherit_mode_and_map_in_owner_mode(datalayer): +def test_owner_can_edit_in_inherit_mode_and_map_in_owner_mode(datalayer, fake_request): datalayer.edit_status = DataLayer.INHERIT datalayer.save() map = datalayer.map map.edit_status = Map.OWNER map.save() - assert datalayer.can_edit(map.owner) + fake_request.user = map.owner + assert datalayer.can_edit(fake_request) -def test_editors_cannot_edit_in_inherit_mode_and_map_in_owner_mode(datalayer, user): +def test_editors_cannot_edit_in_inherit_mode_and_map_in_owner_mode( + datalayer, user, fake_request +): datalayer.edit_status = DataLayer.INHERIT datalayer.save() map = datalayer.map map.editors.add(user) map.edit_status = Map.OWNER map.save() - assert not datalayer.can_edit(user) + fake_request.user = user + assert not datalayer.can_edit(fake_request) def test_team_members_cannot_edit_in_inherit_mode_and_map_in_owner_mode( - datalayer, user, team + datalayer, user, team, fake_request ): datalayer.edit_status = DataLayer.INHERIT datalayer.save() @@ -192,50 +206,66 @@ def test_team_members_cannot_edit_in_inherit_mode_and_map_in_owner_mode( map.team = team map.edit_status = Map.OWNER map.save() - assert not datalayer.can_edit(user) + fake_request.user = user + assert not datalayer.can_edit(fake_request) -def test_anonymous_cannot_edit_in_inherit_mode_and_map_in_owner_mode(datalayer): +def test_anonymous_cannot_edit_in_inherit_mode_and_map_in_owner_mode( + datalayer, fake_request +): datalayer.edit_status = DataLayer.INHERIT datalayer.save() map = datalayer.map map.edit_status = Map.OWNER map.save() - assert not datalayer.can_edit() + fake_request.user = AnonymousUser() + assert not datalayer.can_edit(fake_request) -def test_owner_can_edit_in_inherit_mode_and_map_in_editors_mode(datalayer): +def test_owner_can_edit_in_inherit_mode_and_map_in_editors_mode( + datalayer, fake_request +): datalayer.edit_status = DataLayer.INHERIT datalayer.save() map = datalayer.map map.edit_status = Map.COLLABORATORS map.save() - assert datalayer.can_edit(map.owner) + fake_request.user = map.owner + assert datalayer.can_edit(fake_request) -def test_editors_can_edit_in_inherit_mode_and_map_in_editors_mode(datalayer, user): +def test_editors_can_edit_in_inherit_mode_and_map_in_editors_mode( + datalayer, user, fake_request +): datalayer.edit_status = DataLayer.INHERIT datalayer.save() map = datalayer.map map.editors.add(user) map.edit_status = Map.COLLABORATORS map.save() - assert datalayer.can_edit(user) + fake_request.user = user + assert datalayer.can_edit(fake_request) -def test_anonymous_cannot_edit_in_inherit_mode_and_map_in_editors_mode(datalayer): +def test_anonymous_cannot_edit_in_inherit_mode_and_map_in_editors_mode( + datalayer, fake_request +): datalayer.edit_status = DataLayer.INHERIT datalayer.save() map = datalayer.map map.edit_status = Map.COLLABORATORS map.save() - assert not datalayer.can_edit() + fake_request.user = AnonymousUser() + assert not datalayer.can_edit(fake_request) -def test_anonymous_can_edit_in_inherit_mode_and_map_in_public_mode(datalayer): +def test_anonymous_can_edit_in_inherit_mode_and_map_in_public_mode( + datalayer, fake_request +): datalayer.edit_status = DataLayer.INHERIT datalayer.save() map = datalayer.map map.edit_status = Map.ANONYMOUS map.save() - assert datalayer.can_edit() + fake_request.user = AnonymousUser() + assert datalayer.can_edit(fake_request) diff --git a/umap/tests/test_map.py b/umap/tests/test_map.py index ff134e4c..07af12be 100644 --- a/umap/tests/test_map.py +++ b/umap/tests/test_map.py @@ -9,63 +9,69 @@ from .base import MapFactory pytestmark = pytest.mark.django_db -def test_anonymous_can_edit_if_status_anonymous(map): - anonymous = AnonymousUser() +def test_anonymous_can_edit_if_status_anonymous(map, fake_request): map.edit_status = map.ANONYMOUS map.save() - assert map.can_edit(anonymous) + fake_request.user = AnonymousUser() + assert map.can_edit(fake_request) -def test_anonymous_cannot_edit_if_not_status_anonymous(map): - anonymous = AnonymousUser() +def test_anonymous_cannot_edit_if_not_status_anonymous(map, fake_request): map.edit_status = map.OWNER map.save() - assert not map.can_edit(anonymous) + fake_request.user = AnonymousUser() + assert not map.can_edit(fake_request) -def test_non_editors_can_edit_if_status_anonymous(map, user): +def test_non_editors_can_edit_if_status_anonymous(map, user, fake_request): assert map.owner != user map.edit_status = map.ANONYMOUS map.save() - assert map.can_edit(user) + fake_request.user = user + assert map.can_edit(fake_request) -def test_non_editors_cannot_edit_if_not_status_anonymous(map, user): +def test_non_editors_cannot_edit_if_not_status_anonymous(map, user, fake_request): map.edit_status = map.OWNER map.save() - assert not map.can_edit(user) + fake_request.user = user + assert not map.can_edit(fake_request) -def test_editors_cannot_edit_if_status_owner(map, user): +def test_editors_cannot_edit_if_status_owner(map, user, fake_request): map.edit_status = map.OWNER map.editors.add(user) map.save() - assert not map.can_edit(user) + fake_request.user = user + assert not map.can_edit(fake_request) -def test_editors_can_edit_if_status_collaborators(map, user): +def test_editors_can_edit_if_status_collaborators(map, user, fake_request): map.edit_status = map.COLLABORATORS map.editors.add(user) map.save() - assert map.can_edit(user) + fake_request.user = user + assert map.can_edit(fake_request) -def test_team_members_cannot_edit_if_status_owner(map, user, team): +def test_team_members_cannot_edit_if_status_owner(map, user, team, fake_request): user.teams.add(team) user.save() map.edit_status = map.OWNER map.team = team map.save() - assert not map.can_edit(user) + fake_request.user = user + assert not map.can_edit(fake_request) -def test_team_members_can_edit_if_status_collaborators(map, user, team): +def test_team_members_can_edit_if_status_collaborators(map, user, team, fake_request): user.teams.add(team) user.save() map.edit_status = map.COLLABORATORS map.team = team map.save() - assert map.can_edit(user) + fake_request.user = user + assert map.can_edit(fake_request) def test_logged_in_user_should_be_allowed_for_anonymous_map_with_anonymous_edit_status( @@ -77,14 +83,17 @@ def test_logged_in_user_should_be_allowed_for_anonymous_map_with_anonymous_edit_ url = reverse("map_update", kwargs={"map_id": map.pk}) request = rf.get(url) request.user = user - assert map.can_edit(user, request) + assert map.can_edit(request) -def test_anonymous_user_should_not_be_allowed_for_anonymous_map(map, user, rf): # noqa +def test_anonymous_user_should_not_be_allowed_for_anonymous_map( + map, user, fake_request +): map.owner = None map.edit_status = map.OWNER map.save() - assert not map.can_edit() + fake_request.user = AnonymousUser() + assert not map.can_edit(fake_request) def test_clone_should_return_new_instance(map, user): diff --git a/umap/views.py b/umap/views.py index b7efd747..ed1be9ca 100644 --- a/umap/views.py +++ b/umap/views.py @@ -546,7 +546,7 @@ class SessionMixin: data = {} user = self.request.user if hasattr(self, "object"): - data["is_owner"] = self.object.is_owner(user, self.request) + data["is_owner"] = self.object.is_owner(self.request) if user.is_anonymous: return data return { @@ -725,20 +725,14 @@ class MapView(MapDetailMixin, PermissionsMixin, DetailView): return self.object.get_absolute_url() def get_datalayers(self): - return [ - dl.metadata(self.request.user, self.request) - for dl in self.object.datalayer_set.all() - ] + return [dl.metadata(self.request) for dl in self.object.datalayer_set.all()] @property def edit_mode(self): edit_mode = "disabled" - if self.object.can_edit(self.request.user, self.request): + if self.object.can_edit(self.request): edit_mode = "advanced" - elif any( - d.can_edit(self.request.user, self.request) - for d in self.object.datalayer_set.all() - ): + elif any(d.can_edit(self.request) for d in self.object.datalayer_set.all()): edit_mode = "simple" return edit_mode @@ -901,7 +895,7 @@ def get_websocket_auth_token(request, map_id, map_inst): map_object: Map = Map.objects.get(pk=map_id) permissions = ["edit"] - if map_object.is_owner(request.user, request): + if map_object.is_owner(request): permissions.append("owner") if request.user.is_authenticated: @@ -959,7 +953,7 @@ class AttachAnonymousMap(View): if ( self.object.owner or not self.object.is_anonymous_owner(self.request) - or not self.object.can_edit(self.request.user, self.request) + or not self.object.can_edit(self.request) or not self.request.user.is_authenticated ): return HttpResponseForbidden() @@ -976,7 +970,7 @@ class SendEditLink(FormLessEditMixin, FormView): if ( self.object.owner or not self.object.is_anonymous_owner(self.request) - or not self.object.can_edit(self.request.user, self.request) + or not self.object.can_edit(self.request) ): return HttpResponseForbidden() form = self.get_form() @@ -1009,7 +1003,7 @@ class MapDelete(DeleteView): def form_valid(self, form): self.object = self.get_object() - if not self.object.can_delete(self.request.user, self.request): + if not self.object.can_delete(self.request): return HttpResponseForbidden(_("Only its owner can delete the map.")) self.object.delete() home_url = reverse("home") @@ -1186,9 +1180,7 @@ class DataLayerCreate(FormLessEditMixin, GZipMixin, CreateView): form.instance.map = self.kwargs["map_inst"] self.object = form.save() # Simple response with only metadata (including new id) - response = simple_json_response( - **self.object.metadata(self.request.user, self.request) - ) + response = simple_json_response(**self.object.metadata(self.request)) response["X-Datalayer-Version"] = self.version return response @@ -1242,7 +1234,7 @@ class DataLayerUpdate(FormLessEditMixin, GZipMixin, UpdateView): if self.object.map.pk != int(self.kwargs["map_id"]): return HttpResponseForbidden() - if not self.object.can_edit(user=self.request.user, request=self.request): + if not self.object.can_edit(request=self.request): return HttpResponseForbidden() reference_version = self.request.headers.get("X-Datalayer-Reference") @@ -1262,7 +1254,7 @@ class DataLayerUpdate(FormLessEditMixin, GZipMixin, UpdateView): def form_valid(self, form): self.object = form.save() - data = {**self.object.metadata(self.request.user, self.request)} + data = {**self.object.metadata(self.request)} if self.request.session.get("needs_reload"): data["geojson"] = json.loads(self.object.geojson.read().decode()) self.request.session["needs_reload"] = False