Browse Source

Merge pull request from GHSA-w2v8-php4-p8hc

* Pass user to settings form to enable permissions checks

* Add tests for settings creation and editing

* Ensure all generic create / edit view forms receive `for_user`

Co-authored-by: Sage Abdullah <sage.abdullah@torchbox.com>

* Test field permissions on ModelViewTest

---------

Co-authored-by: Sage Abdullah <sage.abdullah@torchbox.com>
Jake Howard 11 months ago
parent
commit
ee57f6d4dc

+ 29 - 0
wagtail/admin/tests/viewsets/test_model_viewset.py

@@ -1571,6 +1571,35 @@ class TestEditHandler(WagtailTestUtils, TestCase):
             self.assertIsNotNone(rendered_heading)
             self.assertEqual(rendered_heading.text.strip(), expected_heading)
 
+    def test_field_permissions(self):
+        self.user.is_superuser = False
+        self.user.save()
+        self.user.user_permissions.add(
+            Permission.objects.get(
+                content_type__app_label="wagtailadmin", codename="access_admin"
+            ),
+            Permission.objects.get(
+                content_type__app_label=self.object._meta.app_label,
+                codename=get_permission_codename("change", self.object._meta),
+            ),
+        )
+
+        response = self.client.get(self.url)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(list(response.context["form"].fields), ["name"])
+
+        self.user.user_permissions.add(
+            Permission.objects.get(
+                codename="can_set_release_date",
+            )
+        )
+
+        response = self.client.get(self.url)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            list(response.context["form"].fields), ["name", "release_date"]
+        )
+
 
 class TestDefaultMessages(WagtailTestUtils, TestCase):
     def setUp(self):

+ 26 - 0
wagtail/admin/views/generic/models.py

@@ -28,6 +28,7 @@ from django.views.generic.edit import (
 from wagtail.actions.unpublish import UnpublishAction
 from wagtail.admin import messages
 from wagtail.admin.filters import WagtailFilterSet
+from wagtail.admin.forms.models import WagtailAdminModelForm
 from wagtail.admin.forms.search import SearchForm
 from wagtail.admin.panels import get_edit_handler
 from wagtail.admin.ui.components import Component, MediaContainer
@@ -628,6 +629,24 @@ class CreateView(
             for locale in Locale.objects.all().exclude(id=self.locale.id)
         ]
 
+    def get_initial_form_instance(self):
+        if self.locale:
+            instance = self.model()
+            instance.locale = self.locale
+            return instance
+
+    def get_form_kwargs(self):
+        if instance := self.get_initial_form_instance():
+            # super().get_form_kwargs() will use self.object as the instance kwarg
+            self.object = instance
+        kwargs = super().get_form_kwargs()
+
+        form_class = self.get_form_class()
+        # Add for_user support for PermissionedForm
+        if issubclass(form_class, WagtailAdminModelForm):
+            kwargs["for_user"] = self.request.user
+        return kwargs
+
     def save_instance(self):
         """
         Called after the form is successfully validated - saves the object to the db
@@ -802,6 +821,13 @@ class EditView(
             for translation in self.object.get_translations().select_related("locale")
         ]
 
+    def get_form_kwargs(self):
+        kwargs = super().get_form_kwargs()
+        form_class = self.get_form_class()
+        if issubclass(form_class, WagtailAdminModelForm):
+            kwargs["for_user"] = self.request.user
+        return kwargs
+
     def save_instance(self):
         """
         Called after the form is successfully validated - saves the object to the db.

+ 88 - 1
wagtail/contrib/settings/tests/generic/test_admin.py

@@ -14,6 +14,7 @@ from wagtail.test.testapp.models import (
     PanelGenericSettings,
     TabbedGenericSettings,
     TestGenericSetting,
+    TestPermissionedGenericSetting,
 )
 from wagtail.test.utils import WagtailTestUtils
 
@@ -76,6 +77,11 @@ class BaseTestGenericSettingView(WagtailTestUtils, TestCase):
 class TestGenericSettingCreateView(BaseTestGenericSettingView):
     def setUp(self):
         self.user = self.login()
+        self.user.user_permissions.add(
+            Permission.objects.get(
+                content_type__app_label="wagtailadmin", codename="access_admin"
+            )
+        )
 
     def test_get_edit(self):
         response = self.get()
@@ -107,6 +113,38 @@ class TestGenericSettingCreateView(BaseTestGenericSettingView):
         # Ensure the form supports file uploads
         self.assertContains(response, 'enctype="multipart/form-data"')
 
+    def test_create_restricted_field_without_permission(self):
+        self.user.is_superuser = False
+        self.user.save()
+
+        self.assertFalse(TestPermissionedGenericSetting.objects.exists())
+        response = self.post(
+            post_data={"sensitive_email": "test@example.com", "title": "test"},
+            setting=TestPermissionedGenericSetting,
+        )
+        self.assertEqual(response.status_code, 302)
+
+        settings = TestPermissionedGenericSetting.objects.get()
+        self.assertEqual(settings.title, "test")
+        self.assertEqual(settings.sensitive_email, "")
+
+    def test_create_restricted_field(self):
+        self.user.is_superuser = False
+        self.user.save()
+        self.user.user_permissions.add(
+            Permission.objects.get(codename="can_edit_sensitive_email_generic_setting")
+        )
+        self.assertFalse(TestPermissionedGenericSetting.objects.exists())
+        response = self.post(
+            post_data={"sensitive_email": "test@example.com", "title": "test"},
+            setting=TestPermissionedGenericSetting,
+        )
+        self.assertEqual(response.status_code, 302)
+
+        settings = TestPermissionedGenericSetting.objects.get()
+        self.assertEqual(settings.title, "test")
+        self.assertEqual(settings.sensitive_email, "test@example.com")
+
 
 class TestGenericSettingEditView(BaseTestGenericSettingView):
     def setUp(self):
@@ -114,7 +152,12 @@ class TestGenericSettingEditView(BaseTestGenericSettingView):
         self.test_setting.title = "Setting title"
         self.test_setting.save()
 
-        self.login()
+        self.user = self.login()
+        self.user.user_permissions.add(
+            Permission.objects.get(
+                content_type__app_label="wagtailadmin", codename="access_admin"
+            )
+        )
 
     def test_get_edit(self):
         response = self.get()
@@ -162,6 +205,50 @@ class TestGenericSettingEditView(BaseTestGenericSettingView):
             expected_url=f"{url}{TestGenericSetting.objects.first().pk}/",
         )
 
+    def test_edit_restricted_field(self):
+        test_setting = TestPermissionedGenericSetting()
+        test_setting.sensitive_email = "test@example.com"
+        test_setting.save()
+        self.user.is_superuser = False
+        self.user.save()
+
+        self.user.user_permissions.add(
+            Permission.objects.get(codename="can_edit_sensitive_email_generic_setting")
+        )
+
+        response = self.get(setting=TestPermissionedGenericSetting)
+        self.assertEqual(response.status_code, 200)
+        self.assertIn("sensitive_email", list(response.context["form"].fields))
+
+        response = self.post(
+            setting=TestPermissionedGenericSetting,
+            post_data={"sensitive_email": "test-updated@example.com", "title": "title"},
+        )
+        self.assertEqual(response.status_code, 302)
+
+        test_setting.refresh_from_db()
+        self.assertEqual(test_setting.sensitive_email, "test-updated@example.com")
+
+    def test_edit_restricted_field_without_permission(self):
+        test_setting = TestPermissionedGenericSetting()
+        test_setting.sensitive_email = "test@example.com"
+        test_setting.save()
+        self.user.is_superuser = False
+        self.user.save()
+
+        response = self.get(setting=TestPermissionedGenericSetting)
+        self.assertEqual(response.status_code, 200)
+        self.assertNotIn("sensitive_email", list(response.context["form"].fields))
+
+        response = self.post(
+            setting=TestPermissionedGenericSetting,
+            post_data={"sensitive_email": "test-updated@example.com", "title": "title"},
+        )
+        self.assertEqual(response.status_code, 302)
+
+        test_setting.refresh_from_db()
+        self.assertEqual(test_setting.sensitive_email, "test@example.com")
+
 
 class TestAdminPermission(WagtailTestUtils, TestCase):
     def test_registered_permission(self):

+ 92 - 3
wagtail/contrib/settings/tests/site_specific/test_admin.py

@@ -14,6 +14,7 @@ from wagtail.test.testapp.models import (
     IconSiteSetting,
     PanelSiteSettings,
     TabbedSiteSettings,
+    TestPermissionedSiteSetting,
     TestSiteSetting,
 )
 from wagtail.test.utils import WagtailTestUtils
@@ -72,6 +73,11 @@ class BaseTestSiteSettingView(WagtailTestUtils, TestCase):
 class TestSiteSettingCreateView(BaseTestSiteSettingView):
     def setUp(self):
         self.user = self.login()
+        self.user.user_permissions.add(
+            Permission.objects.get(
+                content_type__app_label="wagtailadmin", codename="access_admin"
+            )
+        )
 
     def test_get_edit(self):
         response = self.get()
@@ -103,18 +109,55 @@ class TestSiteSettingCreateView(BaseTestSiteSettingView):
         # Ensure the form supports file uploads
         self.assertContains(response, 'enctype="multipart/form-data"')
 
+    def test_create_restricted_field_without_permission(self):
+        self.user.is_superuser = False
+        self.user.save()
+
+        self.assertFalse(TestPermissionedSiteSetting.objects.exists())
+        response = self.post(
+            post_data={"sensitive_email": "test@example.com", "title": "test"},
+            setting=TestPermissionedSiteSetting,
+        )
+        self.assertEqual(response.status_code, 302)
+
+        settings = TestPermissionedSiteSetting.objects.get()
+        self.assertEqual(settings.title, "test")
+        self.assertEqual(settings.sensitive_email, "")
+
+    def test_create_restricted_field(self):
+        self.user.is_superuser = False
+        self.user.save()
+        self.user.user_permissions.add(
+            Permission.objects.get(codename="can_edit_sensitive_email_site_setting")
+        )
+        self.assertFalse(TestPermissionedSiteSetting.objects.exists())
+        response = self.post(
+            post_data={"sensitive_email": "test@example.com", "title": "test"},
+            setting=TestPermissionedSiteSetting,
+        )
+        self.assertEqual(response.status_code, 302)
+
+        settings = TestPermissionedSiteSetting.objects.get()
+        self.assertEqual(settings.title, "test")
+        self.assertEqual(settings.sensitive_email, "test@example.com")
+
 
 class TestSiteSettingEditView(BaseTestSiteSettingView):
     def setUp(self):
-        default_site = Site.objects.get(is_default_site=True)
+        self.default_site = Site.objects.get(is_default_site=True)
 
         self.test_setting = TestSiteSetting()
         self.test_setting.title = "Site title"
         self.test_setting.email = "initial@example.com"
-        self.test_setting.site = default_site
+        self.test_setting.site = self.default_site
         self.test_setting.save()
 
-        self.login()
+        self.user = self.login()
+        self.user.user_permissions.add(
+            Permission.objects.get(
+                content_type__app_label="wagtailadmin", codename="access_admin"
+            )
+        )
 
     def test_get_edit(self):
         response = self.get()
@@ -167,6 +210,52 @@ class TestSiteSettingEditView(BaseTestSiteSettingView):
         response = self.client.get(url)
         self.assertRedirects(response, status_code=302, expected_url="/admin/")
 
+    def test_edit_restricted_field(self):
+        test_setting = TestPermissionedSiteSetting()
+        test_setting.sensitive_email = "test@example.com"
+        test_setting.site = self.default_site
+        test_setting.save()
+        self.user.is_superuser = False
+        self.user.save()
+
+        self.user.user_permissions.add(
+            Permission.objects.get(codename="can_edit_sensitive_email_site_setting")
+        )
+
+        response = self.get(setting=TestPermissionedSiteSetting)
+        self.assertEqual(response.status_code, 200)
+        self.assertIn("sensitive_email", list(response.context["form"].fields))
+
+        response = self.post(
+            setting=TestPermissionedSiteSetting,
+            post_data={"sensitive_email": "test-updated@example.com", "title": "title"},
+        )
+        self.assertEqual(response.status_code, 302)
+
+        test_setting.refresh_from_db()
+        self.assertEqual(test_setting.sensitive_email, "test-updated@example.com")
+
+    def test_edit_restricted_field_without_permission(self):
+        test_setting = TestPermissionedSiteSetting()
+        test_setting.sensitive_email = "test@example.com"
+        test_setting.site = self.default_site
+        test_setting.save()
+        self.user.is_superuser = False
+        self.user.save()
+
+        response = self.get(setting=TestPermissionedSiteSetting)
+        self.assertEqual(response.status_code, 200)
+        self.assertNotIn("sensitive_email", list(response.context["form"].fields))
+
+        response = self.post(
+            setting=TestPermissionedSiteSetting,
+            post_data={"sensitive_email": "test-updated@example.com", "title": "title"},
+        )
+        self.assertEqual(response.status_code, 302)
+
+        test_setting.refresh_from_db()
+        self.assertEqual(test_setting.sensitive_email, "test@example.com")
+
 
 @override_settings(
     ALLOWED_HOSTS=["testserver", "example.com", "noneoftheabove.example.com"]

+ 0 - 19
wagtail/snippets/views/snippets.py

@@ -233,22 +233,6 @@ class CreateView(generic.CreateEditViewOptionalFeaturesMixin, generic.CreateView
     def _get_action_menu(self):
         return SnippetActionMenu(self.request, view=self.view_name, model=self.model)
 
-    def _get_initial_form_instance(self):
-        instance = self.model()
-
-        # Set locale of the new instance
-        if self.locale:
-            instance.locale = self.locale
-
-        return instance
-
-    def get_form_kwargs(self):
-        return {
-            **super().get_form_kwargs(),
-            "instance": self._get_initial_form_instance(),
-            "for_user": self.request.user,
-        }
-
     def get_side_panels(self):
         side_panels = [
             SnippetStatusSidePanel(
@@ -308,9 +292,6 @@ class EditView(generic.CreateEditViewOptionalFeaturesMixin, generic.EditView):
             locked_for_user=self.locked_for_user,
         )
 
-    def get_form_kwargs(self):
-        return {**super().get_form_kwargs(), "for_user": self.request.user}
-
     def get_side_panels(self):
         side_panels = [
             SnippetStatusSidePanel(

+ 42 - 0
wagtail/test/testapp/migrations/0037_testpermissionedgenericsetting_and_more.py

@@ -0,0 +1,42 @@
+# Generated by Django 4.2.11 on 2024-04-25 15:51
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('wagtailcore', '0093_uploadedfile'),
+        ('tests', '0036_complexdefaultstreampage'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='TestPermissionedGenericSetting',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('title', models.CharField(max_length=100)),
+                ('sensitive_email', models.EmailField(max_length=50)),
+            ],
+            options={
+                'permissions': [('can_edit_sensitive_email_generic_setting', 'Can edit sensitive email generic setting.')],
+            },
+        ),
+        migrations.AlterModelOptions(
+            name='featurecompletetoy',
+            options={'permissions': [('can_set_release_date', 'Can set release date')]},
+        ),
+        migrations.CreateModel(
+            name='TestPermissionedSiteSetting',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('title', models.CharField(max_length=100)),
+                ('sensitive_email', models.EmailField(max_length=50)),
+                ('site', models.OneToOneField(editable=False, on_delete=django.db.models.deletion.CASCADE, to='wagtailcore.site')),
+            ],
+            options={
+                'permissions': [('can_edit_sensitive_email_site_setting', 'Can edit sensitive email site setting.')],
+            },
+        ),
+    ]

+ 46 - 0
wagtail/test/testapp/models.py

@@ -1685,6 +1685,49 @@ class TestGenericSetting(BaseGenericSetting):
     email = models.EmailField(max_length=50)
 
 
+@register_setting
+class TestPermissionedGenericSetting(BaseGenericSetting):
+    title = models.CharField(max_length=100)
+    sensitive_email = models.EmailField(max_length=50)
+
+    panels = [
+        FieldPanel("title"),
+        FieldPanel(
+            "sensitive_email",
+            permission="tests.can_edit_sensitive_email_generic_setting",
+        ),
+    ]
+
+    class Meta:
+        permissions = [
+            (
+                "can_edit_sensitive_email_generic_setting",
+                "Can edit sensitive email generic setting.",
+            ),
+        ]
+
+
+@register_setting
+class TestPermissionedSiteSetting(BaseSiteSetting):
+    title = models.CharField(max_length=100)
+    sensitive_email = models.EmailField(max_length=50)
+
+    panels = [
+        FieldPanel("title"),
+        FieldPanel(
+            "sensitive_email", permission="tests.can_edit_sensitive_email_site_setting"
+        ),
+    ]
+
+    class Meta:
+        permissions = [
+            (
+                "can_edit_sensitive_email_site_setting",
+                "Can edit sensitive email site setting.",
+            ),
+        ]
+
+
 @register_setting
 class ImportantPagesSiteSetting(BaseSiteSetting):
     sign_up_page = models.ForeignKey(
@@ -2274,6 +2317,9 @@ class FeatureCompleteToy(index.Indexed, models.Model):
     def __str__(self):
         return f"{self.name} ({self.release_date})"
 
+    class Meta:
+        permissions = [("can_set_release_date", "Can set release date")]
+
 
 class PurgeRevisionsProtectedTestModel(models.Model):
     revision = models.OneToOneField(

+ 1 - 1
wagtail/test/testapp/views.py

@@ -229,7 +229,7 @@ class FeatureCompleteToyViewSet(ModelViewSet):
 
     panels = [
         FieldPanel("name"),
-        FieldPanel("release_date"),
+        FieldPanel("release_date", permission="tests.can_set_release_date"),
     ]