mirror of
https://github.com/Combodo/iTop.git
synced 2026-02-13 07:24:13 +01:00
N°8534 - Prevent Admin, SuperUser from loose of rights (#774)
* N°8534 - Prevent Admin & SuperUser from suicide Prevent creation/modification of Administrator, SuperUser, REST User, combined with a Profile denying access to the backoffice
This commit is contained in:
@@ -35,6 +35,7 @@ use DBObjectSearch;
|
||||
use DBObjectSet;
|
||||
use DeleteException;
|
||||
use MetaModel;
|
||||
use UserLocal;
|
||||
use UserRights;
|
||||
use utils;
|
||||
|
||||
@@ -80,6 +81,21 @@ class UserRightsTest extends ItopDataTestCase
|
||||
return $oUser;
|
||||
}
|
||||
|
||||
protected function GivenUserWithProfiles(string $sLogin, array $aProfileIds): DBObject
|
||||
{
|
||||
$oProfiles = new \ormLinkSet(\UserLocal::class, 'profile_list', \DBObjectSet::FromScratch(\URP_UserProfile::class));
|
||||
foreach ($aProfileIds as $iProfileId) {
|
||||
$oProfiles->AddItem(MetaModel::NewObject('URP_UserProfile', ['profileid' => $iProfileId, 'reason' => 'UNIT Tests']));
|
||||
}
|
||||
$oUser = MetaModel::NewObject('UserLocal', array(
|
||||
'login' => $sLogin,
|
||||
'password' => 'Password1!',
|
||||
'expiration' => UserLocal::EXPIRE_NEVER,
|
||||
'profile_list' => $oProfiles,
|
||||
));
|
||||
return $oUser;
|
||||
}
|
||||
|
||||
public function testIsLoggedIn()
|
||||
{
|
||||
$this->assertFalse(UserRights::IsLoggedIn());
|
||||
@@ -279,87 +295,79 @@ class UserRightsTest extends ItopDataTestCase
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider ProfileDenyingConsoleProvider
|
||||
* @doesNotPerformAssertions
|
||||
* @dataProvider UserCannotLoseConsoleAccessProvider
|
||||
*
|
||||
* @throws \CoreException
|
||||
* @throws \DictExceptionUnknownLanguage
|
||||
* @throws \OQLException
|
||||
*/
|
||||
public function testProfileDenyingConsole(int $iProfileId)
|
||||
public function testUserCannotLoseConsoleAccess(int $iProfileId)
|
||||
{
|
||||
$oUser = $this->CreateUniqueUserAndLogin('test1', $iProfileId);
|
||||
|
||||
try {
|
||||
$this->AddProfileToUser($oUser, 2);
|
||||
$this->fail('Profile should not be added');
|
||||
} catch (CoreCannotSaveObjectException $e) {
|
||||
}
|
||||
$this->expectException(CoreCannotSaveObjectException::class);
|
||||
$this->expectExceptionMessage('Profile "Portal user" cannot be added it will deny the access to backoffice');
|
||||
$this->AddProfileToUser($oUser, 2);
|
||||
}
|
||||
|
||||
public function ProfileDenyingConsoleProvider(): array
|
||||
public function UserCannotLoseConsoleAccessProvider(): array
|
||||
{
|
||||
return [
|
||||
'Administrator' => [1],
|
||||
'SuperUser' => [117],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider ProfileCannotModifySelfProvider
|
||||
* @doesNotPerformAssertions
|
||||
* @dataProvider UserCannotElevateTheirOwnRightsProvider
|
||||
*
|
||||
* @throws \CoreException
|
||||
* @throws \DictExceptionUnknownLanguage
|
||||
* @throws \OQLException
|
||||
*/
|
||||
public function testProfileCannotModifySelf(int $iProfileId)
|
||||
public function testUserCannotElevateTheirOwnRights(int $iCurrentProfileId, int $iElevatedProfileId)
|
||||
{
|
||||
$oUser = $this->CreateUniqueUserAndLogin('test1', $iProfileId);
|
||||
$oUser = $this->CreateUniqueUserAndLogin('test1', $iCurrentProfileId);
|
||||
|
||||
try {
|
||||
$this->AddProfileToUser($oUser, 1); // trying to become an admin
|
||||
$this->fail('User should not modify self');
|
||||
} catch (CoreException $e) {
|
||||
}
|
||||
$this->expectException(CoreCannotSaveObjectException::class);
|
||||
$this->AddProfileToUser($oUser, $iElevatedProfileId);
|
||||
}
|
||||
|
||||
public function ProfileCannotModifySelfProvider(): array
|
||||
public function UserCannotElevateTheirOwnRightsProvider(): array
|
||||
{
|
||||
return [
|
||||
'Configuration manager' => [3],
|
||||
'Configuration manager to SuperUser' => ['current'=> 3, 'added' => 117],
|
||||
'Configuration manager to Administrator' => ['current'=> 3, 'added' => 1],
|
||||
'SuperUser to Administrator' => ['current'=> 117, 'added' => 1],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider DeletingSelfUserProvider
|
||||
* @doesNotPerformAssertions
|
||||
* @dataProvider UserCannotDeleteOwnUserProvider
|
||||
*
|
||||
* @throws \CoreException
|
||||
* @throws \DictExceptionUnknownLanguage
|
||||
* @throws \OQLException
|
||||
*/
|
||||
public function testDeletingSelfUser(int $iProfileId)
|
||||
public function testUserCannotDeleteOwnUser(int $iProfileId)
|
||||
{
|
||||
$oUser = $this->CreateUniqueUserAndLogin('test1', $iProfileId);
|
||||
|
||||
try {
|
||||
$oUser->DBDelete();
|
||||
$this->fail('Current User cannot be deleted');
|
||||
} catch (DeleteException $e) {
|
||||
}
|
||||
$this->expectException(DeleteException::class);
|
||||
$oUser->DBDelete();
|
||||
}
|
||||
|
||||
public function DeletingSelfUserProvider(): array
|
||||
public function UserCannotDeleteOwnUserProvider(): array
|
||||
{
|
||||
return [
|
||||
'Administrator' => [1],
|
||||
'Configuration manager' => [3],
|
||||
'SuperUser' => [117],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider RemovingOwnContactProvider
|
||||
* @doesNotPerformAssertions
|
||||
* @dataProvider UserCannotRemoveOwnContactProvider
|
||||
*
|
||||
* @param int $iProfileId
|
||||
*
|
||||
@@ -367,68 +375,84 @@ class UserRightsTest extends ItopDataTestCase
|
||||
* @throws \DictExceptionUnknownLanguage
|
||||
* @throws \OQLException
|
||||
*/
|
||||
public function testRemovingOwnContact(int $iProfileId)
|
||||
public function testUserCannotRemoveOwnContact(int $iProfileId)
|
||||
{
|
||||
$oUser = $this->CreateUniqueUserAndLogin('test1', $iProfileId);
|
||||
|
||||
$oUser->Set('contactid', 0);
|
||||
|
||||
try {
|
||||
$oUser->DBWrite();
|
||||
$this->fail('Current User cannot remove his own contact');
|
||||
} catch (CoreCannotSaveObjectException $e) {
|
||||
}
|
||||
$this->expectException(CoreCannotSaveObjectException::class);
|
||||
$oUser->DBWrite();
|
||||
}
|
||||
|
||||
public function RemovingOwnContactProvider(): array
|
||||
public function UserCannotRemoveOwnContactProvider(): array
|
||||
{
|
||||
return [
|
||||
'Administrator' => [1],
|
||||
'Configuration manager' => [3],
|
||||
'SuperUser' => [117],
|
||||
];
|
||||
}
|
||||
|
||||
public function testAdminCannotRemoveOwnAdminProfile()
|
||||
{
|
||||
$oUser = $this->CreateUniqueUserAndLogin('admin111', 1); // Administrator
|
||||
// Keep only the SuperUser profile (remove Administrator profile)
|
||||
$this->AddProfileToUser($oUser, 117); // SuperUser profile for the test
|
||||
|
||||
$this->expectException(CoreCannotSaveObjectException::class);
|
||||
$this->expectExceptionMessage('You cannot remove your own Administrator profile. Ask another Administrator to do it for you');
|
||||
$this->RemoveProfileFromUser($oUser, 1); // Remove admin profile
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider UserCannotLoseUserEditionRightsProvider
|
||||
*/
|
||||
public function testUserCannotLoseUserEditionRights(int $iProfileId)
|
||||
{
|
||||
$oUser = $this->CreateUniqueUserAndLogin('configmgr111', $iProfileId); // SuperUser
|
||||
$this->AddProfileToUser($oUser, 3);
|
||||
|
||||
$this->expectException(CoreCannotSaveObjectException::class);
|
||||
$this->expectExceptionMessage('You cannot remove your own rights to edit Users');
|
||||
$this->RemoveProfileFromUser($oUser, $iProfileId);
|
||||
}
|
||||
|
||||
|
||||
public function UserCannotLoseUserEditionRightsProvider(): array
|
||||
{
|
||||
return [
|
||||
'Administrator' => [1],
|
||||
'SuperUser' => [117],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @doesNotPerformAssertions
|
||||
*
|
||||
* @throws \CoreException
|
||||
* @throws \DictExceptionUnknownLanguage
|
||||
* @throws \OQLException
|
||||
* @dataProvider PrivilegedUsersMustHaveBackofficeAccessProvider
|
||||
*/
|
||||
public function testUpgradingToAdmin()
|
||||
public function testPrivilegedUsersMustHaveBackofficeAccess(int $iProfileId)
|
||||
{
|
||||
$oUser = $this->CreateUniqueUserAndLogin('test1', 3);
|
||||
$oUser = $this->GivenUserWithProfiles('test1', [$iProfileId, 2]);
|
||||
|
||||
$this->expectException(CoreCannotSaveObjectException::class);
|
||||
$this->expectExceptionMessage('Profile "Portal user" cannot be given to privileged Users (Administrators, SuperUsers and REST Services Users)');
|
||||
$oUser->DBInsert();
|
||||
|
||||
try {
|
||||
$this->AddProfileToUser($oUser, 1);
|
||||
$this->fail('Should not be able to upgrade to Administrator');
|
||||
} catch (CoreCannotSaveObjectException $e) {
|
||||
} catch (CoreException $e) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @doesNotPerformAssertions
|
||||
*
|
||||
* @throws \CoreException
|
||||
* @throws \DictExceptionUnknownLanguage
|
||||
* @throws \OQLException
|
||||
*/
|
||||
public function testDenyingUserModification()
|
||||
public function PrivilegedUsersMustHaveBackofficeAccessProvider(): array
|
||||
{
|
||||
$oUser = $this->CreateUniqueUserAndLogin('test1', 1);
|
||||
$this->AddProfileToUser($oUser, 3);
|
||||
return [
|
||||
'killing another administrator' => [1],
|
||||
'killing superuser ' => [117],
|
||||
'killing Rest User' => [1024],
|
||||
|
||||
// Keep only the profile 3 (remove profile 1)
|
||||
$oSet = new \ormLinkSet(\UserLocal::class, 'profile_list', \DBObjectSet::FromScratch(\URP_UserProfile::class));
|
||||
$oSet->AddItem(MetaModel::NewObject('URP_UserProfile', ['profileid' => 3, 'reason' => 'UNIT Tests']));
|
||||
$oUser->Set('profile_list', $oSet);
|
||||
|
||||
try {
|
||||
$oUser->DBWrite();
|
||||
$this->fail('Should not be able to deny User modifications');
|
||||
} catch (CoreCannotSaveObjectException $e) {
|
||||
}
|
||||
];
|
||||
}
|
||||
public function testNonPrivilegedUsersCanBeDeniedFromBackoffice()
|
||||
{
|
||||
$oUser = $this->GivenUserWithProfiles('test1', [5, 2]);
|
||||
// No exception expected
|
||||
$oUser->DBInsert();
|
||||
$this->expectNotToPerformAssertions();
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -471,18 +495,18 @@ class UserRightsTest extends ItopDataTestCase
|
||||
$oSearch = new DBObjectSearch('URP_UserProfile');
|
||||
$oSearch->AddCondition('userid', $oUserAdmin->GetKey());
|
||||
$oSet = new DBObjectSet($oSearch);
|
||||
$this->assertEquals($iExpectedCount, $oSet->Count());
|
||||
$this->assertEquals($iExpectedCount, $oSet->Count(), 'Visibility on Link between User and Administrator Profiles should be controlled by hide_administrators setting');
|
||||
// Get the Profiles as well
|
||||
$oSearch = DBObjectSearch::FromOQL('SELECT URP_Profiles JOIN URP_UserProfile ON URP_UserProfile.profileid = URP_Profiles.id WHERE URP_UserProfile.userid='.$oUserAdmin->GetKey());
|
||||
$oSet = new DBObjectSet($oSearch);
|
||||
$this->assertEquals($iExpectedCount, $oSet->Count());
|
||||
$this->assertEquals($iExpectedCount, $oSet->Count(), 'Visibility on Administrator Profiles should be controlled by hide_administrators setting');
|
||||
}
|
||||
|
||||
public function NonAdminCannotListAdminProfilesProvider(): array
|
||||
{
|
||||
return [
|
||||
'with Admins visible' => [false, 1],
|
||||
'with Admins hidden' => [true, 0],
|
||||
'with Admins visible' => ['hide_administrators' => false, 'visible_objects' => 1],
|
||||
'with Admins hidden' => ['hide_administrators' => true, 'visible_objects' => 0],
|
||||
];
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user