Split the UI to have list and add/edit as separate action, #PG-4995

.gitignore

@@ -1,3 +1,4 @@
 /tests/System/processed/
 /tests/Integration/Importers/processed/
-/tests/UI/processed-ui-screenshots/
+/tests/UI/processed-ui-screenshots/
+.codex

API.php

@@ -37,7 +37,27 @@ public function __construct(
     public function getClients(): array
     {
         Piwik::checkUserHasSuperUserAccess();
-        return $this->clientModel->all();
+        return array_map([$this, 'sanitizeClient'], $this->clientModel->all());
+    }
+
+    /**
+     * Returns one OAuth2 client configured in Matomo (super users only).
+     *
+     * @param string $clientId 32-character hexadecimal client identifier.
+     * @return array
+     */
+    public function getClient(string $clientId): array
+    {
+        Piwik::checkUserHasSuperUserAccess();
+
+        $clientId = $this->assertValidClientId($clientId);
+        $client = $this->clientModel->find($clientId);
+
+        if (empty($client)) {
+            throw new \InvalidArgumentException(Piwik::translate('OAuth2_ClientNotFound'));
+        }
+
+        return $this->sanitizeClient($client);
     }
 
     /**
@@ -76,42 +96,75 @@ public function createClient(string $name, array $grantTypes, string $scope, $re
     {
         Piwik::checkUserHasSuperUserAccess();
 
-        $type = $type === 'public' ? 'public' : 'confidential';
+        $data = $this->buildValidatedClientData(
+            $name,
+            $grantTypes,
+            $scope,
+            $redirectUris,
+            $description,
+            $type,
+            $active
+        );
 
-        $redirects = is_array($redirectUris) ? $redirectUris : preg_split('/[\r\n]+/', (string) $redirectUris);
-        if ($redirects === false) {
-            $redirects = [];
-        }
-        $redirects = array_values(array_filter(array_map('trim', $redirects), static function ($value) {
-            return $value !== '';
-        }));
+        $result = $this->clientManager->create([
+            'name' => $data['name'],
+            'description' => $data['description'],
+            'redirect_uris' => $data['redirect_uris'],
+            'grant_types' => $data['grant_types'],
+            'scopes' => $data['scopes'],
+            'type' => $data['type'],
+            'active' => $data['active'],
+        ], Piwik::getCurrentUserLogin());
 
-        $grantTypes = array_values(array_filter(array_map('trim', (array) $grantTypes), static function ($value) {
-            return $value !== '';
-        }));
-        $grantTypes = $this->validateGrantTypes($grantTypes);
+        $result['client'] = $this->sanitizeClient($result['client']);
 
-        $this->validateRedirectUris($redirects, $grantTypes);
+        return $result;
+    }
 
-        if ($type === 'public' && in_array('client_credentials', $grantTypes, true)) {
-            throw new \InvalidArgumentException(Piwik::translate('OAuth2_ClientCredentialsExceptionPublicClient'));
-        }
+    /**
+     * Updates an OAuth2 client and optionally returns a newly generated secret.
+     *
+     * @param string          $clientId 32-character hexadecimal client identifier.
+     * @param string          $name Display name shown in the Matomo UI.
+     * @param string[]        $grantTypes Grant types to enable.
+     * @param string          $scope Scope identifier to allow.
+     * @param string|string[] $redirectUris Allowed redirect URIs.
+     * @param string          $description Optional description for administrators.
+     * @param string          $type `confidential` or `public`.
+     * @param string          $active `'1'` to enable the client or `'0'` to disable it.
+     * @return array{client: array, secret: string|null}
+     */
+    public function updateClient(string $clientId, string $name, array $grantTypes, string $scope, $redirectUris = [], string $description = '', string $type = 'confidential', string $active = '1'): array
+    {
+        Piwik::checkUserHasSuperUserAccess();
 
-        $scope = array_values(array_intersect([$scope], $this->scopeRepository->getAllowedScopeIds()));
+        $clientId = $this->assertValidClientId($clientId);
 
-        if (empty($scope)) {
-            throw new \InvalidArgumentException(Piwik::translate('OAuth2_InvalidScopeValue'));
+        if (empty($this->clientModel->find($clientId))) {
+            throw new \InvalidArgumentException(Piwik::translate('OAuth2_ClientNotFound'));
         }
 
-        $result = $this->clientManager->create([
-            'name' => $name,
-            'description' => $description,
-            'redirect_uris' => $redirects,
-            'grant_types' => $grantTypes,
-            'scopes' => $scope,
-            'type' => $type,
-            'active' => $active,
-        ], Piwik::getCurrentUserLogin());
+        $data = $this->buildValidatedClientData(
+            $name,
+            $grantTypes,
+            $scope,
+            $redirectUris,
+            $description,
+            $type,
+            $active
+        );
+
+        $result = $this->clientManager->update($clientId, [
+            'name' => $data['name'],
+            'description' => $data['description'],
+            'redirect_uris' => $data['redirect_uris'],
+            'grant_types' => $data['grant_types'],
+            'scopes' => $data['scopes'],
+            'type' => $data['type'],
+            'active' => $data['active'],
+        ]);
+
+        $result['client'] = $this->sanitizeClient($result['client']);
 
         return $result;
     }
@@ -132,6 +185,10 @@ public function rotateSecret(string $clientId): array
         Piwik::checkUserHasSuperUserAccess();
 
         $clientId = $this->assertValidClientId($clientId);
+        $client = $this->getClient($clientId);
+        if (empty($clientId) || empty($client['type']) || $client['type'] != 'confidential') {
+            throw new \InvalidArgumentException(Piwik::translate('OAuth2_InvalidClientToRotateSecretExceptionMessage'));
+        }
 
         $secret = $this->clientManager->rotateSecret($clientId);
 
@@ -141,6 +198,25 @@ public function rotateSecret(string $clientId): array
         ];
     }
 
+    /**
+     * Updates whether an OAuth2 client is active (super users only).
+     *
+     * @param string $clientId 32-character hexadecimal client identifier.
+     * @param string $active `'1'` to enable the client or `'0'` to disable it.
+     * @return array{client: array}
+     */
+    public function setClientActive(string $clientId, string $active): array
+    {
+        Piwik::checkUserHasSuperUserAccess();
+
+        $clientId = $this->assertValidClientId($clientId);
+        $client = $this->clientManager->setActive($clientId, $active === '1');
+
+        return [
+            'client' => $this->sanitizeClient($client),
+        ];
+    }
+
     // TODO: Do we require password for confirmation?
     /**
      * Deletes an OAuth2 client and its related access tokens, refresh tokens, and auth codes (super users only).
@@ -172,6 +248,53 @@ private function assertValidClientId(string $clientId): string
         return $clientId;
     }
 
+    private function buildValidatedClientData(
+        string $name,
+        array $grantTypes,
+        string $scope,
+        $redirectUris,
+        string $description,
+        string $type,
+        string $active
+    ): array {
+        $type = $type === 'public' ? 'public' : 'confidential';
+
+        $redirects = is_array($redirectUris) ? $redirectUris : preg_split('/[\r\n]+/', (string) $redirectUris);
+        if ($redirects === false) {
+            $redirects = [];
+        }
+        $redirects = array_values(array_filter(array_map('trim', $redirects), static function ($value) {
+            return $value !== '';
+        }));
+
+        $grantTypes = array_values(array_filter(array_map('trim', (array) $grantTypes), static function ($value) {
+            return $value !== '';
+        }));
+        $grantTypes = $this->validateGrantTypes($grantTypes);
+
+        $this->validateRedirectUris($redirects, $grantTypes);
+
+        if ($type === 'public' && in_array('client_credentials', $grantTypes, true)) {
+            throw new \InvalidArgumentException(Piwik::translate('OAuth2_ClientCredentialsExceptionPublicClient'));
+        }
+
+        $scope = array_values(array_intersect([$scope], $this->scopeRepository->getAllowedScopeIds()));
+
+        if (empty($scope)) {
+            throw new \InvalidArgumentException(Piwik::translate('OAuth2_InvalidScopeValue'));
+        }
+
+        return [
+            'name' => trim($name),
+            'description' => $description,
+            'redirect_uris' => $redirects,
+            'grant_types' => $grantTypes,
+            'scopes' => $scope,
+            'type' => $type,
+            'active' => $active,
+        ];
+    }
+
     private function validateRedirectUris(array $redirectUris, array $grantTypes): void
     {
         if (!in_array('authorization_code', $grantTypes, true)) {
@@ -206,4 +329,11 @@ private function validateGrantTypes(array $grantTypes): array
 
         return array_values(array_unique($grantTypes));
     }
+
+    private function sanitizeClient(array $client): array
+    {
+        unset($client['secret_hash']);
+
+        return $client;
+    }
 }

Activity/CreateClient.php

@@ -37,6 +37,6 @@ public function getTranslatedDescription($activityData, $performingUser)
     {
         $client = $activityData['client'] ?? [];
 
-        return sprintf('created OAuth2 client "%s"', $this->getClientLabel($client));
+        return sprintf('created OAuth 2.0 client "%s"', $this->getClientLabel($client));
     }
 }

Activity/DeleteClient.php

@@ -40,6 +40,6 @@ public function getTranslatedDescription($activityData, $performingUser)
     {
         $client = $activityData['client'] ?? [];
 
-        return sprintf('deleted OAuth2 client "%s"', $this->getClientLabel($client));
+        return sprintf('deleted OAuth 2.0 client "%s"', $this->getClientLabel($client));
     }
 }

Activity/RotateSecret.php

@@ -39,6 +39,6 @@ public function getTranslatedDescription($activityData, $performingUser)
     {
         $client = $activityData['client'] ?? [];
 
-        return sprintf('rotated secret for OAuth2 client "%s"', $this->getClientLabel($client));
+        return sprintf('rotated secret for OAuth 2.0 client "%s"', $this->getClientLabel($client));
     }
 }

Activity/SetClientActive.php

@@ -0,0 +1,47 @@
+<?php
+
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link    https://matomo.org
+ * @license https://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\OAuth2\Activity;
+
+class SetClientActive extends BaseActivity
+{
+    protected $eventName = 'API.OAuth2.setClientActive.end';
+
+    public function extractParams($eventData)
+    {
+        if (!is_array($eventData) || count($eventData) < 2) {
+            return false;
+        }
+
+        list($result) = $eventData;
+        $client = $result['client'] ?? null;
+
+        if (empty($client['client_id'])) {
+            return false;
+        }
+
+        return [
+            'version' => 'v1',
+            'client' => $this->formatClientData($client),
+            'action' => !empty($client['active']) ? 'resumed' : 'paused',
+        ];
+    }
+
+    public function getTranslatedDescription($activityData, $performingUser)
+    {
+        $client = $activityData['client'] ?? [];
+        $isActive = !empty($client['active']);
+
+        if ($isActive) {
+            return sprintf('resumed OAuth 2.0 client "%s"', $this->getClientLabel($client));
+        }
+
+        return sprintf('paused OAuth 2.0 client "%s"', $this->getClientLabel($client));
+    }
+}

Activity/UpdateClient.php

@@ -0,0 +1,42 @@
+<?php
+
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link    https://matomo.org
+ * @license https://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\OAuth2\Activity;
+
+class UpdateClient extends BaseActivity
+{
+    protected $eventName = 'API.OAuth2.updateClient.end';
+
+    public function extractParams($eventData)
+    {
+        if (!is_array($eventData) || count($eventData) < 2) {
+            return false;
+        }
+
+        list($result) = $eventData;
+        $client = $result['client'] ?? null;
+
+        if (empty($client['client_id'])) {
+            return false;
+        }
+
+        return [
+            'version' => 'v1',
+            'client' => $this->formatClientData($client),
+            'action' => 'updated',
+        ];
+    }
+
+    public function getTranslatedDescription($activityData, $performingUser)
+    {
+        $client = $activityData['client'] ?? [];
+
+        return sprintf('updated OAuth 2.0 client "%s"', $this->getClientLabel($client));
+    }
+}

Controller.php

@@ -44,7 +44,10 @@ public function index()
         Piwik::checkUserHasSuperUserAccess();
 
         $viewData = [
-            'clients' => $this->clientModel->all(),
+            'clients' => array_map(static function (array $client) {
+                unset($client['secret_hash']);
+                return $client;
+            }, $this->clientModel->all()),
             'scopes' => $this->scopeRepository->describeScopes(),
         ];
 

Diagnostic/OpenSslRsaCheck.php

@@ -31,11 +31,11 @@ public function execute()
             $missing[] = 'openssl_pkey_get_details()';
         }
 
-        $label = 'OAuth2 OpenSSL RSA support';
+        $label = 'OAuth 2.0 OpenSSL RSA support';
 
         if (!empty($missing)) {
             $comment = 'Missing OpenSSL RSA constant/function(s): ' . implode(', ', $missing)
-                . '. Enable the PHP OpenSSL extension to generate OAuth2 RSA keys.';
+                . '. Enable the PHP OpenSSL extension to generate OAuth 2.0 RSA keys.';
             return [DiagnosticResult::singleResult($label, DiagnosticResult::STATUS_ERROR, $comment)];
         }