Merge pull request #2025 from umap-project/login-from-map

fix: catch login_required from map page and add a way to login

umap/settings/base.py

@@ -152,7 +152,7 @@ WSGI_APPLICATION = "umap.wsgi.application"
 
 LOGIN_URL = "/login/"
 LOGOUT_URL = "/logout/"
-LOGIN_REDIRECT_URL = "/"
+LOGIN_REDIRECT_URL = "login_popup_end"
 
 STATIC_URL = "/static/"
 MEDIA_URL = "/uploads/"

umap/static/umap/js/modules/permissions.js

@@ -34,7 +34,7 @@ export class MapPermissions {
   }
 
   isOwner() {
-    return this.map.options.user?.id === this.map.options.permissions.owner?.id
+    return Boolean(this.map.options.user?.is_owner)
   }
 
   isAnonymousMap() {

umap/static/umap/js/umap.controls.js

@@ -640,7 +640,7 @@ const ControlsMixin = {
       L.DomEvent.on(shareStatusButton, 'click', this.permissions.edit, this.permissions)
     }
     this.on('postsync', L.bind(update, this))
-    if (this.options.user) {
+    if (this.options.user?.id) {
       L.DomUtil.createLink(
         'umap-user',
         rightContainer,

umap/static/umap/js/umap.js

@@ -395,7 +395,7 @@ U.Map = L.Map.extend({
     this._controls.search = new U.SearchControl()
     this._controls.embed = new L.Control.Embed(this)
     this._controls.tilelayersChooser = new U.TileLayerChooser(this)
-    if (this.options.user) this._controls.star = new U.StarControl(this)
+    if (this.options.user?.id) this._controls.star = new U.StarControl(this)
     this._controls.editinosm = new L.Control.EditInOSM({
       position: 'topleft',
       widgetOptions: {
@@ -1048,7 +1048,15 @@ U.Map = L.Map.extend({
     if (error) {
       return
     }
-
+    if (data.login_required) {
+      window.onLogin = () => this.saveSelf()
+      window.open(data.login_required)
+      return
+    }
+    if (data.user?.id) {
+      this.options.user = data.user
+      this.renderEditToolbar()
+    }
     if (!this.options.umap_id) {
       this.options.umap_id = data.id
       this.permissions.setOptions(data.permissions)
@@ -1630,11 +1638,13 @@ U.Map = L.Map.extend({
   },
 
   del: async function () {
-    if (confirm(L._('Are you sure you want to delete this map?'))) {
-      const url = this.urls.get('map_delete', { map_id: this.options.umap_id })
-      const [data, response, error] = await this.server.post(url)
-      if (data.redirect) window.location = data.redirect
-    }
+    this.dialog
+      .confirm(L._('Are you sure you want to delete this map?'))
+      .then(async () => {
+        const url = this.urls.get('map_delete', { map_id: this.options.umap_id })
+        const [data, response, error] = await this.server.post(url)
+        if (data.redirect) window.location = data.redirect
+      })
   },
 
   clone: async function () {

umap/templates/registration/login.html

@@ -1,5 +1,10 @@
 {% extends "base.html" %}
 
+{% load i18n %}
+
+{% block head_title %}
+  {% trans "Login" %}
+{% endblock head_title %}
 {% load umap_tags i18n %}
 
 {% block extra_head %}

umap/templates/umap/login_popup_end.html

@@ -5,10 +5,13 @@
 </h3>
 <script type="text/javascript">
   function proceed() {
-    if (window.opener && window.opener.umap_proceed) {
-      window.opener.umap_proceed()
+    if (window.opener?.onLogin) {
+      // We are in the normal process login
+      window.opener.onLogin()
+      window.close()
     } else {
-      // Trade off as Twitter does not allow us to access window.opener
+      // we may be in login process that includes a magic link, so user
+      // has opened a new window from this link, and we cannot close it then.
       window.location.href = '{% url "user_dashboard" %}'
     }
   }

umap/tests/integration/test_anonymous_owned_map.py

@@ -219,3 +219,26 @@ def test_alert_message_after_create_show_link_even_without_mail(
     ).to_be_visible()
     expect(alert.get_by_role("button", name="Copy")).to_be_visible()
     expect(alert.get_by_role("button", name="Send me the link")).to_be_hidden()
+
+
+def test_anonymous_owner_can_delete_the_map(anonymap, live_server, owner_session):
+    assert Map.objects.count() == 1
+    owner_session.goto(f"{live_server.url}{anonymap.get_absolute_url()}")
+    owner_session.get_by_role("button", name="Edit").click()
+    owner_session.get_by_role("link", name="Map advanced properties").click()
+    owner_session.get_by_text("Advanced actions").click()
+    expect(owner_session.get_by_role("button", name="Delete")).to_be_visible()
+    owner_session.get_by_role("button", name="Delete").click()
+    with owner_session.expect_response(re.compile(r".*/update/delete/.*")):
+        owner_session.get_by_role("button", name="OK").click()
+    assert not Map.objects.count()
+
+
+def test_non_owner_cannot_see_delete_button(anonymap, live_server, page):
+    anonymap.edit_status = Map.ANONYMOUS
+    anonymap.save()
+    page.goto(f"{live_server.url}{anonymap.get_absolute_url()}")
+    page.get_by_role("button", name="Edit").click()
+    page.get_by_role("link", name="Map advanced properties").click()
+    page.get_by_text("Advanced actions").click()
+    expect(page.get_by_role("button", name="Delete")).to_be_hidden()

umap/tests/integration/test_basics.py

@@ -1,6 +1,10 @@
+import re
+
 import pytest
 from playwright.sync_api import expect
 
+from umap.models import Map
+
 
 def test_page_title(page, live_server):
     page.goto(live_server.url)
@@ -66,3 +70,28 @@ def test_cannot_put_script_tag_in_datalayer_name_or_description(
     expect(page.get_by_text('<script>alert("attack")</script>')).to_be_visible()
     # Description should contain escaped HTML
     expect(page.get_by_text("before after")).to_be_visible()
+
+
+def test_login_from_map_page(live_server, page, tilelayer, settings, user, context):
+    settings.ENABLE_ACCOUNT_LOGIN = True
+    assert Map.objects.count() == 0
+    page.goto(f"{live_server.url}/en/map/new/")
+    with (
+        page.expect_response(re.compile(r".*/map/create/")),
+        context.expect_page() as login_page_info,
+    ):
+        page.get_by_role("button", name="Save").click()
+    assert Map.objects.count() == 0
+    login_page = login_page_info.value
+    expect(login_page).to_have_title("Login")
+    login_page.get_by_placeholder("Username").fill(user.username)
+    login_page.get_by_placeholder("Password").fill("123123")
+    with page.expect_response(re.compile(r".*/map/create/")):
+        login_page.locator('#login_form input[type="submit"]').click()
+    # Login page should be closed
+    page.wait_for_timeout(500)  # Seems needed from time to time…
+    assert len(context.pages) == 1
+    # Save should have proceed
+    assert Map.objects.count() == 1
+    # Use name should now appear on the header toolbar
+    expect(page.get_by_text("My Dashboard (Joe)")).to_be_visible()

umap/tests/integration/test_owned_map.py

@@ -134,17 +134,9 @@ def test_owner_has_delete_map_button(map, live_server, login):
     advanced.click()
     delete = page.get_by_role("button", name="Delete", exact=True)
     expect(delete).to_be_visible()
-    dialog_shown = False
-
-    def handle_dialog(dialog):
-        dialog.accept()
-        nonlocal dialog_shown
-        dialog_shown = True
-
-    page.on("dialog", handle_dialog)
+    delete.click()
     with page.expect_navigation():
-        delete.click()
-    assert dialog_shown
+        page.get_by_role("button", name="OK").click()
     assert Map.objects.all().count() == 0
 
 

umap/views.py

@@ -456,6 +456,21 @@ def simple_json_response(**kwargs):
 # ############## #
 
 
+class SessionMixin:
+    def get_user_data(self):
+        data = {}
+        if hasattr(self, "object"):
+            data["is_owner"] = self.object.is_owner(self.request.user, self.request)
+        if self.request.user.is_anonymous:
+            return data
+        return {
+            "id": self.request.user.pk,
+            "name": str(self.request.user),
+            "url": reverse("user_dashboard"),
+            **data,
+        }
+
+
 class FormLessEditMixin:
     http_method_names = [
         "post",
@@ -470,7 +485,7 @@ class FormLessEditMixin:
         return self.get_form_class()(**kwargs)
 
 
-class MapDetailMixin:
+class MapDetailMixin(SessionMixin):
     model = Map
     pk_url_kwarg = "map_id"
 
@@ -522,12 +537,7 @@ class MapDetailMixin:
         if self.get_short_url():
             properties["shortUrl"] = self.get_short_url()
 
-        if not user.is_anonymous:
-            properties["user"] = {
-                "id": user.pk,
-                "name": str(user),
-                "url": reverse("user_dashboard"),
-            }
+        properties["user"] = self.get_user_data()
         return properties
 
     def get_context_data(self, **kwargs):
@@ -755,7 +765,7 @@ class MapPreview(MapDetailMixin, TemplateView):
         return properties
 
 
-class MapCreate(FormLessEditMixin, PermissionsMixin, CreateView):
+class MapCreate(FormLessEditMixin, PermissionsMixin, SessionMixin, CreateView):
     model = Map
     form_class = MapSettingsForm
 
@@ -772,6 +782,7 @@ class MapCreate(FormLessEditMixin, PermissionsMixin, CreateView):
             id=self.object.pk,
             url=self.object.get_absolute_url(),
             permissions=permissions,
+            user=self.get_user_data(),
         )
         if not self.request.user.is_authenticated:
             key, value = self.object.signed_cookie_elements