no logic in XenForo_ControllerHelper_Admin

[ragtek]

I'm not sure if it's because of the yesterdays alcohol, or because i worked too much, but this makes no sense for me:

XenForo_ControllerHelper_Admin extends XenForo_ControllerHelper_Abstract
{
	public function checkSuperAdminEdit(array $user)
	{
		if ($user['is_admin'] && !XenForo_Visitor::getInstance()->isSuperAdmin())
		{
			$superAdmins = preg_split(
				'#\s*,\s*#', XenForo_Application::get('config')->superAdmins,
				-1, PREG_SPLIT_NO_EMPTY
			);
			if (in_array($user['user_id'], $superAdmins))
			{
				throw $this->_controller->responseException(
					$this->_controller->responseError(new XenForo_Phrase('you_must_be_super_administrator_to_edit_user'))
				);
			}
		}
	}

XenForo_Visitor::getInstance()->isSuperAdmin()

	public function isSuperAdmin()
	{
		if ($this->_isSuperAdmin === null)
		{
			$superAdmins = preg_split(
				'#\s*,\s*#', XenForo_Application::get('config')->superAdmins,
				-1, PREG_SPLIT_NO_EMPTY
			);
			$this->_isSuperAdmin = in_array($this->_user['user_id'], $superAdmins);
		}

		return $this->_isSuperAdmin;
	}

So, you check the same thing twice.
It's unneccessery.
If the first check is false, the second will also be false.
BUT

for example:
user is not superadmin
then: visitor->getInstance()->isSuperAdmin()

$this->_isSuperAdmin = in_array($this->_user['user_id'], $superAdmins);

will return false and the helper will make the same check once again.

And it will find nothing once again!
The second strange thing is, it throws the exception ONLY if it founds the element!
Right?
if not sorryyyyyyyyyy

 

[Mike]

It's because of the alcohol.

That function throws an error if the passed user (not the visitor!) is a super admin and the visitor is not. Look at the error message thrown.

 

[ragtek]

nevermind
i've saw that the second check is NOT for the current user

oh mike was faster

 

[a legacy reborn]

I think we should just allow everyone into the ACP