permissions: Allow permissions to be composed #5753
Conversation
|
Ooh. This looks quite good fun. 🙂 |
There was a problem hiding this comment.
Hey @xordoquy. This looks great.
On a standalone basis the main thing now is docs.
The tests seem about right: https://codecov.io/gh/encode/django-rest-framework/pull/5753/diff
I'm going to start pulling together the v3.8 release. This would make a great headline feature. 🎉
|
Great feature, this will make project code more simple than before. I'm wondering if it is a good idea to also include If we have an API for account registration from mobile device. example: |
I though about it but think using an explicit not is better although I haven't tested it yet. |
|
This looks wonderful. It means that example: |
|
yeah, wonderfull, what's the hold up? |
|
lack of time to update the documentation. |
|
Hey @xordoquy. I don't know if you have capacity to add a small section to the permissions docs for this? If not I'll try and have a look at it. Thanks. 👍 |
|
I apprciate the work here, but if I'm being honest I'd have to say that I'm lukewarm on us allowing users to add further indirections to the flow of determining permissions. I'd probably rather force users to write explicit custom permission classes. |
59fb01e to
cae5507
Compare
I'd sort of agree here but also consider that it raises the bar higher because you have to get a more in-depth understanding of the rest framework permissions. Anyone that don't spent a lot of time in APIs might not be able to spend a lot of time in DRF itself. |
|
I also ❤️ this. 🙂 (I think it's nice API.) One thing I'd half-pondered was including a decent example of writing a test case to make sure you'd covered the truth-table correctly when composing permissions. What's the danger here? Users getting (Of course there's always the third-party package but...) |
|
Yup, coverage should be good enough but the truth table should be better. |
Implement a system to compose permissions with and / or. This is performed by returning an `OperationHolder` instance that keeps the permission classes and type of composition (and / or). When called it will return a AND/OR instance that will then delegate the permission check to the operands.
cae5507 to
daea006
Compare
|
PR rebased & documentation updated. |
|
let me know if you think the documentation is not enough. |
|
@xordoquy I can't see the documentation changes. There's just the code + test. (Did you push? 🙂) |
|
sure, I did push. Looks like I forgot to git add the file though... |
|
There you are |
| return request.method in SAFE_METHODS | ||
|
|
||
| class ExampleView(APIView): | ||
| permission_classes = (IsAuthenticated|ReadOnly) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Let me know if this requires other changes |
|
I think it looks good to me. (Thanks for the effort @xordoquy!) |
|
Ace work @xordoquy. Lovely headline feature, that. |
* permissions: Allow permissions to be composed Implement a system to compose permissions with and / or. This is performed by returning an `OperationHolder` instance that keeps the permission classes and type of composition (and / or). When called it will return a AND/OR instance that will then delegate the permission check to the operands. * permissions: Add documentation about composed permissions * Fix documentation typo in permissions
Implement a system to compose permissions with and / or.
This is performed by returning an
OperationHolderinstance that keeps thepermission classes and type of composition (and / or).
When called it will return a AND/OR instance that will then delegate the
permission check to the operands.
This will allow permissions to look such as: