[Permission Refactor Proposal]create/change documents of company A, read documents of company A and B, Row level action(create/write/read) wise permission control

[szufisher]

Problem
Current permission control is mainly on following 3 levels

• role: doctype (table) level
• user permission: document (record) level
• permission level: field level

No permission(create/write/read…) defined in user permission, system applied permissions(read/write…) from all assigned roles on target doctype for filtered documents( allowed value) in user permission. so it is not possible to restricted user to create and change documents of company A, while allow the same user to read access to documents of both company A and B.

Typically in order to control data quality and promote data sharing, it is very common in business world to restrict user create/change permission on documents of his/her own organization(company) and allow the same user read access to other organizations.

Proposed solution

1. add document relevant permissions(create/write/read…) fields into User Permission DocType,
   

   

2. change user_permission.get_user_permissions function: add ptype parameter with default value read

def get_user_permissions(user=None, ptype =‘read’)

in this function retrieve the allowed value per ptype(permission)

3. change permissions.has_user_permission, to pass the ptype parameter to get_user_permission

4. change permissions.get_doc_permissions function to retrieve the restricted value from User Permissions by ptype and apply it to the where condition.

Already tested the above solution in my local instance, here I would like to check with community

1. whether this feature is really needed?
2. whether the above propose solution is acceptable?

Further Thought
Assigning allowed values to each user via user permission will be tedious if so many users to be assigned different values. SAP’s approach is assigning the org levels(Allowed doctype and value in user permission) to each role, in other words, there will be multiple different roles assigned to same set of DocTypes but different org levels. the user’s allowed values(Org Level) is derived from the roles assigned. I am also considering this approach in ERPNext.

What do you think?

Any feedback are welcomed. based on the feedback I will decide to whether and when to initiate the PR.

A little bit background
My previous PR https://github.com/frappe/frappe/pull/6582 user permission refactor which simulates SAP logic had been rejected long time ago because it makes too big changes to the existing framework, it is somewhat too complicated, this proposal seems more aligned within the existing framework.

[SubhajitDey]

I like the idea and might need this feature.

Some idea, maybe if we can incorporate a function in user permission to define whether we want to overwrite the permission(create/update,etc) or just follow the doctype/role permission.

Regards,
Subhajit

[szufisher]

Some idea, maybe if we can incorporate a function in user permission to define whether we want to overwrite the permission(create/update,etc) or just follow the doctype/role permission.

Thanks for your feedback, for the above idea, can you explain a little bit with examples?

[SubhajitDey]

Something like this.
Before the permissions checkboxes column (read,write,etc), there is a another checkbox (Label : Overwrite perhaps).

User : A
Allow : Company
Company : ACME
Overwrite : Yes
(And below fields are only shown when Overwrite = Yes)
Read : 1
Write : 0
.
So with this additional checkbox, if we want to use the default permissions from Role Permission Manager, just set Overwrite = No

Hope this clear enough

[mrn3]

I need this functionality as well - @szufisher, do you have sample code of your proposed solution (can you provide your updated get_user_permissions, has_user_permission, and get_doc_permissions functions?). Also, did you just fork from the main frappe GitHub repo (since these changes are in Core)?

[szufisher]

sorry, the old code is not available now. maybe you can based on the above idea to implement by yourself.

[mrn3]

Thanks @szufisher for your response. So could you give a bit more detail about the changes that you made? Specifically, it isn’t clear to me what the changes were that you made in step 3 and 4.

Also, did you decide not to push this PR for approval? It seems like it would be a good one to have.

[Smit_Vora]

If this is implemented, I think better/logical way to implement it would be to give company-wise roles for users.

[mrn3]

Yes, I agree @Smit_Vora - so it seems we should have some kind of “company” entity in Core frappe. So a user would have a role (let’s say read-only) at Company A. Then, Company A would have a relationship/connection to Company B. This would entitle the user to have read-only access to all the DocTypes on Company B. Is that how you are seeing it would work?

I wonder if this could be implemented out of the box from Frappe (by creating a Company A - Company B - Read Only role, or if we need to make code changes. I think it requires code changes, but still wrapping my head around how that would work, and whether we would need to add a Company DocType to Frappe Core.

[Smit_Vora]

Let me clarify what I meant:

In DocType User, we assign roles to users as below.

Further permissions are designed based on how these roles are assigned.

Currently, there is no relationship between company and these roles.

If you plan to make a core contribution, I think company wise roles for users would be a better way to go.

Definitely you will be required to code for this to work.

[mrn3]

These are my changes to implement the original solution of this ticket. Any feedback is welcome. I will probably add the override capability too. I am not sure if I will take on the challenge of getting the “company-wise roles” solution implemented for now, but I may soon do that.

[Patrick.St]

Hi @mrn3,
what is the benefit of extending UserPerm in contrast to making use of the DocShares do grant read/write/… permissions based on individual documents?

What we do right now is, that we have the combination of UserPerm, Roles and DocShares to give individual users read/write/… access to individual documents. I am just thinking, if extending the UserPerm as suggested would give us some advantage.

[mrn3]

So the original change has the benefit that I don’t have to give permission per document. With this approach, in my case, I can give a user read access to one company, and write access to another company, and then once I do that, the user can read all products/documents from the first company, and write to all products/documents on the second. This way, I don’t have to grant permission for every product that gets created. That would be really hard to manage.

[Patrick.St]

Ok, I see, thanks!

PS: In our scenario, we have just one company but we have to decide eg. based on Customer level, which users will be part of a “Customer Team” and therefore get access to the Customer (as well as associated Contacts + Addresses).