Skip to content

Composed permissions should be lazily evaluated #6463

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

Merged
merged 1 commit into from
Feb 25, 2019

Conversation

FMCorz
Copy link
Contributor

@FMCorz FMCorz commented Feb 20, 2019

When composing permissions, all permissions are evaluated even when the outcome is already set. This can lead to errors when the order of the permission was logically set.

For example with IsReadonly | IsAssumingNotReadonly, if IsReadonly responds True for GET requests, logically IsAssumingNotReadonly can assume that the method is not GET. Presently, this is not the case as IsAssumingNotReadonly is being evaluated.

The patch suggests switching to lazy or and and instead of the bitwise operators.

(Initial report)

@carltongibson
Copy link
Collaborator

@tomchristie: Tests use assert_called_once() which is new in Python 3.6 (hence initial failures). I've added a skipif() annotation. (I don't see the behaviour of and and/or or being version dependent.) You OK with that? (If not how do you want to go?) Ta!

@carltongibson carltongibson added this to the 3.9.2 Release milestone Feb 25, 2019
@carltongibson carltongibson requested review from tomchristie and removed request for xordoquy February 25, 2019 11:15
@tomchristie
Copy link
Member

Seems p. reasonable to me yup. Happy with it when you are.

@carltongibson
Copy link
Collaborator

Ta!

@carltongibson carltongibson removed the request for review from tomchristie February 25, 2019 11:16
Copy link
Collaborator

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this looks good. I'll just wait for CI. Thanks @FMCorz! Welcome aboard. 🎉

@FMCorz
Copy link
Contributor Author

FMCorz commented Feb 25, 2019

Thanks everyone, glad to being able to contribute!

@carltongibson carltongibson merged commit 94fbfcb into encode:master Feb 25, 2019
oyvindkolbu added a commit to unioslo/mreg that referenced this pull request Mar 28, 2019
Also adds a small nice feature which is handy now when coding
authorization: encode/django-rest-framework#6463

While here update the other requirements to latest.
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request Nov 17, 2020
sigvef pushed a commit to sigvef/django-rest-framework that referenced this pull request Dec 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants