Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

permissions.DjangoModelPermissionsOrAnonReadOnly doesn't actually enable anonymous read-only access in 3.15 #9299

Closed
tomchristie opened this issue Mar 18, 2024 Discussed in #9298 · 3 comments · Fixed by #9332
Labels

Comments

@tomchristie
Copy link
Member

Discussed in #9298

Originally posted by lpomfrey March 18, 2024
As the title states, it seems from DRF 3.15 the permissions.DjangoModelPermissionsOrAnonReadOnly doesn't actually allow anonymous read only access as it inherits the check for the view permission on the model from permissions.DjangoModelPermissions class.

It would seem to replicate the older behaviour DjangoModelPermissionsOrAnonReadOnly should set 'GET' and 'HEAD' in the perms_map to [] (along with setting authenticated_users_only = False).

I'm not sure if this is by design and the recommended solution is to compose a set of permissions like permissions.DjangoModelPermissions | ReadOnly (providing a custom ReadOnly class), but the documentation still suggests it should work as it did in 3.14 and before.

@shammellee
Copy link

Confirmed. I thought I was going crazy.

@tomchristie
Copy link
Member Author

So... I think we should reassess and probably revert #8009, at least for a 3.15.1 release. (?)

Requiring "view" permissions is a decent idea, but it unavoidably changes an existing behaviour in a way that's going to break some existing installations.

Given that our permissions system was introduced before Django included the model "view" permission (I think?) it's not obvious that there's really a good way forward other than documenting what our built-in permissions do, and describing how to change them if needed. (Eg. how to require "view" permissions.)

Is this the most sensible course of action on this one?

@lpomfrey
Copy link

If we were ignoring existing installations, it kind of feels like the plain DjangoModelPermissions should handle the "view" permission.

Although that then raises the question of what DjangoModelPermissionsOrAnonReadOnly should do in the case of a logged in user that doesn't have the "view" permission, I'm not sure there's a common use case where you'd want an anonymous user to have more privileges than an authenticated one, but it could at first glance be expected to work either way.

So, yes, ignoring the "view" permission and documenting that is probably the most sensible route, at least in the short-term.

This was referenced Mar 21, 2024
edigiacomo added a commit to edigiacomo/django-statusboard that referenced this issue Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants