diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index 036fffa13..23ce99f0f 100755 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -11,6 +11,8 @@ namespace OCA\OpenProject\Controller; +use OCA\OAuth2\Controller\SettingsController; +use OCA\OAuth2\Exceptions\ClientNotFoundException; use OCP\IURLGenerator; use OCP\IConfig; use OCP\IL10N; @@ -64,6 +66,10 @@ class ConfigController extends Controller { */ private $oauthService; + /** + * @var SettingsController + */ + private $oauthSettingsController; public function __construct(string $appName, IRequest $request, IConfig $config, @@ -73,6 +79,7 @@ public function __construct(string $appName, OpenProjectAPIService $openprojectAPIService, LoggerInterface $logger, OauthService $oauthService, + SettingsController $oauthSettingsController, ?string $userId) { parent::__construct($appName, $request); $this->config = $config; @@ -83,6 +90,7 @@ public function __construct(string $appName, $this->logger = $logger; $this->userId = $userId; $this->oauthService = $oauthService; + $this->oauthSettingsController = $oauthSettingsController; } /** @@ -346,7 +354,10 @@ private function deleteOauthClient(): void { ); if ($oauthClientInternalId !== '') { $id = (int) $oauthClientInternalId; - $this->oauthService->deleteClient($id); + try { + $this->oauthSettingsController->deleteClient($id); + } catch (ClientNotFoundException $e) { + } $this->config->deleteAppValue(Application::APP_ID, 'nc_oauth_client_id'); } } diff --git a/lib/Service/OauthService.php b/lib/Service/OauthService.php index dd5a9b68b..b979c577d 100644 --- a/lib/Service/OauthService.php +++ b/lib/Service/OauthService.php @@ -11,7 +11,6 @@ namespace OCA\OpenProject\Service; -use OCA\OAuth2\Db\AccessTokenMapper; use OCA\OAuth2\Db\Client; use OCA\OAuth2\Db\ClientMapper; use OCA\OAuth2\Exceptions\ClientNotFoundException; @@ -28,20 +27,15 @@ class OauthService { * @var ClientMapper */ private $clientMapper; - /** - * @var AccessTokenMapper - */ - private $accessTokenMapper; /** * Service to manipulate Nextcloud oauth clients */ public function __construct(ClientMapper $clientMapper, - AccessTokenMapper $accessTokenMapper, - ISecureRandom $secureRandom) { + ISecureRandom $secureRandom + ) { $this->secureRandom = $secureRandom; $this->clientMapper = $clientMapper; - $this->accessTokenMapper = $accessTokenMapper; } /** @@ -105,17 +99,4 @@ private function generateClientInfo(Client $client): array { 'clientSecret' => $client->getSecret(), ]; } - - /** - * @param int $id - * @return void - */ - public function deleteClient(int $id): void { - try { - $client = $this->clientMapper->getByUid($id); - $this->accessTokenMapper->deleteByClientId($id); - $this->clientMapper->delete($client); - } catch (ClientNotFoundException $e) { - } - } } diff --git a/tests/lib/Controller/ConfigControllerTest.php b/tests/lib/Controller/ConfigControllerTest.php index 91658acaa..35522f958 100644 --- a/tests/lib/Controller/ConfigControllerTest.php +++ b/tests/lib/Controller/ConfigControllerTest.php @@ -2,6 +2,7 @@ namespace OCA\OpenProject\Controller; +use OCA\OAuth2\Controller\SettingsController; use OCA\OpenProject\Service\OauthService; use OCA\OpenProject\Service\OpenProjectAPIService; use OCP\IConfig; @@ -129,7 +130,7 @@ public function testOauthRedirectSuccess():void { $apiServiceMock, $this->createMock(LoggerInterface::class), $this->createMock(OauthService::class), - 'testUser' + $this->createMock(SettingsController::class), 'testUser' ); $result = $configController->oauthRedirect('code', 'randomString'); $this->assertSame('https://nc.np/apps/files/', $result->getRedirectURL()); @@ -193,6 +194,7 @@ public function testOauthRedirectCorrectRedirectUrl( $this->createMock(OpenProjectAPIService::class), $this->createMock(LoggerInterface::class), $this->createMock(OauthService::class), + $this->createMock(SettingsController::class), 'testUser' ); $result = $configController->oauthRedirect('code', 'randomString'); @@ -226,6 +228,7 @@ public function testOauthRedirectWrongState() { $this->createMock(OpenProjectAPIService::class), $this->createMock(LoggerInterface::class), $this->createMock(OauthService::class), + $this->createMock(SettingsController::class), 'testUser' ); $configController->oauthRedirect('code', 'stateNotSameAsSaved'); @@ -292,6 +295,7 @@ public function testOauthRedirectCodeVerifier($codeVerifier, $valid) { $this->createMock(OpenProjectAPIService::class), $loggerMock, $this->createMock(OauthService::class), + $this->createMock(SettingsController::class), 'testUser' ); $configController->oauthRedirect('code', 'randomString'); @@ -360,6 +364,7 @@ public function testOauthRedirectSecret($clientSecret, $valid) { $this->createMock(OpenProjectAPIService::class), $loggerMock, $this->createMock(OauthService::class), + $this->createMock(SettingsController::class), 'testUser' ); $configController->oauthRedirect('code', 'randomString'); @@ -433,6 +438,7 @@ public function testOauthNoAccessTokenInResponse($oauthResponse, $expectedErrorM $apiServiceMock, $this->createMock(LoggerInterface::class), $this->createMock(OauthService::class), + $this->createMock(SettingsController::class), 'testUser' ); $configController->oauthRedirect('code', 'randomString'); @@ -513,6 +519,7 @@ public function testSetAdminConfigForDifferentAdminConfigStatus($credsToUpdate, $apiService, $this->createMock(LoggerInterface::class), $this->createMock(OauthService::class), + $this->createMock(SettingsController::class), 'test101' ); @@ -639,7 +646,9 @@ public function testSetAdminConfigClearUserDataChangeNCOauthClient( $this->user1 = $userManager->createUser('test101', 'test101'); $configMock = $this->getMockBuilder(IConfig::class)->getMock(); $oauthServiceMock = $this->createMock(OauthService::class); - + $oauthSettingsControllerMock = $this->getMockBuilder(SettingsController::class) + ->disableOriginalConstructor() + ->getMock(); if ($updateNCOAuthClient) { $configMock ->method('getAppValue') @@ -666,14 +675,14 @@ public function testSetAdminConfigClearUserDataChangeNCOauthClient( ->expects($this->once()) ->method('setClientRedirectUri') ->with(123, $credsToUpdate['oauth_instance_url']); - $oauthServiceMock + $oauthSettingsControllerMock ->expects($this->never()) ->method('deleteClient'); } else { // delete the client $oauthServiceMock ->expects($this->never()) ->method('setClientRedirectUri'); - $oauthServiceMock + $oauthSettingsControllerMock ->expects($this->once()) ->method('deleteClient') ->with(123); @@ -735,12 +744,12 @@ public function testSetAdminConfigClearUserDataChangeNCOauthClient( $apiService, $this->createMock(LoggerInterface::class), $oauthServiceMock, + $oauthSettingsControllerMock, 'test101' ); $configController->setAdminConfig($credsToUpdate); } - /** * @return void */ @@ -762,6 +771,7 @@ public function testSetAdminConfigNotAllowedConfigValues() { $apiService, $this->createMock(LoggerInterface::class), $oauthServiceMock, + $this->createMock(SettingsController::class), 'test101' );