From fbda74291a16001a94282c9d77e304eaba3aafbe Mon Sep 17 00:00:00 2001 From: "Bernd.Rederlechner@t-systems.com" Date: Wed, 23 Aug 2023 12:27:34 +0200 Subject: [PATCH 1/7] Add bearersecret field, with base64 encoding and encryption --- lib/Command/UpsertProvider.php | 17 +++++++++---- lib/Db/Provider.php | 3 +++ lib/Db/ProviderMapper.php | 6 ++++- .../Version00008Date20211114183344.php | 25 +++++++++++++++++++ 4 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 lib/Migration/Version00008Date20211114183344.php diff --git a/lib/Command/UpsertProvider.php b/lib/Command/UpsertProvider.php index 932574ad..45f201c1 100644 --- a/lib/Command/UpsertProvider.php +++ b/lib/Command/UpsertProvider.php @@ -73,8 +73,8 @@ protected function configure() { ->addOption('mapping-quota', null, InputOption::VALUE_OPTIONAL, 'Attribute mapping of the quota') ->addOption('mapping-uid', null, InputOption::VALUE_OPTIONAL, 'Attribute mapping of the user id') ->addOption('extra-claims', null, InputOption::VALUE_OPTIONAL, 'Extra claims to request when getting tokens') - - ->addOption( + ->addOption('bearersecret', 'bs', InputOption::VALUE_OPTIONAL, 'Telekom bearer token requires a different client secret for bearer tokens') + ->addOption( 'output', null, InputOption::VALUE_OPTIONAL, @@ -100,11 +100,18 @@ protected function execute(InputInterface $input, OutputInterface $output) { return $this->listProviders($input, $output); } + // bearersecret is usually base64 encoded, but SAM delivers it non-encoded + // by default; so always encode/decode for this field + $bearersecret = $input->getOption('bearersecret'); + if ($bearersecret !== null) { + $bearersecret = $this->crypto->encrypt(\Base64Url\Base64Url::encode($bearersecret)); + } + // check if any option for updating is provided $updateOptions = array_filter($input->getOptions(), static function ($value, $option) { return in_array($option, [ - 'identifier', 'clientid', 'clientsecret', 'discoveryuri', - 'scope', 'unique-uid', 'check-bearer', + 'identifier', 'clientid', 'clientsecret', 'discoveryuri', + 'scope', 'unique-uid', 'check-bearer', 'bearersecret', 'mapping-uid', 'mapping-display-name', 'mapping-email', 'mapping-quota', 'extra-claims' ]) && $value !== null; @@ -146,7 +153,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { $scope = $scope ?? 'openid email profile'; } try { - $provider = $this->providerMapper->createOrUpdateProvider($identifier, $clientid, $clientsecret, $discoveryuri, $scope); + $provider = $this->providerMapper->createOrUpdateProvider($identifier, $clientid, $clientsecret, $discoveryuri, $scope, $bearersecret); // invalidate JWKS cache (even if it was just created) $this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE, ''); $this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE_TIMESTAMP, ''); diff --git a/lib/Db/Provider.php b/lib/Db/Provider.php index 2636ac95..c80cbb7a 100644 --- a/lib/Db/Provider.php +++ b/lib/Db/Provider.php @@ -55,6 +55,9 @@ class Provider extends Entity implements \JsonSerializable { /** @var string */ protected $scope; + /** @var string */ + protected $bearerSecret; + /** * @return string */ diff --git a/lib/Db/ProviderMapper.php b/lib/Db/ProviderMapper.php index 9c841e4d..bf2d6f4c 100644 --- a/lib/Db/ProviderMapper.php +++ b/lib/Db/ProviderMapper.php @@ -98,7 +98,7 @@ public function getProviders() { */ public function createOrUpdateProvider(string $identifier, string $clientid = null, string $clientsecret = null, string $discoveryuri = null, - string $scope = 'openid email profile') { + string $scope = 'openid email profile', string $bearersecret = null) { try { $provider = $this->findProviderByIdentifier($identifier); } catch (DoesNotExistException $eNotExist) { @@ -115,6 +115,7 @@ public function createOrUpdateProvider(string $identifier, string $clientid = nu $provider->setClientSecret($clientsecret); $provider->setDiscoveryEndpoint($discoveryuri); $provider->setScope($scope); + $provider->setBearerSecret($bearersecret ?? ''); return $this->insert($provider); } else { if ($clientid !== null) { @@ -126,6 +127,9 @@ public function createOrUpdateProvider(string $identifier, string $clientid = nu if ($discoveryuri !== null) { $provider->setDiscoveryEndpoint($discoveryuri); } + if ($bearerSecret !== null) { + $provider->setBearerSecret($bearersecret); + } $provider->setScope($scope); return $this->update($provider); } diff --git a/lib/Migration/Version00008Date20211114183344.php b/lib/Migration/Version00008Date20211114183344.php new file mode 100644 index 00000000..1c9cf6ea --- /dev/null +++ b/lib/Migration/Version00008Date20211114183344.php @@ -0,0 +1,25 @@ +getTable('user_oidc_providers'); + $table->addColumn('bearer_secret', 'string', [ + 'notnull' => true, + 'length' => 64, + ]); + + return $schema; + } +} From a0d3320e96ab9a6273821001f2d8588493f8d49e Mon Sep 17 00:00:00 2001 From: "Bernd.Rederlechner@t-systems.com" Date: Wed, 23 Aug 2023 12:54:10 +0200 Subject: [PATCH 2/7] Code cleanup --- lib/Command/UpsertProvider.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Command/UpsertProvider.php b/lib/Command/UpsertProvider.php index 45f201c1..ca27b7a2 100644 --- a/lib/Command/UpsertProvider.php +++ b/lib/Command/UpsertProvider.php @@ -74,7 +74,7 @@ protected function configure() { ->addOption('mapping-uid', null, InputOption::VALUE_OPTIONAL, 'Attribute mapping of the user id') ->addOption('extra-claims', null, InputOption::VALUE_OPTIONAL, 'Extra claims to request when getting tokens') ->addOption('bearersecret', 'bs', InputOption::VALUE_OPTIONAL, 'Telekom bearer token requires a different client secret for bearer tokens') - ->addOption( + ->addOption( 'output', null, InputOption::VALUE_OPTIONAL, @@ -101,7 +101,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { } // bearersecret is usually base64 encoded, but SAM delivers it non-encoded - // by default; so always encode/decode for this field + // by default; so always encode/decode for this field $bearersecret = $input->getOption('bearersecret'); if ($bearersecret !== null) { $bearersecret = $this->crypto->encrypt(\Base64Url\Base64Url::encode($bearersecret)); @@ -110,7 +110,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { // check if any option for updating is provided $updateOptions = array_filter($input->getOptions(), static function ($value, $option) { return in_array($option, [ - 'identifier', 'clientid', 'clientsecret', 'discoveryuri', + 'identifier', 'clientid', 'clientsecret', 'discoveryuri', 'scope', 'unique-uid', 'check-bearer', 'bearersecret', 'mapping-uid', 'mapping-display-name', 'mapping-email', 'mapping-quota', 'extra-claims' From d5d345cc0dbb91cc45098468d86441d8008be82a Mon Sep 17 00:00:00 2001 From: "Bernd.Rederlechner@t-systems.com" Date: Fri, 1 Sep 2023 08:57:24 +0000 Subject: [PATCH 3/7] Add settings UI for bearersecret --- lib/Controller/SettingsController.php | 9 +- src/components/SettingsForm.vue | 9 + .../unit/MagentaCloud/BearerSettingsTest.php | 420 ++++++++++++++++++ 3 files changed, 436 insertions(+), 2 deletions(-) create mode 100644 tests/unit/MagentaCloud/BearerSettingsTest.php diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index 4fb4d601..00bbab77 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -64,7 +64,7 @@ public function __construct( } public function createProvider(string $identifier, string $clientId, string $clientSecret, string $discoveryEndpoint, - array $settings = [], string $scope = 'openid email profile'): JSONResponse { + string $bearerSecret, array $settings = [], string $scope = 'openid email profile'): JSONResponse { if ($this->providerService->getProviderByIdentifier($identifier) !== null) { return new JSONResponse(['message' => 'Provider with the given identifier already exists'], Http::STATUS_CONFLICT); } @@ -76,6 +76,8 @@ public function createProvider(string $identifier, string $clientId, string $cli $provider->setClientSecret($encryptedClientSecret); $provider->setDiscoveryEndpoint($discoveryEndpoint); $provider->setScope($scope); + $encryptedBearerSecret = $this->crypto->encrypt(\Base64Url\Base64Url::encode($bearerSecret)); + $provider->setBearerSecret($encryptedBearerSecret); $provider = $this->providerMapper->insert($provider); $providerSettings = $this->providerService->setSettings($provider->getId(), $settings); @@ -84,7 +86,7 @@ public function createProvider(string $identifier, string $clientId, string $cli } public function updateProvider(int $providerId, string $identifier, string $clientId, string $discoveryEndpoint, string $clientSecret = null, - array $settings = [], string $scope = 'openid email profile'): JSONResponse { + string $bearerSecret = null, array $settings = [], string $scope = 'openid email profile'): JSONResponse { $provider = $this->providerMapper->getProvider($providerId); if ($this->providerService->getProviderByIdentifier($identifier) === null) { @@ -97,6 +99,9 @@ public function updateProvider(int $providerId, string $identifier, string $clie $encryptedClientSecret = $this->crypto->encrypt($clientSecret); $provider->setClientSecret($encryptedClientSecret); } + if ($bearerSecret) { + $provider->setBearerSecret(\Base64Url\Base64Url::encode($bearerSecret)); + } $provider->setDiscoveryEndpoint($discoveryEndpoint); $provider->setScope($scope); $provider = $this->providerMapper->update($provider); diff --git a/src/components/SettingsForm.vue b/src/components/SettingsForm.vue index 7d128be0..3550c2ab 100644 --- a/src/components/SettingsForm.vue +++ b/src/components/SettingsForm.vue @@ -46,6 +46,15 @@ :required="!update" autocomplete="off">

+

+ + +

{{ t('user_oidc', 'Warning, if the protocol of the URLs in the discovery content is HTTP, the ID token will be delivered through an insecure connection.') }} diff --git a/tests/unit/MagentaCloud/BearerSettingsTest.php b/tests/unit/MagentaCloud/BearerSettingsTest.php new file mode 100644 index 00000000..8ac57376 --- /dev/null +++ b/tests/unit/MagentaCloud/BearerSettingsTest.php @@ -0,0 +1,420 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +declare(strict_types=1); + +use OCP\IRequest; +use OCP\IConfig; + +use OCA\UserOIDC\AppInfo\Application; + +use OCA\UserOIDC\Service\ProviderService; +use OCA\UserOIDC\Db\Provider; +use OCA\UserOIDC\Db\ProviderMapper; + +use OCP\Security\ICrypto; + +use OCA\UserOIDC\Command\UpsertProvider; +use Symfony\Component\Console\Tester\CommandTester; + + +use PHPUnit\Framework\TestCase; + +class BearerSettingsTest extends TestCase { + /** + * @var ProviderService + */ + private $provider; + + /** + * @var IConfig; + */ + private $config; + + public function setUp(): void { + parent::setUp(); + + $app = new \OCP\AppFramework\App(Application::APP_ID); + $this->requestMock = $this->createMock(IRequest::class); + + $this->config = $this->createMock(IConfig::class); + $this->providerMapper = $this->createMock(ProviderMapper::class); + $providers = [ + new \OCA\UserOIDC\Db\Provider(), + ]; + $providers[0]->setId(1); + $providers[0]->setIdentifier('Fraesbook'); + + $this->providerMapper->expects(self::any()) + ->method('getProviders') + ->willReturn($providers); + + $this->providerService = $this->getMockBuilder(ProviderService::class) + ->setConstructorArgs([ $this->config, $this->providerMapper]) + ->onlyMethods(['getProviderByIdentifier']) + ->getMock(); + $this->crypto = $app->getContainer()->get(ICrypto::class); + } + + protected function mockCreateUpdate( + string $providername, + string|null $clientid, + string|null $clientsecret, + string|null $discovery, + string $scope, + string|null $bearersecret, + array $options, + int $id = 2 + ) { + $provider = $this->getMockBuilder(Provider::class) + ->addMethods(['getIdentifier', 'getId']) + ->getMock(); + $provider->expects($this->any()) + ->method('getIdentifier') + ->willReturn($providername); + $provider->expects($this->any()) + ->method('getId') + ->willReturn($id); + + $this->providerMapper->expects($this->once()) + ->method('createOrUpdateProvider') + ->with( + $this->equalTo($providername), + $this->equalTo($clientid), + $this->anything(), + $this->equalTo($discovery), + $this->equalTo($scope), + $this->anything() + ) + ->willReturnCallback(function ($id, $clientid, $secret, $discovery, $scope, $bsecret) use ($clientsecret, $bearersecret, $provider) { + if ($secret !== null) { + $this->assertEquals($clientsecret, $this->crypto->decrypt($secret)); + } else { + $this->assertNull($secret); + } + if ($bsecret !== null) { + $this->assertEquals($bearersecret, \Base64Url\Base64Url::decode($this->crypto->decrypt($bsecret))); + } else { + $this->assertNull($bsecret); + } + return $provider; + }); + + + $this->config->expects($this->any()) + ->method('setAppValue') + ->with($this->equalTo(Application::APP_ID), $this->anything(), $this->anything()) + ->willReturnCallback(function ($appid, $key, $value) use ($options) { + if (array_key_exists($key, $options)) { + $this->assertEquals($options[$key], $value); + } + return ''; + }); + } + + + public function testCommandAddProvider() { + $this->providerService->expects($this->once()) + ->method('getProviderByIdentifier') + ->with($this->equalTo('Telekom')) + ->willReturn(null); + + $this->mockCreateUpdate('Telekom', + '10TVL0SAM30000004901NEXTMAGENTACLOUDTEST', + 'clientsecret***', + 'https://accounts.login00.idm.ver.sul.t-online.de/.well-known/openid-configuration', + 'openid email profile', + 'bearersecret***', + [ + 'provider-2-' . ProviderService::SETTING_UNIQUE_UID => '0', + 'provider-2-' . ProviderService::SETTING_MAPPING_DISPLAYNAME => 'urn:telekom.com:displayname', + 'provider-2-' . ProviderService::SETTING_MAPPING_EMAIL => 'urn:telekom.com:mainEmail', + 'provider-2-' . ProviderService::SETTING_MAPPING_QUOTA => 'quota', + 'provider-2-' . ProviderService::SETTING_MAPPING_UID => 'sub' + ]); + + $command = new UpsertProvider($this->providerService, $this->providerMapper, $this->crypto); + $commandTester = new CommandTester($command); + + $commandTester->execute(array( + 'identifier' => 'Telekom', + '--clientid' => '10TVL0SAM30000004901NEXTMAGENTACLOUDTEST', + '--clientsecret' => 'clientsecret***', + '--bearersecret' => 'bearersecret***', + '--discoveryuri' => 'https://accounts.login00.idm.ver.sul.t-online.de/.well-known/openid-configuration', + '--scope' => 'openid email profile', + '--unique-uid' => '0', + '--mapping-display-name' => 'urn:telekom.com:displayname', + '--mapping-email' => 'urn:telekom.com:mainEmail', + '--mapping-quota' => 'quota', + '--mapping-uid' => 'sub', + )); + + + //$output = $commandTester->getOutput(); + //$this->assertContains('done', $output); + } + + protected function mockProvider(string $providername, + string $clientid, + string $clientsecret, + string $discovery, + string $scope, + string $bearersecret, + int $id = 2) : Provider { + $provider = $this->getMockBuilder(Provider::class) + ->addMethods(['getIdentifier', 'getClientId', 'getClientSecret', 'getBearerSecret', 'getDiscoveryEndpoint']) + ->setMethods(['getScope', 'getId']) + ->getMock(); + $provider->expects($this->any()) + ->method('getIdentifier') + ->willReturn($providername); + $provider->expects($this->any()) + ->method('getId') + ->willReturn(2); + $provider->expects($this->any()) + ->method('getClientId') + ->willReturn($clientid); + $provider->expects($this->any()) + ->method('getClientSecret') + ->willReturn($clientsecret); + $provider->expects($this->any()) + ->method('getBearerSecret') + ->willReturn(\Base64Url\Base64Url::encode($bearersecret)); + $provider->expects($this->any()) + ->method('getDiscoveryEndpoint') + ->willReturn($discovery); + $provider->expects($this->any()) + ->method('getScope') + ->willReturn($scope); + + return $provider; + } + + public function testCommandUpdateFull() { + $provider = $this->getMockBuilder(Provider::class) + ->addMethods(['getIdentifier', 'getClientId', 'getClientSecret', 'getBearerSecret', 'getDiscoveryEndpoint']) + ->setMethods(['getScope']) + ->getMock(); + $provider->expects($this->any()) + ->method('getIdentifier') + ->willReturn('Telekom'); + $provider->expects($this->never())->method('getClientId'); + $provider->expects($this->never())->method('getClientSecret'); + $provider->expects($this->never())->method('getBearerSecret'); + $provider->expects($this->never())->method('getDiscoveryEndpoint'); + $provider->expects($this->never())->method('getScope'); + + $this->providerService->expects($this->once()) + ->method('getProviderByIdentifier') + ->with($this->equalTo('Telekom')) + ->willReturn(null); + $this->mockCreateUpdate('Telekom', + '10TVL0SAM30000004902NEXTMAGENTACLOUDTEST', + 'client*secret***', + 'https://accounts.login00.idm.ver.sul.t-online.de/.well-unknown/openid-configuration', + 'openid profile', + 'bearer*secret***', + [ + 'provider-2-' . ProviderService::SETTING_UNIQUE_UID => '1', + 'provider-2-' . ProviderService::SETTING_MAPPING_DISPLAYNAME => 'urn:telekom.com:displaykrame', + 'provider-2-' . ProviderService::SETTING_MAPPING_EMAIL => 'urn:telekom.com:mainDemail', + 'provider-2-' . ProviderService::SETTING_MAPPING_QUOTA => 'quotas', + 'provider-2-' . ProviderService::SETTING_MAPPING_UID => 'flop' + ]); + + $command = new UpsertProvider($this->providerService, $this->providerMapper, $this->crypto); + $commandTester = new CommandTester($command); + $commandTester->execute(array( + 'identifier' => 'Telekom', + '--clientid' => '10TVL0SAM30000004902NEXTMAGENTACLOUDTEST', + '--clientsecret' => 'client*secret***', + '--bearersecret' => 'bearer*secret***', + '--discoveryuri' => 'https://accounts.login00.idm.ver.sul.t-online.de/.well-unknown/openid-configuration', + '--scope' => 'openid profile', + '--mapping-display-name' => 'urn:telekom.com:displaykrame', + '--mapping-email' => 'urn:telekom.com:mainDemail', + '--mapping-quota' => 'quotas', + '--mapping-uid' => 'flop', + '--unique-uid' => '1' + )); + } + + public function testCommandUpdateSingleClientId() { + $provider = $this->mockProvider('Telekom', '10TVL0SAM30000004901NEXTMAGENTACLOUDTEST', 'clientsecret***', + 'https://accounts.login00.idm.ver.sul.t-online.de/.well-known/openid-configuration', + 'openid email profile', 'bearersecret***'); + $this->providerService->expects($this->once()) + ->method('getProviderByIdentifier') + ->with($this->equalTo('Telekom')) + ->willReturn($provider); + $this->mockCreateUpdate( + 'Telekom', + '10TVL0SAM30000004903NEXTMAGENTACLOUDTEST', + null, + null, + 'openid email profile', + null, + []); + + $command = new UpsertProvider($this->providerService, $this->providerMapper, $this->crypto); + $commandTester = new CommandTester($command); + + $commandTester->execute(array( + 'identifier' => 'Telekom', + '--clientid' => '10TVL0SAM30000004903NEXTMAGENTACLOUDTEST', + )); + } + + + public function testCommandUpdateSingleClientSecret() { + $provider = $this->mockProvider('Telekom', '10TVL0SAM30000004901NEXTMAGENTACLOUDTEST', 'clientsecret***', + 'https://accounts.login00.idm.ver.sul.t-online.de/.well-known/openid-configuration', + 'openid email profile', 'bearersecret***'); + $this->providerService->expects($this->once()) + ->method('getProviderByIdentifier') + ->with($this->equalTo('Telekom')) + ->willReturn($provider); + $this->mockCreateUpdate( + 'Telekom', + null, + '***clientsecret***', + null, + 'openid email profile', + null, + []); + + $command = new UpsertProvider($this->providerService, $this->providerMapper, $this->crypto); + $commandTester = new CommandTester($command); + + $commandTester->execute(array( + 'identifier' => 'Telekom', + '--clientsecret' => '***clientsecret***', + )); + } + + public function testCommandUpdateSingleBearerSecret() { + $provider = $this->mockProvider('Telekom', '10TVL0SAM30000004901NEXTMAGENTACLOUDTEST', 'clientsecret***', + 'https://accounts.login00.idm.ver.sul.t-online.de/.well-known/openid-configuration', + 'openid email profile', 'bearersecret***'); + $this->providerService->expects($this->once()) + ->method('getProviderByIdentifier') + ->with($this->equalTo('Telekom')) + ->willReturn($provider); + $this->mockCreateUpdate( + 'Telekom', + null, + null, + null, + 'openid email profile', + '***bearersecret***', + []); + + + $command = new UpsertProvider($this->providerService, $this->providerMapper, $this->crypto); + $commandTester = new CommandTester($command); + + $commandTester->execute(array( + 'identifier' => 'Telekom', + '--bearersecret' => '***bearersecret***', + )); + } + + public function testCommandUpdateSingleDiscoveryEndpoint() { + $provider = $this->mockProvider('Telekom', '10TVL0SAM30000004901NEXTMAGENTACLOUDTEST', 'clientsecret***', + 'https://accounts.login00.idm.ver.sul.t-online.de/.well-known/openid-configuration', + 'openid email profile', 'bearersecret***'); + $this->providerService->expects($this->once()) + ->method('getProviderByIdentifier') + ->with($this->equalTo('Telekom')) + ->willReturn($provider); + $this->mockCreateUpdate( + 'Telekom', + null, + null, + 'https://accounts.login00.idm.ver.sul.t-online.de/.well-unknown/openid-configuration', + 'openid email profile', + null, []); + + $command = new UpsertProvider($this->providerService, $this->providerMapper, $this->crypto); + $commandTester = new CommandTester($command); + + $commandTester->execute(array( + 'identifier' => 'Telekom', + '--discoveryuri' => 'https://accounts.login00.idm.ver.sul.t-online.de/.well-unknown/openid-configuration', + )); + } + + public function testCommandUpdateSingleScope() { + $provider = $this->mockProvider('Telekom', '10TVL0SAM30000004901NEXTMAGENTACLOUDTEST', 'clientsecret***', + 'https://accounts.login00.idm.ver.sul.t-online.de/.well-known/openid-configuration', + 'openid email profile', 'bearersecret***'); + $this->providerService->expects($this->once()) + ->method('getProviderByIdentifier') + ->with($this->equalTo('Telekom')) + ->willReturn($provider); + $this->mockCreateUpdate( + 'Telekom', + null, + null, + null, + 'openid profile', + '***bearersecret***', + []); + + + $command = new UpsertProvider($this->providerService, $this->providerMapper, $this->crypto); + $commandTester = new CommandTester($command); + + $commandTester->execute(array( + 'identifier' => 'Telekom', + '--scope' => 'openid profile', + )); + } + + public function testCommandUpdateSingleUniqueUid() { + $provider = $this->mockProvider('Telekom', '10TVL0SAM30000004901NEXTMAGENTACLOUDTEST', 'clientsecret***', + 'https://accounts.login00.idm.ver.sul.t-online.de/.well-known/openid-configuration', + 'openid email profile', 'bearersecret***'); + $this->providerService->expects($this->once()) + ->method('getProviderByIdentifier') + ->with($this->equalTo('Telekom')) + ->willReturn($provider); + $this->mockCreateUpdate( + 'Telekom', + null, + null, + null, + 'openid email profile', + null, + ['provider-2-' . ProviderService::SETTING_UNIQUE_UID => '1']); + + $command = new UpsertProvider($this->providerService, $this->providerMapper, $this->crypto); + $commandTester = new CommandTester($command); + + $commandTester->execute(array( + 'identifier' => 'Telekom', + '--unique-uid' => '1', + )); + } +} From 1fba489f8463d807baed6f46e3a6530433775e8f Mon Sep 17 00:00:00 2001 From: "Bernd.Rederlechner@t-systems.com" Date: Fri, 1 Sep 2023 08:57:57 +0000 Subject: [PATCH 4/7] Clean up ORM entity and mapper --- lib/Db/Provider.php | 3 +++ lib/Db/ProviderMapper.php | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/Db/Provider.php b/lib/Db/Provider.php index c80cbb7a..3943e4e6 100644 --- a/lib/Db/Provider.php +++ b/lib/Db/Provider.php @@ -34,8 +34,11 @@ * @method void setClientId(string $clientId) * @method string getClientSecret() * @method void setClientSecret(string $clientSecret) + * @method string getBearerSecret() + * @method void setBearerSecret(string $bearerSecret) * @method string getDiscoveryEndpoint() * @method void setDiscoveryEndpoint(string $discoveryEndpoint) + * @method string getScope() * @method void setScope(string $scope) */ class Provider extends Entity implements \JsonSerializable { diff --git a/lib/Db/ProviderMapper.php b/lib/Db/ProviderMapper.php index bf2d6f4c..7a0bbed8 100644 --- a/lib/Db/ProviderMapper.php +++ b/lib/Db/ProviderMapper.php @@ -93,6 +93,7 @@ public function getProviders() { * @param string|null $clientsecret * @param string|null $discoveryuri * @param string scope + * @param string|null $bearersecret * @throws \OCP\AppFramework\Db\DoesNotExistException * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException */ @@ -127,7 +128,7 @@ public function createOrUpdateProvider(string $identifier, string $clientid = nu if ($discoveryuri !== null) { $provider->setDiscoveryEndpoint($discoveryuri); } - if ($bearerSecret !== null) { + if ($bearersecret !== null) { $provider->setBearerSecret($bearersecret); } $provider->setScope($scope); From 50e702675de653304166062f6aa9a0eb1aae7237 Mon Sep 17 00:00:00 2001 From: "Bernd.Rederlechner@t-systems.com" Date: Thu, 7 Sep 2023 13:23:04 +0200 Subject: [PATCH 5/7] Enlarge bearersecret field for encryption --- .../Version010304Date20230902125945.php | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 lib/Migration/Version010304Date20230902125945.php diff --git a/lib/Migration/Version010304Date20230902125945.php b/lib/Migration/Version010304Date20230902125945.php new file mode 100644 index 00000000..8c9a2d55 --- /dev/null +++ b/lib/Migration/Version010304Date20230902125945.php @@ -0,0 +1,98 @@ + + * + * @author B. Rederlechner + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\UserOIDC\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; +use OCP\Security\ICrypto; + +class Version010304Date20230902125945 extends SimpleMigrationStep { + + /** + * @var IDBConnection + */ + private $connection; + /** + * @var ICrypto + */ + private $crypto; + + public function __construct( + IDBConnection $connection, + ICrypto $crypto + ) { + $this->connection = $connection; + $this->crypto = $crypto; + } + + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + foreach (['user_oidc_providers', 'user_oidc_id4me'] as $tableName) { + if ($schema->hasTable($tableName)) { + $table = $schema->getTable($tableName); + if ($table->hasColumn('bearer_secret')) { + $column = $table->getColumn('bearer_secret'); + $column->setLength(512); + return $schema; + } + } + } + + return null; + } + + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { + // update secrets in user_oidc_providers and user_oidc_id4me + foreach (['user_oidc_providers', 'user_oidc_id4me'] as $tableName) { + $qbUpdate = $this->connection->getQueryBuilder(); + $qbUpdate->update($tableName) + ->set('bearer_secret', $qbUpdate->createParameter('updateSecret')) + ->where( + $qbUpdate->expr()->eq('id', $qbUpdate->createParameter('updateId')) + ); + + $qbSelect = $this->connection->getQueryBuilder(); + $qbSelect->select('id', 'bearer_secret') + ->from($tableName); + $req = $qbSelect->executeQuery(); + while ($row = $req->fetch()) { + $id = $row['id']; + $secret = $row['bearer_secret']; + $encryptedSecret = $this->crypto->encrypt($secret); + $qbUpdate->setParameter('updateSecret', $encryptedSecret, IQueryBuilder::PARAM_STR); + $qbUpdate->setParameter('updateId', $id, IQueryBuilder::PARAM_INT); + $qbUpdate->executeStatement(); + } + $req->closeCursor(); + } + } +} From 9d06a7a8c1d1a9f39eb24bd59167f06701f80b66 Mon Sep 17 00:00:00 2001 From: "Bernd.Rederlechner@t-systems.com" Date: Thu, 19 Oct 2023 13:31:07 +0200 Subject: [PATCH 6/7] Remove unaffected id4me table from migration --- .../Version010304Date20230902125945.php | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/lib/Migration/Version010304Date20230902125945.php b/lib/Migration/Version010304Date20230902125945.php index 8c9a2d55..9d817df8 100644 --- a/lib/Migration/Version010304Date20230902125945.php +++ b/lib/Migration/Version010304Date20230902125945.php @@ -55,15 +55,14 @@ public function __construct( public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) { /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); + $tableName = 'user_oidc_providers'; - foreach (['user_oidc_providers', 'user_oidc_id4me'] as $tableName) { - if ($schema->hasTable($tableName)) { - $table = $schema->getTable($tableName); - if ($table->hasColumn('bearer_secret')) { - $column = $table->getColumn('bearer_secret'); - $column->setLength(512); - return $schema; - } + if ($schema->hasTable($tableName)) { + $table = $schema->getTable($tableName); + if ($table->hasColumn('bearer_secret')) { + $column = $table->getColumn('bearer_secret'); + $column->setLength(512); + return $schema; } } @@ -71,28 +70,28 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt } public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { + $tableName = 'user_oidc_providers'; + // update secrets in user_oidc_providers and user_oidc_id4me - foreach (['user_oidc_providers', 'user_oidc_id4me'] as $tableName) { - $qbUpdate = $this->connection->getQueryBuilder(); - $qbUpdate->update($tableName) + $qbUpdate = $this->connection->getQueryBuilder(); + $qbUpdate->update($tableName) ->set('bearer_secret', $qbUpdate->createParameter('updateSecret')) ->where( $qbUpdate->expr()->eq('id', $qbUpdate->createParameter('updateId')) ); - $qbSelect = $this->connection->getQueryBuilder(); - $qbSelect->select('id', 'bearer_secret') + $qbSelect = $this->connection->getQueryBuilder(); + $qbSelect->select('id', 'bearer_secret') ->from($tableName); - $req = $qbSelect->executeQuery(); - while ($row = $req->fetch()) { - $id = $row['id']; - $secret = $row['bearer_secret']; - $encryptedSecret = $this->crypto->encrypt($secret); - $qbUpdate->setParameter('updateSecret', $encryptedSecret, IQueryBuilder::PARAM_STR); - $qbUpdate->setParameter('updateId', $id, IQueryBuilder::PARAM_INT); - $qbUpdate->executeStatement(); - } - $req->closeCursor(); + $req = $qbSelect->executeQuery(); + while ($row = $req->fetch()) { + $id = $row['id']; + $secret = $row['bearer_secret']; + $encryptedSecret = $this->crypto->encrypt($secret); + $qbUpdate->setParameter('updateSecret', $encryptedSecret, IQueryBuilder::PARAM_STR); + $qbUpdate->setParameter('updateId', $id, IQueryBuilder::PARAM_INT); + $qbUpdate->executeStatement(); } + $req->closeCursor(); } } From e71c92d96a002cfa29c985ca4000f6953c6a301c Mon Sep 17 00:00:00 2001 From: "Bernd.Rederlechner@t-systems.com" Date: Thu, 19 Oct 2023 13:38:10 +0200 Subject: [PATCH 7/7] Clean code --- src/components/SettingsForm.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/SettingsForm.vue b/src/components/SettingsForm.vue index 3550c2ab..1a60095c 100644 --- a/src/components/SettingsForm.vue +++ b/src/components/SettingsForm.vue @@ -46,7 +46,7 @@ :required="!update" autocomplete="off">

-

+