Merge pull request #166 from Xpirix/block-replace-plugin-version

Block replace approved plugin version

Author: Xpirix

qgis-app/plugins/templates/plugins/plugin_detail.html

@@ -577,7 +577,18 @@ <h2>
                                                 {% endif %}
                                             </a>
                                             {% if user.is_staff or user in version.plugin.editors %}
-                                            <a class="button is-success is-small is-outlined" href="{% url "version_update" object.package_name version.version %}" title="{% trans "Edit" %}"><i class="fas fa-pencil"></i></a>
+                                            {% if not version.approved %}
+                                                <a 
+                                                    class="button is-success is-small is-outlined" 
+                                                    href="{% url "version_update" object.package_name version.version %}" 
+                                                    title="{% trans "Edit" %}">
+                                                    <i class="fas fa-pencil"></i>
+                                                </a>
+                                            {% else %}
+                                                <button class="button is-success is-small is-outlined" disabled title="{% trans 'Cannot edit approved version' %}">
+                                                    <i class="fas fa-pencil"></i>
+                                                </button>
+                                            {% endif %}
                                             <a class="button is-danger is-small is-outlined" href="{% url "version_delete" object.package_name version.version %}" title="{% trans "Delete" %}"><i class="fas fa-remove"></i></a>
                                             {% endif %}
                                             </form>

qgis-app/plugins/templates/plugins/version_permission_deny.html

@@ -1,4 +1,7 @@
 {% extends 'plugins/plugin_base.html' %}{% load i18n %}
 {% block content %}
-    <div class="error">{% trans "You cannot create or modify versions of this plugin." %}</div>
+    <div class="notification is-danger is-light">
+        <strong>{% trans "Permission Denied" %}</strong><br>
+        {% trans "You cannot create or modify versions of this plugin." %}
+    </div>
 {% endblock %}

qgis-app/plugins/tests/test_plugin_update.py

@@ -136,12 +136,10 @@ def test_plugin_version_update(self):
         })
         self.assertEqual(response.status_code, 302)
 
-        # The old version should not exist anymore
-        # TODO: The old version still exist, not sure why
-        # self.assertFalse(PluginVersion.objects.filter(
-        #     plugin__name='Test Plugin', 
-        #     version='0.0.1').exists()
-        # )
+        self.assertFalse(PluginVersion.objects.filter(
+            plugin__name='Test Plugin', 
+            version='0.0.1').exists()
+        )
         self.assertTrue(PluginVersion.objects.filter(
             plugin__name='Test Plugin', 
             version='0.0.2').exists()
@@ -172,6 +170,21 @@ def test_plugin_version_update(self):
             settings.EMAIL_HOST_USER
         )
 
+    def test_plugin_version_approved_update(self):
+        """
+        Test update a plugin version that is already approved
+        """
+        package_name = self.plugin.package_name
+        self.url_add_version = reverse('version_update', args=[package_name, '0.0.1'])
+        version = PluginVersion.objects.get(plugin__name='Test Plugin', version='0.0.1')
+        version.approved = True
+        version.save()
+        self.assertTrue(version.approved)
+
+        response = self.client.get(self.url_add_version)
+        # Should redirect to the plugin details page
+        self.assertEqual(response.status_code, 302)
+        self.assertRedirects(response, reverse('plugin_detail', args=[package_name]))
 
     def tearDown(self):
         self.client.logout()

qgis-app/plugins/tests/test_token_auth.py

@@ -135,10 +135,38 @@ def test_update_version_with_valid_token(self):
             'package': uploaded_file,
         })
         self.assertEqual(response.status_code, 302)
-        # This will create a new version because this one is using token and doesn't have a created_by column
-        self.assertTrue(PluginVersion.objects.filter(plugin__name='Test Plugin', version='0.0.1').exists())
+        self.assertFalse(PluginVersion.objects.filter(plugin__name='Test Plugin', version='0.0.1').exists())
         self.assertTrue(PluginVersion.objects.filter(plugin__name='Test Plugin', version='0.0.2').exists())
 
+    def test_update_approved_version_with_token(self):
+        # Generate a token for the authenticated user
+        self.client.post(self.url_token_create, {})
+        outstanding_token = OutstandingToken.objects.last().token
+        refresh = RefreshToken(outstanding_token)
+        refresh['plugin_id'] = self.plugin.pk
+        refresh['refresh_jti'] = refresh['jti']
+        access_token = str(refresh.access_token)
+
+        version = PluginVersion.objects.get(plugin__name='Test Plugin', version='0.0.1')
+        version.approved = True
+        version.save()
+        self.assertTrue(version.approved)
+
+        # Log out the user and use the token
+        self.client.logout()
+
+        c = Client(HTTP_AUTHORIZATION=f"Bearer {access_token}")
+
+        # Test request with access token
+        response = c.get(self.url_update_version)
+        # Check that the response is forbidden
+        self.assertEqual(response.status_code, 401)
+        self.assertIn("application/json", response["Content-Type"])
+        self.assertEqual(
+            response.json().get("detail"),
+            "You cannot edit an approved version, please create a new version instead."
+        )
+
     def test_update_version_with_invalid_token(self):
         # Log out the user and use the token
         self.client.logout()

qgis-app/plugins/views.py

@@ -1327,7 +1327,14 @@ def version_update_api(request, package_name, version):
     disabling CSRF protection.
     """
     plugin = get_object_or_404(Plugin, package_name=package_name)
-    version = PluginVersion(plugin=plugin, is_from_token=True, token=request.plugin_token)
+    version = get_object_or_404(PluginVersion, plugin=plugin, version=version)
+    if version.approved:
+        msg = _(
+            "You cannot edit an approved version, please create a new version instead."
+        )
+        return JsonResponse({"detail": msg}, status=401)
+    version.is_from_token = True
+    version.token = request.plugin_token
     return _version_update(request, plugin, version)
 
 
@@ -1339,6 +1346,12 @@ def version_update(request, package_name, version):
         return render(
             request, "plugins/version_permission_deny.html", {"plugin": plugin}
         )
+    if version.approved:
+        msg = _(
+            "You cannot edit an approved version, please create a new version instead."
+        )
+        messages.error(request, msg, fail_silently=True, extra_tags="is-danger")
+        return HttpResponseRedirect(plugin.get_absolute_url())
     version.created_by = request.user
     is_trusted=request.user.has_perm("plugins.can_approve")
     return _version_update(request, plugin, version, is_trusted=is_trusted)

qgis-app/templates/base.html

@@ -62,12 +62,12 @@
                 </div>
                 <div class="content column is-9">
                     {% if messages %}
-                    <div class="notification is-light">
-                        <button class="delete"></button>
-                        {% for message in messages %}
-                            <p{% if message.tags %} class="{{ message.tags }}"{% endif %}>{{ message|safe }}</p>
-                        {% endfor %}
-                    </div>
+                    {% for message in messages %}
+                        <div class="notification is-light {% if message.tags %} {{ message.tags }}{% endif %}">
+                            <button class="delete"></button>
+                            {{ message|safe }}
+                        </div>
+                    {% endfor %}
                     {% endif %}
                     {% block content %}
                     {% endblock %}