Skip to content

Commit a0deed1

Browse files
julien-ncjulien-nc
committed
encrypt tokens, client id and client secret. bump min NC version to 27
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
1 parent 3444a7b commit a0deed1

File tree

10 files changed

+281
-85
lines changed

10 files changed

+281
-85
lines changed

‎appinfo/info.xml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
<description><![CDATA[GitHub integration provides a dashboard widget displaying your most important notifications
1111
and a unified search provider for repositories, issues and pull requests. It also provides a link reference provider
1212
to render links to issues, pull requests and comments in Talk and Text.]]></description>
13-
<version>3.0.0</version>
13+
<version>3.0.1</version>
1414
<licence>agpl</licence>
1515
<author>Julien Veyssier</author>
1616
<namespace>Github</namespace>
@@ -23,7 +23,7 @@
2323
<bugs>https://github.com/nextcloud/integration_github/issues</bugs>
2424
<screenshot>https://github.com/nextcloud/integration_github/raw/main/img/screenshot1.jpg</screenshot>
2525
<dependencies>
26-
<nextcloud min-version="26" max-version="31"/>
26+
<nextcloud min-version="27" max-version="31"/>
2727
</dependencies>
2828
<settings>
2929
<admin>OCA\Github\Settings\Admin</admin>

‎lib/Controller/ConfigController.php‎

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99
use OCA\Github\AppInfo\Application;
1010
use OCA\Github\Reference\GithubIssuePrReferenceProvider;
1111
use OCA\Github\Service\GithubAPIService;
12+
use OCA\Github\Service\SecretService;
1213
use OCP\AppFramework\Controller;
14+
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
15+
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
1316
use OCP\AppFramework\Http\DataResponse;
1417
use OCP\AppFramework\Http\RedirectResponse;
1518
use OCP\AppFramework\Http\TemplateResponse;
@@ -24,27 +27,28 @@
2427
class ConfigController extends Controller {
2528

2629
public function __construct(
27-
string $appName,
28-
IRequest $request,
29-
private IConfig $config,
30-
private IURLGenerator $urlGenerator,
31-
private IL10N $l,
32-
private IInitialState $initialStateService,
33-
private GithubAPIService $githubAPIService,
30+
string $appName,
31+
IRequest $request,
32+
private IConfig $config,
33+
private IURLGenerator $urlGenerator,
34+
private IL10N $l,
35+
private IInitialState $initialStateService,
36+
private GithubAPIService $githubAPIService,
37+
private SecretService $secretService,
3438
private GithubIssuePrReferenceProvider $githubIssuePrReferenceProvider,
35-
private ?string $userId,
39+
private ?string $userId,
3640
) {
3741
parent::__construct($appName, $request);
3842
}
3943

4044
/**
41-
* @NoAdminRequired
4245
* Set config values
4346
*
4447
* @param array $values key/value pairs to store in user preferences
4548
* @return DataResponse
4649
* @throws PreConditionNotMetException
4750
*/
51+
#[NoAdminRequired]
4852
public function setConfig(array $values): DataResponse {
4953
// revoke the oauth token if needed
5054
if (isset($values['token']) && $values['token'] === '') {
@@ -56,7 +60,11 @@ public function setConfig(array $values): DataResponse {
5660

5761
// save values
5862
foreach ($values as $key => $value) {
59-
$this->config->setUserValue($this->userId, Application::APP_ID, $key, $value);
63+
if ($key === 'token') {
64+
$this->secretService->setEncryptedUserValue($this->userId, $key, $value);
65+
} else {
66+
$this->config->setUserValue($this->userId, Application::APP_ID, $key, $value);
67+
}
6068
}
6169
$result = [];
6270

@@ -76,6 +84,7 @@ public function setConfig(array $values): DataResponse {
7684
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'user_name');
7785
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'user_displayname');
7886
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'token_type');
87+
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'token');
7988
$result['user_name'] = '';
8089
}
8190
// connect or disconnect: invalidate the user-related cache
@@ -92,39 +101,41 @@ public function setConfig(array $values): DataResponse {
92101
*/
93102
public function setAdminConfig(array $values): DataResponse {
94103
foreach ($values as $key => $value) {
95-
$this->config->setAppValue(Application::APP_ID, $key, $value);
104+
if (in_array($key, ['client_id', 'client_secret', 'default_link_token'], true)) {
105+
$this->secretService->setEncryptedAppValue($key, $value);
106+
} else {
107+
$this->config->setAppValue(Application::APP_ID, $key, $value);
108+
}
96109
}
97110
return new DataResponse(1);
98111
}
99112

100113
/**
101-
* @NoAdminRequired
102-
* @NoCSRFRequired
103-
*
104114
* @param string $user_name
105115
* @param string $user_displayname
106116
* @return TemplateResponse
107117
*/
118+
#[NoAdminRequired]
119+
#[NoCSRFRequired]
108120
public function popupSuccessPage(string $user_name, string $user_displayname): TemplateResponse {
109121
$this->initialStateService->provideInitialState('popup-data', ['user_name' => $user_name, 'user_displayname' => $user_displayname]);
110122
return new TemplateResponse(Application::APP_ID, 'popupSuccess', [], TemplateResponse::RENDER_AS_GUEST);
111123
}
112124

113125
/**
114-
* @NoAdminRequired
115-
* @NoCSRFRequired
116-
*
117126
* Receive oauth code and get oauth access token
118127
*
119128
* @param string $code request code to use when requesting oauth token
120129
* @param string $state value that was sent with original GET request. Used to check auth redirection is valid
121130
* @return RedirectResponse to user settings
122131
* @throws PreConditionNotMetException
123132
*/
133+
#[NoAdminRequired]
134+
#[NoCSRFRequired]
124135
public function oauthRedirect(string $code, string $state): RedirectResponse {
125136
$configState = $this->config->getUserValue($this->userId, Application::APP_ID, 'oauth_state');
126-
$clientID = $this->config->getAppValue(Application::APP_ID, 'client_id');
127-
$clientSecret = $this->config->getAppValue(Application::APP_ID, 'client_secret');
137+
$clientID = $this->secretService->getEncryptedAppValue('client_id');
138+
$clientSecret = $this->secretService->getEncryptedAppValue('client_secret');
128139

129140
// anyway, reset state
130141
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'oauth_state');
@@ -139,7 +150,7 @@ public function oauthRedirect(string $code, string $state): RedirectResponse {
139150
if (isset($result['access_token'])) {
140151
$this->githubIssuePrReferenceProvider->invalidateUserCache($this->userId);
141152
$accessToken = $result['access_token'];
142-
$this->config->setUserValue($this->userId, Application::APP_ID, 'token', $accessToken);
153+
$this->secretService->setEncryptedUserValue($this->userId, 'token', $accessToken);
143154
$this->config->setUserValue($this->userId, Application::APP_ID, 'token_type', 'oauth');
144155
$userInfo = $this->storeUserInfo();
145156

‎lib/Migration/Version030010Date20241002015812.php‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace OCA\Github\Migration;
6+
7+
use Closure;
8+
use OCA\Github\AppInfo\Application;
9+
use OCP\DB\QueryBuilder\IQueryBuilder;
10+
use OCP\IConfig;
11+
use OCP\IDBConnection;
12+
use OCP\Migration\IOutput;
13+
use OCP\Migration\SimpleMigrationStep;
14+
use OCP\Security\ICrypto;
15+
16+
class Version030010Date20241002015812 extends SimpleMigrationStep {
17+
18+
public function __construct(
19+
private IDBConnection $connection,
20+
private ICrypto $crypto,
21+
private IConfig $config,
22+
) {
23+
}
24+
25+
/**
26+
* @param IOutput $output
27+
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
28+
* @param array $options
29+
*/
30+
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) {
31+
// client_id, client_secret, default_link_token
32+
foreach (['client_id', 'client_secret', 'default_link_token'] as $key) {
33+
$value = $this->config->getAppValue(Application::APP_ID, $key);
34+
if ($value !== '') {
35+
$encryptedValue = $this->crypto->encrypt($value);
36+
$this->config->setAppValue(Application::APP_ID, $key, $encryptedValue);
37+
}
38+
}
39+
40+
// user tokens
41+
$qbUpdate = $this->connection->getQueryBuilder();
42+
$qbUpdate->update('preferences')
43+
->set('configvalue', $qbUpdate->createParameter('updateValue'))
44+
->where(
45+
$qbUpdate->expr()->eq('appid', $qbUpdate->createNamedParameter(Application::APP_ID, IQueryBuilder::PARAM_STR))
46+
)
47+
->andWhere(
48+
$qbUpdate->expr()->eq('userid', $qbUpdate->createParameter('updateUserId'))
49+
)
50+
->andWhere(
51+
$qbUpdate->expr()->eq('configkey', $qbUpdate->createNamedParameter('token', IQueryBuilder::PARAM_STR))
52+
);
53+
54+
$qbSelect = $this->connection->getQueryBuilder();
55+
$qbSelect->select('userid', 'configvalue')
56+
->from('preferences')
57+
->where(
58+
$qbSelect->expr()->eq('appid', $qbSelect->createNamedParameter(Application::APP_ID, IQueryBuilder::PARAM_STR))
59+
)
60+
->andWhere(
61+
$qbSelect->expr()->eq('configkey', $qbSelect->createNamedParameter('token', IQueryBuilder::PARAM_STR))
62+
)
63+
->andWhere(
64+
$qbSelect->expr()->nonEmptyString('configvalue')
65+
)
66+
->andWhere(
67+
$qbSelect->expr()->isNotNull('configvalue')
68+
);
69+
$req = $qbSelect->executeQuery();
70+
while ($row = $req->fetch()) {
71+
$userId = $row['userid'];
72+
$storedClearToken = $row['configvalue'];
73+
$encryptedToken = $this->crypto->encrypt($storedClearToken);
74+
$qbUpdate->setParameter('updateValue', $encryptedToken, IQueryBuilder::PARAM_STR);
75+
$qbUpdate->setParameter('updateUserId', $userId, IQueryBuilder::PARAM_STR);
76+
$qbUpdate->executeStatement();
77+
}
78+
$req->closeCursor();
79+
}
80+
}

‎lib/Search/GithubSearchIssuesProvider.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
use OCA\Github\AppInfo\Application;
1212
use OCA\Github\Service\GithubAPIService;
13+
use OCA\Github\Service\SecretService;
1314
use OCP\App\IAppManager;
1415
use OCP\IConfig;
1516
use OCP\IL10N;
@@ -28,6 +29,7 @@ public function __construct(
2829
private IConfig $config,
2930
private IURLGenerator $urlGenerator,
3031
private GithubAPIService $service,
32+
private SecretService $secretService,
3133
) {
3234
}
3335

@@ -78,7 +80,7 @@ public function search(IUser $user, ISearchQuery $query): SearchResult {
7880
return SearchResult::paginated($this->getName(), [], 0);
7981
}
8082

81-
$accessToken = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'token');
83+
$accessToken = $this->secretService->getAccessToken($user->getUID());
8284
if ($accessToken === '') {
8385
return SearchResult::paginated($this->getName(), [], 0);
8486
}

‎lib/Search/GithubSearchReposProvider.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
use OCA\Github\AppInfo\Application;
1212
use OCA\Github\Service\GithubAPIService;
13+
use OCA\Github\Service\SecretService;
1314
use OCP\App\IAppManager;
1415
use OCP\IConfig;
1516
use OCP\IL10N;
@@ -28,6 +29,7 @@ public function __construct(
2829
private IConfig $config,
2930
private IURLGenerator $urlGenerator,
3031
private GithubAPIService $service,
32+
private SecretService $secretService,
3133
) {
3234
}
3335

@@ -78,7 +80,7 @@ public function search(IUser $user, ISearchQuery $query): SearchResult {
7880
return SearchResult::paginated($this->getName(), [], 0);
7981
}
8082

81-
$accessToken = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'token');
83+
$accessToken = $this->secretService->getAccessToken($user->getUID());
8284
if ($accessToken === '') {
8385
return SearchResult::paginated($this->getName(), [], 0);
8486
}

‎lib/Service/GithubAPIService.php‎

Lines changed: 9 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,12 @@ class GithubAPIService {
3030
private IClient $client;
3131

3232
public function __construct(
33-
string $appName,
33+
private SecretService $secretService,
3434
private LoggerInterface $logger,
35-
private IL10N $l10n,
36-
private IConfig $config,
37-
private IURLGenerator $urlGenerator,
38-
private IUserManager $userManager,
39-
IClientService $clientService,
35+
private IL10N $l10n,
36+
private IConfig $config,
37+
private IURLGenerator $urlGenerator,
38+
IClientService $clientService,
4039
) {
4140
$this->client = $clientService->newClient();
4241
}
@@ -359,43 +358,6 @@ public function getCommitInfo(?string $userId, string $owner, string $repo, stri
359358
return $this->request($userId, $endpoint, [], 'GET', true, 5);
360359
}
361360

362-
/**
363-
* Get the user access token
364-
* If there is none, get the default one, check:
365-
* - if we use it for this endpoint
366-
* - if user is anonymous
367-
* - if user is a guest
368-
* @param string|null $userId
369-
* @param bool $endpointUsesDefaultToken
370-
* @return string
371-
*/
372-
public function getAccessToken(?string $userId, bool $endpointUsesDefaultToken = false): string {
373-
// use user access token in priority
374-
$accessToken = '';
375-
// for logged in users
376-
if ($userId !== null) {
377-
$accessToken = $this->config->getUserValue($userId, Application::APP_ID, 'token');
378-
// fallback to admin default token if $useDefaultToken
379-
if ($accessToken === '' && $endpointUsesDefaultToken) {
380-
$user = $this->userManager->get($userId);
381-
$isGuestUser = $user->getBackendClassName() === 'Guests';
382-
$allowDefaultTokenToGuests = $this->config->getAppValue(Application::APP_ID, 'allow_default_link_token_to_guests', '0') === '1';
383-
384-
if ((!$isGuestUser) || $allowDefaultTokenToGuests) {
385-
$accessToken = $this->config->getAppValue(Application::APP_ID, 'default_link_token');
386-
}
387-
}
388-
} elseif ($endpointUsesDefaultToken) {
389-
// anonymous users
390-
$allowDefaultTokenToAnonymous = $this->config->getAppValue(Application::APP_ID, 'allow_default_link_token_to_anonymous', '0') === '1';
391-
if ($allowDefaultTokenToAnonymous) {
392-
$accessToken = $this->config->getAppValue(Application::APP_ID, 'default_link_token');
393-
}
394-
}
395-
396-
return $accessToken;
397-
}
398-
399361
/**
400362
* Make an authenticated HTTP request to GitHub API
401363
* @param string|null $userId
@@ -416,7 +378,7 @@ public function request(?string $userId, string $endPoint, array $params = [], s
416378
'User-Agent' => 'Nextcloud GitHub integration',
417379
],
418380
];
419-
$accessToken = $this->getAccessToken($userId, $endpointUsesDefaultToken);
381+
$accessToken = $this->secretService->getAccessToken($userId, $endpointUsesDefaultToken);
420382
if ($accessToken !== '') {
421383
$options['headers']['Authorization'] = 'token ' . $accessToken;
422384
}
@@ -469,9 +431,9 @@ public function request(?string $userId, string $endPoint, array $params = [], s
469431
}
470432

471433
public function revokeOauthToken(string $userId): array {
472-
$accessToken = $this->config->getUserValue($userId, Application::APP_ID, 'token');
473-
$clientId = $this->config->getAppValue(Application::APP_ID, 'client_id');
474-
$clientSecret = $this->config->getAppValue(Application::APP_ID, 'client_secret');
434+
$accessToken = $this->secretService->getEncryptedUserValue($userId, 'token');
435+
$clientId = $this->secretService->getEncryptedAppValue('client_id');
436+
$clientSecret = $this->secretService->getEncryptedAppValue('client_secret');
475437
$endPoint = 'applications/' . $clientId . '/token';
476438
try {
477439
$url = 'https://api.github.com/' . $endPoint;

0 commit comments

Comments
 (0)