Skip to content

Commit

Permalink
#10290 Optimized queries using eager loading & added canLoginAs, canG…
Browse files Browse the repository at this point in the history
…ossip, canMergeUser to schemas for a consistent API response
  • Loading branch information
Hafsa-Naeem committed Feb 5, 2025
1 parent cf31936 commit 1f1962a
Show file tree
Hide file tree
Showing 6 changed files with 281 additions and 61 deletions.
144 changes: 87 additions & 57 deletions classes/security/Validation.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
use PKP\user\User;
use PKP\userGroup\UserGroup;
use PKP\security\Role;
use PKP\core\PKPApplication;

class Validation
{
Expand Down Expand Up @@ -484,87 +485,116 @@ public static function canAdminister($administeredUserId, $administratorUserId)
*
* @return int The authorized administration level
*/
public static function getAdministrationLevel(int $administeredUserId, int $administratorUserId, ?int $contextId = null): int
{
// You can administer yourself
if ($administeredUserId == $administratorUserId) {
public static function getAdministrationLevel(
int $administeredUserId,
int $administratorUserId,
?int $contextId = null
): int {

if ($administeredUserId === $administratorUserId) {
return self::ADMINISTRATION_FULL;
}

$siteContextId = \PKP\core\PKPApplication::SITE_CONTEXT_ID;
$siteContextId = PKPApplication::SITE_CONTEXT_ID;

// Check if administered user is site admin
$isAdministeredUserSiteAdmin = UserGroup::query()
->withContextIds($siteContextId)
->withRoleIds(Role::ROLE_ID_SITE_ADMIN)
->whereHas('userUserGroups', function ($query) use ($administeredUserId) {
$query->withUserId($administeredUserId)
->withActive();
// single query to fetch user groups assigned to either user
$allUserGroups = UserGroup::query()
->whereHas('userUserGroups', function ($q) use ($administratorUserId, $administeredUserId) {
$q->withActive()->withUserIds([$administratorUserId, $administeredUserId]);
})
->exists();
->with(['userUserGroups' => function ($q) use ($administratorUserId, $administeredUserId) {
$q->withActive()->withUserIds([$administratorUserId, $administeredUserId]);
}])
->get();

if ($isAdministeredUserSiteAdmin) {
return self::ADMINISTRATION_PROHIBITED;
}
// parsing them in memory
$adminUserRoles = [
'siteAdmin' => false,
'managerContexts' => [],
'anyRoleContexts' => [],
];
$administeredUserRoles = [
'siteAdmin' => false,
'allContexts' => [],
];

// Check if administrator user is site admin
$isAdministratorUserSiteAdmin = UserGroup::query()
->withContextIds($siteContextId)
->withRoleIds(Role::ROLE_ID_SITE_ADMIN)
->whereHas('userUserGroups', function ($query) use ($administratorUserId) {
$query->withUserId($administratorUserId)
->withActive();
})
->exists();
foreach ($allUserGroups as $userGroup) {
$roleId = $userGroup->roleId;
$ctxId = $userGroup->contextId ?? 0;

if ($isAdministratorUserSiteAdmin) {
// then each user assignment row
foreach ($userGroup->userUserGroups as $uug) {
$uid = $uug->userId;

// checking site admin
if ($roleId === Role::ROLE_ID_SITE_ADMIN && $ctxId === $siteContextId) {
if ($uid === $administeredUserId) {
$administeredUserRoles['siteAdmin'] = true;
} elseif ($uid === $administratorUserId) {
$adminUserRoles['siteAdmin'] = true;
}
}

// manager role => store context
if ($roleId === Role::ROLE_ID_MANAGER && $uid === $administratorUserId) {
$adminUserRoles['managerContexts'][] = $ctxId;
}

// for the administered user => track any role’s context
if ($uid === $administeredUserId) {
$administeredUserRoles['allContexts'][] = $ctxId;
}

// for the administrator => track all roles
if ($uid === $administratorUserId) {
$adminUserRoles['anyRoleContexts'][] = $ctxId;
}
}
}

// perform the same siteAdmin / manager checks
if ($administeredUserRoles['siteAdmin']) {
return self::ADMINISTRATION_PROHIBITED;
}
if ($adminUserRoles['siteAdmin']) {
return self::ADMINISTRATION_FULL;
}

// Get contexts where administrator has manager role
$administratorManagerContexts = UserGroup::query()
->withRoleIds(Role::ROLE_ID_MANAGER)
->whereHas('userUserGroups', function ($query) use ($administratorUserId) {
$query->withUserId($administratorUserId)
->withActive();
})
->get()
->map(fn ($userGroup) => $userGroup->contextId)
->unique()
->values()
->toArray();

// Ensure the administrator has a manager role somewhere
$administratorManagerContexts = array_unique($adminUserRoles['managerContexts']);
if (empty($administratorManagerContexts)) {
return self::ADMINISTRATION_PROHIBITED;
}

// Get contexts where administered user has roles
$administeredUserContexts = UserGroup::query()
->whereHas('userUserGroups', function ($query) use ($administeredUserId) {
$query->withUserId($administeredUserId)
->withActive();
})
->get()
->map(fn ($userGroup) => $userGroup->contextId)
->unique()
->values()
->toArray();

// Check for conflicting contexts
$administeredUserContexts = array_unique($administeredUserRoles['allContexts']);
$conflictingContexts = array_diff($administeredUserContexts, $administratorManagerContexts);

if (!empty($conflictingContexts)) {
// Check for partial administration
if ($conflictingContexts) {
if ($contextId !== null && in_array($contextId, $administratorManagerContexts)) {
return self::ADMINISTRATION_PARTIAL;
}
return self::ADMINISTRATION_PROHIBITED;
}

// There were no conflicting roles. Permit administration.

return self::ADMINISTRATION_FULL;
}

/**
* Determine if the current user can "Log In As" the target user.
*
* By default, we do a cross-journal check (contextId = null)
* to enforce "Log In As" only in a single journal context, pass $contextId.
*/
public static function canUserLoginAs(
int $targetUserId,
int $currentUserId,
?int $contextId = null
): bool {
// prevent self-login
if ($targetUserId === $currentUserId) {
return false;
}
return self::getAdministrationLevel($targetUserId, $currentUserId, $contextId) === self::ADMINISTRATION_FULL;
}
}

if (!PKP_STRICT_MODE) {
Expand Down
85 changes: 81 additions & 4 deletions classes/submission/maps/Schema.php
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,9 @@ protected function mapByProperties(array $props, Submission $submission, bool|Co
case 'reviewAssignments':
$output[$prop] = $this->getPropertyReviewAssignments($this->reviewAssignments, $anonymizeReviews);
break;
case 'participants':
$output[$prop] = $this->getPropertyParticipants($submission);
break;
case 'reviewersNotAssigned':
$output[$prop] = $currentReviewRound && $this->reviewAssignments->count() >= intval($this->context->getData('numReviewersPerSubmission'));
break;
Expand Down Expand Up @@ -526,24 +529,43 @@ protected function canChangeMetadata(Enumerable $stageAssignments, Submission $s
protected function getPropertyReviewAssignments(Enumerable $reviewAssignments, bool|Collection $anonymizeReviews = false): array
{
$reviews = [];
$request = Application::get()->getRequest();
$currentUser = $request->getUser();

foreach ($reviewAssignments as $reviewAssignment) {
// @todo for now, only show reviews that haven't been
// declined or cancelled
if ($reviewAssignment->getDeclined() || $reviewAssignment->getCancelled()) {
continue;
}

$request = Application::get()->getRequest();
$currentUser = $request->getUser();
$dateDue = is_null($reviewAssignment->getDateDue()) ? null : date('Y-m-d', strtotime($reviewAssignment->getDateDue()));
$dateResponseDue = is_null($reviewAssignment->getDateResponseDue()) ? null : date('Y-m-d', strtotime($reviewAssignment->getDateResponseDue()));
$dateConfirmed = is_null($reviewAssignment->getDateConfirmed()) ? null : date('Y-m-d', strtotime($reviewAssignment->getDateConfirmed()));
$dateCompleted = is_null($reviewAssignment->getDateCompleted()) ? null : date('Y-m-d', strtotime($reviewAssignment->getDateCompleted()));
$dateAssigned = is_null($reviewAssignment->getDateAssigned()) ? null : date('Y-m-d', strtotime($reviewAssignment->getDateAssigned()));

// calculate canLoginAs, default to false
$canLoginAs = false;
$reviewerId = $reviewAssignment->getReviewerId();
if ($reviewerId) {
$canLoginAs = \PKP\security\Validation::canUserLoginAs(
$reviewerId,
$currentUser->getId(),
// pass null for a site-wide check, or submission->getData('contextId') for a context check
null
);
}

// check canGossip (only if this is a reviewer)
$canGossip = false;
if ($reviewerId) {
$canGossip = Repo::user()->canCurrentUserGossip($reviewerId);
}

$reviews[] = [
'id' => (int) $reviewAssignment->getId(),
'isCurrentUserAssigned' => $currentUser->getId() == (int) $reviewAssignment->getReviewerId(),
'isCurrentUserAssigned' => $currentUser->getId() === (int) $reviewAssignment->getReviewerId(),
'statusId' => (int) $reviewAssignment->getStatus(),
'status' => __($reviewAssignment->getStatusKey()),
'dateDue' => $dateDue,
Expand All @@ -559,8 +581,10 @@ protected function getPropertyReviewAssignments(Enumerable $reviewAssignments, b
'reviewerId' => $anonymizeReviews && $anonymizeReviews->contains($reviewAssignment->getId()) ? null : $reviewAssignment->getReviewerId(),
'reviewerFullName' => $anonymizeReviews && $anonymizeReviews->contains($reviewAssignment->getId()) ? '' : $reviewAssignment->getData('reviewerFullName'),
'reviewMethod' => $reviewAssignment->getData('reviewMethod'),
'canLoginAs' => $canLoginAs,
'canGossip' => $canGossip,
'reviewerDisplayInitials' => $anonymizeReviews && $anonymizeReviews->contains($reviewAssignment->getId()) ? '' : Repo::user()->get($reviewAssignment->getReviewerId())->getDisplayInitials(),
'reviewerHasOrcid' => !($anonymizeReviews && $anonymizeReviews->contains($reviewAssignment->getId())) && !!Repo::user()->get($reviewAssignment->getReviewerId())->getData('orcidIsVerified'),
'reviewerHasOrcid' => !($anonymizeReviews && $anonymizeReviews->contains($reviewAssignment->getId())) && !!Repo::user()->get($reviewAssignment->getReviewerId())->getData('orcidIsVerified')
];
}

Expand Down Expand Up @@ -588,6 +612,59 @@ protected function getPropertyReviewRounds(Collection $reviewRounds): array
return $rounds;
}

/**
* Get a list of participants assigned to this submission
* and build an array with canLoginAs
*
* @param Submission $submission
* @return array
*/
protected function getPropertyParticipants(Submission $submission): array
{
$participants = [];

$request = Application::get()->getRequest();
$currentUser = $request->getUser();
$context = $request->getContext();

if (!$context) {
// If the context is somehow null, bail out
return [];
}

// collect all users assigned to this submission
// also can optionally pass a stageId if to get participants from a specific stage,
// but here getting them all.
$usersIterator = Repo::user()
->getCollector()
->filterByContextIds([$context->getId()])
->assignedTo($submission->getId(), null /* or pass a stageId if desired */)
->getMany();

// build the array for each user
foreach ($usersIterator as $user) {
// gather the properties to return
$userId = $user->getId();
$fullName = $user->getFullName();

// get value of canLoginAs
$canLoginAs = \PKP\security\Validation::canUserLoginAs(
$userId,
$currentUser->getId(),
// passing the submission's contextId null for site-wide check
$submission->getData('contextId')
);

$participants[] = [
'id' => $userId,
'fullName' => $fullName,
'canLoginAs' => $canLoginAs,
];
}

return $participants;
}

/**
* Get details about a submission's stage(s)
*
Expand Down
54 changes: 54 additions & 0 deletions classes/user/maps/Schema.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

namespace PKP\user\maps;

use APP\core\Application;
use APP\facades\Repo;
use APP\submission\Submission;
use Illuminate\Support\Enumerable;
Expand All @@ -26,6 +27,7 @@
use PKP\userGroup\relationships\UserUserGroup;
use PKP\userGroup\UserGroup;
use PKP\workflow\WorkflowStageDAO;
use PKP\security\Validation;

class Schema extends \PKP\core\maps\Schema
{
Expand Down Expand Up @@ -126,6 +128,13 @@ protected function mapByProperties(array $props, User $user, array $auxiliaryDat
$output[$prop] = $user->getGossip();
}
break;
case 'canLoginAs':
$output[$prop] = $this->getPropertyCanLoginAs($user);
break;

case 'canMergeUsers':
$output[$prop] = $this->getPropertyCanMergeUsers($user);
break;
case 'reviewsActive':
$output[$prop] = $user->getData('incompleteCount');
break;
Expand Down Expand Up @@ -272,4 +281,49 @@ protected function mapByProperties(array $props, User $user, array $auxiliaryDat

return $output;
}

/**
* Decide if the current user can "log in as" the target user
*/
protected function getPropertyCanLoginAs(User $targetUser): bool
{
$request = Application::get()->getRequest();
$currentUser = $request->getUser();
if (!$currentUser) {
return false; // Not logged in
}

// Prevent logging in as self
if ($currentUser->getId() === $targetUser->getId()) {
return false;
}

// This calls Validation::canUserLoginAs(...) if you have it
return Validation::canUserLoginAs($targetUser->getId(), $currentUser->getId());
}

/**
* Determine if the current user can merge the target user
*/
protected function getPropertyCanMergeUsers(User $targetUser): bool
{
$request = Application::get()->getRequest();
$currentUser = $request->getUser();

// ensure a user is logged in
if (!$currentUser) {
return false;
}

// prevent merging oneself
if ($currentUser->getId() === $targetUser->getId()) {
return false;
}

// ensure user administration permissions
$canAdminister = Validation::getAdministrationLevel($targetUser->getId(), $currentUser->getId()) === Validation::ADMINISTRATION_FULL;

// allow merging if the current user is a site admin or has full admin rights over the target user
return Validation::isSiteAdmin() || $canAdminister;
}
}
Loading

0 comments on commit 1f1962a

Please sign in to comment.