Review and update of login auto initiation PR

For PR #3406

- Updated naming from 'redirect' to 'initate/initation'.
- Updated phpunit.xml and .env.example.complete files with the new
  option.
- Cleaned up controller logic a bit.
- Added content and design to the new initation view to not leave user
  on a blank view for a while.
- Added non-JS button to initiation view as fallback option for
  progression.
- Moved new test to it's own Test class and expanded with additional
  scenario tests for better functionality coverage.
This commit is contained in:
Dan Brown 2022-06-21 15:32:18 +01:00
parent d5ce6b680c
commit 8b211ed461
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
10 changed files with 153 additions and 42 deletions

View File

@ -143,6 +143,10 @@ STORAGE_URL=false
# Can be 'standard', 'ldap', 'saml2' or 'oidc' # Can be 'standard', 'ldap', 'saml2' or 'oidc'
AUTH_METHOD=standard AUTH_METHOD=standard
# Automatically initiate login via external auth system if it's the only auth method.
# Works with saml2 or oidc auth methods.
AUTH_AUTO_INITIATE=false
# Social authentication configuration # Social authentication configuration
# All disabled by default. # All disabled by default.
# Refer to https://www.bookstackapp.com/docs/admin/third-party-auth/ # Refer to https://www.bookstackapp.com/docs/admin/third-party-auth/

View File

@ -13,10 +13,9 @@ return [
// Options: standard, ldap, saml2, oidc // Options: standard, ldap, saml2, oidc
'method' => env('AUTH_METHOD', 'standard'), 'method' => env('AUTH_METHOD', 'standard'),
// Automatically redirect to external login provider if only one provider is being used // Automatically initiate login via external auth system if it's the sole auth method.
// instead of displaying a single-button login page and requiring users to click through // Works with saml2 or oidc auth methods.
// Supported methods: saml2, oidc 'auto_initiate' => env('AUTH_AUTO_INITIATE', false),
'auto_redirect' => env('AUTH_AUTO_REDIRECT', false),
// Authentication Defaults // Authentication Defaults
// This option controls the default authentication "guard" and password // This option controls the default authentication "guard" and password

View File

@ -32,10 +32,9 @@ class LoginController extends Controller
*/ */
protected $redirectTo = '/'; protected $redirectTo = '/';
protected $redirectPath = '/'; protected $redirectPath = '/';
protected $redirectAfterLogout = '/';
protected $socialAuthService; protected SocialAuthService $socialAuthService;
protected $loginService; protected LoginService $loginService;
/** /**
* Create a new controller instance. * Create a new controller instance.
@ -50,7 +49,6 @@ class LoginController extends Controller
$this->loginService = $loginService; $this->loginService = $loginService;
$this->redirectPath = url('/'); $this->redirectPath = url('/');
$this->redirectAfterLogout = url(config('auth.auto_redirect') ? '/login?logout=1' : '/');
} }
public function username() public function username()
@ -73,7 +71,7 @@ class LoginController extends Controller
{ {
$socialDrivers = $this->socialAuthService->getActiveDrivers(); $socialDrivers = $this->socialAuthService->getActiveDrivers();
$authMethod = config('auth.method'); $authMethod = config('auth.method');
$autoRedirect = config('auth.auto_redirect'); $preventInitiation = $request->get('prevent_auto_init') === 'true';
if ($request->has('email')) { if ($request->has('email')) {
session()->flashInput([ session()->flashInput([
@ -85,8 +83,8 @@ class LoginController extends Controller
// Store the previous location for redirect after login // Store the previous location for redirect after login
$this->updateIntendedFromPrevious(); $this->updateIntendedFromPrevious();
if ($autoRedirect && !($request->has('logout') && $request->get('logout') == '1') && count($socialDrivers) == 0 && in_array($authMethod, ['oidc', 'saml2'])) { if (!$preventInitiation && $this->shouldAutoInitiate()) {
return view('auth.login-redirect', [ return view('auth.login-initiate', [
'authMethod' => $authMethod, 'authMethod' => $authMethod,
]); ]);
} }
@ -259,6 +257,18 @@ class LoginController extends Controller
redirect()->setIntendedUrl($previous); redirect()->setIntendedUrl($previous);
} }
/**
* Check if login auto-initiate should be valid based upon authentication config.
*/
protected function shouldAutoInitiate(): bool
{
$socialDrivers = $this->socialAuthService->getActiveDrivers();
$authMethod = config('auth.method');
$autoRedirect = config('auth.auto_initiate');
return $autoRedirect && count($socialDrivers) === 0 && in_array($authMethod, ['oidc', 'saml2']);
}
/** /**
* Logout user and perform subsequent redirect. * Logout user and perform subsequent redirect.
* *
@ -270,6 +280,8 @@ class LoginController extends Controller
{ {
$this->traitLogout($request); $this->traitLogout($request);
return redirect($this->redirectAfterLogout); $redirectUri = $this->shouldAutoInitiate() ? '/login?prevent_auto_init=true' : '/';
return redirect($redirectUri);
} }
} }

View File

@ -29,6 +29,7 @@
<server name="MAIL_DRIVER" value="array"/> <server name="MAIL_DRIVER" value="array"/>
<server name="LOG_CHANNEL" value="single"/> <server name="LOG_CHANNEL" value="single"/>
<server name="AUTH_METHOD" value="standard"/> <server name="AUTH_METHOD" value="standard"/>
<server name="AUTH_AUTO_INITIATE" value="false"/>
<server name="DISABLE_EXTERNAL_SERVICES" value="true"/> <server name="DISABLE_EXTERNAL_SERVICES" value="true"/>
<server name="ALLOW_UNTRUSTED_SERVER_FETCHING" value="false"/> <server name="ALLOW_UNTRUSTED_SERVER_FETCHING" value="false"/>
<server name="AVATAR_URL" value=""/> <server name="AVATAR_URL" value=""/>

View File

@ -38,6 +38,11 @@ return [
'registration_email_domain_invalid' => 'That email domain does not have access to this application', 'registration_email_domain_invalid' => 'That email domain does not have access to this application',
'register_success' => 'Thanks for signing up! You are now registered and signed in.', 'register_success' => 'Thanks for signing up! You are now registered and signed in.',
// Login auto-initiation
'auto_init_starting' => 'Attempting Login',
'auto_init_starting_desc' => 'We\'re contacting your authentication system to start the login process. If there\'s no progress after 5 seconds you can try clicking the link below.',
'auto_init_start_link' => 'Proceed with authentication',
// Password Reset // Password Reset
'reset_password' => 'Reset Password', 'reset_password' => 'Reset Password',
'reset_password_send_instructions' => 'Enter your email below and you will be sent an email with a password reset link.', 'reset_password_send_instructions' => 'Enter your email below and you will be sent an email with a password reset link.',

View File

@ -99,6 +99,9 @@ button {
fill: var(--color-primary); fill: var(--color-primary);
} }
} }
.text-button.hover-underline:hover {
text-decoration: underline;
}
.button.block { .button.block {
width: 100%; width: 100%;

View File

@ -0,0 +1,37 @@
@extends('layouts.simple')
@section('content')
<div class="container very-small">
<div class="my-l">&nbsp;</div>
<div class="card content-wrap auto-height">
<h1 class="list-heading">{{ trans('auth.auto_init_starting') }}</h1>
<div style="display:none">
@include('auth.parts.login-form-' . $authMethod)
</div>
<div class="grid half left-focus">
<div>
<p class="text-small">{{ trans('auth.auto_init_starting_desc') }}</p>
<p>
<button type="submit" form="login-form" class="p-none text-button hover-underline">
{{ trans('auth.auto_init_start_link') }}
</button>
</p>
</div>
<div class="text-center">
@include('common.loading-icon')
</div>
</div>
<script nonce="{{ $cspNonce }}">
window.addEventListener('load', () => document.forms['login-form'].submit());
</script>
</div>
</div>
@stop

View File

@ -1,16 +0,0 @@
<!DOCTYPE html>
<html lang="{{ config('app.lang') }}"
dir="{{ config('app.rtl') ? 'rtl' : 'ltr' }}">
<head>
<meta charset="utf-8">
</head>
<body>
<div id="loginredirect-wrapper" style="display:none">
@include('auth.parts.login-form-' . $authMethod)
</div>
<script nonce="{{ $cspNonce }}">
window.onload = function(){document.forms['login-form'].submit()};
</script>
</body>
</html>

View File

@ -0,0 +1,80 @@
<?php
namespace Tests\Auth;
use Tests\TestCase;
class LoginAutoInitiateTest extends TestCase
{
protected function setUp(): void
{
parent::setUp();
config()->set([
'auth.auto_initiate' => true,
'services.google.client_id' => false,
'services.github.client_id' => false,
]);
}
public function test_with_oidc()
{
config()->set([
'auth.method' => 'oidc',
]);
$req = $this->get('/login');
$req->assertSeeText('Attempting Login');
$req->assertElementExists('form[action$="/oidc/login"][method=POST][id="login-form"] button');
$req->assertElementExists('button[form="login-form"]');
}
public function test_with_saml2()
{
config()->set([
'auth.method' => 'saml2',
]);
$req = $this->get('/login');
$req->assertSeeText('Attempting Login');
$req->assertElementExists('form[action$="/saml2/login"][method=POST][id="login-form"] button');
$req->assertElementExists('button[form="login-form"]');
}
public function test_it_does_not_run_if_social_provider_is_active()
{
config()->set([
'auth.method' => 'oidc',
'services.google.client_id' => 'abc123a',
'services.google.client_secret' => 'def456',
]);
$req = $this->get('/login');
$req->assertDontSeeText('Attempting Login');
$req->assertSee('Log In');
}
public function test_it_does_not_run_if_prevent_query_string_exists()
{
config()->set([
'auth.method' => 'oidc',
]);
$req = $this->get('/login?prevent_auto_init=true');
$req->assertDontSeeText('Attempting Login');
$req->assertSee('Log In');
}
public function test_logout_with_auto_init_leads_to_login_page_with_prevention_query()
{
config()->set([
'auth.method' => 'oidc',
]);
$this->actingAs($this->getEditor());
$req = $this->post('/logout');
$req->assertRedirect('/login?prevent_auto_init=true');
}
}

View File

@ -26,7 +26,6 @@ class OidcTest extends TestCase
config()->set([ config()->set([
'auth.method' => 'oidc', 'auth.method' => 'oidc',
'auth.auto_redirect' => false,
'auth.defaults.guard' => 'oidc', 'auth.defaults.guard' => 'oidc',
'oidc.name' => 'SingleSignOn-Testing', 'oidc.name' => 'SingleSignOn-Testing',
'oidc.display_name_claims' => ['name'], 'oidc.display_name_claims' => ['name'],
@ -112,19 +111,6 @@ class OidcTest extends TestCase
$this->assertPermissionError($resp); $this->assertPermissionError($resp);
} }
public function test_automatic_redirect_on_login()
{
config()->set([
'auth.auto_redirect' => true,
'services.google.client_id' => false,
'services.github.client_id' => false,
]);
$req = $this->get('/login');
$req->assertSeeText('SingleSignOn-Testing');
$req->assertElementExists('form[action$="/oidc/login"][method=POST] button');
$req->assertElementExists('div#loginredirect-wrapper');
}
public function test_login() public function test_login()
{ {
$req = $this->post('/oidc/login'); $req = $this->post('/oidc/login');