pdsinterop · Potherca · Dec 10, 2025 · Dec 9, 2025 · Dec 10, 2025 · Dec 10, 2025
diff --git a/init.sh b/init.sh
@@ -6,7 +6,8 @@ php console.php app:enable solid
 php console.php config:system:set trusted_domains 1 --value=server
 php console.php config:system:set trusted_domains 2 --value=nextcloud.local
 php console.php config:system:set trusted_domains 3 --value=thirdparty
-# set 'tester' and 'https://tester' as allowed clients for the test suite to run
-php console.php user:setting alice solid allowedClients '["f5d1278e8109edd94e1e4197e04873b9", "2e5cddcf0f663544e98982931e6cc5a6"]'
+# set 'tester' and 'https://tester' as tyrusted apps for the test suite to run
+php console.php config:app:set solid trustedApps --value='["https://tester", "tester"]'
+
 echo configured
 mkdir -p /var/www/html/data/files_trashbin/versions
diff --git a/solid/lib/BaseServerConfig.php b/solid/lib/BaseServerConfig.php
@@ -157,25 +157,35 @@ public function removeClientConfig($clientId) {
 			$this->config->setAppValue('solid', 'clientScopes', $scopes);
 		}
 
-		public function saveClientRegistration($origin, $clientData) {
-			$originHash = md5($origin);
-			$existingRegistration = $this->getClientRegistration($originHash);
-			if ($existingRegistration && isset($existingRegistration['redirect_uris'])) {
-				foreach ($existingRegistration['redirect_uris'] as $uri) {
-					$clientData['redirect_uris'][] = $uri;
-				}
-				$clientData['redirect_uris'] = array_unique($clientData['redirect_uris']);
-				if (isset($existingRegistration['blocked'])) {
-					$clientData['blocked'] = $existingRegistration['blocked'];
+		public function saveClientRegistration($clientData)
+		{
+			$generatedClientId = bin2hex(random_bytes(16)); // 32 chars for the client Id
+
+			// Avoid collision, give up after 5 tries.
+			for ($i = 0; $i < 5; $i++) {
+				$existingRegistration = $this->getClientRegistration($generatedClientId);
+				if ($existingRegistration['client_id'] ?? false) {
+					$generatedClientId = bin2hex(random_bytes(16));
+				} else {
+					break;
 				}
 			}
 
-			$clientData['client_id'] = $originHash;
-			$clientData['client_name'] = $origin;
-			$clientData['client_secret'] = md5(random_bytes(32));
-			$this->config->setAppValue('solid', "client-" . $originHash, json_encode($clientData));
+			if ($existingRegistration['client_id'] ?? false) {
+				throw new \Exception("Could not generate unique client ID");
+			}
+
+			$generatedClientSecret = bin2hex(random_bytes(32)); // and 64 chars for the client secret
+
+			$clientData['client_id'] = $generatedClientId;
+			$clientData['client_secret'] = $generatedClientSecret;
+
+			if (!isset($clientData['client_name'])) {
+				$clientData['client_name'] = "Client $generatedClientId";
+			}
+
+			$this->config->setAppValue('solid', "client-" . $generatedClientId, json_encode($clientData));
 
-			$this->config->setAppValue('solid', "client-" . $origin, json_encode($clientData));
 			return $clientData;
 		}
 
@@ -188,6 +198,13 @@ public function getClientRegistration($clientId) {
 			return json_decode($data, true);
 		}
 
+		public function getTrustedApps()
+		{
+			$appValue = $this->config->getAppValue('solid', 'trustedApps', '[]');
+
+			return json_decode($appValue, true, 512, JSON_THROW_ON_ERROR);
+		}
+
 		public function getUserSubDomainsEnabled() {
 			$value = $this->config->getAppValue('solid', 'userSubDomainsEnabled', false);
 

diff --git a/solid/lib/Controller/ServerController.php b/solid/lib/Controller/ServerController.php
@@ -18,6 +18,7 @@
 use Lcobucci\JWT\Configuration;
 use Lcobucci\JWT\Signer\Key\InMemory;
 use Lcobucci\JWT\Signer\Rsa\Sha256;
+use Pdsinterop\Solid\Auth\Enum\Authorization;
 
 class ServerController extends Controller
 {
@@ -208,22 +209,25 @@ public function authorize() {
 					$getVars['redirect_uri']
 				)
 			);
-			$clientId = $this->config->saveClientRegistration($origin, $clientData)['client_id'];
-			$clientId = $this->config->saveClientRegistration($getVars['client_id'], $clientData)['client_id'];
+
+			$clientId = $this->config->saveClientRegistration($clientData)['client_id'];
 			$returnUrl = $getVars['redirect_uri'];
+			$clientRegistration = $this->config->getClientRegistration($clientId);
+			// @FIXME: Implement $clientRegistration = $this->fetchRemoteClientDocument($getVars['client_id'])
+			//        to replace this section of this `if` statement.
 		} else {
 			$clientId = $getVars['client_id'];
 			$returnUrl = $_SERVER['REQUEST_URI'];
+			$clientRegistration = $this->config->getClientRegistration($clientId);
 		}
 
-		$clientRegistration = $this->config->getClientRegistration($clientId);
 		if (isset($clientRegistration['blocked']) && ($clientRegistration['blocked'] === true)) {
 			$result = new JSONResponse('Unauthorized client');
 			$result->setStatus(403);
 			return $result;
 		}
 
-		$approval = $this->checkApproval($clientId);
+		$approval = $this->checkApproval($clientId, $clientRegistration);
 		if (!$approval) {
 			$result = new JSONResponse('Approval required');
 			$result->setStatus(302);
@@ -283,13 +287,23 @@ public function authorize() {
 		return $this->respond($response); // ->addHeader('Access-Control-Allow-Origin', '*');
 	}
 
-	private function checkApproval($clientId) {
+	private function checkApproval($clientId, $clientRegistration)
+	{
+		$approved = Authorization::DENIED;
+
 		$allowedClients = $this->config->getAllowedClients($this->userId);
 		if (in_array($clientId, $allowedClients)) {
-			return \Pdsinterop\Solid\Auth\Enum\Authorization::APPROVED;
-		} else {
-			return \Pdsinterop\Solid\Auth\Enum\Authorization::DENIED;
+			$approved = Authorization::APPROVED;
+		} elseif (isset($clientRegistration['origin'])) {
+			$origin = $clientRegistration['origin'];
+			$trustedApps = $this->config->getTrustedApps();
+
+			if (in_array($origin, $trustedApps)) {
+				$approved = Authorization::APPROVED;
+			}
 		}
+
+		return $approved;
 	}
 
 	private function getProfilePage() {
@@ -405,20 +419,24 @@ public function register() {
 		if (! isset($clientData['redirect_uris'])) {
 			return new JSONResponse("Missing redirect URIs", Http::STATUS_BAD_REQUEST);
 		}
+
 		$clientData['client_id_issued_at'] = time();
-		$parsedOrigin = parse_url($clientData['redirect_uris'][0]);
+		$parsedOrigin = parse_url($clientData['redirect_uris'][0]); // FIXME: Should we have multiple origins?
 		$origin = $parsedOrigin['scheme'] . '://' . $parsedOrigin['host'];
 		if (isset($parsedOrigin['port'])) {
 			$origin .= ":" . $parsedOrigin['port'];
 		}
+		$clientData['origin'] = $origin;
+
+		$clientData = $this->config->saveClientRegistration($clientData);
 
-		$clientData = $this->config->saveClientRegistration($origin, $clientData);
 		$registration = array(
 			'client_id' => $clientData['client_id'],
 			'client_secret' => $clientData['client_secret'],
 			'registration_client_uri' => $this->urlGenerator->getAbsoluteURL($this->urlGenerator->linkToRoute("solid.server.registeredClient", array("clientId" => $clientData['client_id']))),
 			'client_id_issued_at' => $clientData['client_id_issued_at'],
 			'redirect_uris' => $clientData['redirect_uris'],
+			'origin' => $clientData['origin'],
 		);
 		$registration = $this->tokenGenerator->respondToRegistration($registration, $this->config->getPrivateKey());
 		return (new JSONResponse($registration));

diff --git a/solid/tests/Unit/BaseServerConfigTest.php b/solid/tests/Unit/BaseServerConfigTest.php
@@ -149,21 +149,6 @@ public function testGetClientRegistrationForNonExistingClient()
 		$this->assertEquals($expected, $actual);
 	}
 
-	/**
-	 * @testdox BaseServerConfig should complain when asked to save ClientRegistration without origin
-	 * @covers ::saveClientRegistration
-	 */
-	public function testSaveClientRegistrationWithoutOrigin()
-	{
-		$this->expectException(TypeError::class);
-		$this->expectExceptionMessage('Too few arguments to function');
-
-		$configMock = $this->createMock(IConfig::class);
-		$baseServerConfig = new BaseServerConfig($configMock);
-
-		$baseServerConfig->saveClientRegistration();
-	}
-
 	/**
 	 * @testdox BaseServerConfig should complain when asked to save ClientRegistration without client data
 	 * @covers ::saveClientRegistration
@@ -176,7 +161,7 @@ public function testSaveClientRegistrationWithoutClientData()
 		$configMock = $this->createMock(IConfig::class);
 		$baseServerConfig = new BaseServerConfig($configMock);
 
-		$baseServerConfig->saveClientRegistration(self::MOCK_ORIGIN);
+		$baseServerConfig->saveClientRegistration();
 	}
 
 	/**
@@ -189,156 +174,61 @@ public function testSaveClientRegistrationForNewClient()
 
 		$configMock->expects($this->once())
 			->method('getAppValue')
-			->with(Application::APP_ID, 'client-' . md5(self::MOCK_ORIGIN))
 			->willReturnArgument(2);
 
-		$expected = [
-			'client_id' => md5(self::MOCK_ORIGIN),
-			'client_name' => self::MOCK_ORIGIN,
-			'client_secret' => md5(self::MOCK_RANDOM_BYTES),
-		];
-
-		$configMock->expects($this->exactly(2))
+		$configMock->expects($this->exactly(1))
 			->method('setAppValue')
 			->willReturnMap([
 				// Using willReturnMap as withConsecutive is removed since PHPUnit 10
-				[Application::APP_ID, 'client-' . md5(self::MOCK_ORIGIN), json_encode($expected)],
-				[Application::APP_ID, 'client-' . self::MOCK_ORIGIN, json_encode($expected)]
+				[Application::APP_ID, 'client-' . self::MOCK_ORIGIN, json_encode('{}')]
 			]);
 
 		$baseServerConfig = new BaseServerConfig($configMock);
 
-		$actual = $baseServerConfig->saveClientRegistration(self::MOCK_ORIGIN, []);
+		$actual = $baseServerConfig->saveClientRegistration([]);
 
-		$this->assertEquals($expected, $actual);
+		$this->assertArrayHasKey('client_id', $actual);
+		$this->assertArrayHasKey('client_secret', $actual);
+		$this->assertArrayHasKey('client_name', $actual);
 	}
 
 	/**
-	 * @testdox BaseServerConfig should save ClientRegistration when asked to save ClientRegistration for existing client
-	 * @covers ::saveClientRegistration
+	 * @testdox BaseServerConfig should remove ClientRegistration when asked to remove ClientRegistration
+	 * @covers ::removeClientRegistration
 	 */
-	public function testSaveClientRegistrationForExistingClient()
+	public function testRemoveClientRegistration()
 	{
 		$configMock = $this->createMock(IConfig::class);
-
-		$expected = [
-			'client_id' => md5(self::MOCK_ORIGIN),
-			'client_name' => self::MOCK_ORIGIN,
-			'client_secret' => md5(self::MOCK_RANDOM_BYTES),
-			'redirect_uris' => [self::MOCK_REDIRECT_URI],
-		];
-
-		$configMock->expects($this->once())
-			->method('getAppValue')
-			->with(Application::APP_ID, 'client-' . md5(self::MOCK_ORIGIN))
-			->willReturn(json_encode($expected));
-
-		$configMock->expects($this->exactly(2))
-			->method('setAppValue')
-			->willReturnMap([
-				// Using willReturnMap as withConsecutive is deprecated since PHPUnit 10
-				[Application::APP_ID, 'client-' . md5(self::MOCK_ORIGIN), json_encode($expected)],
-				[Application::APP_ID, 'client-' . self::MOCK_ORIGIN, json_encode($expected)]
-			]);
-
 		$baseServerConfig = new BaseServerConfig($configMock);
 
-		$actual = $baseServerConfig->saveClientRegistration(self::MOCK_ORIGIN, []);
-
-		$this->assertEquals($expected, $actual);
-	}
-
-	/**
-	 * @testdox BaseServerConfig should save ClientRegistration when asked to save ClientRegistration for blocked client
-	 * @covers ::saveClientRegistration
-	 */
-	public function testSaveClientRegistrationForBlockedClient()
-	{
-		$configMock = $this->createMock(IConfig::class);
-
-		$expected = [
-			'client_id' => md5(self::MOCK_ORIGIN),
-			'client_name' => self::MOCK_ORIGIN,
-			'client_secret' => md5(self::MOCK_RANDOM_BYTES),
-			'redirect_uris' => [self::MOCK_REDIRECT_URI],
-			'blocked' => true,
-		];
-
 		$configMock->expects($this->once())
-			->method('getAppValue')
-			->with(Application::APP_ID, 'client-' . md5(self::MOCK_ORIGIN))
-			->willReturn(json_encode($expected));
-
-		$configMock->expects($this->exactly(2))
-			->method('setAppValue')
-			->willReturnMap([
-				// Using willReturnMap as withConsecutive is deprecated since PHPUnit 10
-				[Application::APP_ID, 'client-' . md5(self::MOCK_ORIGIN), json_encode($expected)],
-				[Application::APP_ID, 'client-' . self::MOCK_ORIGIN, json_encode($expected)]
-			]);
-
-		$baseServerConfig = new BaseServerConfig($configMock);
-
-		$actual = $baseServerConfig->saveClientRegistration(self::MOCK_ORIGIN, $expected);
+			->method('deleteAppValue')
+			->with(Application::APP_ID, 'client-' . self::MOCK_CLIENT_ID);
 
-		$this->assertEquals($expected, $actual);
+		$baseServerConfig->removeClientRegistration(self::MOCK_CLIENT_ID);
 	}
 
 	/**
-	 * @testdox BaseServerConfig should always "blocked" to existing value when asked to save ClientRegistration for blocked client
-	 * @covers ::saveClientRegistration
+	 * @testdox BaseServerConfig should return decoded trusted apps when asked to GetTrustedApps
+	 * @covers ::getTrustedApps
 	 */
-	public function testSaveClientRegistrationSetsBlocked()
+	public function testGetTrustedApps()
 	{
 		$configMock = $this->createMock(IConfig::class);
+		$baseServerConfig = new BaseServerConfig($configMock);
 
-		$expected = [
-			'client_id' => md5(self::MOCK_ORIGIN),
-			'client_name' => self::MOCK_ORIGIN,
-			'client_secret' => md5(self::MOCK_RANDOM_BYTES),
-			'redirect_uris' => [self::MOCK_REDIRECT_URI],
-			'blocked' => true,
-		];
+		$expected = [self::MOCK_ORIGIN];
 
 		$configMock->expects($this->once())
 			->method('getAppValue')
-			->with(Application::APP_ID, 'client-' . md5(self::MOCK_ORIGIN))
+			->with(Application::APP_ID, 'trustedApps', '[]')
 			->willReturn(json_encode($expected));
 
-		$clientData = $expected;
-		$clientData['blocked'] = false;
-
-		$configMock->expects($this->exactly(2))
-			->method('setAppValue')
-			->willReturnMap([
-				// Using willReturnMap as withConsecutive is deprecated since PHPUnit 10
-				[Application::APP_ID, 'client-' . md5(self::MOCK_ORIGIN), json_encode($expected)],
-				[Application::APP_ID, 'client-' . self::MOCK_ORIGIN, json_encode($expected)]
-			]);
-
-		$baseServerConfig = new BaseServerConfig($configMock);
-
-		$actual = $baseServerConfig->saveClientRegistration(self::MOCK_ORIGIN, $clientData);
+		$actual = $baseServerConfig->getTrustedApps();
 
 		$this->assertEquals($expected, $actual);
 	}
 
-	/**
-	 * @testdox BaseServerConfig should remove ClientRegistration when asked to remove ClientRegistration
-	 * @covers ::removeClientRegistration
-	 */
-	public function testRemoveClientRegistration()
-	{
-		$configMock = $this->createMock(IConfig::class);
-		$baseServerConfig = new BaseServerConfig($configMock);
-
-		$configMock->expects($this->once())
-			->method('deleteAppValue')
-			->with(Application::APP_ID, 'client-' . self::MOCK_CLIENT_ID);
-
-		$baseServerConfig->removeClientRegistration(self::MOCK_CLIENT_ID);
-	}
-
 	/////////////////////////////// DATAPROVIDERS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
 
 	public function provideBooleans()