Skip to content

Commit

Permalink
pkp#3666 Prevent dual-assigned editors from accessing discussions
Browse files Browse the repository at this point in the history
  • Loading branch information
NateWr committed May 24, 2018
1 parent a8edb45 commit c5f5c54
Show file tree
Hide file tree
Showing 10 changed files with 198 additions and 68 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<?php
/**
* @file classes/security/authorization/AssignedStageRoleHandlerOperationPolicy.inc.php
*
* Copyright (c) 2014-2018 Simon Fraser University
* Copyright (c) 2000-2018 John Willinsky
* Distributed under the GNU GPL v2. For full terms see the file docs/COPYING.
*
* @class AssignedStageRoleHandlerOperationPolicy
* @ingroup security_authorization
*
* @brief Class to control access to handler operations based on assigned
* role(s) in a submission's workflow stage.
*/

import('lib.pkp.classes.security.authorization.RoleBasedHandlerOperationPolicy');

class AssignedStageRoleHandlerOperationPolicy extends RoleBasedHandlerOperationPolicy {

/** @var int */
var $_stageId;

/**
* Constructor
* @param $request PKPRequest
* @param $roles array|integer either a single role ID or an array of role ids
* @param $operations array|string either a single operation or a list of operations that
* this policy is targeting.
* @param $stageId int The stage ID to check for assigned roles
* @param $message string a message to be displayed if the authorization fails
* @param $allRoles boolean whether all roles must match ("all of") or whether it is
* enough for only one role to match ("any of"). Default: false ("any of")
*/
function __construct($request, $roles, $operations, $stageId,
$message = 'user.authorization.assignedStageRoleBasedAccessDenied',
$allRoles = false) {
parent::__construct($request, $roles, $operations, $message, $allRoles);

$this->_stageId = $stageId;
}

//
// Implement template methods from AuthorizationPolicy
//
/**
* @see AuthorizationPolicy::effect()
*/
function effect() {
// Check whether the user has one of the allowed roles
// assigned. If that's the case we'll permit access.
// Get user roles grouped by context.
$userRoles = $this->getAuthorizedContextObject(ASSOC_TYPE_ACCESSIBLE_WORKFLOW_STAGES);
if (empty($userRoles) || empty($userRoles[$this->_stageId])) return AUTHORIZATION_DENY;

if (!$this->_checkUserRoleAssignment($userRoles[$this->_stageId])) return AUTHORIZATION_DENY;
if (!$this->_checkOperationWhitelist()) return AUTHORIZATION_DENY;

return AUTHORIZATION_PERMIT;
}
}

?>
18 changes: 4 additions & 14 deletions classes/security/authorization/QueryAccessPolicy.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,15 @@ function __construct($request, $args, $roleAssignments, $stageId) {
// We need a valid workflow stage.
import('lib.pkp.classes.security.authorization.QueryWorkflowStageAccessPolicy');
$this->addPolicy(new QueryWorkflowStageAccessPolicy($request, $args, $roleAssignments, 'submissionId', $stageId));


// We need a query matching the submission in the request.
import('lib.pkp.classes.security.authorization.internal.QueryRequiredPolicy');
$this->addPolicy(new QueryRequiredPolicy($request, $args));

// The query must be assigned to the current user, with exceptions for Managers
import('lib.pkp.classes.security.authorization.internal.QueryAssignedToUserAccessPolicy');
$this->addPolicy(new QueryAssignedToUserAccessPolicy($request));

// Authors, reviewers, context managers and sub editors potentially have
// access to queries. We'll have to define
// differentiated policies for those roles in a policy set.
Expand All @@ -48,7 +51,6 @@ function __construct($request, $args, $roleAssignments, $stageId) {
$queryAccessPolicy->addPolicy(new RoleBasedHandlerOperationPolicy($request, ROLE_ID_MANAGER, $roleAssignments[ROLE_ID_MANAGER]));
}


//
// Assistants
//
Expand All @@ -62,10 +64,6 @@ function __construct($request, $args, $roleAssignments, $stageId) {
import('lib.pkp.classes.security.authorization.QueryWorkflowStageAccessPolicy'); // pulled from context-specific class path.
$assistantQueryAccessPolicy->addPolicy(new QueryWorkflowStageAccessPolicy($request, $args, $roleAssignments, 'submissionId', $stageId));

// 3) ... and the assistant is assigned to the query.
import('lib.pkp.classes.security.authorization.internal.QueryAssignedToUserAccessPolicy');
$assistantQueryAccessPolicy->addPolicy(new QueryAssignedToUserAccessPolicy($request));

$queryAccessPolicy->addPolicy($assistantQueryAccessPolicy);
}

Expand All @@ -81,10 +79,6 @@ function __construct($request, $args, $roleAssignments, $stageId) {
import('lib.pkp.classes.security.authorization.QueryWorkflowStageAccessPolicy');
$reviewerQueryAccessPolicy->addPolicy(new QueryWorkflowStageAccessPolicy($request, $args, $roleAssignments, 'submissionId', $stageId));

// 3) ... and the reviewer is assigned to the query.
import('lib.pkp.classes.security.authorization.internal.QueryAssignedToUserAccessPolicy');
$reviewerQueryAccessPolicy->addPolicy(new QueryAssignedToUserAccessPolicy($request));

$queryAccessPolicy->addPolicy($reviewerQueryAccessPolicy);
}

Expand All @@ -100,10 +94,6 @@ function __construct($request, $args, $roleAssignments, $stageId) {
import('lib.pkp.classes.security.authorization.QueryWorkflowStageAccessPolicy');
$authorQueryAccessPolicy->addPolicy(new QueryWorkflowStageAccessPolicy($request, $args, $roleAssignments, 'submissionId', $stageId));

// 3) ... and the author is assigned to the query.
import('lib.pkp.classes.security.authorization.internal.QueryAssignedToUserAccessPolicy');
$authorQueryAccessPolicy->addPolicy(new QueryAssignedToUserAccessPolicy($request));

$queryAccessPolicy->addPolicy($authorQueryAccessPolicy);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class RoleBasedHandlerOperationPolicy extends HandlerOperationPolicy {
* this policy is targeting.
* @param $message string a message to be displayed if the authorization fails
* @param $allRoles boolean whether all roles must match ("all of") or whether it is
* enough for only one role to match ("any of").
* enough for only one role to match ("any of"). Default: false ("any of")
*/
function __construct($request, $roles, $operations,
$message = 'user.authorization.roleBasedAccessDenied',
Expand Down
39 changes: 34 additions & 5 deletions classes/security/authorization/SubmissionFileAccessPolicy.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,16 @@ function buildFileAccessPolicy($request, $args, $roleAssignments, $mode, $fileId
// Managerial role
//
if (isset($roleAssignments[ROLE_ID_MANAGER])) {
// Managers have all access to all submissions.
$fileAccessPolicy->addPolicy(new RoleBasedHandlerOperationPolicy($request, ROLE_ID_MANAGER, $roleAssignments[ROLE_ID_MANAGER]));
// Managers can access all submission files as long as the manager has not
// been assigned to a lesser role in the stage.
$managerFileAccessPolicy = new PolicySet(COMBINING_DENY_OVERRIDES);
$managerFileAccessPolicy->addPolicy(new RoleBasedHandlerOperationPolicy($request, ROLE_ID_MANAGER, $roleAssignments[ROLE_ID_MANAGER]));
import('lib.pkp.classes.security.authorization.WorkflowStageAccessPolicy');
$managerFileAccessPolicy->addPolicy(new WorkflowStageAccessPolicy($request, $args, $roleAssignments, 'submissionId', $request->getUserVar('stageId')));
import('lib.pkp.classes.security.authorization.AssignedStageRoleHandlerOperationPolicy');
$managerFileAccessPolicy->addPolicy(new AssignedStageRoleHandlerOperationPolicy($request, ROLE_ID_MANAGER, $roleAssignments[ROLE_ID_MANAGER], $request->getUserVar('stageId')));

$fileAccessPolicy->addPolicy($managerFileAccessPolicy);
}


Expand All @@ -83,9 +91,11 @@ function buildFileAccessPolicy($request, $args, $roleAssignments, $mode, $fileId
$authorFileAccessPolicy = new PolicySet(COMBINING_DENY_OVERRIDES);
$authorFileAccessPolicy->addPolicy(new RoleBasedHandlerOperationPolicy($request, ROLE_ID_AUTHOR, $roleAssignments[ROLE_ID_AUTHOR]));

// 2) ...if they are assigned to the workflow stage. Note: This loads the application-specific policy class.
// 2) ...if they are assigned to the workflow stage as an author. Note: This loads the application-specific policy class.
import('lib.pkp.classes.security.authorization.WorkflowStageAccessPolicy');
$authorFileAccessPolicy->addPolicy(new WorkflowStageAccessPolicy($request, $args, $roleAssignments, 'submissionId', $request->getUserVar('stageId')));
import('lib.pkp.classes.security.authorization.AssignedStageRoleHandlerOperationPolicy');
$authorFileAccessPolicy->addPolicy(new AssignedStageRoleHandlerOperationPolicy($request, ROLE_ID_AUTHOR, $roleAssignments[ROLE_ID_AUTHOR], $request->getUserVar('stageId')));

// 3) ...and if they meet one of the following requirements:
$authorFileAccessOptionsPolicy = new PolicySet(COMBINING_PERMIT_OVERRIDES);
Expand Down Expand Up @@ -169,10 +179,27 @@ function buildFileAccessPolicy($request, $args, $roleAssignments, $mode, $fileId
$contextAssistantFileAccessPolicy = new PolicySet(COMBINING_DENY_OVERRIDES);
$contextAssistantFileAccessPolicy->addPolicy(new RoleBasedHandlerOperationPolicy($request, ROLE_ID_ASSISTANT, $roleAssignments[ROLE_ID_ASSISTANT]));

// 2) ... but only if they have been assigned to the submission workflow.
// 2) ... but only if they have been assigned to the submission workflow as an assistant.
// Note: This loads the application-specific policy class
import('lib.pkp.classes.security.authorization.WorkflowStageAccessPolicy');
$contextAssistantFileAccessPolicy->addPolicy(new WorkflowStageAccessPolicy($request, $args, $roleAssignments, 'submissionId', $request->getUserVar('stageId')));
import('lib.pkp.classes.security.authorization.AssignedStageRoleHandlerOperationPolicy');
$contextAssistantFileAccessPolicy->addPolicy(new AssignedStageRoleHandlerOperationPolicy($request, ROLE_ID_ASSISTANT, $roleAssignments[ROLE_ID_ASSISTANT], $request->getUserVar('stageId')));

// 3) ...and if they meet one of the following requirements:
$contextAssistantFileAccessOptionsPolicy = new PolicySet(COMBINING_PERMIT_OVERRIDES);

// 3a) ...the file not part of a query...
import('lib.pkp.classes.security.authorization.internal.SubmissionFileNotQueryAccessPolicy');
$contextAssistantFileAccessOptionsPolicy->addPolicy(new SubmissionFileNotQueryAccessPolicy($request, $fileIdAndRevision));

// 3b) ...or the file is part of a query they are assigned to...
import('lib.pkp.classes.security.authorization.internal.SubmissionFileAssignedQueryAccessPolicy');
$contextAssistantFileAccessOptionsPolicy->addPolicy(new SubmissionFileAssignedQueryAccessPolicy($request, $fileIdAndRevision));

// Add the rules from 3
$contextAssistantFileAccessPolicy->addPolicy($contextAssistantFileAccessOptionsPolicy);

$fileAccessPolicy->addPolicy($contextAssistantFileAccessPolicy);
}

Expand All @@ -184,9 +211,11 @@ function buildFileAccessPolicy($request, $args, $roleAssignments, $mode, $fileId
$subEditorFileAccessPolicy = new PolicySet(COMBINING_DENY_OVERRIDES);
$subEditorFileAccessPolicy->addPolicy(new RoleBasedHandlerOperationPolicy($request, ROLE_ID_SUB_EDITOR, $roleAssignments[ROLE_ID_SUB_EDITOR]));

// 2) ... but only if they have been assigned to the requested submission.
// 2) ... but only if they have been assigned as a subeditor to the requested submission.
import('lib.pkp.classes.security.authorization.internal.UserAccessibleWorkflowStageRequiredPolicy');
$subEditorFileAccessPolicy->addPolicy(new UserAccessibleWorkflowStageRequiredPolicy($request));
import('lib.pkp.classes.security.authorization.AssignedStageRoleHandlerOperationPolicy');
$subEditorFileAccessPolicy->addPolicy(new AssignedStageRoleHandlerOperationPolicy($request, ROLE_ID_SUB_EDITOR, $roleAssignments[ROLE_ID_SUB_EDITOR], $request->getUserVar('stageId')));

$fileAccessPolicy->addPolicy($subEditorFileAccessPolicy);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ function effect() {
$queryDao = DAORegistry::getDAO('QueryDAO');
if ($queryDao->getParticipantIds($query->getId(), $user->getId())) return AUTHORIZATION_PERMIT;

// Managers are allowed to access discussions they are not participants in
// as long as they have Manager-level access to the workflow stage
$accessibleWorkflowStages = $this->getAuthorizedContextObject(ASSOC_TYPE_ACCESSIBLE_WORKFLOW_STAGES);
$managerAssignments = array_intersect(array(ROLE_ID_MANAGER), $accessibleWorkflowStages[$query->getStageId()]);
if (!empty($managerAssignments)) return AUTHORIZATION_PERMIT;

// Otherwise, deny.
return AUTHORIZATION_DENY;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php
/**
* @file classes/security/authorization/internal/SubmissionFileNotQueryAccessPolicy.inc.php
*
* Copyright (c) 2014-2018 Simon Fraser University
* Copyright (c) 2000-2018 John Willinsky
* Distributed under the GNU GPL v2. For full terms see the file docs/COPYING.
*
* @class SubmissionFileNotQueryAccessPolicy
* @ingroup security_authorization_internal
*
* @brief Submission file policy to check if the requested file is not attached
* to a query. This returns AUTHORIZATION_PERMIT for _any_ file that is not
* attached to a query note.
*/

import('lib.pkp.classes.security.authorization.internal.SubmissionFileBaseAccessPolicy');

class SubmissionFileNotQueryAccessPolicy extends SubmissionFileBaseAccessPolicy {

/**
* @see AuthorizationPolicy::effect()
*/
function effect() {
$request = $this->getRequest();

// Get the submission file
$submissionFile = $this->getSubmissionFile($request);
if (!is_a($submissionFile, 'SubmissionFile')) return AUTHORIZATION_DENY;

// Check if it's associated with a note.
if ($submissionFile->getAssocType() != ASSOC_TYPE_NOTE) return AUTHORIZATION_PERMIT;

// Check if that note is associated with a query
$noteDao = DAORegistry::getDAO('NoteDAO');
$note = $noteDao->getById($submissionFile->getAssocId());
if ($note->getAssocType() != ASSOC_TYPE_QUERY) return AUTHORIZATION_PERMIT;

return AUTHORIZATION_DENY;
}
}

?>
Loading

0 comments on commit c5f5c54

Please sign in to comment.