From 0ec4ec09edf4419420ca109e1b56319422af489d Mon Sep 17 00:00:00 2001
From: romanetar <roman_ag@hotmail.com>
Date: Tue, 3 Nov 2026 13:52:38 +0100
Subject: [PATCH] fix(oauth2): ignore client_id, client_secret,
 client_secret_expires_at and owner on client update

PUT /admin/clients/{id} forwarded the raw request payload to
ClientFactory::populate(), which assigned credentials when present.
Strip them in ClientApiController::curateUpdatePayload() so credentials
only change through ClientCredentialGenerator (create / regenerate), and
a string owner no longer causes a TypeError (HTTP 500).

Adds regression tests for the update endpoint and for secret regeneration.
---
 .../Controllers/Api/ClientApiController.php   | 17 ++++++
 tests/ClientApiTest.php                       | 52 ++++++++++++++++++-
 2 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/app/Http/Controllers/Api/ClientApiController.php b/app/Http/Controllers/Api/ClientApiController.php
index a507143a..72ae3660 100644
--- a/app/Http/Controllers/Api/ClientApiController.php
+++ b/app/Http/Controllers/Api/ClientApiController.php
@@ -742,6 +742,23 @@ protected function getCreatePayloadValidationRules(): array
         ];
     }
 
+    /**
+     * Credentials and ownership must never be set through the update endpoint:
+     * credentials change only via ClientCredentialGenerator (create / regenerate).
+     * @param array $payload
+     * @return array
+     */
+    protected function curateUpdatePayload(array $payload): array
+    {
+        unset(
+            $payload['client_id'],
+            $payload['client_secret'],
+            $payload['client_secret_expires_at'],
+            $payload['owner']
+        );
+        return $payload;
+    }
+
     protected function getResourceServerContext(): IResourceServerContext
     {
         // TODO: Implement getResourceServerContext() method.
diff --git a/tests/ClientApiTest.php b/tests/ClientApiTest.php
index 7f552af1..6f9dccc3 100644
--- a/tests/ClientApiTest.php
+++ b/tests/ClientApiTest.php
@@ -560,4 +560,54 @@ public function testUpdateNativeClientRejectsSchemeAlreadyRegisteredInAuthorityL
         $this->assertResponseStatus(412);
     }
 
-}
\ No newline at end of file
+    public function testUpdateIgnoresCredentialsAndOwner(){
+
+        $client = EntityManager::getRepository(Client::class)->findOneBy(['app_name' => 'oauth2_test_app']);
+        $id         = $client->id;
+        $client_id  = $client->getClientId();
+        $secret     = $client->getClientSecret();
+        $expires_at = $client->getClientSecretExpiration();
+        $owner_id   = $client->getOwner()->getId();
+
+        $this->action("PUT", "Api\\ClientApiController@update",
+            [
+                'id'                       => $id,
+                'app_name'                 => 'oauth2_test_app',
+                'application_type'         => $client->getApplicationType(),
+                'client_id'                => 'hijacked.client.id',
+                'client_secret'            => 'short',
+                'client_secret_expires_at' => time() + 31536000,
+                'owner'                    => '123',
+            ],
+            [], [], []);
+
+        $this->assertResponseStatus(201);
+
+        EntityManager::clear();
+        $client = EntityManager::getRepository(Client::class)->find($id);
+        $this->assertEquals($client_id, $client->getClientId());
+        $this->assertEquals($secret, $client->getClientSecret());
+        $this->assertEquals($expires_at, $client->getClientSecretExpiration());
+        $this->assertEquals($owner_id, $client->getOwner()->getId());
+    }
+
+    public function testRegenerateClientSecretStillRotatesAndDispatchesEvent(){
+
+        $client = EntityManager::getRepository(Client::class)->findOneBy(['app_name' => 'oauth2_test_app']);
+        $id        = $client->id;
+        $client_id = $client->getClientId();
+        $old       = $client->getClientSecret();
+
+        $dispatched = [];
+        \Illuminate\Support\Facades\Event::listen('oauth2.client.regenerate.secret', function ($cid) use (&$dispatched) {
+            $dispatched[] = $cid;
+        });
+
+        $new_client = App::make(ClientService::class)->regenerateClientSecret($id);
+
+        $this->assertNotEquals($old, $new_client->getClientSecret());
+        $this->assertEquals(64, strlen($new_client->getClientSecret()));
+        $this->assertEquals([$client_id], $dispatched);
+    }
+
+}