diff --git a/.github/workflows/matomo-tests.yml b/.github/workflows/matomo-tests.yml index 6332192..58f0ef0 100644 --- a/.github/workflows/matomo-tests.yml +++ b/.github/workflows/matomo-tests.yml @@ -1,7 +1,7 @@ # Action for running tests # This file has been automatically created. # To recreate it you can run this command -# ./console generate:test-action --plugin="OAuth2" --php-versions="8.1,matomo5_max_php" --schedule-cron="0 5 * * 6" +# ./console generate:test-action --plugin="OAuth2" --php-versions="8.1,matomo5_max_php" --dependent-plugins="matomo-org/plugin-CustomAlerts innocraft/plugin-ActivityLog" --schedule-cron="0 5 * * 6" name: Plugin OAuth2 Tests @@ -57,7 +57,7 @@ jobs: artifacts-pass: ${{ secrets.ARTIFACTS_PASS }} upload-artifacts: ${{ matrix.php == 'matomo5_max_php' && matrix.target == 'maximum_supported_matomo' }} artifacts-protected: true - dependent-plugins: 'matomo-org/plugin-CustomAlerts' + dependent-plugins: 'matomo-org/plugin-CustomAlerts innocraft/plugin-ActivityLog' github-token: ${{ secrets.TESTS_ACCESS_TOKEN || secrets.GITHUB_TOKEN }} UI: runs-on: ubuntu-24.04 @@ -78,5 +78,5 @@ jobs: artifacts-pass: ${{ secrets.ARTIFACTS_PASS }} upload-artifacts: true artifacts-protected: true - dependent-plugins: 'matomo-org/plugin-CustomAlerts' + dependent-plugins: 'matomo-org/plugin-CustomAlerts innocraft/plugin-ActivityLog' github-token: ${{ secrets.TESTS_ACCESS_TOKEN || secrets.GITHUB_TOKEN }} diff --git a/Activity/AuthorizeClient.php b/Activity/AuthorizeClient.php index bb8a051..9ed2c10 100644 --- a/Activity/AuthorizeClient.php +++ b/Activity/AuthorizeClient.php @@ -9,6 +9,8 @@ namespace Piwik\Plugins\OAuth2\Activity; +use Piwik\Piwik; + class AuthorizeClient extends BaseActivity { protected $eventName = 'OAuth2.authorize.decision.end'; @@ -40,13 +42,31 @@ public function extractParams($eventData) public function getTranslatedDescription($activityData, $performingUser) { - $client = $activityData['client'] ?? []; + $clientLabel = $this->getClientLabel($activityData['client'] ?? []); $decision = $activityData['decision'] ?? ''; + $granted = implode(', ', array_values((array) ($activityData['scopes'] ?? []))); + $requested = implode(', ', array_values((array) ($activityData['requestedScopes'] ?? []))); + // decisions recorded before these scopes were stored can only name the client if ($decision === 'allowed') { - return sprintf('allowed OAuth 2.0 authorization request for client "%s"', $this->getClientLabel($client)); + if ($granted === '') { + return Piwik::translate('OAuth2_AuthorizeAllowedActivity', [$clientLabel]); + } + + if ($requested === '') { + return Piwik::translate('OAuth2_AuthorizeAllowedWithScopeActivity', [$clientLabel, $granted]); + } + + return Piwik::translate( + 'OAuth2_AuthorizeAllowedWithScopeAndRequestActivity', + [$clientLabel, $granted, $requested] + ); + } + + if ($requested === '') { + return Piwik::translate('OAuth2_AuthorizeDeniedActivity', [$clientLabel]); } - return sprintf('denied OAuth 2.0 authorization request for client "%s"', $this->getClientLabel($client)); + return Piwik::translate('OAuth2_AuthorizeDeniedWithRequestActivity', [$clientLabel, $requested]); } } diff --git a/Activity/CreateClient.php b/Activity/CreateClient.php index 169a48f..745ad06 100644 --- a/Activity/CreateClient.php +++ b/Activity/CreateClient.php @@ -9,6 +9,8 @@ namespace Piwik\Plugins\OAuth2\Activity; +use Piwik\Piwik; + class CreateClient extends BaseActivity { protected $eventName = 'API.OAuth2.createClient.end'; @@ -37,6 +39,6 @@ public function getTranslatedDescription($activityData, $performingUser) { $client = $activityData['client'] ?? []; - return sprintf('created OAuth 2.0 client "%s"', $this->getClientLabel($client)); + return Piwik::translate('OAuth2_CreateClientActivity', [$this->getClientLabel($client)]); } } diff --git a/Activity/DeleteClient.php b/Activity/DeleteClient.php index cc9770c..8013733 100644 --- a/Activity/DeleteClient.php +++ b/Activity/DeleteClient.php @@ -9,6 +9,8 @@ namespace Piwik\Plugins\OAuth2\Activity; +use Piwik\Piwik; + class DeleteClient extends BaseActivity { protected $eventName = 'API.OAuth2.deleteClient.end'; @@ -40,6 +42,6 @@ public function getTranslatedDescription($activityData, $performingUser) { $client = $activityData['client'] ?? []; - return sprintf('deleted OAuth 2.0 client "%s"', $this->getClientLabel($client)); + return Piwik::translate('OAuth2_DeleteClientActivity', [$this->getClientLabel($client)]); } } diff --git a/Activity/RotateSecret.php b/Activity/RotateSecret.php index a7a4929..ed7376b 100644 --- a/Activity/RotateSecret.php +++ b/Activity/RotateSecret.php @@ -9,6 +9,8 @@ namespace Piwik\Plugins\OAuth2\Activity; +use Piwik\Piwik; + class RotateSecret extends BaseActivity { protected $eventName = 'API.OAuth2.rotateSecret.end'; @@ -39,6 +41,6 @@ public function getTranslatedDescription($activityData, $performingUser) { $client = $activityData['client'] ?? []; - return sprintf('rotated secret for OAuth 2.0 client "%s"', $this->getClientLabel($client)); + return Piwik::translate('OAuth2_RotateSecretActivity', [$this->getClientLabel($client)]); } } diff --git a/Activity/SetClientActive.php b/Activity/SetClientActive.php index bde6ffd..98ebba7 100644 --- a/Activity/SetClientActive.php +++ b/Activity/SetClientActive.php @@ -9,6 +9,8 @@ namespace Piwik\Plugins\OAuth2\Activity; +use Piwik\Piwik; + class SetClientActive extends BaseActivity { protected $eventName = 'API.OAuth2.setClientActive.end'; @@ -39,9 +41,9 @@ public function getTranslatedDescription($activityData, $performingUser) $isActive = !empty($client['active']); if ($isActive) { - return sprintf('resumed OAuth 2.0 client "%s"', $this->getClientLabel($client)); + return Piwik::translate('OAuth2_ResumeClientActivity', [$this->getClientLabel($client)]); } - return sprintf('paused OAuth 2.0 client "%s"', $this->getClientLabel($client)); + return Piwik::translate('OAuth2_PauseClientActivity', [$this->getClientLabel($client)]); } } diff --git a/Activity/UpdateClient.php b/Activity/UpdateClient.php index 54a41a0..76921ed 100644 --- a/Activity/UpdateClient.php +++ b/Activity/UpdateClient.php @@ -9,6 +9,8 @@ namespace Piwik\Plugins\OAuth2\Activity; +use Piwik\Piwik; + class UpdateClient extends BaseActivity { protected $eventName = 'API.OAuth2.updateClient.end'; @@ -37,6 +39,6 @@ public function getTranslatedDescription($activityData, $performingUser) { $client = $activityData['client'] ?? []; - return sprintf('updated OAuth 2.0 client "%s"', $this->getClientLabel($client)); + return Piwik::translate('OAuth2_UpdateClientActivity', [$this->getClientLabel($client)]); } } diff --git a/CHANGELOG.md b/CHANGELOG.md index 746db89..8dd91ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ - The scope configured for a client is now applied as the maximum access level a user can grant it, as the setting has always been documented. A client configured with `matomo:admin` can therefore be granted `matomo:write` or `matomo:read` as well, which is what lets the user choose a scope on the consent screen. Clients can never be granted more than their configured scope. The client credentials grant is unchanged and still requires the exact configured scope, as no user is involved to choose a lower one. - Added code to harden the consent decision handling on the authorize endpoint - The `OAuth2.authorize.decision.end` event and the authorize activity keep reporting the granted scope in `scopes`, which is now empty when the request was denied, and add `requestedScopes` with everything the client asked for. +- The activity log entry for an authorization decision now names the granted scope and the scopes the client requested, instead of only naming the client. +- The activity log descriptions for OAuth 2.0 clients are now translatable, where they were previously always shown in English. 5.2.4 - 2026-07-27 - Added code to warn users if scope is downgraded diff --git a/lang/en.json b/lang/en.json index 3aeec9c..78fb360 100644 --- a/lang/en.json +++ b/lang/en.json @@ -2,6 +2,17 @@ "OAuth2": { "PlatformMenu": "OAuth 2.0", "AuthorizeTitle": "Authorise %s", + "AuthorizeAllowedActivity": "allowed OAuth 2.0 authorization request for client \"%1$s\"", + "AuthorizeAllowedWithScopeActivity": "allowed OAuth 2.0 authorization request for client \"%1$s\" with scope %2$s", + "AuthorizeAllowedWithScopeAndRequestActivity": "allowed OAuth 2.0 authorization request for client \"%1$s\" with scope %2$s, requested %3$s", + "AuthorizeDeniedActivity": "denied OAuth 2.0 authorization request for client \"%1$s\"", + "AuthorizeDeniedWithRequestActivity": "denied OAuth 2.0 authorization request for client \"%1$s\", requested %2$s", + "CreateClientActivity": "created OAuth 2.0 client \"%1$s\"", + "DeleteClientActivity": "deleted OAuth 2.0 client \"%1$s\"", + "PauseClientActivity": "paused OAuth 2.0 client \"%1$s\"", + "ResumeClientActivity": "resumed OAuth 2.0 client \"%1$s\"", + "RotateSecretActivity": "rotated secret for OAuth 2.0 client \"%1$s\"", + "UpdateClientActivity": "updated OAuth 2.0 client \"%1$s\"", "AuthorizeIntro": "%s is requesting access to your Matomo account.", "AuthorizeTextTitle": "is requesting access to your Matomo account", "AuthorizeHelpText": "You can revoke access at any time from your Matomo account settings.", diff --git a/tests/Integration/ActivityDescriptionsTest.php b/tests/Integration/ActivityDescriptionsTest.php new file mode 100644 index 0000000..166cc56 --- /dev/null +++ b/tests/Integration/ActivityDescriptionsTest.php @@ -0,0 +1,160 @@ +addDirectory(PIWIK_INCLUDE_PATH . '/plugins/OAuth2/lang'); + + $this->activity = new AuthorizeClient(); + } + + public function test_getTranslatedDescription_namesTheGrantedAndTheRequestedScopes() + { + $description = $this->activity->getTranslatedDescription($this->activityData([ + 'scopes' => ['matomo:write'], + 'requestedScopes' => ['matomo:read', 'matomo:write', 'matomo:admin'], + 'decision' => 'allowed', + ]), 'superUserLogin'); + + $this->assertSame( + 'allowed OAuth 2.0 authorization request for client "Claude Code Demo (c0dec0dec0dec0dec0dec0dec0dec0de)" with scope matomo:write' + . ', requested matomo:read, matomo:write, matomo:admin', + $description + ); + } + + public function test_getTranslatedDescription_namesOnlyTheRequestedScopesWhenDenied() + { + $description = $this->activity->getTranslatedDescription($this->activityData([ + 'scopes' => [], + 'requestedScopes' => ['matomo:read', 'matomo:write'], + 'decision' => 'denied', + ]), 'superUserLogin'); + + $this->assertSame( + 'denied OAuth 2.0 authorization request for client "Claude Code Demo (c0dec0dec0dec0dec0dec0dec0dec0de)"' + . ', requested matomo:read, matomo:write', + $description + ); + } + + public function test_getTranslatedDescription_describesDecisionsRecordedBeforeScopesWereStored() + { + // rows written by earlier versions carry neither the granted nor the requested scopes + $description = $this->activity->getTranslatedDescription($this->activityData([ + 'decision' => 'allowed', + ]), 'superUserLogin'); + + $this->assertSame( + 'allowed OAuth 2.0 authorization request for client "Claude Code Demo (c0dec0dec0dec0dec0dec0dec0dec0de)"', + $description + ); + } + + public function test_getTranslatedDescription_namesOnlyTheGrantedScopeWhenRequestedScopesWereNotStored() + { + $description = $this->activity->getTranslatedDescription($this->activityData([ + 'scopes' => ['matomo:write'], + 'decision' => 'allowed', + ]), 'superUserLogin'); + + $this->assertSame( + 'allowed OAuth 2.0 authorization request for client "Claude Code Demo (c0dec0dec0dec0dec0dec0dec0dec0de)"' + . ' with scope matomo:write', + $description + ); + } + + public function test_getTranslatedDescription_describesDenialsRecordedBeforeScopesWereStored() + { + $description = $this->activity->getTranslatedDescription($this->activityData([ + 'decision' => 'denied', + ]), 'superUserLogin'); + + $this->assertSame( + 'denied OAuth 2.0 authorization request for client "Claude Code Demo (c0dec0dec0dec0dec0dec0dec0dec0de)"', + $description + ); + } + + /** + * @dataProvider getClientActivityDescriptions + */ + public function test_getTranslatedDescription_describesTheClientActivities( + string $activityClass, + array $activityData, + string $expected + ) { + $activity = new $activityClass(); + + $description = $activity->getTranslatedDescription($this->activityData($activityData), 'superUserLogin'); + + $this->assertSame($expected, $description); + } + + public function getClientActivityDescriptions(): array + { + $label = 'Claude Code Demo (c0dec0dec0dec0dec0dec0dec0dec0de)'; + + return [ + [CreateClient::class, [], 'created OAuth 2.0 client "' . $label . '"'], + [UpdateClient::class, [], 'updated OAuth 2.0 client "' . $label . '"'], + [DeleteClient::class, [], 'deleted OAuth 2.0 client "' . $label . '"'], + [RotateSecret::class, [], 'rotated secret for OAuth 2.0 client "' . $label . '"'], + [SetClientActive::class, ['client' => $this->client(true)], 'resumed OAuth 2.0 client "' . $label . '"'], + [SetClientActive::class, ['client' => $this->client(false)], 'paused OAuth 2.0 client "' . $label . '"'], + ]; + } + + private function client(bool $active = true): array + { + return [ + 'id' => 'c0dec0dec0dec0dec0dec0dec0dec0de', + 'name' => 'Claude Code Demo', + 'active' => $active, + ]; + } + + private function activityData(array $data): array + { + return array_merge([ + 'version' => 'v1', + 'client' => $this->client(), + 'userLogin' => 'superUserLogin', + ], $data); + } +} + +ActivityDescriptionsTest::$fixture = new OAuth2Fixture();