Skip to content

Commit

Permalink
pkp#10318 Revoke ORCID tokens when deleting authenticated ORCIDs
Browse files Browse the repository at this point in the history
  • Loading branch information
taslangraham committed Aug 26, 2024
1 parent 8af045a commit 952507f
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 60 deletions.
3 changes: 2 additions & 1 deletion classes/author/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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));

Expand Down
99 changes: 49 additions & 50 deletions classes/orcid/OrcidManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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;
}

Expand All @@ -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,
Expand All @@ -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]);
}

Expand Down Expand Up @@ -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]);
}

/**
Expand Down Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -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;
}
}
16 changes: 12 additions & 4 deletions classes/user/form/IdentityForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public function readInputData()
parent::readInputData();

$this->readUserVars([
'givenName', 'familyName', 'preferredPublicName', 'orcid',
'givenName', 'familyName', 'preferredPublicName', 'orcid','removeOrcidId'
]);
}

Expand All @@ -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);
}
Expand Down
10 changes: 7 additions & 3 deletions controllers/tab/user/ProfileTabHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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();
Expand Down
70 changes: 70 additions & 0 deletions jobs/orcid/RevokeOrcidToken.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<?php

/**
* @file jobs/orcid/RevokeOrcidToken.php
*
* Copyright (c) 2014-2024 Simon Fraser University
* Copyright (c) 2000-2024 John Willinsky
* Distributed under the GNU GPL v3. For full terms see the file docs/COPYING.
*
* @class DepositOrcidSubmission
*
* @ingroup jobs
*
* @brief Job to revoke a user's ORCID access token for the application.
*/

namespace pkp\jobs\orcid;

use APP\core\Application;
use GuzzleHttp\Exception\ClientException;
use PKP\context\Context;
use PKP\identity\Identity;
use PKP\jobs\BaseJob;
use PKP\orcid\OrcidManager;
use PKP\user\User;

class RevokeOrcidToken extends BaseJob
{
public function __construct(
private readonly Context $context,
private readonly Identity $identity
) {
parent::__construct();
}

/**
* @inheritDoc
*/
public function handle()
{
$token = $this->identity->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,
],
);

$identityTypeName = $this->identity instanceof User ? 'User' : 'Author';
OrcidManager::logInfo("Token revoked for {$identityTypeName}, with ID: " . $this->identity->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']);
}
}
}
12 changes: 11 additions & 1 deletion templates/form/orcidProfile.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,17 @@

{capture name=orcidLink assign=orcidLink}
{if $orcidAuthenticated}
<a href="{$orcid}" target="_blank">{$orcidIcon}{$orcid}</a>
<a href="{$orcid}" target="_blank" id='orcid-link' >{$orcidIcon}{$orcid}</a>
<style>
#orcid-link {
display: flex;
gap: 0.5rem;
}
#orcid-link svg {
width: 2rem;
height: 1.5rem;
}
</style>
{else}
<a href="{$orcid}" target="_blank">{$orcid}</a>&nbsp;{$orcidButton}
{/if}
Expand Down
17 changes: 16 additions & 1 deletion templates/user/identityForm.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -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('<input type="checkbox" id="removeOrcidId" name="removeOrcidId" checked value="true"/>');
$('#identityForm').submit();
$('#removeOrcidId').remove();
}
});
{rdelim});
</script>

Expand Down Expand Up @@ -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}

<p>
Expand Down

0 comments on commit 952507f

Please sign in to comment.