Try ERPNext Buy Support Partners Foundation

[Proposal] User Permissions Simplifcation

Hello all,

The User Permissions feature in Frappe is a source of a big headache https://frappe.github.io/erpnext/user/manual/en/setting-up/users-and-permissions/user-permissions

The use case for this is to restrict users by Company or Territory or some other value in a document.

The current way to set it is to go to Role Permission Manager and check every document where you want the user permissions to be applied (for example in Quotations or Sales Invoices).

This is very hard to set and in our experience, users find it very hard to configure and maintain this.

So here is what we are proposing

  1. Remove the “Apply User Permissions” setting from Role Permissions
  2. Users permissions will apply to all documents where the link field is present. For example if you set Company=X, this will automatically apply to all documents where “company” is a field.
  3. If you want some fields to be exempt, there is still the “Ignore User Permissions” check on the field settings which you can customize via “Customize Form”

What will happen when we implement this

Once this is implemented, user permissions will automatically apply to all documents where it is set. So in a sense your current permissions will become stricter, unless you define exceptions

Benefits

  1. User Permissions will become easy to understand and design
  2. It will be a new rule-set just like Role Permissions and become independent from Role Permissions
  3. No more trial-and-error and guess work!

CC: @adityaduggal

18 Likes

This will be very good feature. It will helpful for those who want to restrict user based on Customer, Company, Territory.

We have one more difficulty in setting user permissions.

We want to make new role Custom Sales User and we want to give access to Customer, Sales Invoice only,
But it is very difficult to set access for new role as we need to ensure new role has read access to all linked masters.

To just give Sales Invoice Read access, we need to give Customer, Item, UOM, Mode Of Payment (all linked mastered) read access

Lets fix one problem at a time :sweat_smile:

1 Like

The regression I see is that there will be no way to “Ignore User Permissions” for a single role anymore. You either set permissions for all, or set for none. That seems to be constricting.

There are instances where, in a particular doctype, we have needed permissions to be applied for one role, but not another. In those cases, the “Ignore User Permissions” checkbox would not suffice. I know it is possible to say “Just Add all Values” to the user permissions manager, but that simply doesn’t scale in certain scenarios. I cant add 1000+ items to every single user who needs access to all.

How will this handle cases where the field is blank? Currently, that is dealt with using the “Ignore User Permissions If Missing” checkbox in System Settings. Will that continue to be the case?

The reason this is important is because giving Users rights to all values for a particular Territory (for example) can be done in one of two ways

  1. Assign them all territories individually in User Permissions Manager
  2. Assign them no territories at all, and the above setting takes care of what is shown

My guess is that there will be no change, but just wanted to verify.

EDIT: My solution for the regression is to just change default behavior. Default behavior for the checkbox right now is “Apply User Permissions” is unchecked. Make it checked - this would result in behavior as described in the initial post. Then for more power, users can uncheck it as needed.

1 Like

@felix thanks for the feedback!

Can you share a use case? I just want to understand if there is a better design to do it.

Well I knew a day would come when the permissions would be looked into and reworked.
The current problem with permission is that they are way too complex to setup for a new person and one needs some getting used to how the permissions work.

Now I am sure this cannot be a single thing which can cover all the scenarios but I guess that the best way would be that users like me share their use cases and the new permissions manager (permissions-engine) is designed taking in view those requirements.

Now the current proposal I don’t think we are looking into the deeper use of permission manager, like if we set permissions in the proposed way would we be able to do the following:

  1. Limit a Sales Partner to only be able to view the transaction of the customers assigned to them.
    1.1 Sales Partner has editing rights for customer master would he have the access to the Address
  2. Customers should be able to view only their transactions.
  3. How would you limit a user to only certain doctypes, like if there is a Warehouse Incharge who is only concerned about the Delivery Notes and Stock Entries for his own warehouse then how would it be implemented. Currently

Also the current permission manager becomes too lengthy if we have multiple roles and various level of fields.
Instead I was thinking that the team had an idea about the TREE Structure of Permissions (Hierarchy).

So I presume that a rework on permissions should be in the correct direction which I think that hierarchy was something better.

Current problem if a Role Sales User has editing right for Sales Order but Sales Manager does not then the role Sales User and Sales Manager both should be assigned to the user with role Sales Manager in the company.

So to make the permissions more easy to understand I would suggest 2-sets of permissions to go ahead with the initial period and an option or checkbox in the System setup to choose the new permission so that there is NO HEART burn for the existing users especially given the complexity of the current permissions which confuse the power users I can very well imagine how would it be to code such thing.

@rmehta Use case is more about framework possibilities. There’s often times where we need certain behavior in the Stock Module, and other behavior in the Sales Module. The present setup allows flexible behavior which users can tailor to their needs.

The existing permissions model allows for everything I’ve ever been able to throw at it, and it has worked (and I’ve thrown a lot at it!). But I agree that it is difficult to figure out.

Rather than redoing everything, just use Sane Defaults. By default, check “Apply User Permissions” and you get the outcome listed above. I see the outcome as trying to match user expectations with reality. A new user expects to be able to go to the User Permissions Manager, set a field/value setting, and then it works. That’s possible with the sane default.

And power users can continue as normal. They can uncheck and do whatever they want to match their business requirement.

1 Like

One use case the current scenarios is not able to handle.
Suppose a user has access to Customer ABC and hence the user is limited to view the customer master ABC.
If we give permission to this user for ADDRESS then there is no way to control which address the user is able to see since the customer or supplier is a DYNAMIC Link in a CHILD Table.

If we uncheck the apply user permission ALL address are shown and if we Check it then NONE are shown even though the user permission has Customer ABC in their user permission mentioned.

This is basically a use case which I think the new permission manager would be able to resolve (I hope it applies to Dynamic Links) other wise its of no use.

But I still think that going forward should be for Hierarchy based permission since reason for current permission engine being confusing is a user has multiple role assigned due to the current structure. Ideally speaking a user should have only 1 role assigned in a particular tree branch.

So we could have something like Chart of Accounts as per below

The proposal above would not rectify this scenario.

To rectify the scenario, you would need some kind of “Apply Permissions for Child Doctypes” option as well, and that would make things more complicated.

But don’t you think a person giving permission to a user for Customer Master and Address master with Apply Permission assume that if an address belongs to a Customer whom a user has access to edit should be able to edit the address of that particular customer as well.

Just brought up this scenario since this scenario is not getting addressed since the child table introduction in address table for links, so the child table has its advantages and disadvantages as well.

Ideally, yes. But as you said, the problem is because of the architecture of the doctypes. We can either have Address as a Parent Doctype (which it is right now) and link it to multiple doctypes, or we can have Address as a Child Doctype (which it was) and have permissions work.

Address is uniquely architected, so that has this main downfall.

1 Like

I would still like to understand a specific use case :slight_smile:[quote=“felix, post:8, topic:25641”]

Rather than redoing everything, just use Sane Defaults. By default, check “Apply User Permissions” and you get the outcome listed above. I see the outcome as trying to match user expectations with reality. A new user expects to be able to go to the User Permissions Manager, set a field/value setting, and then it works. That’s possible with the sane default.
[/quote]

So basically when you add a user permission, it should add “Apply User Permissions” on all doctypes with that field (for example, which would mean many doctypes) and then check apply user permission on Company.

So what about when someone manually unchecks on of these and then applies another User Permission? It is not clear what needs to be done…

In case you still need finer permissions (show me a use case!), maybe we can add “ignore user permissions” on certain roles, so you can explicitly create super Roles that would override user permissions. Would that work?

Hierarchical permissions is another issue. Lets first fix this user permissions :slight_smile:

Yeah - that could be an option, but then we would need two maintain different implementations… maybe a global switch will turn one of them on. I think that would satisfy @felix too

1 Like

Given:
Doctype Territory
Doctype Customer - has mandatory linked field to Territory
Doctype Sales Invoice - has optional linked field to doctype Territory

Need Regional Sales User to be able to see Customers in All Territories, but only Sales Invoice in their territory.
Need Regional Service User to be able to see customer and maintenance in only their territory

Using Current Permissions Model
Sales User: Apply user permissions on Doctype Territory and Sales Invoice. Don’t Apply User permissions on Customer.
Service User: Apply user permission on Customer and maintenance visit.

Using Permissions Model without Apply User Permissions Option
To deal with sales user, just check “Ignore User Permissions” on the territory linked field in Customer doctype.
But, that would break things for service user. So this isn’t possible.

Analysis of “Ignore User Permissions” idea
The pro is that it is consistent in terminology with “Ignore User Permissions” at the field level. Its fundamentally the same thing as “apply user permissions” though. So any cons of apply user permissions will be the same here. A user can come and check it, and then there is “unexpected” behavior.

Whatever the solution, I think there should be an ability to either “Apply User Permissions” or “Ignore User Permissions” at the role level. The main difference is to change default behavior at the role level to be more consistent with user expectations: either “Apply User Permissions” is checked, or if renamed, “Ignore User Permissions” is unchecked.

This would be the worst of all worlds! Even more confusion.More Bugs! More complexity. As an analogy, we would be building a new house to deal with bad paint rather than just repainting the existing house,

2 Likes

Ok so here goes the pull:

I have not changed the model. Basically added a check in while creating User Permission to automatically apply to all roles of that user so its easer UX.

Also User Permission is now a DocType (removed a lot of custom UI!) and made it RESTful.

Please check the docs in the Pull Request.

This will be going into develop (hopefully soon!). It should not mean any change to current users.

2 Likes

I like the option to default to “access to all doc types” when setting up a new user. But i would like to suggest something simpler to prohibit access to doc types if they are not necessary for a user. Our needs at our facility are not as specific as others needing regional access for individuals. It would be helpful through to give “accountant” access and not have to give access to sales orders, but not invoices etc. For the system to know the “accountant / bookkeeper” and the sensative docs they have access to. In contrast to a salesman who needs to create sales orders (and maybe invoices), but not see the P/L report etc. To have built into the system that common knowledge for a 1 quick selection of “ignore user permissions” for those that are not accountants, or salesmen etc.