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.

diff --git a/.env.example.complete b/.env.example.complete
index 0c7f8f6a5f5167647a0358789358e354f6f38296..c40ab1380ffd82f63294bd7493e47436d11acb20 100644 (file)
--- a/.env.example.complete
+++ b/.env.example.complete
@@ -143,6 +143,10 @@ STORAGE_URL=false
 # Can be 'standard', 'ldap', 'saml2' or 'oidc'
 AUTH_METHOD=standard
 
 # Can be 'standard', 'ldap', 'saml2' or 'oidc'
 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
 # All disabled by default.
 # Refer to https://www.bookstackapp.com/docs/admin/third-party-auth/
 # Social authentication configuration
 # All disabled by default.
 # Refer to https://www.bookstackapp.com/docs/admin/third-party-auth/

diff --git a/app/Config/auth.php b/app/Config/auth.php
index ec4389a6ce3ff9dbedfec4a6063e6136f8128a9f..37190156a96f13a70e2a3fe2c044797a2ed6f3fd 100644 (file)
--- a/app/Config/auth.php
+++ b/app/Config/auth.php
@@ -13,10 +13,9 @@ return [
     // Options: standard, ldap, saml2, oidc
     'method' => env('AUTH_METHOD', 'standard'),
 
     // Options: standard, ldap, saml2, oidc
     'method' => env('AUTH_METHOD', 'standard'),
 

-    // Automatically redirect to external login provider if only one provider is being used
-    // instead of displaying a single-button login page and requiring users to click through
-    // Supported methods: saml2, oidc
-    'auto_redirect' => env('AUTH_AUTO_REDIRECT', false),
+    // Automatically initiate login via external auth system if it's the sole auth method.
+    // Works with saml2 or oidc auth methods.
+    'auto_initiate' => env('AUTH_AUTO_INITIATE', false),

 
     // Authentication Defaults
     // This option controls the default authentication "guard" and password
 
     // Authentication Defaults
     // This option controls the default authentication "guard" and password

diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php
index 695bfa28d47f14948f7766b517f96c37666556fe..f1a1a8bd6930bbc2cbf5f6b1e0f268d9dfefcaa5 100644 (file)
--- a/app/Http/Controllers/Auth/LoginController.php
+++ b/app/Http/Controllers/Auth/LoginController.php
@@ -32,10 +32,9 @@ class LoginController extends Controller
      */
     protected $redirectTo = '/';
     protected $redirectPath = '/';
      */
     protected $redirectTo = '/';
     protected $redirectPath = '/';

-    protected $redirectAfterLogout = '/';

 
 

-    protected $socialAuthService;
-    protected $loginService;
+    protected SocialAuthService $socialAuthService;
+    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->redirectPath = url('/');
         $this->loginService = $loginService;
 
         $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();
         $authMethod = config('auth.method');
     {
         $socialDrivers = $this->socialAuthService->getActiveDrivers();
         $authMethod = config('auth.method');

-        $autoRedirect = config('auth.auto_redirect');
+        $preventInitiation = $request->get('prevent_auto_init') === 'true';

 
         if ($request->has('email')) {
             session()->flashInput([
 
         if ($request->has('email')) {
             session()->flashInput([


@@ -85,8 +83,8 @@ class LoginController extends Controller
         // Store the previous location for redirect after login
         $this->updateIntendedFromPrevious();
 
         // Store the previous location for redirect after login
         $this->updateIntendedFromPrevious();
 

-        if ($autoRedirect && !($request->has('logout') && $request->get('logout') == '1') && count($socialDrivers) == 0 && in_array($authMethod, ['oidc', 'saml2'])) {
-            return view('auth.login-redirect', [
+        if (!$preventInitiation && $this->shouldAutoInitiate()) {
+            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);

     }
 }
     }
 }

diff --git a/phpunit.xml b/phpunit.xml
index 90320ff41971661f47b2cc4836f5ffa4ada955fb..56a510b101d14fb28a140128ef48ef0a99418732 100644 (file)
--- a/phpunit.xml
+++ b/phpunit.xml
@@ -29,6 +29,7 @@
     <server name="MAIL_DRIVER" value="array"/>
     <server name="LOG_CHANNEL" value="single"/>
     <server name="AUTH_METHOD" value="standard"/>
     <server name="MAIL_DRIVER" value="array"/>
     <server name="LOG_CHANNEL" value="single"/>
     <server name="AUTH_METHOD" value="standard"/>

+    <server name="AUTH_AUTO_INITIATE" value="false"/>

     <server name="DISABLE_EXTERNAL_SERVICES" value="true"/>
     <server name="ALLOW_UNTRUSTED_SERVER_FETCHING" value="false"/>
     <server name="AVATAR_URL" value=""/>
     <server name="DISABLE_EXTERNAL_SERVICES" value="true"/>
     <server name="ALLOW_UNTRUSTED_SERVER_FETCHING" value="false"/>
     <server name="AVATAR_URL" value=""/>

diff --git a/resources/lang/en/auth.php b/resources/lang/en/auth.php
index ad0d516bb1d5e2f6a9d7006d422cf1ad9b910141..c670106f9b64d79f28c666e8122e077e6b0f355d 100644 (file)
--- a/resources/lang/en/auth.php
+++ b/resources/lang/en/auth.php
@@ -38,6 +38,11 @@ return [
     '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.',
 
     '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.',
 

+    // 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
     'reset_password' => 'Reset Password',
     'reset_password_send_instructions' => 'Enter your email below and you will be sent an email with a password reset link.',
     // Password Reset
     'reset_password' => 'Reset Password',
     'reset_password_send_instructions' => 'Enter your email below and you will be sent an email with a password reset link.',

diff --git a/resources/sass/_buttons.scss b/resources/sass/_buttons.scss
index 850443d9a7c02a500832990eaa0b9d4e7a7ca8d5..714dfc42c5905212c752a010489435774fecc79b 100644 (file)
--- a/resources/sass/_buttons.scss
+++ b/resources/sass/_buttons.scss
@@ -99,6 +99,9 @@ button {
     fill: var(--color-primary);
   }
 }
     fill: var(--color-primary);
   }
 }

+.text-button.hover-underline:hover {
+  text-decoration: underline;
+}

 
 .button.block {
   width: 100%;
 
 .button.block {
   width: 100%;

diff --git a/resources/views/auth/login-initiate.blade.php b/resources/views/auth/login-initiate.blade.php
new file mode 100644 (file)
index 0000000..520175f
--- /dev/null
+++ b/resources/views/auth/login-initiate.blade.php
@@ -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

diff --git a/resources/views/auth/login-redirect.blade.php b/resources/views/auth/login-redirect.blade.php
deleted file mode 100644 (file)
index 08501c6..0000000
--- a/resources/views/auth/login-redirect.blade.php
+++ /dev/null
@@ -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>

diff --git a/tests/Auth/LoginAutoInitiateTest.php b/tests/Auth/LoginAutoInitiateTest.php
new file mode 100644 (file)
index 0000000..a2941e2
--- /dev/null
+++ b/tests/Auth/LoginAutoInitiateTest.php
@@ -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');
+    }
+
+}
\ No newline at end of file

diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php
index 34fd70115e345a839b0ea2441d18494d72a62555..9aebb4d04a6694c812c6d2a57f74f1f3660257bf 100644 (file)
--- a/tests/Auth/OidcTest.php
+++ b/tests/Auth/OidcTest.php
@@ -26,7 +26,6 @@ class OidcTest extends TestCase
 
         config()->set([
             'auth.method'                 => 'oidc',
 
         config()->set([
             'auth.method'                 => 'oidc',

-            'auth.auto_redirect'          => false,

             'auth.defaults.guard'         => 'oidc',
             'oidc.name'                   => 'SingleSignOn-Testing',
             'oidc.display_name_claims'    => ['name'],
             'auth.defaults.guard'         => 'oidc',
             'oidc.name'                   => 'SingleSignOn-Testing',
             '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()
     {
         $req = $this->post('/oidc/login');
     public function test_login()
     {
         $req = $this->post('/oidc/login');