]> BookStack Code Mirror - bookstack/commitdiff
Increased LDAP testing and fixed any Auth-based bugs found
authorDan Brown <redacted>
Sun, 17 Jan 2016 15:20:07 +0000 (15:20 +0000)
committerDan Brown <redacted>
Sun, 17 Jan 2016 15:20:07 +0000 (15:20 +0000)
.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

index d32d96c0d9dcba332829df3eac4ca80c3809b9bc..6d706abddf5ff6c0d5632275136f697fd7593199 100644 (file)
@@ -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
index d601b4985f9469fedded73b32907ab58420fb02b..fef87d5c87cff3a6759a6b835c0eb496fc29d79d 100644 (file)
@@ -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());
index f504f447734c77d242c9146d536ce99f60d518e9..325d3118f947acb42b310d3ea24a29af4658b1ee 100644 (file)
@@ -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());
-        $user->password = bcrypt($request->get('password'));
-        $user->save();
 
+        if ($authMethod === 'standard') {
+            $user->password = bcrypt($request->get('password'));
+        } 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
index 77ad22f3996cf070e5b62c50ed413e326986e81e..d02ffe9290551ae685ca713fe5d3697074b1ca15 100644 (file)
@@ -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;
     }
 
index 600c85a24443413f82946c6c24c65f9ff1380f76..0a4ef8849a2987e5de94a6f8a3b42ac5cff237f1 100644 (file)
@@ -1,5 +1,6 @@
 <?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;
-        $image = Image::forceCreate([
+        $imageDetails = [
             'name'       => $imageName,
             'path'       => $fullPath,
             'url'        => $this->getPublicUrl($fullPath),
-            'type'       => $type,
-            'created_by' => $userId,
-            'updated_by' => $userId
-        ]);
+            'type'       => $type
+        ];
+
+        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;
     }
index d33f8c378fdb1fc87d2878d2dfda3fff40efcb4e..84883b09a20fa9422956578fa48d6909e7f14682 100644 (file)
@@ -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'));
     }
 
     /**
index 1a429369bb00e5a88f4fe4cb7a0b232312d14c5b..5483404b286f4ef4631775337775e06e0cb4c9f2 100644 (file)
--- a/readme.md
+++ b/readme.md
@@ -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.
-
-Currently BookStack requires its own domain/subdomain and will not work in a site subdirectory.
+Ensure the above requirements are met before installing. 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.
 
@@ -67,11 +77,11 @@ To update BookStack you can run the following command in the root directory of t
 ```
 git pull origin release && composer install && php artisan migrate
 ```
-This command will update the repository that was created in the installation, install the PHP dependencies using `composer` then run the database migrations. 
+This command will update the repository that was created in the installation, install the PHP dependencies using `composer` then run the database migrations.
 
 ## 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.
index 6b026dab9d09d1ebd016d528f288fe0ef0061a6e..14f2f8196d693834192f1285b1ee2adb379bf6f1 100644 (file)
@@ -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