search

onaio / rdt-standard

RDT Open Guidelines Application
Apache License 2.0
1 stars 5 forks source link

User tied to health facility A should not be able to see data entered by user tied to health facility B #931

Open mwkhan1983 opened 3 years ago

mwkhan1983 commented 3 years ago

Steps to reproduce:

  1. Login from rarang_lotim1, create a patient record "Patient A"
  2. Sync data
  3. Login from kotaraja_lotim2
  4. Patient A is see on client list

Screenshot_20210319-144916

In above SS, I am logged in from kotaraja_lotim2 but can see client "Patient Lotim1" created from user rarang_lotim1

sid-yussin commented 3 years ago

Hi @vincent-karuri, I found in core.client table that every client have teamId and locationId. And in the app, RDTSyncConfiguration, client is filtered by TEAM_ID. So if I'm not mistaken, currently there is only 2 filter options: team and location. How to add the filter by health facility?

vincent-karuri commented 3 years ago

Do we need to filter by health facility? Each team is tied to a health facility so syncing by team should ideally have the same result.

Are the correct teams assigned to the users in the DB? Are the correct teams synced on the device for each user?

sid-yussin commented 3 years ago

I think the teams its not assigned to the correct users. In which table these information are stored (team and health facility)?

vincent-karuri commented 3 years ago

check the team schema on postgres

anuraj-shankar commented 3 years ago

When testing this please use a dummy data intentionally constructed with the scenarios you want to test. That will make the verification faster and possibly more complete, rather than using the actual field data we have now.

anuraj-shankar commented 3 years ago

Once the dummy data check out, then move to the field data.

sid-yussin commented 3 years ago

Done. The problem is every health worker users are member of health facility and DHO, and supervisor users only member of DHO.

For example we have user A and B as health worker, and C as supervisor. And organization named X and Y as health facility, and Z as DHO. A is member of X and Z. B is member of Y and Z. C is member of Z. If A registered a patient M, then B can see M in the list, so do C. Because A, B, and C is under the same organization Z. And in database, M is linked to organization Z. Why not X? I'm not sure, but possibly because Z is stored first before X. Order matter.

The solution is to reverse the link. Every health worker users are only member of health facility, and supervisor users are member of DHO and health facilities.

@mwkhan1983 please check, I have updated the prod DB. And then you can close this issue.

mwkhan1983 commented 3 years ago

@vincent-karuri do you have any questions on https://github.com/onaio/rdt-standard/issues/931#issuecomment-810825225 @yussin-hidayat I will check on prod build and let you know the outcome of testing.

vincent-karuri commented 3 years ago

@yussin-hidayat thanks, a few questions:

  1. What change was made in the database?
  2. What is meant by A is a member of X and Z, how is that represented in the database?
  3. On the Android Client, did you confirm that sync is happening based on TEAM_ID? In that case, is the correct TEAM_ID used?
  4. On the events generated after form submission, is the correct TEAM_ID, TEAM, and location populated on each event?
sid-yussin commented 3 years ago
  1. Fixed wrong mapping on team.practitioner_role table.
  2. A is team.practitioner, for example user "rarang_lotim1". X is team.organization, for example "Puskesmas Rarang". And Z also team.organization, for example "Dinas Kesehatan Kabupaten Lombok Timur".
  3. Yes.
  4. Yes.
vincent-karuri commented 3 years ago

ok, what was the wrong mapping that was fixed in 1? were there two entries for A, one associating them with X and another with Z?

sid-yussin commented 3 years ago

As I stated above, A is member of X and Z, C is member of Z. I have moved X to C, so A is member of Z, C is member of X and Z.

vincent-karuri commented 3 years ago

I'm trying to understand how it was determined that A is a member of both X and Z organizations? What in the team.practitioner_role table suggests that?

vincent-karuri commented 3 years ago

Were there 2 entries for A?

sid-yussin commented 3 years ago

Look carefully, its a mapping table between practitioner and organization.

vincent-karuri commented 3 years ago

@mwkhan1983 you can test on the device, if it works then it's fine. I still can't tell what changes were made - don't have DB access.

@yussin-hidayat also, I don't think it is necessary to map the supervisor to each organization (not sure how this is achieved either way). Them being mapped to a higher location already allows them to sync information from the health centers by location.

sid-yussin commented 3 years ago

I have tried twice to filter by location with different data mapping, all failed. You can try yourself. Is this another bug?

mwkhan1983 commented 3 years ago

@yussin-hidayat I did the testing. Tested with users rarang_lotim1 , kotaraja_lotim2 , sidbetatester1 and hunduhon_banggai1 . All mentioned users could only see their own created data which is the desired behavior. However, a word of caution. I could only check from front end. We should make sure that data is correctly mapped in the backend database as well.

Also I found an issue. I am unable to login with dinkes_lotim and dinkes_banggai. Previously I was able to login with these users. It seems that supervisor users has some issue. Please check that at your end. We can discuss more in tomorrow's call if you like.

sid-yussin commented 3 years ago

@mwkhan1983 maybe the data is not synced yet? If yes, then you need to force it by pressing "sinkronisasi" button.

anuraj-shankar commented 3 years ago

Ellya reported that we have separation of facility data, but that hierarchical sharing is still not completed. Please confirm.

mwkhan1983 commented 3 years ago

@yussin-hidayat thanks for sharing supervisor usernames. Was able to login after that. However, user lotim_spv is not able to see data created by rarang_lotim1 or kotaraja_lotim2

mwkhan1983 commented 3 years ago

Ellya reported that we have separation of facility data, but that hierarchical sharing is still not completed. Please confirm.

Yes. this is correct

anuraj-shankar commented 3 years ago

Ok, what's next step in the solution?

anuraj-shankar commented 3 years ago

Is there a document that describes the data sharing architecture or permissions settings?

anuraj-shankar commented 3 years ago

@vincent-karuri I heard from Ellya today that you were not sure how to fix this issue. Could you share any details on what or where you are thinking the problem might be? We can then use that as a starting point.

vincent-karuri commented 3 years ago

@anuraj-shankar could you provide a detailed use case of what exactly the "supervisor app" should be able to do in terms of viewing data?

Bulleted points of how the data would flow in different scenarios will be helpful here.

@yussin-hidayat already has context on how to solve the particular use case he mentioned during our call. So for clarity, we should document the requirements here in case it's something different we're talking about.

anuraj-shankar commented 3 years ago

@anuraj-shankar could you provide a detailed use case of what exactly the "supervisor app" should be able to do in terms of viewing data?

Bulleted points of how the data would flow in different scenarios will be helpful here.

@yussin-hidayat already has context on how to solve the particular use case he mentioned during our call. So for clarity, we should document the requirements here in case it's something different we're talking about.

@vincent-karuri Could you share any details on what or where you are thinking the problem might be? We can then use that as a starting point.

anuraj-shankar commented 3 years ago

@vincent-karuri we have discussed this a few times. We need a hierarchical permissions structure for the data wherein if there are users in health clinic A, and in health clinic B, then all data are accessible for persons within A and within B but not shared by A and B, but person at another higher level C can access data from both A and B. This is a very common issue in app access, and many solutions. I Googled "diagram of hierarchical permissions" and this URL came up that describes the issue and some solutions are suggested. https://stackoverflow.com/questions/7489710/managing-user-permissions-with-a-hierarchy/12898457

anuraj-shankar commented 3 years ago

@yussin-hidayat and Ellya, is this what you were thinking as well? Seems to match the needed scenario described above by @yussin-hidayat.

anuraj-shankar commented 3 years ago

Done. The problem is every health worker users are member of health facility and DHO, and supervisor users only member of DHO.

For example we have user A and B as health worker, and C as supervisor. And organization named X and Y as health facility, and Z as DHO. A is member of X and Z. B is member of Y and Z. C is member of Z. If A registered a patient M, then B can see M in the list, so do C. Because A, B, and C is under the same organization Z. And in database, M is linked to organization Z. Why not X? I'm not sure, but possibly because Z is stored first before X. Order matter.

The solution is to reverse the link. Every health worker users are only member of health facility, and supervisor users are member of DHO and health facilities.

@mwkhan1983 please check, I have updated the prod DB. And then you can close this issue.

@vincent-karuri This seems clear to me. Was there some ambiguity here that needed clarification?

vincent-karuri commented 3 years ago

@vincent-karuri we have discussed this a few times. We need a hierarchical permissions structure for the data wherein if there are users in health clinic A, and in health clinic B, then all data are accessible for persons within A and within B but not shared by A and B, but person at another higher level C can access data from both A and B. This is a very common issue in app access, and many solutions. I Googled "diagram of hierarchical permissions" and this URL came up that describes the issue and some solutions are suggested. https://stackoverflow.com/questions/7489710/managing-user-permissions-with-a-hierarchy/12898457

@anuraj-shankar then the solution to this is already explained to @yussin-hidayat. And so https://github.com/onaio/rdt-standard/issues/931#issuecomment-816926012 is really not accurate. Please sync with SID IT team as well so that we're on the same page.

anuraj-shankar commented 3 years ago

@yussin-hidayat thanks for your inputs. @vincent-karuri seems we are all on the same page with the functionality needed. @yussin-hidayat may have crafted a solution as well. Please have a look and let's finalize this functionality.