fix: do not create OAuth client when setting up with OIDC auth method

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

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);
 		}

psalm.xml

@@ -21,6 +21,9 @@
 		<directory name="tests" />
 		<ignoreFiles>
 			<directory name="vendor" />
+			<directory name="tests/stub" />
+		</ignoreFiles>
+		<ignoreFiles allowMissingFiles="true">
 			<directory name="server/lib" />
 		</ignoreFiles>
 	</projectFiles>

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<string> $oldCreds
-	 * @param array<string> $credsToUpdate
+	 * @param array<string, bool|string> $settings
+	 * @param array<string, bool|string> $expectedSettings
 	 * @return void
 	 * @dataProvider integrationDefaultSettingsProvider
 	 */
@@ -1899,4 +1898,129 @@ public function testSetUpIntegrationDefaultSettings(array $settings, array $expe
 		$configController = new ConfigController(...$constructArgs);
 		$configController->setUpIntegration($settings);
 	}
+
+	/**
+	 * @return array<mixed>
+	 */
+	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_OIDC,
+				'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',
+				],
+			],
+		];
+	}
+
+	/**
+	 * @param string $authMethod
+	 * @param array<string, bool|string> $settings
+	 * @param array<string> $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);
+
+		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_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())
+			->method('validateAdminSettingsForm');
+
+		$constructArgs = [
+			'oauthService' => $oauthMock,
+			'settingsService' => $settingsServiceMock,
+			'openprojectAPIService' => $openprojectAPIServiceMock,
+			'userManager' => $userManagerMock,
+		];
+		$constructArgs = $this->getConfigControllerConstructArgs($constructArgs);
+		$configController = new ConfigController(...$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);
+		}
+	}
 }