From 1a815b313d7f3a941660228b53257e1607810a18 Mon Sep 17 00:00:00 2001
From: Yohan Boniface <yb@enix.org>
Date: Sat, 7 Jul 2018 16:44:40 +0200
Subject: [PATCH] Add an explicit button to attach an owner to an anonymous map

fix #568
---
 umap/models.py                          |  8 ---
 umap/static/umap/js/umap.permissions.js | 35 +++++++++----
 umap/templates/umap/map_messages.html   |  1 -
 umap/tests/conftest.py                  |  4 +-
 umap/tests/test_map_views.py            | 68 +++++++++++++++++++------
 umap/urls.py                            |  2 +
 umap/views.py                           | 18 ++++++-
 7 files changed, 97 insertions(+), 39 deletions(-)

diff --git a/umap/models.py b/umap/models.py
index 0f7e8f16..0e7812b2 100644
--- a/umap/models.py
+++ b/umap/models.py
@@ -172,14 +172,6 @@ class Map(NamedModel):
             if (getattr(settings, "UMAP_ALLOW_ANONYMOUS", False)
                     and self.is_anonymous_owner(request)):
                 can = True
-                if user and user.is_authenticated:
-                    # TODO: only when using the anonymous-edit URL with an
-                    # authenticated user
-                    # if user is authenticated, attach as owner
-                    self.owner = user
-                    self.save()
-                    msg = _("Your anonymous map has been attached to your account %s" % user)
-                    messages.info(request, msg)
         if self.edit_status == self.ANONYMOUS:
             can = True
         elif not user.is_authenticated:
diff --git a/umap/static/umap/js/umap.permissions.js b/umap/static/umap/js/umap.permissions.js
index 6f7e7c92..5cd5bb00 100644
--- a/umap/static/umap/js/umap.permissions.js
+++ b/umap/static/umap/js/umap.permissions.js
@@ -65,19 +65,30 @@ L.U.MapPermissions = L.Class.extend({
         var builder = new L.U.FormBuilder(this, fields);
         var form = builder.build();
         container.appendChild(form);
+        if (this.isAnonymousMap() && this.map.options.user) {
+            // We have a user, and this user has come through here, so they can edit the map, so let's allow to own the map.
+            // Note: real check is made on the back office anyway.
+            var advancedActions = L.DomUtil.createFieldset(container, L._('Advanced actions'));
+            var advancedButtons = L.DomUtil.create('div', 'button-bar', advancedActions);
+            var download = L.DomUtil.create('a', 'button', advancedButtons);
+            download.href = '#';
+            download.innerHTML = L._('Attach the map to my account');
+            L.DomEvent
+                .on(download, 'click', L.DomEvent.stop)
+                .on(download, 'click', this.attach, this);
+        }
         this.map.ui.openPanel({data: {html: container}, className: 'dark'});
     },
 
-    anonymousMapPanel: function () {
-        var container = L.DomUtil.create('div'),
-            fields = [],
-            title = L.DomUtil.create('h4', '', container);
-        fields.push(['options.edit_status', {handler: 'IntSelect', label: L._('Who can edit'), selectOptions: this.map.options.edit_statuses}]);
-        title.innerHTML = L._('Update permissions');
-        var builder = new L.U.FormBuilder(this, fields);
-        var form = builder.build();
-        container.appendChild(form);
-        this.map.ui.openPanel({data: {html: container}, className: 'dark'});
+    attach: function () {
+        this.map.post(this.getAttachUrl(), {
+            callback: function () {
+                this.options.owner = this.map.options.user;
+                this.map.ui.alert({content: L._("Map has been attached to your account"), level: 'info'});
+                this.map.ui.closePanel();
+            },
+            context: this
+        })
     },
 
     save: function () {
@@ -106,6 +117,10 @@ L.U.MapPermissions = L.Class.extend({
         return L.Util.template(this.map.options.urls.map_update_permissions, {'map_id': this.map.options.umap_id});
     },
 
+    getAttachUrl: function () {
+        return L.Util.template(this.map.options.urls.map_attach_owner, {'map_id': this.map.options.umap_id});
+    },
+
     addOwnerLink: function (element, container) {
         if (this.options.owner && this.options.owner.name && this.options.owner.url) {
             var ownerContainer = L.DomUtil.add(element, 'umap-map-owner', container, ' ' + L._('by') + ' '),
diff --git a/umap/templates/umap/map_messages.html b/umap/templates/umap/map_messages.html
index 5ee2c2fb..646cfd0a 100644
--- a/umap/templates/umap/map_messages.html
+++ b/umap/templates/umap/map_messages.html
@@ -1,6 +1,5 @@
 <script type="text/javascript">
     {% for m in messages %}
-        "loooping"
         {# We have just one, but we need to loop, as for messages API #}
         L.U.fire('ui:alert', {
             content: "{{ m }}",
diff --git a/umap/tests/conftest.py b/umap/tests/conftest.py
index fe8cbe68..e3d237e3 100644
--- a/umap/tests/conftest.py
+++ b/umap/tests/conftest.py
@@ -44,8 +44,8 @@ def anonymap(map):
 
 
 @pytest.fixture
-def cookieclient(client, anonymap):
-    key, value = anonymap.signed_cookie_elements
+def cookieclient(client, map):
+    key, value = map.signed_cookie_elements
     client.cookies[key] = get_cookie_signer(salt=key).sign(value)
     return client
 
diff --git a/umap/tests/test_map_views.py b/umap/tests/test_map_views.py
index 26541c62..70e8fa00 100644
--- a/umap/tests/test_map_views.py
+++ b/umap/tests/test_map_views.py
@@ -378,22 +378,6 @@ def test_bad_anonymous_edit_url_should_return_403(cookieclient, anonymap):
     assert response.status_code == 403
 
 
-@pytest.mark.usefixtures('allow_anonymous')
-def test_authenticated_user_with_cookie_is_attached_as_owner(cookieclient, anonymap, post_data, user):  # noqa
-    url = reverse('map_update', kwargs={'map_id': anonymap.pk})
-    cookieclient.login(username=user.username, password="123123")
-    assert anonymap.owner is None
-    # POST only mendatory filds
-    name = 'new map name for authenticat_anonymoused user'
-    post_data['name'] = name
-    response = cookieclient.post(url, post_data)
-    assert response.status_code == 200
-    j = json.loads(response.content.decode())
-    updated_map = Map.objects.get(pk=anonymap.pk)
-    assert j['id'] == updated_map.pk
-    assert updated_map.owner.pk, user.pk
-
-
 @pytest.mark.usefixtures('allow_anonymous')
 def test_clone_anonymous_map_should_not_be_possible_if_user_is_not_allowed(client, anonymap, user):  # noqa
     assert Map.objects.count() == 1
@@ -435,3 +419,55 @@ def test_anyone_can_access_anonymous_map(cookieclient, anonymap):
     anonymap.share_status = anonymap.PRIVATE
     response = cookieclient.get(url)
     assert response.status_code == 200
+
+
+@pytest.mark.usefixtures('allow_anonymous')
+def test_map_attach_owner(cookieclient, anonymap, user):
+    url = reverse('map_attach_owner', kwargs={'map_id': anonymap.pk})
+    cookieclient.login(username=user.username, password="123123")
+    assert anonymap.owner is None
+    response = cookieclient.post(url)
+    assert response.status_code == 200
+    map = Map.objects.get(pk=anonymap.pk)
+    assert map.owner == user
+
+
+@pytest.mark.usefixtures('allow_anonymous')
+def test_map_attach_owner_not_logged_in(cookieclient, anonymap, user):
+    url = reverse('map_attach_owner', kwargs={'map_id': anonymap.pk})
+    assert anonymap.owner is None
+    response = cookieclient.post(url)
+    assert response.status_code == 403
+
+
+@pytest.mark.usefixtures('allow_anonymous')
+def test_map_attach_owner_with_already_an_owner(cookieclient, map, user):
+    url = reverse('map_attach_owner', kwargs={'map_id': map.pk})
+    cookieclient.login(username=user.username, password="123123")
+    assert map.owner
+    assert map.owner != user
+    response = cookieclient.post(url)
+    assert response.status_code == 403
+
+
+def test_map_attach_owner_anonymous_not_allowed(cookieclient, anonymap, user):
+    url = reverse('map_attach_owner', kwargs={'map_id': anonymap.pk})
+    cookieclient.login(username=user.username, password="123123")
+    assert anonymap.owner is None
+    response = cookieclient.post(url)
+    assert response.status_code == 403
+
+    # # GET anonymous
+    # response = client.get(url)
+    # assert login_required(response)
+    # # POST anonymous
+    # response = client.post(url, {})
+    # assert login_required(response)
+    # # GET with wrong permissions
+    # client.login(username=user.username, password="123123")
+    # response = client.get(url)
+    # assert response.status_code == 403
+    # # POST with wrong permissions
+    # client.login(username=user.username, password="123123")
+    # response = client.post(url, {})
+    # assert response.status_code == 403
diff --git a/umap/urls.py b/umap/urls.py
index ff515024..44a7535a 100644
--- a/umap/urls.py
+++ b/umap/urls.py
@@ -66,6 +66,8 @@ i18n_urls += decorated_patterns(
         name='map_update'),
     url(r'^map/(?P<map_id>[\d]+)/update/permissions/$',
         views.UpdateMapPermissions.as_view(), name='map_update_permissions'),
+    url(r'^map/(?P<map_id>[\d]+)/update/owner/$',
+        views.AttachAnonymousMap.as_view(), name='map_attach_owner'),
     url(r'^map/(?P<map_id>[\d]+)/update/delete/$',
         views.MapDelete.as_view(), name='map_delete'),
     url(r'^map/(?P<map_id>[\d]+)/update/clone/$',
diff --git a/umap/views.py b/umap/views.py
index 849f9173..62e4494f 100644
--- a/umap/views.py
+++ b/umap/views.py
@@ -332,7 +332,7 @@ def simple_json_response(**kwargs):
 # ############## #
 
 
-class FormLessEditMixin(object):
+class FormLessEditMixin:
     http_method_names = [u'post', ]
 
     def form_invalid(self, form):
@@ -345,7 +345,7 @@ class FormLessEditMixin(object):
         return self.get_form_class()(**kwargs)
 
 
-class MapDetailMixin(object):
+class MapDetailMixin:
 
     model = Map
 
@@ -574,6 +574,20 @@ class UpdateMapPermissions(FormLessEditMixin, UpdateView):
             info=_("Map editors updated with success!"))
 
 
+class AttachAnonymousMap(View):
+
+    def post(self, *args, **kwargs):
+        self.object = kwargs['map_inst']
+        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.request.user.is_authenticated):
+            return HttpResponseForbidden('Forbidden.')
+        self.object.owner = self.request.user
+        self.object.save()
+        return simple_json_response()
+
+
 class MapDelete(DeleteView):
     model = Map
     pk_url_kwarg = "map_id"