Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow request mutation by Access checkers #140

Merged
merged 3 commits into from
Apr 18, 2023

Conversation

vivekmittal07
Copy link
Collaborator

@vivekmittal07 vivekmittal07 commented Apr 5, 2023

Description of what I changed

Add request mutation capability in AccessDecision

  • Defined getRequestMutation in the AccessDecision Interface
  • RequestMutation allows updating the request params for GET method
  • We only support mutation to the GET query params. Created Request mutation for POST requests #143 for supporting mutation to POST requests.

#122

E2E test

TESTED:
Verified using unit test on request mutation.

Checklist: I completed these to help reviewers :)

  • I have read and will follow the review process.

  • I am familiar with Google Style Guides for the language I have coded in.

    No? Please take some time and review Java and Python style guides.

  • My IDE is configured to follow the Google code styles.

    No? Unsure? -> configure your IDE.

  • I have added tests to cover my changes. (If you refactored existing code that was well tested you do not have to add tests)

  • I ran mvn clean package right before creating this pull request and added all formatting changes to my commit.

  • All new and existing tests passed.

  • My pull request is based on the latest changes of the master branch.

    No? Unsure? -> execute command git pull --rebase upstream master

Copy link
Collaborator Author

@vivekmittal07 vivekmittal07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have defined an interface on how we can support request mutation.
@bashir2 patl.

This is not complete. Sending it for initial review of the interface.
We also wanted to support updating query params for search by POST.
This is currently not supported - #87,

// TODO Check why this does not work Content-Type is application/x-www-form-urlencoded.

Should we still support this for search query updates now or just add this as an AI to the above bug ?

Copy link
Collaborator

@bashir2 bashir2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @vivekmittal07 for the suggested interface. I think this is generally fine, I have just made some minor suggestions.

@bashir2
Copy link
Collaborator

bashir2 commented Apr 12, 2023

I have defined an interface on how we can support request mutation. @bashir2 patl.

This is not complete. Sending it for initial review of the interface. We also wanted to support updating query params for search by POST. This is currently not supported - #87,

// TODO Check why this does not work Content-Type is application/x-www-form-urlencoded.

Should we still support this for search query updates now or just add this as an AI to the above bug ?

Just replied on #87; I think search by POST works for HAPI.

@vivekmittal07 vivekmittal07 changed the title Define access decision preprocess stage to mutate the request params Allow request mutation by Access checkers Apr 13, 2023
Copy link
Collaborator

@bashir2 bashir2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @vivekmittal07 for the changes. I just asked for reverting the static change; please feel free to merge this after addressing that.

@vivekmittal07 vivekmittal07 merged commit 6a557a0 into google:main Apr 18, 2023
1 check passed
@vivekmittal07 vivekmittal07 deleted the request-mutation branch April 19, 2023 05:08
ndegwamartin pushed a commit to opensrp/fhir-gateway that referenced this pull request May 15, 2023
…gle FHIR Gateway repo (#29)

* Allow request mutation by Access checkers (google#140)

* Define access decision preprocess stage to mutate the request params

* Documentation for request mutation change. Added test cases.

* Removed static identifier from request mutation method

* Updated the scan paths for demo app to load the core beans (google#132)

Add default scan paths in the exec application

* Support gzip encoding in data transfer (google#147)

* supporting gzip for client and server data transfer

* Version change for the next dev cycle (google#159)

* Fix in unit test

* Fixed unimplemented method in the OpenSRPSyncAccessDecision

---------

Co-authored-by: vivekmittal07 <105198504+vivekmittal07@users.noreply.github.com>
Co-authored-by: anchita-g <109063673+anchita-g@users.noreply.github.com>
Co-authored-by: Bashir Sadjad <bashir@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants