Quick test of email confirmation routes and fix of tests

diff --git a/app/Auth/Access/EmailConfirmationService.php b/app/Auth/Access/EmailConfirmationService.php
index 3425c1e720de0416f066018f436e15c9c06eae54..75c03b31857b6f37e945b0c479b9957c246075f0 100644 (file)
--- a/app/Auth/Access/EmailConfirmationService.php
+++ b/app/Auth/Access/EmailConfirmationService.php
@@ -14,9 +14,6 @@ class EmailConfirmationService extends UserTokenService
     /**
      * Create new confirmation for a user,
      * Also removes any existing old ones.
-     *
-     * @param User $user
-     *
      * @throws ConfirmationEmailException
      */
     public function sendConfirmation(User $user)
@@ -33,8 +30,6 @@ class EmailConfirmationService extends UserTokenService
 
     /**
      * Check if confirmation is required in this instance.
-     *
-     * @return bool
      */
     public function confirmationRequired(): bool
     {

diff --git a/app/Auth/Access/LoginService.php b/app/Auth/Access/LoginService.php
index f6ea7517f75c65bd3a66cfe499ec3c02a4344594..998259deeb6a13593359e9b2e55da534553c35b6 100644 (file)
--- a/app/Auth/Access/LoginService.php
+++ b/app/Auth/Access/LoginService.php
@@ -10,7 +10,6 @@ use BookStack\Facades\Activity;
 use BookStack\Facades\Theme;
 use BookStack\Theming\ThemeEvents;
 use Exception;
-use phpDocumentor\Reflection\DocBlock\Tags\Method;
 
 class LoginService
 {
@@ -18,10 +17,12 @@ class LoginService
     protected const LAST_LOGIN_ATTEMPTED_SESSION_KEY = 'auth-login-last-attempted';
 
     protected $mfaSession;
+    protected $emailConfirmationService;
 
-    public function __construct(MfaSession $mfaSession)
+    public function __construct(MfaSession $mfaSession, EmailConfirmationService $emailConfirmationService)
     {
         $this->mfaSession = $mfaSession;
+        $this->emailConfirmationService = $emailConfirmationService;
     }
 
     /**
@@ -149,8 +150,7 @@ class LoginService
      */
     public function awaitingEmailConfirmation(User $user): bool
     {
-        $requireConfirmation = (setting('registration-confirmation') || setting('registration-restrict'));
-        return $requireConfirmation && !$user->email_confirmed;
+        return $this->emailConfirmationService->confirmationRequired() && !$user->email_confirmed;
     }
 
     /**

diff --git a/app/Auth/Access/RegistrationService.php b/app/Auth/Access/RegistrationService.php
index 16e3edbb44e8dc799c4bd3939a358d5d3b9b28df..4c8b53c0d786e5f9c938773dbce9fb01e38fdbbb 100644 (file)
--- a/app/Auth/Access/RegistrationService.php
+++ b/app/Auth/Access/RegistrationService.php
@@ -88,7 +88,6 @@ class RegistrationService
                 session()->flash('sent-email-confirmation', true);
             } catch (Exception $e) {
                 $message = trans('auth.email_confirm_send_error');
-
                 throw new UserRegistrationException($message, '/register/confirm');
             }
         }

diff --git a/app/Auth/Access/Saml2Service.php b/app/Auth/Access/Saml2Service.php
index d0b6a655ba2f31a140b6fc4f1c689019e6075228..3f0f40ccc2a52dec4809ba0a0a2a2d1923b3302c 100644 (file)
--- a/app/Auth/Access/Saml2Service.php
+++ b/app/Auth/Access/Saml2Service.php
@@ -6,6 +6,7 @@ use BookStack\Actions\ActivityType;
 use BookStack\Auth\User;
 use BookStack\Exceptions\JsonDebugException;
 use BookStack\Exceptions\SamlException;
+use BookStack\Exceptions\StoppedAuthenticationException;
 use BookStack\Exceptions\UserRegistrationException;
 use BookStack\Facades\Activity;
 use BookStack\Facades\Theme;
@@ -357,6 +358,7 @@ class Saml2Service extends ExternalAuthService
      * @throws SamlException
      * @throws JsonDebugException
      * @throws UserRegistrationException
+     * @throws StoppedAuthenticationException
      */
     public function processLoginCallback(string $samlID, array $samlAttributes): User
     {

diff --git a/app/Exceptions/StoppedAuthenticationException.php b/app/Exceptions/StoppedAuthenticationException.php
index de8898df72bec7640810baa4e58c03cc1fa57af1..7a978ffc61428ad6015dbc7d8170b8a2eac34b89 100644 (file)
--- a/app/Exceptions/StoppedAuthenticationException.php
+++ b/app/Exceptions/StoppedAuthenticationException.php
@@ -5,6 +5,7 @@ namespace BookStack\Exceptions;
 use BookStack\Auth\Access\LoginService;
 use BookStack\Auth\User;
 use Illuminate\Contracts\Support\Responsable;
+use Illuminate\Http\Request;
 
 class StoppedAuthenticationException extends \Exception implements Responsable
 {
@@ -30,11 +31,35 @@ class StoppedAuthenticationException extends \Exception implements Responsable
         $redirect = '/login';
 
         if ($this->loginService->awaitingEmailConfirmation($this->user)) {
-            $redirect = '/register/confirm/awaiting';
-        } else if  ($this->loginService->needsMfaVerification($this->user)) {
+            return $this->awaitingEmailConfirmationResponse($request);
+        }
+
+        if ($this->loginService->needsMfaVerification($this->user)) {
             $redirect = '/mfa/verify';
         }
 
         return redirect($redirect);
     }
+
+    /**
+     * Provide an error response for when the current user's email is not confirmed
+     * in a system which requires it.
+     */
+    protected function awaitingEmailConfirmationResponse(Request $request)
+    {
+        if ($request->wantsJson()) {
+            return response()->json([
+                'error' => [
+                    'code' => 401,
+                    'message' => trans('errors.email_confirmation_awaiting'),
+                ],
+            ], 401);
+        }
+
+        if (session()->get('sent-email-confirmation') === true) {
+            return redirect('/register/confirm');
+        }
+
+        return redirect('/register/confirm/awaiting');
+    }
 }
\ No newline at end of file

diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php
index 4008439bc7e69a7816b6dd9f504dc2f49e658d26..e372b38ef3ec30eda13c463b19317f16046a0771 100644 (file)
--- a/app/Http/Controllers/Auth/RegisterController.php
+++ b/app/Http/Controllers/Auth/RegisterController.php
@@ -6,6 +6,7 @@ use BookStack\Auth\Access\LoginService;
 use BookStack\Auth\Access\RegistrationService;
 use BookStack\Auth\Access\SocialAuthService;
 use BookStack\Auth\User;
+use BookStack\Exceptions\StoppedAuthenticationException;
 use BookStack\Exceptions\UserRegistrationException;
 use BookStack\Http\Controllers\Controller;
 use Illuminate\Foundation\Auth\RegistersUsers;
@@ -93,6 +94,7 @@ class RegisterController extends Controller
      * Handle a registration request for the application.
      *
      * @throws UserRegistrationException
+     * @throws StoppedAuthenticationException
      */
     public function postRegister(Request $request)
     {

diff --git a/app/Http/Middleware/Authenticate.php b/app/Http/Middleware/Authenticate.php
index b9b5545f2a541a302e6a60b81985009f4dc62723..dede8f26056528445d01792d119243025c0ebf0b 100644 (file)
--- a/app/Http/Middleware/Authenticate.php
+++ b/app/Http/Middleware/Authenticate.php
@@ -21,26 +21,4 @@ class Authenticate
 
         return $next($request);
     }
-
-    /**
-     * Provide an error response for when the current user's email is not confirmed
-     * in a system which requires it.
-     */
-    protected function emailConfirmationErrorResponse(Request $request)
-    {
-        if ($request->wantsJson()) {
-            return response()->json([
-                'error' => [
-                    'code'    => 401,
-                    'message' => trans('errors.email_confirmation_awaiting'),
-                ],
-            ], 401);
-        }
-
-        if (session()->get('sent-email-confirmation') === true) {
-            return redirect('/register/confirm');
-        }
-
-        return redirect('/register/confirm/awaiting');
-    }
 }

diff --git a/tests/Auth/AuthTest.php b/tests/Auth/AuthTest.php
index febf583998d460e10440aa0ec36a2852eaa01641..d57a3253f0b24e3e5f1029fc3746ee646e671008 100644 (file)
--- a/tests/Auth/AuthTest.php
+++ b/tests/Auth/AuthTest.php
@@ -131,7 +131,8 @@ class AuthTest extends BrowserKitTest
             ->seePageIs('/register/confirm/awaiting')
             ->see('Resend')
             ->visit('/books')
-            ->seePageIs('/register/confirm/awaiting')
+            ->seePageIs('/login')
+            ->visit('/register/confirm/awaiting')
             ->press('Resend Confirmation Email');
 
         // Get confirmation and confirm notification matches
@@ -172,10 +173,7 @@ class AuthTest extends BrowserKitTest
             ->seePageIs('/register/confirm')
             ->seeInDatabase('users', ['name' => $user->name, 'email' => $user->email, 'email_confirmed' => false]);
 
-        $this->visit('/')
-            ->seePageIs('/register/confirm/awaiting');
-
-        auth()->logout();
+        $this->assertNull(auth()->user());
 
         $this->visit('/')->seePageIs('/login')
             ->type($user->email, '#email')
@@ -209,10 +207,8 @@ class AuthTest extends BrowserKitTest
             ->seePageIs('/register/confirm')
             ->seeInDatabase('users', ['name' => $user->name, 'email' => $user->email, 'email_confirmed' => false]);
 
-        $this->visit('/')
-            ->seePageIs('/register/confirm/awaiting');
+        $this->assertNull(auth()->user());
 
-        auth()->logout();
         $this->visit('/')->seePageIs('/login')
             ->type($user->email, '#email')
             ->type($user->password, '#password')

diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php
index 9d491fd0f75b641f5b6978eadb266a7881e38372..3bd6988e4064abd7b50102e5a23fbe156323ded4 100644 (file)
--- a/tests/Auth/LdapTest.php
+++ b/tests/Auth/LdapTest.php
@@ -27,18 +27,18 @@ class LdapTest extends TestCase
             define('LDAP_OPT_REFERRALS', 1);
         }
         config()->set([
-            'auth.method'                          => 'ldap',
-            'auth.defaults.guard'                  => 'ldap',
-            'services.ldap.base_dn'                => 'dc=ldap,dc=local',
-            'services.ldap.email_attribute'        => 'mail',
+            'auth.method' => 'ldap',
+            'auth.defaults.guard' => 'ldap',
+            'services.ldap.base_dn' => 'dc=ldap,dc=local',
+            'services.ldap.email_attribute' => 'mail',
             'services.ldap.display_name_attribute' => 'cn',
-            'services.ldap.id_attribute'           => 'uid',
-            'services.ldap.user_to_groups'         => false,
-            'services.ldap.version'                => '3',
-            'services.ldap.user_filter'            => '(&(uid=${user}))',
-            'services.ldap.follow_referrals'       => false,
-            'services.ldap.tls_insecure'           => false,
-            'services.ldap.thumbnail_attribute'    => null,
+            'services.ldap.id_attribute' => 'uid',
+            'services.ldap.user_to_groups' => false,
+            'services.ldap.version' => '3',
+            'services.ldap.user_filter' => '(&(uid=${user}))',
+            'services.ldap.follow_referrals' => false,
+            'services.ldap.tls_insecure' => false,
+            'services.ldap.thumbnail_attribute' => null,
         ]);
         $this->mockLdap = \Mockery::mock(Ldap::class);
         $this->app[Ldap::class] = $this->mockLdap;
@@ -70,9 +70,9 @@ class LdapTest extends TestCase
     protected function mockUserLogin(?string $email = null): TestResponse
     {
         return $this->post('/login', [
-            'username' => $this->mockUser->name,
-            'password' => $this->mockUser->password,
-        ] + ($email ? ['email' => $email] : []));
+                'username' => $this->mockUser->name,
+                'password' => $this->mockUser->password,
+            ] + ($email ? ['email' => $email] : []));
     }
 
     /**
@@ -95,8 +95,8 @@ class LdapTest extends TestCase
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
-                'cn'  => [$this->mockUser->name],
-                'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')],
             ]]);
 
         $resp = $this->mockUserLogin();
@@ -109,8 +109,8 @@ class LdapTest extends TestCase
         $resp->assertElementExists('#home-default');
         $resp->assertSee($this->mockUser->name);
         $this->assertDatabaseHas('users', [
-            'email'            => $this->mockUser->email,
-            'email_confirmed'  => false,
+            'email' => $this->mockUser->email,
+            'email_confirmed' => false,
             'external_auth_id' => $this->mockUser->name,
         ]);
     }
@@ -126,8 +126,8 @@ class LdapTest extends TestCase
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
-                'cn'  => [$this->mockUser->name],
-                'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')],
             ]]);
 
         $resp = $this->mockUserLogin();
@@ -150,8 +150,8 @@ class LdapTest extends TestCase
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
-                'cn'   => [$this->mockUser->name],
-                'dn'   => $ldapDn,
+                'cn' => [$this->mockUser->name],
+                'dn' => $ldapDn,
                 'mail' => [$this->mockUser->email],
             ]]);
 
@@ -170,10 +170,10 @@ class LdapTest extends TestCase
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
-                'cn'           => [$this->mockUser->name],
-                'dn'           => $ldapDn,
+                'cn' => [$this->mockUser->name],
+                'dn' => $ldapDn,
                 'my_custom_id' => ['cooluser456'],
-                'mail'         => [$this->mockUser->email],
+                'mail' => [$this->mockUser->email],
             ]]);
 
         $resp = $this->mockUserLogin();
@@ -189,8 +189,8 @@ class LdapTest extends TestCase
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
-                'cn'  => [$this->mockUser->name],
-                'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')],
             ]]);
         $this->mockLdap->shouldReceive('bind')->times(2)->andReturn(true, false);
 
@@ -219,14 +219,14 @@ class LdapTest extends TestCase
         $userForm->assertDontSee('Password');
 
         $save = $this->post('/settings/users/create', [
-            'name'  => $this->mockUser->name,
+            'name' => $this->mockUser->name,
             'email' => $this->mockUser->email,
         ]);
         $save->assertSessionHasErrors(['external_auth_id' => 'The external auth id field is required.']);
 
         $save = $this->post('/settings/users/create', [
-            'name'             => $this->mockUser->name,
-            'email'            => $this->mockUser->email,
+            'name' => $this->mockUser->name,
+            'email' => $this->mockUser->email,
             'external_auth_id' => $this->mockUser->name,
         ]);
         $save->assertRedirect('/settings/users');
@@ -241,8 +241,8 @@ class LdapTest extends TestCase
         $editPage->assertDontSee('Password');
 
         $update = $this->put("/settings/users/{$editUser->id}", [
-            'name'             => $editUser->name,
-            'email'            => $editUser->email,
+            'name' => $editUser->name,
+            'email' => $editUser->email,
             'external_auth_id' => 'test_auth_id',
         ]);
         $update->assertRedirect('/settings/users');
@@ -271,8 +271,8 @@ class LdapTest extends TestCase
         $this->mockUser->attachRole($existingRole);
 
         app('config')->set([
-            'services.ldap.user_to_groups'     => true,
-            'services.ldap.group_attribute'    => 'memberOf',
+            'services.ldap.user_to_groups' => true,
+            'services.ldap.group_attribute' => 'memberOf',
             'services.ldap.remove_from_groups' => false,
         ]);
 
@@ -280,14 +280,14 @@ class LdapTest extends TestCase
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
-                'uid'      => [$this->mockUser->name],
-                'cn'       => [$this->mockUser->name],
-                'dn'       => ['dc=test' . config('services.ldap.base_dn')],
-                'mail'     => [$this->mockUser->email],
+                'uid' => [$this->mockUser->name],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')],
+                'mail' => [$this->mockUser->email],
                 'memberof' => [
                     'count' => 2,
-                    0       => 'cn=ldaptester,ou=groups,dc=example,dc=com',
-                    1       => 'cn=ldaptester-second,ou=groups,dc=example,dc=com',
+                    0 => 'cn=ldaptester,ou=groups,dc=example,dc=com',
+                    1 => 'cn=ldaptester-second,ou=groups,dc=example,dc=com',
                 ],
             ]]);
 
@@ -316,8 +316,8 @@ class LdapTest extends TestCase
         $this->mockUser->attachRole($existingRole);
 
         app('config')->set([
-            'services.ldap.user_to_groups'     => true,
-            'services.ldap.group_attribute'    => 'memberOf',
+            'services.ldap.user_to_groups' => true,
+            'services.ldap.group_attribute' => 'memberOf',
             'services.ldap.remove_from_groups' => true,
         ]);
 
@@ -325,13 +325,13 @@ class LdapTest extends TestCase
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(3)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
-                'uid'      => [$this->mockUser->name],
-                'cn'       => [$this->mockUser->name],
-                'dn'       => ['dc=test' . config('services.ldap.base_dn')],
-                'mail'     => [$this->mockUser->email],
+                'uid' => [$this->mockUser->name],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')],
+                'mail' => [$this->mockUser->email],
                 'memberof' => [
                     'count' => 1,
-                    0       => 'cn=ldaptester,ou=groups,dc=example,dc=com',
+                    0 => 'cn=ldaptester,ou=groups,dc=example,dc=com',
                 ],
             ]]);
 
@@ -361,8 +361,8 @@ class LdapTest extends TestCase
         $roleToNotReceive = factory(Role::class)->create(['display_name' => 'ex-auth-a', 'external_auth_id' => 'test-second-param']);
 
         app('config')->set([
-            'services.ldap.user_to_groups'     => true,
-            'services.ldap.group_attribute'    => 'memberOf',
+            'services.ldap.user_to_groups' => true,
+            'services.ldap.group_attribute' => 'memberOf',
             'services.ldap.remove_from_groups' => true,
         ]);
 
@@ -370,13 +370,13 @@ class LdapTest extends TestCase
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(3)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
-                'uid'      => [$this->mockUser->name],
-                'cn'       => [$this->mockUser->name],
-                'dn'       => ['dc=test' . config('services.ldap.base_dn')],
-                'mail'     => [$this->mockUser->email],
+                'uid' => [$this->mockUser->name],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')],
+                'mail' => [$this->mockUser->email],
                 'memberof' => [
                     'count' => 1,
-                    0       => 'cn=ex-auth-a,ou=groups,dc=example,dc=com',
+                    0 => 'cn=ex-auth-a,ou=groups,dc=example,dc=com',
                 ],
             ]]);
 
@@ -402,8 +402,8 @@ class LdapTest extends TestCase
         setting()->put('registration-role', $roleToReceive->id);
 
         app('config')->set([
-            'services.ldap.user_to_groups'     => true,
-            'services.ldap.group_attribute'    => 'memberOf',
+            'services.ldap.user_to_groups' => true,
+            'services.ldap.group_attribute' => 'memberOf',
             'services.ldap.remove_from_groups' => true,
         ]);
 
@@ -411,14 +411,14 @@ class LdapTest extends TestCase
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
-                'uid'      => [$this->mockUser->name],
-                'cn'       => [$this->mockUser->name],
-                'dn'       => ['dc=test' . config('services.ldap.base_dn')],
-                'mail'     => [$this->mockUser->email],
+                'uid' => [$this->mockUser->name],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')],
+                'mail' => [$this->mockUser->email],
                 'memberof' => [
                     'count' => 2,
-                    0       => 'cn=ldaptester,ou=groups,dc=example,dc=com',
-                    1       => 'cn=ldaptester-second,ou=groups,dc=example,dc=com',
+                    0 => 'cn=ldaptester,ou=groups,dc=example,dc=com',
+                    1 => 'cn=ldaptester-second,ou=groups,dc=example,dc=com',
                 ],
             ]]);
 
@@ -445,9 +445,9 @@ class LdapTest extends TestCase
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
-                'uid'         => [$this->mockUser->name],
-                'cn'          => [$this->mockUser->name],
-                'dn'          => ['dc=test' . config('services.ldap.base_dn')],
+                'uid' => [$this->mockUser->name],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')],
                 'displayname' => 'displayNameAttribute',
             ]]);
 
@@ -471,8 +471,8 @@ class LdapTest extends TestCase
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
-                'cn'  => [$this->mockUser->name],
-                'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')],
             ]]);
 
         $this->mockUserLogin()->assertRedirect('/login');
@@ -482,10 +482,10 @@ class LdapTest extends TestCase
         $resp->assertRedirect('/');
         $this->get('/')->assertSee($this->mockUser->name);
         $this->assertDatabaseHas('users', [
-            'email'            => $this->mockUser->email,
-            'email_confirmed'  => false,
+            'email' => $this->mockUser->email,
+            'email_confirmed' => false,
             'external_auth_id' => $this->mockUser->name,
-            'name'             => $this->mockUser->name,
+            'name' => $this->mockUser->name,
         ]);
     }
 
@@ -499,8 +499,8 @@ class LdapTest extends TestCase
         $this->commonLdapMocks(0, 1, 1, 2, 1);
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)->andReturn(['count' => 1, 0 => [
             'uid' => [$this->mockUser->name],
-            'cn'  => [$this->mockUser->name],
-            'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+            'cn' => [$this->mockUser->name],
+            'dn' => ['dc=test' . config('services.ldap.base_dn')],
         ]]);
 
         $this->mockLdap->shouldReceive('connect')->once()
@@ -566,8 +566,8 @@ class LdapTest extends TestCase
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
-                'cn'  => [$this->mockUser->name],
-                'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')],
             ]]);
 
         $resp = $this->post('/login', [
@@ -575,7 +575,7 @@ class LdapTest extends TestCase
             'password' => $this->mockUser->password,
         ]);
         $resp->assertJsonStructure([
-            'details_from_ldap'        => [],
+            'details_from_ldap' => [],
             'details_bookstack_parsed' => [],
         ]);
     }
@@ -605,8 +605,8 @@ class LdapTest extends TestCase
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), ['cn', 'dn', 'uid', 'mail', 'cn'])
             ->andReturn(['count' => 1, 0 => [
                 'uid' => [hex2bin('FFF8F7')],
-                'cn'  => [$this->mockUser->name],
-                'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')],
             ]]);
 
         $details = $ldapService->getUserDetails('test');
@@ -619,14 +619,14 @@ class LdapTest extends TestCase
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
-                'uid'  => [$this->mockUser->name],
-                'cn'   => [$this->mockUser->name],
-                'dn'   => ['dc=test' . config('services.ldap.base_dn')],
+                'uid' => [$this->mockUser->name],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')],
                 'mail' => 'tester@example.com',
             ]], ['count' => 1, 0 => [
-                'uid'  => ['Barry'],
-                'cn'   => ['Scott'],
-                'dn'   => ['dc=bscott' . config('services.ldap.base_dn')],
+                'uid' => ['Barry'],
+                'cn' => ['Scott'],
+                'dn' => ['dc=bscott' . config('services.ldap.base_dn')],
                 'mail' => 'tester@example.com',
             ]]);
 
@@ -646,28 +646,29 @@ class LdapTest extends TestCase
         setting()->put('registration-confirmation', 'true');
 
         app('config')->set([
-            'services.ldap.user_to_groups'     => true,
-            'services.ldap.group_attribute'    => 'memberOf',
+            'services.ldap.user_to_groups' => true,
+            'services.ldap.group_attribute' => 'memberOf',
             'services.ldap.remove_from_groups' => true,
         ]);
 
-        $this->commonLdapMocks(1, 1, 3, 4, 3, 2);
+        $this->commonLdapMocks(1, 1, 6, 8, 6, 4);
         $this->mockLdap->shouldReceive('searchAndGetEntries')
-            ->times(3)
+            ->times(6)
             ->andReturn(['count' => 1, 0 => [
-                'uid'      => [$user->name],
-                'cn'       => [$user->name],
-                'dn'       => ['dc=test' . config('services.ldap.base_dn')],
-                'mail'     => [$user->email],
+                'uid' => [$user->name],
+                'cn' => [$user->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')],
+                'mail' => [$user->email],
                 'memberof' => [
                     'count' => 1,
-                    0       => 'cn=ldaptester,ou=groups,dc=example,dc=com',
+                    0 => 'cn=ldaptester,ou=groups,dc=example,dc=com',
                 ],
             ]]);
 
-        $this->followingRedirects()->mockUserLogin()->assertSee('Thanks for registering!');
+        $login = $this->followingRedirects()->mockUserLogin();
+        $login->assertSee('Thanks for registering!');
         $this->assertDatabaseHas('users', [
-            'email'           => $user->email,
+            'email' => $user->email,
             'email_confirmed' => false,
         ]);
 
@@ -677,8 +678,13 @@ class LdapTest extends TestCase
             'role_id' => $roleToReceive->id,
         ]);
 
+        $this->assertNull(auth()->user());
+
         $homePage = $this->get('/');
-        $homePage->assertRedirect('/register/confirm/awaiting');
+        $homePage->assertRedirect('/login');
+
+        $login = $this->followingRedirects()->mockUserLogin();
+        $login->assertSee('Email Address Not Confirmed');
     }
 
     public function test_failed_logins_are_logged_when_message_configured()
@@ -698,8 +704,8 @@ class LdapTest extends TestCase
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
-                'cn'        => [$this->mockUser->name],
-                'dn'        => $ldapDn,
+                'cn' => [$this->mockUser->name],
+                'dn' => $ldapDn,
                 'jpegphoto' => [base64_decode('/9j/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgGBgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8Q
 EBEQCgwSExIQEw8QEBD/yQALCAABAAEBAREA/8wABgAQEAX/2gAIAQEAAD8A0s8g/9k=')],
                 'mail' => [$this->mockUser->email],

diff --git a/tests/Auth/Saml2Test.php b/tests/Auth/Saml2Test.php
index 096f862e7c94c2244477322b23b37702a54389b7..8ace3e2ee4f19dd9ea8fee11f606f2225f601277 100644 (file)
--- a/tests/Auth/Saml2Test.php
+++ b/tests/Auth/Saml2Test.php
@@ -289,16 +289,18 @@ class Saml2Test extends TestCase
 
             $this->assertEquals('http://localhost/register/confirm', url()->current());
             $acsPost->assertSee('Please check your email and click the confirmation button to access BookStack.');
+            /** @var User $user */
             $user = User::query()->where('external_auth_id', '=', 'user')->first();
 
             $userRoleIds = $user->roles()->pluck('id');
             $this->assertContains($memberRole->id, $userRoleIds, 'User was assigned to member role');
             $this->assertContains($adminRole->id, $userRoleIds, 'User was assigned to admin role');
-            $this->assertTrue($user->email_confirmed == false, 'User email remains unconfirmed');
+            $this->assertFalse(boolval($user->email_confirmed), 'User email remains unconfirmed');
         });
 
+        $this->assertNull(auth()->user());
         $homeGet = $this->get('/');
-        $homeGet->assertRedirect('/register/confirm/awaiting');
+        $homeGet->assertRedirect('/login');
     }
 
     public function test_login_where_existing_non_saml_user_shows_warning()