From 87a70d56b636172e3bdf2928960ec8534957b45c Mon Sep 17 00:00:00 2001 From: Taslan Graham Date: Tue, 20 Aug 2024 16:56:35 -0500 Subject: [PATCH] pkp/pkp-lib#10318 Revoke ORCID tokens when deleting authenticated ORCIDs --- classes/author/Repository.php | 3 +- classes/orcid/OrcidManager.php | 99 +++++++++++----------- classes/user/form/IdentityForm.php | 16 +++- controllers/tab/user/ProfileTabHandler.php | 10 ++- jobs/orcid/RevokeOrcidToken.php | 68 +++++++++++++++ templates/form/orcidProfile.tpl | 12 ++- templates/user/identityForm.tpl | 17 +++- 7 files changed, 165 insertions(+), 60 deletions(-) create mode 100644 jobs/orcid/RevokeOrcidToken.php diff --git a/classes/author/Repository.php b/classes/author/Repository.php index 1d206aac06d..8673d415616 100644 --- a/classes/author/Repository.php +++ b/classes/author/Repository.php @@ -148,6 +148,7 @@ public function validate($author, $props, Submission $submission, Context $conte * @copydoc \PKP\services\entityProperties\EntityWriteInterface::add() * * @hook Author::add [[$author]] + * @hook Author::add::before [[$author]] */ public function add(Author $author): int { @@ -173,7 +174,7 @@ public function add(Author $author): int * * @hook Author::edit [[$newAuthor, $author, $params]] */ - public function edit(Author $author, array $params) + public function edit(Author $author, array $params = []) { $newAuthor = Repo::author()->newDataObject(array_merge($author->_data, $params)); diff --git a/classes/orcid/OrcidManager.php b/classes/orcid/OrcidManager.php index 6f61edd00cf..672e92e58d3 100644 --- a/classes/orcid/OrcidManager.php +++ b/classes/orcid/OrcidManager.php @@ -20,6 +20,8 @@ use PKP\config\Config; use PKP\context\Context; use PKP\core\Core; +use pkp\jobs\orcid\RevokeOrcidToken; +use PKP\user\User; class OrcidManager { @@ -99,14 +101,7 @@ public static function isEnabled(?Context $context = null): bool */ public static function getOrcidUrl(?Context $context = null): string { - if (self::isGloballyConfigured()) { - $apiType = Application::get()->getRequest()->getSite()->getData(self::API_TYPE); - } else { - if ($context === null) { - $context = Application::get()->getRequest()->getContext(); - } - $apiType = $context->getData(self::API_TYPE); - } + $apiType = self::getApiType(); return in_array($apiType, [self::API_PUBLIC_PRODUCTION, self::API_MEMBER_PRODUCTION]) ? self::ORCID_URL : self::ORCID_URL_SANDBOX; } @@ -117,14 +112,7 @@ public static function getOrcidUrl(?Context $context = null): string */ public static function getApiPath(?Context $context = null): string { - if (self::isGloballyConfigured()) { - $apiType = Application::get()->getRequest()->getSite()->getData(self::API_TYPE); - } else { - if ($context === null) { - $context = Application::get()->getRequest()->getContext(); - } - $apiType = $context->getData(self::API_TYPE); - } + $apiType = self::getApiType(); return match ($apiType) { self::API_PUBLIC_SANDBOX => self::ORCID_API_URL_PUBLIC_SANDBOX, @@ -141,15 +129,7 @@ public static function getApiPath(?Context $context = null): string */ public static function isSandbox(?Context $context = null): bool { - if (self::isGloballyConfigured()) { - $apiType = Application::get()->getRequest()->getSite()->getData(self::API_TYPE); - } else { - if ($context === null) { - $context = Application::get()->getRequest()->getContext(); - } - $apiType = $context->getData(self::API_TYPE); - } - + $apiType = self::getApiType(); return in_array($apiType, [self::API_PUBLIC_SANDBOX, self::API_MEMBER_SANDBOX]); } @@ -228,20 +208,8 @@ public static function getCountry(?Context $context = null): string */ public static function isMemberApiEnabled(?Context $context = null): bool { - if (self::isGloballyConfigured()) { - $apiType = Application::get()->getRequest()->getSite()->getData(self::API_TYPE); - } else { - if ($context === null) { - $context = Application::get()->getRequest()->getContext(); - } - $apiType = $context->getData(self::API_TYPE); - } - - if (in_array($apiType, [self::API_MEMBER_PRODUCTION, self::API_MEMBER_SANDBOX])) { - return true; - } else { - return false; - } + $apiType = self::getApiType(); + return in_array($apiType, [self::API_MEMBER_PRODUCTION, self::API_MEMBER_SANDBOX]); } /** @@ -315,20 +283,26 @@ public static function getClientSecret(?Context $context = null): string /** * Remove all data fields, which belong to an ORCID access token from the - * given Author object. Also updates fields in the db. + * given Author or User object. Also updates fields in the db. * - * @param bool $updateAuthor If true, update the author fields in the database. - * Use only if not called from a function, which will already update the author. + * @param bool $updateDb If true, update the underlying fields in the database. + * Use only if not called from a function, which will already update the object. */ - public static function removeOrcidAccessToken(Author $author, bool $updateAuthor = false): void + public static function removeOrcidAccessToken(Author|User $identity, bool $updateDb = false): void { - $author->setData('orcidAccessToken', null); - $author->setData('orcidAccessScope', null); - $author->setData('orcidRefreshToken', null); - $author->setData('orcidAccessExpiresOn', null); - - if ($updateAuthor) { - Repo::author()->dao->update($author); + dispatch(new RevokeOrcidToken(Application::get()->getRequest()->getContext(), $identity)); + + $identity->setData('orcidAccessToken', null); + $identity->setData('orcidAccessScope', null); + $identity->setData('orcidRefreshToken', null); + $identity->setData('orcidAccessExpiresOn', null); + + if ($updateDb) { + if ($identity instanceof User) { + Repo::user()->edit($identity); + } else { + Repo::author()->edit($identity); + } } } @@ -363,4 +337,29 @@ private static function writeLog(string $message, string $level): void $logFilePath = Config::getVar('files', 'files_dir') . '/orcid.log'; error_log("{$fineStamp} {$level} {$message}\n", 3, $logFilePath); } + + /** + * Gets the ORCID API endpoint to revoke an access token + */ + public static function getTokenRevocationUrl(): string + { + return self::getOauthPath() . 'revoke'; + } + + /** + * Helper method get the ORCID Api Type. + */ + private static function getApiType(?Context $context = null): string + { + if (self::isGloballyConfigured()) { + $apiType = Application::get()->getRequest()->getSite()->getData(self::API_TYPE); + } else { + if ($context === null) { + $context = Application::get()->getRequest()->getContext(); + } + $apiType = $context->getData(self::API_TYPE); + } + + return $apiType; + } } diff --git a/classes/user/form/IdentityForm.php b/classes/user/form/IdentityForm.php index d4503e2218f..1cdfe9ef948 100644 --- a/classes/user/form/IdentityForm.php +++ b/classes/user/form/IdentityForm.php @@ -109,7 +109,7 @@ public function readInputData() parent::readInputData(); $this->readUserVars([ - 'givenName', 'familyName', 'preferredPublicName', 'orcid', + 'givenName', 'familyName', 'preferredPublicName', 'orcid','removeOrcidId' ]); } @@ -121,9 +121,17 @@ public function execute(...$functionArgs) $request = Application::get()->getRequest(); $user = $request->getUser(); - $user->setGivenName($this->getData('givenName'), null); - $user->setFamilyName($this->getData('familyName'), null); - $user->setPreferredPublicName($this->getData('preferredPublicName'), null); + + // Request to delete ORCID token is handle separately from other form field updates + if($this->getData('removeOrcidId') === 'true') { + $user->setOrcid(null); + $user->setOrcidVerified(false); + OrcidManager::removeOrcidAccessToken($user); + } else { + $user->setGivenName($this->getData('givenName'), null); + $user->setFamilyName($this->getData('familyName'), null); + $user->setPreferredPublicName($this->getData('preferredPublicName'), null); + } parent::execute(...$functionArgs); } diff --git a/controllers/tab/user/ProfileTabHandler.php b/controllers/tab/user/ProfileTabHandler.php index df61bcec3fd..2e00fd4f50b 100644 --- a/controllers/tab/user/ProfileTabHandler.php +++ b/controllers/tab/user/ProfileTabHandler.php @@ -84,7 +84,11 @@ public function saveIdentity($args, $request) $identityForm->execute(); $notificationMgr = new NotificationManager(); $notificationMgr->createTrivialNotification($request->getUser()->getId()); - return new JSONMessage(true); + + $json = new JSONMessage(true); + $identityForm->initData(); + $json->setEvent('refreshForm', $identityForm->fetch($request)); + return $json; } return new JSONMessage(true, $identityForm->fetch($request)); } @@ -122,14 +126,14 @@ public function saveContact($args, $request) if ($contactForm->getData('action') == ContactForm::ACTION_CANCEL_EMAIL_CHANGE) { $contactForm->cancelPendingEmail(); - + $notificationMgr = new NotificationManager(); $notificationMgr->createTrivialNotification($request->getUser()->getId()); $contactForm->initData(); return new JSONMessage(true, $contactForm->fetch($request)); } - + if ($contactForm->validate()) { $contactForm->execute(); $notificationMgr = new NotificationManager(); diff --git a/jobs/orcid/RevokeOrcidToken.php b/jobs/orcid/RevokeOrcidToken.php new file mode 100644 index 00000000000..120d93b5d7d --- /dev/null +++ b/jobs/orcid/RevokeOrcidToken.php @@ -0,0 +1,68 @@ +user->getData('orcidAccessToken'); + $httpClient = Application::get()->getHttpClient(); + $headers = ['Accept' => 'application/json']; + + $postData = [ + 'token' => $token, + 'client_id' => OrcidManager::getClientId($this->context), + 'client_secret' => OrcidManager::getClientSecret($this->context) + ]; + + try { + $httpClient->request( + 'POST', + OrcidManager::getTokenRevocationUrl(), + [ + 'headers' => $headers, + 'form_params' => $postData, + ], + ); + + OrcidManager::logInfo('Token revoked for user, with ID: ' . $this->user->getId()); + } catch (ClientException $exception) { + $this->fail($exception); + $httpStatus = $exception->getCode(); + $error = json_decode($exception->getResponse()->getBody(), true); + OrcidManager::logError("ORCID token revocation failed with status {$httpStatus}. Error: " . $error['error_description']); + } + } +} diff --git a/templates/form/orcidProfile.tpl b/templates/form/orcidProfile.tpl index f350aad235a..17fff7874a7 100644 --- a/templates/form/orcidProfile.tpl +++ b/templates/form/orcidProfile.tpl @@ -25,7 +25,17 @@ {capture name=orcidLink assign=orcidLink} {if $orcidAuthenticated} - {$orcidIcon}{$orcid} + {$orcidIcon}{$orcid} + {else} {$orcid} {$orcidButton} {/if} diff --git a/templates/user/identityForm.tpl b/templates/user/identityForm.tpl index 032281e9823..b36f9c42946 100644 --- a/templates/user/identityForm.tpl +++ b/templates/user/identityForm.tpl @@ -12,6 +12,16 @@ $(function() {ldelim} // Attach the form handler. $('#identityForm').pkpHandler('$.pkp.controllers.form.AjaxFormHandler'); + + $('#deleteOrcidButton').on('click', function(e) { + const isModalConfirmTrigger = !e.originalEvent; + // Only execute logic when button was clicked via ButtonConfirmationModalHandler + if(isModalConfirmTrigger){ + $('#identityForm').append(''); + $('#identityForm').submit(); + $('#removeOrcidId').remove(); + } + }); {rdelim}); @@ -44,8 +54,13 @@ {* FIXME: The form element is still required for "connect ORCID" functionality to work. *} {fbvFormSection} {fbvElement type="text" label="user.orcid" name="orcid" id="orcid" value=$orcid maxlength="46"} + + {include file="form/orcidProfile.tpl"} + {if $orcid && $orcidAuthenticated} + {include file="linkAction/buttonConfirmationLinkAction.tpl" titleIcon="modal_delete" buttonSelector="#deleteOrcidButton" dialogText="orcid.field.deleteOrcidModal.message"} + {fbvElement type="button" id="deleteOrcidButton" label="common.delete" class="pkp_button pkp_button_offset"} + {/if} {/fbvFormSection} - {include file="form/orcidProfile.tpl"} {/if}