diff --git a/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php b/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php index 3d7b26546c1..83735a0afdf 100644 --- a/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php +++ b/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php @@ -19,8 +19,8 @@ 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 @@ -35,8 +35,8 @@ final class CleanupExpiredAuthorizationCode extends TimedJob { protected function run($argument): void { try { $this->accessTokenMapper->cleanupExpiredAuthorizationCode($this->time); - } catch (Exception $e) { - $this->logger->warning('Failed to cleanup tokens with expired authorization code', ['exception' => $e]); + } catch (Exception $exception) { + $this->logger->warning('Failed to cleanup tokens with expired authorization code', ['exception' => $exception]); } } } diff --git a/apps/oauth2/lib/Command/AddClient.php b/apps/oauth2/lib/Command/AddClient.php index 8962f1e4df4..639d1bc5881 100644 --- a/apps/oauth2/lib/Command/AddClient.php +++ b/apps/oauth2/lib/Command/AddClient.php @@ -19,6 +19,7 @@ use Symfony\Component\Console\Output\OutputInterface; 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( diff --git a/apps/oauth2/lib/Command/DeleteClient.php b/apps/oauth2/lib/Command/DeleteClient.php index 12440aeb477..0a2f54c248b 100644 --- a/apps/oauth2/lib/Command/DeleteClient.php +++ b/apps/oauth2/lib/Command/DeleteClient.php @@ -53,6 +53,7 @@ final class DeleteClient extends Base { $output->writeln('' . $exception->getMessage() . ''); return Command::FAILURE; } + return Command::SUCCESS; } } diff --git a/apps/oauth2/lib/Command/ImportLegacyOcClient.php b/apps/oauth2/lib/Command/ImportLegacyOcClient.php index 5a369010d1e..7408a8925e8 100644 --- a/apps/oauth2/lib/Command/ImportLegacyOcClient.php +++ b/apps/oauth2/lib/Command/ImportLegacyOcClient.php @@ -20,6 +20,7 @@ use Symfony\Component\Console\Output\OutputInterface; final class ImportLegacyOcClient extends Command { private const string ARGUMENT_CLIENT_ID = 'client-id'; + private const string ARGUMENT_CLIENT_SECRET = 'client-secret'; public function __construct( @@ -71,6 +72,7 @@ final class ImportLegacyOcClient extends Command { $client->redirectUri = 'http://localhost:*'; $client->clientIdentifier = $clientId; $client->secret = $hashedClientSecret; + $this->clientMapper->insert($client); $output->writeln('Client imported successfully'); diff --git a/apps/oauth2/lib/Controller/LoginRedirectorController.php b/apps/oauth2/lib/Controller/LoginRedirectorController.php index e180f3b65f5..ab5a2490666 100644 --- a/apps/oauth2/lib/Controller/LoginRedirectorController.php +++ b/apps/oauth2/lib/Controller/LoginRedirectorController.php @@ -64,7 +64,7 @@ final class LoginRedirectorController extends Controller { ): 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.'), ]; @@ -111,6 +111,7 @@ final class LoginRedirectorController extends Controller { ] ); } + return new RedirectResponse($targetUrl); } } diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index a7a8b420f14..94416464aa4 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -31,6 +31,7 @@ use OCP\GlobalScale\IGlobalScaleService; use OCP\IDBConnection; use OCP\IRequest; use OCP\IURLGenerator; +use OCP\IUser; use OCP\IUserManager; use OCP\Security\Bruteforce\IThrottler; use OCP\Security\ICrypto; @@ -112,7 +113,7 @@ final class OauthApiController extends Controller { try { $accessToken = $this->accessTokenMapper->getByCode($code); - } catch (AccessTokenNotFoundException $e) { + } catch (AccessTokenNotFoundException) { $response = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); @@ -149,7 +150,7 @@ final class OauthApiController extends Controller { try { $client = $this->clientMapper->getByUid($accessToken->clientId); - } catch (ClientNotFoundException $e) { + } catch (ClientNotFoundException) { $response = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); @@ -177,13 +178,14 @@ final class OauthApiController extends Controller { try { $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->clientIdentifier !== $client_id || $storedClientSecretHash !== $clientSecretHash) { $response = new JSONResponse([ @@ -200,7 +202,7 @@ final class OauthApiController extends Controller { $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([ @@ -251,15 +253,16 @@ final class OauthApiController extends Controller { $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()]); @@ -286,7 +289,7 @@ final class OauthApiController extends Controller { */ 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; } @@ -294,8 +297,8 @@ final class OauthApiController extends Controller { 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; } @@ -312,9 +315,10 @@ final class OauthApiController extends Controller { '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; } @@ -336,8 +340,8 @@ final class OauthApiController extends Controller { 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; @@ -362,8 +366,8 @@ final class OauthApiController extends Controller { (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; diff --git a/apps/oauth2/lib/Controller/SettingsController.php b/apps/oauth2/lib/Controller/SettingsController.php index 4a46f0f7c83..2ab6457b1d3 100644 --- a/apps/oauth2/lib/Controller/SettingsController.php +++ b/apps/oauth2/lib/Controller/SettingsController.php @@ -21,7 +21,7 @@ final class SettingsController extends Controller { public function __construct( string $appName, IRequest $request, - private IL10N $l, + private readonly IL10N $l, private readonly ClientService $clientService, ) { parent::__construct($appName, $request); diff --git a/apps/oauth2/lib/Db/AccessTokenMapper.php b/apps/oauth2/lib/Db/AccessTokenMapper.php index 50139a994dd..408aefdda93 100644 --- a/apps/oauth2/lib/Db/AccessTokenMapper.php +++ b/apps/oauth2/lib/Db/AccessTokenMapper.php @@ -12,20 +12,17 @@ namespace OCA\OAuth2\Db; use OCA\OAuth2\Controller\OauthApiController; use OCA\OAuth2\Exceptions\AccessTokenNotFoundException; use OCP\AppFramework\Db\DoesNotExistException; -use OCP\AppFramework\Db\IMapperException; -use OCP\AppFramework\Db\QBMapper; use OCP\AppFramework\ORM\Repository; use OCP\AppFramework\Utility\ITimeFactory; use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\IDBConnection; /** * @template-extends Repository * @psalm-suppress ClassMustBeFinal For unit tests */ class AccessTokenMapper extends Repository { - const string entityClass = AccessToken::class; + public const string entityClass = AccessToken::class; /** * @throws AccessTokenNotFoundException @@ -35,8 +32,8 @@ class AccessTokenMapper extends Repository { return $this->findOneBy([ 'hashedCode' => hash('sha512', $code), ]); - } catch (DoesNotExistException $e) { - throw new AccessTokenNotFoundException('Could not find access token', 0, $e); + } catch (DoesNotExistException $doesNotExistException) { + throw new AccessTokenNotFoundException('Could not find access token', 0, $doesNotExistException); } } diff --git a/apps/oauth2/lib/Db/Client.php b/apps/oauth2/lib/Db/Client.php index 92d6269a6d0..43c986b0ff9 100644 --- a/apps/oauth2/lib/Db/Client.php +++ b/apps/oauth2/lib/Db/Client.php @@ -9,7 +9,6 @@ declare(strict_types=1); namespace OCA\OAuth2\Db; - use OCP\AppFramework\ORM\Attribute\Column; use OCP\AppFramework\ORM\Attribute\Entity; use OCP\AppFramework\ORM\Attribute\Id; diff --git a/apps/oauth2/lib/Db/ClientMapper.php b/apps/oauth2/lib/Db/ClientMapper.php index b8bf91b46ae..6c371a25337 100644 --- a/apps/oauth2/lib/Db/ClientMapper.php +++ b/apps/oauth2/lib/Db/ClientMapper.php @@ -11,11 +11,7 @@ namespace OCA\OAuth2\Db; use OCA\OAuth2\Exceptions\ClientNotFoundException; use OCP\AppFramework\Db\DoesNotExistException; -use OCP\AppFramework\Db\IMapperException; -use OCP\AppFramework\Db\QBMapper; use OCP\AppFramework\ORM\Repository; -use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\IDBConnection; /** * @template-extends Repository @@ -25,8 +21,6 @@ class ClientMapper extends Repository { public const string entityClass = Client::class; /** - * @param string $clientIdentifier - * @return Client * @throws ClientNotFoundException */ public function getByIdentifier(string $clientIdentifier): Client { @@ -34,14 +28,13 @@ class ClientMapper extends Repository { return $this->findOneBy([ 'clientIdentifier' => $clientIdentifier, ]); - } catch (DoesNotExistException $e) { - throw new ClientNotFoundException('Could not find client ' . $clientIdentifier, previous: $e); + } catch (DoesNotExistException $doesNotExistException) { + throw new ClientNotFoundException('Could not find client ' . $clientIdentifier, $doesNotExistException->getCode(), previous: $doesNotExistException); } } /** * @param int $id internal id of the client - * @return Client * @throws ClientNotFoundException */ public function getByUid(int $id): Client { @@ -49,8 +42,8 @@ class ClientMapper extends Repository { return $this->findOneBy([ 'id' => $id, ]); - } catch (DoesNotExistException $e) { - throw new ClientNotFoundException('could not find client with id ' . $id, previous: $e); + } catch (DoesNotExistException $doesNotExistException) { + throw new ClientNotFoundException('could not find client with id ' . $id, $doesNotExistException->getCode(), previous: $doesNotExistException); } } diff --git a/apps/oauth2/lib/Migration/SetTokenExpiration.php b/apps/oauth2/lib/Migration/SetTokenExpiration.php index cca900f42ea..b8f3491a901 100644 --- a/apps/oauth2/lib/Migration/SetTokenExpiration.php +++ b/apps/oauth2/lib/Migration/SetTokenExpiration.php @@ -16,7 +16,7 @@ use OCP\IDBConnection; use OCP\Migration\IOutput; use OCP\Migration\IRepairStep; -final class SetTokenExpiration implements IRepairStep { +final readonly class SetTokenExpiration implements IRepairStep { public function __construct( private IDBConnection $connection, @@ -44,10 +44,11 @@ final class SetTokenExpiration implements IRepairStep { $appToken = $this->tokenProvider->getTokenById($tokenId); $appToken->setExpires($this->time->getTime() + 3600); $this->tokenProvider->updateToken($appToken); - } catch (InvalidTokenException $e) { + } catch (InvalidTokenException) { //Skip this token } } + $cursor->closeCursor(); } } diff --git a/apps/oauth2/lib/Migration/Version010401Date20181207190718.php b/apps/oauth2/lib/Migration/Version010401Date20181207190718.php index 3d1a984f09a..2adb279c548 100644 --- a/apps/oauth2/lib/Migration/Version010401Date20181207190718.php +++ b/apps/oauth2/lib/Migration/Version010401Date20181207190718.php @@ -71,6 +71,7 @@ final class Version010401Date20181207190718 extends SimpleMigrationStep { $table->addUniqueIndex(['hashed_code'], 'oauth2_access_hash_idx'); $table->addIndex(['client_id'], 'oauth2_access_client_id_idx'); } + return $schema; } } diff --git a/apps/oauth2/lib/Migration/Version010402Date20190107124745.php b/apps/oauth2/lib/Migration/Version010402Date20190107124745.php index 09a2ae2403a..096496adb3f 100644 --- a/apps/oauth2/lib/Migration/Version010402Date20190107124745.php +++ b/apps/oauth2/lib/Migration/Version010402Date20190107124745.php @@ -17,9 +17,7 @@ use OCP\Migration\SimpleMigrationStep; final class Version010402Date20190107124745 extends SimpleMigrationStep { /** - * @param IOutput $output * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` - * @param array $options * @return null|ISchemaWrapper */ #[\Override] diff --git a/apps/oauth2/lib/Migration/Version011601Date20230522143227.php b/apps/oauth2/lib/Migration/Version011601Date20230522143227.php index 08d706f6e23..8e633ed42a2 100644 --- a/apps/oauth2/lib/Migration/Version011601Date20230522143227.php +++ b/apps/oauth2/lib/Migration/Version011601Date20230522143227.php @@ -19,8 +19,8 @@ use OCP\Security\ICrypto; final class Version011601Date20230522143227 extends SimpleMigrationStep { public function __construct( - private IDBConnection $connection, - private ICrypto $crypto, + private readonly IDBConnection $connection, + private readonly ICrypto $crypto, ) { } @@ -61,6 +61,7 @@ final class Version011601Date20230522143227 extends SimpleMigrationStep { $qbUpdate->setParameter('updateId', $id, IQueryBuilder::PARAM_INT); $qbUpdate->executeStatement(); } + $req->closeCursor(); } } diff --git a/apps/oauth2/lib/Migration/Version011602Date20230613160650.php b/apps/oauth2/lib/Migration/Version011602Date20230613160650.php index 6b01a7505cd..fee395d8f45 100644 --- a/apps/oauth2/lib/Migration/Version011602Date20230613160650.php +++ b/apps/oauth2/lib/Migration/Version011602Date20230613160650.php @@ -15,10 +15,6 @@ use OCP\Migration\SimpleMigrationStep; final class Version011602Date20230613160650 extends SimpleMigrationStep { - public function __construct( - ) { - } - #[\Override] public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) { $schema = $schemaClosure(); diff --git a/apps/oauth2/lib/Migration/Version011603Date20230620111039.php b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php index b23987c2573..4e2c308c92b 100644 --- a/apps/oauth2/lib/Migration/Version011603Date20230620111039.php +++ b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php @@ -20,7 +20,7 @@ use OCP\Migration\SimpleMigrationStep; final class Version011603Date20230620111039 extends SimpleMigrationStep { public function __construct( - private IDBConnection $connection, + private readonly IDBConnection $connection, ) { } @@ -39,6 +39,7 @@ final class Version011603Date20230620111039 extends SimpleMigrationStep { ]); $dbChanged = true; } + if (!$table->hasColumn('token_count')) { $table->addColumn('token_count', Types::BIGINT, [ 'notnull' => true, @@ -47,10 +48,12 @@ final class Version011603Date20230620111039 extends SimpleMigrationStep { ]); $dbChanged = true; } + if (!$table->hasIndex('oauth2_tk_c_created_idx')) { $table->addIndex(['token_count', 'code_created_at'], 'oauth2_tk_c_created_idx'); $dbChanged = true; } + if ($dbChanged) { return $schema; } diff --git a/apps/oauth2/lib/Migration/Version011901Date20240829164356.php b/apps/oauth2/lib/Migration/Version011901Date20240829164356.php index 1c0d8978b39..1c768c3a1bb 100644 --- a/apps/oauth2/lib/Migration/Version011901Date20240829164356.php +++ b/apps/oauth2/lib/Migration/Version011901Date20240829164356.php @@ -19,8 +19,8 @@ use OCP\Security\ICrypto; final class Version011901Date20240829164356 extends SimpleMigrationStep { public function __construct( - private IDBConnection $connection, - private ICrypto $crypto, + private readonly IDBConnection $connection, + private readonly ICrypto $crypto, ) { } @@ -46,6 +46,7 @@ final class Version011901Date20240829164356 extends SimpleMigrationStep { $qbUpdate->setParameter('updateId', $id, IQueryBuilder::PARAM_INT); $qbUpdate->executeStatement(); } + $req->closeCursor(); } } diff --git a/apps/oauth2/lib/Service/ClientService.php b/apps/oauth2/lib/Service/ClientService.php index b8c41e845c3..fc393d829b7 100644 --- a/apps/oauth2/lib/Service/ClientService.php +++ b/apps/oauth2/lib/Service/ClientService.php @@ -22,17 +22,17 @@ use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; use Psr\Log\LoggerInterface; -final class ClientService { +final readonly class ClientService { public const string 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, + private ISecureRandom $secureRandom, + private ICrypto $crypto, + private ClientMapper $clientMapper, + private IUserManager $userManager, + private IAuthTokenProvider $tokenProvider, + private LoggerInterface $logger, + private AccessTokenMapper $accessTokenMapper, ) { } @@ -79,6 +79,7 @@ final class ClientService { if ($token->getName() !== $client->name) { continue; } + try { $this->tokenProvider->getTokenById($token->getId()); } catch (WipeTokenException) { @@ -90,6 +91,7 @@ final class ClientService { } catch (InvalidTokenException) { // Token already invalid; let invalidateTokenById handle it. } + $this->tokenProvider->invalidateTokenById($user->getUID(), $token->getId()); } }); diff --git a/apps/oauth2/lib/Settings/Admin.php b/apps/oauth2/lib/Settings/Admin.php index 3579160eaa8..7644fea878e 100644 --- a/apps/oauth2/lib/Settings/Admin.php +++ b/apps/oauth2/lib/Settings/Admin.php @@ -16,7 +16,7 @@ use OCP\IURLGenerator; use OCP\Settings\ISettings; use OCP\Util; -final class Admin implements ISettings { +final readonly class Admin implements ISettings { public function __construct( private IInitialState $initialState, @@ -39,6 +39,7 @@ final class Admin implements ISettings { 'clientSecret' => '', ]; } + $this->initialState->provideInitialState('clients', $result); $this->initialState->provideInitialState('oauth2-doc-link', $this->urlGenerator->linkToDocs('admin-oauth2')); diff --git a/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php b/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php index eae2b72ac30..c87dc71f720 100644 --- a/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php +++ b/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php @@ -1,5 +1,7 @@ crypto ->method('calculateHMAC') - ->with($this->callback(function (string $text) { - return $text === 'clientSecret' || $text === 'invalidClientSecret'; - })) - ->willReturnCallback(function (string $text) { - return $text === 'clientSecret' + ->with($this->callback(fn (string $text): bool => $text === 'clientSecret' || $text === 'invalidClientSecret')) + ->willReturnCallback(fn (string $text): string => $text === 'clientSecret' ? 'hashedClientSecret' - : 'hashedInvalidClientSecret'; - }); + : 'hashedInvalidClientSecret'); $client = new Client(); $client->clientIdentifier = 'clientId'; @@ -369,9 +382,7 @@ final class OauthApiControllerTest extends TestCase { ->with($accessToken); $this->secureRandom->method('generate') - ->willReturnCallback(function (int $len) { - return 'random' . $len; - }); + ->willReturnCallback(fn (int $len): string => 'random' . $len); $this->tokenProvider->expects($this->once()) ->method('rotate') @@ -399,9 +410,7 @@ final class OauthApiControllerTest extends TestCase { $this->tokenProvider->expects($this->once()) ->method('updateToken') ->with( - $this->callback(function (PublicKeyToken $token) { - return $token->getExpires() === 4600; - }) + $this->callback(fn (PublicKeyToken $token): bool => $token->getExpires() === 4600) ); $this->crypto->method('encrypt') @@ -479,9 +488,7 @@ final class OauthApiControllerTest extends TestCase { ->with($accessToken); $this->secureRandom->method('generate') - ->willReturnCallback(function (int $len) { - return 'random' . $len; - }); + ->willReturnCallback(fn (int $len): string => 'random' . $len); $this->tokenProvider->expects($this->once()) ->method('rotate') @@ -509,9 +516,7 @@ final class OauthApiControllerTest extends TestCase { $this->tokenProvider->expects($this->once()) ->method('updateToken') ->with( - $this->callback(function (PublicKeyToken $token) { - return $token->getExpires() === 4600; - }) + $this->callback(fn (PublicKeyToken $token): bool => $token->getExpires() === 4600) ); $this->crypto->method('encrypt') @@ -592,9 +597,7 @@ final class OauthApiControllerTest extends TestCase { ->with($accessToken); $this->secureRandom->method('generate') - ->willReturnCallback(function (int $len) { - return 'random' . $len; - }); + ->willReturnCallback(fn (int $len): string => 'random' . $len); $this->tokenProvider->expects($this->once()) ->method('rotate') @@ -622,9 +625,7 @@ final class OauthApiControllerTest extends TestCase { $this->tokenProvider->expects($this->once()) ->method('updateToken') ->with( - $this->callback(function (PublicKeyToken $token) { - return $token->getExpires() === 4600; - }) + $this->callback(fn (PublicKeyToken $token): bool => $token->getExpires() === 4600) ); $this->crypto->method('encrypt') @@ -703,9 +704,7 @@ final class OauthApiControllerTest extends TestCase { ->willReturn($appToken); $this->secureRandom->method('generate') - ->willReturnCallback(function (int $len) { - return 'random' . $len; - }); + ->willReturnCallback(fn (int $len): string => 'random' . $len); $this->tokenProvider->expects($this->never()) ->method('rotate'); @@ -793,9 +792,7 @@ final class OauthApiControllerTest extends TestCase { ->willReturn($appToken); $this->secureRandom->method('generate') - ->willReturnCallback(function (int $len) { - return 'random' . $len; - }); + ->willReturnCallback(fn (int $len): string => 'random' . $len); $this->time->method('getTime')->willReturn(1000); $this->accessTokenMapper->method('rotateToken')->willReturn(1); $this->request->method('getRemoteAddress')->willReturn('1.2.3.4'); @@ -869,7 +866,7 @@ final class OauthApiControllerTest extends TestCase { $this->globalScaleConfig->method('isGlobalScaleEnabled')->willReturn(true); $this->globalScaleConfig->method('isPrimary')->willReturn(true); - $user = $this->createMock(IUser::class); + $user = $this->createStub(IUser::class); $this->userManager->method('get')->with('userId')->willReturn($user); $this->container->method('get') @@ -888,7 +885,7 @@ final class OauthApiControllerTest extends TestCase { $this->globalScaleConfig->method('isGlobalScaleEnabled')->willReturn(true); $this->globalScaleConfig->method('isPrimary')->willReturn(true); - $user = $this->createMock(IUser::class); + $user = $this->createStub(IUser::class); $this->userManager->method('get')->with('userId')->willReturn($user); $this->urlGenerator->method('linkToRoute') @@ -928,7 +925,7 @@ final class OauthApiControllerTest extends TestCase { $this->globalScaleConfig->method('isGlobalScaleEnabled')->willReturn(true); $this->globalScaleConfig->method('isPrimary')->willReturn(true); - $user = $this->createMock(IUser::class); + $user = $this->createStub(IUser::class); $this->userManager->method('get')->with('userId')->willReturn($user); $globalScaleService = $this->createMock(IGlobalScaleService::class); diff --git a/apps/oauth2/tests/Db/AccessTokenMapperTest.php b/apps/oauth2/tests/Db/AccessTokenMapperTest.php index 2e137d601f0..8eecc597ba5 100644 --- a/apps/oauth2/tests/Db/AccessTokenMapperTest.php +++ b/apps/oauth2/tests/Db/AccessTokenMapperTest.php @@ -1,5 +1,7 @@ tokenId = time(); $token->encryptedToken = 'MyEncryptedToken'; $token->hashedCode = hash('sha512', 'MyAwesomeToken'); + $this->accessTokenMapper->insert($token); $result = $this->accessTokenMapper->getByCode('MyAwesomeToken'); @@ -46,6 +49,7 @@ final class AccessTokenMapperTest extends TestCase { $token->tokenId = time(); $token->encryptedToken = 'MyEncryptedToken'; $token->hashedCode = hash('sha512', 'MyAwesomeToken'); + $this->accessTokenMapper->insert($token); $this->accessTokenMapper->deleteByClientId(1234); $this->accessTokenMapper->getByCode('MyAwesomeToken'); diff --git a/apps/oauth2/tests/Db/ClientMapperTest.php b/apps/oauth2/tests/Db/ClientMapperTest.php index 9e9cb44a332..1b6c7ccdfab 100644 --- a/apps/oauth2/tests/Db/ClientMapperTest.php +++ b/apps/oauth2/tests/Db/ClientMapperTest.php @@ -1,5 +1,7 @@ name = 'Client Name'; $client->redirectUri = 'https://example.com/'; $client->secret = 'TotallyNotSecret'; + $this->clientMapper->insert($client); $this->assertEquals($client, $this->clientMapper->getByIdentifier('MyAwesomeClientIdentifier')); } @@ -54,6 +57,7 @@ final class ClientMapperTest extends TestCase { $client->name = 'Client Name'; $client->redirectUri = 'https://example.com/'; $client->secret = 'TotallyNotSecret'; + $this->clientMapper->insert($client); $this->assertEquals($client, $this->clientMapper->getByUid($client->id)); } @@ -74,6 +78,7 @@ final class ClientMapperTest extends TestCase { $client->name = 'Client Name'; $client->redirectUri = 'https://example.com/'; $client->secret = 'b81dc8e2dc178817bf28ca7b37265aa96559ca02e6dcdeb74b42221d096ed5ef63681e836ae0ba1077b5fb5e6c2fa7748c78463f66fe0110c8dcb8dd7eb0305b16d0cd993e2ae275879994a2abf88c68|e466d9befa6b0102341458e45ecd551a|013af9e277374483123437f180a3b0371a411ad4f34c451547909769181a7d7cc191f0f5c2de78376d124dd7751b8c9660aabdd913f5e071fc6b819ba2e3d919|3'; + $this->clientMapper->insert($client); $this->assertTrue(true); } diff --git a/apps/oauth2/tests/Service/ClientServiceTest.php b/apps/oauth2/tests/Service/ClientServiceTest.php index 991f31cf706..5cb4c90e507 100644 --- a/apps/oauth2/tests/Service/ClientServiceTest.php +++ b/apps/oauth2/tests/Service/ClientServiceTest.php @@ -1,5 +1,7 @@ clientMapper ->expects($this->once()) ->method('insert') - ->with($this->callback(function (Client $c) { - return $c->name === 'My Client Name' + ->with($this->callback(fn (Client $c): bool => $c->name === 'My Client Name' && $c->redirectUri === 'https://example.com/' && $c->secret === bin2hex('MyHashedSecret') - && $c->clientIdentifier === 'MyClientIdentifier'; - }))->willReturnCallback(function (Client $c) { - $c->id = 42; - return $c; - }); + && $c->clientIdentifier === 'MyClientIdentifier'))->willReturnCallback(function (Client $c): Client { + $c->id = 42; + return $c; + }); $result = $this->clientService->addClient('My Client Name', 'https://example.com/'); @@ -107,7 +114,7 @@ final class ClientServiceTest extends TestCase { $count = 0; $function = function (IUser $user) use (&$count): void { if ($user->getLastLogin() > 0) { - $count++; + ++$count; } }; $userManager->callForAllUsers($function); @@ -157,6 +164,7 @@ final class ClientServiceTest extends TestCase { ); $this->clientService->deleteClient(123); + $user1->delete(); } @@ -190,11 +198,9 @@ final class ClientServiceTest extends TestCase { $this->authTokenProvider ->method('getTokenByUser') - ->willReturnCallback(function (string $uid) use ($wipeToken, $regularToken, $otherToken) { - return $uid === 'test_wipe_preserve' + ->willReturnCallback(fn (string $uid): array => $uid === 'test_wipe_preserve' ? [$wipeToken, $regularToken, $otherToken] - : []; - }); + : []); // Wipe state is signalled via WipeTokenException from getTokenById. $this->authTokenProvider ->method('getTokenById') @@ -202,6 +208,7 @@ final class ClientServiceTest extends TestCase { if ($id === 11) { throw new WipeTokenException($wipeToken); } + return $regularToken; }); $this->authTokenProvider @@ -224,10 +231,8 @@ final class ClientServiceTest extends TestCase { $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'; - })); + ->with($this->stringContains('Preserving token'), $this->callback(fn (array $context): bool => ($context['tokenId'] ?? null) === 11 + && ($context['uid'] ?? null) === 'test_wipe_preserve')); $clientService = new ClientService( $this->secureRandom, diff --git a/apps/oauth2/tests/Settings/AdminTest.php b/apps/oauth2/tests/Settings/AdminTest.php index 8839f76cde0..6ef24dc04f3 100644 --- a/apps/oauth2/tests/Settings/AdminTest.php +++ b/apps/oauth2/tests/Settings/AdminTest.php @@ -1,5 +1,7 @@ withAutoloadPaths([ // ensure rector properly autoload the public interfaces