Skip to content

Commit

Permalink
Prevent concurrent changes on user permissions (#22959)
Browse files Browse the repository at this point in the history 
* Prevent concurrent changes on user permissions

* Esnrue not to work with cached permissions when checking them
  • Loading branch information
@sgiehl
sgiehl authored Jan 22, 2025
1 parent bb196a1 commit 815c37e
Showing 1 changed file with 112 additions and 84 deletions.
196 changes: 112 additions & 84 deletions plugins/UsersManager/API.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
use Piwik\Access\RolesProvider;
use Piwik\Auth\Password;
use Piwik\Common;
use Piwik\Concurrency\Lock;
use Piwik\Concurrency\LockBackend;
use Piwik\Config;
use Piwik\Container\StaticContainer;
use Piwik\Date;
Expand Down Expand Up @@ -825,33 +827,35 @@ public function inviteUser($userLogin, $email, $initialIdSite = null, $expiryInD
*/
public function setSuperUserAccess($userLogin, $hasSuperUserAccess, $passwordConfirmation = null)
{
Piwik::checkUserHasSuperUserAccess();
$this->checkUserIsNotAnonymous($userLogin);
UsersManager::dieIfUsersAdminIsDisabled();

$requirePasswordConfirmation = self::$SET_SUPERUSER_ACCESS_REQUIRE_PASSWORD_CONFIRMATION;
self::$SET_SUPERUSER_ACCESS_REQUIRE_PASSWORD_CONFIRMATION = true;

$isCliMode = Common::isPhpCliMode() && !(defined('PIWIK_TEST_MODE') && PIWIK_TEST_MODE);
if (
!$isCliMode
&& $requirePasswordConfirmation
) {
$this->confirmCurrentUserPassword($passwordConfirmation);
}
$this->checkUserExists($userLogin);
$this->executeConcurrencySafe($userLogin, function () use ($userLogin, $hasSuperUserAccess, $passwordConfirmation) {
Piwik::checkUserHasSuperUserAccess();
$this->checkUserIsNotAnonymous($userLogin);
UsersManager::dieIfUsersAdminIsDisabled();

$requirePasswordConfirmation = self::$SET_SUPERUSER_ACCESS_REQUIRE_PASSWORD_CONFIRMATION;
self::$SET_SUPERUSER_ACCESS_REQUIRE_PASSWORD_CONFIRMATION = true;

$isCliMode = Common::isPhpCliMode() && !(defined('PIWIK_TEST_MODE') && PIWIK_TEST_MODE);
if (
!$isCliMode
&& $requirePasswordConfirmation
) {
$this->confirmCurrentUserPassword($passwordConfirmation);
}
$this->checkUserExists($userLogin);

if (!$hasSuperUserAccess && $this->isUserTheOnlyUserHavingSuperUserAccess($userLogin)) {
$message = Piwik::translate("UsersManager_ExceptionRemoveSuperUserAccessOnlySuperUser", $userLogin)
. " "
. Piwik::translate("UsersManager_ExceptionYouMustGrantSuperUserAccessFirst");
throw new Exception($message);
}
if (!$hasSuperUserAccess && $this->isUserTheOnlyUserHavingSuperUserAccess($userLogin)) {
$message = Piwik::translate("UsersManager_ExceptionRemoveSuperUserAccessOnlySuperUser", $userLogin)
. " "
. Piwik::translate("UsersManager_ExceptionYouMustGrantSuperUserAccessFirst");
throw new Exception($message);
}

$this->model->deleteUserAccess($userLogin);
$this->model->setSuperUserAccess($userLogin, $hasSuperUserAccess);
$this->model->deleteUserAccess($userLogin);
$this->model->setSuperUserAccess($userLogin, $hasSuperUserAccess);

Cache::deleteTrackerCache();
Cache::deleteTrackerCache();
});
}

/**
Expand Down Expand Up @@ -1152,46 +1156,50 @@ public function setUserAccess($userLogin, $access, $idSites, $passwordConfirmati
}

$this->checkUserExist($userLogin);
$this->checkUsersHasNotSuperUserAccess($userLogin);

$this->model->deleteUserAccess($userLogin, $idSites);
$this->executeConcurrencySafe($userLogin, function () use ($userLogin, $access, $idSites, $roles, $capabilities) {
$idSites = $this->getIdSitesCheckAdminAccess($idSites);
$this->checkUsersHasNotSuperUserAccess($userLogin);

if ($access === 'noaccess') {
// if the access is noaccess then we don't save it as this is the default value
// when no access are specified
Piwik::postEvent('UsersManager.removeSiteAccess', [$userLogin, $idSites]);
} else {
$role = array_shift($roles);
$this->model->addUserAccess($userLogin, $role, $idSites);
}
$this->model->deleteUserAccess($userLogin, $idSites);

if (!empty($capabilities)) {
$this->addCapabilities($userLogin, $capabilities, $idSites);
}
if ($access === 'noaccess') {
// if the access is noaccess then we don't save it as this is the default value
// when no access are specified
Piwik::postEvent('UsersManager.removeSiteAccess', [$userLogin, $idSites]);
} else {
$role = array_shift($roles);
$this->model->addUserAccess($userLogin, $role, $idSites);
}

// Send notification to all super users if anonymous access is set for a site
if ($userLogin === 'anonymous' && $access === 'view') {
$container = StaticContainer::getContainer();
if (!empty($capabilities)) {
$this->addCapabilitesToUser($userLogin, $capabilities, $idSites);
}

$siteNames = [];
// Send notification to all super users if anonymous access is set for a site
if ($userLogin === 'anonymous' && $access === 'view') {
$container = StaticContainer::getContainer();

foreach ($idSites as $idSite) {
$siteNames[] = Site::getNameFor($idSite);
}
$siteNames = [];

$superUsers = Piwik::getAllSuperUserAccessEmailAddresses();
foreach ($superUsers as $login => $email) {
$email = $container->make(AnonymousAccessEnabledEmail::class, array(
'login' => $login,
'emailAddress' => $email,
'siteName' => implode(', ', $siteNames)
));
$email->safeSend();
foreach ($idSites as $idSite) {
$siteNames[] = Site::getNameFor($idSite);
}

$superUsers = Piwik::getAllSuperUserAccessEmailAddresses();
foreach ($superUsers as $login => $email) {
$email = $container->make(AnonymousAccessEnabledEmail::class, array(
'login' => $login,
'emailAddress' => $email,
'siteName' => implode(', ', $siteNames)
));
$email->safeSend();
}
}
}

// we reload the access list which doesn't yet take in consideration this new user access
$this->reloadPermissions();
// we reload the access list which doesn't yet take in consideration this new user access
$this->reloadPermissions();
});
}

/**
Expand All @@ -1208,28 +1216,40 @@ public function setUserAccess($userLogin, $access, $idSites, $passwordConfirmati
*/
public function addCapabilities($userLogin, $capabilities, $idSites)
{
$idSites = $this->getIdSitesCheckAdminAccess($idSites);
$this->executeConcurrencySafe($userLogin, function () use ($userLogin, $capabilities, $idSites) {
$idSites = $this->getIdSitesCheckAdminAccess($idSites);

if ($userLogin == 'anonymous') {
throw new Exception(Piwik::translate("UsersManager_ExceptionAnonymousNoCapabilities"));
}
if ($userLogin == 'anonymous') {
throw new Exception(Piwik::translate("UsersManager_ExceptionAnonymousNoCapabilities"));
}

$this->checkUserExists($userLogin);
$this->checkUsersHasNotSuperUserAccess([$userLogin]);
$this->checkUserExists($userLogin);
$this->checkUsersHasNotSuperUserAccess([$userLogin]);

if (!is_array($capabilities)) {
$capabilities = [$capabilities];
}
if (!is_array($capabilities)) {
$capabilities = [$capabilities];
}

foreach ($capabilities as $entry) {
$this->capabilityProvider->checkValidCapability($entry);
}
foreach ($capabilities as $entry) {
$this->capabilityProvider->checkValidCapability($entry);
}

$this->addCapabilitesToUser($userLogin, $capabilities, $idSites);

// we reload the access list which doesn't yet take in consideration this new user access
$this->reloadPermissions();
});
}

private function addCapabilitesToUser(string $userLogin, array $capabilities, $idSites)
{
[$sitesIdWithRole, $sitesIdWithCapability] = $this->getRolesAndCapabilitiesForLogin($userLogin);

foreach ($idSites as $idSite) {
if (!array_key_exists($idSite, $sitesIdWithRole)) {
throw new Exception(Piwik::translate('UsersManager_ExceptionNoCapabilitiesWithoutRole', [$userLogin, $idSite]));
throw new Exception(
Piwik::translate('UsersManager_ExceptionNoCapabilitiesWithoutRole', [$userLogin, $idSite])
);
}
}

Expand All @@ -1252,9 +1272,6 @@ public function addCapabilities($userLogin, $capabilities, $idSites)
}
}
}

// we reload the access list which doesn't yet take in consideration this new user access
$this->reloadPermissions();
}

private function getRolesAndCapabilitiesForLogin($userLogin)
Expand Down Expand Up @@ -1290,24 +1307,26 @@ private function getRolesAndCapabilitiesForLogin($userLogin)
*/
public function removeCapabilities($userLogin, $capabilities, $idSites)
{
$idSites = $this->getIdSitesCheckAdminAccess($idSites);
$this->executeConcurrencySafe($userLogin, function () use ($userLogin, $capabilities, $idSites) {
$idSites = $this->getIdSitesCheckAdminAccess($idSites);

$this->checkUserExists($userLogin);
$this->checkUserExists($userLogin);

if (!is_array($capabilities)) {
$capabilities = [$capabilities];
}
if (!is_array($capabilities)) {
$capabilities = [$capabilities];
}

foreach ($capabilities as $capability) {
$this->capabilityProvider->checkValidCapability($capability);
}
foreach ($capabilities as $capability) {
$this->capabilityProvider->checkValidCapability($capability);
}

foreach ($capabilities as $capability) {
$this->model->removeUserAccess($userLogin, $capability, $idSites);
}
foreach ($capabilities as $capability) {
$this->model->removeUserAccess($userLogin, $capability, $idSites);
}

// we reload the access list which doesn't yet take in consideration this removed capability
$this->reloadPermissions();
// we reload the access list which doesn't yet take in consideration this removed capability
$this->reloadPermissions();
});
}

private function reloadPermissions()
Expand All @@ -1318,6 +1337,9 @@ private function reloadPermissions()

private function getIdSitesCheckAdminAccess($idSites)
{
// reload access to ensure we're not working with cached entries that might have been changed in between
Access::getInstance()->reloadAccess();

if ($idSites === 'all') {
// in case idSites is all we grant access to all the websites on which the current connected user has an 'admin' access
$idSites = \Piwik\Plugins\SitesManager\API::getInstance()->getSitesIdWithAdminAccess();
Expand Down Expand Up @@ -1658,4 +1680,10 @@ public function generateInviteLink($userLogin, $expiryInDays = 7, $passwordConfi
'token' => $token,
]);
}

private function executeConcurrencySafe(string $userLogin, callable $callback = null)
{
$lock = new Lock(StaticContainer::get(LockBackend::class), 'UsersManager.changePermissions');
$lock->execute($userLogin, $callback);
}
}

0 comments on commit 815c37e

Please sign in to comment.