From e06f9f7fe3af9a0218b6530c4ebe3fef1c7925f6 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 22 Dec 2019 13:17:14 +0000 Subject: [PATCH] Removed setting override system due to confusing behaviour - Was only used to disable registration when LDAP was enabled. - Caused saved option not to show on settings page causing confusion. - Extended setting logic where used to take ldap into account instead of global override. - Added warning on setting page to show registration enable setting is not used while ldap is active. For #1541 --- app/Auth/Access/SocialAuthService.php | 2 +- .../Controllers/Auth/RegisterController.php | 2 +- app/Settings/SettingService.php | 22 ------------------- resources/lang/en/settings.php | 1 + resources/views/auth/login.blade.php | 2 +- resources/views/common/header.blade.php | 2 +- resources/views/settings/index.blade.php | 4 ++++ tests/Auth/{Saml2.php => Saml2Test.php} | 2 +- tests/Uploads/UsesImages.php | 2 +- 9 files changed, 11 insertions(+), 28 deletions(-) rename tests/Auth/{Saml2.php => Saml2Test.php} (99%) diff --git a/app/Auth/Access/SocialAuthService.php b/app/Auth/Access/SocialAuthService.php index 9c8d1a81f..bc5b7a4d5 100644 --- a/app/Auth/Access/SocialAuthService.php +++ b/app/Auth/Access/SocialAuthService.php @@ -137,7 +137,7 @@ class SocialAuthService // Otherwise let the user know this social account is not used by anyone. $message = trans('errors.social_account_not_used', ['socialAccount' => $titleCaseDriver]); - if (setting('registration-enabled')) { + if (setting('registration-enabled') && config('auth.method') !== 'ldap') { $message .= trans('errors.social_account_register_instructions', ['socialAccount' => $titleCaseDriver]); } diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index 000833029..8e4dd57c3 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -89,7 +89,7 @@ class RegisterController extends Controller */ protected function checkRegistrationAllowed() { - if (!setting('registration-enabled')) { + if (!setting('registration-enabled') || config('auth.method') === 'ldap') { throw new UserRegistrationException(trans('auth.registrations_disabled'), '/login'); } } diff --git a/app/Settings/SettingService.php b/app/Settings/SettingService.php index dede8fcc4..1c053b384 100644 --- a/app/Settings/SettingService.php +++ b/app/Settings/SettingService.php @@ -98,12 +98,6 @@ class SettingService */ protected function getValueFromStore($key, $default) { - // Check for an overriding value - $overrideValue = $this->getOverrideValue($key); - if ($overrideValue !== null) { - return $overrideValue; - } - // Check the cache $cacheKey = $this->cachePrefix . $key; $cacheVal = $this->cache->get($cacheKey, null); @@ -255,20 +249,4 @@ class SettingService { return $this->setting->where('setting_key', '=', $key)->first(); } - - - /** - * Returns an override value for a setting based on certain app conditions. - * Used where certain configuration options overrule others. - * Returns null if no override value is available. - * @param $key - * @return bool|null - */ - protected function getOverrideValue($key) - { - if ($key === 'registration-enabled' && config('auth.method') === 'ldap') { - return false; - } - return null; - } } diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php index 8255b4cbe..6be7cc9cb 100755 --- a/resources/lang/en/settings.php +++ b/resources/lang/en/settings.php @@ -56,6 +56,7 @@ return [ 'reg_enable_toggle' => 'Enable registration', 'reg_enable_desc' => 'When registration is enabled user will be able to sign themselves up as an application user. Upon registration they are given a single, default user role.', 'reg_default_role' => 'Default user role after registration', + 'reg_enable_ldap_warning' => 'The option above is not used while LDAP authentication is active. User accounts for non-existing members will be auto-created if authentication, against the LDAP system in use, is successful.', 'reg_email_confirmation' => 'Email Confirmation', 'reg_email_confirmation_toggle' => 'Require email confirmation', 'reg_confirm_email_desc' => 'If domain restriction is used then email confirmation will be required and this option will be ignored.', diff --git a/resources/views/auth/login.blade.php b/resources/views/auth/login.blade.php index 836150d69..098ce2100 100644 --- a/resources/views/auth/login.blade.php +++ b/resources/views/auth/login.blade.php @@ -55,7 +55,7 @@ @endif - @if(setting('registration-enabled', false)) + @if(setting('registration-enabled') && config('auth.method') !== 'ldap')
{{ trans('auth.dont_have_account') }} diff --git a/resources/views/common/header.blade.php b/resources/views/common/header.blade.php index 192996950..b06036031 100644 --- a/resources/views/common/header.blade.php +++ b/resources/views/common/header.blade.php @@ -42,7 +42,7 @@ @endif @if(!signedInUser()) - @if(setting('registration-enabled', false)) + @if(setting('registration-enabled') && config('auth.method') !== 'ldap') @icon('new-user') {{ trans('auth.sign_up') }} @endif @icon('login') {{ trans('auth.log_in') }} diff --git a/resources/views/settings/index.blade.php b/resources/views/settings/index.blade.php index 1bc454385..94605da6f 100644 --- a/resources/views/settings/index.blade.php +++ b/resources/views/settings/index.blade.php @@ -219,6 +219,10 @@ 'label' => trans('settings.reg_enable_toggle') ]) + @if(config('auth.method') === 'ldap') +
{{ trans('settings.reg_enable_ldap_warning') }}
+ @endif +