From 0147256ccd6dc25d793f1230a4d62199516c98c1 Mon Sep 17 00:00:00 2001 From: Ashim Shrestha Date: Tue, 11 Aug 2026 15:36:57 +0545 Subject: [PATCH] fix: user shown connected in webUI after converting exisiting OAuth to SSO via setup endpoint Signed-off-by: Ashim Shrestha --- CHANGELOG.md | 1 + lib/Controller/ConfigController.php | 21 ++--- tests/lib/Controller/ConfigControllerTest.php | 77 +++++++++++++++++-- 3 files changed, 79 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9878b657..b0c3158a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Fix: Default OIDC provider name not set while updating to Nextcloud Hub setup [#1126](https://github.com/nextcloud/integration_openproject/pull/1126) +- Fix: User shown connected in webUI after converting existing OAuth to SSO via setup endpoint [#1132](https://github.com/nextcloud/integration_openproject/pull/1132) ### Changed diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index dbb1a9449..92ede69ee 100755 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -241,14 +241,9 @@ private function setIntegrationConfig(array $values): array { } // since we can now switch between both authorization method we need to know what we are resetting (either "oauth2" or "oidc" method) // determines if we are switching from "oauth2" to "oidc" auth method - $runningOauth2Reset = ( - key_exists('openproject_client_id', $values) && - !$values['openproject_client_id'] && - key_exists('openproject_client_secret', $values) && - !$values['openproject_client_secret'] && - $oldOpenProjectOauthUrl && - $oldClientId && - $oldClientSecret + $switchingOAuthToOIDC = ( + $oldAuthMethod === Application::AUTH_METHOD_OAUTH && key_exists('authorization_method', $values) && + $values['authorization_method'] === Application::AUTH_METHOD_OIDC ); // determines if we are switching from "oidc" to "oauth2" auth method @@ -309,13 +304,14 @@ private function setIntegrationConfig(array $values): array { } $this->config->deleteAppValue(Application::APP_ID, 'oPOAuthTokenRevokeStatus'); + $resetOpenProjectClient = ((key_exists('openproject_client_id', $values) && $values['openproject_client_id'] !== $oldClientId) || + (key_exists('openproject_client_secret', $values) && $values['openproject_client_secret'] !== $oldClientSecret)); if ( // when the OP client information has changed - (!$runningFullResetWithOIDCAuth && ((key_exists('openproject_client_id', $values) && $values['openproject_client_id'] !== $oldClientId) || - (key_exists('openproject_client_secret', $values) && $values['openproject_client_secret'] !== $oldClientSecret))) || + (!$runningFullResetWithOIDCAuth && $resetOpenProjectClient) || // when the OP client information is reset $runningFullResetWithOAuth2Auth || - $runningOauth2Reset + $switchingOAuthToOIDC ) { $this->userManager->callForAllUsers(function (IUser $user) use ( $oldOpenProjectOauthUrl, $oldClientId, $oldClientSecret @@ -378,8 +374,7 @@ private function setIntegrationConfig(array $values): array { } // when switching from "oauth2" to "oidc" authorization method - if (key_exists('authorization_method', $values) && - $values['authorization_method'] === Application::AUTH_METHOD_OIDC && $runningOauth2Reset) { + if ($switchingOAuthToOIDC) { $this->resetOauth2Configs(); } diff --git a/tests/lib/Controller/ConfigControllerTest.php b/tests/lib/Controller/ConfigControllerTest.php index 9960ecd79..318204b38 100644 --- a/tests/lib/Controller/ConfigControllerTest.php +++ b/tests/lib/Controller/ConfigControllerTest.php @@ -2007,7 +2007,7 @@ public function updateIntegrationSuccessProvider(): array { 'status', ], ], - "oauth to oidc" => [ + "oauth to oidc: no existing oauth client" => [ "authMethod" => Application::AUTH_METHOD_OAUTH, "oauthClientId" => 0, 'settings' => [ @@ -2017,6 +2017,16 @@ public function updateIntegrationSuccessProvider(): array { 'status', ], ], + "oauth to oidc: existing oauth client" => [ + "authMethod" => Application::AUTH_METHOD_OAUTH, + "oauthClientId" => 1, + 'settings' => [ + 'authorization_method' => Application::AUTH_METHOD_OIDC, + ], + 'responseProps' => [ + 'status', + ], + ], "oidc to oauth" => [ "authMethod" => Application::AUTH_METHOD_OIDC, "oauthClientId" => 0, @@ -2043,22 +2053,59 @@ public function updateIntegrationSuccessProvider(): array { * @dataProvider updateIntegrationSuccessProvider */ public function testUpdateIntegrationSuccess(string $authMethod, int $oauthClientId, array $settings, array $responseProps): void { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('testUser'); + $userManagerMock = $this->createMock(IUserManager::class); $userManagerMock->method('userExists')->willReturn(true); + $userManagerMock->method('callForAllUsers') + ->willReturnCallback(function (callable $callback) use ($user) { + $callback($user); + }); + $oauthMock = $this->createMock(OauthService::class); + $oldAuthMethod = $authMethod; // change in authorization method if (isset($settings['authorization_method'])) { $authMethod = $settings['authorization_method']; } + $configState = [ + 'authorization_method' => $oldAuthMethod, + 'nc_oauth_client_id' => $oauthClientId, + ]; + + $setAppValueCalls = []; + $deletedKeys = []; + $configMock = $this->createMock(IConfig::class); - $configMock - ->method('getAppValue') - ->willReturnMap([ - [Application::APP_ID, 'authorization_method', '', $authMethod], - [Application::APP_ID, 'nc_oauth_client_id', '', $oauthClientId], - ]); + + $configMock->method('getAppValue') + ->willReturnCallback(function ($app, $key, $default = '') use (&$configState) { + return $configState[$key] ?? $default; + }); + + $configMock->method('setAppValue') + ->willReturnCallback(function ($app, $key, $value) use (&$configState, &$setAppValueCalls) { + $setAppValueCalls[] = [$app, $key, $value]; + $configState[$key] = $value; + return true; + }); + + $configMock->method('deleteAppValue') + ->willReturnCallback(function ($app, $key) use (&$configState, &$deletedKeys) { + $deletedKeys[] = $key; + unset($configState[$key]); + return true; + }); + + $deletedUserValues = []; + $configMock->method('deleteUserValue') + ->willReturnCallback(function ($uid, $app, $key) use (&$deletedUserValues) { + $deletedUserValues[] = [$uid, $app, $key]; + return true; + }); if ($authMethod === Application::AUTH_METHOD_OAUTH) { if ($oauthClientId) { @@ -2122,6 +2169,22 @@ public function testUpdateIntegrationSuccess(string $authMethod, int $oauthClien $this->assertEquals(Http::STATUS_OK, $response->getStatus()); $this->assertArrayHasKey('status', $data); + if ($oldAuthMethod === Application::AUTH_METHOD_OAUTH + && isset($settings['authorization_method']) + && $settings['authorization_method'] === Application::AUTH_METHOD_OIDC) { + $this->assertContains([Application::APP_ID, 'openproject_client_id', ''], $setAppValueCalls); + $this->assertContains([Application::APP_ID, 'openproject_client_secret', ''], $setAppValueCalls); + $this->assertContains('nc_oauth_client_id', $deletedKeys); + + $expectedDeletedKeys = ['token', 'login', 'user_id', 'user_name', 'refresh_token', 'token_expires_at']; + foreach ($expectedDeletedKeys as $key) { + $this->assertContains( + ['testUser', Application::APP_ID, $key], + $deletedUserValues + ); + } + } + foreach ($responseProps as $prop) { $this->assertArrayHasKey($prop, $data); }