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

diff --git a/app/Auth/Access/SocialAuthService.php b/app/Auth/Access/SocialAuthService.php
index 9c8d1a81f43ebb8f30d66807a14349cab2882a39..bc5b7a4d5dea27497be83bf37acbd67b407a22bd 100644 (file)
--- 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 000833029a48cfe663dc43384b357496bf9f0f50..8e4dd57c3bd9ffbf4dcdc546e57f67787a98896b 100644 (file)
--- 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 dede8fcc418634db821a3d6948b84203e872fd28..1c053b3848ea779d480adea01834dce0059d629d 100644 (file)
--- 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 8255b4cbecc756619de3b71e315276f9e5df0a76..6be7cc9cb18a0f0ab30c373f84dd1b0d5a3a1004 100755 (executable)
--- 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 836150d69bdb6db3fa54a2129280a27365b59252..098ce2100d31b9a4193ec345525146103edec0fa 100644 (file)
--- a/resources/views/auth/login.blade.php
+++ b/resources/views/auth/login.blade.php
@@ -55,7 +55,7 @@
                 </div>
             @endif
 
-            @if(setting('registration-enabled', false))
+            @if(setting('registration-enabled') && config('auth.method') !== 'ldap')
                 <div class="text-center pb-s">
                     <hr class="my-l">
                     <a href="{{ url('/register') }}">{{ trans('auth.dont_have_account') }}</a>

diff --git a/resources/views/common/header.blade.php b/resources/views/common/header.blade.php
index 19299695042e98ded5b55735865c31f6fee9528b..b060360315eba00b62bae30193b0ba7f304ae06c 100644 (file)
--- 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')
                             <a href="{{ url('/register') }}">@icon('new-user') {{ trans('auth.sign_up') }}</a>
                         @endif
                         <a href="{{ url('/login') }}">@icon('login') {{ trans('auth.log_in') }}</a>

diff --git a/resources/views/settings/index.blade.php b/resources/views/settings/index.blade.php
index 1bc454385159758d4cb9d2dfbb7b6c352e74c84c..94605da6f11029c946c25032949a14ffea971ba2 100644 (file)
--- 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')
+                                <div class="text-warn text-small mb-l">{{ trans('settings.reg_enable_ldap_warning') }}</div>
+                            @endif
+
                             <label for="setting-registration-role">{{ trans('settings.reg_default_role') }}</label>
                             <select id="setting-registration-role" name="setting-registration-role" @if($errors->has('setting-registration-role')) class="neg" @endif>
                                 @foreach(\BookStack\Auth\Role::all() as $role)

diff --git a/tests/Auth/Saml2.php b/tests/Auth/Saml2Test.php
similarity index 99%
rename from tests/Auth/Saml2.php
rename to tests/Auth/Saml2Test.php
index ef1ca8d133da399795212dd06c1088c5e593903b..45b6efa07e475401beb373e5ac674fd2ee83163f 100644 (file)
--- a/tests/Auth/Saml2.php
+++ b/tests/Auth/Saml2Test.php
@@ -3,7 +3,7 @@
 use BookStack\Auth\Role;
 use BookStack\Auth\User;
 
-class Saml2 extends TestCase
+class Saml2Test extends TestCase
 {
 
     public function setUp(): void

diff --git a/tests/Uploads/UsesImages.php b/tests/Uploads/UsesImages.php
index 9ce559acdb167e595482632b92961bf676dfd39f..b24b483d945ec1e7f510ac621bdd6244fdec49a4 100644 (file)
--- a/tests/Uploads/UsesImages.php
+++ b/tests/Uploads/UsesImages.php
@@ -15,7 +15,7 @@ trait UsesImages
         if (is_null($fileName)) {
             $fileName = 'test-image.png';
         }
-        
+
         return base_path('tests/test-data/' . $fileName);
     }