diff --git a/apps/oauth2/appinfo/info.xml b/apps/oauth2/appinfo/info.xml index fd17afbb11f73..65e0886c68a4e 100644 --- a/apps/oauth2/appinfo/info.xml +++ b/apps/oauth2/appinfo/info.xml @@ -5,7 +5,7 @@ OAuth 2.0 Allows OAuth2 compatible authentication from other web applications. The OAuth2 app allows administrators to configure the built-in authentication workflow to also allow OAuth2 compatible authentication from other web applications. - 1.16.3 + 1.16.4 agpl Lukas Reschke OAuth2 diff --git a/apps/oauth2/composer/composer/autoload_classmap.php b/apps/oauth2/composer/composer/autoload_classmap.php index 536342c075113..cba0ef43dfc2a 100644 --- a/apps/oauth2/composer/composer/autoload_classmap.php +++ b/apps/oauth2/composer/composer/autoload_classmap.php @@ -23,5 +23,6 @@ 'OCA\\OAuth2\\Migration\\Version011601Date20230522143227' => $baseDir . '/../lib/Migration/Version011601Date20230522143227.php', 'OCA\\OAuth2\\Migration\\Version011602Date20230613160650' => $baseDir . '/../lib/Migration/Version011602Date20230613160650.php', 'OCA\\OAuth2\\Migration\\Version011603Date20230620111039' => $baseDir . '/../lib/Migration/Version011603Date20230620111039.php', + 'OCA\\OAuth2\\Migration\\Version011901Date20240829164356' => $baseDir . '/../lib/Migration/Version011901Date20240829164356.php', 'OCA\\OAuth2\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php', ); diff --git a/apps/oauth2/composer/composer/autoload_static.php b/apps/oauth2/composer/composer/autoload_static.php index 1dc659778a3d3..c4905bc96c477 100644 --- a/apps/oauth2/composer/composer/autoload_static.php +++ b/apps/oauth2/composer/composer/autoload_static.php @@ -38,6 +38,7 @@ class ComposerStaticInitOAuth2 'OCA\\OAuth2\\Migration\\Version011601Date20230522143227' => __DIR__ . '/..' . '/../lib/Migration/Version011601Date20230522143227.php', 'OCA\\OAuth2\\Migration\\Version011602Date20230613160650' => __DIR__ . '/..' . '/../lib/Migration/Version011602Date20230613160650.php', 'OCA\\OAuth2\\Migration\\Version011603Date20230620111039' => __DIR__ . '/..' . '/../lib/Migration/Version011603Date20230620111039.php', + 'OCA\\OAuth2\\Migration\\Version011901Date20240829164356' => __DIR__ . '/..' . '/../lib/Migration/Version011901Date20240829164356.php', 'OCA\\OAuth2\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php', ); diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 46b68b1d5859a..0ce607edb5c22 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -156,7 +156,8 @@ public function getToken( } try { - $storedClientSecret = $this->crypto->decrypt($client->getSecret()); + $storedClientSecretHash = $client->getSecret(); + $clientSecretHash = bin2hex($this->crypto->calculateHMAC($client_secret)); } catch (\Exception $e) { $this->logger->error('OAuth client secret decryption error', ['exception' => $e]); // we don't throttle here because it might not be a bruteforce attack @@ -165,7 +166,7 @@ public function getToken( ], Http::STATUS_BAD_REQUEST); } // The client id and secret must match. Else we don't provide an access token! - if ($client->getClientIdentifier() !== $client_id || $storedClientSecret !== $client_secret) { + if ($client->getClientIdentifier() !== $client_id || $storedClientSecretHash !== $clientSecretHash) { $response = new JSONResponse([ 'error' => 'invalid_client', ], Http::STATUS_BAD_REQUEST); diff --git a/apps/oauth2/lib/Controller/SettingsController.php b/apps/oauth2/lib/Controller/SettingsController.php index d49dd6da0381b..5d00e23382810 100644 --- a/apps/oauth2/lib/Controller/SettingsController.php +++ b/apps/oauth2/lib/Controller/SettingsController.php @@ -72,8 +72,8 @@ public function addClient(string $name, $client->setName($name); $client->setRedirectUri($redirectUri); $secret = $this->secureRandom->generate(64, self::validChars); - $encryptedSecret = $this->crypto->encrypt($secret); - $client->setSecret($encryptedSecret); + $hashedSecret = bin2hex($this->crypto->calculateHMAC($secret)); + $client->setSecret($hashedSecret); $client->setClientIdentifier($this->secureRandom->generate(64, self::validChars)); $client = $this->clientMapper->insert($client); diff --git a/apps/oauth2/lib/Migration/Version011901Date20240829164356.php b/apps/oauth2/lib/Migration/Version011901Date20240829164356.php new file mode 100644 index 0000000000000..20f5754bf1138 --- /dev/null +++ b/apps/oauth2/lib/Migration/Version011901Date20240829164356.php @@ -0,0 +1,49 @@ +connection->getQueryBuilder(); + $qbUpdate->update('oauth2_clients') + ->set('secret', $qbUpdate->createParameter('updateSecret')) + ->where( + $qbUpdate->expr()->eq('id', $qbUpdate->createParameter('updateId')) + ); + + $qbSelect = $this->connection->getQueryBuilder(); + $qbSelect->select('id', 'secret') + ->from('oauth2_clients'); + $req = $qbSelect->executeQuery(); + while ($row = $req->fetch()) { + $id = $row['id']; + $storedEncryptedSecret = $row['secret']; + $secret = $this->crypto->decrypt($storedEncryptedSecret); + $hashedSecret = bin2hex($this->crypto->calculateHMAC($secret)); + $qbUpdate->setParameter('updateSecret', $hashedSecret, IQueryBuilder::PARAM_STR); + $qbUpdate->setParameter('updateId', $id, IQueryBuilder::PARAM_INT); + $qbUpdate->executeStatement(); + } + $req->closeCursor(); + } +} diff --git a/apps/oauth2/lib/Settings/Admin.php b/apps/oauth2/lib/Settings/Admin.php index 5cba6805f7cf1..a44b0ff05b552 100644 --- a/apps/oauth2/lib/Settings/Admin.php +++ b/apps/oauth2/lib/Settings/Admin.php @@ -30,7 +30,6 @@ use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; use OCP\IURLGenerator; -use OCP\Security\ICrypto; use OCP\Settings\ISettings; use Psr\Log\LoggerInterface; @@ -40,7 +39,6 @@ public function __construct( private IInitialState $initialState, private ClientMapper $clientMapper, private IURLGenerator $urlGenerator, - private ICrypto $crypto, private LoggerInterface $logger, ) { } @@ -51,13 +49,12 @@ public function getForm(): TemplateResponse { foreach ($clients as $client) { try { - $secret = $this->crypto->decrypt($client->getSecret()); $result[] = [ 'id' => $client->getId(), 'name' => $client->getName(), 'redirectUri' => $client->getRedirectUri(), 'clientId' => $client->getClientIdentifier(), - 'clientSecret' => $secret, + 'clientSecret' => '', ]; } catch (\Exception $e) { $this->logger->error('[Settings] OAuth client secret decryption error', ['exception' => $e]); diff --git a/apps/oauth2/src/App.vue b/apps/oauth2/src/App.vue index 71cf22dbf89e2..411643168f462 100644 --- a/apps/oauth2/src/App.vue +++ b/apps/oauth2/src/App.vue @@ -50,6 +50,10 @@ @delete="deleteClient" /> + + {{ t('oauth2', 'Make sure you store the secret key, it cannot be recovered.') }} +

{{ t('oauth2', 'Add client') }}

@@ -83,6 +87,7 @@ import { generateUrl } from '@nextcloud/router' import { getCapabilities } from '@nextcloud/capabilities' import NcSettingsSection from '@nextcloud/vue/dist/Components/NcSettingsSection.js' import NcButton from '@nextcloud/vue/dist/Components/NcButton.js' +import NcNoteCard from '@nextcloud/vue/dist/Components/NcNoteCard.js' import { loadState } from '@nextcloud/initial-state' import NcTextField from '@nextcloud/vue/dist/Components/NcTextField.js' @@ -93,6 +98,7 @@ export default { NcSettingsSection, NcButton, NcTextField, + NcNoteCard, }, props: { clients: { @@ -109,6 +115,7 @@ export default { error: false, }, oauthDocLink: loadState('oauth2', 'oauth2-doc-link'), + showSecretWarning: false, } }, computed: { @@ -136,6 +143,7 @@ export default { ).then(response => { // eslint-disable-next-line vue/no-mutating-props this.clients.push(response.data) + this.showSecretWarning = true this.newClient.name = '' this.newClient.redirectUri = '' diff --git a/apps/oauth2/src/components/OAuthItem.vue b/apps/oauth2/src/components/OAuthItem.vue index 2a2eced77017f..4fc0d63b8e770 100644 --- a/apps/oauth2/src/components/OAuthItem.vue +++ b/apps/oauth2/src/components/OAuthItem.vue @@ -27,7 +27,8 @@
{{ renderedSecret }} -