Allow request mutation based on Access decision

[vivekmittal07]

Parent bug - #108

ONA enabled preprocessing of request for an access decision - opensrp@09584ef.

We wanted to support Query rewrite option in the design doc - https://github.com/google/fhir-gateway/blob/main/doc/design.md

[vivekmittal07]

This is not straight forward task as the AccessChecker design explicitly prevents request mutation.

[bashir2]

So yeah, we explicitly prevent mutating the input request by the access-checker plugin because otherwise it would be hard to reason about the query in the server code. Looking at ONA's use-case, it seems that the issues of access-control and implementing a sync-strategy are mixed together into an access-checker. That is probably the bigger question to answer whether we want to support that pattern or not; because I think sync-strategy is a separate concern.

To answer what to do for this issue, here are some options after talking with @vivekmittal07 (in the order of my personal preference):

• Encourage ONA to separate the sync-strategy and move it to the client as discussed above (so access-checker plugin does access-control only).
• Add an interface for mutating the FHIR query but only allow adding URL parameters (this is mostly for read/search queries and can be applied to both URLs and POST body content when doing search by POST). This is what we wanted to support in the original design doc too.
• Send the raw query to the access-checker and let it change however it wants.

[jjtswan]

@vivekmittal07 Any thoughts on the approaches above? Do we think that Bashir's first option will work in their currently designed architecture?

[vivekmittal07]

Yes this can work but need significant changes in the client by ONA. We will have to discuss with them if this is something they are willing to do.

[jjtswan]

Can we clear this up quickly? Changing out client-APIs can be done, but gets harder once something is deployed and gets more rolled out.

[vivekmittal07]

Had a discussion with ONA team.

Notes -
The approach of moving the sync filters to client is not simple also it has performance impact on the client.
eg- App will just know the immediate location of the practitioner but sync logic requires fetching all the child locations.
Number of such child locations can be of the order of a few hundred and they don't want to add additional step before sync. This will have impact on clients where internet connectivity is low.

They are also thinking of using FHIR gateway to host their custom endpoints. This is ideally outside of the scope of FHIR gateway but they don't want to maintain different servers. We will have to evaluate how we should make the Gateway more flexible for these usecases

[vivekmittal07]

We decided to proceed with 2nd option mentioned in #122 (comment)

Supporting custom endpoints is not straightforward and it will be good to see how we can support this. This is outside the scope of the bug. I will create a new feature request for this.

[bashir2]

Thanks @vivekmittal07 for the updates, I went ahead and created #139 for separating the custom endpoint question. That part is not a Beta blocker. So let's focus this issue on request mutation question, especially using extra URL params.

[jjtswan]

This bug was closed, but I cannot tell if we decided to drop it, or if it was resolved? If the latter, can you associate the PR?

[vivekmittal07]

The changes are already merged and the PR is attached above - #140