From 0ec3816f9330edc50913575282d2764c903b9f1b Mon Sep 17 00:00:00 2001
From: David Larlet <david@larlet.fr>
Date: Fri, 5 Apr 2024 13:17:58 -0400
Subject: [PATCH] Redirect to the login view when login is required
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When the `UMAP_ALLOW_ANONYMOUS` setting is False, we return the login url through a JSON response. We lost that ability in #1555.

The JS part was not following that link in that particular case and lead to more errors because the map was not saved (hence, no `map_id`).

For now, the current work on the map is lost because of the redirection and we have a confirmation dialog to quit the edited page with unsaved changes.

Maybe we should display a custom message instead of a brutal redirection? Like: you’re not logged in, do it in a separate tab to keep you work? (A bit ugly…)

Sidenote: we might want to use the `redirect` pattern/key in the JSON response that we already use for deletion and clone for consistency.
---
 umap/decorators.py           |  8 ++++++--
 umap/static/umap/js/umap.js  | 17 +++++++++--------
 umap/tests/base.py           |  2 +-
 umap/tests/test_map_views.py |  2 +-
 4 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/umap/decorators.py b/umap/decorators.py
index 3ae667c50..05d339eb6 100644
--- a/umap/decorators.py
+++ b/umap/decorators.py
@@ -19,7 +19,9 @@ def wrapper(request, *args, **kwargs):
             not getattr(settings, "UMAP_ALLOW_ANONYMOUS", False)
             and not request.user.is_authenticated
         ):
-            return simple_json_response(login_required=str(LOGIN_URL))
+            response = simple_json_response(login_required=str(LOGIN_URL))
+            response.status_code = 401
+            return response
         return view_func(request, *args, **kwargs)
 
     return wrapper
@@ -39,7 +41,9 @@ def wrapper(request, *args, **kwargs):
             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))
+                    response = simple_json_response(login_required=str(LOGIN_URL))
+                    response.status_code = 401
+                    return response
                 return HttpResponseForbidden()
         return view_func(request, *args, **kwargs)
 
diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js
index 1fae103fc..fb55841ec 100644
--- a/umap/static/umap/js/umap.js
+++ b/umap/static/umap/js/umap.js
@@ -76,7 +76,6 @@ U.Map = L.Map.extend({
         .split(',')
     }
 
-
     let editedFeature = null
     const self = this
     try {
@@ -345,7 +344,7 @@ U.Map = L.Map.extend({
       document.body,
       'umap-caption-bar-enabled',
       this.options.captionBar ||
-      (this.options.slideshow && this.options.slideshow.active)
+        (this.options.slideshow && this.options.slideshow.active)
     )
     L.DomUtil.classIf(
       document.body,
@@ -962,8 +961,10 @@ U.Map = L.Map.extend({
     formData.append('settings', JSON.stringify(geojson))
     const uri = this.urls.get('map_save', { map_id: this.options.umap_id })
     const [data, response, error] = await this.server.post(uri, {}, formData)
-    // FIXME: login_required response will not be an error, so it will not
-    // stop code while it should
+    if (error && response.status === 401) {
+      const data = await response.json()
+      window.location = data.login_required
+    }
     if (!error) {
       let duration = 3000,
         alert = { content: L._('Map has been saved!'), level: 'info' }
@@ -1573,10 +1574,10 @@ U.Map = L.Map.extend({
 
   initCaptionBar: function () {
     const container = L.DomUtil.create(
-      'div',
-      'umap-caption-bar',
-      this._controlContainer
-    ),
+        'div',
+        'umap-caption-bar',
+        this._controlContainer
+      ),
       name = L.DomUtil.create('h3', '', container)
     L.DomEvent.disableClickPropagation(container)
     this.permissions.addOwnerLink('span', container)
diff --git a/umap/tests/base.py b/umap/tests/base.py
index 62f948ebd..9e3310960 100644
--- a/umap/tests/base.py
+++ b/umap/tests/base.py
@@ -134,7 +134,7 @@ class Meta:
 
 
 def login_required(response):
-    assert response.status_code == 200
+    assert response.status_code == 401
     j = json.loads(response.content.decode())
     assert "login_required" in j
     redirect_url = reverse("login")
diff --git a/umap/tests/test_map_views.py b/umap/tests/test_map_views.py
index 7f9299a2c..30ea90719 100644
--- a/umap/tests/test_map_views.py
+++ b/umap/tests/test_map_views.py
@@ -608,7 +608,7 @@ def test_cannot_send_link_on_owned_map(client, map):
     assert len(mail.outbox) == 0
     url = reverse("map_send_edit_link", args=(map.pk,))
     resp = client.post(url, {"email": "foo@bar.org"})
-    assert resp.status_code == 200
+    assert resp.status_code == 401
     assert json.loads(resp.content.decode()) == {"login_required": "/en/login/"}
     assert len(mail.outbox) == 0