Skip to content

Commit 293dcf9

Browse files
julien-ncjulien-nc
committed
encrypt client secrets
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
1 parent b313e2a commit 293dcf9

9 files changed

Lines changed: 117 additions & 47 deletions

File tree

‎apps/oauth2/composer/composer/LICENSE‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
Copyright (c) Nils Adermann, Jordi Boggiano
32

43
Permission is hereby granted, free of charge, to any person obtaining a copy
@@ -18,4 +17,3 @@ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
1817
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
1918
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
2019
THE SOFTWARE.
21-

‎apps/oauth2/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,6 @@
1919
'OCA\\OAuth2\\Migration\\SetTokenExpiration' => $baseDir . '/../lib/Migration/SetTokenExpiration.php',
2020
'OCA\\OAuth2\\Migration\\Version010401Date20181207190718' => $baseDir . '/../lib/Migration/Version010401Date20181207190718.php',
2121
'OCA\\OAuth2\\Migration\\Version010402Date20190107124745' => $baseDir . '/../lib/Migration/Version010402Date20190107124745.php',
22+
'OCA\\OAuth2\\Migration\\Version011601Date20230522143227' => $baseDir . '/../lib/Migration/Version011601Date20230522143227.php',
2223
'OCA\\OAuth2\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php',
2324
);

‎apps/oauth2/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ class ComposerStaticInitOAuth2
3434
'OCA\\OAuth2\\Migration\\SetTokenExpiration' => __DIR__ . '/..' . '/../lib/Migration/SetTokenExpiration.php',
3535
'OCA\\OAuth2\\Migration\\Version010401Date20181207190718' => __DIR__ . '/..' . '/../lib/Migration/Version010401Date20181207190718.php',
3636
'OCA\\OAuth2\\Migration\\Version010402Date20190107124745' => __DIR__ . '/..' . '/../lib/Migration/Version010402Date20190107124745.php',
37+
'OCA\\OAuth2\\Migration\\Version011601Date20230522143227' => __DIR__ . '/..' . '/../lib/Migration/Version011601Date20230522143227.php',
3738
'OCA\\OAuth2\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php',
3839
);
3940

‎apps/oauth2/lib/Controller/OauthApiController.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,8 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client
125125
}
126126

127127
// The client id and secret must match. Else we don't provide an access token!
128-
if ($client->getClientIdentifier() !== $client_id || $client->getSecret() !== $client_secret) {
128+
$storedClientSecret = $this->crypto->decrypt($client->getSecret());
129+
if ($client->getClientIdentifier() !== $client_id || $storedClientSecret !== $client_secret) {
129130
return new JSONResponse([
130131
'error' => 'invalid_client',
131132
], Http::STATUS_BAD_REQUEST);

‎apps/oauth2/lib/Controller/SettingsController.php‎

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -41,41 +41,25 @@
4141
use OCP\IRequest;
4242
use OCP\IUser;
4343
use OCP\IUserManager;
44+
use OCP\Security\ICrypto;
4445
use OCP\Security\ISecureRandom;
4546

4647
class SettingsController extends Controller {
47-
/** @var ClientMapper */
48-
private $clientMapper;
49-
/** @var ISecureRandom */
50-
private $secureRandom;
51-
/** @var AccessTokenMapper */
52-
private $accessTokenMapper;
53-
/** @var IL10N */
54-
private $l;
55-
/** @var IAuthTokenProvider */
56-
private $tokenProvider;
57-
/**
58-
* @var IUserManager
59-
*/
60-
private $userManager;
48+
6149
public const validChars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
6250

63-
public function __construct(string $appName,
64-
IRequest $request,
65-
ClientMapper $clientMapper,
66-
ISecureRandom $secureRandom,
67-
AccessTokenMapper $accessTokenMapper,
68-
IL10N $l,
69-
IAuthTokenProvider $tokenProvider,
70-
IUserManager $userManager
51+
public function __construct(
52+
string $appName,
53+
IRequest $request,
54+
private ClientMapper $clientMapper,
55+
private ISecureRandom $secureRandom,
56+
private AccessTokenMapper $accessTokenMapper,
57+
private IL10N $l,
58+
private IAuthTokenProvider $tokenProvider,
59+
private IUserManager $userManager,
60+
private ICrypto $crypto
7161
) {
7262
parent::__construct($appName, $request);
73-
$this->secureRandom = $secureRandom;
74-
$this->clientMapper = $clientMapper;
75-
$this->accessTokenMapper = $accessTokenMapper;
76-
$this->l = $l;
77-
$this->tokenProvider = $tokenProvider;
78-
$this->userManager = $userManager;
7963
}
8064

8165
public function addClient(string $name,
@@ -87,7 +71,9 @@ public function addClient(string $name,
8771
$client = new Client();
8872
$client->setName($name);
8973
$client->setRedirectUri($redirectUri);
90-
$client->setSecret($this->secureRandom->generate(64, self::validChars));
74+
$secret = $this->secureRandom->generate(64, self::validChars);
75+
$encryptedSecret = $this->crypto->encrypt($secret);
76+
$client->setSecret($encryptedSecret);
9177
$client->setClientIdentifier($this->secureRandom->generate(64, self::validChars));
9278
$client = $this->clientMapper->insert($client);
9379

@@ -96,7 +82,7 @@ public function addClient(string $name,
9682
'name' => $client->getName(),
9783
'redirectUri' => $client->getRedirectUri(),
9884
'clientId' => $client->getClientIdentifier(),
99-
'clientSecret' => $client->getSecret(),
85+
'clientSecret' => $secret,
10086
];
10187

10288
return new JSONResponse($result);

‎apps/oauth2/lib/Migration/Version011601Date20230522143227.php‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* @copyright Copyright 2023, Julien Veyssier <julien-nc@posteo.net>
7+
*
8+
* @author Julien Veyssier <julien-nc@posteo.net>
9+
*
10+
* @license GNU AGPL version 3 or any later version
11+
*
12+
* This program is free software: you can redistribute it and/or modify
13+
* it under the terms of the GNU Affero General Public License as
14+
* published by the Free Software Foundation, either version 3 of the
15+
* License, or (at your option) any later version.
16+
*
17+
* This program is distributed in the hope that it will be useful,
18+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
19+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
20+
* GNU Affero General Public License for more details.
21+
*
22+
* You should have received a copy of the GNU Affero General Public License
23+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
24+
*
25+
*/
26+
namespace OCA\OAuth2\Migration;
27+
28+
use Closure;
29+
use OCP\DB\ISchemaWrapper;
30+
use OCP\DB\QueryBuilder\IQueryBuilder;
31+
use OCP\IDBConnection;
32+
use OCP\Migration\IOutput;
33+
use OCP\Migration\SimpleMigrationStep;
34+
use OCP\Security\ICrypto;
35+
36+
class Version011601Date20230522143227 extends SimpleMigrationStep {
37+
38+
public function __construct(
39+
private IDBConnection $connection,
40+
private ICrypto $crypto,
41+
) {
42+
}
43+
44+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) {
45+
/** @var ISchemaWrapper $schema */
46+
$schema = $schemaClosure();
47+
48+
if ($schema->hasTable('oauth2_clients')) {
49+
$table = $schema->getTable('oauth2_clients');
50+
if ($table->hasColumn('secret')) {
51+
$column = $table->getColumn('secret');
52+
$column->setLength(256);
53+
return $schema;
54+
}
55+
}
56+
57+
return null;
58+
}
59+
60+
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) {
61+
$qbUpdate = $this->connection->getQueryBuilder();
62+
$qbUpdate->update('oauth2_clients')
63+
->set('secret', $qbUpdate->createParameter('updateSecret'))
64+
->where(
65+
$qbUpdate->expr()->eq('id', $qbUpdate->createParameter('updateId'))
66+
);
67+
68+
$qbSelect = $this->connection->getQueryBuilder();
69+
$qbSelect->select('id', 'secret')
70+
->from('oauth2_clients');
71+
$req = $qbSelect->executeQuery();
72+
while ($row = $req->fetch()) {
73+
$id = $row['id'];
74+
$secret = $row['secret'];
75+
$encryptedSecret = $this->crypto->encrypt($secret);
76+
$qbUpdate->setParameter('updateSecret', $encryptedSecret);
77+
$qbUpdate->setParameter('updateId', $id);
78+
$qbUpdate->executeStatement();
79+
}
80+
$req->closeCursor();
81+
}
82+
}

‎apps/oauth2/lib/Settings/Admin.php‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,35 +29,32 @@
2929
use OCA\OAuth2\Db\ClientMapper;
3030
use OCP\AppFramework\Http\TemplateResponse;
3131
use OCP\AppFramework\Services\IInitialState;
32+
use OCP\Security\ICrypto;
3233
use OCP\Settings\ISettings;
3334
use OCP\IURLGenerator;
3435

3536
class Admin implements ISettings {
36-
private IInitialState $initialState;
37-
private ClientMapper $clientMapper;
38-
private IURLGenerator $urlGenerator;
3937

4038
public function __construct(
41-
IInitialState $initialState,
42-
ClientMapper $clientMapper,
43-
IURLGenerator $urlGenerator
39+
private IInitialState $initialState,
40+
private ClientMapper $clientMapper,
41+
private IURLGenerator $urlGenerator,
42+
private ICrypto $crypto
4443
) {
45-
$this->initialState = $initialState;
46-
$this->clientMapper = $clientMapper;
47-
$this->urlGenerator = $urlGenerator;
4844
}
4945

5046
public function getForm(): TemplateResponse {
5147
$clients = $this->clientMapper->getClients();
5248
$result = [];
5349

5450
foreach ($clients as $client) {
51+
$secret = $this->crypto->decrypt($client->getSecret());
5552
$result[] = [
5653
'id' => $client->getId(),
5754
'name' => $client->getName(),
5855
'redirectUri' => $client->getRedirectUri(),
5956
'clientId' => $client->getClientIdentifier(),
60-
'clientSecret' => $client->getSecret(),
57+
'clientSecret' => $secret,
6158
];
6259
}
6360
$this->initialState->provideInitialState('clients', $result);

‎apps/oauth2/tests/Controller/SettingsControllerTest.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
use OCP\IRequest;
3939
use OCP\IUser;
4040
use OCP\IUserManager;
41+
use OCP\Security\ICrypto;
4142
use OCP\Security\ISecureRandom;
4243
use Test\TestCase;
4344

@@ -61,6 +62,8 @@ class SettingsControllerTest extends TestCase {
6162
private $settingsController;
6263
/** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */
6364
private $l;
65+
/** @var ICrypto|\PHPUnit\Framework\MockObject\MockObject */
66+
private $crypto;
6467

6568
protected function setUp(): void {
6669
parent::setUp();
@@ -71,6 +74,7 @@ protected function setUp(): void {
7174
$this->accessTokenMapper = $this->createMock(AccessTokenMapper::class);
7275
$this->authTokenProvider = $this->createMock(IAuthTokenProvider::class);
7376
$this->userManager = $this->createMock(IUserManager::class);
77+
$this->crypto = $this->createMock(ICrypto::class);
7478
$this->l = $this->createMock(IL10N::class);
7579
$this->l->method('t')
7680
->willReturnArgument(0);
@@ -82,7 +86,8 @@ protected function setUp(): void {
8286
$this->accessTokenMapper,
8387
$this->l,
8488
$this->authTokenProvider,
85-
$this->userManager
89+
$this->userManager,
90+
$this->crypto
8691
);
8792

8893
}
@@ -175,7 +180,8 @@ public function testDeleteClient() {
175180
$this->accessTokenMapper,
176181
$this->l,
177182
$tokenProviderMock,
178-
$userManager
183+
$userManager,
184+
$this->crypto
179185
);
180186

181187
$result = $settingsController->deleteClient(123);

‎lib/composer/composer/LICENSE‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
Copyright (c) Nils Adermann, Jordi Boggiano
32

43
Permission is hereby granted, free of charge, to any person obtaining a copy
@@ -18,4 +17,3 @@ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
1817
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
1918
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
2019
THE SOFTWARE.
21-

0 commit comments

Comments
 (0)