-
Notifications
You must be signed in to change notification settings - Fork 149
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
Return rule denial reasons (and improved integration with rest framework) #63
Comments
I'm not sure if the logic for returning the correct message is quite correct yet though, it works by holding onto the message for the last rule that returned false (that had messages set). This works for normal rules, and negated rules (as I override it to swap the messages over), but might fail for more complex combinations, I didn't spend much time on that aspect yet... |
I think providing messages at predicate-level a bit too fine-grained and feels like overloading the predicates. I'm not strictly opposed to your suggestion, but I'd have to see a real nice interface proposed to be convinced. BTW, would #59 be enough for your use-case too? Rule-level names similar to Django permissions do feel more natural to be honest and should be easy/quick/clean to implement. |
I think a good approach would be to allow a predicate to fail by raising a specific type of exception instead of returning |
You mean there would then be two fails to fail a predicate check?
If so, it would seem confusing to me to have two ways to achieve a similar goal. Also, the predicate itself doesn't always know which is the good or bad outcome (as you can combine them with a |
Perhaps a nicer way to avoid breaking backwards compatibility for predicates would be to return either True/False as now or True/ |
Any news regarding this functionality? |
This is a very old issue, but as it happens I will be doing some work soon (as in sometime this year) around permissions in the project that I was writing the original issue about, so the topic will arise again for me, and I'll be looking how to solve it. I wonder how the landscape looks for permissions in rules today and if anything notable changed about django rules since 2017! |
My vision of how this could work is instead of a predicate returning True or False, they could return True or str, with the str being the failure message. This would require some changes to how truthy values are handled though, as a string would be truthy even though it is meant to indicate failure in this case. |
Implementing a It probably comes down to whether there is an approach that @dfunckt would be interested in having in the codebase, and someone to implement it in that way. For my scenario I will likely use a database-backed approach as the permissions will be customizable by users. |
I've just started using these rules. Nice idea and implementation! However, I would also like to see some kind reason code or string as to why a permission wasn't granted returned or an exception thrown so I can inform the api user of his error. |
Thanks for the great library! I love the philosophy to use rules written in simple python functions over database tables.
I ran into a few troubles trying to integrate it with a django rest framework project - we don't use django permissions, which means we can't use DjangoObjectPermissions easily.
I thought to connect django-rules directly to drf permissions - ends up similar-ish to dry-rest-permissions but using django-rules.
I also wanted to be able to return custom error messages explaining why a permission was denied.
I managed to implement this by subclassing stuff in django-rules so that you can use it like this:
If the
CanArchiveTeam
does not pass, then it returns a permission denied error with the messageTeam is archived
.There is a bit more information in our project discussion.
I wanted to avoid magic naming conventions (the approach taken by dry-rest-permissions) or referring to permissions using string names - explicit imports are much clearer to me.
Actually, the only API change for rules is changing/adding a method that returns
(result, message)
instead ofresult
, and accepting messages as predicate args.My questions are:
The alternative is a fork, but I don't want to contribute to the ever fragmenting django permissions ecosystem. Thanks!
The text was updated successfully, but these errors were encountered: