Skip to content

Commit

Permalink
fixes #4314 Don't let "admin" users see all other users in Piwik
Browse files Browse the repository at this point in the history
  • Loading branch information
tsteur committed Dec 11, 2015
1 parent c40d440 commit 58f39ca
Show file tree
Hide file tree
Showing 52 changed files with 1,508 additions and 32 deletions.
11 changes: 9 additions & 2 deletions config/environment/test.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,16 @@
'Piwik\Access' => DI\decorate(function ($previous, ContainerInterface $c) {
$testUseMockAuth = $c->get('test.vars.testUseMockAuth');
if ($testUseMockAuth) {
$idSitesAdmin = $c->get('test.vars.idSitesAdminAccess');
$access = new FakeAccess();
FakeAccess::$superUser = true;
FakeAccess::$superUserLogin = 'superUserLogin';
if (!empty($idSitesAdmin)) {
FakeAccess::$superUser = false;
FakeAccess::$idSitesAdmin = $idSitesAdmin;
FakeAccess::$identity = 'adminUserLogin';
} else {
FakeAccess::$superUser = true;
FakeAccess::$superUserLogin = 'superUserLogin';
}
return $access;
} else {
return $previous;
Expand Down
20 changes: 15 additions & 5 deletions core/Access.php
Original file line number Diff line number Diff line change
Expand Up @@ -336,19 +336,29 @@ public function checkUserHasSuperUserAccess()
}

/**
* If the user doesn't have an ADMIN access for at least one website, throws an exception
* Returns `true` if the current user has admin access to at least one site.
*
* @throws \Piwik\NoAccessException
* @return bool
*/
public function checkUserHasSomeAdminAccess()
public function isUserHasSomeAdminAccess()
{
if ($this->hasSuperUserAccess()) {
return;
return true;
}

$idSitesAccessible = $this->getSitesIdWithAdminAccess();

if (count($idSitesAccessible) == 0) {
return count($idSitesAccessible) > 0;
}

/**
* If the user doesn't have an ADMIN access for at least one website, throws an exception
*
* @throws \Piwik\NoAccessException
*/
public function checkUserHasSomeAdminAccess()
{
if (!$this->isUserHasSomeAdminAccess()) {
throw new NoAccessException(Piwik::translate('General_ExceptionPrivilegeAtLeastOneWebsite', array('admin')));
}
}
Expand Down
7 changes: 1 addition & 6 deletions core/Piwik.php
Original file line number Diff line number Diff line change
Expand Up @@ -394,12 +394,7 @@ public static function checkUserHasAdminAccess($idSites)
*/
public static function isUserHasSomeAdminAccess()
{
try {
self::checkUserHasSomeAdminAccess();
return true;
} catch (Exception $e) {
return false;
}
return Access::getInstance()->isUserHasSomeAdminAccess();
}

/**
Expand Down
1 change: 1 addition & 0 deletions plugins/UsersManager/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
tests/System/processed/*xml
58 changes: 52 additions & 6 deletions plugins/UsersManager/API.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,20 @@ class API extends \Piwik\Plugin\API
*/
private $model;

/**
* @var UserAccessFilter
*/
private $userFilter;

const PREFERENCE_DEFAULT_REPORT = 'defaultReport';
const PREFERENCE_DEFAULT_REPORT_DATE = 'defaultReportDate';

private static $instance = null;

public function __construct(Model $model)
public function __construct(Model $model, UserAccessFilter $filter)
{
$this->model = $model;
$this->userFilter = $filter;
}

/**
Expand Down Expand Up @@ -201,6 +207,7 @@ public function getUsers($userLogins = '')
}

$users = $this->model->getUsers($logins);
$users = $this->userFilter->filterUsers($users);

// Non Super user can only access login & alias
if (!Piwik::hasUserSuperUserAccess()) {
Expand All @@ -221,7 +228,10 @@ public function getUsersLogin()
{
Piwik::checkUserHasSomeAdminAccess();

return $this->model->getUsersLogin();
$logins = $this->model->getUsersLogin();
$logins = $this->userFilter->filterLogins($logins);

return $logins;
}

/**
Expand All @@ -244,7 +254,10 @@ public function getUsersSitesFromAccess($access)

$this->checkAccessType($access);

return $this->model->getUsersSitesFromAccess($access);
$userSites = $this->model->getUsersSitesFromAccess($access);
$userSites = $this->userFilter->filterLoginIndexedArray($userSites);

return $userSites;
}

/**
Expand All @@ -266,7 +279,10 @@ public function getUsersAccessFromSite($idSite)
{
Piwik::checkUserHasAdminAccess($idSite);

return $this->model->getUsersAccessFromSite($idSite);
$usersAccess = $this->model->getUsersAccessFromSite($idSite);
$usersAccess = $this->userFilter->filterLoginIndexedArray($usersAccess);

return $usersAccess;
}

public function getUsersWithSiteAccess($idSite, $access)
Expand All @@ -280,6 +296,7 @@ public function getUsersWithSiteAccess($idSite, $access)
return array();
}

$logins = $this->userFilter->filterLogins($logins);
$logins = implode(',', $logins);

return $this->getUsers($logins);
Expand Down Expand Up @@ -336,7 +353,9 @@ public function getUser($userLogin)
Piwik::checkUserHasSuperUserAccessOrIsTheUser($userLogin);
$this->checkUserExists($userLogin);

return $this->model->getUser($userLogin);
$user = $this->model->getUser($userLogin);

return $this->userFilter->filterUser($user);
}

/**
Expand All @@ -351,7 +370,9 @@ public function getUserByEmail($userEmail)
Piwik::checkUserHasSuperUserAccess();
$this->checkUserEmailExists($userEmail);

return $this->model->getUserByEmail($userEmail);
$user = $this->model->getUserByEmail($userEmail);

return $this->userFilter->filterUser($user);
}

private function checkLogin($userLogin)
Expand Down Expand Up @@ -485,6 +506,9 @@ public function getUsersHavingSuperUserAccess()
unset($user['token_auth']);
}

// we do not filter these users by access and return them all since we need to print this information in the
// UI and they are allowed to see this.

return $users;
}

Expand Down Expand Up @@ -607,10 +631,32 @@ public function userExists($userLogin)
public function userEmailExists($userEmail)
{
Piwik::checkUserIsNotAnonymous();
Piwik::checkUserHasSomeViewAccess();

return $this->model->userEmailExists($userEmail);
}

/**
* Returns the first login name of an existing user that has the given email address. If no user can be found for
* this user an error will be returned.
*
* @param string $userEmail
* @return bool true if the user is known
*/
public function getUserLoginFromUserEmail($userEmail)
{
Piwik::checkUserIsNotAnonymous();
Piwik::checkUserHasSomeAdminAccess();

$this->checkUserEmailExists($userEmail);

$user = $this->model->getUserByEmail($userEmail);

// any user with some admin access is allowed to find any user by email, no need to filter by access here

return $user['login'];
}

/**
* Set an access level to a given user for a list of websites ID.
*
Expand Down
2 changes: 2 additions & 0 deletions plugins/UsersManager/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace Piwik\Plugins\UsersManager;

use Exception;
use Piwik\Access;
use Piwik\API\Request;
use Piwik\API\ResponseBuilder;
use Piwik\Common;
Expand Down Expand Up @@ -123,6 +124,7 @@ function index()
}
}

$view->hasOnlyAdminAccess = Piwik::isUserHasSomeAdminAccess() && !Piwik::hasUserSuperUserAccess();
$view->anonymousHasViewAccess = $this->hasAnonymousUserViewAccess($usersAccessByWebsite);
$view->idSiteSelected = $idSiteSelected;
$view->defaultReportSiteName = $defaultReportSiteName;
Expand Down
Loading

0 comments on commit 58f39ca

Please sign in to comment.