From 595d0789677bd12acb8cdf50da93f10fb55a27f2 Mon Sep 17 00:00:00 2001 From: Stephen Nielson Date: Sat, 17 Jun 2023 11:15:15 -0400 Subject: [PATCH] Openemr fix 6574 6575 6576 fhir updates (#6577) * Fixes #6575 refactor oauth2 client save Made it possible to save an oauth2 client using the client repository. This makes it so OpenEMR callers don't need to include the RestConfig class or deal with the AuthorizationController to generate a smart app client record if needed. Module writers can register their own smart app as needed using this functionality. * Fixes #6576 FHIR Capability statement createUpdate made it so the create update flag is set to false so API consumers know they can't provider their own logical ids when creating a resource. Fixes #6576 * Add systems for task/questionnaire IN preparation for future Task and Questionnaire endpoints we want to add the system constants here. * Add mapped service helper methods. Add helper methods for grabbing a token using a subset of codes as well as grabbing a service based on a specific code. * Fix url bug in utils service to prevent slashes * Add helper method to Token search for isUuid * Fixes #6574 QuestionnaireResponse search The questionnaire response search would fail when searching on things such as id due to duplicate column names. In order to make this service work in a FHIR context we need to fix up the search method and handle the uuid translations. Fixes #6574 --- .../Repositories/ClientRepository.php | 107 ++++++++++++++++++ src/Common/Utils/HttpUtils.php | 20 ++++ .../AuthorizationController.php | 84 ++------------ src/RestControllers/RestControllerHelper.php | 3 + src/Services/FHIR/FhirCodeSystemConstants.php | 8 ++ .../FHIR/Traits/MappedServiceCodeTrait.php | 25 ++++ .../FHIR/Traits/MappedServiceTrait.php | 15 +++ src/Services/FHIR/UtilsService.php | 6 +- src/Services/QuestionnaireResponseService.php | 71 +++++++++--- src/Services/Search/TokenSearchField.php | 5 + 10 files changed, 253 insertions(+), 91 deletions(-) create mode 100644 src/Common/Utils/HttpUtils.php diff --git a/src/Common/Auth/OpenIDConnect/Repositories/ClientRepository.php b/src/Common/Auth/OpenIDConnect/Repositories/ClientRepository.php index d004de76838..37f87d4b82d 100644 --- a/src/Common/Auth/OpenIDConnect/Repositories/ClientRepository.php +++ b/src/Common/Auth/OpenIDConnect/Repositories/ClientRepository.php @@ -16,6 +16,8 @@ use OpenEMR\Common\Auth\OpenIDConnect\Entities\ClientEntity; use OpenEMR\Common\Crypto\CryptoGen; use OpenEMR\Common\Logging\SystemLogger; +use OpenEMR\Common\Utils\HttpUtils; +use OpenEMR\Common\Utils\RandomGenUtils; use Psr\Log\LoggerInterface; class ClientRepository implements ClientRepositoryInterface @@ -25,9 +27,104 @@ class ClientRepository implements ClientRepositoryInterface */ private $logger; + private $cryptoGen; + public function __construct() { $this->logger = new SystemLogger(); + $this->cryptoGen = new CryptoGen(); + } + + /** + * @return CryptoGen + */ + public function getCryptoGen(): CryptoGen + { + return $this->cryptoGen; + } + + /** + * @param CryptoGen $cryptoGen + * @return ClientRepository + */ + public function setCryptoGen(CryptoGen $cryptoGen): ClientRepository + { + $this->cryptoGen = $cryptoGen; + return $this; + } + + 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; + + $contacts = $info['contacts']; + $redirects = $info['redirect_uris']; + if (is_array($redirects)) { + // need to combine our redirects if we are an array... this is due to the legacy implementation of this data + $redirects = implode("|", $redirects); + } + $logout_redirect_uris = $info['post_logout_redirect_uris'] ?? null; + $info['client_secret'] = $info['client_secret'] ?? null; // just to be sure empty is null; + // set our list of default scopes for the registration if our scope is empty + // This is how a client can set if they support SMART apps and other stuff by passing in the 'launch' + // scope to the dynamic client registration. + // per RFC 7591 @see https://tools.ietf.org/html/rfc7591#section-2 + // TODO: adunsulag do we need to reject the registration if there are certain scopes here we do not support + // TODO: adunsulag should we check these scopes against our '$this->supportedScopes'? + $info['scope'] = $info['scope'] ?? 'openid email phone address api:oemr api:fhir api:port'; + + $scopes = explode(" ", $info['scope']); + $scopeRepo = new ScopeRepository(); + + if ($scopeRepo->hasScopesThatRequireManualApproval($is_confidential_client == 1, $scopes)) { + $is_client_enabled = 0; // disabled + } else { + $is_client_enabled = 1; // enabled + } + + // encrypt the client secret + if (!empty($info['client_secret'])) { + $cryptoGen = $this->getCryptoGen(); + $info['client_secret'] = $cryptoGen->encryptStandard($info['client_secret']); + } + + // 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', ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"; + $i_vals = array( + $clientId, + $info['client_role'], + $info['client_name'], + $info['client_secret'], + $info['registration_access_token'], + $info['registration_client_uri_path'], + $contacts, + $redirects, + $info['scope'], + $user, + $site, + $is_confidential_client, + $logout_redirect_uris, + ($info['jwks_uri'] ?? null), + ($info['jwks'] ?? null), + ($info['initiate_login_uri'] ?? null), + ($info['endorsements'] ?? null), + ($info['policy_uri'] ?? null), + ($info['tos_uri'] ?? null), + $is_client_enabled + ); + + return sqlQueryNoLog($sql, $i_vals, true); // throw an exception if it fails + } + + public function generateClientId() + { + return HttpUtils::base64url_encode(RandomGenUtils::produceRandomBytes(32)); + } + + public function generateClientSecret() + { + return HttpUtils::base64url_encode(RandomGenUtils::produceRandomBytes(64)); } /** @@ -165,4 +262,14 @@ private function hydrateClientEntityFromArray($client_record): ClientEntity $client->setRegistrationDate($client_record['register_date']); return $client; } + + public function generateRegistrationAccessToken() + { + return HttpUtils::base64url_encode(RandomGenUtils::produceRandomBytes(32)); + } + + public function generateRegistrationClientUriPath() + { + return HttpUtils::base64url_encode(RandomGenUtils::produceRandomBytes(16)); + } } diff --git a/src/Common/Utils/HttpUtils.php b/src/Common/Utils/HttpUtils.php new file mode 100644 index 00000000000..04fbc18fe8c --- /dev/null +++ b/src/Common/Utils/HttpUtils.php @@ -0,0 +1,20 @@ + + * @copyright Copyright (c) 2023 Discover and Change, Inc. + * @license https://github.com/openemr/openemr/blob/master/LICENSE GNU General Public License 3 + */ + +namespace OpenEMR\Common\Utils; + +class HttpUtils +{ + public static function base64url_encode($data): string + { + return rtrim(strtr(base64_encode($data), '+/', '-_'), '='); + } +} diff --git a/src/RestControllers/AuthorizationController.php b/src/RestControllers/AuthorizationController.php index 0c0fc0a0dfe..199188dbeb6 100644 --- a/src/RestControllers/AuthorizationController.php +++ b/src/RestControllers/AuthorizationController.php @@ -59,6 +59,7 @@ use OpenEMR\Common\Http\Psr17Factory; use OpenEMR\Common\Logging\SystemLogger; use OpenEMR\Common\Session\SessionUtil; +use OpenEMR\Common\Utils\HttpUtils; use OpenEMR\Common\Utils\RandomGenUtils; use OpenEMR\Common\Uuid\UuidRegistry; use OpenEMR\FHIR\Config\ServerConfig; @@ -214,9 +215,10 @@ public function clientRegistration(): void // @see https://tools.ietf.org/html/rfc7591#section-2 'scope' => null ); - $client_id = $this->base64url_encode(RandomGenUtils::produceRandomBytes(32)); - $reg_token = $this->base64url_encode(RandomGenUtils::produceRandomBytes(32)); - $reg_client_uri_path = $this->base64url_encode(RandomGenUtils::produceRandomBytes(16)); + $clientRepository = new ClientRepository(); + $client_id = $clientRepository->generateClientId(); + $reg_token = $clientRepository->generateRegistrationAccessToken(); + $reg_client_uri_path = $clientRepository->generateRegistrationClientUriPath(); $params = array( 'client_id' => $client_id, 'client_id_issued_at' => time(), @@ -228,7 +230,7 @@ public function clientRegistration(): void // only include secret if a confidential app else force PKCE for native and web apps. $client_secret = ''; if ($data['application_type'] === 'private') { - $client_secret = $this->base64url_encode(RandomGenUtils::produceRandomBytes(64)); + $client_secret = $clientRepository->generateClientSecret(); $params['client_secret'] = $client_secret; $params['client_role'] = 'user'; @@ -271,9 +273,10 @@ public function clientRegistration(): void throw new OAuthServerException('post_logout_redirect_uris is invalid', 0, 'invalid_client_metadata'); } // save to oauth client table - $badSave = $this->newClientSave($client_id, $params); - if (!empty($badSave)) { - throw OAuthServerException::serverError("Try again. Unable to create account"); + try { + $clientRepository->insertNewClient($client_id, $params, $this->siteId); + } catch (\Exception $exception) { + throw OAuthServerException::serverError("Try again. Unable to create account", $exception); } $reg_uri = $this->authBaseFullUrl . '/client/' . $reg_client_uri_path; unset($params['registration_client_uri_path']); @@ -352,7 +355,7 @@ private function createServerRequest(): ServerRequestInterface public function base64url_encode($data): string { - return rtrim(strtr(base64_encode($data), '+/', '-_'), '='); + return HttpUtils::base64url_encode($data); } public function base64url_decode($token) @@ -361,71 +364,6 @@ public function base64url_decode($token) return base64_decode($b64); } - public function newClientSave($clientId, $info): bool - { - $user = $_SESSION['authUserID'] ?? null; // future use for provider client. - $site = $this->siteId; - $is_confidential_client = empty($info['client_secret']) ? 0 : 1; - - $contacts = $info['contacts']; - $redirects = $info['redirect_uris']; - $logout_redirect_uris = $info['post_logout_redirect_uris'] ?? null; - $info['client_secret'] = $info['client_secret'] ?? null; // just to be sure empty is null; - // set our list of default scopes for the registration if our scope is empty - // This is how a client can set if they support SMART apps and other stuff by passing in the 'launch' - // scope to the dynamic client registration. - // per RFC 7591 @see https://tools.ietf.org/html/rfc7591#section-2 - // TODO: adunsulag do we need to reject the registration if there are certain scopes here we do not support - // TODO: adunsulag should we check these scopes against our '$this->supportedScopes'? - $info['scope'] = $info['scope'] ?? 'openid email phone address api:oemr api:fhir api:port'; - - $scopes = explode(" ", $info['scope']); - $scopeRepo = new ScopeRepository(); - - if ($scopeRepo->hasScopesThatRequireManualApproval($is_confidential_client == 1, $scopes)) { - $is_client_enabled = 0; // disabled - } else { - $is_client_enabled = 1; // enabled - } - - // encrypt the client secret - if (!empty($info['client_secret'])) { - $info['client_secret'] = $this->cryptoGen->encryptStandard($info['client_secret']); - } - - - try { - // 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', ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"; - $i_vals = array( - $clientId, - $info['client_role'], - $info['client_name'], - $info['client_secret'], - $info['registration_access_token'], - $info['registration_client_uri_path'], - $contacts, - $redirects, - $info['scope'], - $user, - $site, - $is_confidential_client, - $logout_redirect_uris, - ($info['jwks_uri'] ?? null), - ($info['jwks'] ?? null), - ($info['initiate_login_uri'] ?? null), - ($info['endorsements'] ?? null), - ($info['policy_uri'] ?? null), - ($info['tos_uri'] ?? null), - $is_client_enabled - ); - - return sqlQueryNoLog($sql, $i_vals); - } catch (\RuntimeException $e) { - die($e); - } - } - public function emitResponse($response): void { if (headers_sent()) { diff --git a/src/RestControllers/RestControllerHelper.php b/src/RestControllers/RestControllerHelper.php index 43d364b0d37..2dee0978d4f 100644 --- a/src/RestControllers/RestControllerHelper.php +++ b/src/RestControllers/RestControllerHelper.php @@ -367,6 +367,9 @@ public function getCapabilityRESTObject($routes, $serviceClassNameSpace = self:: if (empty($capResource)) { $capResource = new FHIRCapabilityStatementResource(); + // make it explicit that we do not let the user use their own resource ids to create a new resource + // in the PUT/update operation. + $capResource->setUpdateCreate(false); $capResource->setType(new FHIRCode($type)); $capResource->setProfile(new FHIRCanonical($structureDefinition . $type)); diff --git a/src/Services/FHIR/FhirCodeSystemConstants.php b/src/Services/FHIR/FhirCodeSystemConstants.php index 0d2016a2023..28dc4f6cc7b 100644 --- a/src/Services/FHIR/FhirCodeSystemConstants.php +++ b/src/Services/FHIR/FhirCodeSystemConstants.php @@ -80,4 +80,12 @@ class FhirCodeSystemConstants * available here: https://vsac.nlm.nih.gov/valueset/2.16.840.1.113762.1.4.1099.27/expansion/Latest */ const CARE_TEAM_MEMBER_FUNCTION_SNOMEDCT = "2.16.840.1.113762.1.4.1099.27"; + + /** + * Required for Structured Data Collection (SDC) Task implementations + * @see https://build.fhir.org/ig/HL7/sdc/ValueSet-task-code.html + */ + const HL7_SDC_TASK_TEMP = "https://build.fhir.org/ig/HL7/sdc/CodeSystem-temp.html"; + + const HL7_SDC_TASK_SERVICE_REQUEST = "http://hl7.org/fhir/CodeSystem/task-code"; } diff --git a/src/Services/FHIR/Traits/MappedServiceCodeTrait.php b/src/Services/FHIR/Traits/MappedServiceCodeTrait.php index dcb2da88afb..a189127bb1a 100644 --- a/src/Services/FHIR/Traits/MappedServiceCodeTrait.php +++ b/src/Services/FHIR/Traits/MappedServiceCodeTrait.php @@ -20,6 +20,19 @@ trait MappedServiceCodeTrait { use MappedServiceTrait; + public function getServiceListForCode(TokenSearchField $field) + { + // TODO: @adunsulag if we want to aggregate multiple code parameters we will need to handle selecting a subset of codes + // per service + $serviceList = []; + foreach ($this->getMappedServices() as $service) { + $subsetCodes = $this->getTokenSearchFieldWithSupportedCodes($service, $field); + if (!empty($subsetCodes->getValues())) { + $serviceList[] = $service; + } + } + return $serviceList; + } public function getServiceForCode(TokenSearchField $field, $defaultCode) { // shouldn't ever hit the default but we have it there just in case. @@ -35,6 +48,18 @@ public function getServiceForCode(TokenSearchField $field, $defaultCode) throw new SearchFieldException($field->getField(), "Invalid or unsupported code"); } + public function getTokenSearchFieldWithSupportedCodes(FhirServiceBase $service, TokenSearchField $field) + { + $subsetCodes = []; + foreach ($field->getValues() as $value) { + $searchCode = $value->getCode(); + if ($service->supportsCode($searchCode)) { + $subsetCodes[] = $value; + } + } + return new TokenSearchField($field->getField(), $subsetCodes, $field->isUuid()); + } + public function getServiceForCategory(TokenSearchField $category, $defaultCategory): FhirServiceBase { // let the field parse our category diff --git a/src/Services/FHIR/Traits/MappedServiceTrait.php b/src/Services/FHIR/Traits/MappedServiceTrait.php index a767cdcb6ed..baa74c1e3ec 100644 --- a/src/Services/FHIR/Traits/MappedServiceTrait.php +++ b/src/Services/FHIR/Traits/MappedServiceTrait.php @@ -64,6 +64,21 @@ public function searchAllServices($fhirSearchParams, $puuidBind) return $processingResult; } + public function searchServices(array $services, $fhirSearchParams, $puuidBind) + { + $processingResult = new ProcessingResult(); + foreach ($services as $service) { + $innerResult = $service->getAll($fhirSearchParams, $puuidBind); + $processingResult->addProcessingResult($innerResult); + if ($processingResult->hasErrors()) { + // clear our data out and just return the errors + $processingResult->clearData(); + return $processingResult; + } + } + return $processingResult; + } + public function searchAllServicesWithSupportedFields($fhirSearchParams, $puuidBind) { $processingResult = new ProcessingResult(); diff --git a/src/Services/FHIR/UtilsService.php b/src/Services/FHIR/UtilsService.php index c2319b97c52..1b7cbf01486 100644 --- a/src/Services/FHIR/UtilsService.php +++ b/src/Services/FHIR/UtilsService.php @@ -56,7 +56,11 @@ public static function createCanonicalUrlForResource($resourceType, $uuid): FHIR { $siteConfig = new ServerConfig(); - $url = $siteConfig->getFhirUrl() . $resourceType . '/' . $uuid; + $fhirUrl = $siteConfig->getFhirUrl() ?? ""; + if (strrpos("/", $fhirUrl) != strlen($fhirUrl)) { + $fhirUrl .= "/"; + } + $url = $fhirUrl . $resourceType . '/' . $uuid; $cannonical = new FHIRCanonical(); $cannonical->setValue($url); return $cannonical; diff --git a/src/Services/QuestionnaireResponseService.php b/src/Services/QuestionnaireResponseService.php index a7a762d1589..5d229ba2cd0 100644 --- a/src/Services/QuestionnaireResponseService.php +++ b/src/Services/QuestionnaireResponseService.php @@ -248,36 +248,73 @@ public function parseQuestionnaireResponseForms($response): array return $question_results; } + public function getUuidFields(): array + { + return ['questionnaire_response_uuid', 'encounter_uuid', 'puuid']; + } + public function search($search, $isAndCondition = true) { - $sqlSelectIds = "SELECT DISTINCT questionnaire_response.uuid "; - $sqlSelectData = " SELECT questionnaire_response.* - ,fe.uuid AS encounter_uuid - "; - $sql = "FROM questionnnaire_response qr " - . " LEFT JOIN form_questionnaire_assessments ON fqa.response_id = qr.response_id " // TODO: @adunsulag is this field indexed? + $sqlSelectIds = "SELECT DISTINCT qr.questionnaire_response_uuid "; + $sqlSelectData = " SELECT qr.* + ,fe.encounter_uuid + ,pd.puuid "; + $sql = "FROM ( + SELECT + uuid AS questionnaire_response_uuid + ,id AS id + ,response_id + ,questionnaire_foreign_id + ,questionnaire_id + ,questionnaire_name + ,patient_id + ,encounter + ,audit_user_id + ,creator_user_id + ,create_time + ,last_updated + ,version + ,status + ,questionnaire + ,questionnaire_response + ,form_response + ,form_score + ,tscore + ,error + FROM questionnaire_response + ) qr " + . " LEFT JOIN form_questionnaire_assessments fqa ON fqa.response_id = qr.response_id " // TODO: @adunsulag is this field indexed? . " LEFT JOIN forms f ON fqa.id = f.form_id AND f.formdir='questionnaire_assessments' " - . " LEFT JOIN form_encounter fe ON f.encounter = fe.encounter " - . " LEFT JOIN patient_data pd ON qr.patient_id = patient_data.pid " + . " LEFT JOIN ( + SELECT + uuid AS encounter_uuid + ,encounter + FROM form_encounter + ) fe ON f.encounter = fe.encounter " + . " LEFT JOIN ( + SELECT + uuid AS puuid + ,pid + FROM patient_data + ) pd ON qr.patient_id = pd.pid " // we only grab users that are actual Practitioners with a valid NPI number - . " LEFT JOIN users ON qr.creator_user_id = users.id AND user.username IS NOT NULL and user.npi IS NOT NULL AND user.npi != ''" ; - - $sqlIds = $sqlSelectIds . $sql; - QueryUtils::fetchTableColumn($sqlIds, 'uuid'); + . " LEFT JOIN users ON qr.creator_user_id = users.id AND users.username IS NOT NULL and users.npi IS NOT NULL AND users.npi != ''" ; $whereUuidClause = FhirSearchWhereClauseBuilder::build($search, $isAndCondition); - - $sqlUuids = $sqlSelectIds . $sql . $whereUuidClause->getFragment(); - $uuidResults = QueryUtils::fetchTableColumn($sqlUuids, 'uuid', $whereUuidClause->getBoundValues()); + $sqlUuids = $sqlSelectIds . " " . $sql . " " . $whereUuidClause->getFragment(); + $uuidResults = QueryUtils::fetchTableColumn($sqlUuids, 'questionnaire_response_uuid', $whereUuidClause->getBoundValues()); if (!empty($uuidResults)) { // now we are going to run through this again and grab all of our data w only the uuid search as our filter // this makes sure we grab the entire patient record and associated data - $whereClause = " WHERE patient_data.uuid IN (" . implode(",", array_map(function ($uuid) { + $whereClause = " WHERE qr.questionnaire_response_uuid IN (" . implode(",", array_map(function ($uuid) { return "?"; }, $uuidResults)) . ")"; $statementResults = QueryUtils::sqlStatementThrowException($sqlSelectData . $sql . $whereClause, $uuidResults); - $processingResult = $this->hydrateSearchResultsFromQueryResource($statementResults); + $processingResult = new ProcessingResult(); + foreach ($statementResults as $record) { + $processingResult->addData($this->createResultRecordFromDatabaseResult($record)); + } return $processingResult; } else { return new ProcessingResult(); diff --git a/src/Services/Search/TokenSearchField.php b/src/Services/Search/TokenSearchField.php index 1ff378d0e81..70c2d97c4ab 100644 --- a/src/Services/Search/TokenSearchField.php +++ b/src/Services/Search/TokenSearchField.php @@ -26,6 +26,11 @@ public function __construct($field, $values, $isUUID = false) parent::__construct($field, SearchFieldType::TOKEN, $field, $values); } + public function isUuid() + { + return $this->isUUID; + } + public function addValue(TokenSearchValue $value) { $values = $this->getValues();