Skip to content

Commit

Permalink
New feature #12690: Allow super-admin set by other user's than 1 (#810)
Browse files Browse the repository at this point in the history 
Dev: this add [supermadmin][create] by DB
Dev: add a confug var (array) 'forcedsuperadmin'
  • Loading branch information
@Shnoulle
Shnoulle committed Sep 22, 2017
1 parent 3d79581 commit 01a1b9d
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 106 deletions.
13 changes: 5 additions & 8 deletions application/config/config-defaults.php
View file
Expand Up @@ -593,16 +593,13 @@
$config['proxy_host_name'] = '';
$config['proxy_host_port'] = 80;

/** Forced superadmin rights, users in this array can not have superadmin total right disable.
* Default use the user created at the installation.
* @var integer[]
*/
$config['forcedsuperadmin'] = [1];

This comment has been minimized.

Sign in to view

Copy link
@olleharstedt

olleharstedt Oct 7, 2017

Contributor

Short-hand array notation is NOT supported by all PHP versions! Please change to array(1)!

This comment has been minimized.

Sign in to view

Copy link
@olleharstedt

olleharstedt Oct 7, 2017

Contributor

OK, fixed in latest commit.

This comment has been minimized.

Sign in to view

Copy link
@Shnoulle

Shnoulle Oct 7, 2017

Author Collaborator

Oh right … only for develop : right ?

This comment has been minimized.

Sign in to view

Copy link
@olleharstedt

olleharstedt Oct 7, 2017

Contributor

Yeah. :)

This comment has been minimized.

Sign in to view

Copy link
@Shnoulle

Shnoulle Oct 7, 2017

Author Collaborator

Yes, but Manual are broken … see 23d6bd4#commitcomment-24826427

// === Advanced Setup
// The following parameters need information from config.php
// and thus are defined here (After reading your config.php file).
// This means that if you want to tweak these very advanced parameters
// you'll have to do this in this file and not in config.php
// In this case, don't forget to backup your config-defaults.php settings when upgrading LS
// and report them to the new config-defaults.php file (Do not simply overwrite the new
// config-defaults file with your old one

//The following url and dir locations do not need to be modified unless you have a non-standard
//LimeSurvey installation. Do not change unless you know what you are doing.

Expand Down
65 changes: 21 additions & 44 deletions application/controllers/admin/useraction.php
View file
Expand Up @@ -461,35 +461,26 @@ public function savepermissions()

$iUserID=(int)App()->request->getPost('uid');
// A user may not modify his own permissions
if (Yii::app()->session['loginID']==$iUserID) {
if (Yii::app()->session['loginID'] == $iUserID) {
Yii::app()->setFlashMessage(gT("You are not allowed to edit your own user permissions."),"error");
$this->getController()->redirect(array("admin/user/sa/index"));
}
// Can not update initial superadmin permissions (with findByAttributes : found the first user without parent)
$oInitialAdmin = User::model()->findByAttributes(array('parent_id' => 0));
if ($oInitialAdmin && $oInitialAdmin->uid == $iUserID) // it's the original superadmin !!!
{
Yii::app()->setFlashMessage(gT("Initial Superadmin permissions cannot be updated!"),'error');
// Can not update forced superadmin rights
if ( Permission::isForcedSuperAdmin($iUserID) ) {
Yii::app()->setFlashMessage(gT("This Superadmin permissions cannot be updated!"),'error');
$this->getController()->redirect(array("admin/user/sa/index"));
}
$aBaseUserPermissions = Permission::model()->getGlobalBasePermissions();

$aPermissions=array();
foreach ($aBaseUserPermissions as $sPermissionKey=>$aCRUDPermissions)
{
foreach ($aCRUDPermissions as $sCRUDKey=>$CRUDValue)
{
if (!in_array($sCRUDKey,array('create','read','update','delete','import','export'))) continue;

if ($CRUDValue)
{
if(isset($_POST["perm_{$sPermissionKey}_{$sCRUDKey}"])){
$aPermissions[$sPermissionKey][$sCRUDKey]=1;
}
else
{
$aPermissions[$sPermissionKey][$sCRUDKey]=0;
}
foreach ($aCRUDPermissions as $sCRUDKey=>$CRUDValue) {
if (!in_array($sCRUDKey,array('create','read','update','delete','import','export'))) {
continue;
}
if ($CRUDValue) {
$aPermissions[$sPermissionKey][$sCRUDKey] = intval(Yii::app()->getRequest()->getPost("perm_{$sPermissionKey}_{$sCRUDKey}"));
}
}
}
Expand All @@ -510,30 +501,20 @@ public function savepermissions()
public function setuserpermissions()
{
$iUserID = (int) Yii::app()->request->getPost('uid');

// Can not update initial superadmin permissions (with findByAttributes : found the first user without parent)
$oInitialAdmin = User::model()->findByAttributes(array('parent_id' => 0));

if ($oInitialAdmin && $oInitialAdmin->uid == $iUserID) // Trying to update the original superadmin !!!
{
Yii::app()->setFlashMessage(gT("Initial Superadmin permissions cannot be updated!"),'error');
$this->getController()->redirect(array("admin/user/sa/index"));
}

$aBaseUserPermissions = Permission::model()->getGlobalBasePermissions();
if ($iUserID)
{
//Never update 1st admin
if(Permission::model()->hasGlobalPermission('superadmin','read'))
if ($iUserID) {
//Only super admin (read) can update other user
if(Permission::model()->hasGlobalPermission('superadmin','read')) {
$oUser = User::model()->findByAttributes(array('uid' => $iUserID));
else
} else {
$oUser = User::model()->findByAttributes(array('uid' => $iUserID, 'parent_id' => Yii::app()->session['loginID']));
}
}

// Check permissions
$aBasePermissions=Permission::model()->getGlobalBasePermissions();
if (!Permission::model()->hasGlobalPermission('superadmin','read')) // if not superadmin filter the available permissions as no admin may give more permissions than he owns
{
if (!Permission::model()->hasGlobalPermission('superadmin','read')) {
// if not superadmin filter the available permissions as no admin may give more permissions than he owns
Yii::app()->session['flashmessage'] = gT("Note: You can only give limited permissions to other users because your own permissions are limited, too.");
$aFilteredPermissions=array();
foreach ($aBasePermissions as $PermissionName=>$aPermission)
Expand All @@ -543,24 +524,20 @@ public function setuserpermissions()
if ($sPermissionKey!='title' && $sPermissionKey!='img' && !Permission::model()->hasGlobalPermission($PermissionName, $sPermissionKey)) $sPermissionValue=false;
}
// Only show a row for that permission if there is at least one permission he may give to other users
if ($aPermission['create'] || $aPermission['read'] || $aPermission['update'] || $aPermission['delete'] || $aPermission['import'] || $aPermission['export'])
{
if ($aPermission['create'] || $aPermission['read'] || $aPermission['update'] || $aPermission['delete'] || $aPermission['import'] || $aPermission['export']) {
$aFilteredPermissions[$PermissionName]=$aPermission;
}
}
$aBasePermissions=$aFilteredPermissions;
}

if(isset($oUser))
{
if(isset($oUser)) {
if ( $oUser && (Permission::model()->hasGlobalPermission('superadmin','read') || Permission::model()->hasGlobalPermission('users','update') && Yii::app()->session['loginID'] != $iUserID) )
{
// Only the original superadmin (UID 1) may create superadmins
if (Yii::app()->session['loginID']!=1)
{
// Show superadmin right if create is set (review for delete too ?)
if (!Permission::model()->hasGlobalPermission('superadmin','create') ) {
unset($aBasePermissions['superadmin']);
}

$aData = array();
$aData['aBasePermissions'] = $aBasePermissions;
$aData['oUser'] = $oUser;
Expand Down
68 changes: 36 additions & 32 deletions application/models/Permission.php
View file
Expand Up @@ -221,7 +221,7 @@ public static function getGlobalBasePermissions()
);
uasort($aPermissions, array(__CLASS__,"comparePermissionTitle"));
$aPermissions['superadmin'] = array(
'create' => false,
'create' => true, // Currently : is set/unset tis Permission to other user's
'update' => false,
'delete' => false,
'import' => false,
Expand Down Expand Up @@ -261,8 +261,7 @@ public static function getGlobalBasePermissions()
'img' => 'usergroup'
);

foreach ($aPermissions as &$permission)
{
foreach ($aPermissions as &$permission) {
$permission = array_merge($defaults, $permission);
}
return $aPermissions;
Expand Down Expand Up @@ -343,7 +342,7 @@ public static function setPermissions($iUserID, $iEntityID, $sEntityName, $aPerm
}
$aBasePermissions=$aFilteredPermissions;
}
elseif (Permission::model()->hasGlobalPermission('superadmin','read') && Yii::app()->session['loginID']!=1)
elseif (!Permission::model()->hasGlobalPermission('superadmin','create'))
{
unset($aBasePermissions['superadmin']);
}
Expand Down Expand Up @@ -537,62 +536,58 @@ public function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD='re
App()->getPluginManager()->dispatchEvent($oEvent);
$pluginbPermission=$oEvent->get('bPermission');

if (isset($pluginbPermission))
{
return $pluginbPermission;
if (isset($pluginbPermission)) {
return $pluginbPermission;
}

/* Always return true for CConsoleApplication (before or after plugin ? All other seems better after plugin) */
// TODO: see above about entry script and superadmin
if(is_null($iUserID) && Yii::app() instanceof CConsoleApplication)
{
if(is_null($iUserID) && Yii::app() instanceof CConsoleApplication) {
return true;
}

/* Always return false for unknow sCRUD */
// TODO: should not be necessary
if (!in_array($sCRUD,array('create','read','update','delete','import','export')))
{
if (!in_array($sCRUD,array('create','read','update','delete','import','export'))) {
return false;
}
$sCRUD=$sCRUD.'_p';

/* Always return false for guests */
// TODO: should not be necessary
if(!$this->getUserId($iUserID))
{
if(!$this->getUserId($iUserID)) {
return false;
}
else
{
} else {
$iUserID=$this->getUserId($iUserID);
}

/* Always return true if you are the owner : this can be done in core plugin ? */
// TODO: give the rights to owner adding line in permissions table, so it will return true with the normal way
if ($iUserID==$this->getOwnerId($iEntityID, $sEntityName))
{
if ($iUserID==$this->getOwnerId($iEntityID, $sEntityName)) {
return true;
}

/* Check if superadmin and static it */
// TODO: give the rights to superadmin adding line in permissions table, so it will return true with the normal way
if (!isset($aPermissionStatic[0]['global'][$iUserID]['superadmin']['read_p']))
{
if (!isset($aPermissionStatic[0]['global'][$iUserID]['superadmin']['read_p'])) {
$aPermission = $this->findByAttributes(array("entity_id"=>0,'entity'=>'global', "uid"=> $iUserID, "permission"=>'superadmin'));
$bPermission = is_null($aPermission) ? array() : $aPermission->attributes;
if (!isset($bPermission['read_p']) || $bPermission['read_p']==0)
{
$bPermission=false;
}
else
{
$bPermission=true;
}
$aPermissionStatic[0]['global'][$iUserID]['superadmin']['read_p']= $bPermission;
$aPermissionStatic[0]['global'][$iUserID]['superadmin']= array_merge(
array(
'create_p'=>false,
'read_p'=>false,
'update_p'=>false,
'delete_p'=>false,
'import_p'=>false,
'export_p'=>false,
),
$bPermission
);
}
if ($aPermissionStatic[0]['global'][$iUserID]['superadmin']['read_p'])
{
/* If it's a superadmin Permission : get and return */
if($sPermission == 'superadmin') {
return self::isForcedSuperAdmin($iUserID) || $aPermissionStatic[0]['global'][$iUserID][$sPermission][$sCRUD];
}
if ( self::isForcedSuperAdmin($iUserID) || $aPermissionStatic[0]['global'][$iUserID]['superadmin']['read_p']) {
return true;
}

Expand All @@ -619,6 +614,15 @@ public function hasPermission($iEntityID, $sEntityName, $sPermission, $sCRUD='re
return $aPermissionStatic[$iEntityID][$sEntityName][$iUserID][$sPermission][$sCRUD];
}

/**
* Returns true if user is a forced superadmin (can not disable superadmin rights)
* @var int
* @return boolean
*/
public static function isForcedSuperAdmin($iUserID)
{
return in_array($iUserID,App()->getConfig('forcedsuperadmin'));
}
/**
* Returns true if a user has global permission for a certain action.
* @param string $sPermission string Name of the permission - see function getGlobalPermissions
Expand Down
47 changes: 25 additions & 22 deletions application/models/User.php
View file
Expand Up @@ -351,20 +351,20 @@ public function getButtons(){
$changeOwnershipUrl = Yii::app()->getController()->createUrl('admin/user/sa/setasadminchild');

$oUser = $this->getName($this->uid);
if($this->uid == Yii::app()->user->getId())
{
$editUser = "<button
data-toggle='tooltip'
title='".gT("Edit this user")."'
data-url='".$editUrl."'
data-uid='".$this->uid."'
data-user='".htmlspecialchars($oUser['full_name'])."'
data-action='modifyuser'
class='btn btn-default btn-xs action_usercontrol_button'>
<span class='fa fa-pencil text-success'></span>
</button>";
if ($this->parent_id != 0 && Permission::model()->hasGlobalPermission('users','delete') )
{
if($this->uid == Yii::app()->user->getId()) {
// Edit self
$editUser = "<button
data-toggle='tooltip'
title='".gT("Edit this user")."'
data-url='".$editUrl."'
data-uid='".$this->uid."'
data-user='".htmlspecialchars($oUser['full_name'])."'
data-action='modifyuser'
class='btn btn-default btn-xs action_usercontrol_button'>
<span class='fa fa-pencil text-success'></span>
</button>";
// Can delete himself except is forced superadmin
if (!Permission::isForcedSuperAdmin($this->uid) && Permission::model()->hasGlobalPermission('users','delete') ) {
$deleteUrl = Yii::app()->getController()->createUrl('admin/user/sa/deluser', array(
"action"=> "deluser"
));
Expand All @@ -388,30 +388,31 @@ class='btn btn-default btn-xs'>
} else {
if (Permission::model()->hasGlobalPermission('superadmin','read')
|| $this->uid == Yii::app()->session['loginID']
|| (Permission::model()->hasGlobalPermission('users','update')
&& $this->parent_id == Yii::app()->session['loginID']))
{
|| ( Permission::model()->hasGlobalPermission('users','update')
&& $this->parent_id == Yii::app()->session['loginID']
)
) {
$editUser = "<button data-toggle='tooltip' data-url='".$editUrl."' data-user='".htmlspecialchars($oUser['full_name'])."' data-uid='".$this->uid."' data-action='modifyuser' title='".gT("Edit this user")."' type='submit' class='btn btn-default btn-xs action_usercontrol_button'><span class='fa fa-pencil text-success'></span></button>";
}

if (((Permission::model()->hasGlobalPermission('superadmin','read') &&
$this->uid != Yii::app()->session['loginID'] ) ||
(Permission::model()->hasGlobalPermission('users','update') &&
$this->parent_id == Yii::app()->session['loginID'])) && $this->uid!=1)
$this->parent_id == Yii::app()->session['loginID'])) && !Permission::isForcedSuperAdmin($this->uid))
{
//'admin/user/sa/setuserpermissions'
$setPermissionsUser = "<button data-toggle='tooltip' data-user='".htmlspecialchars($oUser['full_name'])."' data-url='".$setPermissionsUrl."' data-uid='".$this->uid."' data-action='setuserpermissions' title='".gT("Set global permissions for this user")."' type='submit' class='btn btn-default btn-xs action_usercontrol_button'><span class='icon-security text-success'></span></button>";
}
if ((Permission::model()->hasGlobalPermission('superadmin','read')
|| Permission::model()->hasGlobalPermission('templates','read'))
&& $this->uid!=1)
&& !Permission::isForcedSuperAdmin($this->uid))
{
//'admin/user/sa/setusertemplates')
$setTemplatePermissionUser = "<button type='submit' data-user='".htmlspecialchars($oUser['full_name'])."' data-url='".$setTemplatePermissionsUrl."' data-uid='".$this->uid."' data-action='setusertemplates' data-toggle='tooltip' title='".gT("Set template permissions for this user")."' class='btn btn-default btn-xs action_usercontrol_button'><span class='icon-templatepermissions text-success'></span></button>";
}
if ((Permission::model()->hasGlobalPermission('superadmin','read')
|| (Permission::model()->hasGlobalPermission('users','delete')
&& $this->parent_id == Yii::app()->session['loginID'])) && $this->uid!=1)
&& $this->parent_id == Yii::app()->session['loginID'])) && !Permission::isForcedSuperAdmin($this->uid))
{
$deleteUrl = Yii::app()->getController()->createUrl('admin/user/sa/deluser', array(
"action"=> "deluser",
Expand All @@ -433,8 +434,10 @@ class='btn btn-default btn-xs '>
<span class='fa fa-trash text-danger'></span>
</button>";
}
if (Yii::app()->session['loginID'] == "1" && $this->parent_id !=1 ) {
//'admin/user/sa/setasadminchild'
if (Permission::isForcedSuperAdmin(Yii::app()->session['loginID'])
&& $this->parent_id != Yii::app()->session['loginID']
) {
//'admin/user/sa/setasadminchild'
$changeOwnership = "<button data-toggle='tooltip' data-url='".$changeOwnershipUrl."' data-user='".htmlspecialchars($oUser['full_name'])."' data-uid='".$this->uid."' data-action='setasadminchild' title='".gT("Take ownership")."' class='btn btn-default btn-xs action_usercontrol_button' type='submit'><span class='icon-takeownership text-success'></span></button>";
}
}
Expand Down

5 comments on commit 01a1b9d

@c-schmitz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch break saving global permissions completely. We will have to do another release.

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, i look at it and make a patch

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error in POST value … think sending 1, but send on i fix it in some minute …

@c-schmitz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patch done in 6b94cd9

@Shnoulle
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Please sign in to comment.