Harden the check for UsersManager plugin, #AS-610

Auth/Oauth2Auth.php

@@ -20,18 +20,22 @@ class Oauth2Auth implements Auth
     private $login;
 
     /**
-     * The real subject the OAuth2 token was issued for. authenticate() only ever succeeds for
-     * this identity. setLogin() may mutate $login (the Piwik\Auth contract allows it, and core
-     * code such as PasswordVerifier does so), but the token's subject is fixed and must never
-     * change - otherwise an OAuth2-authenticated request could authenticate as an arbitrary
-     * account.
+     * The identity the OAuth2 token was issued for. authenticate() only succeeds for this
+     * identity; $login may be mutated via setLogin() but the subject is fixed.
      *
      * @var string
      */
     private $subject;
 
     private bool $isSuperUser;
 
+    /**
+     * Set once a password has been supplied to this adapter (see setPassword()).
+     *
+     * @var bool
+     */
+    private bool $passwordVerificationRequested = false;
+
     private string $tokenAuth;
 
     public array $scopes;
@@ -79,7 +83,10 @@ public function setPassword(
         #[\SensitiveParameter]
         $password
     ) {
-        // not used
+        // OAuth2 authenticates by bearer token, not by password.
+        if ($password !== null && $password !== '') {
+            $this->passwordVerificationRequested = true;
+        }
     }
 
     public function setPasswordHash(
@@ -91,9 +98,12 @@ public function setPasswordHash(
 
     public function authenticate()
     {
-        // Only authenticate the identity the token was actually issued for. If the login was
-        // mutated to a different account (e.g. via setLogin() from PasswordVerifier), refuse -
-        // this token proves nothing about any other user.
+        // A token does not carry a password; refuse if asked to authenticate by one.
+        if ($this->passwordVerificationRequested) {
+            return new AuthResult(AuthResult::FAILURE, $this->login, $this->tokenAuth);
+        }
+
+        // Only authenticate the identity the token was issued for.
         if ($this->login !== $this->subject) {
             return new AuthResult(AuthResult::FAILURE, $this->login, $this->tokenAuth);
         }

CHANGELOG.md

@@ -1,6 +1,9 @@
 ## Changelog
 
-5.2.2 - 2026-07-20
+5.2.3 - 2026-07-20
+- Added code to harden check and disallow update user action
+
+5.2.2 - 2026-07-16
 - Added code to harden check and disallow app specific token action
 
 5.2.1 - 2026-07-13

OAuth2.php

@@ -82,15 +82,23 @@ public function onApiRequestDispatch(&$finalParameters, $pluginName, $methodName
             return;
         }
 
-        // Oauth2Auth authenticates the bearer of an OAuth2 token; its password setters are
-        // no-ops and authenticate() always succeeds. UsersManager.createAppSpecificTokenAuth
-        // uses password confirmation as its ONLY authorization gate (no Access check) and
-        // accepts an arbitrary target userLogin, so under OAuth2 auth it would mint a full
-        // token_auth for any account. Block it here regardless of scope. This mirrors the
-        // dispatch guard LoginSaml installs for the same method. Fires for the top-level
-        // request and for API.getBulkRequest children alike.
-        if ($pluginName === 'UsersManager' && $methodName === 'createAppSpecificTokenAuth') {
-            throw new \Exception(Piwik::translate('OAuth2_CreateAppSpecificTokenAuthBlocked'));
+        // Not permitted for OAuth2-authenticated requests, regardless of scope.
+        $blockedUsersManagerMethods = [
+            'createAppSpecificTokenAuth' => 'OAuth2_CreateAppSpecificTokenAuthBlocked',
+            'updateUser'                 => 'OAuth2_UpdateUserBlocked',
+        ];
+        if ($pluginName === 'UsersManager' && isset($blockedUsersManagerMethods[$methodName])) {
+            throw new \Exception(Piwik::translate($blockedUsersManagerMethods[$methodName]));
+        }
+
+        // Not permitted for read-scope OAuth2 tokens; write and above may proceed.
+        $readScopeBlockedMethods = ['setUserPreference', 'initUserPreferenceWithDefault'];
+        if (
+            $pluginName === 'UsersManager'
+            && in_array($methodName, $readScopeBlockedMethods, true)
+            && !$this->scopeGrantsAtLeastWrite($auth->getPrimaryScope())
+        ) {
+            throw new \Exception(Piwik::translate('OAuth2_SetUserPreferenceBlocked'));
         }
 
         $access = Access::getInstance();
@@ -376,6 +384,16 @@ public static function getAuthorizationHeader(): ?string
         return null;
     }
 
+    private function scopeGrantsAtLeastWrite(?string $scope): bool
+    {
+        $scopeToLevel = ['matomo:read' => 1, 'matomo:write' => 2, 'matomo:admin' => 3, 'matomo:superuser' => 4];
+
+        // Unknown/empty scopes are treated as below write.
+        $level = $scopeToLevel[$scope] ?? 0;
+
+        return $level >= $scopeToLevel['matomo:write'];
+    }
+
     private function modifyAccessBasedOnScope(?array $idSitesAccess, ?string $scope): array
     {
         $levels = ['view' => 1, 'write' => 2, 'admin' => 3, 'superuser' => 4];

lang/en.json

@@ -105,6 +105,8 @@
     "InvalidValueException": "Invalid value, it should be greater than 0.",
     "InvalidNumericValueException": "Invalid value, it should be a numeric value, greater than 0.",
     "TokenEndpointException": "Token endpoint only accepts POST",
-    "CreateAppSpecificTokenAuthBlocked": "Creating an app-specific token is not allowed when authenticating with an OAuth2 access token."
+    "CreateAppSpecificTokenAuthBlocked": "Creating an app-specific token is not allowed when authenticating with an OAuth2 access token.",
+    "UpdateUserBlocked": "Updating a user's password or email is not allowed when authenticating with an OAuth2 access token.",
+    "SetUserPreferenceBlocked": "Setting a user preference is not allowed when authenticating with a read-only OAuth2 access token."
   }
 }

plugin.json

@@ -1,7 +1,7 @@
 {
   "name": "OAuth2",
   "description": "Provide secure access to the Matomo API using scoped permissions. No static credentials.",
-  "version": "5.2.2",
+  "version": "5.2.3",
   "theme": false,
   "require": {
     "php": ">=8.1.0",

tests/Integration/CreateAppSpecificTokenGuardTest.php

@@ -22,23 +22,9 @@
 use Piwik\Plugins\UsersManager\API as UsersManagerAPI;
 use Piwik\Plugins\UsersManager\Model as UserModel;
 use Piwik\Plugins\UsersManager\UsersManager;
+use Piwik\Settings\Storage\UserScopedSettingsAccessManager;
 use Piwik\Tests\Framework\Fixture;
 
-/**
- * Regression tests for the OAuth2 privilege-escalation guard on
- * UsersManager.createAppSpecificTokenAuth.
- *
- * Oauth2Auth authenticates the bearer of an OAuth2 token; its password setters are no-ops
- * and authenticate() always succeeds for whatever login is set on it. Because
- * PasswordVerifier::isPasswordCorrect() reuses the globally installed Piwik\Auth adapter,
- * an OAuth2-authenticated request could otherwise mint a full-access token_auth for ANY
- * account through createAppSpecificTokenAuth (whose only authorization gate is that password
- * confirmation). The plugin blocks that method under Oauth2Auth via its API.Request.dispatch
- * listener; these tests pin that behaviour down.
- *
- * @group OAuth2
- * @group Plugins
- */
 class CreateAppSpecificTokenGuardTest extends \Piwik\Tests\Framework\TestCase\IntegrationTestCase
 {
     public static $fixture = null;
@@ -56,7 +42,6 @@ public function setUp(): void
 
         $userModel = new UserModel();
 
-        // Low-privilege attacker, holder of the OAuth2 token.
         if (empty($userModel->getUser(self::ATTACKER_LOGIN))) {
             UsersManagerAPI::getInstance()->addUser(
                 self::ATTACKER_LOGIN,
@@ -67,8 +52,6 @@ public function setUp(): void
             );
         }
 
-        // Victim super user, deliberately created WITHOUT any app-specific token so that any
-        // token found for it can only have been minted by the request under test.
         if (empty($userModel->getUser(self::VICTIM_LOGIN))) {
             $hashedPassword = (new Password())->hash(UsersManager::getPasswordHash('VictimPassword123'));
             $userModel->addUser(
@@ -81,11 +64,6 @@ public function setUp(): void
         }
     }
 
-    /**
-     * The adapter must refuse to authenticate any identity other than the token's real subject,
-     * even when driven through the exact setter sequence PasswordVerifier performs. This closes
-     * the identity-switch that let an OAuth2 request authenticate as an arbitrary account.
-     */
     public function test_oauth2Auth_refusesToAuthenticateADifferentIdentity()
     {
         $auth = new Oauth2Auth(self::ATTACKER_LOGIN, false, 'token-id', 'client-id', ['matomo:read']);
@@ -101,23 +79,28 @@ public function test_oauth2Auth_refusesToAuthenticateADifferentIdentity()
         $this->assertSame(AuthResult::FAILURE, $result->getCode());
     }
 
-    /**
-     * For its own subject the adapter still succeeds without verifying the password (it
-     * authenticates the bearer of the token, not a password). This is why the dispatch guard on
-     * createAppSpecificTokenAuth remains necessary even after the identity check above.
-     */
-    public function test_oauth2Auth_authenticatesOwnSubjectRegardlessOfPassword()
+    public function test_oauth2Auth_authenticatesOwnSubject_whenNoPasswordSupplied()
     {
         $auth = new Oauth2Auth(self::ATTACKER_LOGIN, false, 'token-id', 'client-id', ['matomo:read']);
 
-        $auth->setPassword('definitely-wrong-password');
-
         $result = $auth->authenticate();
 
         $this->assertTrue($result->wasAuthenticationSuccessful());
         $this->assertSame(self::ATTACKER_LOGIN, $result->getIdentity());
     }
 
+    public function test_oauth2Auth_refusesPasswordVerification_forOwnSubject()
+    {
+        $auth = new Oauth2Auth(self::ATTACKER_LOGIN, false, 'token-id', 'client-id', ['matomo:read']);
+
+        $auth->setPassword('definitely-wrong-password');
+
+        $result = $auth->authenticate();
+
+        $this->assertFalse($result->wasAuthenticationSuccessful());
+        $this->assertSame(AuthResult::FAILURE, $result->getCode());
+    }
+
     public function test_guard_blocksCreateAppSpecificTokenAuth_whenAuthenticatedViaOauth2()
     {
         $this->installOauth2Context(self::ATTACKER_LOGIN, false, ['matomo:read']);
@@ -129,21 +112,97 @@ public function test_guard_blocksCreateAppSpecificTokenAuth_whenAuthenticatedVia
         (new OAuth2())->onApiRequestDispatch($parameters, 'UsersManager', 'createAppSpecificTokenAuth');
     }
 
+    public function test_guard_blocksSetUserPreference_whenAuthenticatedViaOauth2()
+    {
+        $this->installOauth2Context(self::ATTACKER_LOGIN, false, ['matomo:read']);
+
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage(Piwik::translate('OAuth2_SetUserPreferenceBlocked'));
+
+        $parameters = ['userLogin' => self::ATTACKER_LOGIN];
+        (new OAuth2())->onApiRequestDispatch($parameters, 'UsersManager', 'setUserPreference');
+    }
+
+    public function test_guard_blocksInitUserPreferenceWithDefault_whenAuthenticatedViaOauth2()
+    {
+        $this->installOauth2Context(self::ATTACKER_LOGIN, false, ['matomo:read']);
+
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage(Piwik::translate('OAuth2_SetUserPreferenceBlocked'));
+
+        $parameters = ['userLogin' => self::ATTACKER_LOGIN];
+        (new OAuth2())->onApiRequestDispatch($parameters, 'UsersManager', 'initUserPreferenceWithDefault');
+    }
+
+    public function test_guard_allowsSetUserPreference_whenWriteScope()
+    {
+        $this->installOauth2Context(self::ATTACKER_LOGIN, false, ['matomo:write']);
+
+        // A write-scope token may set its own preferences - the guard must not fire.
+        $parameters = ['userLogin' => self::ATTACKER_LOGIN];
+        (new OAuth2())->onApiRequestDispatch($parameters, 'UsersManager', 'setUserPreference');
+
+        $this->assertTrue(true);
+    }
+
     public function test_guard_allowsOtherUsersManagerMethods_whenAuthenticatedViaOauth2()
     {
         $this->installOauth2Context(self::ATTACKER_LOGIN, false, ['matomo:read']);
 
         $parameters = [];
-        // Must not throw for any method other than createAppSpecificTokenAuth.
         (new OAuth2())->onApiRequestDispatch($parameters, 'UsersManager', 'getUsers');
 
         $this->assertTrue(true);
     }
 
+    public function test_guard_allowsReadingOwnPreference_whenAuthenticatedViaOauth2()
+    {
+        $this->installOauth2Context(self::ATTACKER_LOGIN, false, ['matomo:read']);
+
+        $parameters = ['userLogin' => self::ATTACKER_LOGIN];
+        (new OAuth2())->onApiRequestDispatch($parameters, 'UsersManager', 'getUserPreference');
+
+        $this->assertTrue(true);
+    }
+
+    public function test_endToEnd_readScopeToken_cannotChangeOwnPreference()
+    {
+        if (!class_exists(UserScopedSettingsAccessManager::class)) {
+            $this->markTestSkipped('UserScopedSettingsAccessManager is not available in this Matomo version');
+        }
+
+        UsersManagerAPI::getInstance()->setUserPreference(
+            self::ATTACKER_LOGIN,
+            UsersManagerAPI::PREFERENCE_DEFAULT_REPORT,
+            'MultiSites'
+        );
+
+        $this->installOauth2Context(self::ATTACKER_LOGIN, false, ['matomo:read']);
+
+        $threw = false;
+        try {
+            Request::processRequest('UsersManager.setUserPreference', [
+                'userLogin'       => self::ATTACKER_LOGIN,
+                'preferenceName'  => UsersManagerAPI::PREFERENCE_DEFAULT_REPORT,
+                'preferenceValue' => 'Live',
+            ], []);
+        } catch (\Exception $e) {
+            $threw = true;
+            $this->assertStringContainsString(
+                Piwik::translate('OAuth2_SetUserPreferenceBlocked'),
+                $e->getMessage()
+            );
+        }
+
+        $this->assertTrue($threw, 'Expected the dispatch guard to reject the request');
+
+        $stored = StaticContainer::get(UserScopedSettingsAccessManager::class)
+            ->get('UsersManager', self::ATTACKER_LOGIN, UsersManagerAPI::PREFERENCE_DEFAULT_REPORT, false);
+        $this->assertSame('MultiSites', $stored);
+    }
+
     public function test_guard_doesNotBlock_whenNotAuthenticatedViaOauth2()
     {
-        // No OAuth2 token on the Access singleton: the guard must not interfere with the
-        // normal password-authenticated flow.
         $access = Access::getInstance();
         $tokenAuthProperty = new \ReflectionProperty(Access::class, 'token_auth');
         $tokenAuthProperty->setAccessible(true);
@@ -187,8 +246,6 @@ public function test_endToEnd_bulkRequestChild_cannotMintTokenForSuperUser()
             . '&passwordConfirmation=' . urlencode('this-is-not-the-victims-password')
             . '&description=pwned';
 
-        // getBulkRequest swallows per-child exceptions into the response, so we assert on the
-        // security property directly: no token_auth row must exist for the victim.
         try {
             Request::processRequest('API.getBulkRequest', [
                 'urls' => [$childUrl],
@@ -200,22 +257,30 @@ public function test_endToEnd_bulkRequestChild_cannotMintTokenForSuperUser()
         $this->assertNoTokenExistsFor(self::VICTIM_LOGIN);
     }
 
-    /**
-     * Positive control: the guard only fires under Oauth2Auth. Genuine password authentication
-     * with the correct password must still create a token for the caller's own account.
-     */
-    public function test_passwordAuth_withCorrectPassword_stillCreatesToken()
+    public function test_endToEnd_superUserScopeToken_cannotGrantSuperUserWithoutPassword()
     {
-        Access::getInstance()->setSuperUserAccess(true);
+        $this->installOauth2Context(self::VICTIM_LOGIN, true, ['matomo:superuser']);
 
-        $token = UsersManagerAPI::getInstance()->createAppSpecificTokenAuth(
-            Fixture::ADMIN_USER_LOGIN,
-            Fixture::ADMIN_USER_PASSWORD,
-            'legitimate app token'
-        );
+        $threw = false;
+        try {
+            Request::processRequest('UsersManager.setSuperUserAccess', [
+                'userLogin'            => self::ATTACKER_LOGIN,
+                'hasSuperUserAccess'   => 1,
+                'passwordConfirmation' => 'not-the-victims-password',
+            ], []);
+        } catch (\Exception $e) {
+            $threw = true;
+            $this->assertStringContainsString(
+                Piwik::translate('UsersManager_CurrentPasswordNotCorrect'),
+                $e->getMessage()
+            );
+        }
 
-        $this->assertNotEmpty($token);
-        $this->assertNotEmpty($this->hashedTokensFor(Fixture::ADMIN_USER_LOGIN));
+        $this->assertTrue($threw, 'Expected the password step-up to reject the request');
+        $this->assertFalse(
+            (new UserModel())->getUser(self::ATTACKER_LOGIN)['superuser_access'] == 1,
+            'Attacker must not have been granted super user access'
+        );
     }
 
     private function installOauth2Context(string $login, bool $isSuperUser, array $scopes): void