Increased LDAP testing and fixed any Auth-based bugs found

Dan Brown
Commit 7bcd967fd9a6dc25be81fcccbe7f1d4df17a7b3e 7bcd967f 1 parent bb87401d
Showing 8 changed files with 170 additions and 20 deletions
.env.example
app/Http/Controllers/Auth/AuthController.php
app/Http/Controllers/UserController.php
app/Repos/UserRepo.php
app/Services/ImageService.php
app/Services/LdapService.php
readme.md
tests/Auth/LdapTest.php
--- a/.env.example
View file @7bcd967
+++ b/.env.example
View file @7bcd967
@@ -25,6 +25,9 @@ STORAGE_S3_BUCKET=false
 # Used to prefix image urls for when using custom domains/cdns
 STORAGE_URL=false
+# General auth
+AUTH_METHOD=standard
+
 # Social Authentication information. Defaults as off.
 GITHUB_APP_ID=false
 GITHUB_APP_SECRET=false
--- a/app/Http/Controllers/Auth/AuthController.php
View file @7bcd967
+++ b/app/Http/Controllers/Auth/AuthController.php
View file @7bcd967
@@ -191,6 +191,7 @@ class AuthController extends Controller
         }
         $newUser->email_confirmed = true;
+
         auth()->login($newUser);
         session()->flash('success', 'Thanks for signing up! You are now registered and signed in.');
         return redirect($this->redirectPath());
--- a/app/Http/Controllers/UserController.php
View file @7bcd967
+++ b/app/Http/Controllers/UserController.php
View file @7bcd967
@@ -58,18 +58,31 @@ class UserController extends Controller
     public function store(Request $request)
     {
         $this->checkPermission('user-create');
-        $this->validate($request, [
+        $validationRules = [
             'name'             => 'required',
             'email'            => 'required|email|unique:users,email',
-            'password'         => 'required|min:5',
-            'password-confirm' => 'required|same:password',
             'role'             => 'required|exists:roles,id'
-        ]);
+        ];
+
+        $authMethod = config('auth.method');
+        if ($authMethod === 'standard') {
+            $validationRules['password'] = 'required|min:5';
+            $validationRules['password-confirm'] = 'required|same:password';
+        } elseif ($authMethod === 'ldap') {
+            $validationRules['external_auth_id'] = 'required';
+        }
+        $this->validate($request, $validationRules);
+
         $user = $this->user->fill($request->all());
+
+        if ($authMethod === 'standard') {
             $user->password = bcrypt($request->get('password'));
-        $user->save();
+        } elseif ($authMethod === 'ldap') {
+            $user->external_auth_id = $request->get('external_auth_id');
+        }
+        $user->save();
         $user->attachRoleId($request->get('role'));
         // Get avatar from gravatar and save
--- a/app/Repos/UserRepo.php
View file @7bcd967
+++ b/app/Repos/UserRepo.php
View file @7bcd967
@@ -48,6 +48,14 @@ class UserRepo
     {
         $user = $this->create($data);
         $this->attachDefaultRole($user);
+
+        // Get avatar from gravatar and save
+        if (!config('services.disable_services')) {
+            $avatar = \Images::saveUserGravatar($user);
+            $user->avatar()->associate($avatar);
+            $user->save();
+        }
+
         return $user;
     }
--- a/app/Services/ImageService.php
View file @7bcd967
+++ b/app/Services/ImageService.php
View file @7bcd967
 <?php namespace BookStack\Services;
+use BookStack\Exceptions\ImageUploadException;
 use BookStack\Image;
 use BookStack\User;
 use Intervention\Image\ImageManager;
@@ -71,6 +72,7 @@ class ImageService
      * @param string $imageData
      * @param string $type
      * @return Image
+     * @throws ImageUploadException
      */
     private function saveNew($imageName, $imageData, $type)
     {
@@ -86,17 +88,24 @@ class ImageService
         }
         $fullPath = $imagePath . $imageName;
+        if(!is_writable(dirname(public_path($fullPath)))) throw new ImageUploadException('Image Directory ' . public_path($fullPath) . ' is not writable by the server.');
+
         $storage->put($fullPath, $imageData);
-        $userId = auth()->user()->id;
+        $imageDetails = [
-        $image = Image::forceCreate([
             'name'       => $imageName,
             'path'       => $fullPath,
             'url'        => $this->getPublicUrl($fullPath),
-            'type'       => $type,
+            'type'       => $type
-            'created_by' => $userId,
+        ];
-            'updated_by' => $userId
+
-        ]);
+        if (auth()->user() && auth()->user()->id !== 0) {
+            $userId = auth()->user()->id;
+            $imageDetails['created_by'] = $userId;
+            $imageDetails['updated_by'] = $userId;
+        }
+
+        $image = Image::forceCreate($imageDetails);
         return $image;
     }
@@ -188,6 +197,7 @@ class ImageService
         $imageName = str_replace(' ', '-', $user->name . '-gravatar.png');
         $image = $this->saveNewFromUrl($url, 'user', $imageName);
         $image->created_by = $user->id;
+        $image->updated_by = $user->id;
         $image->save();
         return $image;
     }
--- a/app/Services/LdapService.php
View file @7bcd967
+++ b/app/Services/LdapService.php
View file @7bcd967
@@ -36,6 +36,7 @@ class LdapService
     public function getUserDetails($userName)
     {
         $ldapConnection = $this->getConnection();
+        $this->bindSystemUser($ldapConnection);
         // Find user
         $userFilter = $this->buildFilter($this->config['user_filter'], ['user' => $userName]);
@@ -93,7 +94,7 @@ class LdapService
             $ldapBind = $this->ldap->bind($connection, $ldapDn, $ldapPass);
         }
-        if (!$ldapBind) throw new LdapException('LDAP access failed using ' . $isAnonymous ? ' anonymous bind.' : ' given dn & pass details');
+        if (!$ldapBind) throw new LdapException('LDAP access failed using ' . ($isAnonymous ? ' anonymous bind.' : ' given dn & pass details'));
     }
     /**
--- a/readme.md
View file @7bcd967
+++ b/readme.md
View file @7bcd967
@@ -2,12 +2,24 @@
 A platform to create documentation/wiki content. General information about BookStack can be found at https://www.bookstackapp.com/
+1. [Requirements](#requirements)
+2. [Installation](#installation)
+  - [Server Rewrite Rules](#url-rewrite-rules)
+3. [Updating](#updating-bookstack)
+4. [Social Authentication](#social-authentication)
+ - [Google](#google)
+ - [GitHub](#github)
+5. [LDAP Authentication](#ldap-authentication)
+6. [Testing](#testing)
+7. [License](#license)
+8. [Attribution](#attribution)
+
 ## Requirements
 BookStack has similar requirements to Laravel. On top of those are some front-end build tools which are only required when developing.
-* PHP >= 5.5.9
+* PHP >= 5.5.9, Will need to be usable from the command line.
 * OpenSSL PHP Extension
 * PDO PHP Extension
 * MBstring PHP Extension
@@ -21,9 +33,7 @@ BookStack has similar requirements to Laravel. On top of those are some front-en
 ## Installation
-Ensure the requirements are met before installing.
+Ensure the above requirements are met before installing. Currently BookStack requires its own domain/subdomain and will not work in a site subdirectory.
-
-Currently BookStack requires its own domain/subdomain and will not work in a site subdirectory.
 This project currently uses the `release` branch of this repository as a stable channel for providing updates.
@@ -71,7 +81,7 @@ This command will update the repository that was created in the installation, in
 ## Social Authentication
-BookStack currently supports login via both Google and Github. Once enabled options for these services will show up in the login, registration and user profile pages. By default these services are disabled. To enable them you will have to create an application on the external services to obtain the require application id's and secrets. Here are instructions to do this for the current supported services:
+BookStack currently supports login via both Google and GitHub. Once enabled options for these services will show up in the login, registration and user profile pages. By default these services are disabled. To enable them you will have to create an application on the external services to obtain the require application id's and secrets. Here are instructions to do this for the current supported services:
 ### Google
@@ -97,6 +107,43 @@ BookStack currently supports login via both Google and Github. Once enabled opti
 5. Set the 'APP_URL' environment variable to be the same domain as you entered in step 3.
 6. All done! Users should now be able to link to their social accounts in their account profile pages and also register/login using their Github account.
+## LDAP Authentication
+
+BookStack can be configured to allow LDAP based user login. While LDAP login is enabled you cannot log in with the standard user/password login and new user registration is disabled. BookStack will only use the LDAP server for getting user details and for authentication. Data on the LDAP server is not currently editable through BookStack.
+
+When a LDAP user logs into BookStack for the first time their BookStack profile will be created and they will be given the default role set under the 'Default user role after registration' option in the application settings.    
+
+To set up LDAP-based authentication add or modify the following variables in your `.env` file:
+
+```
+# General auth
+AUTH_METHOD=ldap
+
+# The LDAP host, Adding a port is optional
+LDAP_SERVER=ldap://example.com:389
+
+# The base DN from where users will be searched within.
+LDAP_BASE_DN=ou=People,dc=example,dc=com
+
+# The full DN and password of the user used to search the server
+# Can both be left as false to bind anonymously
+LDAP_DN=false
+LDAP_PASS=false
+
+# A filter to use when searching for users
+# The user-provided user-name used to replace any occurrences of '${user}'
+LDAP_USER_FILTER=(&(uid=${user}))
+
+# Set the LDAP version to use when connecting to the server.
+LDAP_VERSION=false
+```
+
+You will also need to have the php-ldap extension installed on your system. It's recommended to change your `APP_DEBUG` variable to `true` while setting up LDAP to make any errors visible. Remember to change this back after LDAP is functioning.
+
+A user in BookStack will be linked to a LDAP user via a 'uid'. If a LDAP user uid changes it can be updated in BookStack by an admin by changing the 'External Authentication ID' field on the user's profile.
+
+You may find that you cannot log in with your initial Admin account after changing the `AUTH_METHOD` to `ldap`. To get around this set the `AUTH_METHOD` to `standard`, login with your admin account then change it back to `ldap`. You get then edit your profile and add your LDAP uid under the 'External Authentication ID' field. You will then be able to login in with that ID.
+
 ## Testing
 BookStack has many integration tests that use Laravel's built-in testing capabilities which makes use of PHPUnit. To use you will need PHPUnit installed and accessible via command line. There is a `mysql_testing` database defined within the app config which is what is used by PHPUnit. This database is set with the following database name, user name and password defined as `bookstack-test`. You will have to create that database and credentials before testing.
--- a/tests/Auth/LdapTest.php
View file @7bcd967
+++ b/tests/Auth/LdapTest.php
View file @7bcd967
@@ -19,18 +19,18 @@ class LdapTest extends \TestCase
         $this->mockUser = factory(User::class)->make();
     }
-    public function test_ldap_login()
+    public function test_login()
     {
         $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId);
         $this->mockLdap->shouldReceive('setOption')->once();
-        $this->mockLdap->shouldReceive('searchAndGetEntries')->twice()
+        $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')]
             ]]);
-        $this->mockLdap->shouldReceive('bind')->times(1)->andReturn(true);
+        $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true);
         $this->visit('/login')
             ->see('Username')
@@ -38,6 +38,73 @@ class LdapTest extends \TestCase
             ->type($this->mockUser->password, '#password')
             ->press('Sign In')
             ->seePageIs('/login')->see('Please enter an email to use for this account.');
+
+        $this->type($this->mockUser->email, '#email')
+            ->press('Sign In')
+            ->seePageIs('/')
+            ->see($this->mockUser->name)
+            ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => 1, 'external_auth_id' => $this->mockUser->name]);
+    }
+
+    public function test_initial_incorrect_details()
+    {
+        $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId);
+        $this->mockLdap->shouldReceive('setOption')->once();
+        $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(3)->andReturn(true, true, false);
+
+        $this->visit('/login')
+            ->see('Username')
+            ->type($this->mockUser->name, '#username')
+            ->type($this->mockUser->password, '#password')
+            ->press('Sign In')
+            ->seePageIs('/login')->see('These credentials do not match our records.')
+            ->dontSeeInDatabase('users', ['external_auth_id' => $this->mockUser->name]);
+    }
+
+    public function test_create_user_form()
+    {
+        $this->asAdmin()->visit('/users/create')
+            ->dontSee('Password')
+            ->type($this->mockUser->name, '#name')
+            ->type($this->mockUser->email, '#email')
+            ->press('Save')
+            ->see('The external auth id field is required.')
+            ->type($this->mockUser->name, '#external_auth_id')
+            ->press('Save')
+            ->seePageIs('/users')
+            ->seeInDatabase('users', ['email' => $this->mockUser->email, 'external_auth_id' => $this->mockUser->name, 'email_confirmed' => true]);
+    }
+
+    public function test_user_edit_form()
+    {
+        $editUser = User::all()->last();
+        $this->asAdmin()->visit('/users/' . $editUser->id)
+            ->see('Edit User')
+            ->dontSee('Password')
+            ->type('test_auth_id', '#external_auth_id')
+            ->press('Save')
+            ->seePageIs('/users')
+            ->seeInDatabase('users', ['email' => $editUser->email, 'external_auth_id' => 'test_auth_id']);
+    }
+
+    public function test_registration_disabled()
+    {
+        $this->visit('/register')
+            ->seePageIs('/login');
+    }
+
+    public function test_non_admins_cannot_change_auth_id()
+    {
+        $testUser = User::all()->last();
+        $this->actingAs($testUser)->visit('/users/' . $testUser->id)
+            ->dontSee('External Authentication');
     }
 }
\ No newline at end of file