From ec70c2625e517eafa80fcef5272a10867f248ddb Mon Sep 17 00:00:00 2001 From: Saw-jan Date: Wed, 10 Jun 2026 17:07:35 +0545 Subject: [PATCH 1/6] fix: do not create oauth client during oidc setup Signed-off-by: Saw-jan --- lib/Controller/ConfigController.php | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index 8d924bdf3..36eb7f1a4 100755 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -651,29 +651,38 @@ public function setUpIntegration(?array $values): DataResponse { // check all required settings $this->settingsService->validateAdminSettingsForm($values, true); - $status = $this->setIntegrationConfig($values); - $result = $this->recreateOauthClientInformation(); - if ($status['oPOAuthTokenRevokeStatus'] !== '') { - $result['openproject_revocation_status'] = $status['oPOAuthTokenRevokeStatus']; + $setup = $this->setIntegrationConfig($values); + $response = ['status' => $setup['status']]; + + if ($values['authorization_method'] === OpenProjectAPIService::AUTH_METHOD_OAUTH) { + $response = \array_merge($response, $this->recreateOauthClientInformation()); } - if ($status['oPUserAppPassword'] !== null) { - $result['openproject_user_app_password'] = $status['oPUserAppPassword']; + + if ($setup['oPOAuthTokenRevokeStatus']) { + $response['openproject_revocation_status'] = $setup['oPOAuthTokenRevokeStatus']; } - return new DataResponse($result); + if ($setup['oPUserAppPassword']) { + $response['openproject_user_app_password'] = $setup['oPUserAppPassword']; + } + return new DataResponse($response); } catch (OpenprojectGroupfolderSetupConflictException $e) { return new DataResponse([ + 'status' => false, 'error' => $this->l->t($e->getMessage()), ], Http::STATUS_CONFLICT); } catch (NoUserException $e) { return new DataResponse([ + 'status' => false, 'error' => $this->l->t($e->getMessage()) ], Http::STATUS_NOT_FOUND); } catch (InvalidArgumentException $e) { return new DataResponse([ + 'status' => false, "error" => $e->getMessage() ], Http::STATUS_BAD_REQUEST); } catch (\Exception $e) { return new DataResponse([ + 'status' => false, "error" => $e->getMessage() ], Http::STATUS_INTERNAL_SERVER_ERROR); } From 93397fef287ee507a71973e313641b985f5650ab Mon Sep 17 00:00:00 2001 From: Saw-jan Date: Wed, 10 Jun 2026 17:48:06 +0545 Subject: [PATCH 2/6] test: add phpunit tests Signed-off-by: Saw-jan --- tests/lib/Controller/ConfigControllerTest.php | 122 +++++++++++++++++- 1 file changed, 118 insertions(+), 4 deletions(-) diff --git a/tests/lib/Controller/ConfigControllerTest.php b/tests/lib/Controller/ConfigControllerTest.php index 851418f1c..8a9268011 100644 --- a/tests/lib/Controller/ConfigControllerTest.php +++ b/tests/lib/Controller/ConfigControllerTest.php @@ -154,11 +154,10 @@ private function getConfigControllerMock( array $constructParams = [], ): MockObject { $constructArgs = $this->getConfigControllerConstructArgs($constructParams); - $mock = $this->getMockBuilder(ConfigController::class) + return $this->getMockBuilder(ConfigController::class) ->setConstructorArgs($constructArgs) ->onlyMethods($mockMethods) ->getMock(); - return $mock; } public function testOauthRedirectSuccess():void { @@ -1875,8 +1874,8 @@ public function integrationDefaultSettingsProvider() { } /** - * @param array $oldCreds - * @param array $credsToUpdate + * @param array $settings + * @param array $expectedSettings * @return void * @dataProvider integrationDefaultSettingsProvider */ @@ -1899,4 +1898,119 @@ public function testSetUpIntegrationDefaultSettings(array $settings, array $expe $configController = new ConfigController(...$constructArgs); $configController->setUpIntegration($settings); } + + /** + * @return array + */ + public function setUpIntegrationSuccessProvider(): array { + $defaultSettings = [ + 'openproject_instance_url' => 'https://test.example.com', + 'default_enable_navigation' => false, + 'default_enable_unified_search' => false, + 'setup_project_folder' => false, + 'setup_app_password' => false, + ]; + return [ + "complete oauth2 setup" => [ + "authMethod" => OpenProjectAPIService::AUTH_METHOD_OAUTH, + 'settings' => [ + ...$defaultSettings, + 'authorization_method' => OpenProjectAPIService::AUTH_METHOD_OAUTH, + 'openproject_client_id' => 'test', + 'openproject_client_secret' => 'test', + ], + 'response' => [ + 'status', + 'nextcloud_oauth_client_name', + 'nextcloud_client_id', + 'nextcloud_client_secret', + 'openproject_redirect_uri', + ], + ], + "complete oidc setup" => [ + "authMethod" => OpenProjectAPIService::AUTH_METHOD_OIDC, + 'settings' => [ + ...$defaultSettings, + 'authorization_method' => OpenProjectAPIService::AUTH_METHOD_OIDC, + 'sso_provider_type' => SettingsService::NEXTCLOUDHUB_OIDC_PROVIDER_TYPE, + 'targeted_audience_client_id' => 'test', + ], + 'response' => [ + 'status', + ], + ], + "with team folder" => [ + "authMethod" => OpenProjectAPIService::AUTH_METHOD_OAUTH, + 'settings' => [ + ...$defaultSettings, + 'authorization_method' => OpenProjectAPIService::AUTH_METHOD_OIDC, + 'sso_provider_type' => SettingsService::NEXTCLOUDHUB_OIDC_PROVIDER_TYPE, + 'targeted_audience_client_id' => 'test', + 'setup_project_folder' => true, + 'setup_app_password' => true, + ], + 'response' => [ + 'status', + 'openproject_user_app_password', + ], + ], + ]; + } + + /** + * @param string $authMethod + * @param array $settings + * @param array $responseProps + * @return void + * @dataProvider setUpIntegrationSuccessProvider + */ + public function testSetUpIntegrationSuccess(string $authMethod, array $settings, array $responseProps): void { + $userManagerMock = $this->createMock(IUserManager::class); + $userManagerMock->method('userExists')->willReturn(true); + $oauthMock = $this->createMock(OauthService::class); + $oauthMock->method('createNcOauthClient')->willReturn([ + 'id' => '1234', + 'nextcloud_oauth_client_name' => 'Openproject Client', + 'openproject_redirect_uri' => 'http://openproject.test/oauth/callback', + 'nextcloud_client_id' => 'client_id', + 'nextcloud_client_secret' => 'client_secret', + ]); + + $openprojectAPIServiceMock = $this->createMock(OpenProjectAPIService::class); + if ($settings['setup_project_folder']) { + $openprojectAPIServiceMock->expects($this->once()) + ->method('generateAppPasswordTokenForUser') + ->willReturn('app_pass'); + } + $settingsServiceMock = $this->createMock(SettingsService::class); + $settingsServiceMock->expects($this->once()) + ->method('validateAdminSettingsForm'); + + $constructArgs = [ + 'oauthService' => $oauthMock, + 'settingsService' => $settingsServiceMock, + 'openprojectAPIService' => $openprojectAPIServiceMock, + 'userManager' => $userManagerMock, + ]; + $configController = $this->getConfigControllerMock([], $constructArgs); + + $response = $configController->setUpIntegration($settings); + $data = $response->getData(); + + $this->assertEquals(Http::STATUS_OK, $response->getStatus()); + $this->assertArrayHasKey('status', $data); + + foreach ($responseProps as $prop) { + $this->assertArrayHasKey($prop, $data); + } + if ($authMethod === OpenProjectAPIService::AUTH_METHOD_OIDC) { + $this->assertArrayNotHasKey('nextcloud_oauth_client_name', $data); + $this->assertArrayNotHasKey('nextcloud_client_id', $data); + $this->assertArrayNotHasKey('nextcloud_client_secret', $data); + $this->assertArrayNotHasKey('openproject_redirect_uri', $data); + } + if (!$settings['setup_app_password']) { + $this->assertArrayNotHasKey('openproject_user_app_password', $data); + } + } } From 4dacded0245874dc70d7888effd84daba5ed8478 Mon Sep 17 00:00:00 2001 From: Saw-jan Date: Wed, 10 Jun 2026 17:56:56 +0545 Subject: [PATCH 3/6] chore: add changelog entry Signed-off-by: Saw-jan --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d2cd26b6..c4992f7e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added +### Fixed + +- Fix: Do not create OAuth client when setting up with OIDC auth method [#1049](https://github.com/nextcloud/integration_openproject/pull/1049) + ### Changed ### Removed From cb96a7fcf0faaaebb80de43bf7afa980a1daee4e Mon Sep 17 00:00:00 2001 From: Saw-jan Date: Thu, 11 Jun 2026 09:40:45 +0545 Subject: [PATCH 4/6] test: fix test data Signed-off-by: Saw-jan --- tests/lib/Controller/ConfigControllerTest.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/lib/Controller/ConfigControllerTest.php b/tests/lib/Controller/ConfigControllerTest.php index 8a9268011..a859d4e46 100644 --- a/tests/lib/Controller/ConfigControllerTest.php +++ b/tests/lib/Controller/ConfigControllerTest.php @@ -1941,14 +1941,13 @@ public function setUpIntegrationSuccessProvider(): array { ], "with team folder" => [ "authMethod" => OpenProjectAPIService::AUTH_METHOD_OAUTH, - 'settings' => [ - ...$defaultSettings, + 'settings' => array_merge($defaultSettings, [ 'authorization_method' => OpenProjectAPIService::AUTH_METHOD_OIDC, 'sso_provider_type' => SettingsService::NEXTCLOUDHUB_OIDC_PROVIDER_TYPE, 'targeted_audience_client_id' => 'test', 'setup_project_folder' => true, 'setup_app_password' => true, - ], + ]), 'response' => [ 'status', 'openproject_user_app_password', @@ -1992,7 +1991,7 @@ public function testSetUpIntegrationSuccess(string $authMethod, array $settings, 'openprojectAPIService' => $openprojectAPIServiceMock, 'userManager' => $userManagerMock, ]; - $configController = $this->getConfigControllerMock([], $constructArgs); + $configController = $this->getConfigControllerMock(['setUpIntegration'], $constructArgs); $response = $configController->setUpIntegration($settings); $data = $response->getData(); From 3d26fc76a0ac1a1a17c921d4998249674b933646 Mon Sep 17 00:00:00 2001 From: Saw-jan Date: Thu, 11 Jun 2026 10:07:30 +0545 Subject: [PATCH 5/6] test: fix psalm config Signed-off-by: Saw-jan --- psalm.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/psalm.xml b/psalm.xml index fdcf738b0..8113062e7 100644 --- a/psalm.xml +++ b/psalm.xml @@ -21,6 +21,9 @@ + + + From d4e6b70d33065eb726a0c7d431226a9225fd4aca Mon Sep 17 00:00:00 2001 From: Saw-jan Date: Thu, 11 Jun 2026 10:45:31 +0545 Subject: [PATCH 6/6] test: fix phpunit test Signed-off-by: Saw-jan --- tests/lib/Controller/ConfigControllerTest.php | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/tests/lib/Controller/ConfigControllerTest.php b/tests/lib/Controller/ConfigControllerTest.php index a859d4e46..6757d2dde 100644 --- a/tests/lib/Controller/ConfigControllerTest.php +++ b/tests/lib/Controller/ConfigControllerTest.php @@ -1874,8 +1874,8 @@ public function integrationDefaultSettingsProvider() { } /** - * @param array $settings - * @param array $expectedSettings + * @param array $settings + * @param array $expectedSettings * @return void * @dataProvider integrationDefaultSettingsProvider */ @@ -1940,7 +1940,7 @@ public function setUpIntegrationSuccessProvider(): array { ], ], "with team folder" => [ - "authMethod" => OpenProjectAPIService::AUTH_METHOD_OAUTH, + "authMethod" => OpenProjectAPIService::AUTH_METHOD_OIDC, 'settings' => array_merge($defaultSettings, [ 'authorization_method' => OpenProjectAPIService::AUTH_METHOD_OIDC, 'sso_provider_type' => SettingsService::NEXTCLOUDHUB_OIDC_PROVIDER_TYPE, @@ -1958,7 +1958,7 @@ public function setUpIntegrationSuccessProvider(): array { /** * @param string $authMethod - * @param array $settings + * @param array $settings * @param array $responseProps * @return void * @dataProvider setUpIntegrationSuccessProvider @@ -1967,19 +1967,29 @@ public function testSetUpIntegrationSuccess(string $authMethod, array $settings, $userManagerMock = $this->createMock(IUserManager::class); $userManagerMock->method('userExists')->willReturn(true); $oauthMock = $this->createMock(OauthService::class); - $oauthMock->method('createNcOauthClient')->willReturn([ - 'id' => '1234', - 'nextcloud_oauth_client_name' => 'Openproject Client', - 'openproject_redirect_uri' => 'http://openproject.test/oauth/callback', - 'nextcloud_client_id' => 'client_id', - 'nextcloud_client_secret' => 'client_secret', - ]); + + if ($authMethod === OpenProjectAPIService::AUTH_METHOD_OAUTH) { + $oauthMock->expects($this->once()) + ->method('createNcOauthClient')->willReturn([ + 'id' => '1234', + 'nextcloud_oauth_client_name' => 'Openproject Client', + 'openproject_redirect_uri' => 'http://openproject.test/oauth/callback', + 'nextcloud_client_id' => 'client_id', + 'nextcloud_client_secret' => 'client_secret', + ]); + } else { + $oauthMock->expects($this->never()) + ->method('createNcOauthClient'); + } $openprojectAPIServiceMock = $this->createMock(OpenProjectAPIService::class); - if ($settings['setup_project_folder']) { + if ($settings['setup_app_password']) { $openprojectAPIServiceMock->expects($this->once()) ->method('generateAppPasswordTokenForUser') ->willReturn('app_pass'); + } else { + $openprojectAPIServiceMock->expects($this->never()) + ->method('generateAppPasswordTokenForUser'); } $settingsServiceMock = $this->createMock(SettingsService::class); $settingsServiceMock->expects($this->once()) @@ -1991,7 +2001,8 @@ public function testSetUpIntegrationSuccess(string $authMethod, array $settings, 'openprojectAPIService' => $openprojectAPIServiceMock, 'userManager' => $userManagerMock, ]; - $configController = $this->getConfigControllerMock(['setUpIntegration'], $constructArgs); + $constructArgs = $this->getConfigControllerConstructArgs($constructArgs); + $configController = new ConfigController(...$constructArgs); $response = $configController->setUpIntegration($settings); $data = $response->getData();