From d640cc1eeef8fe5786f663b13dc3910b3d4d6b2e Mon Sep 17 00:00:00 2001 From: Brennan Murphy Date: Mon, 2 Jul 2018 17:09:39 +0000 Subject: [PATCH 1/5] LDAP groups sync to Bookstack roles. Closes #75 --- .env.example | 9 + app/Http/Controllers/Auth/LoginController.php | 10 +- app/Repos/LdapRepo.php | 84 ++++ app/Services/LdapService.php | 378 +++++++++++------- config/services.php | 6 +- 5 files changed, 350 insertions(+), 137 deletions(-) create mode 100644 app/Repos/LdapRepo.php diff --git a/.env.example b/.env.example index ccafaf4fb..57e6af6a9 100644 --- a/.env.example +++ b/.env.example @@ -67,6 +67,15 @@ LDAP_DN=false LDAP_PASS=false LDAP_USER_FILTER=false LDAP_VERSION=false +#do you want to sync LDAP groups to BookStack roles for a user +LDAP_USER_TO_GROUPS=false +#what is the LDAP attribute for group memberships +LDAP_GROUP_ATTRIBUTE="memberOf" +#what LDAP group should the user be a part of to be an admin on BookStack +LDAP_ADMIN_GROUP="Domain Admins" +#would you like to remove users from roles on bookstack if they do not match on LDAP +#if false, the ldap groups-roles sync will only add users to roles +LDAP_REMOVE_FROM_GROUPS=false # Mail settings MAIL_DRIVER=smtp diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 106b90524..4c846d0e0 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -5,6 +5,7 @@ namespace BookStack\Http\Controllers\Auth; use BookStack\Exceptions\AuthException; use BookStack\Http\Controllers\Controller; use BookStack\Repos\UserRepo; +use BookStack\Repos\LdapRepo; use BookStack\Services\SocialAuthService; use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Foundation\Auth\AuthenticatesUsers; @@ -96,7 +97,14 @@ class LoginController extends Controller auth()->login($user); } - $path = session()->pull('url.intended', '/'); + // ldap groups refresh + if (config('services.ldap.user_to_groups') !== false && $request->filled('username')) { + $ldapRepo = new LdapRepo($this->userRepo); + $ldapRepo->syncGroups($user,$request->input('username')); + } + + + $path = session()->pull('url.intended', '/'); $path = baseUrl($path, true); return redirect($path); } diff --git a/app/Repos/LdapRepo.php b/app/Repos/LdapRepo.php new file mode 100644 index 000000000..33d05ea88 --- /dev/null +++ b/app/Repos/LdapRepo.php @@ -0,0 +1,84 @@ +config = config('services.ldap'); + + if (config('auth.method') !== 'ldap') { + return false; + } + + $this->ldapService = new LdapService(new Ldap); + $this->userRepo = $userRepo; + } + + /** + * If there is no ldap connection, all methods calls to this library will return null + */ + public function __call($method, $arguments) + { + if ($this->ldap === null) { + return null; + } + + return call_user_func_array(array($this,$method),$arguments); + } + + /** + * Sync the LDAP groups to the user roles for the current user + * @param \BookStack\User $user + * @param string $userName + * @throws \BookStack\Exceptions\NotFoundException + */ + public function syncGroups($user,$userName) + { + $userLdapGroups = $this->ldapService->getUserGroups($userName); + $userLdapGroups = $this->groupNameFilter($userLdapGroups); + // get the ids for the roles from the names + $ldapGroupsAsRoles = Role::whereIn('name',$userLdapGroups)->pluck('id'); + // sync groups + if ($this->config['remove_from_groups']) { + $user->roles()->sync($ldapGroupsAsRoles); + $this->userRepo->attachDefaultRole($user); + } else { + $user->roles()->syncWithoutDetaching($ldapGroupsAsRoles); + } + + // make the user an admin? + if (in_array($this->config['admin'],$userLdapGroups)) { + $this->userRepo->attachSystemRole($user,'admin'); + } + } + + /** + * Filter to convert the groups from ldap to the format of the roles name on BookStack + * Spaces replaced with -, all lowercase letters + * @param array $groups + * @return array + */ + private function groupNameFilter($groups) + { + $return = []; + foreach ($groups as $groupName) { + $return[] = str_replace(' ', '-', strtolower($groupName)); + } + return $return; + } +} \ No newline at end of file diff --git a/app/Services/LdapService.php b/app/Services/LdapService.php index 3eb2f2830..e56f45e4e 100644 --- a/app/Services/LdapService.php +++ b/app/Services/LdapService.php @@ -11,155 +11,263 @@ use Illuminate\Contracts\Auth\Authenticatable; class LdapService { - protected $ldap; - protected $ldapConnection; - protected $config; + protected $ldap; + protected $ldapConnection; + protected $config; - /** - * LdapService constructor. - * @param Ldap $ldap - */ - public function __construct(Ldap $ldap) - { - $this->ldap = $ldap; - $this->config = config('services.ldap'); - } + /** + * LdapService constructor. + * @param Ldap $ldap + */ + public function __construct(Ldap $ldap) + { + $this->ldap = $ldap; + $this->config = config('services.ldap'); + } - /** - * Get the details of a user from LDAP using the given username. - * User found via configurable user filter. - * @param $userName - * @return array|null - * @throws LdapException - */ - public function getUserDetails($userName) - { - $ldapConnection = $this->getConnection(); - $this->bindSystemUser($ldapConnection); + /** + * Search for attributes for a specific user on the ldap + * @param string $userName + * @param array $attributes + * @return null|array + * @throws LdapException + */ + private function getUserWithAttributes($userName,$attributes) + { + $ldapConnection = $this->getConnection(); + $this->bindSystemUser($ldapConnection); - // Find user - $userFilter = $this->buildFilter($this->config['user_filter'], ['user' => $userName]); - $baseDn = $this->config['base_dn']; - $emailAttr = $this->config['email_attribute']; - $followReferrals = $this->config['follow_referrals'] ? 1 : 0; - $this->ldap->setOption($ldapConnection, LDAP_OPT_REFERRALS, $followReferrals); - $users = $this->ldap->searchAndGetEntries($ldapConnection, $baseDn, $userFilter, ['cn', 'uid', 'dn', $emailAttr]); - if ($users['count'] === 0) { - return null; - } + // Find user + $userFilter = $this->buildFilter($this->config['user_filter'], ['user' => $userName]); + $baseDn = $this->config['base_dn']; - $user = $users[0]; - return [ - 'uid' => (isset($user['uid'])) ? $user['uid'][0] : $user['dn'], - 'name' => $user['cn'][0], - 'dn' => $user['dn'], - 'email' => (isset($user[$emailAttr])) ? (is_array($user[$emailAttr]) ? $user[$emailAttr][0] : $user[$emailAttr]) : null - ]; - } + $followReferrals = $this->config['follow_referrals'] ? 1 : 0; + $this->ldap->setOption($ldapConnection, LDAP_OPT_REFERRALS, $followReferrals); + $users = $this->ldap->searchAndGetEntries($ldapConnection, $baseDn, $userFilter, $attributes); + if ($users['count'] === 0) { + return null; + } - /** - * @param Authenticatable $user - * @param string $username - * @param string $password - * @return bool - * @throws LdapException - */ - public function validateUserCredentials(Authenticatable $user, $username, $password) - { - $ldapUser = $this->getUserDetails($username); - if ($ldapUser === null) { - return false; - } - if ($ldapUser['uid'] !== $user->external_auth_id) { - return false; - } + return $users[0]; + } - $ldapConnection = $this->getConnection(); - try { - $ldapBind = $this->ldap->bind($ldapConnection, $ldapUser['dn'], $password); - } catch (\ErrorException $e) { - $ldapBind = false; - } + /** + * Get the details of a user from LDAP using the given username. + * User found via configurable user filter. + * @param $userName + * @return array|null + * @throws LdapException + */ + public function getUserDetails($userName) + { + $emailAttr = $this->config['email_attribute']; + $user = $this->getUserWithAttributes($userName, ['cn', 'uid', 'dn', $emailAttr]); - return $ldapBind; - } + if ($user === null) { + return null; + } - /** - * Bind the system user to the LDAP connection using the given credentials - * otherwise anonymous access is attempted. - * @param $connection - * @throws LdapException - */ - protected function bindSystemUser($connection) - { - $ldapDn = $this->config['dn']; - $ldapPass = $this->config['pass']; + return [ + 'uid' => (isset($user['uid'])) ? $user['uid'][0] : $user['dn'], + 'name' => $user['cn'][0], + 'dn' => $user['dn'], + 'email' => (isset($user[$emailAttr])) ? (is_array($user[$emailAttr]) ? $user[$emailAttr][0] : $user[$emailAttr]) : null + ]; + } - $isAnonymous = ($ldapDn === false || $ldapPass === false); - if ($isAnonymous) { - $ldapBind = $this->ldap->bind($connection); - } else { - $ldapBind = $this->ldap->bind($connection, $ldapDn, $ldapPass); - } + /** + * @param Authenticatable $user + * @param string $username + * @param string $password + * @return bool + * @throws LdapException + */ + public function validateUserCredentials(Authenticatable $user, $username, $password) + { + $ldapUser = $this->getUserDetails($username); + if ($ldapUser === null) { + return false; + } + if ($ldapUser['uid'] !== $user->external_auth_id) { + return false; + } - if (!$ldapBind) { - throw new LdapException(($isAnonymous ? trans('errors.ldap_fail_anonymous') : trans('errors.ldap_fail_authed'))); - } - } + $ldapConnection = $this->getConnection(); + try { + $ldapBind = $this->ldap->bind($ldapConnection, $ldapUser['dn'], $password); + } catch (\ErrorException $e) { + $ldapBind = false; + } - /** - * Get the connection to the LDAP server. - * Creates a new connection if one does not exist. - * @return resource - * @throws LdapException - */ - protected function getConnection() - { - if ($this->ldapConnection !== null) { - return $this->ldapConnection; - } + return $ldapBind; + } - // Check LDAP extension in installed - if (!function_exists('ldap_connect') && config('app.env') !== 'testing') { - throw new LdapException(trans('errors.ldap_extension_not_installed')); - } + /** + * Bind the system user to the LDAP connection using the given credentials + * otherwise anonymous access is attempted. + * @param $connection + * @throws LdapException + */ + protected function bindSystemUser($connection) + { + $ldapDn = $this->config['dn']; + $ldapPass = $this->config['pass']; - // Get port from server string and protocol if specified. - $ldapServer = explode(':', $this->config['server']); - $hasProtocol = preg_match('/^ldaps{0,1}\:\/\//', $this->config['server']) === 1; - if (!$hasProtocol) { - array_unshift($ldapServer, ''); - } - $hostName = $ldapServer[0] . ($hasProtocol?':':'') . $ldapServer[1]; - $defaultPort = $ldapServer[0] === 'ldaps' ? 636 : 389; - $ldapConnection = $this->ldap->connect($hostName, count($ldapServer) > 2 ? intval($ldapServer[2]) : $defaultPort); + $isAnonymous = ($ldapDn === false || $ldapPass === false); + if ($isAnonymous) { + $ldapBind = $this->ldap->bind($connection); + } else { + $ldapBind = $this->ldap->bind($connection, $ldapDn, $ldapPass); + } - if ($ldapConnection === false) { - throw new LdapException(trans('errors.ldap_cannot_connect')); - } + if (!$ldapBind) { + throw new LdapException(($isAnonymous ? trans('errors.ldap_fail_anonymous') : trans('errors.ldap_fail_authed'))); + } + } - // Set any required options - if ($this->config['version']) { - $this->ldap->setVersion($ldapConnection, $this->config['version']); - } + /** + * Get the connection to the LDAP server. + * Creates a new connection if one does not exist. + * @return resource + * @throws LdapException + */ + protected function getConnection() + { + if ($this->ldapConnection !== null) { + return $this->ldapConnection; + } - $this->ldapConnection = $ldapConnection; - return $this->ldapConnection; - } + // Check LDAP extension in installed + if (!function_exists('ldap_connect') && config('app.env') !== 'testing') { + throw new LdapException(trans('errors.ldap_extension_not_installed')); + } - /** - * Build a filter string by injecting common variables. - * @param string $filterString - * @param array $attrs - * @return string - */ - protected function buildFilter($filterString, array $attrs) - { - $newAttrs = []; - foreach ($attrs as $key => $attrText) { - $newKey = '${' . $key . '}'; - $newAttrs[$newKey] = $attrText; - } - return strtr($filterString, $newAttrs); - } -} + // Get port from server string and protocol if specified. + $ldapServer = explode(':', $this->config['server']); + $hasProtocol = preg_match('/^ldaps{0,1}\:\/\//', $this->config['server']) === 1; + if (!$hasProtocol) { + array_unshift($ldapServer, ''); + } + $hostName = $ldapServer[0] . ($hasProtocol?':':'') . $ldapServer[1]; + $defaultPort = $ldapServer[0] === 'ldaps' ? 636 : 389; + $ldapConnection = $this->ldap->connect($hostName, count($ldapServer) > 2 ? intval($ldapServer[2]) : $defaultPort); + + if ($ldapConnection === false) { + throw new LdapException(trans('errors.ldap_cannot_connect')); + } + + // Set any required options + if ($this->config['version']) { + $this->ldap->setVersion($ldapConnection, $this->config['version']); + } + + $this->ldapConnection = $ldapConnection; + return $this->ldapConnection; + } + + /** + * Build a filter string by injecting common variables. + * @param string $filterString + * @param array $attrs + * @return string + */ + protected function buildFilter($filterString, array $attrs) + { + $newAttrs = []; + foreach ($attrs as $key => $attrText) { + $newKey = '${' . $key . '}'; + $newAttrs[$newKey] = $attrText; + } + return strtr($filterString, $newAttrs); + } + + /** + * Get the groups a user is a part of on ldap + * @param string $userName + * @return array|null + */ + public function getUserGroups($userName) + { + $groupsAttr = $this->config['group_attribute']; + $user = $this->getUserWithAttributes($userName, [$groupsAttr]); + + if ($user === null) { + return null; + } + + $userGroups = $this->groupFilter($user); + $userGroups = $this->getGroupsRecursive($userGroups,[]); + return $userGroups; + } + + /** + * Get the parent groups of an array of groups + * @param array $groupsArray + * @param array $checked + * @return array + */ + private function getGroupsRecursive($groupsArray,$checked) { + $groups_to_add = []; + foreach ($groupsArray as $groupName) { + if (in_array($groupName,$checked)) continue; + + $groupsToAdd = $this->getGroupGroups($groupName); + $groups_to_add = array_merge($groups_to_add,$groupsToAdd); + $checked[] = $groupName; + } + $groupsArray = array_unique(array_merge($groupsArray,$groups_to_add), SORT_REGULAR); + + if (!empty($groups_to_add)) { + return $this->getGroupsRecursive($groupsArray,$checked); + } else { + return $groupsArray; + } + } + + /** + * Get the parent groups of a single group + * @param string $groupName + * @return array + */ + private function getGroupGroups($groupName) + { + $ldapConnection = $this->getConnection(); + $this->bindSystemUser($ldapConnection); + + $followReferrals = $this->config['follow_referrals'] ? 1 : 0; + $this->ldap->setOption($ldapConnection, LDAP_OPT_REFERRALS, $followReferrals); + + $baseDn = $this->config['base_dn']; + $groupsAttr = strtolower($this->config['group_attribute']); + + $groups = $this->ldap->searchAndGetEntries($ldapConnection, $baseDn, 'CN='.$groupName, [$groupsAttr]); + if ($groups['count'] === 0) { + return []; + } + + $groupGroups = $this->groupFilter($groups[0]); + return $groupGroups; + } + + /** + * Filter out LDAP CN and DN language in a ldap search return + * Gets the base CN (common name) of the string + * @param string $ldapSearchReturn + * @return array + */ + protected function groupFilter($ldapSearchReturn) + { + $groupsAttr = strtolower($this->config['group_attribute']); + $ldapGroups = []; + $count = 0; + if (isset($ldapSearchReturn[$groupsAttr]['count'])) $count = (int) $ldapSearchReturn[$groupsAttr]['count']; + for ($i=0;$i<$count;$i++) { + $dnComponents = ldap_explode_dn($ldapSearchReturn[$groupsAttr][$i],1); + if (!in_array($dnComponents[0],$ldapGroups)) { + $ldapGroups[] = $dnComponents[0]; + } + } + return $ldapGroups; + } + +} \ No newline at end of file diff --git a/config/services.php b/config/services.php index 825b1f109..daa160643 100644 --- a/config/services.php +++ b/config/services.php @@ -118,6 +118,10 @@ return [ 'version' => env('LDAP_VERSION', false), 'email_attribute' => env('LDAP_EMAIL_ATTRIBUTE', 'mail'), 'follow_referrals' => env('LDAP_FOLLOW_REFERRALS', false), - ] + 'user_to_groups' => env('LDAP_USER_TO_GROUPS',false), + 'group_attribute' => env('LDAP_GROUP_ATTRIBUTE', 'memberOf'), + 'admin' => env('LDAP_ADMIN_GROUP','Domain Admins'), + 'remove_from_groups' => env('LDAP_REMOVE_FROM_GROUPS',false), + ] ]; From 37aa8b05f873f0453f154bdf5df2774c595d5118 Mon Sep 17 00:00:00 2001 From: Brennan Murphy Date: Mon, 2 Jul 2018 17:27:43 +0000 Subject: [PATCH 2/5] Update files to PSR-2 standards --- app/Http/Controllers/Auth/LoginController.php | 12 +- app/Repos/LdapRepo.php | 130 ++--- app/Services/LdapService.php | 458 +++++++++--------- 3 files changed, 302 insertions(+), 298 deletions(-) diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 4c846d0e0..21c9dc4c5 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -97,14 +97,14 @@ class LoginController extends Controller auth()->login($user); } - // ldap groups refresh - if (config('services.ldap.user_to_groups') !== false && $request->filled('username')) { - $ldapRepo = new LdapRepo($this->userRepo); - $ldapRepo->syncGroups($user,$request->input('username')); - } + // ldap groups refresh + if (config('services.ldap.user_to_groups') !== false && $request->filled('username')) { + $ldapRepo = new LdapRepo($this->userRepo); + $ldapRepo->syncGroups($user, $request->input('username')); + } - $path = session()->pull('url.intended', '/'); + $path = session()->pull('url.intended', '/'); $path = baseUrl($path, true); return redirect($path); } diff --git a/app/Repos/LdapRepo.php b/app/Repos/LdapRepo.php index 33d05ea88..12bde0cdd 100644 --- a/app/Repos/LdapRepo.php +++ b/app/Repos/LdapRepo.php @@ -8,77 +8,77 @@ use BookStack\Repos\UserRepo; class LdapRepo { - protected $ldap = null; - protected $ldapService = null; + protected $ldap = null; + protected $ldapService = null; - protected $config; + protected $config; - /** - * LdapRepo constructor. - * @param \BookStack\Repos\UserRepo $userRepo - */ - public function __construct(UserRepo $userRepo) - { - $this->config = config('services.ldap'); + /** + * LdapRepo constructor. + * @param \BookStack\Repos\UserRepo $userRepo + */ + public function __construct(UserRepo $userRepo) + { + $this->config = config('services.ldap'); - if (config('auth.method') !== 'ldap') { - return false; - } + if (config('auth.method') !== 'ldap') { + return false; + } - $this->ldapService = new LdapService(new Ldap); - $this->userRepo = $userRepo; - } + $this->ldapService = new LdapService(new Ldap); + $this->userRepo = $userRepo; + } - /** - * If there is no ldap connection, all methods calls to this library will return null - */ - public function __call($method, $arguments) - { - if ($this->ldap === null) { - return null; - } + /** + * If there is no ldap connection, all methods calls to this library will return null + */ + public function __call($method, $arguments) + { + if ($this->ldap === null) { + return null; + } - return call_user_func_array(array($this,$method),$arguments); - } + return call_user_func_array(array($this,$method), $arguments); + } - /** - * Sync the LDAP groups to the user roles for the current user - * @param \BookStack\User $user - * @param string $userName - * @throws \BookStack\Exceptions\NotFoundException - */ - public function syncGroups($user,$userName) - { - $userLdapGroups = $this->ldapService->getUserGroups($userName); - $userLdapGroups = $this->groupNameFilter($userLdapGroups); - // get the ids for the roles from the names - $ldapGroupsAsRoles = Role::whereIn('name',$userLdapGroups)->pluck('id'); - // sync groups - if ($this->config['remove_from_groups']) { - $user->roles()->sync($ldapGroupsAsRoles); - $this->userRepo->attachDefaultRole($user); - } else { - $user->roles()->syncWithoutDetaching($ldapGroupsAsRoles); - } + /** + * Sync the LDAP groups to the user roles for the current user + * @param \BookStack\User $user + * @param string $userName + * @throws \BookStack\Exceptions\NotFoundException + */ + public function syncGroups($user, $userName) + { + $userLdapGroups = $this->ldapService->getUserGroups($userName); + $userLdapGroups = $this->groupNameFilter($userLdapGroups); + // get the ids for the roles from the names + $ldapGroupsAsRoles = Role::whereIn('name', $userLdapGroups)->pluck('id'); + // sync groups + if ($this->config['remove_from_groups']) { + $user->roles()->sync($ldapGroupsAsRoles); + $this->userRepo->attachDefaultRole($user); + } else { + $user->roles()->syncWithoutDetaching($ldapGroupsAsRoles); + } - // make the user an admin? - if (in_array($this->config['admin'],$userLdapGroups)) { - $this->userRepo->attachSystemRole($user,'admin'); - } - } + // make the user an admin? + if (in_array($this->config['admin'], $userLdapGroups)) { + $this->userRepo->attachSystemRole($user, 'admin'); + } + } - /** - * Filter to convert the groups from ldap to the format of the roles name on BookStack - * Spaces replaced with -, all lowercase letters - * @param array $groups - * @return array - */ - private function groupNameFilter($groups) - { - $return = []; - foreach ($groups as $groupName) { - $return[] = str_replace(' ', '-', strtolower($groupName)); - } - return $return; - } -} \ No newline at end of file + /** + * Filter to convert the groups from ldap to the format of the roles name on BookStack + * Spaces replaced with -, all lowercase letters + * @param array $groups + * @return array + */ + private function groupNameFilter($groups) + { + $return = []; + foreach ($groups as $groupName) { + $return[] = str_replace(' ', '-', strtolower($groupName)); + } + return $return; + } +} diff --git a/app/Services/LdapService.php b/app/Services/LdapService.php index e56f45e4e..d51b89409 100644 --- a/app/Services/LdapService.php +++ b/app/Services/LdapService.php @@ -11,263 +11,267 @@ use Illuminate\Contracts\Auth\Authenticatable; class LdapService { - protected $ldap; - protected $ldapConnection; - protected $config; + protected $ldap; + protected $ldapConnection; + protected $config; - /** - * LdapService constructor. - * @param Ldap $ldap - */ - public function __construct(Ldap $ldap) - { - $this->ldap = $ldap; - $this->config = config('services.ldap'); - } + /** + * LdapService constructor. + * @param Ldap $ldap + */ + public function __construct(Ldap $ldap) + { + $this->ldap = $ldap; + $this->config = config('services.ldap'); + } - /** - * Search for attributes for a specific user on the ldap - * @param string $userName - * @param array $attributes - * @return null|array - * @throws LdapException - */ - private function getUserWithAttributes($userName,$attributes) - { - $ldapConnection = $this->getConnection(); - $this->bindSystemUser($ldapConnection); + /** + * Search for attributes for a specific user on the ldap + * @param string $userName + * @param array $attributes + * @return null|array + * @throws LdapException + */ + private function getUserWithAttributes($userName, $attributes) + { + $ldapConnection = $this->getConnection(); + $this->bindSystemUser($ldapConnection); - // Find user - $userFilter = $this->buildFilter($this->config['user_filter'], ['user' => $userName]); - $baseDn = $this->config['base_dn']; + // Find user + $userFilter = $this->buildFilter($this->config['user_filter'], ['user' => $userName]); + $baseDn = $this->config['base_dn']; - $followReferrals = $this->config['follow_referrals'] ? 1 : 0; - $this->ldap->setOption($ldapConnection, LDAP_OPT_REFERRALS, $followReferrals); - $users = $this->ldap->searchAndGetEntries($ldapConnection, $baseDn, $userFilter, $attributes); - if ($users['count'] === 0) { - return null; - } + $followReferrals = $this->config['follow_referrals'] ? 1 : 0; + $this->ldap->setOption($ldapConnection, LDAP_OPT_REFERRALS, $followReferrals); + $users = $this->ldap->searchAndGetEntries($ldapConnection, $baseDn, $userFilter, $attributes); + if ($users['count'] === 0) { + return null; + } - return $users[0]; - } + return $users[0]; + } - /** - * Get the details of a user from LDAP using the given username. - * User found via configurable user filter. - * @param $userName - * @return array|null - * @throws LdapException - */ - public function getUserDetails($userName) - { - $emailAttr = $this->config['email_attribute']; - $user = $this->getUserWithAttributes($userName, ['cn', 'uid', 'dn', $emailAttr]); + /** + * Get the details of a user from LDAP using the given username. + * User found via configurable user filter. + * @param $userName + * @return array|null + * @throws LdapException + */ + public function getUserDetails($userName) + { + $emailAttr = $this->config['email_attribute']; + $user = $this->getUserWithAttributes($userName, ['cn', 'uid', 'dn', $emailAttr]); - if ($user === null) { - return null; - } + if ($user === null) { + return null; + } - return [ - 'uid' => (isset($user['uid'])) ? $user['uid'][0] : $user['dn'], - 'name' => $user['cn'][0], - 'dn' => $user['dn'], - 'email' => (isset($user[$emailAttr])) ? (is_array($user[$emailAttr]) ? $user[$emailAttr][0] : $user[$emailAttr]) : null - ]; - } + return [ + 'uid' => (isset($user['uid'])) ? $user['uid'][0] : $user['dn'], + 'name' => $user['cn'][0], + 'dn' => $user['dn'], + 'email' => (isset($user[$emailAttr])) ? (is_array($user[$emailAttr]) ? $user[$emailAttr][0] : $user[$emailAttr]) : null + ]; + } - /** - * @param Authenticatable $user - * @param string $username - * @param string $password - * @return bool - * @throws LdapException - */ - public function validateUserCredentials(Authenticatable $user, $username, $password) - { - $ldapUser = $this->getUserDetails($username); - if ($ldapUser === null) { - return false; - } - if ($ldapUser['uid'] !== $user->external_auth_id) { - return false; - } + /** + * @param Authenticatable $user + * @param string $username + * @param string $password + * @return bool + * @throws LdapException + */ + public function validateUserCredentials(Authenticatable $user, $username, $password) + { + $ldapUser = $this->getUserDetails($username); + if ($ldapUser === null) { + return false; + } + if ($ldapUser['uid'] !== $user->external_auth_id) { + return false; + } - $ldapConnection = $this->getConnection(); - try { - $ldapBind = $this->ldap->bind($ldapConnection, $ldapUser['dn'], $password); - } catch (\ErrorException $e) { - $ldapBind = false; - } + $ldapConnection = $this->getConnection(); + try { + $ldapBind = $this->ldap->bind($ldapConnection, $ldapUser['dn'], $password); + } catch (\ErrorException $e) { + $ldapBind = false; + } - return $ldapBind; - } + return $ldapBind; + } - /** - * Bind the system user to the LDAP connection using the given credentials - * otherwise anonymous access is attempted. - * @param $connection - * @throws LdapException - */ - protected function bindSystemUser($connection) - { - $ldapDn = $this->config['dn']; - $ldapPass = $this->config['pass']; + /** + * Bind the system user to the LDAP connection using the given credentials + * otherwise anonymous access is attempted. + * @param $connection + * @throws LdapException + */ + protected function bindSystemUser($connection) + { + $ldapDn = $this->config['dn']; + $ldapPass = $this->config['pass']; - $isAnonymous = ($ldapDn === false || $ldapPass === false); - if ($isAnonymous) { - $ldapBind = $this->ldap->bind($connection); - } else { - $ldapBind = $this->ldap->bind($connection, $ldapDn, $ldapPass); - } + $isAnonymous = ($ldapDn === false || $ldapPass === false); + if ($isAnonymous) { + $ldapBind = $this->ldap->bind($connection); + } else { + $ldapBind = $this->ldap->bind($connection, $ldapDn, $ldapPass); + } - if (!$ldapBind) { - throw new LdapException(($isAnonymous ? trans('errors.ldap_fail_anonymous') : trans('errors.ldap_fail_authed'))); - } - } + if (!$ldapBind) { + throw new LdapException(($isAnonymous ? trans('errors.ldap_fail_anonymous') : trans('errors.ldap_fail_authed'))); + } + } - /** - * Get the connection to the LDAP server. - * Creates a new connection if one does not exist. - * @return resource - * @throws LdapException - */ - protected function getConnection() - { - if ($this->ldapConnection !== null) { - return $this->ldapConnection; - } + /** + * Get the connection to the LDAP server. + * Creates a new connection if one does not exist. + * @return resource + * @throws LdapException + */ + protected function getConnection() + { + if ($this->ldapConnection !== null) { + return $this->ldapConnection; + } - // Check LDAP extension in installed - if (!function_exists('ldap_connect') && config('app.env') !== 'testing') { - throw new LdapException(trans('errors.ldap_extension_not_installed')); - } + // Check LDAP extension in installed + if (!function_exists('ldap_connect') && config('app.env') !== 'testing') { + throw new LdapException(trans('errors.ldap_extension_not_installed')); + } - // Get port from server string and protocol if specified. - $ldapServer = explode(':', $this->config['server']); - $hasProtocol = preg_match('/^ldaps{0,1}\:\/\//', $this->config['server']) === 1; - if (!$hasProtocol) { - array_unshift($ldapServer, ''); - } - $hostName = $ldapServer[0] . ($hasProtocol?':':'') . $ldapServer[1]; - $defaultPort = $ldapServer[0] === 'ldaps' ? 636 : 389; - $ldapConnection = $this->ldap->connect($hostName, count($ldapServer) > 2 ? intval($ldapServer[2]) : $defaultPort); + // Get port from server string and protocol if specified. + $ldapServer = explode(':', $this->config['server']); + $hasProtocol = preg_match('/^ldaps{0,1}\:\/\//', $this->config['server']) === 1; + if (!$hasProtocol) { + array_unshift($ldapServer, ''); + } + $hostName = $ldapServer[0] . ($hasProtocol?':':'') . $ldapServer[1]; + $defaultPort = $ldapServer[0] === 'ldaps' ? 636 : 389; + $ldapConnection = $this->ldap->connect($hostName, count($ldapServer) > 2 ? intval($ldapServer[2]) : $defaultPort); - if ($ldapConnection === false) { - throw new LdapException(trans('errors.ldap_cannot_connect')); - } + if ($ldapConnection === false) { + throw new LdapException(trans('errors.ldap_cannot_connect')); + } - // Set any required options - if ($this->config['version']) { - $this->ldap->setVersion($ldapConnection, $this->config['version']); - } + // Set any required options + if ($this->config['version']) { + $this->ldap->setVersion($ldapConnection, $this->config['version']); + } - $this->ldapConnection = $ldapConnection; - return $this->ldapConnection; - } + $this->ldapConnection = $ldapConnection; + return $this->ldapConnection; + } - /** - * Build a filter string by injecting common variables. - * @param string $filterString - * @param array $attrs - * @return string - */ - protected function buildFilter($filterString, array $attrs) - { - $newAttrs = []; - foreach ($attrs as $key => $attrText) { - $newKey = '${' . $key . '}'; - $newAttrs[$newKey] = $attrText; - } - return strtr($filterString, $newAttrs); - } + /** + * Build a filter string by injecting common variables. + * @param string $filterString + * @param array $attrs + * @return string + */ + protected function buildFilter($filterString, array $attrs) + { + $newAttrs = []; + foreach ($attrs as $key => $attrText) { + $newKey = '${' . $key . '}'; + $newAttrs[$newKey] = $attrText; + } + return strtr($filterString, $newAttrs); + } - /** - * Get the groups a user is a part of on ldap - * @param string $userName - * @return array|null - */ - public function getUserGroups($userName) - { - $groupsAttr = $this->config['group_attribute']; - $user = $this->getUserWithAttributes($userName, [$groupsAttr]); + /** + * Get the groups a user is a part of on ldap + * @param string $userName + * @return array|null + */ + public function getUserGroups($userName) + { + $groupsAttr = $this->config['group_attribute']; + $user = $this->getUserWithAttributes($userName, [$groupsAttr]); - if ($user === null) { - return null; - } + if ($user === null) { + return null; + } - $userGroups = $this->groupFilter($user); - $userGroups = $this->getGroupsRecursive($userGroups,[]); - return $userGroups; - } + $userGroups = $this->groupFilter($user); + $userGroups = $this->getGroupsRecursive($userGroups, []); + return $userGroups; + } - /** - * Get the parent groups of an array of groups - * @param array $groupsArray - * @param array $checked - * @return array - */ - private function getGroupsRecursive($groupsArray,$checked) { - $groups_to_add = []; - foreach ($groupsArray as $groupName) { - if (in_array($groupName,$checked)) continue; + /** + * Get the parent groups of an array of groups + * @param array $groupsArray + * @param array $checked + * @return array + */ + private function getGroupsRecursive($groupsArray, $checked) + { + $groups_to_add = []; + foreach ($groupsArray as $groupName) { + if (in_array($groupName, $checked)) { + continue; + } - $groupsToAdd = $this->getGroupGroups($groupName); - $groups_to_add = array_merge($groups_to_add,$groupsToAdd); - $checked[] = $groupName; - } - $groupsArray = array_unique(array_merge($groupsArray,$groups_to_add), SORT_REGULAR); + $groupsToAdd = $this->getGroupGroups($groupName); + $groups_to_add = array_merge($groups_to_add, $groupsToAdd); + $checked[] = $groupName; + } + $groupsArray = array_unique(array_merge($groupsArray, $groups_to_add), SORT_REGULAR); - if (!empty($groups_to_add)) { - return $this->getGroupsRecursive($groupsArray,$checked); - } else { - return $groupsArray; - } - } + if (!empty($groups_to_add)) { + return $this->getGroupsRecursive($groupsArray, $checked); + } else { + return $groupsArray; + } + } - /** - * Get the parent groups of a single group - * @param string $groupName - * @return array - */ - private function getGroupGroups($groupName) - { - $ldapConnection = $this->getConnection(); - $this->bindSystemUser($ldapConnection); + /** + * Get the parent groups of a single group + * @param string $groupName + * @return array + */ + private function getGroupGroups($groupName) + { + $ldapConnection = $this->getConnection(); + $this->bindSystemUser($ldapConnection); - $followReferrals = $this->config['follow_referrals'] ? 1 : 0; - $this->ldap->setOption($ldapConnection, LDAP_OPT_REFERRALS, $followReferrals); + $followReferrals = $this->config['follow_referrals'] ? 1 : 0; + $this->ldap->setOption($ldapConnection, LDAP_OPT_REFERRALS, $followReferrals); - $baseDn = $this->config['base_dn']; - $groupsAttr = strtolower($this->config['group_attribute']); + $baseDn = $this->config['base_dn']; + $groupsAttr = strtolower($this->config['group_attribute']); - $groups = $this->ldap->searchAndGetEntries($ldapConnection, $baseDn, 'CN='.$groupName, [$groupsAttr]); - if ($groups['count'] === 0) { - return []; - } + $groups = $this->ldap->searchAndGetEntries($ldapConnection, $baseDn, 'CN='.$groupName, [$groupsAttr]); + if ($groups['count'] === 0) { + return []; + } - $groupGroups = $this->groupFilter($groups[0]); - return $groupGroups; - } + $groupGroups = $this->groupFilter($groups[0]); + return $groupGroups; + } - /** - * Filter out LDAP CN and DN language in a ldap search return - * Gets the base CN (common name) of the string - * @param string $ldapSearchReturn - * @return array - */ - protected function groupFilter($ldapSearchReturn) - { - $groupsAttr = strtolower($this->config['group_attribute']); - $ldapGroups = []; - $count = 0; - if (isset($ldapSearchReturn[$groupsAttr]['count'])) $count = (int) $ldapSearchReturn[$groupsAttr]['count']; - for ($i=0;$i<$count;$i++) { - $dnComponents = ldap_explode_dn($ldapSearchReturn[$groupsAttr][$i],1); - if (!in_array($dnComponents[0],$ldapGroups)) { - $ldapGroups[] = $dnComponents[0]; - } - } - return $ldapGroups; - } - -} \ No newline at end of file + /** + * Filter out LDAP CN and DN language in a ldap search return + * Gets the base CN (common name) of the string + * @param string $ldapSearchReturn + * @return array + */ + protected function groupFilter($ldapSearchReturn) + { + $groupsAttr = strtolower($this->config['group_attribute']); + $ldapGroups = []; + $count = 0; + if (isset($ldapSearchReturn[$groupsAttr]['count'])) { + $count = (int) $ldapSearchReturn[$groupsAttr]['count']; + } + for ($i=0; $i<$count; $i++) { + $dnComponents = ldap_explode_dn($ldapSearchReturn[$groupsAttr][$i], 1); + if (!in_array($dnComponents[0], $ldapGroups)) { + $ldapGroups[] = $dnComponents[0]; + } + } + return $ldapGroups; + } +} From 17bca662a7757a66538a8074527b43b2ba20a592 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 15 Jul 2018 17:57:25 +0100 Subject: [PATCH 3/5] Added tests to cover ldap group mapping Also updated .env.example formatting. Updated how LdapRepo uses Ldap so can be mocked by testing. --- .env.example | 10 +- app/Http/Controllers/Auth/LoginController.php | 3 +- app/Repos/LdapRepo.php | 7 +- tests/Auth/LdapTest.php | 119 +++++++++++++++++- 4 files changed, 126 insertions(+), 13 deletions(-) diff --git a/.env.example b/.env.example index 57e6af6a9..4bb2555b0 100644 --- a/.env.example +++ b/.env.example @@ -67,14 +67,14 @@ LDAP_DN=false LDAP_PASS=false LDAP_USER_FILTER=false LDAP_VERSION=false -#do you want to sync LDAP groups to BookStack roles for a user +# Do you want to sync LDAP groups to BookStack roles for a user LDAP_USER_TO_GROUPS=false -#what is the LDAP attribute for group memberships +# What is the LDAP attribute for group memberships LDAP_GROUP_ATTRIBUTE="memberOf" -#what LDAP group should the user be a part of to be an admin on BookStack +# What LDAP group should the user be a part of to be an admin on BookStack LDAP_ADMIN_GROUP="Domain Admins" -#would you like to remove users from roles on bookstack if they do not match on LDAP -#if false, the ldap groups-roles sync will only add users to roles +# Would you like to remove users from roles on BookStack if they do not match on LDAP +# If false, the ldap groups-roles sync will only add users to roles LDAP_REMOVE_FROM_GROUPS=false # Mail settings diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 21c9dc4c5..08b1bce67 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -6,6 +6,7 @@ use BookStack\Exceptions\AuthException; use BookStack\Http\Controllers\Controller; use BookStack\Repos\UserRepo; use BookStack\Repos\LdapRepo; +use BookStack\Services\LdapService; use BookStack\Services\SocialAuthService; use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Foundation\Auth\AuthenticatesUsers; @@ -99,7 +100,7 @@ class LoginController extends Controller // ldap groups refresh if (config('services.ldap.user_to_groups') !== false && $request->filled('username')) { - $ldapRepo = new LdapRepo($this->userRepo); + $ldapRepo = new LdapRepo($this->userRepo, app(LdapService::class)); $ldapRepo->syncGroups($user, $request->input('username')); } diff --git a/app/Repos/LdapRepo.php b/app/Repos/LdapRepo.php index 12bde0cdd..e57872039 100644 --- a/app/Repos/LdapRepo.php +++ b/app/Repos/LdapRepo.php @@ -1,9 +1,7 @@ config = config('services.ldap'); @@ -25,7 +24,7 @@ class LdapRepo return false; } - $this->ldapService = new LdapService(new Ldap); + $this->ldapService = $ldapService; $this->userRepo = $userRepo; } diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 8880c7b65..ada33d692 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -1,10 +1,17 @@ set(['auth.method' => 'ldap', 'services.ldap.base_dn' => 'dc=ldap,dc=local', 'auth.providers.users.driver' => 'ldap']); - $this->mockLdap = \Mockery::mock(\BookStack\Services\Ldap::class); - $this->app['BookStack\Services\Ldap'] = $this->mockLdap; + app('config')->set([ + 'auth.method' => 'ldap', + 'services.ldap.base_dn' => 'dc=ldap,dc=local', + 'services.ldap.email_attribute' => 'mail', + 'services.ldap.user_to_groups' => false, + 'auth.providers.users.driver' => 'ldap', + ]); + $this->mockLdap = \Mockery::mock(Ldap::class); + $this->app[Ldap::class] = $this->mockLdap; $this->mockUser = factory(User::class)->make(); } @@ -133,4 +146,104 @@ class LdapTest extends BrowserKitTest ->dontSee('External Authentication'); } + public function test_login_maps_roles_and_retains_existsing_roles() + { + $roleToRecieve = factory(Role::class)->create(['name' => 'ldaptester']); + $roleToRecieve2 = factory(Role::class)->create(['name' => 'ldaptester-second']); + $existingRole = factory(Role::class)->create(['name' => 'ldaptester-existing']); + $this->mockUser->forceFill(['external_auth_id' => $this->mockUser->name])->save(); + $this->mockUser->attachRole($existingRole); + + app('config')->set([ + 'services.ldap.user_to_groups' => true, + '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) + ->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], + 'memberof' => [ + 'count' => 2, + 0 => "cn=ldaptester,ou=groups,dc=example,dc=com", + 1 => "cn=ldaptester-second,ou=groups,dc=example,dc=com", + ] + ]]); + $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); + + $this->visit('/login') + ->see('Username') + ->type($this->mockUser->name, '#username') + ->type($this->mockUser->password, '#password') + ->press('Log In') + ->seePageIs('/'); + + $user = User::where('email', $this->mockUser->email)->first(); + $this->seeInDatabase('role_user', [ + 'user_id' => $user->id, + 'role_id' => $roleToRecieve->id + ]); + $this->seeInDatabase('role_user', [ + 'user_id' => $user->id, + 'role_id' => $roleToRecieve2->id + ]); + $this->seeInDatabase('role_user', [ + 'user_id' => $user->id, + 'role_id' => $existingRole->id + ]); + } + + public function test_login_maps_roles_and_removes_old_roles_if_set() + { + $roleToRecieve = factory(Role::class)->create(['name' => 'ldaptester']); + $existingRole = factory(Role::class)->create(['name' => 'ldaptester-existing']); + $this->mockUser->forceFill(['external_auth_id' => $this->mockUser->name])->save(); + $this->mockUser->attachRole($existingRole); + + app('config')->set([ + 'services.ldap.user_to_groups' => true, + '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) + ->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], + 'memberof' => [ + 'count' => 1, + 0 => "cn=ldaptester,ou=groups,dc=example,dc=com", + ] + ]]); + $this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true); + + $this->visit('/login') + ->see('Username') + ->type($this->mockUser->name, '#username') + ->type($this->mockUser->password, '#password') + ->press('Log In') + ->seePageIs('/'); + + $user = User::where('email', $this->mockUser->email)->first(); + $this->seeInDatabase('role_user', [ + 'user_id' => $user->id, + 'role_id' => $roleToRecieve->id + ]); + $this->dontSeeInDatabase('role_user', [ + 'user_id' => $user->id, + 'role_id' => $existingRole->id + ]); + } + } From be2ca9d4bbc8850ab70fce9549cf1087d4e4d721 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 15 Jul 2018 18:21:45 +0100 Subject: [PATCH 4/5] Refactored out the LDAP repo --- app/Http/Controllers/Auth/LoginController.php | 15 ++-- app/Repos/LdapRepo.php | 83 ------------------- app/Services/LdapService.php | 66 ++++++++++++++- 3 files changed, 73 insertions(+), 91 deletions(-) delete mode 100644 app/Repos/LdapRepo.php diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 08b1bce67..e011c642f 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -5,7 +5,6 @@ namespace BookStack\Http\Controllers\Auth; use BookStack\Exceptions\AuthException; use BookStack\Http\Controllers\Controller; use BookStack\Repos\UserRepo; -use BookStack\Repos\LdapRepo; use BookStack\Services\LdapService; use BookStack\Services\SocialAuthService; use Illuminate\Contracts\Auth\Authenticatable; @@ -38,18 +37,21 @@ class LoginController extends Controller protected $redirectAfterLogout = '/login'; protected $socialAuthService; + protected $ldapService; protected $userRepo; /** * Create a new controller instance. * * @param SocialAuthService $socialAuthService + * @param LdapService $ldapService * @param UserRepo $userRepo */ - public function __construct(SocialAuthService $socialAuthService, UserRepo $userRepo) + public function __construct(SocialAuthService $socialAuthService, LdapService $ldapService, UserRepo $userRepo) { $this->middleware('guest', ['only' => ['getLogin', 'postLogin']]); $this->socialAuthService = $socialAuthService; + $this->ldapService = $ldapService; $this->userRepo = $userRepo; $this->redirectPath = baseUrl('/'); $this->redirectAfterLogout = baseUrl('/login'); @@ -98,13 +100,11 @@ class LoginController extends Controller auth()->login($user); } - // ldap groups refresh - if (config('services.ldap.user_to_groups') !== false && $request->filled('username')) { - $ldapRepo = new LdapRepo($this->userRepo, app(LdapService::class)); - $ldapRepo->syncGroups($user, $request->input('username')); + // Sync LDAP groups if required + if ($this->ldapService->shouldSyncGroups()) { + $this->ldapService->syncGroups($user); } - $path = session()->pull('url.intended', '/'); $path = baseUrl($path, true); return redirect($path); @@ -134,6 +134,7 @@ class LoginController extends Controller * Redirect to the relevant social site. * @param $socialDriver * @return \Symfony\Component\HttpFoundation\RedirectResponse + * @throws \BookStack\Exceptions\SocialDriverNotConfigured */ public function getSocialLogin($socialDriver) { diff --git a/app/Repos/LdapRepo.php b/app/Repos/LdapRepo.php deleted file mode 100644 index e57872039..000000000 --- a/app/Repos/LdapRepo.php +++ /dev/null @@ -1,83 +0,0 @@ -config = config('services.ldap'); - - if (config('auth.method') !== 'ldap') { - return false; - } - - $this->ldapService = $ldapService; - $this->userRepo = $userRepo; - } - - /** - * If there is no ldap connection, all methods calls to this library will return null - */ - public function __call($method, $arguments) - { - if ($this->ldap === null) { - return null; - } - - return call_user_func_array(array($this,$method), $arguments); - } - - /** - * Sync the LDAP groups to the user roles for the current user - * @param \BookStack\User $user - * @param string $userName - * @throws \BookStack\Exceptions\NotFoundException - */ - public function syncGroups($user, $userName) - { - $userLdapGroups = $this->ldapService->getUserGroups($userName); - $userLdapGroups = $this->groupNameFilter($userLdapGroups); - // get the ids for the roles from the names - $ldapGroupsAsRoles = Role::whereIn('name', $userLdapGroups)->pluck('id'); - // sync groups - if ($this->config['remove_from_groups']) { - $user->roles()->sync($ldapGroupsAsRoles); - $this->userRepo->attachDefaultRole($user); - } else { - $user->roles()->syncWithoutDetaching($ldapGroupsAsRoles); - } - - // make the user an admin? - if (in_array($this->config['admin'], $userLdapGroups)) { - $this->userRepo->attachSystemRole($user, 'admin'); - } - } - - /** - * Filter to convert the groups from ldap to the format of the roles name on BookStack - * Spaces replaced with -, all lowercase letters - * @param array $groups - * @return array - */ - private function groupNameFilter($groups) - { - $return = []; - foreach ($groups as $groupName) { - $return[] = str_replace(' ', '-', strtolower($groupName)); - } - return $return; - } -} diff --git a/app/Services/LdapService.php b/app/Services/LdapService.php index d51b89409..7606f1271 100644 --- a/app/Services/LdapService.php +++ b/app/Services/LdapService.php @@ -1,6 +1,9 @@ ldap = $ldap; $this->config = config('services.ldap'); + $this->userRepo = $userRepo; + $this->enabled = config('auth.method') === 'ldap'; + } + + /** + * Check if groups should be synced. + * @return bool + */ + public function shouldSyncGroups() + { + return $this->enabled && $this->config['user_to_groups'] !== false; } /** @@ -185,6 +202,7 @@ class LdapService * Get the groups a user is a part of on ldap * @param string $userName * @return array|null + * @throws LdapException */ public function getUserGroups($userName) { @@ -205,6 +223,7 @@ class LdapService * @param array $groupsArray * @param array $checked * @return array + * @throws LdapException */ private function getGroupsRecursive($groupsArray, $checked) { @@ -231,6 +250,7 @@ class LdapService * Get the parent groups of a single group * @param string $groupName * @return array + * @throws LdapException */ private function getGroupGroups($groupName) { @@ -274,4 +294,48 @@ class LdapService } return $ldapGroups; } + + /** + * Sync the LDAP groups to the user roles for the current user + * @param \BookStack\User $user + * @throws LdapException + * @throws \BookStack\Exceptions\NotFoundException + */ + public function syncGroups(User $user) + { + $userLdapGroups = $this->getUserGroups($user->external_auth_id); + $userLdapGroups = $this->groupNameFilter($userLdapGroups); + + // Get the ids for the roles from the names + $ldapGroupsAsRoles = Role::query()->whereIn('name', $userLdapGroups)->pluck('id'); + + // Sync groups + if ($this->config['remove_from_groups']) { + $user->roles()->sync($ldapGroupsAsRoles); + $this->userRepo->attachDefaultRole($user); + } else { + $user->roles()->syncWithoutDetaching($ldapGroupsAsRoles); + } + + // make the user an admin? + // TODO - Remove + if (in_array($this->config['admin'], $userLdapGroups)) { + $this->userRepo->attachSystemRole($user, 'admin'); + } + } + + /** + * Filter to convert the groups from ldap to the format of the roles name on BookStack + * Spaces replaced with -, all lowercase letters + * @param array $groups + * @return array + */ + private function groupNameFilter(array $groups) + { + $return = []; + foreach ($groups as $groupName) { + $return[] = str_replace(' ', '-', strtolower($groupName)); + } + return $return; + } } From f421d83627494d1aae9dab29cb5c5bf7d1ea86ac Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 15 Jul 2018 19:34:42 +0100 Subject: [PATCH 5/5] Added ability to set custom ldap group -> role mapping Added input in role form to allow matching against custom names. Changed default mapping to use role display name instead of the hidden DB name. --- .env.example | 2 - app/Http/Controllers/PermissionController.php | 1 + app/Role.php | 2 +- app/Services/LdapService.php | 64 +++++++++++++------ config/services.php | 1 - ...07_15_173514_add_role_external_auth_id.php | 33 ++++++++++ resources/lang/en/settings.php | 1 + resources/views/settings/roles/form.blade.php | 8 +++ tests/Auth/LdapTest.php | 64 +++++++++++++++++-- 9 files changed, 148 insertions(+), 28 deletions(-) create mode 100644 database/migrations/2018_07_15_173514_add_role_external_auth_id.php diff --git a/.env.example b/.env.example index 4bb2555b0..d09db93d3 100644 --- a/.env.example +++ b/.env.example @@ -71,8 +71,6 @@ LDAP_VERSION=false LDAP_USER_TO_GROUPS=false # What is the LDAP attribute for group memberships LDAP_GROUP_ATTRIBUTE="memberOf" -# What LDAP group should the user be a part of to be an admin on BookStack -LDAP_ADMIN_GROUP="Domain Admins" # Would you like to remove users from roles on BookStack if they do not match on LDAP # If false, the ldap groups-roles sync will only add users to roles LDAP_REMOVE_FROM_GROUPS=false diff --git a/app/Http/Controllers/PermissionController.php b/app/Http/Controllers/PermissionController.php index c4c7fe972..5695705d0 100644 --- a/app/Http/Controllers/PermissionController.php +++ b/app/Http/Controllers/PermissionController.php @@ -78,6 +78,7 @@ class PermissionController extends Controller * @param $id * @param Request $request * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector + * @throws PermissionsException */ public function updateRole($id, Request $request) { diff --git a/app/Role.php b/app/Role.php index e86854e79..54a5bb180 100644 --- a/app/Role.php +++ b/app/Role.php @@ -3,7 +3,7 @@ class Role extends Model { - protected $fillable = ['display_name', 'description']; + protected $fillable = ['display_name', 'description', 'external_auth_id']; /** * The roles that belong to the role. diff --git a/app/Services/LdapService.php b/app/Services/LdapService.php index 7606f1271..4936b2da8 100644 --- a/app/Services/LdapService.php +++ b/app/Services/LdapService.php @@ -5,6 +5,7 @@ use BookStack\Repos\UserRepo; use BookStack\Role; use BookStack\User; use Illuminate\Contracts\Auth\Authenticatable; +use Illuminate\Database\Eloquent\Builder; /** * Class LdapService @@ -299,15 +300,13 @@ class LdapService * Sync the LDAP groups to the user roles for the current user * @param \BookStack\User $user * @throws LdapException - * @throws \BookStack\Exceptions\NotFoundException */ public function syncGroups(User $user) { $userLdapGroups = $this->getUserGroups($user->external_auth_id); - $userLdapGroups = $this->groupNameFilter($userLdapGroups); // Get the ids for the roles from the names - $ldapGroupsAsRoles = Role::query()->whereIn('name', $userLdapGroups)->pluck('id'); + $ldapGroupsAsRoles = $this->matchLdapGroupsToSystemsRoles($userLdapGroups); // Sync groups if ($this->config['remove_from_groups']) { @@ -316,26 +315,55 @@ class LdapService } else { $user->roles()->syncWithoutDetaching($ldapGroupsAsRoles); } - - // make the user an admin? - // TODO - Remove - if (in_array($this->config['admin'], $userLdapGroups)) { - $this->userRepo->attachSystemRole($user, 'admin'); - } } /** - * Filter to convert the groups from ldap to the format of the roles name on BookStack - * Spaces replaced with -, all lowercase letters - * @param array $groups - * @return array + * Match an array of group names from LDAP to BookStack system roles. + * Formats LDAP group names to be lower-case and hyphenated. + * @param array $groupNames + * @return \Illuminate\Support\Collection */ - private function groupNameFilter(array $groups) + protected function matchLdapGroupsToSystemsRoles(array $groupNames) { - $return = []; - foreach ($groups as $groupName) { - $return[] = str_replace(' ', '-', strtolower($groupName)); + foreach ($groupNames as $i => $groupName) { + $groupNames[$i] = str_replace(' ', '-', trim(strtolower($groupName))); } - return $return; + + $roles = Role::query()->where(function(Builder $query) use ($groupNames) { + $query->whereIn('name', $groupNames); + foreach ($groupNames as $groupName) { + $query->orWhere('external_auth_id', 'LIKE', '%' . $groupName . '%'); + } + })->get(); + + $matchedRoles = $roles->filter(function(Role $role) use ($groupNames) { + return $this->roleMatchesGroupNames($role, $groupNames); + }); + + return $matchedRoles->pluck('id'); } + + /** + * Check a role against an array of group names to see if it matches. + * Checked against role 'external_auth_id' if set otherwise the name of the role. + * @param Role $role + * @param array $groupNames + * @return bool + */ + protected function roleMatchesGroupNames(Role $role, array $groupNames) + { + if ($role->external_auth_id) { + $externalAuthIds = explode(',', strtolower($role->external_auth_id)); + foreach ($externalAuthIds as $externalAuthId) { + if (in_array(trim($externalAuthId), $groupNames)) { + return true; + } + } + return false; + } + + $roleName = str_replace(' ', '-', trim(strtolower($role->display_name))); + return in_array($roleName, $groupNames); + } + } diff --git a/config/services.php b/config/services.php index daa160643..9c550f2fa 100644 --- a/config/services.php +++ b/config/services.php @@ -120,7 +120,6 @@ return [ 'follow_referrals' => env('LDAP_FOLLOW_REFERRALS', false), 'user_to_groups' => env('LDAP_USER_TO_GROUPS',false), 'group_attribute' => env('LDAP_GROUP_ATTRIBUTE', 'memberOf'), - 'admin' => env('LDAP_ADMIN_GROUP','Domain Admins'), 'remove_from_groups' => env('LDAP_REMOVE_FROM_GROUPS',false), ] diff --git a/database/migrations/2018_07_15_173514_add_role_external_auth_id.php b/database/migrations/2018_07_15_173514_add_role_external_auth_id.php new file mode 100644 index 000000000..706a883a3 --- /dev/null +++ b/database/migrations/2018_07_15_173514_add_role_external_auth_id.php @@ -0,0 +1,33 @@ +string('external_auth_id', 200)->default(''); + $table->index('external_auth_id'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('roles', function (Blueprint $table) { + $table->dropColumn('external_auth_id'); + }); + } +} diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php index 30abbc1b9..d6fbb6107 100755 --- a/resources/lang/en/settings.php +++ b/resources/lang/en/settings.php @@ -82,6 +82,7 @@ return [ 'role_details' => 'Role Details', 'role_name' => 'Role Name', 'role_desc' => 'Short Description of Role', + 'role_external_auth_id' => 'External Authentication IDs', 'role_system' => 'System Permissions', 'role_manage_users' => 'Manage users', 'role_manage_roles' => 'Manage roles & role permissions', diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php index 15ce7e6d7..6a8e27487 100644 --- a/resources/views/settings/roles/form.blade.php +++ b/resources/views/settings/roles/form.blade.php @@ -15,6 +15,14 @@ @include('form/text', ['name' => 'description']) + + @if(config('auth.method') === 'ldap') +
+ + @include('form/text', ['name' => 'external_auth_id']) +
+ @endif +
{{ trans('settings.role_system') }}
diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index ada33d692..64f3fd742 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -148,8 +148,8 @@ class LdapTest extends BrowserKitTest public function test_login_maps_roles_and_retains_existsing_roles() { - $roleToRecieve = factory(Role::class)->create(['name' => 'ldaptester']); - $roleToRecieve2 = factory(Role::class)->create(['name' => 'ldaptester-second']); + $roleToReceive = factory(Role::class)->create(['name' => 'ldaptester', 'display_name' => 'LdapTester']); + $roleToReceive2 = factory(Role::class)->create(['name' => 'ldaptester-second', 'display_name' => 'LdapTester Second']); $existingRole = factory(Role::class)->create(['name' => 'ldaptester-existing']); $this->mockUser->forceFill(['external_auth_id' => $this->mockUser->name])->save(); $this->mockUser->attachRole($existingRole); @@ -187,11 +187,11 @@ class LdapTest extends BrowserKitTest $user = User::where('email', $this->mockUser->email)->first(); $this->seeInDatabase('role_user', [ 'user_id' => $user->id, - 'role_id' => $roleToRecieve->id + 'role_id' => $roleToReceive->id ]); $this->seeInDatabase('role_user', [ 'user_id' => $user->id, - 'role_id' => $roleToRecieve2->id + 'role_id' => $roleToReceive2->id ]); $this->seeInDatabase('role_user', [ 'user_id' => $user->id, @@ -201,7 +201,7 @@ class LdapTest extends BrowserKitTest public function test_login_maps_roles_and_removes_old_roles_if_set() { - $roleToRecieve = factory(Role::class)->create(['name' => 'ldaptester']); + $roleToReceive = factory(Role::class)->create(['name' => 'ldaptester', 'display_name' => 'LdapTester']); $existingRole = factory(Role::class)->create(['name' => 'ldaptester-existing']); $this->mockUser->forceFill(['external_auth_id' => $this->mockUser->name])->save(); $this->mockUser->attachRole($existingRole); @@ -238,7 +238,7 @@ class LdapTest extends BrowserKitTest $user = User::where('email', $this->mockUser->email)->first(); $this->seeInDatabase('role_user', [ 'user_id' => $user->id, - 'role_id' => $roleToRecieve->id + 'role_id' => $roleToReceive->id ]); $this->dontSeeInDatabase('role_user', [ 'user_id' => $user->id, @@ -246,4 +246,56 @@ class LdapTest extends BrowserKitTest ]); } + public function test_external_auth_id_visible_in_roles_page_when_ldap_active() + { + $role = factory(Role::class)->create(['name' => 'ldaptester', 'external_auth_id' => 'ex-auth-a, test-second-param']); + $this->asAdmin()->visit('/settings/roles/' . $role->id) + ->see('ex-auth-a'); + } + + public function test_login_maps_roles_using_external_auth_ids_if_set() + { + $roleToReceive = factory(Role::class)->create(['name' => 'ldaptester', 'external_auth_id' => 'test-second-param, ex-auth-a']); + $roleToNotReceive = factory(Role::class)->create(['name' => 'ldaptester-not-receive', '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.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) + ->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], + 'memberof' => [ + 'count' => 1, + 0 => "cn=ex-auth-a,ou=groups,dc=example,dc=com", + ] + ]]); + $this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true); + + $this->visit('/login') + ->see('Username') + ->type($this->mockUser->name, '#username') + ->type($this->mockUser->password, '#password') + ->press('Log In') + ->seePageIs('/'); + + $user = User::where('email', $this->mockUser->email)->first(); + $this->seeInDatabase('role_user', [ + 'user_id' => $user->id, + 'role_id' => $roleToReceive->id + ]); + $this->dontSeeInDatabase('role_user', [ + 'user_id' => $user->id, + 'role_id' => $roleToNotReceive->id + ]); + } + }