This issue has come up a few times in the past, but it has never gotten much traction. I am bringing it up again because Frappe/ERPNext is at the center of some very heated discussion right now, taking place on a professional listserv I belong to.
At one organization, a glitch in the User Permissions creation process lead to an employee being able to see all of the salary slips for everyone in the organization going back several years. Those involved are blaming the design of User Permissions. Though I am usually a strident Frappe evangelist in my professional circles, in this case I am very inclined to agree with them.
Background
For those who don’t know, User Permissions provide another layer of permissions management.
- Most people are familiar with the Role Permission Manager, which allows System Managers to give different types of access to different doctypes based on Role assignments.
- User Permissions instead filter permissions by specific fields. So, a company might want department managers to see and process Expense Claims, but only for their own department. User Permissions make this possible.
The implementation of this feature is clever and flexible, using automatic scanning of link fields to allow User Permissions to extend over a wide range of doctypes automatically. That much is great.
The Problem
There is one massive flaw: User Permissions work backwards.
Role Permissions are additive. If I want my HR staff to have read permissions for all Salary Slips, I have to create a role and then a rule that grants read permissions to that role.
User Permissions, on the other hand, are subtractive. I don’t grant permission to a specific field value, but rather I take away permissions from everything but certain specified values.
The result: if I want to grant employees access to only their own salary slips, I first have to give them access to all salary slips with a Role Permission and then restrict that access to only their own with a User Permission.
This design goes against all best practices.
The Result
It’s obvious where this is going. Something went wrong. HR made a new employee, and they linked that employee to a new user account. The “Create User Permissions” checkbox on the Employee doctype was supposed to make sure that this new employee could see only their own Salary Slips, but for whatever reason those User Permissions didn’t get created properly. It might have been a server glitch, a bug in the code, or human error. Who knows.
An automated process silently failed, and as a consequence this organization experienced a significant internal data breach. This is every System Manager’s nightmare.
In hindsight, I’m not sure what they should have done differently. Maybe they should have been manually auditing their permissions more frequently. It’s easy to say that in retrospect. In any case, right now, auditing this effectively requires manual comparison of three different unlinked doctypes. This is not a good situation to be in.
Proposal
I don’t think there is any good argument for keeping User Permissions the way they currently are. The design is inherently dangerous.
But, if the consensus is to keep the current functionality, I would propose at least making a way out of it. One possible approach: in the Role Permission Manager, create a new checkbox option parallel to the “Is Owner” option that can toggle how User Permissions are interpreted:
This checkbox, if set true, would stipulate that the Role-based permission does nothing until user permissions are explicitly set. This would make User Permissions Additive, allowing System Managers to create User-specific permissions that fall back to no access (not full access like happens now).
I’ve looked into permissions.py, and this looks like a relatively lightweight change, both in terms of code complexity and performance. I’m not asking somebody else to do this work for us. I’m very willing to write the code and put it through review. I’m posting here first, however, because permissions changes are always high stakes.
If I get confirmation from the maintainers that there’s support for something like this, I’ll put together a PR. I posted about this last year and didn’t get a response, but if there is interest now I am very eager to put in the time and resources.
