WIP #6225 Use EHR authorization for SMART Apps

First stab at #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.
openemr · Jul 21, 2023 · fc9e4d7 · fc9e4d7
1 parent fab7c87
commit fc9e4d7
Show file tree

Hide file tree

Showing 12 changed files with 355 additions and 32 deletions.
diff --git a/interface/patient_file/summary/demographics.php b/interface/patient_file/summary/demographics.php
@@ -348,7 +348,7 @@ function image_widget($doc_id, $doc_catg)
 
 <head>
     <?php
-    Header::setupHeader(['common']);
+    Header::setupHeader(['common','utility']);
     require_once("$srcdir/options.js.php");
     ?>
     <script>

diff --git a/interface/smart/ehr-launch-client.php b/interface/smart/ehr-launch-client.php
@@ -0,0 +1,16 @@
+<?php
+require_once("../globals.php");
+
+$controller = new \OpenEMR\FHIR\SMART\SmartLaunchController();
+try {
+    $controller->redirectAndLaunchSmartApp($_REQUEST['intent'] ?? null
+        , $_REQUEST['client_id'] ?? null, $_REQUEST['csrf_token'] ?? null);
+} catch (CsrfInvalidException $exception) {
+    CsrfUtils::csrfNotVerified();
+} catch (AccessDeniedException $exception) {
+    (new SystemLogger())->critical($exception->getMessage(), ["trace" => $exception->getTraceAsString()]);
+    die();
+} catch (Exception $exception) {
+    (new SystemLogger())->error($exception->getMessage(), ["trace" => $exception->getTraceAsString()]);
+    die("Unknown system error occurred");
+}
diff --git a/library/globals.inc.php b/library/globals.inc.php
@@ -3192,6 +3192,12 @@ function gblTimeZones()
             '0',
             xl('Approval settings for 3rd party app/api access')
         ),
+        'oauth_ehr_launch_authorization_flow_skip' => array(
+            xl('OAuth2 EHR-Launch Authorization Flow Skip Enable App Setting'),
+            'bool',
+            '0',
+            xl('Enable an OAuth2 Client application to be configured to skip the login screen and the scope authorization screen if the user is already logged into the EHR.')
+        ),
 
         'cc_front_payments' => array(
             xl('Accept Credit Card transactions from Front Payments'),

diff --git a/library/js/utility.js b/library/js/utility.js
@@ -474,3 +474,31 @@ if (typeof top.userDebug !== 'undefined' && (top.userDebug === '1' || top.userDe
     };
 }
 
+(function(window, oeSMART) {
+    oeSMART.initLaunch = function(webroot, csrfToken) {
+        // allows this to be lazy defined
+        let xl = window.top.xl || function(text) { return text; };
+        let smartLaunchers = document.querySelectorAll('.smart-launch-btn');
+        for (let launch of smartLaunchers) {
+                launch.addEventListener('click', function (evt) {
+                    let node = evt.target;
+                    let intent = node.dataset.intent;
+                    let clientId = node.dataset.clientId;
+                    if (!intent || !clientId) {
+                        console.error("mising intent parameter or client-id parameter");
+                        return;
+                    }
+
+                    let url = webroot + '/interface/smart/ehr-launch-client.php?intent='
+                        + encodeURIComponent(intent) + '&client_id=' + encodeURIComponent(clientId)
+                        + "&csrf_token=" + encodeURIComponent(csrfToken);
+                    let title = node.dataset.smartName || JSON.stringify(xl("Smart App"));
+                    // we allow external dialog's  here because that is what a SMART app is
+                    let height = window.top.innerHeight; // do our full height here
+                    dlgopen(url, '_blank', 'modal-full', height, '', title, {allowExternal: true});
+                });
+        }
+    };
+    window.oeSMART = oeSMART;
+})(window, window.top.oeSMART || {});
+
diff --git a/sql/7_0_1-to-7_0_2_upgrade.sql b/sql/7_0_1-to-7_0_2_upgrade.sql
@@ -245,4 +245,7 @@ CREATE TABLE recent_patients (
     patients TEXT,
     PRIMARY KEY (user_id)
 ) ENGINE=InnoDB;
+
+#IfMissingColumn oauth_clients skip_ehr_launch_authorization_flow
+ALTER TABLE `oauth_clients` ADD COLUMN `skip_ehr_launch_authorization_flow` tinyint(1) NOT NULL DEFAULT '0';
 #EndIf
diff --git a/src/Common/Auth/OpenIDConnect/Entities/ClientEntity.php b/src/Common/Auth/OpenIDConnect/Entities/ClientEntity.php
@@ -54,9 +54,15 @@ class ClientEntity implements ClientEntityInterface
 
     protected $registrationDate;
 
+    /**
+     * @var bool true if the authorization flow (authentication and scope authorization) should be skipped for logged in ehr users
+     */
+    private $skipEHRLaunchAuthorizationFlow;
+
     public function __construct()
     {
         $this->scopes = [];
+        $this->skipEHRLaunchAuthorizationFlow = false;
     }
 
     public function setName($name): void
@@ -242,4 +248,17 @@ public function setLogoutRedirectUris(?string $logoutRedirectUris): void
     {
         $this->logoutRedirectUris = $logoutRedirectUris;
     }
+
+    /**
+     * Whether the ehr launch should skip the authorization flow for a logged in user.
+     * @return bool
+     */
+    public function shouldSkipEHRLaunchAuthorizationFlow() : bool
+    {
+        return $this->skipEHRLaunchAuthorizationFlow;
+    }
+
+    public function setSkipEHRLaunchAuthorizationFlow(bool $shouldSkip) {
+        $this->skipEHRLaunchAuthorizationFlow = $shouldSkip;
+    }
 }
diff --git a/src/Common/Auth/OpenIDConnect/Repositories/ClientRepository.php b/src/Common/Auth/OpenIDConnect/Repositories/ClientRepository.php
@@ -57,6 +57,7 @@ public function insertNewClient($clientId, $info, $site): bool
     {
         $user = $_SESSION['authUserID'] ?? null; // future use for provider client.
         $is_confidential_client = empty($info['client_secret']) ? 0 : 1;
+        $skip_ehr_launch_authorization_flow = $info['skip_ehr_launch_authorization_flow'] == true ? 1 : 0;
 
         $contacts = $info['contacts'];
         $redirects = $info['redirect_uris'];
@@ -90,7 +91,7 @@ public function insertNewClient($clientId, $info, $site): bool
         }
 
         // TODO: @adunsulag why do we skip over request_uris when we have it in the outer function?
-        $sql = "INSERT INTO `oauth_clients` (`client_id`, `client_role`, `client_name`, `client_secret`, `registration_token`, `registration_uri_path`, `register_date`, `revoke_date`, `contacts`, `redirect_uri`, `grant_types`, `scope`, `user_id`, `site_id`, `is_confidential`, `logout_redirect_uris`, `jwks_uri`, `jwks`, `initiate_login_uri`, `endorsements`, `policy_uri`, `tos_uri`, `is_enabled`) VALUES (?, ?, ?, ?, ?, ?, NOW(), NULL, ?, ?, 'authorization_code', ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
+        $sql = "INSERT INTO `oauth_clients` (`client_id`, `client_role`, `client_name`, `client_secret`, `registration_token`, `registration_uri_path`, `register_date`, `revoke_date`, `contacts`, `redirect_uri`, `grant_types`, `scope`, `user_id`, `site_id`, `is_confidential`, `logout_redirect_uris`, `jwks_uri`, `jwks`, `initiate_login_uri`, `endorsements`, `policy_uri`, `tos_uri`, `is_enabled`, `skip_ehr_launch_authorization_flow`) VALUES (?, ?, ?, ?, ?, ?, NOW(), NULL, ?, ?, 'authorization_code', ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
         $i_vals = array(
             $clientId,
             $info['client_role'],
@@ -111,7 +112,8 @@ public function insertNewClient($clientId, $info, $site): bool
             ($info['endorsements'] ?? null),
             ($info['policy_uri'] ?? null),
             ($info['tos_uri'] ?? null),
-            $is_client_enabled
+            $is_client_enabled,
+            $skip_ehr_launch_authorization_flow
         );
 
         return sqlQueryNoLog($sql, $i_vals, true); // throw an exception if it fails
@@ -260,6 +262,7 @@ private function hydrateClientEntityFromArray($client_record): ClientEntity
         $client->setLogoutRedirectUris($client_record['logout_redirect_uris']);
         $client->setContacts($client_record['contacts']);
         $client->setRegistrationDate($client_record['register_date']);
+        $client->setSkipEHRLaunchAuthorizationFlow($client_record['skip_ehr_launch_authorization_flow'] == "1");
         return $client;
     }
 
@@ -272,4 +275,19 @@ public function generateRegistrationClientUriPath()
     {
         return HttpUtils::base64url_encode(RandomGenUtils::produceRandomBytes(16));
     }
+
+    public function saveSkipEHRLaunchFlow(ClientEntity $client, bool $skipFlow)
+    {
+        // TODO: adunsulag do we want to eventually just have a save() method.. it would be very handy but not sure
+        // we want any oauth2 values being overwritten.
+        $skipFlowValue = $skipFlow === true ? 1 : 0;
+        $clientId = $client->getIdentifier();
+        $params = [$skipFlowValue, $clientId];
+        $res = sqlStatement("UPDATE oauth_clients SET skip_ehr_launch_authorization_flow=? WHERE client_id = ?", $params);
+        if ($res === false) {
+            // TODO: adunsulag is there a better exception to throw here in OpenEMR than runtime?
+            throw new \RuntimeException("Failed to save oauth_clients skip_ehr_launch_authorization_flow flag.  Check logs for sql error");
+        }
+        return true;
+    }
 }
diff --git a/src/Common/Auth/OpenIDConnect/SMARTSessionTokenContextBuilder.php b/src/Common/Auth/OpenIDConnect/SMARTSessionTokenContextBuilder.php
@@ -32,6 +32,7 @@ public function __construct($sessionArray = array())
     public function getEHRLaunchContext()
     {
         $launch = $this->sessionArray['launch'] ?? null;
+        $this->logger->debug("SMARTSessionTokenContextBuilder->getEHRLaunchContext() launch context is", ['launch' => $launch]);
         if (empty($launch)) {
             return [];
         }
@@ -97,6 +98,7 @@ public function getContextForScopesWithExistingContext(array $context, array $sc
     public function getContextForScopes($scopes): array
     {
         $context = [];
+        $this->logger->debug("SMARTSessionTokenContextBuilder->getContextForScopes()");
         if ($this->isStandaloneLaunchPatientRequest($scopes)) {
             $context = $this->getStandaloneLaunchContext();
         } else if ($this->isEHRLaunchRequest($scopes)) {

diff --git a/src/FHIR/SMART/ClientAdminController.php b/src/FHIR/SMART/ClientAdminController.php
@@ -139,6 +139,10 @@ public function dispatch($action, $request)
                 return $this->revokeAccessToken($clientId, $request);
             } else if ($subAction == self::REVOKE_REFRESH_TOKEN) {
                 return $this->revokeRefreshToken($clientId, $request);
+            } else if ($subAction == 'enable-authorization-flow-skip') {
+                return $this->enableAuthorizationFlowSkipAction($clientId, $request);
+            } else if ($subAction == 'disable-authorization-flow-skip') {
+                return $this->disableAuthorizationFlowSkipAction($clientId, $request);
             } else {
                 return $this->notFoundAction($request);
             }
@@ -369,8 +373,14 @@ private function renderEdit(ClientEntity $client, $request)
         $listAction = $this->getActionUrl(['list']);
         $disableClientLink = $this->getActionUrl(['edit', $client->getIdentifier(), 'disable']);
         $enableClientLink = $this->getActionUrl(['edit', $client->getIdentifier(), 'enable']);
-
+        $disableSkipAuthorizationFlowLink = $this->getActionUrl(['edit', $client->getIdentifier(), 'disable-authorization-flow-skip']);
+        $enableSkipAuthorizationFlowLink = $this->getActionUrl(['edit', $client->getIdentifier(), 'enable-authorization-flow-skip']);
         $isEnabled = $client->isEnabled();
+        $allowSkipAuthSetting = $GLOBALS['oauth_ehr_launch_authorization_flow_skip'] === '1';
+        $skipAuthorizationFlow = $client->shouldSkipEHRLaunchAuthorizationFlow();
+        if (!$allowSkipAuthSetting) {
+            $skipAuthorizationFlow = false; // globals overrides this setting
+        }
         $scopes = $client->getScopes();
 
         $formValues = [
@@ -405,6 +415,12 @@ private function renderEdit(ClientEntity $client, $request)
                 , 'checked' => $isEnabled
                 , 'value' => 1
             ],
+            'skipEHRLaunchAuthorizationFlow' => [
+                'type' => 'checkbox'
+                ,'label' => xl('Skip EHR Launch Authorization Flow')
+                , 'checked' => $skipAuthorizationFlow
+                , 'value' => 1
+            ],
             'role' => [
                 'type' => 'text'
                 ,'label' => xl("Role")
@@ -432,6 +448,10 @@ private function renderEdit(ClientEntity $client, $request)
             ],
         ];
 
+        if (!$allowSkipAuthSetting) {
+            unset($formValues['skipEHRLaunchAuthorizationFlow']);
+        }
+
         ?>
         <a href="<?php echo attr($listAction); ?>" class="btn btn-sm btn-secondary" onclick="top.restoreSession()">&lt; <?php echo xlt("Back to Client List"); ?></a>
 
@@ -440,8 +460,15 @@ private function renderEdit(ClientEntity $client, $request)
                 <h2>
                     <?php echo xlt('Edit'); ?> <em><?php echo text($client->getName()); ?></em>
                     <div class="float-right">
+                        <?php if ($allowSkipAuthSetting) : ?>
+                            <?php if ($skipAuthorizationFlow) : ?>
+                            <a href="<?php echo attr($disableSkipAuthorizationFlowLink); ?>" class="btn btn-sm btn-primary" onclick="top.restoreSession()"><?php echo xlt('Enable EHR Launch Authorization Flow'); ?></a>
+                            <?php else : ?>
+                            <a href="<?php echo attr($enableSkipAuthorizationFlowLink); ?>" class="btn btn-sm btn-danger" onclick="top.restoreSession()"><?php echo xlt('Disable EHR Launch Authorization Flow'); ?></a>
+                            <?php endif; ?>
+                        <?php endif; ?>
                         <?php if ($isEnabled) : ?>
-                        <a href="<?php echo attr($disableClientLink); ?>" class="btn btn-sm btn-primary" onclick="top.restoreSession()"><?php echo xlt('Disable Client'); ?></a>
+                        <a href="<?php echo attr($disableClientLink); ?>" class="btn btn-sm btn-danger" onclick="top.restoreSession()"><?php echo xlt('Disable Client'); ?></a>
                         <?php else : ?>
                         <a href="<?php echo attr($enableClientLink); ?>" class="btn btn-sm btn-primary" onclick="top.restoreSession()"><?php echo xlt('Enable Client'); ?></a>
                         <?php endif; ?>
@@ -1111,4 +1138,44 @@ private function renderTokenToolsFooter()
         <?php
         $this->renderFooter();
     }
+    private function enableAuthorizationFlowSkipAction(string$clientId,array$request){
+         $client = $this->clientRepo->getClientEntity($clientId);
+        if ($client === false) {
+            $this->notFoundAction($request);
+            return;
+        }
+        $message = xl('Enabled Authorization Flow Skip') . " " . $client->getName();
+        $this->handleAuthorizationFlowSkipAction($client, true, $message);
+        exit;
+    }
+    private function disableAuthorizationFlowSkipAction(string $clientId ,array $request){
+         $client = $this->clientRepo->getClientEntity($clientId);
+        if ($client === false) {
+            $this->notFoundAction($request);
+            return;
+        }
+        $message = xl('Disabled Authorization Flow Skip') . " " . $client->getName();
+        $this->handleAuthorizationFlowSkipAction($client, false, $message);
+        exit;
+    }
+    private function handleAuthorizationFlowSkipAction(ClientEntity$client,bool $skipFlow,string $successMessage){
+        $client->setSkipEHRLaunchAuthorizationFlow($skipFlow);
+        try {
+            $this->clientRepo->saveSkipEHRLaunchFlow($client, $skipFlow);
+            $url = $this->getActionUrl(['edit', $client->getIdentifier()], ["queryParams" => ['message' => $successMessage]]);
+            header("Location: " . $url);
+        } catch (\Exception $ex) {
+            $this->logger->error(
+                "Failed to save client",
+                [
+                    "exception" => $ex->getMessage(), "trace" => $ex->getTraceAsString()
+                    , 'client' => $client->getIdentifier()
+                ]
+            );
+
+            $message = xl('Client failed to save. Check system logs');
+            $url = $this->getActionUrl(['edit', $client->getIdentifier()], ["queryParams" => ['message' => $message]]);
+            header("Location: " . $url);
+        }
+    }
 }
diff --git a/src/FHIR/SMART/SMARTLaunchToken.php b/src/FHIR/SMART/SMARTLaunchToken.php
@@ -19,12 +19,24 @@
 class SMARTLaunchToken
 {
     public const INTENT_PATIENT_DEMOGRAPHICS_DIALOG = 'patient.demographics.dialog';
-    public const VALID_INTENTS = [self::INTENT_PATIENT_DEMOGRAPHICS_DIALOG];
 
+    public const INTENT_ENCOUNTER_DIALOG = 'encounter.forms.dialog';
+    public const VALID_INTENTS = [self::INTENT_PATIENT_DEMOGRAPHICS_DIALOG, self::INTENT_ENCOUNTER_DIALOG];
+
+    /**
+     * @var string|null The patient UUID If
+     */
     private $patient;
     private $intent;
     private $encounter;
 
+    private ?string $userUuid;
+
+    /**
+     * @var string The user type, either 'user' or 'patient' referring to coming from users table or patient_data
+     */
+    private string $userType;
+
     public function __construct($patientUUID = null, $encounterUUID = null)
     {
         if (isset($patientUUID) && !is_string($patientUUID)) {
@@ -35,6 +47,8 @@ public function __construct($patientUUID = null, $encounterUUID = null)
         }
         $this->patient = $patientUUID;
         $this->encounter = $encounterUUID;
+        $this->userUuid = null;
+        $this->setUserType('user');
     }
 
     /**
@@ -100,6 +114,7 @@ public function serialize()
         if (!empty($intent)) {
             $context['i'] = $intent;
         }
+        $context['sub'] = $this->userType . '/' . $this->userUuid;
 
         // no security is really needed here... just need to be able to wrap
         // the current context into some kind of opaque id that the app will pass to the server and we can then
@@ -111,7 +126,7 @@ public function serialize()
         return $launchParams;
     }
 
-    public static function deserializeToken($serialized)
+    public static function deserializeToken($serialized) : self
     {
         $token = new self();
         $token->deserialize($serialized);
@@ -138,10 +153,37 @@ public function deserialize($serialized)
         if (!empty($context['i']) && $this->isValidIntent($context['i'])) {
             $this->setIntent($context['i']);
         }
+        $sub = $context['sub'] ?? '';
+        $subParts = explode('/', $sub);
+        if (count($subParts) !== 2) {
+            throw new \InvalidArgumentException("sub field in token must be in the format of 'user/uuid' or 'patient/uuid'");
+        }
+        $this->setUserType($subParts[0]);
+        $this->setUserUuid($subParts[1]);
     }
 
     public function isValidIntent($intent)
     {
         return array_search($intent, self::VALID_INTENTS) !== false;
     }
+
+    public function setUserUuid(?string $userUuid)
+    {
+        $this->userUuid = $userUuid;
+    }
+
+    public function getUserUuid() : ?string{
+        return $this->userUuid;
+    }
+
+    public function getUserType() : string {
+        return $this->userType;
+    }
+
+    public function setUserType(string $userType) {
+        if ($userType !== 'patient') {
+            $userType = 'user';
+        }
+        $this->userType = $userType;
+    }
 }