mirror/Arsse
1
1
Fork 0
mirror of https://code.mensbeam.com/MensBeam/Arsse.git synced 2024-12-22 21:22:40 +00:00
Code Issues Activity

Cover unknown/invalid user levels

Browse source
This commit is contained in:
J. King 2017-02-28 11:19:33 -05:00
parent 7ae3c978b3
commit 766f2b65a4
2 changed files with 42 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
lib/User.php
View file

@ -75,8 +75,8 @@ class User {
$affectedRights = $this->rightsGet($affectedUser); $affectedRights = $this->rightsGet($affectedUser);
// managers can only act on themselves (checked above) or regular users // managers can only act on themselves (checked above) or regular users
if(in_array($rights,[User\Driver::RIGHTS_GLOBAL_MANAGER,User\Driver::RIGHTS_DOMAIN_MANAGER]) && $affectedRights != User\Driver::RIGHTS_NONE) return false; if(in_array($rights,[User\Driver::RIGHTS_GLOBAL_MANAGER,User\Driver::RIGHTS_DOMAIN_MANAGER]) && $affectedRights != User\Driver::RIGHTS_NONE) return false;
// acting for users with rights greater than your own (or equal, for managers) is not allowed // domain admins canot act above themselves
if($affectedRights > $rights || ($rights != User\Driver::RIGHTS_DOMAIN_ADMIN && $affectedRights==$rights)) return false; if(!in_array($affectedRights,[User\Driver::RIGHTS_NONE,User\Driver::RIGHTS_DOMAIN_MANAGER,User\Driver::RIGHTS_DOMAIN_ADMIN])) return false;
return true; return true;
} }

42
tests/User/TestAuthorization.php
View file

@ -17,6 +17,18 @@ class TestAuthorization extends \PHPUnit\Framework\TestCase {
'gman@example.org' => User\Driver::RIGHTS_GLOBAL_MANAGER, 'gman@example.org' => User\Driver::RIGHTS_GLOBAL_MANAGER,
'gadm@example.com' => User\Driver::RIGHTS_GLOBAL_ADMIN, 'gadm@example.com' => User\Driver::RIGHTS_GLOBAL_ADMIN,
'gadm@example.org' => User\Driver::RIGHTS_GLOBAL_ADMIN, 'gadm@example.org' => User\Driver::RIGHTS_GLOBAL_ADMIN,
// invalid rights levels
'bad1@example.com' => User\Driver::RIGHTS_NONE+1,
'bad1@example.org' => User\Driver::RIGHTS_NONE+1,
'bad2@example.com' => User\Driver::RIGHTS_DOMAIN_MANAGER+1,
'bad2@example.org' => User\Driver::RIGHTS_DOMAIN_MANAGER+1,
'bad3@example.com' => User\Driver::RIGHTS_DOMAIN_ADMIN+1,
'bad3@example.org' => User\Driver::RIGHTS_DOMAIN_ADMIN+1,
'bad4@example.com' => User\Driver::RIGHTS_GLOBAL_MANAGER+1,
'bad4@example.org' => User\Driver::RIGHTS_GLOBAL_MANAGER+1,
'bad5@example.com' => User\Driver::RIGHTS_GLOBAL_ADMIN+1,
'bad5@example.org' => User\Driver::RIGHTS_GLOBAL_ADMIN+1,
]; ];
const LEVELS = [ const LEVELS = [
User\Driver::RIGHTS_NONE, User\Driver::RIGHTS_NONE,
@ -127,6 +139,7 @@ class TestAuthorization extends \PHPUnit\Framework\TestCase {
if($actorRights != User\Driver::RIGHTS_DOMAIN_ADMIN) continue; if($actorRights != User\Driver::RIGHTS_DOMAIN_ADMIN) continue;
$actorDomain = substr($actor,strrpos($actor,"@")+1); $actorDomain = substr($actor,strrpos($actor,"@")+1);
$this->data->user->auth($actor, ""); $this->data->user->auth($actor, "");
$allowed = [User\Driver::RIGHTS_NONE,User\Driver::RIGHTS_DOMAIN_MANAGER,User\Driver::RIGHTS_DOMAIN_ADMIN];
foreach(self::USERS as $affected => $affectedRights) { foreach(self::USERS as $affected => $affectedRights) {
$affectedDomain = substr($affected,strrpos($affected,"@")+1); $affectedDomain = substr($affected,strrpos($affected,"@")+1);
// domain admins should be able to check any user on the same domain // domain admins should be able to check any user on the same domain
@ -136,14 +149,14 @@ class TestAuthorization extends \PHPUnit\Framework\TestCase {
$this->assertFalse($this->data->user->authorize($affected, "userExists"), "User $actor acted improperly for $affected, but the action was allowed."); $this->assertFalse($this->data->user->authorize($affected, "userExists"), "User $actor acted improperly for $affected, but the action was allowed.");
} }
// they should be able to act for any user on the same domain who is not a global manager or admin // they should be able to act for any user on the same domain who is not a global manager or admin
if($actorDomain==$affectedDomain && $affectedRights <= User\Driver::RIGHTS_DOMAIN_ADMIN) { if($actorDomain==$affectedDomain && in_array($affectedRights, $allowed)) {
$this->assertTrue($this->data->user->authorize($affected, "userRemove"), "User $actor acted properly for $affected, but the action was denied."); $this->assertTrue($this->data->user->authorize($affected, "userRemove"), "User $actor acted properly for $affected, but the action was denied.");
} else { } else {
$this->assertFalse($this->data->user->authorize($affected, "userRemove"), "User $actor acted improperly for $affected, but the action was allowed."); $this->assertFalse($this->data->user->authorize($affected, "userRemove"), "User $actor acted improperly for $affected, but the action was allowed.");
} }
// they should be able to set rights for any user on their domain who is not a global manager or admin, up to domain admin level // they should be able to set rights for any user on their domain who is not a global manager or admin, up to domain admin level
foreach(self::LEVELS as $level) { foreach(self::LEVELS as $level) {
if($actorDomain==$affectedDomain && $affectedRights <= User\Driver::RIGHTS_DOMAIN_ADMIN && $level <= User\Driver::RIGHTS_DOMAIN_ADMIN) { if($actorDomain==$affectedDomain && in_array($affectedRights, $allowed) && in_array($level, $allowed)) {
$this->assertTrue($this->data->user->authorize($affected, "userRightsSet", $level), "User $actor acted properly for $affected settings rights level $level, but the action was denied."); $this->assertTrue($this->data->user->authorize($affected, "userRightsSet", $level), "User $actor acted properly for $affected settings rights level $level, but the action was denied.");
} else { } else {
$this->assertFalse($this->data->user->authorize($affected, "userRightsSet", $level), "User $actor acted improperly for $affected settings rights level $level, but the action was allowed."); $this->assertFalse($this->data->user->authorize($affected, "userRightsSet", $level), "User $actor acted improperly for $affected settings rights level $level, but the action was allowed.");
@ -209,4 +222,29 @@ class TestAuthorization extends \PHPUnit\Framework\TestCase {
} }
} }
} }
function testInvalidLevelLogic() {
foreach(self::USERS as $actor => $rights) {
if(in_array($rights, self::LEVELS)) continue;
$this->data->user->auth($actor, "");
foreach(array_keys(self::USERS) as $affected) {
// users with unknown/invalid rights should be treated just like regular users and only be able to act for themselves
if($actor==$affected) {
$this->assertTrue($this->data->user->authorize($affected, "userExists"), "User $actor acted properly for $affected, but the action was denied.");
$this->assertTrue($this->data->user->authorize($affected, "userRemove"), "User $actor acted properly for $affected, but the action was denied.");
} else {
$this->assertFalse($this->data->user->authorize($affected, "userExists"), "User $actor acted improperly for $affected, but the action was allowed.");
$this->assertFalse($this->data->user->authorize($affected, "userRemove"), "User $actor acted improperly for $affected, but the action was allowed.");
}
// they should never be able to set rights
foreach(self::LEVELS as $level) {
$this->assertFalse($this->data->user->authorize($affected, "userRightsSet", $level), "User $actor acted improperly for $affected settings rights level $level, but the action was allowed.");
}
}
// they should not be able to list users
foreach(self::DOMAINS as $domain) {
$this->assertFalse($this->data->user->authorize($domain, "userList"), "User $actor improperly checked user list for domain '$domain', but the action was allowed.");
}
}
}
} }