Skip to content

Commit

Permalink
Merge pull request from GHSA-w2v8-php4-p8hc
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
2 people authored and gasman committed May 1, 2024
1 parent 70b9d98 commit fa0d482
Show file tree
Hide file tree
Showing 8 changed files with 324 additions and 24 deletions.
29 changes: 29 additions & 0 deletions wagtail/admin/tests/viewsets/test_model_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -1474,6 +1474,35 @@ def test_edit_form_rendered_with_panels(self):
modal_workflow_script = soup.select_one(f'script[src="{modal_workflow_js}"]')
self.assertIsNotNone(modal_workflow_script)

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):
Expand Down
26 changes: 26 additions & 0 deletions wagtail/admin/views/generic/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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
Expand Down Expand Up @@ -616,6 +617,24 @@ def get_translations(self):
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
Expand Down Expand Up @@ -786,6 +805,13 @@ def get_translations(self):
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.
Expand Down
89 changes: 88 additions & 1 deletion wagtail/contrib/settings/tests/generic/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
PanelGenericSettings,
TabbedGenericSettings,
TestGenericSetting,
TestPermissionedGenericSetting,
)
from wagtail.test.utils import WagtailTestUtils

Expand Down Expand Up @@ -76,6 +77,11 @@ def edit_url(self, setting):
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()
Expand Down Expand Up @@ -107,14 +113,51 @@ def test_file_upload_multipart(self):
# 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):
self.test_setting = TestGenericSetting()
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()
Expand Down Expand Up @@ -153,6 +196,50 @@ def test_for_request(self):
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):
Expand Down
95 changes: 92 additions & 3 deletions wagtail/contrib/settings/tests/site_specific/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
IconSiteSetting,
PanelSiteSettings,
TabbedSiteSettings,
TestPermissionedSiteSetting,
TestSiteSetting,
)
from wagtail.test.utils import WagtailTestUtils
Expand Down Expand Up @@ -72,6 +73,11 @@ def edit_url(self, setting, site_pk=1):
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()
Expand Down Expand Up @@ -103,18 +109,55 @@ def test_file_upload_multipart(self):
# 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()
Expand Down Expand Up @@ -158,6 +201,52 @@ def test_get_redirect_to_relevant_instance_invalid(self):
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"]
Expand Down
19 changes: 0 additions & 19 deletions wagtail/snippets/views/snippets.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,22 +235,6 @@ def run_after_hook(self):
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(
Expand Down Expand Up @@ -310,9 +294,6 @@ def _get_action_menu(self):
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(
Expand Down

0 comments on commit fa0d482

Please sign in to comment.