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

Cookie Support in Headers through networkConfiguration #2570

Closed
parthfloyd opened this issue Jun 13, 2024 · 7 comments
Closed

Cookie Support in Headers through networkConfiguration #2570

parthfloyd opened this issue Jun 13, 2024 · 7 comments

Comments

@parthfloyd
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
Currently it is not possible to provide a cookie using the networkConfiguration of the FhirEngine. Our usecase is for creating patient in the OpenMRS backend.

Overall, when creating a Patient in OpenMRS using POST Patient query, it is required to have a unique OpenMRS Identifier. Out of the multiple ways to create an openMRS Identifier, the most straightforward way to support offline application is to use location based identifier generation. Basically a session id is recieved by hitting a REST endpoint & in the POST FHIR request that session-id is added as a cookie. This way backend generates a unique openmrs ID for that patient.

Describe the solution you'd like
Cookie support in FhirEngine by integrating it in the networkConfiguration.
Possibly something like in this PR: Required changes for cookie support

Describe alternatives you've considered
This would be a straightforward solution. Other alternative would be to make changes in the OpenMRS backend itself.

Additional context
The location based identifier:
"identifier": { "extension": [{ "url": "http://fhir.openmrs.org/ext/patient/identifier#location", "valueReference": { "reference": "Location/2d9378e3-b99f-42af-a109-f68395141bf3", "type": "Location" } }], "type": { "text": "HSU ID" }, "value": "4444-7"

Would you like to work on the issue?
Yes

cc: @MJ1998 , @jingtang10 & ICRC-team: @icrc-fdeniger , @icrc-psousa, @icrc-jofrancisco

@MJ1998
Copy link
Collaborator

MJ1998 commented Jun 13, 2024

Thanks for your interest in this @parthfloyd

Actually you can provide your own authenticator via ServerConfiguration.

This is what I was looking for in the developer's call. Let me know if it makes sense to you.

@parthfloyd
Copy link
Collaborator Author

@MJ1998 Thanks a ton for looking into this! Actually we are using authenticator for providing the bearer token in header in the application (It's based on FHIR app examples: demo)

However our requirement is to add cookie in the header with the sessionID which indicates the user's location. We've this requirement to due to the way this unique openMRS identifier generation is handled in the backend.

This commit <MainActivity.kt in particular> gives more context on this: icrc/openmrs-android-fhir@e21843d#diff-842352ae1f3d0f4ed093a1c6cbdc3d4a8cac0ba09f6cb56cb8c3010ca7e4b78cR62

Let me know if it makes sense for networkConfiguration to have cookie support in particular or in general a customerHeader support, incase some backend application have similar requirements. or If I'm off track and we can provide cookie via authenticator.

Let me know your thoughts on this, I'd be down to work on implementation if required.

@MJ1998
Copy link
Collaborator

MJ1998 commented Jun 14, 2024

You are right.
At first I thought the application can provide their own implementation of HttpAuthenticationMethod but its a sealed class.

I am working on probable solutions. I'll get back to you.

Also, how much of a priority is this issue for you? Is it a blocker ?

@parthfloyd
Copy link
Collaborator Author

@MJ1998 That's great, Thank you so much!
Yes its P1 for us, since its for the patient save. Although it's not a blocker since we've a temporary workaround in a fork, but with this change we can stick to the official repository.

@jingtang10
Copy link
Collaborator

thanks for raising this @parthfloyd. i think we can do this, but i don't know if we should do this.

normally cookies are used for maintaining session on client side, but for restful apis, there isn't the concept of session, so they should be used sparingly. in your case i understand it's used as a seed for a unique id, if this is the case, should you be sending it as part of your http request payload?

@parthfloyd
Copy link
Collaborator Author

@jingtang10 Thank you so much for looking into this!
I agree ideally we shouldn't do this. Hence, we are changing approach & proceeding with change in the backend.

For context, even though the locationId is supplied in the payload, the current implementation of backend expects it to be passed via cookie. We are looking forward to implement change in the backend for the implementation, to accept locationId via the request payload.

cc: @icrc-fdeniger , @icrc-psousa, @icrc-jofrancisco

@jingtang10
Copy link
Collaborator

Closing - feel free to reopen if requirements change - thanks @parthfloyd!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Complete
Development

No branches or pull requests

3 participants