Port oauth2 to new Entity system

apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php

@@ -15,12 +15,12 @@
 use OCP\DB\Exception;
 use Psr\Log\LoggerInterface;
 
-class CleanupExpiredAuthorizationCode extends TimedJob {
+final class CleanupExpiredAuthorizationCode extends TimedJob {
 
 	public function __construct(
 		ITimeFactory $timeFactory,
-		private AccessTokenMapper $accessTokenMapper,
-		private LoggerInterface $logger,
+		private readonly AccessTokenMapper $accessTokenMapper,
+		private readonly LoggerInterface $logger,
 	) {
 		parent::__construct($timeFactory);
 		// 30 days
@@ -30,14 +30,13 @@ public function __construct(
 
 	/**
 	 * @param mixed $argument
-	 * @inheritDoc
 	 */
 	#[\Override]
 	protected function run($argument): void {
 		try {
-			$this->accessTokenMapper->cleanupExpiredAuthorizationCode();
-		} catch (Exception $e) {
-			$this->logger->warning('Failed to cleanup tokens with expired authorization code', ['exception' => $e]);
+			$this->accessTokenMapper->cleanupExpiredAuthorizationCode($this->time);
+		} catch (Exception $exception) {
+			$this->logger->warning('Failed to cleanup tokens with expired authorization code', ['exception' => $exception]);
 		}
 	}
 }

apps/oauth2/lib/Command/AddClient.php

@@ -17,8 +17,9 @@
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 
-class AddClient extends Base {
+final class AddClient extends Base {
 	private const string ARGUMENT_CLIENT_NAME = 'client-name';
+
 	private const string ARGUMENT_CLIENT_REDIRECT_URI = 'client-redirect-uri';
 
 	public function __construct(

apps/oauth2/lib/Command/DeleteClient.php

@@ -17,7 +17,7 @@
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 
-class DeleteClient extends Base {
+final class DeleteClient extends Base {
 	private const string ARGUMENT_CLIENT_ID = 'client-id';
 
 	public function __construct(
@@ -53,6 +53,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			$output->writeln('<error>' . $exception->getMessage() . '</error>');
 			return Command::FAILURE;
 		}
+
 		return Command::SUCCESS;
 	}
 }

apps/oauth2/lib/Command/ImportLegacyOcClient.php

@@ -18,8 +18,9 @@
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 
-class ImportLegacyOcClient extends Command {
+final class ImportLegacyOcClient extends Command {
 	private const string ARGUMENT_CLIENT_ID = 'client-id';
+
 	private const string ARGUMENT_CLIENT_SECRET = 'client-secret';
 
 	public function __construct(
@@ -67,10 +68,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 		$hashedClientSecret = bin2hex($this->crypto->calculateHMAC($clientSecret));
 
 		$client = new Client();
-		$client->setName('ownCloud Desktop Client');
-		$client->setRedirectUri('http://localhost:*');
-		$client->setClientIdentifier($clientId);
-		$client->setSecret($hashedClientSecret);
+		$client->name = 'ownCloud Desktop Client';
+		$client->redirectUri = 'http://localhost:*';
+		$client->clientIdentifier = $clientId;
+		$client->secret = $hashedClientSecret;
+
 		$this->clientMapper->insert($client);
 
 		$output->writeln('<info>Client imported successfully</info>');

apps/oauth2/lib/Controller/LoginRedirectorController.php

@@ -29,7 +29,7 @@
 use OCP\Security\ISecureRandom;
 
 #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
-class LoginRedirectorController extends Controller {
+final class LoginRedirectorController extends Controller {
 	public function __construct(
 		string $appName,
 		IRequest $request,
@@ -64,7 +64,7 @@ public function authorize(
 	): TemplateResponse|RedirectResponse {
 		try {
 			$client = $this->clientMapper->getByIdentifier($client_id);
-		} catch (ClientNotFoundException $e) {
+		} catch (ClientNotFoundException) {
 			$params = [
 				'content' => $this->l->t('Your client is not authorized to connect. Please inform the administrator of your client.'),
 			];
@@ -73,21 +73,22 @@ public function authorize(
 
 		if ($response_type !== 'code') {
 			//Fail
-			$url = $client->getRedirectUri() . '?error=unsupported_response_type&state=' . \urlencode($state);
+			$url = $client->redirectUri . '?error=unsupported_response_type&state=' . \urlencode($state);
 			return new RedirectResponse($url);
 		}
 
 		$enableOcClients = $this->config->getSystemValueBool('oauth2.enable_oc_clients', false);
 
 		$providedRedirectUri = '';
-		if ($enableOcClients && $client->getRedirectUri() === 'http://localhost:*') {
+		if ($enableOcClients && $client->redirectUri === 'http://localhost:*') {
 			$providedRedirectUri = $redirect_uri;
 		}
 
 		$this->session->set('oauth.state', $state);
 
-		if (in_array($client->getName(), $this->appConfig->getValueArray('oauth2', 'skipAuthPickerApplications', []))) {
+		if (in_array($client->name, $this->appConfig->getValueArray('oauth2', 'skipAuthPickerApplications', []))) {
 			/** @see ClientFlowLoginController::showAuthPickerPage **/
+			/** @psalm-suppress DeprecatedMethod No Randomizer-based replacement is mockable in tests yet. */
 			$stateToken = $this->random->generate(
 				64,
 				ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_DIGITS
@@ -97,19 +98,20 @@ public function authorize(
 				'core.ClientFlowLogin.grantPage',
 				[
 					'stateToken' => $stateToken,
-					'clientIdentifier' => $client->getClientIdentifier(),
+					'clientIdentifier' => $client->clientIdentifier,
 					'providedRedirectUri' => $providedRedirectUri,
 				]
 			);
 		} else {
 			$targetUrl = $this->urlGenerator->linkToRouteAbsolute(
 				'core.ClientFlowLogin.showAuthPickerPage',
 				[
-					'clientIdentifier' => $client->getClientIdentifier(),
+					'clientIdentifier' => $client->clientIdentifier,
 					'providedRedirectUri' => $providedRedirectUri,
 				]
 			);
 		}
+
 		return new RedirectResponse($targetUrl);
 	}
 }

apps/oauth2/lib/Controller/OauthApiController.php

@@ -31,6 +31,7 @@
 use OCP\IDBConnection;
 use OCP\IRequest;
 use OCP\IURLGenerator;
+use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Security\Bruteforce\IThrottler;
 use OCP\Security\ICrypto;
@@ -40,9 +41,9 @@
 use Psr\Log\LoggerInterface;
 
 #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
-class OauthApiController extends Controller {
+final class OauthApiController extends Controller {
 	// the authorization code expires after 10 minutes
-	public const AUTHORIZATION_CODE_EXPIRES_AFTER = 10 * 60;
+	public const int AUTHORIZATION_CODE_EXPIRES_AFTER = 10 * 60;
 
 	public function __construct(
 		string $appName,
@@ -69,6 +70,7 @@ public function __construct(
 	 * Get a token
 	 *
 	 * @param 'authorization_code'|'refresh_token' $grant_type Token type that should be granted
+	 * @psalm-param string $grant_type
 	 * @param ?string $code Code of the flow
 	 * @param ?string $refresh_token Refresh token
 	 * @param ?string $client_id Client ID
@@ -101,9 +103,17 @@ public function getToken(
 			$code = $refresh_token;
 		}
 
+		if ($code === null) {
+			$response = new JSONResponse([
+				'error' => 'invalid_request',
+			], Http::STATUS_BAD_REQUEST);
+			$response->throttle(['invalid_request' => 'token not found']);
+			return $response;
+		}
+
 		try {
 			$accessToken = $this->accessTokenMapper->getByCode($code);
-		} catch (AccessTokenNotFoundException $e) {
+		} catch (AccessTokenNotFoundException) {
 			$response = new JSONResponse([
 				'error' => 'invalid_request',
 			], Http::STATUS_BAD_REQUEST);
@@ -113,7 +123,7 @@ public function getToken(
 
 		if ($grant_type === 'authorization_code') {
 			// check this token is in authorization code state
-			$deliveredTokenCount = $accessToken->getTokenCount();
+			$deliveredTokenCount = $accessToken->tokenCount;
 			if ($deliveredTokenCount > 0) {
 				$response = new JSONResponse([
 					'error' => 'invalid_request',
@@ -124,7 +134,7 @@ public function getToken(
 
 			// check authorization code expiration
 			$now = $this->timeFactory->now()->getTimestamp();
-			$codeCreatedAt = $accessToken->getCodeCreatedAt();
+			$codeCreatedAt = $accessToken->codeCreatedAt;
 			if ($codeCreatedAt < $now - self::AUTHORIZATION_CODE_EXPIRES_AFTER) {
 				// we know this token is not useful anymore
 				$this->accessTokenMapper->delete($accessToken);
@@ -139,47 +149,60 @@ public function getToken(
 		}
 
 		try {
-			$client = $this->clientMapper->getByUid($accessToken->getClientId());
-		} catch (ClientNotFoundException $e) {
+			$client = $this->clientMapper->getByUid($accessToken->clientId);
+		} catch (ClientNotFoundException) {
 			$response = new JSONResponse([
 				'error' => 'invalid_request',
 			], Http::STATUS_BAD_REQUEST);
-			$response->throttle(['invalid_request' => 'client not found', 'client_id' => $accessToken->getClientId()]);
+			$response->throttle(['invalid_request' => 'client not found', 'client_id' => $accessToken->clientId]);
 			return $response;
 		}
 
+		/**
+		 * @psalm-suppress NoInterfaceProperties, MixedArrayAccess
+		 * IRequest exposes $server via a magic @property-read for the request's $_SERVER superglobal.
+		 */
 		if (isset($this->request->server['PHP_AUTH_USER'])) {
-			$client_id = $this->request->server['PHP_AUTH_USER'];
-			$client_secret = $this->request->server['PHP_AUTH_PW'];
+			$client_id = (string)$this->request->server['PHP_AUTH_USER'];
+			$client_secret = (string)$this->request->server['PHP_AUTH_PW'];
+		}
+
+		if ($client_secret === null) {
+			$response = new JSONResponse([
+				'error' => 'invalid_client',
+			], Http::STATUS_BAD_REQUEST);
+			$response->throttle(['invalid_client' => 'client ID or secret does not match']);
+			return $response;
 		}
 
 		try {
-			$storedClientSecretHash = $client->getSecret();
+			$storedClientSecretHash = $client->secret;
 			$clientSecretHash = bin2hex($this->crypto->calculateHMAC($client_secret));
-		} catch (\Exception $e) {
-			$this->logger->error('OAuth client secret decryption error', ['exception' => $e]);
+		} catch (\Exception $exception) {
+			$this->logger->error('OAuth client secret decryption error', ['exception' => $exception]);
 			// we don't throttle here because it might not be a bruteforce attack
 			return new JSONResponse([
 				'error' => 'invalid_client',
 			], Http::STATUS_BAD_REQUEST);
 		}
+
 		// The client id and secret must match. Else we don't provide an access token!
-		if ($client->getClientIdentifier() !== $client_id || $storedClientSecretHash !== $clientSecretHash) {
+		if ($client->clientIdentifier !== $client_id || !hash_equals($storedClientSecretHash, $clientSecretHash)) {
 			$response = new JSONResponse([
 				'error' => 'invalid_client',
 			], Http::STATUS_BAD_REQUEST);
 			$response->throttle(['invalid_client' => 'client ID or secret does not match']);
 			return $response;
 		}
 
-		$decryptedToken = $this->crypto->decrypt($accessToken->getEncryptedToken(), $code);
+		$decryptedToken = $this->crypto->decrypt($accessToken->encryptedToken, $code);
 
 		// Obtain the appToken associated
 		try {
-			$appToken = $this->tokenProvider->getTokenById($accessToken->getTokenId());
+			$appToken = $this->tokenProvider->getTokenById($accessToken->tokenId);
 		} catch (ExpiredTokenException $e) {
 			$appToken = $e->getToken();
-		} catch (InvalidTokenException $e) {
+		} catch (InvalidTokenException) {
 			//We can't do anything...
 			$this->accessTokenMapper->delete($accessToken);
 			$response = new JSONResponse([
@@ -190,7 +213,9 @@ public function getToken(
 		}
 
 		// Rotate the apptoken (so the old one becomes invalid basically)
+		/** @psalm-suppress DeprecatedMethod No Randomizer-based replacement is mockable in tests yet. */
 		$newToken = $this->secureRandom->generate(72, ISecureRandom::CHAR_ALPHANUMERIC);
+		/** @psalm-suppress DeprecatedMethod No Randomizer-based replacement is mockable in tests yet. */
 		$newCode = $this->secureRandom->generate(128, ISecureRandom::CHAR_ALPHANUMERIC);
 		$newEncryptedToken = $this->crypto->encrypt($newToken, $newCode);
 		$redeemedThrottleReason = $grant_type === 'authorization_code'
@@ -200,7 +225,7 @@ public function getToken(
 		$this->db->beginTransaction();
 		try {
 			$updatedRows = $this->accessTokenMapper->rotateToken(
-				$accessToken->getId(),
+				$accessToken->id,
 				$code,
 				$newCode,
 				$newEncryptedToken,
@@ -228,15 +253,16 @@ public function getToken(
 			$this->tokenProvider->updateToken($appToken);
 
 			$this->db->commit();
-		} catch (\Throwable $e) {
+		} catch (\Throwable $throwable) {
 			if ($this->db->inTransaction()) {
 				$this->db->rollBack();
 			}
+
 			// rotate() and updateToken() write the auth token to the cache,
 			// so if we are past rotate() we must invalidate the new token
 			$this->tokenProvider->invalidateToken($newToken);
 
-			throw $e;
+			throw $throwable;
 		}
 
 		$this->throttler->resetDelay($this->request->getRemoteAddress(), 'login', ['user' => $appToken->getUID()]);
@@ -263,21 +289,23 @@ public function getToken(
 	 */
 	private function pushTokenToSecondary(IToken $appToken, string $newToken, ?int $expires): ?string {
 		$user = $this->userManager->get($appToken->getUID());
-		if ($user === null) {
+		if (!$user instanceof IUser) {
 			$this->logger->warning('could not push oauth token to secondary: unknown user', ['uid' => $appToken->getUID()]);
 			return null;
 		}
 
 		try {
 			/** @var IGlobalScaleService $globalScaleService */
 			$globalScaleService = $this->container->get(IGlobalScaleService::class);
-		} catch (ContainerExceptionInterface $e) {
-			$this->logger->warning('could not push oauth token to secondary: globalsiteselector is not available', ['exception' => $e]);
+		} catch (ContainerExceptionInterface $containerException) {
+			$this->logger->warning('could not push oauth token to secondary: globalsiteselector is not available', ['exception' => $containerException]);
 			return null;
 		}
 
 		try {
-			return $globalScaleService->sendToSecondary($user, $this->urlGenerator->linkToRoute('oauth2.OauthApi.pushToken'), [
+			/** @var non-empty-string $pushRouteUrl */
+			$pushRouteUrl = $this->urlGenerator->linkToRoute('oauth2.OauthApi.pushToken');
+			return $globalScaleService->sendToSecondary($user, $pushRouteUrl, [
 				'uid' => $appToken->getUID(),
 				'loginName' => $appToken->getLoginName(),
 				'name' => $appToken->getName(),
@@ -287,9 +315,10 @@ private function pushTokenToSecondary(IToken $appToken, string $newToken, ?int $
 				'expires' => $expires,
 				'token' => $newToken,
 			]);
-		} catch (\Exception $e) {
-			$this->logger->warning('could not push oauth token to secondary', ['exception' => $e]);
+		} catch (\Exception $exception) {
+			$this->logger->warning('could not push oauth token to secondary', ['exception' => $exception]);
 		}
+
 		return null;
 	}
 
@@ -311,8 +340,8 @@ public function pushToken(string $jwt): JSONResponse {
 		try {
 			/** @var IGlobalScaleService $globalScaleService */
 			$globalScaleService = $this->container->get(IGlobalScaleService::class);
-		} catch (ContainerExceptionInterface $e) {
-			$this->logger->warning('could not receive oauth token from primary: globalsiteselector is not available', ['exception' => $e]);
+		} catch (ContainerExceptionInterface $containerException) {
+			$this->logger->warning('could not receive oauth token from primary: globalsiteselector is not available', ['exception' => $containerException]);
 			$response = new JSONResponse([], Http::STATUS_BAD_REQUEST);
 			$response->throttle();
 			return $response;
@@ -337,8 +366,8 @@ public function pushToken(string $jwt): JSONResponse {
 				(array)$decoded['scope'],
 				$decoded['expires'] !== null ? (int)$decoded['expires'] : null,
 			);
-		} catch (\Exception $e) {
-			$this->logger->warning('could not create pushed oauth token', ['exception' => $e]);
+		} catch (\Exception $exception) {
+			$this->logger->warning('could not create pushed oauth token', ['exception' => $exception]);
 			$response = new JSONResponse([], Http::STATUS_BAD_REQUEST);
 			$response->throttle();
 			return $response;