-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat: #6525 Use EHR authorization for SMART Apps #6628
Merged
adunsulag
merged 6 commits into
openemr:master
from
DiscoverAndChange:feat-openemr-fix-6225-smart-app-ehr-launch
Aug 15, 2023
Merged
feat: #6525 Use EHR authorization for SMART Apps #6628
adunsulag
merged 6 commits into
openemr:master
from
DiscoverAndChange:feat-openemr-fix-6225-smart-app-ehr-launch
Aug 15, 2023
Commits on Aug 11, 2023
-
WIP openemr#6225 Use EHR authorization for SMART Apps
First stab at openemr#6225 by allowing the EHR to launch a smart app and reuse the authorization context that the currently logged in user has. I introduce a global that by default is off to allow OpenEMR administrators to enable smart apps to skip the authorization screen when the smart app is launched inside the EHR. I refactored the launch sequence now to be an on-demand generation of the launch token using javascript instead of generating the token and stuffing it into the html. I added a flag on the individual client app to turn on or off the authorization skip flag. This gives admins control over allowing apps to launch immediately inside the EHR without requiring an authorization / authentication step. I expanded the size of the SMART app window so that the app can take up more of the window and offer more functionality. I'm wondering if we should offer different size options based upon the registered intent, or if there is some mechanism the app can communicate of how much real estate it wants to take up. The app will receive any permissions it requests that was originally granted in its registration request. The flow will continue to discard permissions that were not granted in the original registration request or were reduced in a subset of the refresh token. Added to the launch token the currently authenticated user and the type of user. Right now the ehr-launch is only inside the provider side of things, but I put in infrastructure so we can later extend it to launch inside the patient portal. I originally relied on opening up the OpenEMR session cookie, but this doesn't work if the SMART app is hosted on a domain other than the domain OpenEMR is running under. Browsers will drop the cookies on the browser redirect as its not considered a first-party origin so we have to rely on the launch token being the authority for the authentication / authorization. I'm thinking that for more security in order to prevent replay attacks we need to store the generated launch token in the database, time expire it, and make it a one time use token. If a repeat is detected we should invalidate all of the current user's access tokens and do some kind of notification to the administrator. I'm not sure yet if this is warranted, but it seems like we should treat this as serious as what we do with the refresh token.
Configuration menu - View commit details
-
Copy full SHA for 3474b11 - Browse repository at this point
Copy the full SHA 3474b11View commit details
Commits on Aug 12, 2023
-
In-EHR launch use session cookies instead of token
By switching to a javascript submitted form on the authorize page to establish a first party context I was able to retrieve the session cookies and verify the user is logged in, even inside of a third party dialog. If for some reason the third party cookie doesn't exist it brings up the standard login form. The downside of this approach is a bit slower processing as an additional round trip to the client has to happen to submit the first party form. Also, switching between the openemr session an oauth2 session has to deal with brief session locking and filesystem access if that is how the php sessions are configured. Overall though this eliminates the security loophole of someone grabbing the launch token via js or some other mechanism and logging in as a user just using the launch token. Now we can tie it to the specific logged in session and close down that security issue.
Configuration menu - View commit details
-
Copy full SHA for 697b712 - Browse repository at this point
Copy the full SHA 697b712View commit details -
Configuration menu - View commit details
-
Copy full SHA for 054dcaa - Browse repository at this point
Copy the full SHA 054dcaaView commit details
Commits on Aug 14, 2023
-
Configuration menu - View commit details
-
Copy full SHA for c66fdef - Browse repository at this point
Copy the full SHA c66fdefView commit details -
Fix the kernel mispelling the authorization controller.
Configuration menu - View commit details
-
Copy full SHA for 0a98691 - Browse repository at this point
Copy the full SHA 0a98691View commit details -
Removing user uuid, added main intent tab
Somehow the rebase removed the user uuid stuff that was part of the original wip. Added a main intent tab for launching smart apps in one of the OpenEMR tab. Added a few more debug logs.
Configuration menu - View commit details
-
Copy full SHA for 47b5852 - Browse repository at this point
Copy the full SHA 47b5852View commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.