Added dump_user_details option to LDAP and added binary attribute decode option

Related to #1872

diff --git a/.env.example.complete b/.env.example.complete
index 04cd73b90cd378d2b105f3b99f3ba01d813c5c08..86a7351c215a675685eebeea1714982eaff5e6bb 100644 (file)
--- a/.env.example.complete
+++ b/.env.example.complete
@@ -200,6 +200,7 @@ LDAP_ID_ATTRIBUTE=uid
 LDAP_EMAIL_ATTRIBUTE=mail
 LDAP_DISPLAY_NAME_ATTRIBUTE=cn
 LDAP_FOLLOW_REFERRALS=true
+LDAP_DUMP_USER_DETAILS=false
 
 # LDAP group sync configuration
 # Refer to https://www.bookstackapp.com/docs/admin/ldap-auth/

diff --git a/app/Auth/Access/LdapService.php b/app/Auth/Access/LdapService.php
index 0d6ebfc802aa076d8237fe867afbc5da573f9641..d377705583ac9011384de7aee2ddc092a984a452 100644 (file)
--- a/app/Auth/Access/LdapService.php
+++ b/app/Auth/Access/LdapService.php
@@ -1,6 +1,7 @@
 <?php namespace BookStack\Auth\Access;
 
 use BookStack\Auth\User;
+use BookStack\Exceptions\JsonDebugException;
 use BookStack\Exceptions\LdapException;
 use ErrorException;
 
@@ -76,26 +77,47 @@ class LdapService extends ExternalAuthService
         }
 
         $userCn = $this->getUserResponseProperty($user, 'cn', null);
-        return [
+        $formatted = [
             'uid'   => $this->getUserResponseProperty($user, $idAttr, $user['dn']),
             'name'  => $this->getUserResponseProperty($user, $displayNameAttr, $userCn),
             'dn'    => $user['dn'],
             'email' => $this->getUserResponseProperty($user, $emailAttr, null),
         ];
+
+        if ($this->config['dump_user_details']) {
+            throw new JsonDebugException([
+                'details_from_ldap' => $user,
+                'details_bookstack_parsed' => $formatted,
+            ]);
+        }
+
+        return $formatted;
     }
 
     /**
      * Get a property from an LDAP user response fetch.
      * Handles properties potentially being part of an array.
+     * If the given key is prefixed with 'BIN;', that indicator will be stripped
+     * from the key and any fetched values will be converted from binary to hex.
      */
     protected function getUserResponseProperty(array $userDetails, string $propertyKey, $defaultValue)
     {
+        $isBinary = strpos($propertyKey, 'BIN;') === 0;
         $propertyKey = strtolower($propertyKey);
+        $value = $defaultValue;
+
+        if ($isBinary) {
+            $propertyKey = substr($propertyKey, strlen('BIN;'));
+        }
+
         if (isset($userDetails[$propertyKey])) {
-            return (is_array($userDetails[$propertyKey]) ? $userDetails[$propertyKey][0] : $userDetails[$propertyKey]);
+            $value = (is_array($userDetails[$propertyKey]) ? $userDetails[$propertyKey][0] : $userDetails[$propertyKey]);
+            if ($isBinary) {
+                $value = bin2hex($value);
+            }
         }
 
-        return $defaultValue;
+        return $value;
     }
 
     /**

diff --git a/app/Config/services.php b/app/Config/services.php
index a0bdd078a9f01318d2c335d341ba9c14a29c2c12..fcde621d2b5296bd9239085adc6409d7d39784a3 100644 (file)
--- a/app/Config/services.php
+++ b/app/Config/services.php
@@ -118,6 +118,7 @@ return [
 
     'ldap' => [
         'server' => env('LDAP_SERVER', false),
+        'dump_user_details' => env('LDAP_DUMP_USER_DETAILS', false),
         'dn' => env('LDAP_DN', false),
         'pass' => env('LDAP_PASS', false),
         'base_dn' => env('LDAP_BASE_DN', false),

diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php
index 06f88c222893e7ab3b84e4795609504d65ddd89e..f6c5997b3c46708aa698ab11f53455c0bbb13e0a 100644 (file)
--- a/tests/Auth/LdapTest.php
+++ b/tests/Auth/LdapTest.php
@@ -1,5 +1,6 @@
 <?php namespace Tests;
 
+use BookStack\Auth\Access\LdapService;
 use BookStack\Auth\Role;
 use BookStack\Auth\Access\Ldap;
 use BookStack\Auth\User;
@@ -20,7 +21,7 @@ class LdapTest extends BrowserKitTest
     {
         parent::setUp();
         if (!defined('LDAP_OPT_REFERRALS')) define('LDAP_OPT_REFERRALS', 1);
-        app('config')->set([
+        config()->set([
             'auth.method' => 'ldap',
             'auth.defaults.guard' => 'ldap',
             'services.ldap.base_dn' => 'dc=ldap,dc=local',
@@ -560,4 +561,53 @@ class LdapTest extends BrowserKitTest
         $resp = $this->post('/register');
         $this->assertPermissionError($resp);
     }
+
+    public function test_dump_user_details_option_works()
+    {
+        config()->set(['services.ldap.dump_user_details' => true]);
+
+        $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId);
+        $this->mockLdap->shouldReceive('setVersion')->once();
+        $this->mockLdap->shouldReceive('setOption')->times(1);
+        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
+            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->andReturn(['count' => 1, 0 => [
+                'uid' => [$this->mockUser->name],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')]
+            ]]);
+        $this->mockLdap->shouldReceive('bind')->times(1)->andReturn(true);
+        $this->mockEscapes(1);
+
+        $this->post('/login', [
+            'username' => $this->mockUser->name,
+            'password' => $this->mockUser->password,
+        ]);
+        $this->seeJsonStructure([
+            'details_from_ldap' => [],
+            'details_bookstack_parsed' => [],
+        ]);
+    }
+
+    public function test_ldap_attributes_can_be_binary_decoded_if_marked()
+    {
+        config()->set(['services.ldap.id_attribute' => 'BIN;uid']);
+        $ldapService = app()->make(LdapService::class);
+
+        $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId);
+        $this->mockLdap->shouldReceive('setVersion')->once();
+        $this->mockLdap->shouldReceive('setOption')->times(1);
+        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
+            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->andReturn(['count' => 1, 0 => [
+                'uid' => [hex2bin('FFF8F7')],
+                'cn' => [$this->mockUser->name],
+                'dn' => ['dc=test' . config('services.ldap.base_dn')]
+            ]]);
+        $this->mockLdap->shouldReceive('bind')->times(1)->andReturn(true);
+        $this->mockEscapes(1);
+
+        $details = $ldapService->getUserDetails('test');
+        $this->assertEquals('fff8f7', $details['uid']);
+    }
 }