diff --git a/appinfo/info.xml b/appinfo/info.xml index d00c1ce..6c3bdb6 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -5,7 +5,7 @@ Integration of GitLab software development management service - 3.1.2 + 3.1.3 agpl Julien Veyssier Gitlab diff --git a/lib/Controller/ConfigController.php b/lib/Controller/ConfigController.php index 6338b87..979d669 100644 --- a/lib/Controller/ConfigController.php +++ b/lib/Controller/ConfigController.php @@ -71,7 +71,7 @@ public function addAccount(string $url, string $token) { $account = new GitlabAccount(); $account->setUserId($this->userId); $account->setUrl($url); - $account->setToken($token); + $account->setEncryptedToken($token); $account->setTokenType('personal'); try { @@ -197,9 +197,9 @@ public function oauthRedirect(string $code = '', string $state = ''): RedirectRe $account = new GitlabAccount(); $account->setUserId($this->userId); $account->setUrl($adminOauthUrl); - $account->setToken($result['access_token']); + $account->setEncryptedToken($result['access_token']); $account->setTokenType('oauth'); - $account->setRefreshToken($result['refresh_token'] ?? ''); + $account->setEncryptedRefreshToken($result['refresh_token'] ?? ''); if (isset($result['expires_in'])) { $nowTs = (new Datetime())->getTimestamp(); $expiresAt = $nowTs + (int)$result['expires_in']; diff --git a/lib/Controller/GitlabAPIController.php b/lib/Controller/GitlabAPIController.php index ac60178..67dd4b7 100644 --- a/lib/Controller/GitlabAPIController.php +++ b/lib/Controller/GitlabAPIController.php @@ -48,7 +48,7 @@ public function __construct( public function getUserAvatar(int $accountId, int $userId) { try { $account = $this->accountMapper->findById($this->userId, $accountId); - if ($account->getToken() === '') { + if ($account->getClearToken() === '') { return new DataResponse('', 400); } @@ -83,7 +83,7 @@ public function getUserAvatar(int $accountId, int $userId) { public function getProjectAvatar(int $accountId, int $projectId) { try { $account = $this->accountMapper->findById($this->userId, $accountId); - if ($account->getToken() === '') { + if ($account->getClearToken() === '') { return new DataResponse('', 400); } @@ -117,7 +117,7 @@ public function getProjectAvatar(int $accountId, int $projectId) { public function getTodos(int $accountId, ?string $since = null): DataResponse { try { $account = $this->accountMapper->findById($this->userId, $accountId); - if ($account->getToken() === '') { + if ($account->getClearToken() === '') { return new DataResponse('', 400); } diff --git a/lib/Db/GitlabAccount.php b/lib/Db/GitlabAccount.php index 032e346..7b43d81 100644 --- a/lib/Db/GitlabAccount.php +++ b/lib/Db/GitlabAccount.php @@ -6,6 +6,7 @@ use JsonSerializable; use OCP\AppFramework\Db\Entity; +use OCP\Security\ICrypto; /** * @method void setUserId(string $userId) @@ -25,14 +26,29 @@ * @method void setUserInfoDisplayName(string $userInfoDisplayName) */ class GitlabAccount extends Entity implements JsonSerializable { - protected string $userId = ''; - protected string $url = ''; - protected string $token = ''; - protected string $tokenType = ''; - protected ?int $tokenExpiresAt = null; - protected ?string $refreshToken = null; - protected ?string $userInfoName = null; - protected ?string $userInfoDisplayName = null; + protected $userId; + protected $url; + protected $token; + protected $tokenType; + protected $tokenExpiresAt; + protected $refreshToken; + protected $userInfoName; + protected $userInfoDisplayName; + + private ICrypto $crypto; + + public function __construct() { + $this->addType('id', 'integer'); + $this->addType('url', 'string'); + $this->addType('token', 'string'); + $this->addType('token_type', 'string'); + $this->addType('token_expires_at', 'integer'); + $this->addType('refresh_token', 'string'); + $this->addType('user_info_name', 'string'); + $this->addType('user_info_display_name', 'string'); + + $this->crypto = \OC::$server->get(ICrypto::class); + } public function jsonSerialize(): array { return [ @@ -46,4 +62,34 @@ public function jsonSerialize(): array { 'userInfoDisplayName' => $this->userInfoDisplayName, ]; } + + public function getClearToken(): string { + if (is_string($this->token) && $this->token !== '') { + return $this->crypto->decrypt($this->token); + } + return $this->token; + } + + public function setEncryptedToken(string $token): void { + if ($token !== '') { + $this->setter('token', [$this->crypto->encrypt($token)]); + } else { + $this->setter('token', [$token]); + } + } + + public function getClearRefreshToken(): ?string { + if (is_string($this->refreshToken) && $this->refreshToken !== '') { + return $this->crypto->decrypt($this->refreshToken); + } + return $this->refreshToken; + } + + public function setEncryptedRefreshToken(?string $refreshToken): void { + if (is_string($refreshToken) && $refreshToken !== '') { + $this->setter('refreshToken', [$this->crypto->encrypt($refreshToken)]); + } else { + $this->setter('refreshToken', [$refreshToken]); + } + } } diff --git a/lib/Migration/MultiAccountRepairStep.php b/lib/Migration/MultiAccountRepairStep.php index 9a2c5e4..cffb16d 100644 --- a/lib/Migration/MultiAccountRepairStep.php +++ b/lib/Migration/MultiAccountRepairStep.php @@ -30,7 +30,7 @@ public function run(IOutput $output) { $account = new GitlabAccount(); $account->setUserId($user->getUID()); $account->setUrl($this->config->getUserUrl($userId)); - $account->setToken($this->config->getUserToken($userId)); + $account->setEncryptedToken($this->config->getUserToken($userId)); $account->setTokenType($this->config->getUserTokenType($userId)); if (!$account->getTokenType()) { if ($this->config->hasUserRefreshToken($userId)) { @@ -43,7 +43,7 @@ public function run(IOutput $output) { $account->setTokenExpiresAt($this->config->getUserTokenExpiresAt($userId)); } if ($this->config->hasUserRefreshToken($userId)) { - $account->setRefreshToken($this->config->getUserRefreshToken($userId)); + $account->setEncryptedRefreshToken($this->config->getUserRefreshToken($userId)); } $account->setUserInfoName($this->config->getUserName($userId)); $account->setUserInfoDisplayName($this->config->getUserDisplayName($userId)); diff --git a/lib/Migration/Version030103Date20241017150114.php b/lib/Migration/Version030103Date20241017150114.php new file mode 100644 index 0000000..f2a88e3 --- /dev/null +++ b/lib/Migration/Version030103Date20241017150114.php @@ -0,0 +1,97 @@ +config->getAppValue(Application::APP_ID, $key); + if ($value !== '') { + $encryptedValue = $this->crypto->encrypt($value); + $this->config->setAppValue(Application::APP_ID, $key, $encryptedValue); + } + } + + // ----------- user tokens + $qbUpdate = $this->connection->getQueryBuilder(); + $qbUpdate->update('gitlab_accounts') + ->set('token', $qbUpdate->createParameter('updateToken')) + ->where( + $qbUpdate->expr()->eq('id', $qbUpdate->createParameter('updateAccountId')) + ); + + $qbSelect = $this->connection->getQueryBuilder(); + $qbSelect->select('id', 'token') + ->from('gitlab_accounts') + ->where( + $qbSelect->expr()->nonEmptyString('token') + ); + $req = $qbSelect->executeQuery(); + while ($row = $req->fetch()) { + $accountId = (int)$row['id']; + $storedClearRefreshToken = $row['token']; + $encryptedToken = $this->crypto->encrypt($storedClearRefreshToken); + $qbUpdate->setParameter('updateAccountId', $accountId, IQueryBuilder::PARAM_INT); + $qbUpdate->setParameter('updateToken', $encryptedToken, IQueryBuilder::PARAM_STR); + $qbUpdate->executeStatement(); + } + $req->closeCursor(); + + // ----------- user refresh tokens + $qbUpdate = $this->connection->getQueryBuilder(); + $qbUpdate->update('gitlab_accounts') + ->set('refresh_token', $qbUpdate->createParameter('updateRefreshToken')) + ->where( + $qbUpdate->expr()->eq('id', $qbUpdate->createParameter('updateAccountId')) + ); + + $qbSelect = $this->connection->getQueryBuilder(); + $qbSelect->select('id', 'refresh_token') + ->from('gitlab_accounts') + ->where( + $qbSelect->expr()->nonEmptyString('refresh_token') + ) + ->andWhere( + $qbSelect->expr()->isNotNull('refresh_token') + ); + $req = $qbSelect->executeQuery(); + while ($row = $req->fetch()) { + $accountId = (int)$row['id']; + $storedClearRefreshToken = $row['refresh_token']; + $encryptedToken = $this->crypto->encrypt($storedClearRefreshToken); + $qbUpdate->setParameter('updateAccountId', $accountId, IQueryBuilder::PARAM_INT); + $qbUpdate->setParameter('updateRefreshToken', $encryptedToken, IQueryBuilder::PARAM_STR); + $qbUpdate->executeStatement(); + } + $req->closeCursor(); + } +} diff --git a/lib/Search/GitlabSearchIssuesProvider.php b/lib/Search/GitlabSearchIssuesProvider.php index fbd5f82..63d9654 100644 --- a/lib/Search/GitlabSearchIssuesProvider.php +++ b/lib/Search/GitlabSearchIssuesProvider.php @@ -102,7 +102,7 @@ public function search(IUser $user, ISearchQuery $query): SearchResult { $accounts = $this->accountMapper->find($user->getUID()); foreach ($accounts as $account) { - $accessToken = $account->getToken(); + $accessToken = $account->getClearToken(); if ($accessToken === '') { continue; } diff --git a/lib/Search/GitlabSearchMergeRequestsProvider.php b/lib/Search/GitlabSearchMergeRequestsProvider.php index a75a867..533541f 100644 --- a/lib/Search/GitlabSearchMergeRequestsProvider.php +++ b/lib/Search/GitlabSearchMergeRequestsProvider.php @@ -102,7 +102,7 @@ public function search(IUser $user, ISearchQuery $query): SearchResult { $accounts = $this->accountMapper->find($user->getUID()); foreach ($accounts as $account) { - $accessToken = $account->getToken(); + $accessToken = $account->getClearToken(); if ($accessToken === '') { continue; } diff --git a/lib/Search/GitlabSearchReposProvider.php b/lib/Search/GitlabSearchReposProvider.php index a1ceada..9fa957c 100644 --- a/lib/Search/GitlabSearchReposProvider.php +++ b/lib/Search/GitlabSearchReposProvider.php @@ -102,7 +102,7 @@ public function search(IUser $user, ISearchQuery $query): SearchResult { $accounts = $this->accountMapper->find($user->getUID()); foreach ($accounts as $account) { - $accessToken = $account->getToken(); + $accessToken = $account->getClearToken(); if ($accessToken === '') { continue; } diff --git a/lib/Service/ConfigService.php b/lib/Service/ConfigService.php index 83ab246..fc03c8b 100644 --- a/lib/Service/ConfigService.php +++ b/lib/Service/ConfigService.php @@ -7,19 +7,38 @@ use OCA\Gitlab\AppInfo\Application; use OCP\IConfig; use OCP\PreConditionNotMetException; +use OCP\Security\ICrypto; class ConfigService { public function __construct( private IConfig $config, + private ICrypto $crypto, ) { } + public function getEncryptedAppValue(string $key): string { + $value = $this->config->getAppValue(Application::APP_ID, $key); + if ($value === '') { + return $value; + } + return $this->crypto->decrypt($value); + } + + public function setEncryptedAppValue(string $key, string $value): void { + if ($value === '') { + $this->config->setAppValue(Application::APP_ID, $key, $value); + } else { + $encryptedValue = $this->crypto->encrypt($value); + $this->config->setAppValue(Application::APP_ID, $key, $encryptedValue); + } + } + public function getAdminClientId(): string { - return $this->config->getAppValue(Application::APP_ID, 'client_id'); + return $this->getEncryptedAppValue('client_id'); } public function setAdminClientId(string $clientId): void { - $this->config->setAppValue(Application::APP_ID, 'client_id', $clientId); + $this->setEncryptedAppValue('client_id', $clientId); } public function hasAdminClientSecret(): bool { @@ -27,11 +46,11 @@ public function hasAdminClientSecret(): bool { } public function getAdminClientSecret(): string { - return $this->config->getAppValue(Application::APP_ID, 'client_secret'); + return $this->getEncryptedAppValue('client_secret'); } public function setAdminClientSecret(string $clientSecret): void { - $this->config->setAppValue(Application::APP_ID, 'client_secret', $clientSecret); + $this->setEncryptedAppValue('client_secret', $clientSecret); } public function getAdminOauthUrl(): string { diff --git a/lib/Service/GitlabAPIService.php b/lib/Service/GitlabAPIService.php index 60d5b45..3b11854 100644 --- a/lib/Service/GitlabAPIService.php +++ b/lib/Service/GitlabAPIService.php @@ -243,7 +243,7 @@ public function request(?GitlabAccount $account, string $baseUrl, string $endPoi // try anonymous request if no user (public page) or user not connected to a gitlab account if ($account !== null) { - $accessToken = $account->getToken(); + $accessToken = $account->getClearToken(); if ($accessToken !== '') { $options['headers']['Authorization'] = 'Bearer ' . $accessToken; } @@ -301,7 +301,7 @@ public function request(?GitlabAccount $account, string $baseUrl, string $endPoi } private function checkTokenExpiration(GitlabAccount $account): void { - if ($account->getRefreshToken() && $account->getTokenExpiresAt()) { + if ($account->getClearRefreshToken() && $account->getTokenExpiresAt()) { $nowTs = (new DateTime())->getTimestamp(); // if token expires in less than a minute or is already expired if ($nowTs > $account->getTokenExpiresAt() - 60) { @@ -312,7 +312,7 @@ private function checkTokenExpiration(GitlabAccount $account): void { private function refreshToken(GitlabAccount $account): bool { $adminOauthUrl = $this->config->getAdminOauthUrl(); - $refreshToken = $account->getRefreshToken(); + $refreshToken = $account->getClearRefreshToken(); if (!$refreshToken) { $this->logger->error('No GitLab refresh token found', ['app' => Application::APP_ID]); return false; @@ -329,8 +329,8 @@ private function refreshToken(GitlabAccount $account): bool { $accessToken = $result['access_token']; $refreshToken = $result['refresh_token']; $account->setUrl($adminOauthUrl); - $account->setToken($accessToken); - $account->setRefreshToken($refreshToken); + $account->setEncryptedToken($accessToken); + $account->setEncryptedRefreshToken($refreshToken); if (isset($result['expires_in'])) { $nowTs = (new DateTime())->getTimestamp(); $expiresAt = $nowTs + (int) $result['expires_in']; @@ -361,7 +361,7 @@ public function revokeOauthToken(GitlabAccount $account): array { 'body' => json_encode([ 'client_id' => $this->config->getAdminClientId(), 'client_secret' => $this->config->getAdminClientSecret(), - 'token' => $account->getToken(), + 'token' => $account->getClearToken(), ]), ]; diff --git a/tests/unit/Controller/GitlabAPIControllerTest.php b/tests/unit/Controller/GitlabAPIControllerTest.php index 926300b..e75b9d1 100644 --- a/tests/unit/Controller/GitlabAPIControllerTest.php +++ b/tests/unit/Controller/GitlabAPIControllerTest.php @@ -62,7 +62,7 @@ protected function setUp(): void { $account = new GitlabAccount(); $account->setUrl('https://gitlab.com'); - $account->setToken(self::API_TOKEN); + $account->setEncryptedToken(self::API_TOKEN); $accountMapper = $this->createMock(GitlabAccountMapper::class); $accountMapper->method('findById')->willReturn($account);