From eeacf6673ce96a95b0d3e04a50ca3fbf33d836e3 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 --- api/v1/orcid/OrcidController.php | 4 + classes/author/Repository.php | 3 +- classes/orcid/OrcidManager.php | 96 +++++++++++----------- classes/user/form/IdentityForm.php | 20 ++++- controllers/tab/user/ProfileTabHandler.php | 10 ++- jobs/orcid/RevokeOrcidToken.php | 68 +++++++++++++++ templates/form/orcidProfile.tpl | 12 ++- templates/user/identityForm.tpl | 17 +++- 8 files changed, 170 insertions(+), 60 deletions(-) create mode 100644 jobs/orcid/RevokeOrcidToken.php diff --git a/api/v1/orcid/OrcidController.php b/api/v1/orcid/OrcidController.php index 305d719492f..64ce2e89db9 100644 --- a/api/v1/orcid/OrcidController.php +++ b/api/v1/orcid/OrcidController.php @@ -23,6 +23,7 @@ use Illuminate\Http\Response; use Illuminate\Support\Facades\Route; use PKP\core\PKPBaseController; +use PKP\jobs\orcid\RevokeOrcidToken; use PKP\jobs\orcid\SendAuthorMail; use PKP\orcid\OrcidManager; use PKP\security\Role; @@ -168,9 +169,12 @@ function (Role $role) { } } + $clonedAuthor = clone $author; $author->setOrcid(null); $author->setOrcidVerified(false); + OrcidManager::removeOrcidAccessToken($author); + dispatch(new RevokeOrcidToken($context, $clonedAuthor)); Repo::author()->edit($author, []); return response()->json([], Response::HTTP_OK); 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..74c464c1aaa 100644 --- a/classes/orcid/OrcidManager.php +++ b/classes/orcid/OrcidManager.php @@ -20,6 +20,7 @@ use PKP\config\Config; use PKP\context\Context; use PKP\core\Core; +use PKP\user\User; class OrcidManager { @@ -99,14 +100,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 +111,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 +128,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 +207,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 +282,24 @@ 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); + $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 +334,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(?Context $context = null): 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..67c150f66c3 100644 --- a/classes/user/form/IdentityForm.php +++ b/classes/user/form/IdentityForm.php @@ -18,6 +18,7 @@ use APP\core\Application; use APP\template\TemplateManager; +use PKP\jobs\orcid\RevokeOrcidToken; use PKP\orcid\OrcidManager; use PKP\user\User; @@ -109,7 +110,7 @@ public function readInputData() parent::readInputData(); $this->readUserVars([ - 'givenName', 'familyName', 'preferredPublicName', 'orcid', + 'givenName', 'familyName', 'preferredPublicName', 'orcid','removeOrcidId' ]); } @@ -121,9 +122,20 @@ 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') { + $clonedUser = clone $user; + $user->setOrcid(null); + $user->setOrcidVerified(false); + + OrcidManager::removeOrcidAccessToken($user); + dispatch(new RevokeOrcidToken($request->getContext(), $clonedUser)); + } 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}