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

Add the ability to supply user-defined state key. #168

Merged
merged 2 commits into from
Feb 17, 2025

Conversation

ben-storyhealth
Copy link

Current Node examples of storing user session with a custom storage implementation include generating a unique ID, and distinguishing between user states with this unique ID. This ID is not related to the state key that is generated by the fhirclient library.

In this Pull Request, I am proposing a change where the user of this library can supply their own state key, and this could ostensibly be the same unique identifier that's used for distinguishing users in whatever custom storage solution is implemented. The benefit to this approach is that the state key is already being passed along in the redirect, so one does not have to set cookies in order to recall which user is which. This seems to be more in line with how the state parameter was intended to be used in the OAuth flow.

Current Node examples of storing user session with a custom storage implementation include generating a unique ID, and distinguishing between user states with this unique ID. This ID is not related to the state key that is generated by the fhirclient library.

In this Pull Request, I am proposing a change where the user of this library can supply their own state key, and this could ostensibly be the same unique identifier that's used for distinguishing users in whatever custom storage solution is implemented. The benefit to this approach is that the state key is already being passed along in the redirect, so one does not have to set cookies in order to recall which user is which. This seems to be more in line with how the state parameter was intended to be used in the OAuth flow.
@ben-storyhealth
Copy link
Author

@vlad-ignatov does this look like something we could merge into main? Is there anything I can do to help land this?

@vlad-ignatov vlad-ignatov merged commit 91d83c7 into smart-on-fhir:master Feb 17, 2025
4 of 5 checks passed
@vlad-ignatov
Copy link
Collaborator

Thank you! We will include this in the next release.

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.

2 participants