Skip to content

fix: wrong message Unauthorized to connect to Openproject shown with valid SSO connection #1018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Ashim-Stha merged 5 commits into from
May 26, 2026
+187 −38
Merged

fix: wrong message Unauthorized to connect to Openproject shown with valid SSO connection #1018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3d73e3d
feat: fix wrong message Unauthorized to connect to Openproject
Ashim-Stha May 8, 2026
7da49a9
test: update php unit tests
Ashim-Stha May 20, 2026
1daa01d
test: add php unit test for initUserInfo
Ashim-Stha May 21, 2026
0d45a98
chore: update changelog
Ashim-Stha May 21, 2026
677426b
address reviews
Ashim-Stha May 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Fixed

- Fix: Wrong message "Unauthorized to connect to OpenProject" shown with valid SSO connection [#1018](https://github.com/nextcloud/integration_openproject/pull/1018)

### Changed

### Removed
Expand Down
4 changes: 2 additions & 2 deletions lib/Controller/ConfigController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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'
Expand Down
46 changes: 33 additions & 13 deletions lib/Service/OpenProjectAPIService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ public function rawRequest(
string $method = 'GET',
array $options = []
) {
$url = $openprojectUrl . '/api/v3/' . $endPoint;
$url = rtrim($openprojectUrl, '/') . '/api/v3/' . ltrim($endPoint, '/');
if (!isset($options['headers']['Authorization'])) {
$options['headers']['Authorization'] = 'Bearer ' . $accessToken;
}
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -1751,16 +1744,24 @@ 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);
}
Comment thread
Ashim-Stha marked this conversation as resolved.
return $token;
}

if ($token) {
$this->logger->debug('Token has expired.', ['app' => $this->appName]);
$this->logger->debug('Refreshing access token.', ['app' => $this->appName]);
if ($authMethod === SettingsService::AUTH_METHOD_OIDC) {
$this->config->deleteUserValue($userId, Application::APP_ID, 'user_name');
$this->config->deleteUserValue($userId, Application::APP_ID, 'user_id');
}
Comment thread
saw-jan marked this conversation as resolved.
}

$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) {
Expand All @@ -1784,20 +1785,39 @@ 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);
}
Comment thread
Ashim-Stha marked this conversation as resolved.
return $token;
}

return '';
}

/**
* @param string $userId
* @param string $accessToken
*
* @return array<mixed>
* @throws PreConditionNotMetException
*/
public function initUserInfo(string $userId): array {
$info = $this->request($userId, '/users/me');
public function initUserInfo(string $userId, string $accessToken): array {
Comment thread
Ashim-Stha marked this conversation as resolved.
$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');
Comment thread
Ashim-Stha marked this conversation as resolved.
if (!$openprojectUrl || !OpenProjectAPIService::validateURL($openprojectUrl)) {
return ['error' => 'OpenProject URL is invalid', 'statusCode' => 500];
}
$response = $this->rawRequest($accessToken, $openprojectUrl, '/users/me');
Comment thread
nabim777 marked this conversation as resolved.
$info = json_decode($response->getBody(), true);
} catch (Exception $e) {
$this->logger->error('OpenProject error : ' . $e->getMessage(), ['app' => $this->appName]);
return ['error' => $e->getMessage()];
}
Comment thread
saw-jan marked this conversation as resolved.
if (isset($info['lastName'], $info['firstName'], $info['id'])) {
$fullName = $info['firstName'] . ' ' . $info['lastName'];
$this->config->setUserValue($userId, Application::APP_ID, 'user_id', $info['id']);
Expand Down
173 changes: 150 additions & 23 deletions tests/lib/Service/OpenProjectAPIServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -781,20 +781,25 @@ 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'
);
if ($authMethod === OpenProjectAPIService::AUTH_METHOD_OIDC) {
$this->defaultConfigMock
->method('getUserValue')
->willReturnMap([
[$userId, 'integration_openproject', 'token', $oAuth2OrOidcToken],
[$userId, 'integration_openproject', 'token_expires_at', $tokenExpiryTime],
[$userId, 'integration_openproject', 'user_id', 'some-user-id'],
[$userId, 'integration_openproject', 'user_name', 'some-user-name'],
]);
} else {
$this->defaultConfigMock
->method('getUserValue')
->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'],
]);
}
if ($authMethod === OpenProjectAPIService::AUTH_METHOD_OIDC) {
$tokenMock = $this->getMockBuilder(Token::class)->disableOriginalConstructor()->getMock();
$exchangeTokenMock->method('getEvent')->willReturn($exchangedTokenRequestedEventMock);
Expand Down Expand Up @@ -4565,12 +4570,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')
Expand All @@ -4587,15 +4586,14 @@ public function testGetOIDCTokenSuccess(): void {
$eventMock->method('getToken')->willReturn($tokenMock);
$tokenMock->method('getAccessToken')->willReturn('exchanged-access-token');
$service = $this->getOpenProjectAPIServiceMock(
['initUserInfo'],
[],
[
'appManager' => $iAppManagerMock,
'config' => $configMock,
'manager' => $iManagerMock,
'tokenEventFactory' => $exchangeTokenEvent,
],
);
$service->expects($this->once())->method('initUserInfo')->with('testUser');
$result = $service->getOIDCToken('testUser');
$this->assertEquals('exchanged-access-token', $result);
$expectedCalls = [
Expand Down Expand Up @@ -4815,7 +4813,7 @@ public function testGetAccessToken(
['testUser', Application::APP_ID, 'refresh_token', '', 'refresh-token'],
]);
$service = $this->getOpenProjectAPIServiceMock(
['isAccessTokenExpired', 'getOIDCToken', 'requestOAuthAccessToken'],
['isAccessTokenExpired', 'getOIDCToken', 'requestOAuthAccessToken', 'initUserInfo'],
[
'config' => $configMock,
],
Expand All @@ -4827,6 +4825,27 @@ public function testGetAccessToken(
->willReturn($expired);
}

if ($token && $expired && $authMethod === SettingsService::AUTH_METHOD_OIDC) {
$configMock
->expects($this->exactly(2))
->method("deleteUserValue")
->willReturnMap([
['testUser', 'integration_openproject', 'user_name', ''],
['testUser', 'integration_openproject', 'user_id', ''],
]);
} else {
$configMock->expects($this->never())->method("deleteUserValue");
}

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())
Expand Down Expand Up @@ -5239,4 +5258,112 @@ 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,
'expectedException' => null
],
'user info missing and API succeeds' => [
'savedUserId' => '',
'savedUsername' => '',
'expectedResult' => ['user_name' => 'test User'],
'expectRawRequestCalled' => true,
'apiResponse' => [
'id' => 'testUserID',
'firstName' => 'test',
'lastName' => 'User',
],
'expectedException' => null
],
'user info missing and API throws exception' => [
'savedUserId' => '',
'savedUsername' => '',
'expectedResult' => ['error' => 'OpenProject error'],
'expectRawRequestCalled' => true,
'apiResponse' => null,
'expectedException' => new \Exception('OpenProject error')
],
Comment thread
Ashim-Stha marked this conversation as resolved.
'user info missing and API returns incomplete data' => [
'savedUserId' => '',
'savedUsername' => '',
'expectedResult' => ['id' => 'testUserID', 'error' => 'Failed to get user profile'],
'expectRawRequestCalled' => true,
'apiResponse' => [
'id' => 'testUserID',
],
'expectedException' => null
],
];
}

/**
* @dataProvider initUserInfoDataProvider
* @param string $savedUserId
* @param string $savedUsername
* @param array $expectedResult
* @param bool $expectRawRequestCalled
* @param array|null $apiResponse
* @param \Exception|null $expectedException
* @return void
*/
public function testInitUserInfo(
string $savedUserId,
string $savedUsername,
array $expectedResult,
bool $expectRawRequestCalled,
?array $apiResponse,
?\Exception $expectedException,
): 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]
Comment thread
saw-jan marked this conversation as resolved.
]);

$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 ($expectedException !== null) {
$service->expects($this->once())->method('rawRequest')->willThrowException($expectedException);
} 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 (isset($expectedResult['user_name']) && $expectRawRequestCalled) {
$this->assertContains(['testUser', Application::APP_ID, 'user_id', 'testUserID'], $calls);
$this->assertContains(['testUser', Application::APP_ID, 'user_name', 'test User'], $calls);
} else {
$this->assertEmpty($calls);
}
Comment thread
Ashim-Stha marked this conversation as resolved.
}
}