mirror of
https://github.com/BookStackApp/BookStack.git
synced 2025-04-29 22:29:57 +00:00
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
This commit is contained in:
parent
32e7f0a2e6
commit
e06f9f7fe3
9 changed files with 11 additions and 28 deletions
app
Auth/Access
Http/Controllers/Auth
Settings
resources
tests
|
@ -137,7 +137,7 @@ class SocialAuthService
|
||||||
|
|
||||||
// Otherwise let the user know this social account is not used by anyone.
|
// Otherwise let the user know this social account is not used by anyone.
|
||||||
$message = trans('errors.social_account_not_used', ['socialAccount' => $titleCaseDriver]);
|
$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]);
|
$message .= trans('errors.social_account_register_instructions', ['socialAccount' => $titleCaseDriver]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -89,7 +89,7 @@ class RegisterController extends Controller
|
||||||
*/
|
*/
|
||||||
protected function checkRegistrationAllowed()
|
protected function checkRegistrationAllowed()
|
||||||
{
|
{
|
||||||
if (!setting('registration-enabled')) {
|
if (!setting('registration-enabled') || config('auth.method') === 'ldap') {
|
||||||
throw new UserRegistrationException(trans('auth.registrations_disabled'), '/login');
|
throw new UserRegistrationException(trans('auth.registrations_disabled'), '/login');
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -98,12 +98,6 @@ class SettingService
|
||||||
*/
|
*/
|
||||||
protected function getValueFromStore($key, $default)
|
protected function getValueFromStore($key, $default)
|
||||||
{
|
{
|
||||||
// Check for an overriding value
|
|
||||||
$overrideValue = $this->getOverrideValue($key);
|
|
||||||
if ($overrideValue !== null) {
|
|
||||||
return $overrideValue;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Check the cache
|
// Check the cache
|
||||||
$cacheKey = $this->cachePrefix . $key;
|
$cacheKey = $this->cachePrefix . $key;
|
||||||
$cacheVal = $this->cache->get($cacheKey, null);
|
$cacheVal = $this->cache->get($cacheKey, null);
|
||||||
|
@ -255,20 +249,4 @@ class SettingService
|
||||||
{
|
{
|
||||||
return $this->setting->where('setting_key', '=', $key)->first();
|
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;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -56,6 +56,7 @@ return [
|
||||||
'reg_enable_toggle' => 'Enable registration',
|
'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_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_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' => 'Email Confirmation',
|
||||||
'reg_email_confirmation_toggle' => 'Require 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.',
|
'reg_confirm_email_desc' => 'If domain restriction is used then email confirmation will be required and this option will be ignored.',
|
||||||
|
|
|
@ -55,7 +55,7 @@
|
||||||
</div>
|
</div>
|
||||||
@endif
|
@endif
|
||||||
|
|
||||||
@if(setting('registration-enabled', false))
|
@if(setting('registration-enabled') && config('auth.method') !== 'ldap')
|
||||||
<div class="text-center pb-s">
|
<div class="text-center pb-s">
|
||||||
<hr class="my-l">
|
<hr class="my-l">
|
||||||
<a href="{{ url('/register') }}">{{ trans('auth.dont_have_account') }}</a>
|
<a href="{{ url('/register') }}">{{ trans('auth.dont_have_account') }}</a>
|
||||||
|
|
|
@ -42,7 +42,7 @@
|
||||||
@endif
|
@endif
|
||||||
|
|
||||||
@if(!signedInUser())
|
@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>
|
<a href="{{ url('/register') }}">@icon('new-user') {{ trans('auth.sign_up') }}</a>
|
||||||
@endif
|
@endif
|
||||||
<a href="{{ url('/login') }}">@icon('login') {{ trans('auth.log_in') }}</a>
|
<a href="{{ url('/login') }}">@icon('login') {{ trans('auth.log_in') }}</a>
|
||||||
|
|
|
@ -219,6 +219,10 @@
|
||||||
'label' => trans('settings.reg_enable_toggle')
|
'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>
|
<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>
|
<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)
|
@foreach(\BookStack\Auth\Role::all() as $role)
|
||||||
|
|
|
@ -3,7 +3,7 @@
|
||||||
use BookStack\Auth\Role;
|
use BookStack\Auth\Role;
|
||||||
use BookStack\Auth\User;
|
use BookStack\Auth\User;
|
||||||
|
|
||||||
class Saml2 extends TestCase
|
class Saml2Test extends TestCase
|
||||||
{
|
{
|
||||||
|
|
||||||
public function setUp(): void
|
public function setUp(): void
|
|
@ -15,7 +15,7 @@ trait UsesImages
|
||||||
if (is_null($fileName)) {
|
if (is_null($fileName)) {
|
||||||
$fileName = 'test-image.png';
|
$fileName = 'test-image.png';
|
||||||
}
|
}
|
||||||
|
|
||||||
return base_path('tests/test-data/' . $fileName);
|
return base_path('tests/test-data/' . $fileName);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue