From 7bcd967fd9a6dc25be81fcccbe7f1d4df17a7b3e Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 17 Jan 2016 15:20:07 +0000 Subject: [PATCH] Increased LDAP testing and fixed any Auth-based bugs found --- .env.example | 3 + app/Http/Controllers/Auth/AuthController.php | 1 + app/Http/Controllers/UserController.php | 25 +++++-- app/Repos/UserRepo.php | 8 +++ app/Services/ImageService.php | 22 ++++-- app/Services/LdapService.php | 3 +- readme.md | 59 ++++++++++++++-- tests/Auth/LdapTest.php | 73 +++++++++++++++++++- 8 files changed, 172 insertions(+), 22 deletions(-) diff --git a/.env.example b/.env.example index d32d96c0d..6d706abdd 100644 --- a/.env.example +++ b/.env.example @@ -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 diff --git a/app/Http/Controllers/Auth/AuthController.php b/app/Http/Controllers/Auth/AuthController.php index d601b4985..fef87d5c8 100644 --- a/app/Http/Controllers/Auth/AuthController.php +++ b/app/Http/Controllers/Auth/AuthController.php @@ -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()); diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index f504f4477..325d3118f 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -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 diff --git a/app/Repos/UserRepo.php b/app/Repos/UserRepo.php index 77ad22f39..d02ffe929 100644 --- a/app/Repos/UserRepo.php +++ b/app/Repos/UserRepo.php @@ -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; } diff --git a/app/Services/ImageService.php b/app/Services/ImageService.php index 600c85a24..0a4ef8849 100644 --- a/app/Services/ImageService.php +++ b/app/Services/ImageService.php @@ -1,5 +1,6 @@ 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; } diff --git a/app/Services/LdapService.php b/app/Services/LdapService.php index d33f8c378..84883b09a 100644 --- a/app/Services/LdapService.php +++ b/app/Services/LdapService.php @@ -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')); } /** diff --git a/readme.md b/readme.md index 1a429369b..5483404b2 100644 --- 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. diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 6b026dab9..14f2f8196 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -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