diff --git a/appinfo/app.php b/appinfo/app.php index 5b3ec6e..d1d3732 100644 --- a/appinfo/app.php +++ b/appinfo/app.php @@ -38,12 +38,8 @@ try { \OC::$server->getLogger()->logException($e); return; } -$samlSettings = new \OCA\User_SAML\SAMLSettings( - $urlGenerator, - $config, - $request, - $session -); + +$samlSettings = \OC::$server->query(\OCA\User_SAML\SAMLSettings::class); $userData = new \OCA\User_SAML\UserData( new \OCA\User_SAML\UserResolver(\OC::$server->getUserManager()), diff --git a/appinfo/info.xml b/appinfo/info.xml index 2e3e91e..87211ae 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -16,7 +16,7 @@ The following providers are supported and tested at the moment: * Any other provider that authenticates using the environment variable While theoretically any other authentication provider implementing either one of those standards is compatible, we like to note that they are not part of any internal test matrix.]]> - 4.3.0 + 5.0.0 agpl Lukas Reschke User_SAML diff --git a/appinfo/routes.php b/appinfo/routes.php index 63e5ca9..3cb584c 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -69,14 +69,32 @@ return [ 'url' => '/saml/selectUserBackEnd', 'verb' => 'GET', ], + [ + 'name' => 'Settings#getSamlProviderIds', + 'url' => '/settings/providers', + 'verb' => 'GET', + ], [ 'name' => 'Settings#getSamlProviderSettings', 'url' => '/settings/providerSettings/{providerId}', 'verb' => 'GET', 'defaults' => [ - 'providerId' => '1' + 'providerId' => 1 ] ], + [ + 'name' => 'Settings#setProviderSetting', + 'url' => '/settings/providerSettings/{providerId}', + 'verb' => 'POST', + 'defaults' => [ + 'providerId' => 1 + ] + ], + [ + 'name' => 'Settings#newSamlProviderSettingsId', + 'url' => '/settings/providerSettings', + 'verb' => 'PUT', + ], [ 'name' => 'Settings#deleteSamlProviderSettings', 'url' => '/settings/providerSettings/{providerId}', diff --git a/js/admin.js b/js/admin.js index 71301fc..2732e54 100644 --- a/js/admin.js +++ b/js/admin.js @@ -8,20 +8,30 @@ currentConfig: '1', providerIds: '1', - _getAppConfig: function (key) { - return $.ajax({ - type: 'GET', - url: OC.linkToOCS('apps/provisioning_api/api/v1', 2) + 'config/apps' + '/user_saml/' + key + '?format=json' - }); - }, init: function(callback) { - this._getAppConfig('providerIds').done(function (data){ - if (data.ocs.data.data !== '') { - OCA.User_SAML.Admin.providerIds = data.ocs.data.data; - OCA.User_SAML.Admin.currentConfig = OCA.User_SAML.Admin.providerIds.split(',').sort()[0]; + var xhr = new XMLHttpRequest(); + xhr.open('GET', OC.generateUrl('/apps/user_saml/settings/providers')); + xhr.setRequestHeader('Content-Type', 'application/json'); + xhr.setRequestHeader('requesttoken', OC.requestToken); + + xhr.onload = function () { + var response = JSON.parse(xhr.response); + if (xhr.status >= 200 && xhr.status < 300) { + if (response.providerIds !== "") { + OCA.User_SAML.Admin.providerIds += ',' + response.providerIds; + OCA.User_SAML.Admin.currentConfig = OCA.User_SAML.Admin.providerIds.split(',').sort()[0]; + } + callback(); + } else { + console.error("Could not fetch new provider ID"); } - callback(); - }); + }; + xhr.onerror = function () { + console.error("Could not fetch new provider ID"); + } + + + xhr.send(); }, chooseEnv: function() { if (OC.PasswordConfirmation.requiresPasswordConfirmation()) { @@ -60,23 +70,49 @@ /** * Add a new provider - * @returns {number} id of the provider */ - addProvider: function(callback) { - var providerIds = OCA.User_SAML.Admin.providerIds.split(','); - var nextId = 1; - if (providerIds.indexOf('1') >= 0) { - nextId = 2; - while ($.inArray('' + nextId, providerIds) >= 0) { - nextId++; + addProvider: function (callback) { + var xhr = new XMLHttpRequest(); + xhr.open('PUT', OC.generateUrl('/apps/user_saml/settings/providerSettings')); + xhr.setRequestHeader('Content-Type', 'application/json') + xhr.setRequestHeader('requesttoken', OC.requestToken) + + xhr.onload = function () { + var response = JSON.parse(xhr.response) + if (xhr.status >= 200 && xhr.status < 300) { + OCA.User_SAML.Admin.providerIds += ',' + response.id; + callback(response.id) + } else { + console.error("Could not fetch new provider ID") } - } - OCP.AppConfig.setValue('user_saml', 'providerIds', OCA.User_SAML.Admin.providerIds + ',' + nextId, { - success: function () { - OCA.User_SAML.Admin.providerIds += ',' + nextId; - callback(nextId) + }; + xhr.onerror = function () { + console.error("Could not fetch new provider ID"); + }; + + xhr.send(); + }, + + updateProvider: function (configKey, configValue, successCb, errorCb) { + var xhr = new XMLHttpRequest(); + xhr.open('POST', OC.generateUrl('/apps/user_saml/settings/providerSettings/' + this.currentConfig)); + xhr.setRequestHeader('Content-Type', 'application/json'); + xhr.setRequestHeader('requesttoken', OC.requestToken); + + xhr.onload = function () { + if (xhr.status >= 200 && xhr.status < 300) { + successCb(); + } else { + console.error("Could not update config"); + errorCb(); } - }); + }; + xhr.onerror = function () { + console.error("Could not update config"); + errorCb(); + }; + + xhr.send(JSON.stringify({configKey: configKey, configValue: configValue.trim()})); }, removeProvider: function(callback) { @@ -86,17 +122,8 @@ if (index > -1) { providerIds.splice(index, 1); } - var config = this.currentConfig; $.ajax({ url: OC.generateUrl('/apps/user_saml/settings/providerSettings/' + this.currentConfig), type: 'DELETE'}) - .done(function(data) { - OCP.AppConfig.setValue('user_saml', 'providerIds', providerIds.join(','), { - success: function () { - OCA.User_SAML.Admin.providerIds = providerIds.join(','); - callback(config); - } - }); - }); - + .done(callback(this.currentConfig)); } }, @@ -105,14 +132,22 @@ OC.PasswordConfirmation.requirePasswordConfirmation(_.bind(this.setSamlConfigValue, this, category, setting, value)); return; } - // store global config flags without idp prefix - var configIdentifier = this.getConfigIdentifier(); - if (global === true) { - configIdentifier = ''; - } OC.msg.startSaving('#user-saml-save-indicator'); - OCP.AppConfig.setValue('user_saml', configIdentifier + category + '-' + setting, value.trim()); - OC.msg.finishedSaving('#user-saml-save-indicator', {status: 'success', data: {message: t('user_saml', 'Saved')}}); + + var callbacks = { + success: function () { + OC.msg.finishedSaving('#user-saml-save-indicator', {status: 'success', data: {message: t('user_saml', 'Saved')}}); + }, + error: function() { + OC.msg.finishedSaving('#user-saml-save-indicator', {status: 'error', data: {message: t('user_saml', 'Could not save')}}); + } + }; + + if (global) { + OCP.AppConfig.setValue('user_saml', category + '-' + setting, value, callbacks); + return; + } + this.updateProvider(category + '-' + setting, value, callbacks.success, callbacks.error); } } })(OCA); @@ -200,7 +235,7 @@ $(function() { }); }); $('input:checkbox[value="1"]').attr('checked', true); - $('input:checkbox[value="0"]').attr('checked', false); + $('input:checkbox[value="0"]').removeAttr('checked', false); var xmlDownloadButton = $('#get-metadata'); var url = xmlDownloadButton.data('base') + '?idp=' + providerId; xmlDownloadButton.attr('href', url); diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 975a095..7ae16d2 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -23,6 +23,7 @@ namespace OCA\User_SAML\AppInfo; use OCA\User_SAML\DavPlugin; use OCA\User_SAML\Middleware\OnlyLoggedInMiddleware; +use OCA\User_SAML\SAMLSettings; use OCP\AppFramework\App; use OCP\AppFramework\IAppContainer; use OCP\SabrePluginEvent; @@ -48,7 +49,8 @@ class Application extends App { return new DavPlugin( $server->getSession(), $server->getConfig(), - $_SERVER + $_SERVER, + $server->get(SAMLSettings::class) ); }); diff --git a/lib/Command/GetMetadata.php b/lib/Command/GetMetadata.php index f3a85e6..a137f3b 100644 --- a/lib/Command/GetMetadata.php +++ b/lib/Command/GetMetadata.php @@ -26,6 +26,7 @@ use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use OCA\User_SAML\SAMLSettings; +use OneLogin\Saml2\Error; use OneLogin\Saml2\Settings; class GetMetadata extends Command { @@ -67,7 +68,7 @@ EOT * @return void */ protected function execute(InputInterface $input, OutputInterface $output) { - $idp = $input->getArgument('idp'); + $idp = (int)$input->getArgument('idp'); $settings = new Settings($this->SAMLSettings->getOneLoginSettingsArray($idp)); $metadata = $settings->getSPMetadata(); $errors = $settings->validateMetadata($metadata); diff --git a/lib/Controller/SAMLController.php b/lib/Controller/SAMLController.php index d3f8573..ab3609c 100644 --- a/lib/Controller/SAMLController.php +++ b/lib/Controller/SAMLController.php @@ -51,7 +51,7 @@ class SAMLController extends Controller { /** @var IUserSession */ private $userSession; /** @var SAMLSettings */ - private $SAMLSettings; + private $samlSettings; /** @var UserBackend */ private $userBackend; /** @var IConfig */ @@ -76,7 +76,7 @@ class SAMLController extends Controller { * @param IRequest $request * @param ISession $session * @param IUserSession $userSession - * @param SAMLSettings $SAMLSettings + * @param SAMLSettings $samlSettings * @param UserBackend $userBackend * @param IConfig $config * @param IURLGenerator $urlGenerator @@ -84,24 +84,24 @@ class SAMLController extends Controller { * @param IL10N $l */ public function __construct( - $appName, - IRequest $request, - ISession $session, - IUserSession $userSession, - SAMLSettings $SAMLSettings, - UserBackend $userBackend, - IConfig $config, + $appName, + IRequest $request, + ISession $session, + IUserSession $userSession, + SAMLSettings $samlSettings, + UserBackend $userBackend, + IConfig $config, IURLGenerator $urlGenerator, - ILogger $logger, - IL10N $l, - UserResolver $userResolver, - UserData $userData, - ICrypto $crypto + ILogger $logger, + IL10N $l, + UserResolver $userResolver, + UserData $userData, + ICrypto $crypto ) { parent::__construct($appName, $request); $this->session = $session; $this->userSession = $userSession; - $this->SAMLSettings = $SAMLSettings; + $this->samlSettings = $samlSettings; $this->userBackend = $userBackend; $this->config = $config; $this->urlGenerator = $urlGenerator; @@ -171,7 +171,7 @@ class SAMLController extends Controller { $type = $this->config->getAppValue($this->appName, 'type'); switch ($type) { case 'saml': - $auth = new Auth($this->SAMLSettings->getOneLoginSettingsArray($idp)); + $auth = new Auth($this->samlSettings->getOneLoginSettingsArray($idp)); $ssoUrl = $auth->login(null, [], false, false, true); $response = new Http\RedirectResponse($ssoUrl); @@ -242,7 +242,7 @@ class SAMLController extends Controller { * @throws Error */ public function getMetadata($idp) { - $settings = new Settings($this->SAMLSettings->getOneLoginSettingsArray($idp)); + $settings = new Settings($this->samlSettings->getOneLoginSettingsArray($idp)); $metadata = $settings->getSPMetadata(); $errors = $settings->validateMetadata($metadata); if (empty($errors)) { @@ -304,7 +304,7 @@ class SAMLController extends Controller { return new Http\RedirectResponse($this->urlGenerator->getAbsoluteURL('/')); } - $auth = new Auth($this->SAMLSettings->getOneLoginSettingsArray($idp)); + $auth = new Auth($this->samlSettings->getOneLoginSettingsArray($idp)); $auth->processResponse($AuthNRequestID); $this->logger->debug('Attributes send by the IDP: ' . json_encode($auth->getAttributes())); @@ -411,14 +411,14 @@ class SAMLController extends Controller { if ($pass) { $idp = $this->session->get('user_saml.Idp'); - $auth = new Auth($this->SAMLSettings->getOneLoginSettingsArray($idp)); + $auth = new Auth($this->samlSettings->getOneLoginSettingsArray($idp)); $stay = true ; // $auth will return the redirect URL but won't perform the redirect himself if ($isFromIDP) { $keepLocalSession = true ; // do not let processSLO to delete the entire session. Let userSession->logout do the job $targetUrl = $auth->processSLO( $keepLocalSession, null, - $this->SAMLSettings->usesSloWebServerDecode(), + $this->samlSettings->usesSloWebServerDecode(), null, $stay ); @@ -490,7 +490,7 @@ class SAMLController extends Controller { public function selectUserBackEnd($redirectUrl) { $attributes = ['loginUrls' => []]; - if ($this->SAMLSettings->allowMultipleUserBackEnds()) { + if ($this->samlSettings->allowMultipleUserBackEnds()) { $displayName = $this->l->t('Direct log in'); $customDisplayName = $this->config->getAppValue('user_saml', 'directLoginName', ''); @@ -520,7 +520,7 @@ class SAMLController extends Controller { */ private function getIdps($redirectUrl) { $result = []; - $idps = $this->SAMLSettings->getListOfIdps(); + $idps = $this->samlSettings->getListOfIdps(); foreach ($idps as $idpId => $displayName) { $result[] = [ 'url' => $this->getSSOUrl($redirectUrl, $idpId), diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index 58f2361..20e71b3 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -23,8 +23,10 @@ namespace OCA\User_SAML\Controller; +use OCA\User_SAML\SAMLSettings; use OCA\User_SAML\Settings\Admin; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\Response; use OCP\IConfig; use OCP\IRequest; @@ -35,21 +37,32 @@ class SettingsController extends Controller { private $config; /** @var Admin */ private $admin; + /** @var SAMLSettings */ + private $samlSettings; - public function __construct($appName, - IRequest $request, - IConfig $config, - Admin $admin) { + public function __construct( + $appName, + IRequest $request, + IConfig $config, + Admin $admin, + SAMLSettings $samlSettings + ) { parent::__construct($appName, $request); $this->config = $config; $this->admin = $admin; + $this->samlSettings = $samlSettings; + } + + public function getSamlProviderIds(): DataResponse { + $keys = array_keys($this->samlSettings->getListOfIdps()); + return new DataResponse([ 'providerIds' => implode(',', $keys)]); } /** * @param $providerId * @return array of categories containing entries for each config parameter with their value */ - public function getSamlProviderSettings($providerId) { + public function getSamlProviderSettings(int $providerId) { /** * This uses the list of available config parameters from the admin section * and extends it with fields that are not coming from \OCA\User_SAML\Settings\Admin @@ -64,12 +77,12 @@ class SettingsController extends Controller { ]; /* Fetch all config values for the given providerId */ $settings = []; + $storedSettings = $this->samlSettings->get($providerId); foreach ($params as $category => $content) { if (empty($content) || $category === 'providers' || $category === 'type') { continue; } foreach ($content as $setting => $details) { - $prefix = $providerId === '1' ? '' : $providerId . '-'; /* use security as category instead of security-* */ if (strpos($category, 'security-') === 0) { $category = 'security'; @@ -78,42 +91,34 @@ class SettingsController extends Controller { // as this is the only category that has the saml- prefix on config keys if (strpos($category, 'attribute-mapping') === 0) { $category = 'attribute-mapping'; - $key = $prefix . 'saml-attribute-mapping' . '-' . $setting; + $key = 'saml-attribute-mapping' . '-' . $setting; } else { - $key = $prefix . $category . '-' . $setting; + $key = $category . '-' . $setting; + } + + if (isset ($details['global']) && $details['global']) { + $settings[$category][$setting] = $this->config->getAppValue('user_saml', $key, ''); + } else { + $settings[$category][$setting] = $storedSettings[$key] ?? ''; } - $settings[$category][$setting] = $this->config->getAppValue('user_saml', $key, ''); } } return $settings; } public function deleteSamlProviderSettings($providerId) { - $params = $this->admin->getForm()->getParams(); - $params['idp'] = [ - 'singleLogoutService.url' => null, - 'singleLogoutService.responseUrl' => null, - 'singleSignOnService.url' => null, - 'idp-entityId' => null, - ]; - /* Fetch all config values for the given providerId */ - foreach ($params as $category => $content) { - if (!is_array($content) || $category === 'providers') { - continue; - } - foreach ($content as $setting => $details) { - if (isset($details['global']) && $details['global'] === true) { - continue; - } - $prefix = $providerId === '1' ? '' : $providerId . '-'; - $key = $prefix . $category . '-' . $setting; - /* use security as category instead of security-* */ - if (strpos($category, 'security-') === 0) { - $category = 'security'; - } - $this->config->deleteAppValue('user_saml', $key); - } - } + $this->samlSettings->delete($providerId); return new Response(); } + + public function setProviderSetting(int $providerId, string $configKey, string $configValue) { + $configuration = $this->samlSettings->get($providerId); + $configuration[$configKey] = $configValue; + $this->samlSettings->set($providerId, $configuration); + return new Response(); + } + + public function newSamlProviderSettingsId() { + return new DataResponse(['id' => $this->samlSettings->getNewProviderId()]); + } } diff --git a/lib/DavPlugin.php b/lib/DavPlugin.php index 59285c4..ebb18a4 100644 --- a/lib/DavPlugin.php +++ b/lib/DavPlugin.php @@ -35,11 +35,13 @@ class DavPlugin extends ServerPlugin { private $auth; /** @var Server */ private $server; + private SAMLSettings $samlSettings; - public function __construct(ISession $session, IConfig $config, array $auth) { + public function __construct(ISession $session, IConfig $config, array $auth, SAMLSettings $samlSettings) { $this->session = $session; $this->config = $config; $this->auth = $auth; + $this->samlSettings = $samlSettings; } @@ -54,7 +56,7 @@ class DavPlugin extends ServerPlugin { $this->config->getAppValue('user_saml', 'type') === 'environment-variable' && !$this->session->exists('user_saml.samlUserData') ) { - $uidMapping = $this->config->getAppValue('user_saml', 'general-uid_mapping'); + $uidMapping = $this->samlSettings->get(1)['general-uid_mapping']; if (isset($this->auth[$uidMapping])) { $this->session->set(Auth::DAV_AUTHENTICATED, $this->auth[$uidMapping]); $this->session->set('user_saml.samlUserData', $this->auth); diff --git a/lib/Db/ConfigurationsEntity.php b/lib/Db/ConfigurationsEntity.php new file mode 100644 index 0000000..fb73457 --- /dev/null +++ b/lib/Db/ConfigurationsEntity.php @@ -0,0 +1,71 @@ + + * + * @author Arthur Schiwon + * + * @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\User_SAML\Db; + +use OCP\AppFramework\Db\Entity; +use function json_decode; +use function json_encode; + +/** + * @method string getName() + * @method void setName(string $value) + * @method void setConfiguration(string $value) + * @method string getConfiguration() + */ +class ConfigurationsEntity extends Entity { + /** @var string */ + public $name; + /** @var string */ + public $configuration; + + public function __construct() { + $this->addType('name', 'string'); + $this->addType('configuration', 'string'); + } + + /** + * sets also the name, because it is a shorthand to 'general-idp0_display_name' + * + * @throws \JsonException + */ + public function importConfiguration(array $configuration): void { + $this->setConfiguration(json_encode($configuration, JSON_THROW_ON_ERROR)); + $this->setName($configuration['general-idp0_display_name'] ?? ''); + } + + public function getConfigurationArray(): array { + return json_decode($this->configuration, true) ?? []; + } + + public function asArray(): array { + return [ + 'id' => $this->getId(), + 'name' => $this->getName(), + 'configuration' => $this->getConfigurationArray() + ]; + } +} diff --git a/lib/Db/ConfigurationsMapper.php b/lib/Db/ConfigurationsMapper.php new file mode 100644 index 0000000..c1a6bee --- /dev/null +++ b/lib/Db/ConfigurationsMapper.php @@ -0,0 +1,102 @@ + + * + * @author Arthur Schiwon + * + * @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\User_SAML\Db; + +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\QBMapper; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; + +class ConfigurationsMapper extends QBMapper { + public function __construct(IDBConnection $db) { + parent::__construct($db, 'user_saml_configurations', ConfigurationsEntity::class); + } + + public function set(int $id, array $configuration): void { + $entity = new ConfigurationsEntity(); + $entity->setId($id); + $entity->importConfiguration($configuration); + $this->insertOrUpdate($entity); + } + + public function deleteById(int $id): void { + $entity = new ConfigurationsEntity(); + $entity->setId($id);; + $this->delete($entity); + } + + public function getAll(): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('id', 'configuration') + ->from('user_saml_configurations') + ->orderBy('id', 'ASC'); + + /** @var ConfigurationsEntity $entity */ + $entities = $this->findEntities($qb); + $result = []; + foreach ($entities as $entity) { + $result[$entity->getId()] = $entity->getConfigurationArray(); + } + return $result; + } + + public function get(int $idp): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('id', 'configuration') + ->from('user_saml_configurations') + ->where($qb->expr()->eq('id', $qb->createNamedParameter($idp, IQueryBuilder::PARAM_INT))); + + /** @var ConfigurationsEntity $entity */ + try { + $entity = $this->findEntity($qb); + } catch (DoesNotExistException $e) { + return []; + } + return $entity->getConfigurationArray(); + } + + public function reserveId(): int { + $qb = $this->db->getQueryBuilder(); + $qb->select('id') + ->from('user_saml_configurations') + ->orderBy('id', 'DESC') + ->setMaxResults(1); + + try { + $entity = $this->findEntity($qb); + $newId = $entity->getId() + 1; + } catch (DoesNotExistException $e) { + $newId = 1; + } + + $newEntity = new ConfigurationsEntity(); + $newEntity->setId($newId); + $newEntity->importConfiguration([]); + return $this->insert($newEntity)->getId(); + } + +} diff --git a/lib/Migration/Version5000Date20211025124248.php b/lib/Migration/Version5000Date20211025124248.php new file mode 100644 index 0000000..887e965 --- /dev/null +++ b/lib/Migration/Version5000Date20211025124248.php @@ -0,0 +1,218 @@ +dbc = $dbc; + } + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + */ + public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + } + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + if (!$schema->hasTable('user_saml_configurations')) { + $table = $schema->createTable('user_saml_configurations'); + $table->addColumn('id', Types::INTEGER, [ + 'notnull' => true, + ]); + $table->addColumn('name', Types::STRING, [ + 'length' => 256, + 'notnull' => false, + 'default' => '', + ]); + $table->addColumn('configuration', Types::TEXT, [ + 'notnull' => true, + ]); + $table->setPrimaryKey(['id'], 'idx_user_saml_config'); + } + + return $schema; + } + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + */ + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + $prefixes = $this->fetchPrefixes(); + foreach ($prefixes as $prefix) { + $keyStart = $prefix === 1 ? '' : $prefix . '-'; + $configKeys = array_reduce( + self::IDP_CONFIG_KEYS, + function (array $carry, string $rawConfigKey) use ($keyStart): array { + $carry[] = $keyStart . $rawConfigKey; + return $carry; + }, + [] + ); + + $configData = $keysToDelete = []; + $gRows = $this->readConfiguration($configKeys); + while ($row = $gRows->current()) { + $configData[$this->normalizeKey($row['configkey'])] = $row['configvalue']; + $keysToDelete[] = $row['configkey']; + $gRows->next(); + } + + if ($this->insertConfiguration($prefix, $configData) && !empty($keysToDelete)) { + $this->deleteOldConfiguration($keysToDelete); + } + } + + $this->deletePrefixes(); + } + + protected function deleteOldConfiguration(array $keys): bool { + static $deleteQuery; + if (!$deleteQuery) { + $deleteQuery = $this->dbc->getQueryBuilder(); + + $deleteQuery->delete('appconfig') + ->where($deleteQuery->expr()->eq('appid', $deleteQuery->createNamedParameter('user_saml'))) + ->andWhere($deleteQuery->expr()->in('configkey', $deleteQuery->createParameter('cfgKeys'))); + } + + $deletedRows = $deleteQuery + ->setParameter('cfgKeys', $keys, IQueryBuilder::PARAM_STR_ARRAY) + ->executeStatement(); + + return $deletedRows > 0; + } + + protected function insertConfiguration(int $id, array $configData): bool { + static $insertQuery; + if (!$insertQuery) { + $insertQuery = $this->dbc->getQueryBuilder(); + $insertQuery->insert('user_saml_configurations') + ->values([ + 'id' => $insertQuery->createParameter('configId'), + 'name' => $insertQuery->createParameter('displayName'), + 'configuration' => $insertQuery->createParameter('configuration'), + ]); + } + + $insertedRows = $insertQuery + ->setParameter('configId', $id) + ->setParameter('displayName', $configData['general-idp0_display_name'] ?? '') + ->setParameter('configuration', \json_encode($configData, JSON_THROW_ON_ERROR)) + ->executeStatement(); + + return $insertedRows > 0; + } + + protected function readConfiguration(array $configKeys): \Generator { + static $readQuery; + if (!$readQuery) { + $readQuery = $this->dbc->getQueryBuilder(); + } + $readQuery->select('configkey', 'configvalue') + ->from('appconfig') + ->where($readQuery->expr()->eq('appid', $readQuery->createNamedParameter('user_saml'))) + ->andWhere($readQuery->expr()->in('configkey', $readQuery->createParameter('cfgKeys'))); + + $r = $readQuery->setParameter('cfgKeys', $configKeys, IQueryBuilder::PARAM_STR_ARRAY) + ->executeQuery(); + + while ($row = $r->fetch()) { + yield $row; + } + $r->closeCursor(); + } + + protected function normalizeKey(string $prefixedKey): string { + $isPrefixed = \preg_match('/^[0-9]*-/', $prefixedKey, $matches); + if ($isPrefixed === 0) { + return $prefixedKey; + } else if ($isPrefixed === 1) { + return \substr($prefixedKey, strlen($matches[0])); + } + throw new \RuntimeException('Invalid regex pattern'); + } + + protected function fetchPrefixes(): array { + $q = $this->dbc->getQueryBuilder(); + $q->select('configvalue') + ->from('appconfig') + ->where($q->expr()->eq('appid', $q->createNamedParameter('user_saml'))) + ->andWhere($q->expr()->eq('configkey', $q->createNamedParameter('providerIds'))); + + $r = $q->executeQuery(); + $prefixes = $r->fetchOne(); + if ($prefixes === false) { + return []; + } + return array_map('intval', explode(',', $prefixes)); + } + + protected function deletePrefixes(): void { + $q = $this->dbc->getQueryBuilder(); + $q->delete('appconfig') + ->where($q->expr()->eq('appid', $q->createNamedParameter('user_saml'))) + ->andWhere($q->expr()->eq('configkey', $q->createNamedParameter('providerIds'))) + ->executeStatement(); + } +} diff --git a/lib/SAMLSettings.php b/lib/SAMLSettings.php index efc35bb..4c27c90 100644 --- a/lib/SAMLSettings.php +++ b/lib/SAMLSettings.php @@ -21,6 +21,9 @@ namespace OCA\User_SAML; +use InvalidArgumentException; +use OCA\User_SAML\Db\ConfigurationsMapper; +use OCP\DB\Exception; use OCP\IConfig; use OCP\IRequest; use OCP\ISession; @@ -28,16 +31,59 @@ use OCP\IURLGenerator; use OneLogin\Saml2\Constants; class SAMLSettings { + private const LOADED_NONE = 0; + private const LOADED_CHOSEN = 1; + private const LOADED_ALL = 2; + + public const IDP_CONFIG_KEYS = [ + 'general-idp0_display_name', + 'general-uid_mapping', + 'idp-entityId', + 'idp-singleLogoutService.responseUrl', + 'idp-singleLogoutService.url', + 'idp-singleSignOnService.url', + 'idp-x509cert', + 'security-authnRequestsSigned', + 'security-general', + 'security-logoutRequestSigned', + 'security-logoutResponseSigned', + 'security-lowercaseUrlencoding', + 'security-nameIdEncrypted', + 'security-offer', + 'security-required', + 'security-signatureAlgorithm', + 'security-signMetadata', + 'security-sloWebServerDecode', + 'security-wantAssertionsEncrypted', + 'security-wantAssertionsSigned', + 'security-wantMessagesSigned', + 'security-wantNameId', + 'security-wantNameIdEncrypted', + 'security-wantXMLValidation', + 'saml-attribute-mapping-displayName_mapping', + 'saml-attribute-mapping-email_mapping', + 'saml-attribute-mapping-group_mapping', + 'saml-attribute-mapping-home_mapping', + 'saml-attribute-mapping-quota_mapping', + 'sp-x509cert', + 'sp-name-id-format', + 'sp-privateKey', + ]; + /** @var IURLGenerator */ private $urlGenerator; /** @var IConfig */ private $config; - /** @var IRequest */ - private $request; /** @var ISession */ private $session; /** @var array list of global settings which are valid for every idp */ private $globalSettings = ['general-require_provisioned_account', 'general-allow_multiple_user_back_ends', 'general-use_saml_auth_for_desktop']; + /** @var array> */ + private $configurations; + /** @var int */ + private $configurationsLoadedState = self::LOADED_NONE; + /** @var ConfigurationsMapper */ + private $mapper; /** * @param IURLGenerator $urlGenerator @@ -45,148 +91,165 @@ class SAMLSettings { * @param IRequest $request * @param ISession $session */ - public function __construct(IURLGenerator $urlGenerator, - IConfig $config, - IRequest $request, - ISession $session) { + public function __construct( + IURLGenerator $urlGenerator, + IConfig $config, + ISession $session, + ConfigurationsMapper $mapper + ) { $this->urlGenerator = $urlGenerator; $this->config = $config; - $this->request = $request; $this->session = $session; + $this->mapper = $mapper; } /** * get list of the configured IDPs * - * @return array + * @return array + * @throws Exception */ - public function getListOfIdps() { + public function getListOfIdps(): array { + $this->ensureConfigurationsLoaded(); + $result = []; - - $providerIds = explode(',', $this->config->getAppValue('user_saml', 'providerIds', '1')); - natsort($providerIds); - - foreach ($providerIds as $id) { - $prefix = $id === '1' ? '' : $id .'-'; - $result[$id] = $this->config->getAppValue('user_saml', $prefix . 'general-idp0_display_name', ''); + foreach ($this->configurations as $configID => $config) { + // no fancy array_* method, because there might be thousands + $result[$configID] = $config['general-idp0_display_name'] ?? ''; } - asort($result); - return $result; } /** * check if multiple user back ends are allowed - * - * @return bool */ - public function allowMultipleUserBackEnds() { + public function allowMultipleUserBackEnds(): bool { $type = $this->config->getAppValue('user_saml', 'type'); $setting = $this->config->getAppValue('user_saml', 'general-allow_multiple_user_back_ends', '0'); - return ($setting === '1' && $type === 'saml'); + return ($setting === '1' && $type === 'saml'); } - public function usesSloWebServerDecode() : bool { + public function usesSloWebServerDecode(): bool { return $this->config->getAppValue('user_saml', 'security-sloWebServerDecode', '0') === '1'; } /** * get config for given IDP * - * @param int $idp - * @return array + * @throws Exception */ - public function getOneLoginSettingsArray($idp) { - $prefix = ''; - if ($idp > 1) { - $prefix = $idp . '-'; - } + public function getOneLoginSettingsArray(int $idp): array { + $this->ensureConfigurationsLoaded($idp); $settings = [ 'strict' => true, 'debug' => $this->config->getSystemValue('debug', false), 'baseurl' => $this->urlGenerator->linkToRouteAbsolute('user_saml.SAML.base'), 'security' => [ - 'nameIdEncrypted' => ($this->config->getAppValue('user_saml', $prefix . 'security-nameIdEncrypted', '0') === '1') ? true : false, - 'authnRequestsSigned' => ($this->config->getAppValue('user_saml', $prefix . 'security-authnRequestsSigned', '0') === '1') ? true : false, - 'logoutRequestSigned' => ($this->config->getAppValue('user_saml', $prefix . 'security-logoutRequestSigned', '0') === '1') ? true : false, - 'logoutResponseSigned' => ($this->config->getAppValue('user_saml', $prefix . 'security-logoutResponseSigned', '0') === '1') ? true : false, - 'signMetadata' => ($this->config->getAppValue('user_saml', $prefix . 'security-signMetadata', '0') === '1') ? true : false, - 'wantMessagesSigned' => ($this->config->getAppValue('user_saml', $prefix . 'security-wantMessagesSigned', '0') === '1') ? true : false, - 'wantAssertionsSigned' => ($this->config->getAppValue('user_saml', $prefix . 'security-wantAssertionsSigned', '0') === '1') ? true : false, - 'wantAssertionsEncrypted' => ($this->config->getAppValue('user_saml', $prefix . 'security-wantAssertionsEncrypted', '0') === '1') ? true : false, - 'wantNameId' => ($this->config->getAppValue('user_saml', $prefix . 'security-wantNameId', '0') === '1') ? true : false, - 'wantNameIdEncrypted' => ($this->config->getAppValue('user_saml', $prefix . 'security-wantNameIdEncrypted', '0') === '1') ? true : false, - 'wantXMLValidation' => ($this->config->getAppValue('user_saml', $prefix . 'security-wantXMLValidation', '0') === '1') ? true : false, + 'nameIdEncrypted' => ($this->configurations[$idp]['security-nameIdEncrypted'] ?? '0') === '1', + 'authnRequestsSigned' => ($this->configurations[$idp]['security-authnRequestsSigned'] ?? '0') === '1', + 'logoutRequestSigned' => ($this->configurations[$idp]['security-logoutRequestSigned'] ?? '0') === '1', + 'logoutResponseSigned' => ($this->configurations[$idp]['security-logoutResponseSigned'] ?? '0') === '1', + 'signMetadata' => ($this->configurations[$idp]['security-signMetadata'] ?? '0') === '1', + 'wantMessagesSigned' => ($this->configurations[$idp]['security-wantMessagesSigned'] ?? '0') === '1', + 'wantAssertionsSigned' => ($this->configurations[$idp]['security-wantAssertionsSigned'] ?? '0') === '1', + 'wantAssertionsEncrypted' => ($this->configurations[$idp]['security-wantAssertionsEncrypted'] ?? '0') === '1', + 'wantNameId' => ($this->configurations[$idp]['security-wantNameId'] ?? '0') === '1', + 'wantNameIdEncrypted' => ($this->configurations[$idp]['security-wantNameIdEncrypted'] ?? '0') === '1', + 'wantXMLValidation' => ($this->configurations[$idp]['security-wantXMLValidation'] ?? '0') === '1', 'requestedAuthnContext' => false, - 'lowercaseUrlencoding' => ($this->config->getAppValue('user_saml', $prefix . 'security-lowercaseUrlencoding', '0') === '1') ? true : false, - 'signatureAlgorithm' => $this->config->getAppValue('user_saml', $prefix . 'security-signatureAlgorithm', null) + 'lowercaseUrlencoding' => ($this->configurations[$idp]['security-lowercaseUrlencoding'] ?? '0') === '1', + 'signatureAlgorithm' => $this->configurations[$idp]['security-signatureAlgorithm'] ?? null, ], 'sp' => [ 'entityId' => $this->urlGenerator->linkToRouteAbsolute('user_saml.SAML.getMetadata'), 'assertionConsumerService' => [ 'url' => $this->urlGenerator->linkToRouteAbsolute('user_saml.SAML.assertionConsumerService'), ], - 'NameIDFormat' => $this->config->getAppValue('user_saml', $prefix . 'sp-name-id-format', Constants::NAMEID_UNSPECIFIED) + 'NameIDFormat' => $this->configurations[$idp]['sp-name-id-format'] ?? Constants::NAMEID_UNSPECIFIED, + 'x509cert' => $this->configurations[$idp]['sp-x509cert'] ?? '', + 'privateKey' => $this->configurations[$idp]['sp-privateKey'] ?? '', ], 'idp' => [ - 'entityId' => $this->config->getAppValue('user_saml', $prefix . 'idp-entityId', ''), + 'entityId' => $this->configurations[$idp]['idp-entityId'] ?? '', 'singleSignOnService' => [ - 'url' => $this->config->getAppValue('user_saml', $prefix . 'idp-singleSignOnService.url', ''), + 'url' => $this->configurations[$idp]['idp-singleSignOnService.url'] ?? '', ], + 'x509cert' => $this->configurations[$idp]['idp-x509cert'] ?? '', ], ]; - $spx509cert = $this->config->getAppValue('user_saml', $prefix . 'sp-x509cert', ''); - $spxprivateKey = $this->config->getAppValue('user_saml', $prefix . 'sp-privateKey', ''); - if ($spx509cert !== '') { - $settings['sp']['x509cert'] = $spx509cert; - } - if ($spxprivateKey !== '') { - $settings['sp']['privateKey'] = $spxprivateKey; - } - - $idpx509cert = $this->config->getAppValue('user_saml', $prefix . 'idp-x509cert', ''); - if ($idpx509cert !== '') { - $settings['idp']['x509cert'] = $idpx509cert; - } - - $slo = $this->config->getAppValue('user_saml', $prefix . 'idp-singleLogoutService.url', ''); - if ($slo !== '') { - $settings['idp']['singleLogoutService'] = [ - 'url' => $this->config->getAppValue('user_saml', $prefix . 'idp-singleLogoutService.url', ''), - ]; - $settings['sp']['singleLogoutService'] = [ - 'url' => $this->urlGenerator->linkToRouteAbsolute('user_saml.SAML.singleLogoutService'), - ]; - - $sloResponseUrl = $this->config->getAppValue('user_saml', $prefix . 'idp-singleLogoutService.responseUrl', ''); - if ($sloResponseUrl !== '') { - $settings['idp']['singleLogoutService']['responseUrl'] = $sloResponseUrl; + // must be added only if configured + if (($this->configurations[$idp]['idp-singleLogoutService.url'] ?? '') !== '') { + $settings['idp']['singleLogoutService'] = ['url' => $this->configurations[$idp]['idp-singleLogoutService.url']]; + $settings['sp']['singleLogoutService'] = ['url' => $this->urlGenerator->linkToRouteAbsolute('user_saml.SAML.singleLogoutService')]; + if (($this->configurations[$idp]['idp-singleLogoutService.responseUrl'] ?? '') !== '') { + $settings['idp']['singleLogoutService']['responseUrl'] = $this->configurations[$idp]['idp-singleLogoutService.responseUrl']; } } return $settings; } + public function getProviderId(): int { + // defaults to 1, needed for env-mode + return (int)($this->session->get('user_saml.Idp') ?? 1); + } + + public function getNewProviderId(): int { + return $this->mapper->reserveId(); + } + /** - * calculate prefix for config values - * - * @param string name of the setting - * @return string + * @throws Exception */ - public function getPrefix($setting = '') { - $prefix = ''; - if (!empty($setting) && in_array($setting, $this->globalSettings)) { - return $prefix; + public function get(int $id): array { + $this->ensureConfigurationsLoaded($id); + return $this->configurations[$id] ?? []; + } + + /** + * @throws InvalidArgumentException + */ + public function set(int $id, array $settings): void { + foreach (array_keys($settings) as $configKey) { + if (!in_array($configKey, self::IDP_CONFIG_KEYS)) { + throw new InvalidArgumentException('Invalid config key'); + } } - $idp = $this->session->get('user_saml.Idp'); - if ((int)$idp > 1) { - $prefix = $idp . '-'; + $this->mapper->set($id, $settings); + } + + /** + * @throws Exception + */ + public function delete(int $id): void { + $this->mapper->deleteById($id); + } + + /** + * @throws Exception + */ + protected function ensureConfigurationsLoaded(int $idp = -1): void { + if (self::LOADED_ALL === $this->configurationsLoadedState + || (self::LOADED_CHOSEN === $this->configurationsLoadedState + && isset($this->configurations[$idp]) + ) + ) { + return; } - return $prefix; + if ($idp !== -1) { + $this->configurations[$idp] = $this->mapper->get($idp); + } else { + $configs = $this->mapper->getAll(); + foreach ($configs as $id => $config) { + $this->configurations[$id] = $config; + } + } + + $this->configurationsLoadedState = $idp === -1 ? self::LOADED_ALL : self::LOADED_CHOSEN; } } diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php index 9423ad0..64c328f 100644 --- a/lib/Settings/Admin.php +++ b/lib/Settings/Admin.php @@ -23,6 +23,7 @@ namespace OCA\User_SAML\Settings; +use OCA\User_SAML\SAMLSettings; use OCP\AppFramework\Http\TemplateResponse; use OCP\Defaults; use OCP\IConfig; @@ -37,30 +38,33 @@ class Admin implements ISettings { private $defaults; /** @var IConfig */ private $config; + /** @var SAMLSettings */ + private $samlSettings; /** * @param IL10N $l10n * @param Defaults $defaults * @param IConfig $config */ - public function __construct(IL10N $l10n, - Defaults $defaults, - IConfig $config) { + public function __construct( + IL10N $l10n, + Defaults $defaults, + IConfig $config, + SAMLSettings $samlSettings + ) { $this->l10n = $l10n; $this->defaults = $defaults; $this->config = $config; + $this->samlSettings = $samlSettings; } /** * @return TemplateResponse */ public function getForm() { - $providerIds = explode(',', $this->config->getAppValue('user_saml', 'providerIds', '1')); - natsort($providerIds); + $providerIds = $this->samlSettings->getListOfIdps(); $providers = []; - foreach ($providerIds as $id) { - $prefix = $id === '1' ? '' : $id .'-'; - $name = $this->config->getAppValue('user_saml', $prefix . 'general-idp0_display_name', ''); + foreach ($providerIds as $id => $name) { $providers[] = [ 'id' => $id, 'name' => $name === '' ? $this->l10n->t('Provider ') . $id : $name @@ -134,43 +138,42 @@ class Admin implements ISettings { ]; - $selectedNameIdFormat = $this->config->getAppValue('user_saml', 'sp-name-id-format', Constants::NAMEID_UNSPECIFIED); $nameIdFormats = [ Constants::NAMEID_EMAIL_ADDRESS => [ 'label' => $this->l10n->t('Email address'), - 'selected' => $selectedNameIdFormat === Constants::NAMEID_EMAIL_ADDRESS, + 'selected' => false, ], Constants::NAMEID_ENCRYPTED => [ 'label' => $this->l10n->t('Encrypted'), - 'selected' => $selectedNameIdFormat === Constants::NAMEID_ENCRYPTED, + 'selected' => false, ], Constants::NAMEID_ENTITY => [ 'label' => $this->l10n->t('Entity'), - 'selected' => $selectedNameIdFormat === Constants::NAMEID_ENTITY, + 'selected' => false, ], Constants::NAMEID_KERBEROS => [ 'label' => $this->l10n->t('Kerberos'), - 'selected' => $selectedNameIdFormat === Constants::NAMEID_KERBEROS, + 'selected' => false, ], Constants::NAMEID_PERSISTENT => [ 'label' => $this->l10n->t('Persistent'), - 'selected' => $selectedNameIdFormat === Constants::NAMEID_PERSISTENT, + 'selected' => false, ], Constants::NAMEID_TRANSIENT => [ 'label' => $this->l10n->t('Transient'), - 'selected' => $selectedNameIdFormat === Constants::NAMEID_TRANSIENT, + 'selected' => false, ], Constants::NAMEID_UNSPECIFIED => [ 'label' => $this->l10n->t('Unspecified'), - 'selected' => $selectedNameIdFormat === Constants::NAMEID_UNSPECIFIED, + 'selected' => false, ], Constants::NAMEID_WINDOWS_DOMAIN_QUALIFIED_NAME => [ 'label' => $this->l10n->t('Windows domain qualified name'), - 'selected' => $selectedNameIdFormat === Constants::NAMEID_WINDOWS_DOMAIN_QUALIFIED_NAME, + 'selected' => false, ], Constants::NAMEID_X509_SUBJECT_NAME => [ 'label' => $this->l10n->t('X509 subject name'), - 'selected' => $selectedNameIdFormat === Constants::NAMEID_X509_SUBJECT_NAME, + 'selected' => false, ], ]; diff --git a/lib/UserBackend.php b/lib/UserBackend.php index d7c1f3a..2461a70 100644 --- a/lib/UserBackend.php +++ b/lib/UserBackend.php @@ -107,8 +107,8 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { * @param string $uid * @param array $attributes */ - public function createUserIfNotExists($uid, array $attributes = []) { - if (!$this->userExistsInDatabase($uid)) { + public function createUserIfNotExists($uid, array $attributes = array()) { + if(!$this->userExistsInDatabase($uid)) { $values = [ 'uid' => $uid, ]; @@ -123,12 +123,12 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { if ($home !== '') { //if attribute's value is an absolute path take this, otherwise append it to data dir //check for / at the beginning or pattern c:\ resp. c:/ - if ('/' !== $home[0] + if( '/' !== $home[0] && !(3 < strlen($home) && ctype_alpha($home[0]) && $home[1] === ':' && ('\\' === $home[2] || '/' === $home[2])) ) { $home = $this->config->getSystemValue('datadirectory', - \OC::$SERVERROOT.'/data') . '/' . $home; + \OC::$SERVERROOT.'/data' ) . '/' . $home; } $values['home'] = $home; @@ -137,12 +137,13 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $qb->insert('user_saml_users'); - foreach ($values as $column => $value) { + foreach($values as $column => $value) { $qb->setValue($column, $qb->createNamedParameter($value)); } $qb->execute(); $this->initializeHomeDir($uid); + } } @@ -202,8 +203,8 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { $data = $result->fetchAll(); $result->closeCursor(); - foreach ($data as $passwords) { - if (password_verify($password, $passwords['token'])) { + foreach($data as $passwords) { + if(password_verify($password, $passwords['token'])) { return $uid; } } @@ -218,7 +219,7 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { * @since 4.5.0 */ public function deleteUser($uid) { - if ($this->userExistsInDatabase($uid)) { + if($this->userExistsInDatabase($uid)) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $qb->delete('user_saml_users') @@ -236,7 +237,7 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { * @return string */ public function getHome($uid) { - if ($this->userExistsInDatabase($uid)) { + if($this->userExistsInDatabase($uid)) { $qb = $this->db->getQueryBuilder(); $qb->select('home') ->from('user_saml_users') @@ -276,7 +277,7 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { * @since 4.5.0 */ public function userExists($uid) { - if ($backend = $this->getActualUserBackend($uid)) { + if($backend = $this->getActualUserBackend($uid)) { return $backend->userExists($uid); } else { return $this->userExistsInDatabase($uid); @@ -284,7 +285,7 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { } public function setDisplayName($uid, $displayName) { - if ($backend = $this->getActualUserBackend($uid)) { + if($backend = $this->getActualUserBackend($uid)) { return $backend->setDisplayName($uid, $displayName); } @@ -308,10 +309,10 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { * @since 4.5.0 */ public function getDisplayName($uid) { - if ($backend = $this->getActualUserBackend($uid)) { + if($backend = $this->getActualUserBackend($uid)) { return $backend->getDisplayName($uid); } else { - if ($this->userExistsInDatabase($uid)) { + if($this->userExistsInDatabase($uid)) { $qb = $this->db->getQueryBuilder(); $qb->select('displayname') ->from('user_saml_users') @@ -373,7 +374,7 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { * @since 4.5.0 */ public function hasUserListings() { - if ($this->autoprovisionAllowed()) { + if($this->autoprovisionAllowed()) { return true; } @@ -394,8 +395,10 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { * {@inheritdoc} */ public function getLogoutUrl() { - $prefix = $this->settings->getPrefix(); - $slo = $this->config->getAppValue('user_saml', $prefix . 'idp-singleLogoutService.url', ''); + $id = $this->settings->getProviderId(); + $settings = $this->settings->get($id); + $slo = $settings['idp-singleLogoutService.url'] ?? ''; + if ($slo === '') { return ''; } @@ -484,14 +487,14 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { public function getCurrentUserId() { $user = \OC::$server->getUserSession()->getUser(); - if ($user instanceof IUser && $this->session->get('user_saml.samlUserData')) { + if($user instanceof IUser && $this->session->get('user_saml.samlUserData')) { $uid = $user->getUID(); } else { $this->userData->setAttributes($this->session->get('user_saml.samlUserData') ?? []); $uid = $this->userData->getEffectiveUid(); } - if ($uid !== '' && $this->userExists($uid)) { + if($uid !== '' && $this->userExists($uid)) { $this->session->set('last-password-confirm', strtotime('+4 year', time())); return $uid; } @@ -524,8 +527,8 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { * @return null|UserInterface */ public function getActualUserBackend($uid) { - foreach (self::$backends as $backend) { - if ($backend->userExists($uid)) { + foreach(self::$backends as $backend) { + if($backend->userExists($uid)) { return $backend; } } @@ -543,9 +546,13 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { self::$backends = $backends; } - private function getAttributeKeys($name) { - $prefix = $this->settings->getPrefix($name); - $keys = explode(' ', $this->config->getAppValue('user_saml', $prefix . $name, '')); + /** + * @throws \OCP\DB\Exception + */ + private function getAttributeKeys($name) + { + $settings = $this->settings->get($this->settings->getProviderId()); + $keys = explode(' ', $settings[$name] ?? $this->config->getAppValue('user_saml', $name, '')); if (count($keys) === 1 && $keys[0] === '') { throw new \InvalidArgumentException('Attribute is not configured'); @@ -557,17 +564,17 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { $keys = $this->getAttributeKeys($name); $value = ''; - foreach ($keys as $key) { + foreach($keys as $key) { if (isset($attributes[$key])) { if (is_array($attributes[$key])) { foreach ($attributes[$key] as $attribute_part_value) { - if ($value !== '') { + if($value !== '') { $value .= ' '; } $value .= $attribute_part_value; } } else { - if ($value !== '') { + if($value !== '') { $value .= ' '; } $value .= $attributes[$key]; @@ -581,8 +588,8 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend { private function getAttributeArrayValue($name, array $attributes) { $keys = $this->getAttributeKeys($name); - $value = []; - foreach ($keys as $key) { + $value = array(); + foreach($keys as $key) { if (isset($attributes[$key])) { if (is_array($attributes[$key])) { $value = array_merge($value, array_values($attributes[$key])); diff --git a/lib/UserData.php b/lib/UserData.php index 01613c4..e0e0607 100644 --- a/lib/UserData.php +++ b/lib/UserData.php @@ -1,5 +1,4 @@ @@ -57,9 +56,8 @@ class UserData { public function hasUidMappingAttribute(): bool { $this->assertIsInitialized(); - $prefix = $this->samlSettings->getPrefix(); - $uidMapping = $this->config->getAppValue('user_saml', $prefix . 'general-uid_mapping'); - return isset($this->attributes[$uidMapping]); + $attribute = $this->getUidMappingAttribute(); + return $attribute !== null && isset($this->attributes[$attribute]); } public function getOriginalUid(): string { @@ -68,7 +66,7 @@ class UserData { } public function getEffectiveUid(): string { - if ($this->uid !== null) { + if($this->uid !== null) { return $this->uid; } $this->assertIsInitialized(); @@ -84,9 +82,8 @@ class UserData { } protected function extractSamlUserId(): string { - $prefix = $this->samlSettings->getPrefix(); - $uidMapping = $this->config->getAppValue('user_saml', $prefix . 'general-uid_mapping'); - if (isset($this->attributes[$uidMapping])) { + $uidMapping = $this->getUidMappingAttribute(); + if($uidMapping !== null && isset($this->attributes[$uidMapping])) { if (is_array($this->attributes[$uidMapping])) { return trim($this->attributes[$uidMapping][0]); } else { @@ -108,13 +105,13 @@ class UserData { } $candidate = base64_decode($uid, true); - if ($candidate === false) { + if($candidate === false) { return $uid; } $candidate = $this->convertObjectGUID2Str($candidate); // the regex only matches the structure of the UUID, not its semantic // (i.e. version or variant) simply to be future compatible - if (preg_match('/^[a-f0-9]{8}(-[a-f0-9]{4}){4}[a-f0-9]{8}$/i', $candidate) === 1) { + if(preg_match('/^[a-f0-9]{8}(-[a-f0-9]{4}){4}[a-f0-9]{8}$/i', $candidate) === 1) { $uid = $candidate; } return $uid; @@ -126,15 +123,15 @@ class UserData { protected function convertObjectGUID2Str($oguid): string { $hex_guid = bin2hex($oguid); $hex_guid_to_guid_str = ''; - for ($k = 1; $k <= 4; ++$k) { + for($k = 1; $k <= 4; ++$k) { $hex_guid_to_guid_str .= substr($hex_guid, 8 - 2 * $k, 2); } $hex_guid_to_guid_str .= '-'; - for ($k = 1; $k <= 2; ++$k) { + for($k = 1; $k <= 2; ++$k) { $hex_guid_to_guid_str .= substr($hex_guid, 12 - 2 * $k, 2); } $hex_guid_to_guid_str .= '-'; - for ($k = 1; $k <= 2; ++$k) { + for($k = 1; $k <= 2; ++$k) { $hex_guid_to_guid_str .= substr($hex_guid, 16 - 2 * $k, 2); } $hex_guid_to_guid_str .= '-' . substr($hex_guid, 16, 4); @@ -144,8 +141,16 @@ class UserData { } protected function assertIsInitialized() { - if ($this->attributes === null) { + if($this->attributes === null) { throw new \LogicException('UserData have to be initialized with setAttributes first'); } } + + protected function getProviderSettings(): array { + return $this->samlSettings->get($this->samlSettings->getProviderId()); + } + + protected function getUidMappingAttribute(): ?string { + return $this->getProviderSettings()['general-uid_mapping'] ?? null; + } } diff --git a/tests/unit/AppInfo/RoutesTest.php b/tests/unit/AppInfo/RoutesTest.php deleted file mode 100644 index a72c7f9..0000000 --- a/tests/unit/AppInfo/RoutesTest.php +++ /dev/null @@ -1,104 +0,0 @@ - - * - * @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\User_SAML\Tests\AppInfo; - -use Test\TestCase; - -class Test extends TestCase { - public function testFile() { - $dir = __DIR__; - $routes = require_once __DIR__ . '/../../../appinfo/routes.php'; - - $expected = [ - 'routes' => [ - [ - 'name' => 'SAML#login', - 'url' => '/saml/login', - 'verb' => 'GET', - ], - [ - 'name' => 'SAML#base', - 'url' => '/saml', - 'verb' => 'GET', - ], - [ - 'name' => 'SAML#getMetadata', - 'url' => '/saml/metadata', - 'verb' => 'GET', - ], - [ - 'name' => 'SAML#assertionConsumerService', - 'url' => '/saml/acs', - 'verb' => 'POST', - ], - [ - 'name' => 'SAML#singleLogoutService', - 'url' => '/saml/sls', - 'verb' => 'GET', - ], - [ - 'name' => 'SAML#singleLogoutService', - 'url' => '/saml/sls', - 'verb' => 'POST', - 'postfix' => 'slspost', - ], - [ - 'name' => 'SAML#notProvisioned', - 'url' => '/saml/notProvisioned', - 'verb' => 'GET', - ], - [ - 'name' => 'SAML#genericError', - 'url' => '/saml/error', - 'verb' => 'GET', - ], - [ - 'name' => 'SAML#selectUserBackEnd', - 'url' => '/saml/selectUserBackEnd', - 'verb' => 'GET', - ], - [ - 'name' => 'Settings#getSamlProviderSettings', - 'url' => '/settings/providerSettings/{providerId}', - 'verb' => 'GET', - 'defaults' => [ - 'providerId' => '1' - ] - ], - [ - 'name' => 'Settings#deleteSamlProviderSettings', - 'url' => '/settings/providerSettings/{providerId}', - 'verb' => 'DELETE', - 'defaults' => [ - 'providerId' => '1' - ] - ], - [ - 'name' => 'Timezone#setTimezone', - 'url' => '/config/timezone', - 'verb' => 'POST', - ], - ], - ]; - $this->assertSame($expected, $routes); - } -} diff --git a/tests/unit/Command/GetMetadataTest.php b/tests/unit/Command/GetMetadataTest.php index 14ee71f..bb0e9f5 100644 --- a/tests/unit/Command/GetMetadataTest.php +++ b/tests/unit/Command/GetMetadataTest.php @@ -23,79 +23,48 @@ namespace OCA\User_SAML\Tests\Command; use OCA\User_SAML\Command\GetMetadata; use OCA\User_SAML\SAMLSettings; +use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; -use OCP\IConfig; -use OCP\IRequest; -use OCP\ISession; -use OCP\IURLGenerator; class GetMetadataTest extends \Test\TestCase { - /** @var GetMetadata|\PHPUnit_Framework_MockObject_MockObject*/ - protected $GetMetadata; - /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ - private $request; - /** @var ISession|\PHPUnit_Framework_MockObject_MockObject */ - private $session; - /** @var SAMLSettings|\PHPUnit_Framework_MockObject_MockObject*/ - private $samlSettings; - /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ - private $config; - /** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */ - private $urlGenerator; + /** @var GetMetadata|MockObject*/ + protected $GetMetadata; + /** @var SAMLSettings|MockObject*/ + private $samlSettings; + protected function setUp(): void { + $this->samlSettings = $this->createMock(SAMLSettings::class); + $this->GetMetadata = new GetMetadata($this->samlSettings); - protected function setUp(): void { - $this->urlGenerator = $this->createMock(IURLGenerator::class); - $this->config = $this->createMock(IConfig::class); - $this->request = $this->createMock(IRequest::class); - $this->session = $this->createMock(ISession::class); - - $this->samlSettings = new SAMLSettings($this->urlGenerator, - $this->config, - $this->request, - $this->session); - $this->GetMetadata = new GetMetadata($this->samlSettings); - - parent::setUp(); - } - - public function testGetMetadata() { + parent::setUp(); + } + public function testGetMetadata(){ $inputInterface = $this->createMock(InputInterface::class); $outputInterface = $this->createMock(OutputInterface::class); - $this->urlGenerator - ->expects($this->at(0)) - ->method('linkToRouteAbsolute') - ->with('user_saml.SAML.base') - ->willReturn('https://nextcloud.com/base/'); - $this->urlGenerator - ->expects($this->at(1)) - ->method('linkToRouteAbsolute') - ->with('user_saml.SAML.getMetadata') - ->willReturn('https://nextcloud.com/metadata/'); - $this->urlGenerator - ->expects($this->at(2)) - ->method('linkToRouteAbsolute') - ->with('user_saml.SAML.assertionConsumerService') - ->willReturn('https://nextcloud.com/acs/'); - $this->config->expects($this->any())->method('getAppValue') - ->willReturnCallback(function ($app, $key, $default) { - if ($key == 'idp-entityId') { - return "dummy"; - } - if ($key == 'idp-singleSignOnService.url') { - return "https://example.com/sso"; - } - if ($key == 'idp-x509cert') { - return "DUMMY CERTIFICATE"; - } - return $default; - }); + + $this->samlSettings->expects($this->any()) + ->method('getOneLoginSettingsArray') + ->willReturn([ + 'baseurl' => 'https://nextcloud.com/base/', + 'idp' => [ + 'entityId' => 'dummy', + 'singleSignOnService' => ['url' => 'https://example.com/sso'], + 'x509cert' => 'DUMMY CERTIFICATE', + ], + 'sp' => [ + 'entityId' => 'https://nextcloud.com/metadata/', + 'assertionConsumerService' => [ + 'url' => 'https://nextcloud.com/acs/', + ], + ] + ]); $outputInterface->expects($this->once())->method('writeln') - ->with($this->stringContains('md:EntityDescriptor')); + ->with($this->stringContains('md:EntityDescriptor')); $this->invokePrivate($this->GetMetadata, 'execute', [$inputInterface, $outputInterface]); } + } diff --git a/tests/unit/Settings/AdminTest.php b/tests/unit/Settings/AdminTest.php index 03d1df2..6162455 100644 --- a/tests/unit/Settings/AdminTest.php +++ b/tests/unit/Settings/AdminTest.php @@ -21,14 +21,19 @@ namespace OCA\User_SAML\Tests\Settings; +use OCA\User_SAML\SAMLSettings; +use OCA\User_SAML\Settings\Admin; use OCP\AppFramework\Http\TemplateResponse; use OCP\Defaults; use OCP\IConfig; use OCP\IL10N; use OneLogin\Saml2\Constants; +use PHPUnit\Framework\MockObject\MockObject; -class AdminTest extends \Test\TestCase { - /** @var \OCA\User_SAML\Settings\Admin */ +class AdminTest extends \Test\TestCase { + /** @var SAMLSettings|MockObject */ + private $settings; + /** @var Admin */ private $admin; /** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */ private $l10n; @@ -41,11 +46,13 @@ class AdminTest extends \Test\TestCase { $this->l10n = $this->createMock(IL10N::class); $this->defaults = $this->createMock(Defaults::class); $this->config = $this->createMock(IConfig::class); + $this->settings = $this->createMock(SAMLSettings::class); - $this->admin = new \OCA\User_SAML\Settings\Admin( + $this->admin = new Admin( $this->l10n, $this->defaults, - $this->config + $this->config, + $this->settings ); parent::setUp(); @@ -55,7 +62,7 @@ class AdminTest extends \Test\TestCase { $this->l10n ->expects($this->any()) ->method('t') - ->will($this->returnCallback(function ($text, $parameters = []) { + ->will($this->returnCallback(function($text, $parameters = array()) { return vsprintf($text, $parameters); })); @@ -169,7 +176,7 @@ class AdminTest extends \Test\TestCase { ], Constants::NAMEID_UNSPECIFIED => [ 'label' => 'Unspecified', - 'selected' => true, + 'selected' => false, ], Constants::NAMEID_WINDOWS_DOMAIN_QUALIFIED_NAME => [ 'label' => 'Windows domain qualified name', @@ -199,26 +206,14 @@ class AdminTest extends \Test\TestCase { } public function testGetFormWithoutType() { + $this->settings->expects($this->once()) + ->method('getListOfIdps') + ->willReturn([ + 1 => 'Provider 1', + 2 => 'Provider 2', + ]); $this->config - ->expects($this->at(0)) - ->method('getAppValue') - ->with('user_saml', 'providerIds') - ->willReturn('1,2'); - $this->config - ->expects($this->at(1)) - ->method('getAppValue') - ->willReturn('Provider 1'); - $this->config - ->expects($this->at(2)) - ->method('getAppValue') - ->willReturn('Provider 2'); - $this->config - ->expects($this->at(3)) - ->method('getAppValue') - ->with('user_saml', 'sp-name-id-format') - ->will($this->returnArgument(2)); - $this->config - ->expects($this->at(4)) + ->expects($this->once()) ->method('getAppValue') ->with('user_saml', 'type') ->willReturn(''); @@ -234,26 +229,14 @@ class AdminTest extends \Test\TestCase { } public function testGetFormWithSaml() { + $this->settings->expects($this->once()) + ->method('getListOfIdps') + ->willReturn([ + 1 => 'Provider 1', + 2 => 'Provider 2', + ]); $this->config - ->expects($this->at(0)) - ->method('getAppValue') - ->with('user_saml', 'providerIds') - ->willReturn('1,2'); - $this->config - ->expects($this->at(1)) - ->method('getAppValue') - ->willReturn('Provider 1'); - $this->config - ->expects($this->at(2)) - ->method('getAppValue') - ->willReturn('Provider 2'); - $this->config - ->expects($this->at(3)) - ->method('getAppValue') - ->with('user_saml', 'sp-name-id-format') - ->will($this->returnArgument(2)); - $this->config - ->expects($this->at(4)) + ->expects($this->once()) ->method('getAppValue') ->with('user_saml', 'type') ->willReturn('saml'); diff --git a/tests/unit/phpunit.xml b/tests/unit/phpunit.xml index 873b0b0..b474397 100644 --- a/tests/unit/phpunit.xml +++ b/tests/unit/phpunit.xml @@ -1,6 +1,5 @@ ../../../user_saml/lib - - - -