From 0780d2f8962188780512a0e21ea9c2b86b42fe6b Mon Sep 17 00:00:00 2001 From: Ashim Shrestha Date: Fri, 8 May 2026 14:31:21 +0545 Subject: [PATCH 1/4] feat: fix wrong message Unauthorized to connect to Openproject Signed-off-by: Ashim Shrestha --- lib/Controller/ConfigController.php | 4 +-- lib/Service/OpenProjectAPIService.php | 39 ++++++++++++++++++--------- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index b4947ef61..8d924bdf3 100755 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -155,7 +155,7 @@ public function setConfig(array $values): DataResponse { if (isset($values['token'])) { if ($values['token']) { - $result = $this->openprojectAPIService->initUserInfo($this->userId); + $result = $this->openprojectAPIService->initUserInfo($this->userId, $values['token']); } else { $this->clearUserInfo(); $result = [ @@ -536,7 +536,7 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe ); if (isset($result['access_token']) && isset($result['refresh_token'])) { // set user info - $userInfo = $this->openprojectAPIService->initUserInfo($this->userId); + $userInfo = $this->openprojectAPIService->initUserInfo($this->userId, $result['access_token']); if (isset($userInfo['user_name'])) { $this->config->setUserValue( $this->userId, Application::APP_ID, 'oauth_connection_result', 'success' diff --git a/lib/Service/OpenProjectAPIService.php b/lib/Service/OpenProjectAPIService.php index 8734faa4e..f29bde0d1 100644 --- a/lib/Service/OpenProjectAPIService.php +++ b/lib/Service/OpenProjectAPIService.php @@ -419,7 +419,7 @@ public function rawRequest( string $method = 'GET', array $options = [] ) { - $url = $openprojectUrl . '/api/v3/' . $endPoint; + $url = $openprojectUrl . '/api/v3/' . ltrim($endPoint, '/'); if (!isset($options['headers']['Authorization'])) { $options['headers']['Authorization'] = 'Bearer ' . $accessToken; } @@ -1716,13 +1716,6 @@ public function getOIDCToken(string $userId): string { $this->config->setUserValue($userId, Application::APP_ID, 'token', $token->getAccessToken()); $this->config->setUserValue($userId, Application::APP_ID, 'token_expires_at', $tokenExpiresAt); - $savedUserId = $this->config->getUserValue($userId, Application::APP_ID, 'user_id'); - $savedUsername = $this->config->getUserValue($userId, Application::APP_ID, 'user_name'); - if (!$savedUserId || !$savedUsername) { - // get user info - $this->initUserInfo($userId); - } - return $token->getAccessToken(); } @@ -1751,16 +1744,22 @@ public function getAccessToken(?string $userId): string { return ''; } $token = $this->config->getUserValue($userId, Application::APP_ID, 'token', ''); + $authMethod = $this->config->getAppValue(Application::APP_ID, 'authorization_method'); + if ($token && !$this->isAccessTokenExpired($userId)) { + if ($authMethod === SettingsService::AUTH_METHOD_OIDC) { + $this->initUserInfo($userId, $token); + } return $token; } if ($token) { $this->logger->debug('Token has expired.', ['app' => $this->appName]); $this->logger->debug('Refreshing access token.', ['app' => $this->appName]); + $this->config->deleteUserValue($userId, Application::APP_ID, 'user_name'); + $this->config->deleteUserValue($userId, Application::APP_ID, 'user_id'); } - $authMethod = $this->config->getAppValue(Application::APP_ID, 'authorization_method'); // For OAuth2 setup, only try to refresh the expired token. // Token exchange needs to be initiated from the UI. if ($authMethod === SettingsService::AUTH_METHOD_OAUTH && $token) { @@ -1784,7 +1783,11 @@ public function getAccessToken(?string $userId): string { } return $result['access_token']; } elseif ($authMethod === SettingsService::AUTH_METHOD_OIDC) { - return $this->getOIDCToken($userId); + $token = $this->getOIDCToken($userId); + if ($token) { + $this->initUserInfo($userId, $token); + } + return $token; } return ''; @@ -1796,8 +1799,20 @@ public function getAccessToken(?string $userId): string { * @return array * @throws PreConditionNotMetException */ - public function initUserInfo(string $userId): array { - $info = $this->request($userId, '/users/me'); + public function initUserInfo(string $userId, string $accessToken): array { + $savedUserId = $this->config->getUserValue($userId, Application::APP_ID, 'user_id'); + $savedUsername = $this->config->getUserValue($userId, Application::APP_ID, 'user_name'); + if ($savedUserId && $savedUsername) { + return ['user_name' => $savedUsername]; + } + try { + $openprojectUrl = $this->config->getAppValue(Application::APP_ID, 'openproject_instance_url'); + $response = $this->rawRequest($accessToken, $openprojectUrl, '/users/me'); + $info = json_decode($response->getBody(), true); + } catch (Exception $e) { + $this->logger->error('OpenProject error : ' . $e->getMessage(), ['app' => $this->appName]); + return ['error' => $e->getMessage()]; + } if (isset($info['lastName'], $info['firstName'], $info['id'])) { $fullName = $info['firstName'] . ' ' . $info['lastName']; $this->config->setUserValue($userId, Application::APP_ID, 'user_id', $info['id']); From 0c9d6eba39089e4e733bba6039421ea342661f03 Mon Sep 17 00:00:00 2001 From: Ashim Shrestha Date: Wed, 20 May 2026 14:13:35 +0545 Subject: [PATCH 2/4] test: update php unit tests Signed-off-by: Ashim Shrestha --- .../lib/Service/OpenProjectAPIServiceTest.php | 50 +++++++++++-------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/tests/lib/Service/OpenProjectAPIServiceTest.php b/tests/lib/Service/OpenProjectAPIServiceTest.php index 867649aaa..05b893a61 100644 --- a/tests/lib/Service/OpenProjectAPIServiceTest.php +++ b/tests/lib/Service/OpenProjectAPIServiceTest.php @@ -783,18 +783,14 @@ private function getOpenProjectAPIService( $tokenExpiryTime = $oAuth2OrOidcToken === 'expired' ? 0 : time() + 7200; $this->defaultConfigMock ->method('getUserValue') - ->withConsecutive( - [$userId, 'integration_openproject', 'token'], - [$userId, 'integration_openproject', 'token_expires_at'], - [$userId, 'integration_openproject', 'refresh_token'], - [$userId, 'integration_openproject', 'token'], - ) - ->willReturnOnConsecutiveCalls( - $oAuth2OrOidcToken, - $tokenExpiryTime, - 'oAuthRefreshToken', - 'new-Token' - ); + ->willReturnMap([ + [$userId, 'integration_openproject', 'token', $oAuth2OrOidcToken], + [$userId, 'integration_openproject', 'token_expires_at', $tokenExpiryTime], + [$userId, 'integration_openproject', 'refresh_token', 'oAuthRefreshToken'], + [$userId, 'integration_openproject', 'token', 'new-Token'], + [$userId, 'integration_openproject', 'user_id', 'some-user-id'], + [$userId, 'integration_openproject', 'user_name', 'some-user-name'], + ]); if ($authMethod === OpenProjectAPIService::AUTH_METHOD_OIDC) { $tokenMock = $this->getMockBuilder(Token::class)->disableOriginalConstructor()->getMock(); $exchangeTokenMock->method('getEvent')->willReturn($exchangedTokenRequestedEventMock); @@ -4565,12 +4561,6 @@ public function testGetOIDCTokenSuccess(): void { ->willReturnMap($this->getAppValues([ 'authorization_method' => OpenProjectAPIService::AUTH_METHOD_OIDC ])); - $configMock - ->method('getUserValue') - ->willReturnMap([ - ['testUser', Application::APP_ID, 'user_id', null], - ['testUser', Application::APP_ID, 'user_name', null], - ]); $calls = []; $configMock ->method('setUserValue') @@ -4587,7 +4577,7 @@ public function testGetOIDCTokenSuccess(): void { $eventMock->method('getToken')->willReturn($tokenMock); $tokenMock->method('getAccessToken')->willReturn('exchanged-access-token'); $service = $this->getOpenProjectAPIServiceMock( - ['initUserInfo'], + [], [ 'appManager' => $iAppManagerMock, 'config' => $configMock, @@ -4595,7 +4585,6 @@ public function testGetOIDCTokenSuccess(): void { 'tokenEventFactory' => $exchangeTokenEvent, ], ); - $service->expects($this->once())->method('initUserInfo')->with('testUser'); $result = $service->getOIDCToken('testUser'); $this->assertEquals('exchanged-access-token', $result); $expectedCalls = [ @@ -4815,7 +4804,7 @@ public function testGetAccessToken( ['testUser', Application::APP_ID, 'refresh_token', '', 'refresh-token'], ]); $service = $this->getOpenProjectAPIServiceMock( - ['isAccessTokenExpired', 'getOIDCToken', 'requestOAuthAccessToken'], + ['isAccessTokenExpired', 'getOIDCToken', 'requestOAuthAccessToken', 'initUserInfo'], [ 'config' => $configMock, ], @@ -4827,6 +4816,25 @@ public function testGetAccessToken( ->willReturn($expired); } + if ($token && $expired) { + $configMock + ->expects($this->exactly(2)) + ->method("deleteUserValue") + ->withConsecutive( + ['testUser', 'integration_openproject', 'user_name'], + ['testUser', 'integration_openproject', 'user_id'], + ); + } + + if ($token && $authMethod === SettingsService::AUTH_METHOD_OIDC && (!$expired || $expectedToken)) { + $expectedTokenForInitUserInfo = $expired ? $expectedToken : $token; + $service->expects($this->once()) + ->method('initUserInfo') + ->with('testUser', $expectedTokenForInitUserInfo); + } else { + $service->expects($this->never())->method('initUserInfo'); + } + if ($authMethod === SettingsService::AUTH_METHOD_OAUTH && $expired) { if ($tokenRefreshFailed) { $service->expects($this->once()) From b18b216ac1c5af7890ca46a7bbb6b862c30776c3 Mon Sep 17 00:00:00 2001 From: Ashim Shrestha Date: Thu, 21 May 2026 13:06:55 +0545 Subject: [PATCH 3/4] test: add php unit test for initUserInfo Signed-off-by: Ashim Shrestha --- .../lib/Service/OpenProjectAPIServiceTest.php | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/tests/lib/Service/OpenProjectAPIServiceTest.php b/tests/lib/Service/OpenProjectAPIServiceTest.php index 05b893a61..a0525afcf 100644 --- a/tests/lib/Service/OpenProjectAPIServiceTest.php +++ b/tests/lib/Service/OpenProjectAPIServiceTest.php @@ -5247,4 +5247,103 @@ public function testGetNCBaseUrl(string $url, string $expected): void { $baseUrl = $service->getNCBaseUrl(); $this->assertEquals($expected, $baseUrl); } + + /** + * Data provider for testInitUserInfo + */ + public function initUserInfoDataProvider(): array { + return [ + 'user info already saved' => [ + 'savedUserId' => 'testUserID', + 'savedUsername' => 'test User', + 'expectedResult' => ['user_name' => 'test User'], + 'expectRawRequestCalled' => false, + 'apiResponse' => null, + 'expectedExpection' => null + ], + 'user info missing and API succeeds' => [ + 'savedUserId' => '', + 'savedUsername' => '', + 'expectedResult' => ['user_name' => 'test User'], + 'expectRawRequestCalled' => true, + 'apiResponse' => [ + 'id' => 'testUserID', + 'firstName' => 'test', + 'lastName' => 'User', + ], + 'expectedExpection' => null + ], + 'user info missing and API throws exception' => [ + 'savedUserId' => '', + 'savedUsername' => '', + 'expectedResult' => ['error' => 'OpenProject error'], + 'expectRawRequestCalled' => true, + 'apiResponse' => null, + 'expectedExpection' => new \Exception('OpenProject error') + ], + 'user info missing and API returns incomplete data' => [ + 'savedUserId' => '', + 'savedUsername' => '', + 'expectedResult' => ['id' => 'testUserID', 'error' => 'Failed to get user profile'], + 'expectRawRequestCalled' => true, + 'apiResponse' => [ + 'id' => 'testUserID', + ], + 'expectedExpection' => null + ], + ]; + } + + /** + * @dataProvider initUserInfoDataProvider + * @param string $savedUserId + * @param string $savedUsername + * @param array $expectedResult + * @param bool $expectRawRequestCalled + * @param array|null $apiResponse + * @param \Exception|null $expectedExpection + * @return void + */ + public function testInitUserInfo(string $savedUserId, string $savedUsername, array $expectedResult, bool $expectRawRequestCalled, ?array $apiResponse, ?\Exception $expectedExpection): void { + $configMock = $this->getMockBuilder(IConfig::class)->getMock(); + $configMock->method('getAppValue') + ->willReturnMap($this->getAppValues([ + 'openproject_instance_url' => 'http://test.local', + ])); + $configMock + ->method('getUserValue') + ->willReturnMap([ + ['testUser', Application::APP_ID, 'user_id', '', $savedUserId], + ['testUser', Application::APP_ID, 'user_name','', $savedUsername] + ]); + + $calls = []; + $configMock + ->method('setUserValue') + ->willReturnCallback(function ($uid, $app, $key, $value) use (&$calls) { + $calls[] = [$uid, $app, $key, $value]; + }); + + $service = $this->getOpenProjectAPIServiceMock(['rawRequest'], [ 'config' => $configMock ]); + + if (!$expectRawRequestCalled) { + $service->expects($this->never())->method('rawRequest'); + } elseif ($expectedExpection !== null) { + $service->expects($this->once())->method('rawRequest')->willThrowException($expectedExpection); + } else { + $mockResponse = $this->createMock(Response::class); + $mockResponse->method('getBody')->willReturn(json_encode($apiResponse)); + $service->expects($this->once())->method('rawRequest')->willReturn( + $mockResponse + ); + } + + $result = $service->initUserInfo('testUser', 'token'); + $this->assertSame($expectedResult, $result); + + if ($expectedResult === ['user_name' => 'test User'] && $expectRawRequestCalled) { + $this->assertContains(['testUser', Application::APP_ID, 'user_id', 'testUserID'], $calls); + $this->assertContains(['testUser', Application::APP_ID, 'user_name', 'test User'], $calls); + } + } } From 66a845a7c14f94ce3b1db066dbf571709a5b85f7 Mon Sep 17 00:00:00 2001 From: Ashim Shrestha Date: Thu, 21 May 2026 13:09:05 +0545 Subject: [PATCH 4/4] chore: update changelog Signed-off-by: Ashim Shrestha --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dbb862c7..c2f1ee1de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +- Fix: Shown wrong message "Unauthorized to connect to Openproject" on Single-Sign-On setup [#1018](https://github.com/nextcloud/integration_openproject/pull/1018) + ### Changed ### Removed