Multiple suppliers - issue with correct displaying requisitions for certain users


(Łukasz Lewczyński) #1

Hi everyone,

Multiple supplies feature has been implemented as part of OLMIS-5142 and after first checks, I found out that there is a problem with correct displaying requisitions for certain users. Currently, both regular and partner users can see both requisitions on the list and they have access to those requisitions because both users/requisitions have the same facilities and programs.

Currently, there are three types of rights in the system:

  • General right, which contains only the right name
  • Fulfillment right, which contains the right name and warehouse id
  • supervision right, which contains the right name, facility id and program id

The only difference between regular and partner users is the supervisory node and I think its ID should be inside supervision rights. The id is required only for requisition rights so eventually, we could create a new type of rights similar to supervision right but with supervisory node id.

Also, because of that change, a supervisory node field in each requisition would need to be set when the requisition is initiated and because of that, we would need to create a cross-service migration for existing requisitions.

This is my first thoughts about how to solve the issue so I know that maybe the solution is not the best.

What are your thoughts about it?


(Chongsun Ahn) #2

Hey Łukasz,

Thanks for bringing up this topic to the community.

So I have some concerns about this approach. The role-based access control (RBAC) in the system is already complex and we should not be quick to add a new dimension to it. This would increase complexity by:

  • Adding a new dimension to permission strings - it becomes less clear what a permission string means. For permission strings with a facility and program, we can say something like “at this facility, for this program, the user can do this”. But once
    we add supervisory nodes, it is unclear. We are not simply asking if a user can do something at a supervisory node, because we use nodes to indicate a hierarchy, meaning a node also indirectly represents all nodes and facilities under it.

  • Requiring all requisitions have a supervisory node - this changes what supervisory node id in a requisition means. Currently it means that it is now in the supervisory chain and the node currently assigned is where it is in the chain. Once a node
    is assigned when created, this meaning becomes unclear; it is assigned a node even before being in the approval chain.

Another issue is that we are making a structural change in reference data, which can potentially affect multiple services (since we are talking about permissions), but it is for a specific requirement in requisition. For instance, CCE also uses
permissions to show the correct equipment and could potentially be affected by this change.

Whatever approach we take in addressing this problem, we should prefer to let reference data’s permission strings keep its definition and then use those permissions to help solve it. The problem in this case is displaying the correct requisitions
for a certain user. Can we look into the code of retrieving requisitions and add logic on how to filter based on partner nodes if a user has permission based on a partner node?

Shalom,

Chongsun

– ​

There are 10 kinds of people in this world; those who understand binary, and those who don’t.

Chongsun Ahn | chongsun.ahn@villagereach.org

Software Development Engineer

Village****Reach* ** Starting
at the Last Mile*

2900 Eastlake Ave. E, Suite 230, Seattle, WA 98102, USA

DIRECT: 1.206.512.1536 **CELL: **1.206.910.0973 FAX: 1.206.860.6972

SKYPE: chongsun.ahn.vr

www.villagereach.org

Connect on Facebook****, Twitter** ** and
our Blog


(Josh Zamor) #3

Agreed. The Requisition service already needs to use a combination of Permission Strings and knowledge of the supervisory nodes in order to show the Requisition to the correct approver. Couldn’t that be extended to multiple suppliers within the Requisition service with a minimum of cross-service complexities?

While I do want us to take priority in resolving this question and unblocking work, I also want to raise, again, that introducing partner nodes as a first class citizen is still something that concerns me: Bidirectional relation between regular and partner supervisory nodes

Best,
Josh


(Łukasz Lewczyński) #4

So I checked the code in the requisition service and each requisition has a single instance of RequisitionPermissionString class (the requisition_permission_strings table in the database) which contains a permission string which presents ability to view the given requisition. Unfortunately, in this string there are only right name, facility id and program id.

I could modify the class so it holds more data but I worry that then we would need to stop using permission strings to retrieve correct requisitions and because of that, the performance of the service can be worse than now.


(Josh Zamor) #5

@llewczynski, surely there is more than just the RequisitionPermissionString class involved - otherwise how would the Requisition Service do what it has been doing correctly all this time - only show the Requisition to the appropriate supervisor based on supervisory node?

Should we schedule a design review to track down how the service is currently working?


(Łukasz Lewczyński) #6

@joshzamor I found more details about permission check in the requisition service:

  1. when a user tries to find a requisition - so he/she uses GET requisition/search endpoint - the service retrieves user permission strings and checks if the requisition view permission is inside user’s permissions.
  2. when a user tries to move a requisition to another status - for example: submit, authorize, etc - the service calls the GET /users/{id}/hasRight endpoint to verify if the user has permission to perform the action.

For the first point we could change that and use user’s role assignments - we do something like that to retrieve correct requisitions for approval - but what about requisitions that do not have assigned supervisory node id? We can’t remove them for results because we break current approach but we can’t have them in results because of partner users. As I said in the first post we could set the field when the requisition is initiated. In this way the field would have always a value and the problem should be solved.

The second point is more difficult to solve because the endpoint does not accept supervisory node id as a parameter. We could still use the given endpoint and if there will be an answer that would allow the user to perform the action we would simply do an extra step to check if the user has correct supervisory node but in this case we back to the question what if the supervisory node id field is not set in a requisition?


(Chongsun Ahn) #7

@joshzamor does have a good point about additional complexity when introducing partner nodes into our supervisory hierarchy. I think the issue is that we use our hierarchy of nodes for two purposes:

  1. An approval chain (for requisitions)
  2. A permission structure

We introduced partner nodes for the first item, to support parallelization of approvals at a certain level in the chain, but these issues are coming up because of the second item.

@llewczynski It seems like you are on the right track, for both use cases. We may need to use a user’s role assignments to determine if a partner can view (or approve) them, matching the user’s partner node with a requisition’s currently assigned node. If a requisition doesn’t have a node assigned yet, then we wouldn’t want the partner to view (or be able to approve) those requisitions anyways.

For a regular, non-partner, user, they would be able to view (and approve) requisitions, whether or not they have a node assigned. They could view requisitions not yet in approval through permission strings, and ones in approval by matching the current supervisory node.

Hopefully this makes sense. We can also schedule a design review to discuss this in more detail.

Shalom,
Chongsun


(Łukasz Lewczyński) #8

Hi all,

I updated the OLMIS-5909 in which we will do the necessary changes required by this topic.

@joshzamor @Chongsun_Ahn To summarize our discussion:

  • we don’t change anything in the role-based access control (RBAC)
    • but we probably need to update permission string generator so if a user has a role assignment with a partner node, permission strings collection should contain strings with facilities from the original node.
  • we adjust right checks in the requisition service
    • it seems like we don’t have to change anything in the GET /requisitionForApproval because it uses user’s role assignments to return correct requisitions
    • we would need to add a check to verify if a user has access to the given supervisory node to endpoints that move a requisition from one status to another. Of course, only if the reference data service returns message that a user has permission to perform an action.
    • for GET /requisition/search endpoint I think we could use RequisitionPermissionString class and add to it a supervisoryNodeId field. When the field is set, the given requisition should be returned only if a user has a role assignment set to the node. If the field is not set, the restriction is not applied. Thanks that we would avoid creating cross-service migration and we would not need to set supervisory node from the beginning.

Any thoughts?


(Chongsun Ahn) #9

Hi @llewczynski,

  • I agree that we don’t need to change anything in the RBAC.

    • I am wondering if it’s even necessary to update the permission string generator. It really depends on how we are thinking about partner nodes, like in the other forum discussion Josh has mentioned. If we can assume that partner nodes do not affect permissions and are only used for requisition approvals with multiple suppliers, then we can leave it.
  • I am thinking we’ll need to do additional checking based on RBAC, but not necessarily adjusting right checks in the requisition service.

    • For moving a requisition along the workflow, for the approval steps, we could check either that the user has the correct requisition permission string, or that the user has the correct right and supervisory node through RBAC
    • For viewing requisitions (requisition search), we could get a list of viewable requisitions using permission strings, and then also a list of viewable requisitions based on its current supervisory node matching a user’s supervisory node in its RBAC.

Let me know if this does not make sense. Do you envision any cases where this would not be sufficient? This way, we should be able to support the existing requisition workflow while also supporting multiple suppliers, without modifying existing classes that deal with permission strings.

Shalom,
Chongsun


(Łukasz Lewczyński) #10

Thanks for reply @Chongsun_Ahn. Please, take a look on my response.

Great.

I had similar thoughts today so we can try to handle the issue without changing the permission string generator.

That is make sense for me so if I understood correctly:

  • for requisitions before approve status we would simple check if a user has a correct right for the given facility and program
  • for requisitions after or during approval we would simple check if a user has a role assignment for the given program and supervisory node

Am I right? I think we would have to do something similar when a user will try to update a requisition.

So basically we would retrieve requisitions based on permission strings and requisitions based on user’s role assignments and we would return a distinct list to the user, right?

Based on how I understood your propositions I am not sure what should happens for requisition search endpoint when a user has two role assignments:

  • assignment with approve right for a partner node and a program
  • assignment with view right for the same partner node and the same program.

Because of not changing the permission string generator, the user will not have any permission strings (because partner nodes do not have requisition groups and because of that they do not have facilities) and based on role assignments, the user will be able to see a requisition only if supervisory node in the requisition would not be changed (because of workflow). In other words:

  1. a user see a requisition in approve status because user’s role assignments contains a program and a supervisory node from the requisition.
  2. the user modify the requisition (set some values in approve column) and approve it. Because of that supervisory node in the requisition has been changed to the parent node.
  3. the user can’t see the requisition because the user does not have any permission strings and because the user does not have any role assignments for the parent node but I assume that the user should be able to see the requisition after approving.

Also, I am a little worry about users that have only submit/authorize rights because I am not sure what they should see after a requisition has been split. I mean should they see original requisition only? Both requisitions?


(Chongsun Ahn) #11

Hey @llewczynski,

Yes, I think we are on the same page about the approach.

As for the concerns about being able to see the requisition if it moves on to a parent partner node, that is a potential issue, as we are showing requisitions based on the current node. However, I am wondering if that is a case we need to worry about at the moment. Do supply partners have multiple approval levels? If not, we may not have to deal with it at this point.

And for the users that only have submit/authorize rights, I would assume they would not have those rights for a partner node, but for a “regular” node, and therefore would be able to see both original and partner requisitions.

Shalom,
Chongsun


(Łukasz Lewczyński) #12

Hi @Chongsun_Ahn

Great. I updated the ticket OLMIS-5909

Unfortunately, I don’t have knowledge if supply partners have multiple approval levels but we could assume that now we would handle only single approval level so a partner requisition after the split will have APPROVED status.

Yes, based on permission strings they would see both requisitions. I assume we are okay with that.

In the end, I would say we are ready to start working on the ticket. If you think like more details are should be placed in the ticket description feel free to do that.


(Sammie Im) #13

@Elias_Muluneh based on your experience, is there a need to support more than one level of approval for the supply partner’s requisition? We currently do not support this.
FYI @llewczynski @Chongsun_Ahn


(Elias Muluneh) #14

@sam.im @llewczynski @Chongsun_Ahn my apologies for the late response.

We implemented a one level approval for a supply partner in Zambia. The supply partner is mostly dealing with a small subset of products, there was no need for more than one level of approval. It is also important to note that the main programs were also configured with 2 level approvals. Since the splitting happened after the first approval, both the supply partner requisition as well as the original requisition will receive only one more approval from different users.

That said, had the configuration of the main program been more than 2 levels of approval, and the supply partner that requires it’s own approval levels, I think there is a potential for needing multiple levels of approval. There is no concrete reason that I am aware of that rules out multiple level of approvals for supply partners.