From 43e51b3ecbcdb4d30127c775c6b0144b610154ff Mon Sep 17 00:00:00 2001 From: Taslan Graham Date: Thu, 5 Sep 2024 15:00:08 -0500 Subject: [PATCH] pkp/pkp-lib#10131 cleanup for Orcid implementation cleanup --- api/v1/orcid/OrcidController.php | 137 ++++++++++++++------------ classes/orcid/OrcidManager.php | 8 +- jobs/orcid/DepositOrcidSubmission.php | 2 +- jobs/orcid/RevokeOrcidToken.php | 2 +- jobs/orcid/SendAuthorMail.php | 4 - 5 files changed, 79 insertions(+), 74 deletions(-) diff --git a/api/v1/orcid/OrcidController.php b/api/v1/orcid/OrcidController.php index 305d719492f..59516d279e2 100644 --- a/api/v1/orcid/OrcidController.php +++ b/api/v1/orcid/OrcidController.php @@ -17,6 +17,7 @@ namespace PKP\API\v1\orcid; +use APP\author\Author; use APP\facades\Repo; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; @@ -72,45 +73,15 @@ public function getGroupRoutes(): void public function requestAuthorVerification(Request $illuminateRequest): JsonResponse { $context = $this->getRequest()->getContext(); - if (!OrcidManager::isEnabled($context)) { - return response()->json([ - 'error' => __('api.orcid.403.orcidNotEnabled'), - ], Response::HTTP_FORBIDDEN); - } - - $authorId = (int) $illuminateRequest->route('authorId'); - $author = Repo::author()->get($authorId); + $validate = $this->validateRequest($illuminateRequest); - if (empty($author)) { + if ($validate['error']) { return response()->json([ - 'error' => __('api.orcid.404.authorNotFound'), - ], Response::HTTP_NOT_FOUND); - } - - $user = $this->getRequest()->getUser(); - $currentRoles = array_map( - function (Role $role) { - return $role->getId(); - }, - $user->getRoles($context->getId()) - ); - - if (!array_intersect([Role::ROLE_ID_SITE_ADMIN, Role::ROLE_ID_MANAGER], $currentRoles)) { - $publicationId = $author->getData('publicationId'); - $submissionId = Repo::publication()->get($publicationId)->getData('submissionId'); - - $editorAssignment = StageAssignment::withSubmissionIds([$submissionId]) - ->withRoleIds([Role::ROLE_ID_SUB_EDITOR]) - ->withUserId($user->getId()) - ->first(); - - if ($editorAssignment === null) { - return response()->json([ - 'error' => __('api.orcid.403.editWithoutPermission'), - ], Response::HTTP_FORBIDDEN); - } + 'error' => $validate['error'], + ], $validate['status']); } + $author = $validate['author']; /** @var Author $author */ try { dispatch(new SendAuthorMail($author, $context, true)); } catch (\Exception $exception) { @@ -128,23 +99,38 @@ function (Role $role) { */ public function deleteForAuthor(Request $illuminateRequest): JsonResponse { - $context = $this->getRequest()->getContext(); - if (!OrcidManager::isEnabled($context)) { - return response()->json([ - 'error' => __('api.orcid.403.orcidNotEnabled'), - ], Response::HTTP_FORBIDDEN); + $validate = $this->validateRequest($illuminateRequest); + + if ($validate['error']) { + return response()->json(['error' => $validate['error'],], $validate['status']); } - $authorId = (int) $illuminateRequest->route('authorId'); - $author = Repo::author()->get($authorId); + $author = $validate['author']; /** @var Author $author */ - if (empty($author)) { - return response()->json([ - 'error' => __('api.orcid.404.authorNotFound'), - ], Response::HTTP_NOT_FOUND); - } + $author->setOrcid(null); + $author->setOrcidVerified(false); + OrcidManager::removeOrcidAccessToken($author); + Repo::author()->edit($author, []); + return response()->json([], Response::HTTP_OK); + } + + /** + * Check if ORCID is enabled in the current context + */ + private function isOrcidEnabled(): bool + { + $context = $this->getRequest()->getContext(); + return OrcidManager::isEnabled($context); + } + + /** + * Check if the current user has editor permissions + */ + private function hasEditorPermissions(int $publicationId): bool + { $user = $this->getRequest()->getUser(); + $context = $this->getRequest()->getContext(); $currentRoles = array_map( function (Role $role) { return $role->getId(); @@ -152,27 +138,50 @@ function (Role $role) { $user->getRoles($context->getId()) ); - if (!array_intersect([Role::ROLE_ID_SITE_ADMIN, Role::ROLE_ID_MANAGER], $currentRoles)) { - $publicationId = $author->getData('publicationId'); - $submissionId = Repo::publication()->get($publicationId)->getData('submissionId'); + if (array_intersect([Role::ROLE_ID_SITE_ADMIN, Role::ROLE_ID_MANAGER], $currentRoles)) { + return true; + } + + $submissionId = Repo::publication()->get($publicationId)->getData('submissionId'); + $editorAssignment = StageAssignment::withSubmissionIds([$submissionId]) + ->withRoleIds([Role::ROLE_ID_SUB_EDITOR]) + ->withUserId($user->getId()) + ->first(); + + return $editorAssignment !== null; + } + + /** + * Performs common validation logic for controller endpoints, return validated data or error information. + * + * @return array Returns an associative array containing either the validated data or error information + */ + private function validateRequest(Request $illuminateRequest): array + { + if (!$this->isOrcidEnabled()) { + return [ + 'error' => __('api.orcid.403.orcidNotEnabled'), + 'status' => Response::HTTP_FORBIDDEN + ]; + } + + $authorId = (int)$illuminateRequest->route('authorId'); + $author = Repo::author()->get($authorId); - $editorAssignment = StageAssignment::withSubmissionIds([$submissionId]) - ->withRoleIds([Role::ROLE_ID_SUB_EDITOR]) - ->withUserId($user->getId()) - ->first(); + if (empty($author)) { + ['error' => __('api.orcid.404.authorNotFound'), + 'status' => Response::HTTP_NOT_FOUND + ]; + } - if ($editorAssignment === null) { - return response()->json([ - 'error' => __('api.orcid.403.editWithoutPermission'), - ], Response::HTTP_FORBIDDEN); - } + if (!$this->hasEditorPermissions($author->getData('publicationId'))) { + return [ + 'error' => __('api.orcid.403.editWithoutPermission'), + 'status' => Response::HTTP_FORBIDDEN + ]; } - $author->setOrcid(null); - $author->setOrcidVerified(false); - OrcidManager::removeOrcidAccessToken($author); - Repo::author()->edit($author, []); + return ['author' => $author]; - return response()->json([], Response::HTTP_OK); } } diff --git a/classes/orcid/OrcidManager.php b/classes/orcid/OrcidManager.php index ceb1a29416c..636d9ccbd15 100644 --- a/classes/orcid/OrcidManager.php +++ b/classes/orcid/OrcidManager.php @@ -101,7 +101,7 @@ public static function isEnabled(?Context $context = null): bool */ public static function getOrcidUrl(?Context $context = null): string { - $apiType = self::getApiType(); + $apiType = self::getApiType($context); return in_array($apiType, [self::API_PUBLIC_PRODUCTION, self::API_MEMBER_PRODUCTION]) ? self::ORCID_URL : self::ORCID_URL_SANDBOX; } @@ -112,7 +112,7 @@ public static function getOrcidUrl(?Context $context = null): string */ public static function getApiPath(?Context $context = null): string { - $apiType = self::getApiType(); + $apiType = self::getApiType($context); return match ($apiType) { self::API_PUBLIC_SANDBOX => self::ORCID_API_URL_PUBLIC_SANDBOX, @@ -129,7 +129,7 @@ public static function getApiPath(?Context $context = null): string */ public static function isSandbox(?Context $context = null): bool { - $apiType = self::getApiType(); + $apiType = self::getApiType($context); return in_array($apiType, [self::API_PUBLIC_SANDBOX, self::API_MEMBER_SANDBOX]); } @@ -208,7 +208,7 @@ public static function getCountry(?Context $context = null): string */ public static function isMemberApiEnabled(?Context $context = null): bool { - $apiType = self::getApiType(); + $apiType = self::getApiType($context); return in_array($apiType, [self::API_MEMBER_PRODUCTION, self::API_MEMBER_SANDBOX]); } diff --git a/jobs/orcid/DepositOrcidSubmission.php b/jobs/orcid/DepositOrcidSubmission.php index 09c8430ead1..803c6b9c717 100644 --- a/jobs/orcid/DepositOrcidSubmission.php +++ b/jobs/orcid/DepositOrcidSubmission.php @@ -39,7 +39,7 @@ public function __construct( /** * @inheritDoc */ - public function handle() + public function handle(): void { // If the application is set to sandbox mode, it will not reach out to external services if (Config::getVar('general', 'sandbox', false)) { diff --git a/jobs/orcid/RevokeOrcidToken.php b/jobs/orcid/RevokeOrcidToken.php index 81dda880d6f..0f251b96b32 100644 --- a/jobs/orcid/RevokeOrcidToken.php +++ b/jobs/orcid/RevokeOrcidToken.php @@ -36,7 +36,7 @@ public function __construct( /** * @inheritDoc */ - public function handle() + public function handle(): void { $token = $this->identity->getData('orcidAccessToken'); $httpClient = Application::get()->getHttpClient(); diff --git a/jobs/orcid/SendAuthorMail.php b/jobs/orcid/SendAuthorMail.php index a02eb0590cc..505a35fffb3 100644 --- a/jobs/orcid/SendAuthorMail.php +++ b/jobs/orcid/SendAuthorMail.php @@ -17,11 +17,8 @@ namespace PKP\jobs\orcid; use APP\author\Author; -use APP\core\Application; use APP\facades\Repo; -use GuzzleHttp\Exception\ClientException; use Illuminate\Support\Facades\Mail; -use PKP\config\Config; use PKP\context\Context; use PKP\jobs\BaseJob; use PKP\mail\mailables\OrcidCollectAuthorId; @@ -30,7 +27,6 @@ class SendAuthorMail extends BaseJob { - public function __construct( private Author $author, private Context $context,