InvenTree
InvenTree copied to clipboard
Django permission group naming can be misleading
Within my work here, I'm noticing the Django permission groups tend to be a bit misleading with regards to what a general user would think they imply.
What a user might think: View: I can see the elements in this category Add: I can create new entries in this category Change: I can edit exisiting entries in this category Delete: I can remove/delete entries in this category
What actually is the case: View: I can perform GET requests Add: I can perform POST requests Change: I Can perform PATCH/PUT requests Delete: I can perform DELETE requests
Here's where theory collides with practise. We have a lot of what one would consider "Change"-defined actions performed via POST requests. State transitions and receiving line items to name two. This inevitably leads to confusion. And for some it might be a problem as they want to granularly limit creating things and managing exisiting things to different groups of employees.
I've created the issue without a template, as it's not a bug per se, but more a call for discussion on the topic.
Case in point, actually:
CUI defines PO state change buttons to be visible if roles.purchase_order.change
is true.
But clicking any button throws:
Action Prohibited
Create operation not allowed
@LavissaWoW I would agree, that a POST
request is not always the same thing as a create
operation - perhaps you can compile a list of API actions where you think the permission is incorrectly specified? We may need to either manually adjust the permissions on those endpoints, or reassess how we implement the roles
@SchrodingersGat Sure thing. I'm aware of the different apps in the system. I only need to run through all the API endpoint definitions, and I'll be able to compile a list of possible discrepancies.
- General:
- Uploading attachments
- Adding Price breaks
- Adding parameters
- Barcode
- The barcode endpoints ALL require ADD permissions
- BOM
- bom_substitute_create (Is this considered an ADD feature, or a CHANGE feature? You're changing the behaviour of a BOM line, after all.)
- Build
- build_allocate_create
- build_auto_allocate_create
- build_cancel_create
- build_complete_create
- build_create_output_create (maybe a corner case, as it's CHANGE for the BO, but ADD for the Stock Item)
- build_delete_outputs_create (Somehow this is a POST request??)
- build_finish_create (Should a CHANGE user not be able to mark his work as finished?)
- build_item_create (Not sure of the usage of this one)
- Labels
- I'm not sure if these actually matter
- Machine
- machine_restart_create (Requiring ADD for restarting machines necessary?)
- Order
- order_'any'_extra_line_create
- order_'any'_line_create
- order_'any'_cancel_create
- order_'any'_complete_create
- order_'any'_issue_create
- order_'po/ro'_receive_create
- order_so_allocate_create
- order_so_allocate_serials_create
- order_so_shipment_create
- order_so_shipment_ship_create
- Part
- part_change_category_create
- part_internal_price_create
- part_parameter_create
- part_related_create
- part_test_template_create
- Search
- search_create (why?)
- Stock
- stock_install_create (Installing a trackable item as a part of a BO should not be limited to ADD roles)
- stock_return_create
- stock_serialize_create
- stock_uninstall_create
- stock_add_create
- stock_assign_create
- stock_change_status_create
- stock_count_create
- stock_merge_create
- stock_remove_create
- stock_test_create
- stock_transfer_create
@SchrodingersGat @matmair See the above list of endpoints that I feel should not require ADD permissions, but do currently, because they're POST requests.
Any change to the permission system is a security risk and a highly breaking so it ought to be well thought out. There are a few open issues with the permission system, those should be addressed too if we require every user to reevaluate all their permissions.
FYI there are discussions on GitHub.
The big "issue" here is that we have associated our "add" role with a "POST" style request - which makes sense for simple CRUD operations but our API is much more complex than that. I think there is space to re-evalate the permissions (in line with more "logical" operations) - but this would be a change which breaks workflow for existing users at a bare minimum.
@matmair there are security concerns to be argued over requiring any meaningful actions require full POST access too. Such a system leads to a lot of users having a lot of power they might not need or "deserve"
This issue seems stale. Please react to show this is still important.