DisallowedModelAdminToField when using get_inline_instances()

[Tim Graham]

As reported in the comments of #23552:

models.py

class Invoice(models.Model):
    ...

class Payment(models.Model):
    ...

class InvoicePayment(models.Model):
    payment = models.OneToOneField(Payment)
    invoice = models.ForeignKey(Invoice)

admin.py:

class InvoicePaymentInline(admin.StackedInline):
    model = InvoicePayment
    extra = 1


class InvoiceAdmin(admin.ModelAdmin):
    #Using sample code from https://docs.djangoproject.com/en/1.7/ref/contrib/admin/#django.contrib.admin.ModelAdmin.get_inline_instances
    def get_inline_instances(self, request, obj=None):
        return [inline(self.model, self.admin_site) for inline in [InvoicePaymentInline]]


# Register your models here.
admin.site.register(Invoice, InvoiceAdmin)
admin.site.register(Payment, )

In admin, from invoice change_form, click on "+" button next payment in inlines display popup with "bad request 400"

[Simon Charette]

The to_field check is failing here because it only collects statically defined inline classes for its sanity checks. This could be solved by one of the following:

1. Document that get_inline_instances() should always return instances of a subset of the classes defined in inlines.
2. Publicly document to_field_allowed() and link to it get_inline_instances()'s documentation noting that it might require overriding if 1. criteria is not met.
3. Completely abandon the whole static analysis against site admin registries and only rely on model relationships to allow of deny reference to fields.

Given the many regression caused by the inner admin site references enforcement I think 3. might be our best option. Thoughts?

[Tim Graham]

How about the documentation route, at least for older versions of Django? Then we can revisit the issue using 3 if you feel that will be less problematic going forward.

[Simon Charette]

Good idea, let's document this as a requirement for older versions of Django. I should be able to come up with a draft in the next few days.

[Simon Charette]

#23839 was a duplicate.

After further analysis I think the correct solution would be to backport the fix proposed by @jphalip there which is to always allow reference to the primary key of the model instead to avoid documenting to_field_allowed() and expose a security foot-gun.

Since the admin doesn't currently work with many to many through a model pointing to a non-primary key field (see #23862) the only remaining edge case would be dealing with dynamically generated inlines with a foreign key pointing to a non-primary key field. For this case I suggest we follow comment:1 suggestion to document that get_inline_instances() should always return instances of a subset of the classes defined in inlines.

Since the initial to_field_allowed patch introduced many regression I'll try to get feedback from the developer mailing list before committing to this solution.

Patch should follow shortly.

[Simon Charette]

Here's ​the mailing list thread.

[Tim Graham <timograham@…>]

In e0d1f2684ae60573b209783f9fd4f9db163ad704:

Added warning about get_inline_instances() permission checking; refs #23754.

[Tim Graham <timograham@…>]

In c3a40af6ec50e537b82ce6ca3acf5d665db76f29:

[1.6.x] Added warning about get_inline_instances() permission checking; refs #23754.

Backport of e0d1f2684ae60573b209783f9fd4f9db163ad704 from master

[Tim Graham <timograham@…>]

In 17ffd24d9b3528c5dcb0d50e86df2b03a9b288fc:

[1.5.x] Added warning about get_inline_instances() permission checking; refs #23754.

Backport of e0d1f2684ae60573b209783f9fd4f9db163ad704 from master

[Tim Graham <timograham@…>]

In f46ebc680b4868d61aec629ceded380ef313a687:

[1.7.x] Added warning about get_inline_instances() permission checking; refs #23754.

Backport of e0d1f2684ae60573b209783f9fd4f9db163ad704 from master

[Tim Graham <timograham@…>]

In f9c4e14aeca7df79991bca8ac2d743953cbd095c:

Fixed #23754 -- Always allowed reference to the primary key in the admin

This change allows dynamically created inlines "Add related" button to work
correcly as long as their associated foreign key is pointing to the primary
key of the related model.

Thanks to amorce for the report, Julien Phalip for the initial patch,
and Collin Anderson for the review.

[Tim Graham <timograham@…>]

In 2a20bccda984e6d516bba3eb7e8f307185754a3b:

[1.7.x] Fixed #23754 -- Always allowed reference to the primary key in the admin

This change allows dynamically created inlines "Add related" button to work
correcly as long as their associated foreign key is pointing to the primary
key of the related model.

Thanks to amorce for the report, Julien Phalip for the initial patch,
and Collin Anderson for the review.

Backport of f9c4e14aeca7df79991bca8ac2d743953cbd095c from master

[Tim Graham <timograham@…>]

In 2fd16232b106cf99ed42eccf57da1eed1f815d41:

[1.6.x] Fixed #23754 -- Always allowed reference to the primary key in the admin

This change allows dynamically created inlines "Add related" button to work
correcly as long as their associated foreign key is pointing to the primary
key of the related model.

Thanks to amorce for the report, Julien Phalip for the initial patch,
and Collin Anderson for the review.

Backport of f9c4e14aeca7df79991bca8ac2d743953cbd095c from master

[Tim Graham <timograham@…>]

In 3d35ea43001991d94c192abca1832628cd255bb0:

[1.5.x] Fixed #23754 -- Always allowed reference to the primary key in the admin

This change allows dynamically created inlines "Add related" button to work
correcly as long as their associated foreign key is pointing to the primary
key of the related model.

Thanks to amorce for the report, Julien Phalip for the initial patch,
and Collin Anderson for the review.

Backport of f9c4e14aeca7df79991bca8ac2d743953cbd095c from master

[Tim Graham <timograham@…>]

In 5940da16afb314c52cf52d4aebfedb77c6cc886b:

[1.4.x] Fixed #23754 -- Always allowed reference to the primary key in the admin

This change allows dynamically created inlines "Add related" button to work
correcly as long as their associated foreign key is pointing to the primary
key of the related model.

Thanks to amorce for the report, Julien Phalip for the initial patch,
and Collin Anderson for the review.

Backport of f9c4e14aeca7df79991bca8ac2d743953cbd095c from master

[Simon Charette <charette.s@…>]

In 3a9aa155e2f7326df669953980ac87e78e932c43:

Fixed #23915 -- Made sure m2m fields through non-pk to_field are allowed in the admin.

refs #23754, #23862