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

Composed permissions should be lazily evaluated #6463

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

carltongibson commented Feb 25, 2019

@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 Feb 25, 2019
@tomchristie
Copy link
Member

tomchristie commented Feb 25, 2019

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

@carltongibson
Copy link
Collaborator

carltongibson commented Feb 25, 2019

Ta!

@carltongibson carltongibson removed the request for review from tomchristie Feb 25, 2019
Copy link
Collaborator

@carltongibson carltongibson left a comment

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
1 check passed
oyvindkolbu added a commit to unioslo/mreg that referenced this issue 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 issue Nov 17, 2020
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.

None yet

3 participants