From 5d08ec3cef1d9a2a1c96f47371f94f0762a49075 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 2 Feb 2020 12:00:41 +0000 Subject: [PATCH] Fixed failing tests caused by auth changes --- app/Config/auth.php | 2 +- .../views/auth/forms/login/ldap.blade.php | 4 +- tests/Auth/LdapTest.php | 111 +++++++++--------- tests/Auth/Saml2Test.php | 42 ++----- 4 files changed, 71 insertions(+), 88 deletions(-) diff --git a/app/Config/auth.php b/app/Config/auth.php index 2afb10ec2..66793308b 100644 --- a/app/Config/auth.php +++ b/app/Config/auth.php @@ -51,7 +51,7 @@ return [ // mechanisms used by this application to persist your user's data. 'providers' => [ 'users' => [ - 'driver' => env('AUTH_METHOD', 'standard') === 'standard' ? 'eloquent' : env('AUTH_METHOD'), + 'driver' => 'eloquent', 'model' => \BookStack\Auth\User::class, ], 'external' => [ diff --git a/resources/views/auth/forms/login/ldap.blade.php b/resources/views/auth/forms/login/ldap.blade.php index 12592d492..92eba80e8 100644 --- a/resources/views/auth/forms/login/ldap.blade.php +++ b/resources/views/auth/forms/login/ldap.blade.php @@ -19,10 +19,8 @@ @include('form.password', ['name' => 'password']) - -
-
+
diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 5d7bbfb1e..8bf9475cc 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -1,4 +1,5 @@ set([ '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, - 'auth.providers.users.driver' => 'ldap', + 'services.ldap.version' => '3', + 'services.ldap.user_filter' => '(&(uid=${user}))', + 'services.ldap.follow_referrals' => false, + 'services.ldap.tls_insecure' => false, ]); $this->mockLdap = \Mockery::mock(Ldap::class); $this->app[Ldap::class] = $this->mockLdap; @@ -60,16 +65,16 @@ class LdapTest extends BrowserKitTest { $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); $this->mockLdap->shouldReceive('setVersion')->once(); - $this->mockLdap->shouldReceive('setOption')->times(4); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) + $this->mockLdap->shouldReceive('setOption')->times(2); + $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')] ]]); - $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); - $this->mockEscapes(4); + $this->mockLdap->shouldReceive('bind')->times(4)->andReturn(true); + $this->mockEscapes(2); $this->mockUserLogin() ->seePageIs('/login')->see('Please enter an email to use for this account.'); @@ -86,16 +91,16 @@ class LdapTest extends BrowserKitTest $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); $this->mockLdap->shouldReceive('setVersion')->once(); $ldapDn = 'cn=test-user,dc=test' . config('services.ldap.base_dn'); - $this->mockLdap->shouldReceive('setOption')->times(2); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2) + $this->mockLdap->shouldReceive('setOption')->times(1); + $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, 'mail' => [$this->mockUser->email] ]]); - $this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true); - $this->mockEscapes(2); + $this->mockLdap->shouldReceive('bind')->times(2)->andReturn(true); + $this->mockEscapes(1); $this->mockUserLogin() ->seePageIs('/') @@ -109,8 +114,8 @@ class LdapTest extends BrowserKitTest $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); $this->mockLdap->shouldReceive('setVersion')->once(); $ldapDn = 'cn=test-user,dc=test' . config('services.ldap.base_dn'); - $this->mockLdap->shouldReceive('setOption')->times(2); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2) + $this->mockLdap->shouldReceive('setOption')->times(1); + $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], @@ -120,8 +125,8 @@ class LdapTest extends BrowserKitTest ]]); - $this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true); - $this->mockEscapes(2); + $this->mockLdap->shouldReceive('bind')->times(2)->andReturn(true); + $this->mockEscapes(1); $this->mockUserLogin() ->seePageIs('/') @@ -133,16 +138,16 @@ class LdapTest extends BrowserKitTest { $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); $this->mockLdap->shouldReceive('setVersion')->once(); - $this->mockLdap->shouldReceive('setOption')->times(2); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2) + $this->mockLdap->shouldReceive('setOption')->times(1); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1) ->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')] ]]); - $this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true, true, false); - $this->mockEscapes(2); + $this->mockLdap->shouldReceive('bind')->times(2)->andReturn(true, false); + $this->mockEscapes(1); $this->mockUserLogin() ->seePageIs('/login')->see('These credentials do not match our records.') @@ -201,10 +206,10 @@ class LdapTest extends BrowserKitTest 'services.ldap.group_attribute' => 'memberOf', 'services.ldap.remove_from_groups' => false, ]); - $this->mockLdap->shouldReceive('connect')->times(2)->andReturn($this->resourceId); - $this->mockLdap->shouldReceive('setVersion')->times(2); - $this->mockLdap->shouldReceive('setOption')->times(5); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(5) + $this->mockLdap->shouldReceive('connect')->times(1)->andReturn($this->resourceId); + $this->mockLdap->shouldReceive('setVersion')->times(1); + $this->mockLdap->shouldReceive('setOption')->times(4); + $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], @@ -217,8 +222,8 @@ class LdapTest extends BrowserKitTest 1 => "cn=ldaptester-second,ou=groups,dc=example,dc=com", ] ]]); - $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); - $this->mockEscapes(5); + $this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true); + $this->mockEscapes(4); $this->mockExplodes(6); $this->mockUserLogin()->seePageIs('/'); @@ -250,10 +255,10 @@ class LdapTest extends BrowserKitTest 'services.ldap.group_attribute' => 'memberOf', 'services.ldap.remove_from_groups' => true, ]); - $this->mockLdap->shouldReceive('connect')->times(2)->andReturn($this->resourceId); - $this->mockLdap->shouldReceive('setVersion')->times(2); - $this->mockLdap->shouldReceive('setOption')->times(4); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) + $this->mockLdap->shouldReceive('connect')->times(1)->andReturn($this->resourceId); + $this->mockLdap->shouldReceive('setVersion')->times(1); + $this->mockLdap->shouldReceive('setOption')->times(3); + $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], @@ -265,8 +270,8 @@ class LdapTest extends BrowserKitTest 0 => "cn=ldaptester,ou=groups,dc=example,dc=com", ] ]]); - $this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true); - $this->mockEscapes(4); + $this->mockLdap->shouldReceive('bind')->times(4)->andReturn(true); + $this->mockEscapes(3); $this->mockExplodes(2); $this->mockUserLogin()->seePageIs('/'); @@ -299,10 +304,10 @@ class LdapTest extends BrowserKitTest 'services.ldap.group_attribute' => 'memberOf', 'services.ldap.remove_from_groups' => true, ]); - $this->mockLdap->shouldReceive('connect')->times(2)->andReturn($this->resourceId); - $this->mockLdap->shouldReceive('setVersion')->times(2); - $this->mockLdap->shouldReceive('setOption')->times(4); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) + $this->mockLdap->shouldReceive('connect')->times(1)->andReturn($this->resourceId); + $this->mockLdap->shouldReceive('setVersion')->times(1); + $this->mockLdap->shouldReceive('setOption')->times(3); + $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], @@ -314,8 +319,8 @@ class LdapTest extends BrowserKitTest 0 => "cn=ex-auth-a,ou=groups,dc=example,dc=com", ] ]]); - $this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true); - $this->mockEscapes(4); + $this->mockLdap->shouldReceive('bind')->times(4)->andReturn(true); + $this->mockEscapes(3); $this->mockExplodes(2); $this->mockUserLogin()->seePageIs('/'); @@ -344,10 +349,10 @@ class LdapTest extends BrowserKitTest 'services.ldap.group_attribute' => 'memberOf', 'services.ldap.remove_from_groups' => true, ]); - $this->mockLdap->shouldReceive('connect')->times(2)->andReturn($this->resourceId); - $this->mockLdap->shouldReceive('setVersion')->times(2); - $this->mockLdap->shouldReceive('setOption')->times(5); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(5) + $this->mockLdap->shouldReceive('connect')->times(1)->andReturn($this->resourceId); + $this->mockLdap->shouldReceive('setVersion')->times(1); + $this->mockLdap->shouldReceive('setOption')->times(4); + $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], @@ -360,8 +365,8 @@ class LdapTest extends BrowserKitTest 1 => "cn=ldaptester-second,ou=groups,dc=example,dc=com", ] ]]); - $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); - $this->mockEscapes(5); + $this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true); + $this->mockEscapes(4); $this->mockExplodes(6); $this->mockUserLogin()->seePageIs('/'); @@ -385,8 +390,8 @@ class LdapTest extends BrowserKitTest $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); $this->mockLdap->shouldReceive('setVersion')->once(); - $this->mockLdap->shouldReceive('setOption')->times(4); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) + $this->mockLdap->shouldReceive('setOption')->times(2); + $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], @@ -394,8 +399,8 @@ class LdapTest extends BrowserKitTest 'dn' => ['dc=test' . config('services.ldap.base_dn')], 'displayname' => 'displayNameAttribute' ]]); - $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); - $this->mockEscapes(4); + $this->mockLdap->shouldReceive('bind')->times(4)->andReturn(true); + $this->mockEscapes(2); $this->mockUserLogin() ->seePageIs('/login')->see('Please enter an email to use for this account.'); @@ -415,16 +420,16 @@ class LdapTest extends BrowserKitTest $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); $this->mockLdap->shouldReceive('setVersion')->once(); - $this->mockLdap->shouldReceive('setOption')->times(4); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) + $this->mockLdap->shouldReceive('setOption')->times(2); + $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')] ]]); - $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); - $this->mockEscapes(4); + $this->mockLdap->shouldReceive('bind')->times(4)->andReturn(true); + $this->mockEscapes(2); $this->mockUserLogin() ->seePageIs('/login')->see('Please enter an email to use for this account.'); @@ -444,14 +449,14 @@ class LdapTest extends BrowserKitTest // Standard mocks $this->mockLdap->shouldReceive('setVersion')->once(); - $this->mockLdap->shouldReceive('setOption')->times(2); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)->andReturn(['count' => 1, 0 => [ + $this->mockLdap->shouldReceive('setOption')->times(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')] ]]); - $this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true); - $this->mockEscapes(2); + $this->mockLdap->shouldReceive('bind')->times(2)->andReturn(true); + $this->mockEscapes(1); $this->mockLdap->shouldReceive('connect')->once() ->with($expectedHost, $expectedPort)->andReturn($this->resourceId); diff --git a/tests/Auth/Saml2Test.php b/tests/Auth/Saml2Test.php index 45b6efa07..2cecad8bf 100644 --- a/tests/Auth/Saml2Test.php +++ b/tests/Auth/Saml2Test.php @@ -11,9 +11,9 @@ class Saml2Test extends TestCase parent::setUp(); // Set default config for SAML2 config()->set([ + 'auth.method' => 'saml2', + 'auth.defaults.guard' => 'saml2', 'saml2.name' => 'SingleSignOn-Testing', - 'saml2.enabled' => true, - 'saml2.auto_register' => true, 'saml2.email_attribute' => 'email', 'saml2.display_name_attributes' => ['first_name', 'last_name'], 'saml2.external_id_attribute' => 'uid', @@ -53,27 +53,12 @@ class Saml2Test extends TestCase { $req = $this->get('/login'); $req->assertSeeText('SingleSignOn-Testing'); - $req->assertElementExists('a[href$="/saml2/login"]'); - } - - public function test_login_option_shows_on_register_page_only_when_auto_register_enabled() - { - $this->setSettings(['app-public' => 'true', 'registration-enabled' => 'true']); - - $req = $this->get('/register'); - $req->assertSeeText('SingleSignOn-Testing'); - $req->assertElementExists('a[href$="/saml2/login"]'); - - config()->set(['saml2.auto_register' => false]); - - $req = $this->get('/register'); - $req->assertDontSeeText('SingleSignOn-Testing'); - $req->assertElementNotExists('a[href$="/saml2/login"]'); + $req->assertElementExists('form[action$="/saml2/login"][method=POST] button'); } public function test_login() { - $req = $this->get('/saml2/login'); + $req = $this->post('/saml2/login'); $redirect = $req->headers->get('location'); $this->assertStringStartsWith('http://saml.local/saml2/idp/SSOService.php', $redirect, 'Login redirects to SSO location'); @@ -138,20 +123,15 @@ class Saml2Test extends TestCase }); } - public function test_logout_redirects_to_saml_logout_when_active_saml_session() + public function test_logout_link_directs_to_saml_path() { config()->set([ 'saml2.onelogin.strict' => false, ]); - $this->withPost(['SAMLResponse' => $this->acsPostData], function () { - $acsPost = $this->post('/saml2/acs'); - $lastLoginType = session()->get('last_login_type'); - $this->assertEquals('saml2', $lastLoginType); - - $req = $this->get('/logout'); - $req->assertRedirect('/saml2/logout'); - }); + $resp = $this->actingAs($this->getEditor())->get('/'); + $resp->assertElementExists('a[href$="/saml2/logout"]'); + $resp->assertElementContains('a[href$="/saml2/logout"]', 'Logout'); } public function test_logout_sls_flow() @@ -235,8 +215,8 @@ class Saml2Test extends TestCase public function test_saml_routes_are_only_active_if_saml_enabled() { - config()->set(['saml2.enabled' => false]); - $getRoutes = ['/login', '/logout', '/metadata', '/sls']; + config()->set(['auth.method' => 'standard']); + $getRoutes = ['/logout', '/metadata', '/sls']; foreach ($getRoutes as $route) { $req = $this->get('/saml2' . $route); $req->assertRedirect('/'); @@ -245,7 +225,7 @@ class Saml2Test extends TestCase session()->flush(); } - $postRoutes = ['/acs']; + $postRoutes = ['/login', '/acs']; foreach ($postRoutes as $route) { $req = $this->post('/saml2' . $route); $req->assertRedirect('/');