Skip to content

Commit 39beecd

Browse files
julien-ncjulien-nc
committed
encrypt oauth2 client secrets
Signed-off-by: Julien Veyssier <[email protected]>
1 parent add4e43 commit 39beecd

File tree

12 files changed

+215
-111
lines changed

12 files changed

+215
-111
lines changed

‎apps/oauth2/appinfo/info.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<name>OAuth 2.0</name>
66
<summary>Allows OAuth2 compatible authentication from other web applications.</summary>
77
<description>The OAuth2 app allows administrators to configure the built-in authentication workflow to also allow OAuth2 compatible authentication from other web applications.</description>
8-
<version>1.15.0</version>
8+
<version>1.15.1</version>
99
<licence>agpl</licence>
1010
<author>Lukas Reschke</author>
1111
<namespace>OAuth2</namespace>

‎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: 23 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -42,40 +42,23 @@
4242
use OCP\IRequest;
4343
use OCP\Security\ICrypto;
4444
use OCP\Security\ISecureRandom;
45+
use Psr\Log\LoggerInterface;
4546

4647
class OauthApiController extends Controller {
47-
/** @var AccessTokenMapper */
48-
private $accessTokenMapper;
49-
/** @var ClientMapper */
50-
private $clientMapper;
51-
/** @var ICrypto */
52-
private $crypto;
53-
/** @var TokenProvider */
54-
private $tokenProvider;
55-
/** @var ISecureRandom */
56-
private $secureRandom;
57-
/** @var ITimeFactory */
58-
private $time;
59-
/** @var Throttler */
60-
private $throttler;
61-
62-
public function __construct(string $appName,
63-
IRequest $request,
64-
ICrypto $crypto,
65-
AccessTokenMapper $accessTokenMapper,
66-
ClientMapper $clientMapper,
67-
TokenProvider $tokenProvider,
68-
ISecureRandom $secureRandom,
69-
ITimeFactory $time,
70-
Throttler $throttler) {
48+
49+
public function __construct(
50+
string $appName,
51+
IRequest $request,
52+
private ICrypto $crypto,
53+
private AccessTokenMapper $accessTokenMapper,
54+
private ClientMapper $clientMapper,
55+
private TokenProvider $tokenProvider,
56+
private ISecureRandom $secureRandom,
57+
private ITimeFactory $time,
58+
private LoggerInterface $logger,
59+
private Throttler $throttler
60+
) {
7161
parent::__construct($appName, $request);
72-
$this->crypto = $crypto;
73-
$this->accessTokenMapper = $accessTokenMapper;
74-
$this->clientMapper = $clientMapper;
75-
$this->tokenProvider = $tokenProvider;
76-
$this->secureRandom = $secureRandom;
77-
$this->time = $time;
78-
$this->throttler = $throttler;
7962
}
8063

8164
/**
@@ -124,8 +107,16 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client
124107
$client_secret = $this->request->server['PHP_AUTH_PW'];
125108
}
126109

110+
try {
111+
$storedClientSecret = $this->crypto->decrypt($client->getSecret());
112+
} catch (\Exception $e) {
113+
$this->logger->error('OAuth client secret decryption error', ['exception' => $e]);
114+
return new JSONResponse([
115+
'error' => 'invalid_client',
116+
], Http::STATUS_BAD_REQUEST);
117+
}
127118
// 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) {
119+
if ($client->getClientIdentifier() !== $client_id || $storedClientSecret !== $client_secret) {
129120
return new JSONResponse([
130121
'error' => 'invalid_client',
131122
], 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 <[email protected]>
7+
*
8+
* @author Julien Veyssier <[email protected]>
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(512);
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, IQueryBuilder::PARAM_STR);
77+
$qbUpdate->setParameter('updateId', $id, IQueryBuilder::PARAM_INT);
78+
$qbUpdate->executeStatement();
79+
}
80+
$req->closeCursor();
81+
}
82+
}

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

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,36 +29,39 @@
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;
35+
use Psr\Log\LoggerInterface;
3436

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

4039
public function __construct(
41-
IInitialState $initialState,
42-
ClientMapper $clientMapper,
43-
IURLGenerator $urlGenerator
40+
private IInitialState $initialState,
41+
private ClientMapper $clientMapper,
42+
private IURLGenerator $urlGenerator,
43+
private ICrypto $crypto,
44+
private LoggerInterface $logger,
4445
) {
45-
$this->initialState = $initialState;
46-
$this->clientMapper = $clientMapper;
47-
$this->urlGenerator = $urlGenerator;
4846
}
4947

5048
public function getForm(): TemplateResponse {
5149
$clients = $this->clientMapper->getClients();
5250
$result = [];
5351

5452
foreach ($clients as $client) {
55-
$result[] = [
56-
'id' => $client->getId(),
57-
'name' => $client->getName(),
58-
'redirectUri' => $client->getRedirectUri(),
59-
'clientId' => $client->getClientIdentifier(),
60-
'clientSecret' => $client->getSecret(),
61-
];
53+
try {
54+
$secret = $this->crypto->decrypt($client->getSecret());
55+
$result[] = [
56+
'id' => $client->getId(),
57+
'name' => $client->getName(),
58+
'redirectUri' => $client->getRedirectUri(),
59+
'clientId' => $client->getClientIdentifier(),
60+
'clientSecret' => $secret,
61+
];
62+
} catch (\Exception $e) {
63+
$this->logger->error('[Settings] OAuth client secret decryption error', ['exception' => $e]);
64+
}
6265
}
6366
$this->initialState->provideInitialState('clients', $result);
6467
$this->initialState->provideInitialState('oauth2-doc-link', $this->urlGenerator->linkToDocs('admin-oauth2'));

0 commit comments

Comments
 (0)