Standalone SMART response handler

Fixed some scope permission checks. Refactored the route parsing algorithm into its own class that can be unit tested against. The parsing logic could then be leveraged in the scope auth check which made matching against the REST FHIR resource a lot easier. Added the additional SMART capabilities we now support with patient standalone and launch standalone. Fixed the refresh token issues. We don't send patient context parameters as part of the refresh_grant oauth2 flow so we only send the parameters now in the authorization_grant flow inside our SMARTResponse object. There may be a better way to make this work, but for now this is functioning.
openemr · bradymiller · Jan 28, 2021 · Jan 11, 2021 · Jan 13, 2021 · Jan 14, 2021
commit 43ff24aecbd68e84115ffecf3aee638dfc652012
diff --git a/apis/dispatch.php b/apis/dispatch.php
@@ -116,7 +116,7 @@
 }
 
 // set the route as well as the resource information.  Note $resource is actually the route and not the resource name.
-$restRequest->setRoute($resource);
+$restRequest->setRequestPath($resource);
 
 if (!$isLocalApi) {
     // Will start the api OpenEMR session/cookie.

diff --git a/src/Common/Auth/OpenIDConnect/IdTokenSMARTResponse.php b/src/Common/Auth/OpenIDConnect/IdTokenSMARTResponse.php
@@ -33,14 +33,24 @@ class IdTokenSMARTResponse extends IdTokenResponse
      */
     private $logger;
 
+    /**
+     * @var boolean
+     */
+    private $isAuthorizationGrant;
+
     public function __construct(
         IdentityProviderInterface $identityProvider,
         ClaimExtractor $claimExtractor
     ) {
+        $this->isAuthorizationGrant = false;
         $this->logger = new SystemLogger();
         parent::__construct($identityProvider, $claimExtractor);
     }
 
+    public function markIsAuthorizationGrant() {
+        $this->isAuthorizationGrant = true;
+    }
+
     protected function getExtraParams(AccessTokenEntityInterface $accessToken)
     {
         $extraParams = parent::getExtraParams($accessToken);
@@ -114,6 +124,11 @@ private function getSmartStyleURL()
      */
     private function isLaunchRequest($scopes)
     {
+        // if we are not in an authorization grant context we don't support SMART launch context params
+        if (!$this->isAuthorizationGrant) {
+            return false;
+        }
+
         return $this->hasScope($scopes, 'launch');
     }
 
@@ -123,6 +138,10 @@ private function isLaunchRequest($scopes)
      */
     private function isStandaloneLaunchPatientRequest($scopes)
     {
+        // if we are not in an authorization grant context we don't support SMART launch context params
+        if (!$this->isAuthorizationGrant) {
+            return false;
+        }
         return $this->hasScope($scopes, 'launch/patient');
     }
 

diff --git a/src/Common/Http/HttpRestParsedRoute.php b/src/Common/Http/HttpRestParsedRoute.php
@@ -0,0 +1,138 @@
+<?php
+/**
+ * HttpRestParsedRoute represents a parsed http rest api request.  It splits apart an OpenEMR route definition and
+ * parses the provided http request against that route definition.  Validates the route definition and extracts the
+ * resource name as well as any route parameters defined in the route definition.
+ * @package openemr
+ * @link      http://www.open-emr.org
+ * @author    Stephen Nielson <stephen@nielson.org>
+ * @copyright Copyright (c) 2021 Stephen Nielson <stephen@nielson.org>
+ * @license   https://github.com/openemr/openemr/blob/master/LICENSE GNU General Public License 3
+ */
+
+namespace OpenEMR\Common\Http;
+
+
+use OpenEMR\Common\Logging\SystemLogger;
+
+class HttpRestParsedRoute
+{
+
+    /**
+     * Whether the route definition is a valid match against the current request
+     * @var bool
+     */
+    private $isValid;
+
+    /**
+     * The endpoint resource that the api request is for.  Only populated if the route definition
+     * matches against the current route
+     * @var string
+     */
+    private $resource;
+
+    /**
+     * The endpoint paramters (identifiers, and anything else marked with the :colon param).
+     * Only populated if the route definition matches against the current route
+     * @var string
+     */
+    private $routeParams;
+
+    /**
+     * The OpenEMR route definition that this request is being matched / parsed against
+     * @var string
+     */
+    private $routeDefinition;
+
+    /**
+     * The current HTTP request route we are attempting to match against a route definition
+     * @var string
+     */
+    private $requestRoute;
+
+    public function __construct($requestMethod, $requestRoute, $routeDefinition)
+    {
+        $this->routeDefinition = $routeDefinition;
+        $this->requestRoute = $requestRoute;
+        $this->requestMethod = $requestMethod;
+
+        $routePieces = explode(" ", $routeDefinition);
+        $routeDefinitionMethod = $routePieces[0];
+        $pattern = $this->getRouteMatchExpression($routePieces[1]);
+        $matches = array();
+        if ($requestMethod === $routeDefinitionMethod && preg_match($pattern, $requestRoute, $matches)) {
+            $this->isValid = true;
+            array_shift($matches); // drop request method
+            $this->routeParams = $matches;
+            $this->resource = $this->getResourceForRoute($routeDefinition);
+            (new SystemLogger())->debug("HttpRestParsedRoute->__construct() ", ['routePath' => $routeDefinition,
+                'requestPath' => $requestRoute
+                ,'method' => $requestMethod, 'routeParams' => $this->routeParams, 'resource' => $this->getResource()]);
+        }
+        else {
+            $this->isValid = false;
+        }
+    }
+
+    /**
+     * Returns true if the
+     *
+     * @return boolean
+     */
+    public function isValid() {
+        return $this->isValid;
+    }
+
+    /**
+     * @return string
+     */
+    public function getResource(): string
+    {
+        return $this->resource;
+    }
+
+    /**
+     * @return array
+     */
+    public function getRouteParams(): array
+    {
+        return $this->routeParams;
+    }
+
+    /**
+     * @return string
+     */
+    public function getRouteDefinition()
+    {
+        return $this->routeDefinition;
+    }
+
+    /**
+     * @return string
+     */
+    public function getRequestRoute()
+    {
+        return $this->requestRoute;
+    }
+
+    /**
+     * Returns the regex for a given path we use to match against a route.
+     * @param $path
+     * @return string
+     */
+    private function getRouteMatchExpression($path) {
+        // Taken from https://stackoverflow.com/questions/11722711/url-routing-regex-php/11723153#11723153
+        return "@^" . preg_replace('/\\\:[a-zA-Z0-9\_\-]+/', '([a-zA-Z0-9\-\_\$]+)', preg_quote($path)) . "$@D";
+    }
+
+
+    private function getResourceForRoute($routePath) {
+        $parts = explode("/", $routePath);
+        $finalArg = end($parts);
+        if (strpos($finalArg, ':') !== false) {
+            array_pop($parts);
+            $finalArg = end($parts);
+        }
+        return $finalArg;
+    }
+}
diff --git a/src/Common/Http/HttpRestRequest.php b/src/Common/Http/HttpRestRequest.php
@@ -1,6 +1,6 @@
 <?php
 /**
- * HttpRestRequest.php
+ * HttpRestRequest represents the current OpenEMR api request
  * @package openemr
  * @link      http://www.open-emr.org
  * @author    Stephen Nielson <stephen@nielson.org>
@@ -88,7 +88,7 @@ class HttpRestRequest
     /**
      * @var string
      */
-    private $route;
+    private $requestPath;
 
     public function __construct($restConfig, $server) {
         $this->restConfig = $restConfig;
@@ -308,23 +308,11 @@ public function isPatientWriteRequest() {
         return $this->isFhir() && $this->isPatientRequest() && $this->getRequestMethod != 'GET';
     }
 
-    public function setRoute(string $route) {
-        $this->route = $route;
-        $this->resource = $this->getResourceFromRoute($route);
+    public function setRequestPath(string $requestPath) {
+        $this->requestPath = $requestPath;
     }
 
-    public function getRoute() : ?string {
-        return $this->route;
-    }
-
-    protected function getResourceFromRoute($route) {
-        $parts = explode("/", $route);
-        $finalArg = end($parts);
-        if (strpos($finalArg, ':') !== false) {
-            array_pop($parts);
-            $finalArg = end($parts);
-        }
-        // set our resource value here
-        $this->setResource($finalArg);
+    public function getRequestPath() : ?string {
+        return $this->requestPath;
     }
 }
diff --git a/src/Common/Http/HttpRestRouteHandler.php b/src/Common/Http/HttpRestRouteHandler.php
@@ -27,9 +27,11 @@ public static function dispatch(&$routes, HttpRestRequest $restRequest, $return_
         (new SystemLogger())->debug("HttpRestRouteHandler::dispatch() start request",
             ['resource' => $restRequest->getResource(), 'method' => $restRequest->getRequestMethod()
                 , 'user' => $restRequest->getRequestUserUUID(), 'role' => $restRequest->getRequestUserRole()
-                , 'client' => $restRequest->getClientId(), 'apiType' => $restRequest->getApiType()]);
+                , 'client' => $restRequest->getClientId(), 'apiType' => $restRequest->getApiType()
+                , 'route' => $restRequest->getRequestPath()
+            ]);
 
-        $route = $dispatchRestRequest->getRoute();
+        $route = $dispatchRestRequest->getRequestPath();
         $request_method = $dispatchRestRequest->getRequestMethod();
 
         // this is already handled somewhere else.
@@ -43,24 +45,24 @@ public static function dispatch(&$routes, HttpRestRequest $restRequest, $return_
         }
 
         try {
-            // make sure our scopes pass the security checks
-            self::checkSecurity($restRequest);
-
             // Taken from https://stackoverflow.com/questions/11722711/url-routing-regex-php/11723153#11723153
             $hasRoute = false;
             foreach ($routes as $routePath => $routeCallback) {
-                $routePieces = explode(" ", $routePath);
-                $method = $routePieces[0];
-                $path = $routePieces[1];
-                $pattern = "@^" . preg_replace('/\\\:[a-zA-Z0-9\_\-]+/', '([a-zA-Z0-9\-\_\$]+)', preg_quote($path)) . "$@D";
-                $matches = array();
-                if ($method === $request_method && preg_match($pattern, $route, $matches)) {
-                    array_shift($matches);
-                    $matches[] = $dispatchRestRequest;
+                $parsedRoute = new HttpRestParsedRoute($dispatchRestRequest->getRequestMethod(), $dispatchRestRequest->getRequestPath(), $routePath);
+                if ($parsedRoute->isValid()) {
+                    $dispatchRestRequest->setResource($parsedRoute->getResource());
+
+                    // make sure our scopes pass the security checks
+                    self::checkSecurity($dispatchRestRequest);
                     (new SystemLogger())->debug("HttpRestRouteHandler->dispatch() dispatching route", ["route" => $routePath,]);
                     $hasRoute = true;
+
+                    // now grab our url parameters and issue the controller callback for the route
+                    $routeControllerParameters = $parsedRoute->getRouteParams();
+                    $routeControllerParameters[] = $dispatchRestRequest; // add in the request object to everything
                     // call the function and use array unpacking to make this faster
-                    $result = $routeCallback(...$matches);
+                    $result = $routeCallback(...$routeControllerParameters);
+
                     if ($return_method === 'standard') {
                         header('Content-Type: application/json');
                         echo json_encode($result);
@@ -95,7 +97,7 @@ public static function dispatch(&$routes, HttpRestRequest $restRequest, $return_
      */
     private static function checkSecurity(HttpRestRequest $restRequest) {
         $scopeType = $restRequest->isPatientRequest() ? "patient" : "user";
-        $permission = $restRequest->getRequestMethod() == "GET" ? "write" : "read";
+        $permission = $restRequest->getRequestMethod() === "GET" ? "read" : "write";
         $resource = $restRequest->getResource();
 
         if ($restRequest->isFhir()) {

diff --git a/src/FHIR/SMART/Capability.php b/src/FHIR/SMART/Capability.php
@@ -25,7 +25,8 @@ class Capability
      * @see ONC final rule commentary https://www.federalregister.gov/d/2020-07419/p-1184 Accessed on December 9th 2020
      */
     const SUPPORTED_CAPABILITIES = [self::LAUNCH_EHR, self::CONTEXT_BANNER, self::CONTEXT_EHR_PATIENT
-        , self::CONTEXT_STYLE, self::SSO_OPENID_CONNECTION, self::CLIENT_CONFIDENTIAL_SYMMETRIC, self::PERMISSION_USER ];
+        , self::CONTEXT_STYLE, self::SSO_OPENID_CONNECTION, self::CLIENT_CONFIDENTIAL_SYMMETRIC, self::PERMISSION_USER
+        , self::CONTEXT_STANDALONE_PATIENT, self::LAUNCH_STANDALONE, self::PERMISSION_PATIENT];
 
     // support for SMART’s EHR Launch mode
     const LAUNCH_EHR = 'launch-ehr';

diff --git a/src/RestControllers/AuthorizationController.php b/src/RestControllers/AuthorizationController.php
@@ -595,6 +595,8 @@ public function getAuthorizationServer(): AuthorizationServer
         }
         $this->logger->debug("AuthorizationController->getAuthorizationServer() grantType is " . $this->grantType);
         if ($this->grantType === 'authorization_code') {
+            $responseType->markIsAuthorizationGrant(); // we have specific SMART responses for an authorization grant.
+
             $grant = new AuthCodeGrant(
                 new AuthCodeRepository(),
                 new RefreshTokenRepository(),
@@ -756,6 +758,8 @@ public function userLogin(): void
         } else {
             $redirect = $this->authBaseFullUrl . self::ENDPOINT_SCOPE_AUTHORIZE_CONFIRM;
         }
+        $this->logger->debug("AuthorizationController->userLogin() complete redirecting", ["scopes" => $_SESSION['scopes']
+            , 'claims' => $_SESSION['claims'], 'redirect' => $redirect]);
 
         header("Location: $redirect");
         exit;