Merge pull request #61363 from nextcloud/backport/61289/stable32

[stable32] feat(oauth2): Add commands for adding and deleting clients

apps/oauth2/appinfo/info.xml

@@ -35,6 +35,8 @@
 
 	<commands>
 		<command>OCA\OAuth2\Command\ImportLegacyOcClient</command>
+		<command>OCA\OAuth2\Command\AddClient</command>
+		<command>OCA\OAuth2\Command\DeleteClient</command>
 	</commands>
 
 	<settings>

apps/oauth2/composer/composer/autoload_classmap.php

@@ -8,6 +8,8 @@ $baseDir = $vendorDir;
 return array(
     'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php',
     'OCA\\OAuth2\\BackgroundJob\\CleanupExpiredAuthorizationCode' => $baseDir . '/../lib/BackgroundJob/CleanupExpiredAuthorizationCode.php',
+    'OCA\\OAuth2\\Command\\AddClient' => $baseDir . '/../lib/Command/AddClient.php',
+    'OCA\\OAuth2\\Command\\DeleteClient' => $baseDir . '/../lib/Command/DeleteClient.php',
     'OCA\\OAuth2\\Command\\ImportLegacyOcClient' => $baseDir . '/../lib/Command/ImportLegacyOcClient.php',
     'OCA\\OAuth2\\Controller\\LoginRedirectorController' => $baseDir . '/../lib/Controller/LoginRedirectorController.php',
     'OCA\\OAuth2\\Controller\\OauthApiController' => $baseDir . '/../lib/Controller/OauthApiController.php',
@@ -25,5 +27,6 @@ return array(
     'OCA\\OAuth2\\Migration\\Version011602Date20230613160650' => $baseDir . '/../lib/Migration/Version011602Date20230613160650.php',
     'OCA\\OAuth2\\Migration\\Version011603Date20230620111039' => $baseDir . '/../lib/Migration/Version011603Date20230620111039.php',
     'OCA\\OAuth2\\Migration\\Version011901Date20240829164356' => $baseDir . '/../lib/Migration/Version011901Date20240829164356.php',
+    'OCA\\OAuth2\\Service\\ClientService' => $baseDir . '/../lib/Service/ClientService.php',
     'OCA\\OAuth2\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php',
 );

apps/oauth2/composer/composer/autoload_static.php

@@ -23,6 +23,8 @@ class ComposerStaticInitOAuth2
     public static $classMap = array (
         'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php',
         'OCA\\OAuth2\\BackgroundJob\\CleanupExpiredAuthorizationCode' => __DIR__ . '/..' . '/../lib/BackgroundJob/CleanupExpiredAuthorizationCode.php',
+        'OCA\\OAuth2\\Command\\AddClient' => __DIR__ . '/..' . '/../lib/Command/AddClient.php',
+        'OCA\\OAuth2\\Command\\DeleteClient' => __DIR__ . '/..' . '/../lib/Command/DeleteClient.php',
         'OCA\\OAuth2\\Command\\ImportLegacyOcClient' => __DIR__ . '/..' . '/../lib/Command/ImportLegacyOcClient.php',
         'OCA\\OAuth2\\Controller\\LoginRedirectorController' => __DIR__ . '/..' . '/../lib/Controller/LoginRedirectorController.php',
         'OCA\\OAuth2\\Controller\\OauthApiController' => __DIR__ . '/..' . '/../lib/Controller/OauthApiController.php',
@@ -40,6 +42,7 @@ class ComposerStaticInitOAuth2
         'OCA\\OAuth2\\Migration\\Version011602Date20230613160650' => __DIR__ . '/..' . '/../lib/Migration/Version011602Date20230613160650.php',
         'OCA\\OAuth2\\Migration\\Version011603Date20230620111039' => __DIR__ . '/..' . '/../lib/Migration/Version011603Date20230620111039.php',
         'OCA\\OAuth2\\Migration\\Version011901Date20240829164356' => __DIR__ . '/..' . '/../lib/Migration/Version011901Date20240829164356.php',
+        'OCA\\OAuth2\\Service\\ClientService' => __DIR__ . '/..' . '/../lib/Service/ClientService.php',
         'OCA\\OAuth2\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php',
     );
 

apps/oauth2/lib/Command/AddClient.php

@@ -0,0 +1,71 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH
+ * SPDX-FileContributor: Carl Schwan
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\OAuth2\Command;
+
+use OC\Core\Command\Base;
+use OCA\OAuth2\Service\ClientService;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class AddClient extends Base {
+	private const ARGUMENT_CLIENT_NAME = 'client-name';
+	private const ARGUMENT_CLIENT_REDIRECT_URI = 'client-redirect-uri';
+
+	public function __construct(
+		private readonly ClientService $clientService,
+	) {
+		parent::__construct();
+	}
+
+	#[\Override]
+	protected function configure(): void {
+		parent::configure();
+
+		$this->setName('oauth2:add-client');
+		$this->setDescription('This command adds a new oauth2 client.');
+		$this->addArgument(
+			self::ARGUMENT_CLIENT_NAME,
+			InputArgument::REQUIRED,
+			'Name of the oauth2 client',
+		);
+		$this->addArgument(
+			self::ARGUMENT_CLIENT_REDIRECT_URI,
+			InputArgument::REQUIRED,
+			'Redirection uri of the oauth2 client ',
+		);
+	}
+
+	#[\Override]
+	protected function execute(InputInterface $input, OutputInterface $output): int {
+		/** @var string $name */
+		$name = $input->getArgument(self::ARGUMENT_CLIENT_NAME);
+
+		/** @var string $redirectUri */
+		$redirectUri = $input->getArgument(self::ARGUMENT_CLIENT_REDIRECT_URI);
+
+		// Should not happen but just to be sure
+		if (empty($redirectUri) || empty($name)) {
+			$output->writeln('<error>Redirect uri or name is empty</error>');
+			return Command::FAILURE;
+		}
+
+		if (filter_var($redirectUri, FILTER_VALIDATE_URL) === false) {
+			$output->writeln('<error>Your redirect URL needs to be a full URL for example: https://yourdomain.com/path</error>');
+			return Command::FAILURE;
+		}
+
+		$result = $this->clientService->addClient($name, $redirectUri);
+		$this->writeArrayInOutputFormat($input, $output, $result);
+		return Command::SUCCESS;
+	}
+}

apps/oauth2/lib/Command/DeleteClient.php

@@ -0,0 +1,58 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH
+ * SPDX-FileContributor: Carl Schwan
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\OAuth2\Command;
+
+use OC\Core\Command\Base;
+use OCA\OAuth2\Service\ClientService;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class DeleteClient extends Base {
+	private const ARGUMENT_CLIENT_ID = 'client-id';
+
+	public function __construct(
+		private readonly ClientService $clientService,
+	) {
+		parent::__construct();
+	}
+
+	#[\Override]
+	protected function configure(): void {
+		parent::configure();
+
+		$this->setName('oauth2:delete-client');
+		$this->setDescription('This command removes an existing oauth2 client.');
+		$this->addArgument(
+			self::ARGUMENT_CLIENT_ID,
+			InputArgument::REQUIRED,
+			'Id of the oauth2 client',
+		);
+	}
+
+	#[\Override]
+	protected function execute(InputInterface $input, OutputInterface $output): int {
+		$id = (int)$input->getArgument(self::ARGUMENT_CLIENT_ID);
+		if ($id === 0) {
+			$output->writeln('<error>The given id is not a valid positive integer.</error>');
+			return Command::FAILURE;
+		}
+
+		try {
+			$this->clientService->deleteClient($id);
+		} catch (\Exception $exception) {
+			$output->writeln('<error>' . $exception->getMessage() . '</error>');
+			return Command::FAILURE;
+		}
+		return Command::SUCCESS;
+	}
+}

apps/oauth2/lib/Controller/SettingsController.php

@@ -8,39 +8,20 @@ declare(strict_types=1);
  */
 namespace OCA\OAuth2\Controller;
 
-use OC\Authentication\Token\IProvider as IAuthTokenProvider;
-use OCA\OAuth2\Db\AccessTokenMapper;
-use OCA\OAuth2\Db\Client;
-use OCA\OAuth2\Db\ClientMapper;
+use OCA\OAuth2\Service\ClientService;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
 use OCP\AppFramework\Http\JSONResponse;
-use OCP\Authentication\Exceptions\InvalidTokenException;
-use OCP\Authentication\Exceptions\WipeTokenException;
 use OCP\IL10N;
 use OCP\IRequest;
-use OCP\IUser;
-use OCP\IUserManager;
-use OCP\Security\ICrypto;
-use OCP\Security\ISecureRandom;
-use Psr\Log\LoggerInterface;
 
 class SettingsController extends Controller {
-
-	public const validChars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
-
 	public function __construct(
 		string $appName,
 		IRequest $request,
-		private ClientMapper $clientMapper,
-		private ISecureRandom $secureRandom,
-		private AccessTokenMapper $accessTokenMapper,
 		private IL10N $l,
-		private IAuthTokenProvider $tokenProvider,
-		private IUserManager $userManager,
-		private ICrypto $crypto,
-		private LoggerInterface $logger,
+		private readonly ClientService $clientService,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -52,55 +33,14 @@ class SettingsController extends Controller {
 			return new JSONResponse(['message' => $this->l->t('Your redirect URL needs to be a full URL for example: https://yourdomain.com/path')], Http::STATUS_BAD_REQUEST);
 		}
 
-		$client = new Client();
-		$client->setName($name);
-		$client->setRedirectUri($redirectUri);
-		$secret = $this->secureRandom->generate(64, self::validChars);
-		$hashedSecret = bin2hex($this->crypto->calculateHMAC($secret));
-		$client->setSecret($hashedSecret);
-		$client->setClientIdentifier($this->secureRandom->generate(64, self::validChars));
-		$client = $this->clientMapper->insert($client);
-
-		$result = [
-			'id' => $client->getId(),
-			'name' => $client->getName(),
-			'redirectUri' => $client->getRedirectUri(),
-			'clientId' => $client->getClientIdentifier(),
-			'clientSecret' => $secret,
-		];
+		$result = $this->clientService->addClient($name, $redirectUri);
 
 		return new JSONResponse($result);
 	}
 
 	#[PasswordConfirmationRequired]
 	public function deleteClient(int $id): JSONResponse {
-		$client = $this->clientMapper->getByUid($id);
-
-		$this->userManager->callForSeenUsers(function (IUser $user) use ($client): void {
-			// Skip tokens that are marked for remote wipe so revoking the
-			// OAuth2 client does not silently cancel a pending wipe.
-			$tokens = $this->tokenProvider->getTokenByUser($user->getUID());
-			foreach ($tokens as $token) {
-				if ($token->getName() !== $client->getName()) {
-					continue;
-				}
-				try {
-					$this->tokenProvider->getTokenById($token->getId());
-				} catch (WipeTokenException $e) {
-					$this->logger->info('Preserving token {tokenId} of user {uid}: marked for remote wipe, OAuth2 client revoke would cancel the wipe.', [
-						'tokenId' => $token->getId(),
-						'uid' => $user->getUID(),
-					]);
-					continue;
-				} catch (InvalidTokenException $e) {
-					// Token already invalid; let invalidateTokenById handle it.
-				}
-				$this->tokenProvider->invalidateTokenById($user->getUID(), $token->getId());
-			}
-		});
-
-		$this->accessTokenMapper->deleteByClientId($id);
-		$this->clientMapper->delete($client);
+		$this->clientService->deleteClient($id);
 		return new JSONResponse([]);
 	}
 }

apps/oauth2/lib/Service/ClientService.php

@@ -0,0 +1,98 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH
+ * SPDX-FileContributor: Carl Schwan
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\OAuth2\Service;
+
+use OC\Authentication\Token\IProvider as IAuthTokenProvider;
+use OCA\OAuth2\Db\AccessTokenMapper;
+use OCA\OAuth2\Db\Client;
+use OCA\OAuth2\Db\ClientMapper;
+use OCP\Authentication\Exceptions\InvalidTokenException;
+use OCP\Authentication\Exceptions\WipeTokenException;
+use OCP\IUser;
+use OCP\IUserManager;
+use OCP\Security\ICrypto;
+use OCP\Security\ISecureRandom;
+use Psr\Log\LoggerInterface;
+
+class ClientService {
+	public const validChars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
+
+	public function __construct(
+		private readonly ISecureRandom $secureRandom,
+		private readonly ICrypto $crypto,
+		private readonly ClientMapper $clientMapper,
+		private readonly IUserManager $userManager,
+		private readonly IAuthTokenProvider $tokenProvider,
+		private readonly LoggerInterface $logger,
+		private readonly AccessTokenMapper $accessTokenMapper,
+	) {
+	}
+
+	/**
+	 * @param non-empty-string $name
+	 * @param non-empty-string $redirectUri
+	 * @return array{
+	 *     id: int,
+	 *     name: string,
+	 *     redirectUri: string,
+	 *     clientId: string,
+	 *     clientSecret: string,
+	 * }
+	 */
+	public function addClient(string $name, string $redirectUri): array {
+		$client = new Client();
+		$client->setName($name);
+		$client->setRedirectUri($redirectUri);
+		$secret = $this->secureRandom->generate(64, self::validChars);
+		$hashedSecret = bin2hex($this->crypto->calculateHMAC($secret));
+		$client->setSecret($hashedSecret);
+		$client->setClientIdentifier($this->secureRandom->generate(64, self::validChars));
+		$client = $this->clientMapper->insert($client);
+
+		return [
+			'id' => $client->getId(),
+			'name' => $client->getName(),
+			'redirectUri' => $client->getRedirectUri(),
+			'clientId' => $client->getClientIdentifier(),
+			'clientSecret' => $secret,
+		];
+	}
+
+	public function deleteClient(int $id): void {
+		$client = $this->clientMapper->getByUid($id);
+
+		$this->userManager->callForSeenUsers(function (IUser $user) use ($client): void {
+			// Skip tokens that are marked for remote wipe so revoking the
+			// OAuth2 client does not silently cancel a pending wipe.
+			$tokens = $this->tokenProvider->getTokenByUser($user->getUID());
+			foreach ($tokens as $token) {
+				if ($token->getName() !== $client->getName()) {
+					continue;
+				}
+				try {
+					$this->tokenProvider->getTokenById($token->getId());
+				} catch (WipeTokenException) {
+					$this->logger->info('Preserving token {tokenId} of user {uid}: marked for remote wipe, OAuth2 client revoke would cancel the wipe.', [
+						'tokenId' => $token->getId(),
+						'uid' => $user->getUID(),
+					]);
+					continue;
+				} catch (InvalidTokenException) {
+					// Token already invalid; let invalidateTokenById handle it.
+				}
+				$this->tokenProvider->invalidateTokenById($user->getUID(), $token->getId());
+			}
+		});
+
+		$this->accessTokenMapper->deleteByClientId($id);
+		$this->clientMapper->delete($client);
+	}
+}

apps/oauth2/tests/Controller/SettingsControllerTest.php

@@ -1,288 +1,26 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 namespace OCA\OAuth2\Tests\Controller;
 
-use OC\Authentication\Token\IProvider as IAuthTokenProvider;
 use OCA\OAuth2\Controller\SettingsController;
-use OCA\OAuth2\Db\AccessTokenMapper;
-use OCA\OAuth2\Db\Client;
-use OCA\OAuth2\Db\ClientMapper;
 use OCP\AppFramework\Http;
-use OCP\AppFramework\Http\JSONResponse;
-use OCP\Authentication\Exceptions\WipeTokenException;
-use OCP\Authentication\Token\IToken;
-use OCP\IL10N;
-use OCP\IRequest;
-use OCP\IUser;
-use OCP\IUserManager;
-use OCP\Security\ICrypto;
-use OCP\Security\ISecureRandom;
 use OCP\Server;
-use PHPUnit\Framework\MockObject\MockObject;
-use Psr\Log\LoggerInterface;
+use PHPUnit\Framework\Attributes\Group;
 use Test\TestCase;
 
 /**
  * @group DB
  */
 class SettingsControllerTest extends TestCase {
-	/** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */
-	private $request;
-	/** @var ClientMapper|\PHPUnit\Framework\MockObject\MockObject */
-	private $clientMapper;
-	/** @var ISecureRandom|\PHPUnit\Framework\MockObject\MockObject */
-	private $secureRandom;
-	/** @var AccessTokenMapper|\PHPUnit\Framework\MockObject\MockObject */
-	private $accessTokenMapper;
-	/** @var IAuthTokenProvider|\PHPUnit\Framework\MockObject\MockObject */
-	private $authTokenProvider;
-	/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
-	private $userManager;
-	/** @var SettingsController */
-	private $settingsController;
-	/** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */
-	private $l;
-	/** @var ICrypto|\PHPUnit\Framework\MockObject\MockObject */
-	private $crypto;
-	private LoggerInterface&MockObject $logger;
-
-	protected function setUp(): void {
-		parent::setUp();
-
-		$this->request = $this->createMock(IRequest::class);
-		$this->clientMapper = $this->createMock(ClientMapper::class);
-		$this->secureRandom = $this->createMock(ISecureRandom::class);
-		$this->accessTokenMapper = $this->createMock(AccessTokenMapper::class);
-		$this->authTokenProvider = $this->createMock(IAuthTokenProvider::class);
-		$this->userManager = $this->createMock(IUserManager::class);
-		$this->crypto = $this->createMock(ICrypto::class);
-		$this->logger = $this->createMock(LoggerInterface::class);
-		$this->l = $this->createMock(IL10N::class);
-		$this->l->method('t')
-			->willReturnArgument(0);
-		$this->settingsController = new SettingsController(
-			'oauth2',
-			$this->request,
-			$this->clientMapper,
-			$this->secureRandom,
-			$this->accessTokenMapper,
-			$this->l,
-			$this->authTokenProvider,
-			$this->userManager,
-			$this->crypto,
-			$this->logger,
-		);
-
-	}
-
-	public function testAddClient(): void {
-		$this->secureRandom
-			->expects($this->exactly(2))
-			->method('generate')
-			->with(64, 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789')
-			->willReturnOnConsecutiveCalls(
-				'MySecret',
-				'MyClientIdentifier');
-
-		$this->crypto
-			->expects($this->once())
-			->method('calculateHMAC')
-			->willReturn('MyHashedSecret');
-
-		$client = new Client();
-		$client->setName('My Client Name');
-		$client->setRedirectUri('https://example.com/');
-		$client->setSecret(bin2hex('MyHashedSecret'));
-		$client->setClientIdentifier('MyClientIdentifier');
-
-		$this->clientMapper
-			->expects($this->once())
-			->method('insert')
-			->with($this->callback(function (Client $c) {
-				return $c->getName() === 'My Client Name'
-					&& $c->getRedirectUri() === 'https://example.com/'
-					&& $c->getSecret() === bin2hex('MyHashedSecret')
-					&& $c->getClientIdentifier() === 'MyClientIdentifier';
-			}))->willReturnCallback(function (Client $c) {
-				$c->setId(42);
-				return $c;
-			});
-
-		$result = $this->settingsController->addClient('My Client Name', 'https://example.com/');
-		$this->assertInstanceOf(JSONResponse::class, $result);
-
-		$data = $result->getData();
-
-		$this->assertEquals([
-			'id' => 42,
-			'name' => 'My Client Name',
-			'redirectUri' => 'https://example.com/',
-			'clientId' => 'MyClientIdentifier',
-			'clientSecret' => 'MySecret',
-		], $data);
-	}
-
-	public function testDeleteClient(): void {
-
-		$userManager = Server::get(IUserManager::class);
-		// count other users in the db before adding our own
-		$count = 0;
-		$function = function (IUser $user) use (&$count): void {
-			if ($user->getLastLogin() > 0) {
-				$count++;
-			}
-		};
-		$userManager->callForAllUsers($function);
-		$user1 = $userManager->createUser('test101', 'test101');
-		$user1->updateLastLoginTimestamp();
-		$tokenProviderMock = $this->getMockBuilder(IAuthTokenProvider::class)->getMock();
-
-		// One getTokenByUser call per user; we return no matching tokens here
-		// so invalidateTokenById is never invoked.
-		$tokenProviderMock
-			->expects($this->exactly($count + 1))
-			->method('getTokenByUser')
-			->willReturn([]);
-		$tokenProviderMock
-			->expects($this->never())
-			->method('invalidateTokenById');
-
-		$client = new Client();
-		$client->setId(123);
-		$client->setName('My Client Name');
-		$client->setRedirectUri('https://example.com/');
-		$client->setSecret(bin2hex('MyHashedSecret'));
-		$client->setClientIdentifier('MyClientIdentifier');
-
-		$this->clientMapper
-			->method('getByUid')
-			->with(123)
-			->willReturn($client);
-		$this->accessTokenMapper
-			->expects($this->once())
-			->method('deleteByClientId')
-			->with(123);
-		$this->clientMapper
-			->expects($this->once())
-			->method('delete')
-			->with($client);
-
-		$settingsController = new SettingsController(
-			'oauth2',
-			$this->request,
-			$this->clientMapper,
-			$this->secureRandom,
-			$this->accessTokenMapper,
-			$this->l,
-			$tokenProviderMock,
-			$userManager,
-			$this->crypto,
-			$this->logger,
-		);
-
-		$result = $settingsController->deleteClient(123);
-		$this->assertInstanceOf(JSONResponse::class, $result);
-		$this->assertEquals([], $result->getData());
-
-		$user1->delete();
-	}
-
-	public function testDeleteClientPreservesWipePendingToken(): void {
-		$userManager = Server::get(IUserManager::class);
-		$user = $userManager->createUser('test_wipe_preserve', 'test_wipe_preserve');
-		$user->updateLastLoginTimestamp();
-
-		$client = new Client();
-		$client->setId(456);
-		$client->setName('My Client Name');
-		$client->setRedirectUri('https://example.com/');
-		$client->setSecret(bin2hex('MyHashedSecret'));
-		$client->setClientIdentifier('MyClientIdentifier');
-
-		// Token marked for wipe with a matching client name: must NOT be invalidated.
-		$wipeToken = $this->createMock(IToken::class);
-		$wipeToken->method('getId')->willReturn(11);
-		$wipeToken->method('getName')->willReturn('My Client Name');
-
-		// Regular token with matching name: must be invalidated.
-		$regularToken = $this->createMock(IToken::class);
-		$regularToken->method('getId')->willReturn(12);
-		$regularToken->method('getName')->willReturn('My Client Name');
-
-		// Non-matching name: must be left alone.
-		$otherToken = $this->createMock(IToken::class);
-		$otherToken->method('getId')->willReturn(13);
-		$otherToken->method('getName')->willReturn('Some Other Client');
-
-		$tokenProviderMock = $this->getMockBuilder(IAuthTokenProvider::class)->getMock();
-		$tokenProviderMock
-			->method('getTokenByUser')
-			->willReturnCallback(function (string $uid) use ($wipeToken, $regularToken, $otherToken) {
-				return $uid === 'test_wipe_preserve'
-					? [$wipeToken, $regularToken, $otherToken]
-					: [];
-			});
-		// Wipe state is signalled via WipeTokenException from getTokenById.
-		$tokenProviderMock
-			->method('getTokenById')
-			->willReturnCallback(function (int $id) use ($wipeToken, $regularToken) {
-				if ($id === 11) {
-					throw new WipeTokenException($wipeToken);
-				}
-				return $regularToken;
-			});
-		$tokenProviderMock
-			->expects($this->once())
-			->method('invalidateTokenById')
-			->with('test_wipe_preserve', 12);
-
-		$this->clientMapper
-			->method('getByUid')
-			->with(456)
-			->willReturn($client);
-		$this->accessTokenMapper
-			->expects($this->once())
-			->method('deleteByClientId')
-			->with(456);
-		$this->clientMapper
-			->expects($this->once())
-			->method('delete')
-			->with($client);
-
-		$logger = $this->createMock(LoggerInterface::class);
-		$logger->expects($this->atLeastOnce())
-			->method('info')
-			->with($this->stringContains('Preserving token'), $this->callback(function (array $context) {
-				return ($context['tokenId'] ?? null) === 11
-					&& ($context['uid'] ?? null) === 'test_wipe_preserve';
-			}));
-
-		$settingsController = new SettingsController(
-			'oauth2',
-			$this->request,
-			$this->clientMapper,
-			$this->secureRandom,
-			$this->accessTokenMapper,
-			$this->l,
-			$tokenProviderMock,
-			$userManager,
-			$this->crypto,
-			$logger,
-		);
-
-		$result = $settingsController->deleteClient(456);
-		$this->assertInstanceOf(JSONResponse::class, $result);
-		$this->assertEquals([], $result->getData());
-
-		$user->delete();
-	}
-
 	public function testInvalidRedirectUri(): void {
-		$result = $this->settingsController->addClient('test', 'invalidurl');
+		$settingsController = Server::get(SettingsController::class);
+		$result = $settingsController->addClient('test', 'invalidurl');
 
 		$this->assertEquals(Http::STATUS_BAD_REQUEST, $result->getStatus());
 		$this->assertSame(['message' => 'Your redirect URL needs to be a full URL for example: https://yourdomain.com/path'], $result->getData());

apps/oauth2/tests/Service/ClientServiceTest.php

@@ -0,0 +1,243 @@
+<?php
+
+/**
+ * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\OAuth2\Tests\Service;
+
+use OC\Authentication\Token\IProvider as IAuthTokenProvider;
+use OCA\OAuth2\Db\AccessTokenMapper;
+use OCA\OAuth2\Db\Client;
+use OCA\OAuth2\Db\ClientMapper;
+use OCA\OAuth2\Service\ClientService;
+use OCP\Authentication\Exceptions\WipeTokenException;
+use OCP\Authentication\Token\IToken;
+use OCP\IUser;
+use OCP\IUserManager;
+use OCP\Security\ICrypto;
+use OCP\Security\ISecureRandom;
+use OCP\Server;
+use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
+use Test\TestCase;
+
+/**
+ * @group DB
+ */
+class ClientServiceTest extends TestCase {
+	private ClientMapper&MockObject $clientMapper;
+	private ISecureRandom&MockObject $secureRandom;
+	private AccessTokenMapper&MockObject $accessTokenMapper;
+	private IAuthTokenProvider&MockObject $authTokenProvider;
+	private IUserManager&MockObject $userManager;
+	private ClientService $clientService;
+	private ICrypto&MockObject $crypto;
+	private LoggerInterface&MockObject $logger;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->clientMapper = $this->createMock(ClientMapper::class);
+		$this->secureRandom = $this->createMock(ISecureRandom::class);
+		$this->accessTokenMapper = $this->createMock(AccessTokenMapper::class);
+		$this->authTokenProvider = $this->createMock(IAuthTokenProvider::class);
+		$this->userManager = $this->createMock(IUserManager::class);
+		$this->crypto = $this->createMock(ICrypto::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
+		$this->clientService = new ClientService(
+			$this->secureRandom,
+			$this->crypto,
+			$this->clientMapper,
+			$this->userManager,
+			$this->authTokenProvider,
+			$this->logger,
+			$this->accessTokenMapper,
+		);
+	}
+
+	public function testAddClient(): void {
+		$this->secureRandom
+			->expects($this->exactly(2))
+			->method('generate')
+			->with(64, 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789')
+			->willReturnOnConsecutiveCalls(
+				'MySecret',
+				'MyClientIdentifier');
+
+		$this->crypto
+			->expects($this->once())
+			->method('calculateHMAC')
+			->willReturn('MyHashedSecret');
+
+		$client = new Client();
+		$client->setName('My Client Name');
+		$client->setRedirectUri('https://example.com/');
+		$client->setSecret(bin2hex('MyHashedSecret'));
+		$client->setClientIdentifier('MyClientIdentifier');
+
+		$this->clientMapper
+			->expects($this->once())
+			->method('insert')
+			->with($this->callback(function (Client $c) {
+				return $c->getName() === 'My Client Name'
+					&& $c->getRedirectUri() === 'https://example.com/'
+					&& $c->getSecret() === bin2hex('MyHashedSecret')
+					&& $c->getClientIdentifier() === 'MyClientIdentifier';
+			}))->willReturnCallback(function (Client $c) {
+				$c->setId(42);
+				return $c;
+			});
+
+		$result = $this->clientService->addClient('My Client Name', 'https://example.com/');
+
+		$this->assertEquals([
+			'id' => 42,
+			'name' => 'My Client Name',
+			'redirectUri' => 'https://example.com/',
+			'clientId' => 'MyClientIdentifier',
+			'clientSecret' => 'MySecret',
+		], $result);
+	}
+
+	public function testDeleteClient(): void {
+
+		$userManager = Server::get(IUserManager::class);
+		// count other users in the db before adding our own
+		$count = 0;
+		$function = function (IUser $user) use (&$count): void {
+			if ($user->getLastLogin() > 0) {
+				$count++;
+			}
+		};
+		$userManager->callForAllUsers($function);
+		$user1 = $userManager->createUser('test101', 'test101');
+		$user1->updateLastLoginTimestamp();
+		$tokenProviderMock = $this->getMockBuilder(IAuthTokenProvider::class)->getMock();
+
+		// One getTokenByUser call per user; we return no matching tokens here
+		// so invalidateTokenById is never invoked.
+		$tokenProviderMock
+			->expects($this->exactly($count + 1))
+			->method('getTokenByUser')
+			->willReturn([]);
+		$tokenProviderMock
+			->expects($this->never())
+			->method('invalidateTokenById');
+
+		$client = new Client();
+		$client->setId(123);
+		$client->setName('My Client Name');
+		$client->setRedirectUri('https://example.com/');
+		$client->setSecret(bin2hex('MyHashedSecret'));
+		$client->setClientIdentifier('MyClientIdentifier');
+
+		$this->clientMapper
+			->method('getByUid')
+			->with(123)
+			->willReturn($client);
+		$this->accessTokenMapper
+			->expects($this->once())
+			->method('deleteByClientId')
+			->with(123);
+		$this->clientMapper
+			->expects($this->once())
+			->method('delete')
+			->with($client);
+
+		$this->clientService = new ClientService(
+			$this->secureRandom,
+			$this->crypto,
+			$this->clientMapper,
+			$userManager,
+			$tokenProviderMock,
+			$this->logger,
+			$this->accessTokenMapper,
+		);
+
+		$this->clientService->deleteClient(123);
+		$user1->delete();
+	}
+
+	public function testDeleteClientPreservesWipePendingToken(): void {
+		$userManager = Server::get(IUserManager::class);
+		$user = $userManager->createUser('test_wipe_preserve', 'test_wipe_preserve');
+		$user->updateLastLoginTimestamp();
+
+		$client = new Client();
+		$client->setId(456);
+		$client->setName('My Client Name');
+		$client->setRedirectUri('https://example.com/');
+		$client->setSecret(bin2hex('MyHashedSecret'));
+		$client->setClientIdentifier('MyClientIdentifier');
+
+		// Token marked for wipe with a matching client name: must NOT be invalidated.
+		$wipeToken = $this->createMock(IToken::class);
+		$wipeToken->method('getId')->willReturn(11);
+		$wipeToken->method('getName')->willReturn('My Client Name');
+
+		// Regular token with matching name: must be invalidated.
+		$regularToken = $this->createMock(IToken::class);
+		$regularToken->method('getId')->willReturn(12);
+		$regularToken->method('getName')->willReturn('My Client Name');
+
+		// Non-matching name: must be left alone.
+		$otherToken = $this->createMock(IToken::class);
+		$otherToken->method('getId')->willReturn(13);
+		$otherToken->method('getName')->willReturn('Some Other Client');
+
+		$this->authTokenProvider
+			->method('getTokenByUser')
+			->willReturnCallback(function (string $uid) use ($wipeToken, $regularToken, $otherToken) {
+				return $uid === 'test_wipe_preserve'
+					? [$wipeToken, $regularToken, $otherToken]
+					: [];
+			});
+		// Wipe state is signalled via WipeTokenException from getTokenById.
+		$this->authTokenProvider
+			->method('getTokenById')
+			->willReturnCallback(function (int $id) use ($wipeToken, $regularToken) {
+				if ($id === 11) {
+					throw new WipeTokenException($wipeToken);
+				}
+				return $regularToken;
+			});
+		$this->authTokenProvider
+			->expects($this->once())
+			->method('invalidateTokenById')
+			->with('test_wipe_preserve', 12);
+
+		$this->clientMapper
+			->method('getByUid')
+			->with(456)
+			->willReturn($client);
+		$this->accessTokenMapper
+			->expects($this->once())
+			->method('deleteByClientId')
+			->with(456);
+		$this->clientMapper
+			->expects($this->once())
+			->method('delete')
+			->with($client);
+
+		$this->logger->expects($this->atLeastOnce())
+			->method('info')
+			->with($this->stringContains('Preserving token'), $this->callback(function (array $context) {
+				return ($context['tokenId'] ?? null) === 11
+					&& ($context['uid'] ?? null) === 'test_wipe_preserve';
+			}));
+
+		$clientService = new ClientService(
+			$this->secureRandom,
+			$this->crypto,
+			$this->clientMapper,
+			$userManager,
+			$this->authTokenProvider,
+			$this->logger,
+			$this->accessTokenMapper,
+		);
+
+		$clientService->deleteClient(456);
+	}
+}