diff --git a/app/Auth/Access/EmailConfirmationService.php b/app/Auth/Access/EmailConfirmationService.php index 3425c1e72..75c03b318 100644 --- 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 f6ea7517f..998259dee 100644 --- 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 16e3edbb4..4c8b53c0d 100644 --- 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 d0b6a655b..3f0f40ccc 100644 --- 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 de8898df7..7a978ffc6 100644 --- 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 4008439bc..e372b38ef 100644 --- 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 b9b5545f2..dede8f260 100644 --- 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 febf58399..d57a3253f 100644 --- 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 9d491fd0f..3bd6988e4 100644 --- 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 096f862e7..8ace3e2ee 100644 --- 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()