From 9dcfecb7c6c8285630ad271888a9ec4ba9140e3a Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Wed, 28 Oct 2015 11:25:25 -0400 Subject: [PATCH] Fixed #25622 -- Accounted for generic relations in the admin to field validation Thanks to Jonathan Liuti for the report and Tim Graham for the review. --- django/contrib/admin/options.py | 4 +++- docs/releases/1.8.6.txt | 3 +++ tests/admin_views/admin.py | 27 +++++++++++++++------------ tests/admin_views/models.py | 11 +++++++++++ tests/admin_views/tests.py | 8 ++++++++ 5 files changed, 40 insertions(+), 13 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 6ccd3d8014..c7ac2ff74f 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -413,8 +413,10 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)): ) for related_object in related_objects: related_model = related_object.related_model + remote_field = related_object.field.remote_field if (any(issubclass(model, related_model) for model in registered_models) and - related_object.field.remote_field.get_related_field() == field): + hasattr(remote_field, 'get_related_field') and + remote_field.get_related_field() == field): return True return False diff --git a/docs/releases/1.8.6.txt b/docs/releases/1.8.6.txt index 3383e253d0..fef0e7c2d0 100644 --- a/docs/releases/1.8.6.txt +++ b/docs/releases/1.8.6.txt @@ -48,3 +48,6 @@ Bugfixes * Fixed a regression in ``URLValidator`` that allowed URLs with consecutive dots in the domain section (like ``http://example..com/``) to pass (:ticket:`25620`). + +* Fixed a crash with ``GenericRelation`` and + ``BaseModelAdmin.to_field_allowed`` (:ticket:`25622`). diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index ca114680d0..2f4cc88d12 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -31,18 +31,19 @@ from .models import ( EmptyModelHidden, EmptyModelMixin, EmptyModelVisible, ExplicitlyProvidedPK, ExternalSubscriber, Fabric, FancyDoodad, FieldOverridePost, FilteredManager, FooAccount, FoodDelivery, FunkyTag, Gadget, Gallery, - Grommet, ImplicitlyGeneratedPK, Ingredient, InlineReference, InlineReferer, - Inquisition, Language, Link, MainPrepopulated, ModelWithStringPrimaryKey, - NotReferenced, OldSubscriber, OtherStory, Paper, Parent, - ParentWithDependentChildren, Person, Persona, Picture, Pizza, Plot, - PlotDetails, PluggableSearchPerson, Podcast, Post, PrePopulatedPost, - PrePopulatedPostLargeSlug, PrePopulatedSubPost, Promo, Question, Recipe, - Recommendation, Recommender, ReferencedByInline, ReferencedByParent, - RelatedPrepopulated, Report, Reservation, Restaurant, - RowLevelChangePermissionModel, Section, ShortMessage, Simple, Sketch, - State, Story, StumpJoke, Subscriber, SuperVillain, Telegram, Thing, - Topping, UnchangeableObject, UndeletableObject, UnorderedObject, - UserMessenger, Villain, Vodcast, Whatsit, Widget, Worker, WorkHour, + GenRelReference, Grommet, ImplicitlyGeneratedPK, Ingredient, + InlineReference, InlineReferer, Inquisition, Language, Link, + MainPrepopulated, ModelWithStringPrimaryKey, NotReferenced, OldSubscriber, + OtherStory, Paper, Parent, ParentWithDependentChildren, Person, Persona, + Picture, Pizza, Plot, PlotDetails, PluggableSearchPerson, Podcast, Post, + PrePopulatedPost, PrePopulatedPostLargeSlug, PrePopulatedSubPost, Promo, + Question, Recipe, Recommendation, Recommender, ReferencedByGenRel, + ReferencedByInline, ReferencedByParent, RelatedPrepopulated, Report, + Reservation, Restaurant, RowLevelChangePermissionModel, Section, + ShortMessage, Simple, Sketch, State, Story, StumpJoke, Subscriber, + SuperVillain, Telegram, Thing, Topping, UnchangeableObject, + UndeletableObject, UnorderedObject, UserMessenger, Villain, Vodcast, + Whatsit, Widget, Worker, WorkHour, ) @@ -944,6 +945,8 @@ site.register(ReferencedByParent) site.register(ChildOfReferer) site.register(ReferencedByInline) site.register(InlineReferer, InlineRefererAdmin) +site.register(ReferencedByGenRel) +site.register(GenRelReference) # We intentionally register Promo and ChapterXtra1 but not Chapter nor ChapterXtra2. # That way we cover all four cases: diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py index 1199da3973..3ecaf57b6d 100644 --- a/tests/admin_views/models.py +++ b/tests/admin_views/models.py @@ -938,3 +938,14 @@ class ExplicitlyProvidedPK(models.Model): class ImplicitlyGeneratedPK(models.Model): name = models.IntegerField(unique=True) + + +# Models for #25622 +class ReferencedByGenRel(models.Model): + content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) + object_id = models.PositiveIntegerField() + content_object = GenericForeignKey('content_type', 'object_id') + + +class GenRelReference(models.Model): + references = GenericRelation(ReferencedByGenRel) diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 501edcad57..b6db228809 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -725,6 +725,14 @@ class AdminViewBasicTest(AdminViewBasicTestCase): response = self.client.get(reverse('admin:admin_views_referencedbyinline_changelist'), {TO_FIELD_VAR: 'name'}) self.assertEqual(response.status_code, 200) + # #25622 - Specifying a field of a model only referred by a generic + # relation should raise DisallowedModelAdminToField. + url = reverse('admin:admin_views_referencedbygenrel_changelist') + with patch_logger('django.security.DisallowedModelAdminToField', 'error') as calls: + response = self.client.get(url, {TO_FIELD_VAR: 'object_id'}) + self.assertEqual(response.status_code, 400) + self.assertEqual(len(calls), 1) + # We also want to prevent the add, change, and delete views from # leaking a disallowed field value. with patch_logger('django.security.DisallowedModelAdminToField', 'error') as calls: