Merge ef3d01f55f into a09bba67cd

This ensures the returned QuerySet of Revisions is always correct regardless whether it's an instance of the base model or the specific model

CHANGELOG.txt

@@ -13,7 +13,9 @@ Changelog
  * Fix: Preserve whitespace in comment replies (Elhussein Almasri)
  * Docs: Remove duplicate section on frontend caching proxies from performance page (Jake Howard)
  * Docs: Document `restriction_type` field on PageViewRestriction (Shlomo Markowitz)
+ * Docs: Document Wagtail's bug bounty policy (Jake Howard)
  * Maintenance: Use `DjangoJSONEncoder` instead of custom `LazyStringEncoder` to serialize Draftail config (Sage Abdullah)
+ * Maintenance: Refactor image chooser pagination to check `WAGTAILIMAGES_CHOOSER_PAGE_SIZE` at runtime (Matt Westcott)
 
 
 6.1 (01.05.2024)

docs/contributing/security.md

@@ -34,6 +34,12 @@ At any given time, the Wagtail team provides official security support for sever
 When new releases are issued for security reasons, the accompanying notice will include a list of affected versions.
 This list is comprised solely of supported versions of Wagtail: older versions may also be affected, but we do not investigate to determine that, and will not issue patches or new releases for those versions.
 
+## Bug Bounties
+
+Wagtail does not have a "Bug Bounty" program. Whilst we appreciate and accept reports from anyone, and will gladly give credit to you and/or your organisation, we aren't able to "reward" you for reporting the vulnerability.
+
+["Beg Bounties"](https://www.troyhunt.com/beg-bounties/) are ever increasing among security researchers, and it's not something we condone or support.
+
 ## How Wagtail discloses security issues
 
 Our process for taking a security issue from private discussion to public disclosure involves multiple steps.
@@ -46,8 +52,8 @@ On the day of disclosure, we will take the following steps:
 1. Apply the relevant patch(es) to Wagtail's codebase.
    The commit messages for these patches will indicate that they are for security issues, but will not describe the issue in any detail; instead, they will warn of upcoming disclosure.
 2. Issue the relevant release(s), by placing new packages on [the Python Package Index](https://pypi.org/project/wagtail/), tagging the new release(s) in Wagtail's GitHub repository and updating Wagtail's [release notes](../releases/index).
-3. Post a public entry on [Wagtail's blog](https://wagtail.org/blog/), describing the issue and its resolution in detail, pointing to the relevant patches and new releases, and crediting the reporter of the issue (if the reporter wishes to be publicly identified).
-4. Post a notice to the [Wagtail discussion board](https://github.com/wagtail/wagtail/discussions), [Slack workspace](https://wagtail.org/slack/) and Twitter feed ([\@WagtailCMS](https://twitter.com/wagtailcms)) that links to the blog post.
+3. Publish a [security advisory](https://github.com/wagtail/wagtail/security/advisories?state=published) on Wagtail's GitHub repository. This describes the issue and its resolution in detail, pointing to the relevant patches and new releases, and crediting the reporter of the issue (if the reporter wishes to be publicly identified)
+4. Post a notice to the [Wagtail discussion board](https://github.com/wagtail/wagtail/discussions), [Slack workspace](https://wagtail.org/slack/) and Twitter feed ([\@WagtailCMS](https://twitter.com/wagtailcms)) that links to the security advisory.
 
 If a reported issue is believed to be particularly time-sensitive -- due to a known exploit in the wild, for example -- the time between advance notification and public disclosure may be shortened considerably.
 

docs/releases/6.2.md

@@ -30,11 +30,13 @@ depth: 1
 
  * Remove duplicate section on frontend caching proxies from performance page (Jake Howard)
  * Document `restriction_type` field on PageViewRestriction (Shlomo Markowitz)
+ * Document Wagtail's bug bounty policy (Jake Howard)
 
 
 ### Maintenance
 
  * Use `DjangoJSONEncoder` instead of custom `LazyStringEncoder` to serialize Draftail config (Sage Abdullah)
+ * Refactor image chooser pagination to check `WAGTAILIMAGES_CHOOSER_PAGE_SIZE` at runtime (Matt Westcott)
 
 
 ## Upgrade considerations - changes affecting all projects

wagtail/admin/viewsets/base.py

@@ -17,6 +17,10 @@ class ViewSet(WagtailMenuRegisterable):
     For more information on how to use this class, see :ref:`using_base_viewset`.
     """
 
+    #: A special value that, when passed in a kwargs dict to construct a view, indicates that
+    #: the attribute should not be written and should instead be left as the view's initial value
+    UNDEFINED = object()
+
     #: A name for this viewset, used as the default URL prefix and namespace.
     name = None
 
@@ -42,12 +46,13 @@ class ViewSet(WagtailMenuRegisterable):
         in addition to any kwargs passed to this method. Items from get_common_view_kwargs will be
         filtered to only include those that are valid for the given view_class.
         """
+        merged_kwargs = self.get_common_view_kwargs()
+        merged_kwargs.update(kwargs)
         filtered_kwargs = {
             key: value
-            for key, value in self.get_common_view_kwargs().items()
-            if hasattr(view_class, key)
+            for key, value in merged_kwargs.items()
+            if hasattr(view_class, key) and value is not self.UNDEFINED
         }
-        filtered_kwargs.update(kwargs)
         return view_class.as_view(**filtered_kwargs)
 
     def inject_view_methods(self, view_class, method_names):

wagtail/admin/viewsets/chooser.py

@@ -29,7 +29,7 @@ class ChooserViewSet(ViewSet):
     )  #: Label for the 'choose' button in the chooser widget, when an item has already been chosen
     edit_item_text = _("Edit")  #: Label for the 'edit' button in the chooser widget
 
-    per_page = 10  #: Number of results to show per page
+    per_page = ViewSet.UNDEFINED  #: Number of results to show per page
 
     #: A list of URL query parameters that should be passed on unmodified as part of any links or
     #: form submissions within the chooser modal workflow.

wagtail/images/tests/test_admin_views.py

@@ -1681,6 +1681,22 @@ class TestImageChooserView(WagtailTestUtils, TestCase):
         response = self.get({"p": 9999})
         self.assertEqual(response.status_code, 404)
 
+    @override_settings(WAGTAILIMAGES_CHOOSER_PAGE_SIZE=4)
+    def test_chooser_page_size(self):
+        images = [
+            Image(
+                title="Test image %i" % i,
+                file=get_test_image_file(size=(1, 1)),
+            )
+            for i in range(1, 12)
+        ]
+        Image.objects.bulk_create(images)
+
+        response = self.get()
+
+        self.assertContains(response, "Page 1 of 3")
+        self.assertEqual(response.status_code, 200)
+
     def test_filter_by_tag(self):
         for i in range(0, 10):
             image = Image.objects.create(

wagtail/images/views/chooser.py

@@ -72,10 +72,15 @@ class ImageCreationFormMixin(CreationFormMixin):
 class BaseImageChooseView(BaseChooseView):
     template_name = "wagtailimages/chooser/chooser.html"
     results_template_name = "wagtailimages/chooser/results.html"
-    per_page = 12
     ordering = "-created_at"
     construct_queryset_hook_name = "construct_image_chooser_queryset"
 
+    @property
+    def per_page(self):
+        # Make per_page into a property so that we can read back WAGTAILIMAGES_CHOOSER_PAGE_SIZE
+        # at runtime.
+        return getattr(settings, "WAGTAILIMAGES_CHOOSER_PAGE_SIZE", 20)
+
     def get_object_list(self):
         return (
             permission_policy.instances_user_has_any_permission_for(
@@ -309,7 +314,6 @@ class ImageChooserViewSet(ChooserViewSet):
     preserve_url_parameters = ChooserViewSet.preserve_url_parameters + ["select_format"]
 
     icon = "image"
-    per_page = getattr(settings, "WAGTAILIMAGES_CHOOSER_PAGE_SIZE", 10)
     choose_one_text = _("Choose an image")
     create_action_label = _("Upload")
     create_action_clicked_label = _("Uploading…")

wagtail/models/__init__.py

@@ -290,7 +290,14 @@ class RevisionMixin(models.Model):
         "latest_revision",
     ]
 
-    @property
+    _revisions = GenericRelation(
+        "wagtailcore.Revision",
+        content_type_field="content_type",
+        object_id_field="object_id",
+        for_concrete_model=False,
+    )
+
+    @cached_property
     def revisions(self):
         """
         Returns revisions that belong to the object.
@@ -302,10 +309,7 @@ class RevisionMixin(models.Model):
         ``related_query_name`` of the ``GenericRelation`` and add custom logic
         (e.g. to always use the specific instance in ``Page``).
         """
-        return Revision.objects.filter(
-            content_type=self.get_content_type(),
-            object_id=self.pk,
-        )
+        return Revision.objects.for_instance(self)
 
     def get_base_content_type(self):
         parents = self._meta.get_parent_list()
@@ -915,9 +919,19 @@ class LockableMixin(models.Model):
             return BasicLock(self)
 
 
-class WorkflowMixin:
+class WorkflowMixin(models.Model):
     """A mixin that allows a model to have workflows."""
 
+    _workflow_states = GenericRelation(
+        "wagtailcore.WorkflowState",
+        content_type_field="base_content_type",
+        object_id_field="object_id",
+        for_concrete_model=False,
+    )
+
+    class Meta:
+        abstract = True
+
     @classmethod
     def check(cls, **kwargs):
         return [
@@ -990,7 +1004,7 @@ class WorkflowMixin:
         """Returns the active workflow assigned to the object."""
         return self.get_default_workflow()
 
-    @property
+    @cached_property
     def workflow_states(self):
         """
         Returns workflow states that belong to the object.
@@ -1199,9 +1213,16 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
         verbose_name=_("latest revision created at"), null=True, editable=False
     )
 
-    _revisions = GenericRelation("wagtailcore.Revision", related_query_name="page")
+    _revisions = GenericRelation(
+        "wagtailcore.Revision",
+        content_type_field="content_type",
+        object_id_field="object_id",
+        related_query_name="page",
+        for_concrete_model=False,
+    )
 
-    # Add GenericRelation to allow WorkflowState.objects.filter(page=...) queries.
+    # Override WorkflowMixin's GenericRelation to specify related_query_name
+    # so we can do WorkflowState.objects.filter(page=...) queries.
     # There is no need to override the workflow_states property, as the default
     # implementation in WorkflowMixin already ensures that the queryset uses the
     # base Page content type.
@@ -1344,7 +1365,7 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
     def __str__(self):
         return self.title
 
-    @property
+    @cached_property
     def revisions(self):
         # Always use the specific page instance when querying for revisions as
         # they are always saved with the specific content_type.
@@ -1707,7 +1728,7 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
         clean=True,
     ):
         # Raise error if this is not the specific version of the page
-        if not isinstance(self, self.specific_class):
+        if not self.is_specific:
             raise RuntimeError(
                 "page.save_revision() must be called on the specific version of the page. "
                 "Call page.specific.save_revision() instead."
@@ -2460,7 +2481,7 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
             and self.specific_class.permissions_for_user
             != type(self).permissions_for_user
         )
-        if is_overridden and not isinstance(self, self.specific_class):
+        if is_overridden and not self.is_specific:
             return self.specific_deferred.permissions_for_user(user)
         return PagePermissionTester(user, self)
 
@@ -2789,12 +2810,23 @@ class RevisionQuerySet(models.QuerySet):
         return self.exclude(self.page_revisions_q())
 
     def for_instance(self, instance):
-        return self.filter(
-            content_type=ContentType.objects.get_for_model(
-                instance, for_concrete_model=False
-            ),
-            object_id=str(instance.pk),
-        )
+        """
+        Filters to only Revisions for the given instance
+        """
+        try:
+            # Use RevisionMixin.get_base_content_type() if available
+            return self.filter(
+                base_content_type=instance.get_base_content_type(),
+                object_id=str(instance.pk),
+            )
+        except AttributeError:
+            # Fallback to ContentType for the model
+            return self.filter(
+                content_type=ContentType.objects.get_for_model(
+                    instance, for_concrete_model=False
+                ),
+                object_id=str(instance.pk),
+            )
 
 
 class RevisionsManager(models.Manager.from_queryset(RevisionQuerySet)):

wagtail/models/specific.py

@@ -118,6 +118,10 @@ class SpecificMixin:
         """
         return self.cached_content_type.model_class()
 
+    @property
+    def is_specific(self):
+        return self.specific_class is not None and isinstance(self, self.specific_class)
+
     @property
     def cached_content_type(self):
         """

wagtail/tests/test_revision_model.py

@@ -173,15 +173,15 @@ class TestRevisableModel(TestCase):
     def test_revision_cascade_on_object_delete(self):
         page = self.create_page()
         full_featured_snippet = FullFeaturedSnippet.objects.create(text="foo")
+        # The RevisionMixin should provide a default `GenericRelation` so that
+        # revisions are deleted when the object is deleted, even if the
+        # model does not explicitly define a `GenericRelation` to `Revision`.
         cases = [
-            # Tuple of (instance, cascades)
-            # For models that define a GenericRelation to Revision, the revision
-            # should be deleted when the instance is deleted.
-            (page, True),
-            (full_featured_snippet, True),
-            (self.instance, False),  # No GenericRelation to Revision
+            page,
+            full_featured_snippet,
+            self.instance,  # No explicit GenericRelation to Revision
         ]
-        for instance, cascades in cases:
+        for instance in cases:
             with self.subTest(instance=instance):
                 revision = instance.save_revision()
                 query = {
@@ -190,4 +190,4 @@ class TestRevisableModel(TestCase):
                 }
                 self.assertEqual(Revision.objects.filter(**query).first(), revision)
                 instance.delete()
-                self.assertIs(Revision.objects.filter(**query).exists(), not cascades)
+                self.assertIs(Revision.objects.filter(**query).exists(), False)

wagtail/tests/test_workflow.py

@@ -449,7 +449,7 @@ class TestPageWorkflows(WagtailTestUtils, TestCase):
         self.assertIsNone(self.object.locked_at)
         self.assertIsNone(self.object.locked_by)
 
-    def test_workflow_state_cascade_on_object_delete(self, cascades=True):
+    def test_workflow_state_cascade_on_object_delete(self):
         data = self.start_workflow()
         query = {
             "base_content_type": self.object.get_base_content_type(),
@@ -460,7 +460,7 @@ class TestPageWorkflows(WagtailTestUtils, TestCase):
             data["workflow_state"],
         )
         self.object.delete()
-        self.assertIs(WorkflowState.objects.filter(**query).exists(), not cascades)
+        self.assertIs(WorkflowState.objects.filter(**query).exists(), False)
 
 
 class TestSnippetWorkflows(TestPageWorkflows):
@@ -493,9 +493,6 @@ class TestSnippetWorkflowsNotLockable(TestSnippetWorkflows):
         self.assertEqual(workflow_state.content_object, self.object)
         self.assertEqual(workflow_state.status, "in_progress")
 
-    def test_workflow_state_cascade_on_object_delete(self):
-        # We expect the cascade to not happen as the model does not define
-        # a GenericRelation to WorkflowState. However, workflows should still
-        # work as expected.
-        # See https://github.com/wagtail/wagtail/issues/11300 for more details.
-        return super().test_workflow_state_cascade_on_object_delete(cascades=False)
+    # The ModeratedModel does not explicitly define a `GenericRelation` to
+    # `WorkflowState`, but the `WorkflowState` should still be deleted when the
+    # object is deleted (test_workflow_state_cascade_on_object_delete passes).