Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkp/pkp-lib#5504 Control access to settings by user group #10380

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/v1/contexts/PKPContextController.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use PKP\plugins\Hook;
use PKP\plugins\Plugin;
use PKP\plugins\PluginRegistry;
use PKP\security\authorization\CanAccessSettingsPolicy;
use PKP\security\authorization\PolicySet;
use PKP\security\authorization\RoleBasedHandlerOperationPolicy;
use PKP\security\authorization\UserRolesRequiredPolicy;
Expand Down Expand Up @@ -116,6 +117,7 @@ public function getGroupRoutes(): void
public function authorize(PKPRequest $request, array &$args, array $roleAssignments): bool
{
$this->addPolicy(new UserRolesRequiredPolicy($request), true);
$this->addPolicy(new CanAccessSettingsPolicy());

$rolePolicy = new PolicySet(PolicySet::COMBINING_PERMIT_OVERRIDES);

Expand Down
1 change: 1 addition & 0 deletions classes/install/PKPInstall.php
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ public function createData()
$adminUserGroup = Repo::userGroup()->newDataObject();
$adminUserGroup->setRoleId(Role::ROLE_ID_SITE_ADMIN);
$adminUserGroup->setContextId(\PKP\core\PKPApplication::SITE_CONTEXT_ID);
$adminUserGroup->setPermitSettings(true);
$adminUserGroup->setDefault(true);
foreach ($this->installedLocales as $locale) {
$name = __('default.groups.name.siteAdmin', [], $locale);
Expand Down
1 change: 1 addition & 0 deletions classes/migration/install/RolesAndUserGroupsMigration.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public function up(): void
$table->smallInteger('show_title')->default(1);
$table->smallInteger('permit_self_registration')->default(0);
$table->smallInteger('permit_metadata_edit')->default(0);
$table->smallInteger('permit_settings')->default(0);
$table->smallInteger('masthead')->default(0);
$table->index(['user_group_id'], 'user_groups_user_group_id');
$table->index(['role_id'], 'user_groups_role_id');
Expand Down
47 changes: 47 additions & 0 deletions classes/migration/upgrade/v3_5_0/I5504_UserGroupsSettings.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

/**
* @file classes/migration/upgrade/v3_5_0/I5504_UserGroupsSettings.php
*
* Copyright (c) 2024 Simon Fraser University
* Copyright (c) 2024 John Willinsky
* Distributed under the GNU GPL v3. For full terms see the file docs/COPYING.
*
* @class I5504_UserGroupsSettings
*
* @brief Add permit_settings column to the user_groups table.
*/

namespace PKP\migration\upgrade\v3_5_0;

use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;
use PKP\migration\Migration;

class I5504_UserGroupsSettings extends Migration
{
/**
* Run the migration.
*/
public function up(): void
{
Schema::table('user_groups', function (Blueprint $table) {
$table->smallInteger('permit_settings')->default(0);
});
DB::table('user_groups')->where('role_id', 1)->update(['permit_settings' => 1]); // role_id = 1 is ROLE_ID_SITE_ADMIN
DB::table('user_groups')->where('role_id', 16)->update(['permit_settings' => 1]); // role_id = 16 is ROLE_ID_MANAGER
}

/**
* Reverse the downgrades
*/
public function down(): void
{
Schema::table('user_groups', function (Blueprint $table) {
if (Schema::hasColumn($table->getTable(), 'permit_settings')) {
$table->dropColumn('permit_settings');
};
});
}
}
42 changes: 42 additions & 0 deletions classes/security/authorization/CanAccessSettingsPolicy.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php
/**
* @file classes/security/authorization/CanAccessSettingsPolicy.php
*
* Copyright (c) 2024 Simon Fraser University
* Copyright (c) 2024 John Willinsky
* Distributed under the GNU GPL v3. For full terms see the file docs/COPYING.
*
* @class CanAccessSettingsPolicy
*
* @brief Check to ensure that the user has access to the context settings area.
*/

namespace PKP\security\authorization;

use APP\core\Application;
use PKP\security\Role;

class CanAccessSettingsPolicy extends AuthorizationPolicy
{
//
// Implement template methods from AuthorizationPolicy
//
/**
* @see AuthorizationPolicy::effect()
*/
public function effect()
{
// At least one user group must be an admin, or a manager with setup access.
$userGroups = $this->getAuthorizedContextObject(Application::ASSOC_TYPE_USER_GROUP);
foreach ($userGroups as $userGroup) {
if ($userGroup->getRoleId() == ROLE_ID_SITE_ADMIN) {
return AuthorizationPolicy::AUTHORIZATION_PERMIT;
}
if ($userGroup->getRoleId() == Role::ROLE_ID_MANAGER && $userGroup->getPermitSettings()) {
return AuthorizationPolicy::AUTHORIZATION_PERMIT;
}
}

return AuthorizationPolicy::AUTHORIZATION_DENY;
}
}
52 changes: 10 additions & 42 deletions classes/security/authorization/UserRolesRequiredPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@

use APP\core\Application;
use APP\core\Request;
use PKP\db\DAORegistry;
use PKP\security\Role;
use PKP\security\RoleDAO;
use APP\facades\Repo;

class UserRolesRequiredPolicy extends AuthorizationPolicy
{
Expand Down Expand Up @@ -50,51 +48,21 @@ public function effect()
$request = $this->_request;
$user = $request->getUser();

if (!$user instanceof \PKP\user\User) {
if (!$user) {
return AuthorizationPolicy::AUTHORIZATION_DENY;
}

// Get all user roles.
$roleDao = DAORegistry::getDAO('RoleDAO'); /** @var RoleDAO $roleDao */
$userRoles = $roleDao->getByUserIdGroupedByContext($user->getId());

$context = $request->getRouter()->getContext($request);
$roleContext = $context?->getId() ?? Application::SITE_CONTEXT_ID;

$contextRoles = $this->_getContextRoles($roleContext, $userRoles);
$userGroups = Repo::userGroup()->getCollector()
->filterByUserIds([$user->getId()])
->filterByContextIds($context ? [$context->getId(), Application::SITE_CONTEXT_ID] : [Application::SITE_CONTEXT_ID])
->getMany()->toArray();

$this->addAuthorizedContextObject(Application::ASSOC_TYPE_USER_ROLES, $contextRoles);
return AuthorizationPolicy::AUTHORIZATION_PERMIT;
}
$roleIds = array_map(fn ($userGroup) => $userGroup->getRoleId(), $userGroups);
$this->addAuthorizedContextObject(Application::ASSOC_TYPE_USER_ROLES, $roleIds);
$this->addAuthorizedContextObject(Application::ASSOC_TYPE_USER_GROUP, $userGroups);

/**
* Get the current context roles from all user roles.
*
* @param array<int,Role[]> $userRoles List of roles grouped by contextId
*/
protected function _getContextRoles(?int $contextId, array $userRoles): array
{
// Adapt the role context based on the passed role id.
$contextRoles = [];

// Check if user has site level or manager roles.
if (array_key_exists((int) Application::SITE_CONTEXT_ID, $userRoles) &&
array_key_exists(Role::ROLE_ID_SITE_ADMIN, $userRoles[(int) Application::SITE_CONTEXT_ID])) {
// site level role
$contextRoles[] = Role::ROLE_ID_SITE_ADMIN;
}

// Get the user roles related to the passed context.
if ($contextId !== Application::SITE_CONTEXT_ID && isset($userRoles[$contextId])) {
// Filter the user roles to the found context id.
return array_merge(
$contextRoles,
array_map(fn ($role) => $role->getRoleId(), $userRoles[$contextId])
);
} else {
// Context id not present in user roles array.
return $contextRoles;
}
return AuthorizationPolicy::AUTHORIZATION_PERMIT;
}
}

Expand Down
62 changes: 33 additions & 29 deletions classes/template/PKPTemplateManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -1046,36 +1046,40 @@ public function setupBackendPage()
'isCurrent' => $request->getRequestedPage() === 'management' && in_array('institutions', (array) $request->getRequestedArgs()),
];
}
$menu['settings'] = [
'name' => __('navigation.settings'),
'submenu' => [
'context' => [
'name' => __('context.context'),
'url' => $router->url($request, null, 'management', 'settings', ['context']),
'isCurrent' => $router->getRequestedPage($request) === 'management' && in_array('context', (array) $router->getRequestedArgs($request)),
],
'website' => [
'name' => __('manager.website'),
'url' => $router->url($request, null, 'management', 'settings', ['website']),
'isCurrent' => $router->getRequestedPage($request) === 'management' && in_array('website', (array) $router->getRequestedArgs($request)),
],
'workflow' => [
'name' => __('manager.workflow'),
'url' => $router->url($request, null, 'management', 'settings', ['workflow']),
'isCurrent' => $router->getRequestedPage($request) === 'management' && in_array('workflow', (array) $router->getRequestedArgs($request)),
],
'distribution' => [
'name' => __('manager.distribution'),
'url' => $router->url($request, null, 'management', 'settings', ['distribution']),
'isCurrent' => $router->getRequestedPage($request) === 'management' && in_array('distribution', (array) $router->getRequestedArgs($request)),
],
'access' => [
'name' => __('navigation.access'),
'url' => $router->url($request, null, 'management', 'settings', ['access']),
'isCurrent' => $router->getRequestedPage($request) === 'management' && in_array('access', (array) $router->getRequestedArgs($request)),
$userGroups = (array) $router->getHandler()->getAuthorizedContextObject(Application::ASSOC_TYPE_USER_GROUP);
$hasSettingsAccess = array_reduce($userGroups, fn ($carry, $userGroup) => $carry || $userGroup->getPermitSettings(), false);
if ($hasSettingsAccess) {
$menu['settings'] = [
'name' => __('navigation.settings'),
'submenu' => [
'context' => [
'name' => __('context.context'),
'url' => $router->url($request, null, 'management', 'settings', ['context']),
'isCurrent' => $router->getRequestedPage($request) === 'management' && in_array('context', (array) $router->getRequestedArgs($request)),
],
'website' => [
'name' => __('manager.website'),
'url' => $router->url($request, null, 'management', 'settings', ['website']),
'isCurrent' => $router->getRequestedPage($request) === 'management' && in_array('website', (array) $router->getRequestedArgs($request)),
],
'workflow' => [
'name' => __('manager.workflow'),
'url' => $router->url($request, null, 'management', 'settings', ['workflow']),
'isCurrent' => $router->getRequestedPage($request) === 'management' && in_array('workflow', (array) $router->getRequestedArgs($request)),
],
'distribution' => [
'name' => __('manager.distribution'),
'url' => $router->url($request, null, 'management', 'settings', ['distribution']),
'isCurrent' => $router->getRequestedPage($request) === 'management' && in_array('distribution', (array) $router->getRequestedArgs($request)),
],
'access' => [
'name' => __('navigation.access'),
'url' => $router->url($request, null, 'management', 'settings', ['access']),
'isCurrent' => $router->getRequestedPage($request) === 'management' && in_array('access', (array) $router->getRequestedArgs($request)),
]
]
]
];
];
}
}

if (count(array_intersect([Role::ROLE_ID_MANAGER, Role::ROLE_ID_SITE_ADMIN, Role::ROLE_ID_SUB_EDITOR], $userRoles))) {
Expand Down
2 changes: 1 addition & 1 deletion classes/userGroup/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
use Illuminate\Support\LazyCollection;
use PKP\core\Core;
use PKP\core\EntityDAO;
use PKP\core\PKPApplication;
use PKP\core\traits\EntityWithParent;
use PKP\services\PKPSchemaService;

Expand Down Expand Up @@ -57,6 +56,7 @@ class DAO extends EntityDAO
'showTitle' => 'show_title',
'permitSelfRegistration' => 'permit_self_registration',
'permitMetadataEdit' => 'permit_metadata_edit',
'permitSettings' => 'permit_settings',
'masthead' => 'masthead',
];

Expand Down
4 changes: 3 additions & 1 deletion classes/userGroup/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,6 @@ public function getFirstSubmitAsAuthorUserGroup(int $contextId): ?UserGroup
/**
* Load the XML file and move the settings to the DB
*
* @param int $contextId
* @param string $filename
*
* @return bool true === success
Expand All @@ -524,11 +523,13 @@ public function installSettings(?int $contextId, $filename)
$abbrevKey = $setting->getAttribute('abbrev');
$permitSelfRegistration = $setting->getAttribute('permitSelfRegistration');
$permitMetadataEdit = $setting->getAttribute('permitMetadataEdit');
$permitSettings = $setting->getAttribute('permitSettings');
$masthead = $setting->getAttribute('masthead');

// If has manager role then permitMetadataEdit can't be overridden
if (in_array($roleId, [Role::ROLE_ID_MANAGER])) {
$permitMetadataEdit = $setting->getAttribute('permitMetadataEdit');
$permitSettings = $setting->getAttribute('permitSettings');
}

$defaultStages = explode(',', (string) $setting->getAttribute('stages'));
Expand All @@ -539,6 +540,7 @@ public function installSettings(?int $contextId, $filename)
$userGroup->setContextId($contextId);
$userGroup->setPermitSelfRegistration($permitSelfRegistration ?? false);
$userGroup->setPermitMetadataEdit($permitMetadataEdit ?? false);
$userGroup->setPermitSettings($permitSettings ?? false);
$userGroup->setDefault(true);
$userGroup->setShowTitle(true);
$userGroup->setMasthead($masthead ?? false);
Expand Down
20 changes: 18 additions & 2 deletions classes/userGroup/UserGroup.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

namespace PKP\userGroup;

use PKP\core\PKPApplication;

class UserGroup extends \PKP\core\DataObject
{
/**
Expand Down Expand Up @@ -243,6 +241,24 @@ public function setPermitMetadataEdit(bool $permitMetadataEdit)
$this->setData('permitMetadataEdit', $permitMetadataEdit);
}

/**
* Getter for permitSettings attribute.
*
* @return bool
*/
public function getPermitSettings()
{
return $this->getData('permitSettings');
}

/**
* Setter for permitSettings attribute.
*/
public function setPermitSettings(bool $permitSettings)
{
$this->setData('permitSettings', $permitSettings);
}

/**
* Get the masthead flag
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
use PKP\linkAction\request\AjaxModal;
use PKP\navigationMenu\NavigationMenuItemDAO;
use PKP\notification\Notification;
use PKP\security\authorization\CanAccessSettingsPolicy;
use PKP\security\authorization\PolicySet;
use PKP\security\authorization\RoleBasedHandlerOperationPolicy;
use PKP\security\Role;
Expand Down Expand Up @@ -69,6 +70,7 @@ public function authorize($request, &$args, $roleAssignments)
$rolePolicy->addPolicy(new RoleBasedHandlerOperationPolicy($request, $role, $operations));
}
$this->addPolicy($rolePolicy);
$this->addPolicy(new CanAccessSettingsPolicy());

$navigationMenuItemId = $request->getUserVar('navigationMenuItemId');
if ($navigationMenuItemId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use PKP\linkAction\request\AjaxModal;
use PKP\navigationMenu\NavigationMenuDAO;
use PKP\notification\Notification;
use PKP\security\authorization\CanAccessSettingsPolicy;
use PKP\security\authorization\PolicySet;
use PKP\security\authorization\RoleBasedHandlerOperationPolicy;
use PKP\security\Role;
Expand Down Expand Up @@ -68,7 +69,7 @@ public function authorize($request, &$args, $roleAssignments)
$rolePolicy->addPolicy(new RoleBasedHandlerOperationPolicy($request, $role, $operations));
}
$this->addPolicy($rolePolicy);

$this->addPolicy(new CanAccessSettingsPolicy());

$navigationMenuId = $request->getUserVar('navigationMenuId');
if ($navigationMenuId) {
Expand Down
2 changes: 2 additions & 0 deletions controllers/grid/plugins/PluginGalleryGridHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
use PKP\plugins\PluginGalleryDAO;
use PKP\plugins\PluginHelper;
use PKP\plugins\PluginRegistry;
use PKP\security\authorization\CanAccessSettingsPolicy;
use PKP\security\authorization\PolicySet;
use PKP\security\authorization\RoleBasedHandlerOperationPolicy;
use PKP\security\Role;
Expand Down Expand Up @@ -130,6 +131,7 @@ public function authorize($request, &$args, $roleAssignments)
$rolePolicy->addPolicy(new RoleBasedHandlerOperationPolicy($request, $role, $operations));
}
$this->addPolicy($rolePolicy);
$this->addPolicy(new CanAccessSettingsPolicy());

return parent::authorize($request, $args, $roleAssignments);
}
Expand Down
Loading