From db1cdd22de1d8e4c135414e3ce732aaeca8d90f7 Mon Sep 17 00:00:00 2001 From: Martin Polanka Date: Wed, 21 Aug 2019 09:44:16 +0200 Subject: [PATCH] If effective role is present in token, check it in Authorizator alongside with user role and scopes --- app/V1Module/security/Authorizator.php | 5 +- app/V1Module/security/Identity.php | 12 +++ .../AuthorizatorWithEffectiveRole.phpt | 88 +++++++++++++++++++ ...s.phpt => AuthorizatorWithScopeRoles.phpt} | 12 +-- tests/Authorizator/MockIdentity.php | 15 +++- .../config/with_effective_role.neon | 22 +++++ ...ctive_roles.neon => with_scope_roles.neon} | 0 7 files changed, 142 insertions(+), 12 deletions(-) create mode 100644 tests/Authorizator/AuthorizatorWithEffectiveRole.phpt rename tests/Authorizator/{AuthorizatorWithEffectiveRoles.phpt => AuthorizatorWithScopeRoles.phpt} (88%) create mode 100644 tests/Authorizator/config/with_effective_role.neon rename tests/Authorizator/config/{with_effective_roles.neon => with_scope_roles.neon} (100%) diff --git a/app/V1Module/security/Authorizator.php b/app/V1Module/security/Authorizator.php index 2be74f0d2..2ff4054b6 100644 --- a/app/V1Module/security/Authorizator.php +++ b/app/V1Module/security/Authorizator.php @@ -20,7 +20,6 @@ abstract class Authorizator implements IAuthorizator { /** @var Roles */ protected $roles; - private $initialized = false; public function __construct(PolicyRegistry $policy, Roles $roles) { $this->policy = $policy; @@ -34,8 +33,10 @@ public function isAllowed(Identity $identity, string $resource, string $privileg $this->queriedContext = $context; $scopeRoles = $identity->getScopeRoles(); + $effectiveRole = $identity->getEffectiveRole(); return $this->checkPermissionsForRoleList($identity->getRoles(), $resource, $privilege) - && ($scopeRoles === [] || $this->checkPermissionsForRoleList($scopeRoles, $resource, $privilege)); + && ($scopeRoles === [] || $this->checkPermissionsForRoleList($scopeRoles, $resource, $privilege)) + && ($effectiveRole === null || $this->checkPermissions($effectiveRole, $resource, $privilege)); } protected function checkPermissionsForRoleList($roleList, $resource, $privilege): bool { diff --git a/app/V1Module/security/Identity.php b/app/V1Module/security/Identity.php index 75a6e9fa1..45f93fe49 100644 --- a/app/V1Module/security/Identity.php +++ b/app/V1Module/security/Identity.php @@ -52,6 +52,18 @@ function getScopeRoles() return array_map(function (string $role) { return "scope-" . $role; }, $this->token->getScopes()); } + /** + * Return effective role that user uses in current session. + * @return string|null + */ + public function getEffectiveRole(): ?string { + if (!$this->token) { + return null; + } + + return $this->token->getEffectiveRole(); + } + function getUserData() { return $this->user; diff --git a/tests/Authorizator/AuthorizatorWithEffectiveRole.phpt b/tests/Authorizator/AuthorizatorWithEffectiveRole.phpt new file mode 100644 index 000000000..9c856959c --- /dev/null +++ b/tests/Authorizator/AuthorizatorWithEffectiveRole.phpt @@ -0,0 +1,88 @@ +loader = new Loader(TEMP_DIR . '/security', __DIR__ . '/config/with_effective_role.neon', [ + 'resource1' => ITestResource1Permissions::class + ], Mockery::mock(UserStorage::class)); + } + + public function setUp() + { + $this->policies = new PolicyRegistry(); + $this->roles = $this->loader->loadRoles(); + $this->authorizator = $this->loader->loadAuthorizator($this->policies, $this->roles); + } + + public function testNoEffectiveRoles() + { + Assert::true($this->authorizator->isAllowed( + new MockIdentity([ 'normal_role' ]), + 'resource1', + 'action1', + [ + 'resource1' => new Resource1(), + ] + )); + } + + public function testRestrictedEffectiveRole() + { + Assert::false($this->authorizator->isAllowed( + new MockIdentity([ 'normal_role' ], [], 'effective_role_2'), + 'resource1', + 'action1', + [ + 'resource1' => new Resource1(), + ] + )); + } + + public function testAgreeingRoles() + { + Assert::true($this->authorizator->isAllowed( + new MockIdentity([ 'normal_role' ], [], 'effective_role_1'), + 'resource1', + 'action1', + [ + 'resource1' => new Resource1(), + ] + )); + } +} + +(new TestAuthorizatorWithEffectiveRole())->run(); diff --git a/tests/Authorizator/AuthorizatorWithEffectiveRoles.phpt b/tests/Authorizator/AuthorizatorWithScopeRoles.phpt similarity index 88% rename from tests/Authorizator/AuthorizatorWithEffectiveRoles.phpt rename to tests/Authorizator/AuthorizatorWithScopeRoles.phpt index 180dc9875..09d695267 100644 --- a/tests/Authorizator/AuthorizatorWithEffectiveRoles.phpt +++ b/tests/Authorizator/AuthorizatorWithScopeRoles.phpt @@ -17,7 +17,7 @@ interface ITestResource1Permissions { /** * @testCase */ -class TestAuthorizatorWithEffectiveRoles extends Tester\TestCase +class TestAuthorizatorWithScopeRoles extends Tester\TestCase { use MockeryTrait; @@ -35,7 +35,7 @@ class TestAuthorizatorWithEffectiveRoles extends Tester\TestCase public function __construct() { - $this->loader = new Loader(TEMP_DIR . '/security', __DIR__ . '/config/with_effective_roles.neon', [ + $this->loader = new Loader(TEMP_DIR . '/security', __DIR__ . '/config/with_scope_roles.neon', [ 'resource1' => ITestResource1Permissions::class ], Mockery::mock(\App\Security\UserStorage::class)); } @@ -47,7 +47,7 @@ class TestAuthorizatorWithEffectiveRoles extends Tester\TestCase $this->authorizator = $this->loader->loadAuthorizator($this->policies, $this->roles); } - public function testNoEffectiveRoles() + public function testNoScopeRoles() { Assert::true($this->authorizator->isAllowed( new MockIdentity([ 'normal_role' ]), @@ -59,7 +59,7 @@ class TestAuthorizatorWithEffectiveRoles extends Tester\TestCase )); } - public function testRestrictedEffectiveRole() + public function testRestrictedScopeRole() { Assert::false($this->authorizator->isAllowed( new MockIdentity([ 'normal_role' ], [ 'effective_role_2' ]), @@ -95,7 +95,7 @@ class TestAuthorizatorWithEffectiveRoles extends Tester\TestCase )); } - public function testBothEffectiveRoles() + public function testBothScopeRoles() { Assert::true($this->authorizator->isAllowed( new MockIdentity([ 'normal_role' ], [ 'effective_role_2', 'effective_role_1' ]), @@ -108,4 +108,4 @@ class TestAuthorizatorWithEffectiveRoles extends Tester\TestCase } } -(new TestAuthorizatorWithEffectiveRoles())->run(); +(new TestAuthorizatorWithScopeRoles())->run(); diff --git a/tests/Authorizator/MockIdentity.php b/tests/Authorizator/MockIdentity.php index d0ec163c4..fcf2a2df8 100644 --- a/tests/Authorizator/MockIdentity.php +++ b/tests/Authorizator/MockIdentity.php @@ -3,13 +3,15 @@ class MockIdentity extends App\Security\Identity { private $roles; - private $effectiveRoles; + private $scopeRoles; + private $effectiveRole; - public function __construct(array $roles, array $effectiveRoles = []) + public function __construct(array $roles, array $scopeRoles = [], string $effectiveRole = null) { parent::__construct(null, null); $this->roles = $roles; - $this->effectiveRoles = $effectiveRoles; + $this->scopeRoles = $scopeRoles; + $this->effectiveRole = $effectiveRole; } public function getRoles() @@ -19,6 +21,11 @@ public function getRoles() function getScopeRoles() { - return $this->effectiveRoles; + return $this->scopeRoles; + } + + function getEffectiveRole(): ?string + { + return $this->effectiveRole; } } diff --git a/tests/Authorizator/config/with_effective_role.neon b/tests/Authorizator/config/with_effective_role.neon new file mode 100644 index 000000000..6f7e08e03 --- /dev/null +++ b/tests/Authorizator/config/with_effective_role.neon @@ -0,0 +1,22 @@ +roles: + - name: normal_role + - name: effective_role_1 + - name: effective_role_2 + +permissions: + - allow: true + role: normal_role + resource: resource1 + actions: + - action1 + - action2 + - allow: true + role: effective_role_1 + resource: resource1 + actions: + - action1 + - allow: true + role: effective_role_2 + resource: resource1 + actions: + - action2 diff --git a/tests/Authorizator/config/with_effective_roles.neon b/tests/Authorizator/config/with_scope_roles.neon similarity index 100% rename from tests/Authorizator/config/with_effective_roles.neon rename to tests/Authorizator/config/with_scope_roles.neon