diff --git a/app/Auth/Access/Guards/LdapSessionGuard.php b/app/Auth/Access/Guards/LdapSessionGuard.php
index a9802054a..71417aed2 100644
--- a/app/Auth/Access/Guards/LdapSessionGuard.php
+++ b/app/Auth/Access/Guards/LdapSessionGuard.php
@@ -90,6 +90,11 @@ class LdapSessionGuard extends ExternalBaseSessionGuard
$this->ldapService->syncGroups($user, $username);
}
+ // Attach avatar if non-existent
+ if (is_null($user->avatar)) {
+ $this->ldapService->saveAndAttachAvatar($user, $userDetails);
+ }
+
$this->login($user, $remember);
return true;
}
@@ -116,15 +121,7 @@ class LdapSessionGuard extends ExternalBaseSessionGuard
];
$user = $this->registrationService->registerUser($details, null, false);
-
- if (config('services.ldap.import_thumbnail_photos')) {
- $imageService = app()->make(ImageService::class);
- $image = $imageService->saveNewFromBase64Uri('data:image/jpg;base64,'.base64_encode($ldapUserDetails['avatar']), $ldapUserDetails['uid'].'.jpg', 'user');
-
- $user['image_id'] = $image->id;
- $user->save();
- }
-
+ $this->ldapService->saveAndAttachAvatar($user, $ldapUserDetails);
return $user;
}
}
diff --git a/app/Auth/Access/LdapService.php b/app/Auth/Access/LdapService.php
index c5b586b4d..2f632b0b5 100644
--- a/app/Auth/Access/LdapService.php
+++ b/app/Auth/Access/LdapService.php
@@ -3,7 +3,9 @@
use BookStack\Auth\User;
use BookStack\Exceptions\JsonDebugException;
use BookStack\Exceptions\LdapException;
+use BookStack\Uploads\UserAvatars;
use ErrorException;
+use Illuminate\Support\Facades\Log;
/**
* Class LdapService
@@ -14,15 +16,17 @@ class LdapService extends ExternalAuthService
protected $ldap;
protected $ldapConnection;
+ protected $userAvatars;
protected $config;
protected $enabled;
/**
* LdapService constructor.
*/
- public function __construct(Ldap $ldap)
+ public function __construct(Ldap $ldap, UserAvatars $userAvatars)
{
$this->ldap = $ldap;
+ $this->userAvatars = $userAvatars;
$this->config = config('services.ldap');
$this->enabled = config('auth.method') === 'ldap';
}
@@ -78,9 +82,11 @@ class LdapService extends ExternalAuthService
$displayNameAttr = $this->config['display_name_attribute'];
$thumbnailAttr = $this->config['thumbnail_attribute'];
- $user = $this->getUserWithAttributes($userName, ['cn', 'dn', $idAttr, $emailAttr, $displayNameAttr]);
+ $user = $this->getUserWithAttributes($userName, array_filter([
+ 'cn', 'dn', $idAttr, $emailAttr, $displayNameAttr, $thumbnailAttr,
+ ]));
- if ($user === null) {
+ if (is_null($user)) {
return null;
}
@@ -90,7 +96,7 @@ class LdapService extends ExternalAuthService
'name' => $this->getUserResponseProperty($user, $displayNameAttr, $userCn),
'dn' => $user['dn'],
'email' => $this->getUserResponseProperty($user, $emailAttr, null),
- 'avatar'=> $this->getUserResponseProperty($user, $thumbnailAttr, null),
+ 'avatar'=> $thumbnailAttr ? $this->getUserResponseProperty($user, $thumbnailAttr, null) : null,
];
if ($this->config['dump_user_details']) {
@@ -352,4 +358,22 @@ class LdapService extends ExternalAuthService
$userLdapGroups = $this->getUserGroups($username);
$this->syncWithGroups($user, $userLdapGroups);
}
+
+ /**
+ * Save and attach an avatar image, if found in the ldap details, and attach
+ * to the given user model.
+ */
+ public function saveAndAttachAvatar(User $user, array $ldapUserDetails): void
+ {
+ if (is_null(config('services.ldap.thumbnail_attribute')) || is_null($ldapUserDetails['avatar'])) {
+ return;
+ }
+
+ try {
+ $imageData = $ldapUserDetails['avatar'];
+ $this->userAvatars->assignToUserFromExistingData($user, $imageData, 'jpg');
+ } catch (\Exception $exception) {
+ Log::info("Failed to use avatar image from LDAP data for user id {$user->id}");
+ }
+ }
}
diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php
index aa7653411..15ce2cbc9 100644
--- a/app/Auth/UserRepo.php
+++ b/app/Auth/UserRepo.php
@@ -8,13 +8,11 @@ use BookStack\Entities\Models\Chapter;
use BookStack\Entities\Models\Page;
use BookStack\Exceptions\NotFoundException;
use BookStack\Exceptions\UserUpdateException;
-use BookStack\Uploads\Image;
use BookStack\Uploads\UserAvatars;
use Exception;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Pagination\LengthAwarePaginator;
-use Images;
use Log;
class UserRepo
@@ -188,13 +186,7 @@ class UserRepo
$user->delete();
// Delete user profile images
- $profileImages = Image::query()->where('type', '=', 'user')
- ->where('uploaded_to', '=', $user->id)
- ->get();
-
- foreach ($profileImages as $image) {
- Images::destroy($image);
- }
+ $this->userAvatar->destroyAllForUser($user);
if (!empty($newOwnerId)) {
$newOwner = User::query()->find($newOwnerId);
diff --git a/app/Config/app.php b/app/Config/app.php
index 88f38423a..ea5252f4f 100755
--- a/app/Config/app.php
+++ b/app/Config/app.php
@@ -184,7 +184,6 @@ return [
// Custom BookStack
'Activity' => BookStack\Facades\Activity::class,
- 'Images' => BookStack\Facades\Images::class,
'Permissions' => BookStack\Facades\Permissions::class,
'Theme' => BookStack\Facades\Theme::class,
],
diff --git a/app/Config/services.php b/app/Config/services.php
index 7fac1f51c..0f9f78d1e 100644
--- a/app/Config/services.php
+++ b/app/Config/services.php
@@ -133,8 +133,7 @@ return [
'remove_from_groups' => env('LDAP_REMOVE_FROM_GROUPS', false),
'tls_insecure' => env('LDAP_TLS_INSECURE', false),
'start_tls' => env('LDAP_START_TLS', false),
- 'import_thumbnail_photos' => env('LDAP_IMPORT_THUMBNAIL_PHOTOS', false),
- 'thumbnail_attribute' => env('LDAP_THUMBNAIL_ATTRIBUTE', 'thumbnailPhoto'),
+ 'thumbnail_attribute' => env('LDAP_THUMBNAIL_ATTRIBUTE', null),
],
];
diff --git a/app/Facades/Images.php b/app/Facades/Images.php
deleted file mode 100644
index fdbd35a99..000000000
--- a/app/Facades/Images.php
+++ /dev/null
@@ -1,16 +0,0 @@
-<?php namespace BookStack\Facades;
-
-use Illuminate\Support\Facades\Facade;
-
-class Images extends Facade
-{
- /**
- * Get the registered name of the component.
- *
- * @return string
- */
- protected static function getFacadeAccessor()
- {
- return 'images';
- }
-}
diff --git a/app/Uploads/Image.php b/app/Uploads/Image.php
index dc26af002..ca1df3c6c 100644
--- a/app/Uploads/Image.php
+++ b/app/Uploads/Image.php
@@ -3,7 +3,6 @@
use BookStack\Entities\Models\Page;
use BookStack\Model;
use BookStack\Traits\HasCreatorAndUpdater;
-use Images;
class Image extends Model
{
@@ -14,23 +13,18 @@ class Image extends Model
/**
* Get a thumbnail for this image.
- * @param int $width
- * @param int $height
- * @param bool|false $keepRatio
- * @return string
* @throws \Exception
*/
- public function getThumb($width, $height, $keepRatio = false)
+ public function getThumb(int $width, int $height, bool $keepRatio = false): string
{
- return Images::getThumbnail($this, $width, $height, $keepRatio);
+ return app()->make(ImageService::class)->getThumbnail($this, $width, $height, $keepRatio);
}
/**
* Get the page this image has been uploaded to.
* Only applicable to gallery or drawio image types.
- * @return Page|null
*/
- public function getPage()
+ public function getPage(): ?Page
{
return $this->belongsTo(Page::class, 'uploaded_to')->first();
}
diff --git a/app/Uploads/UserAvatars.php b/app/Uploads/UserAvatars.php
index f1509bbb8..e98c1cfca 100644
--- a/app/Uploads/UserAvatars.php
+++ b/app/Uploads/UserAvatars.php
@@ -26,6 +26,7 @@ class UserAvatars
}
try {
+ $this->destroyAllForUser($user);
$avatar = $this->saveAvatarImage($user);
$user->avatar()->associate($avatar);
$user->save();
@@ -34,6 +35,35 @@ class UserAvatars
}
}
+ /**
+ * Assign a new avatar image to the given user using the given image data.
+ */
+ public function assignToUserFromExistingData(User $user, string $imageData, string $extension): void
+ {
+ try {
+ $this->destroyAllForUser($user);
+ $avatar = $this->createAvatarImageFromData($user, $imageData, $extension);
+ $user->avatar()->associate($avatar);
+ $user->save();
+ } catch (Exception $e) {
+ Log::error('Failed to save user avatar image');
+ }
+ }
+
+ /**
+ * Destroy all user avatars uploaded to the given user.
+ */
+ public function destroyAllForUser(User $user)
+ {
+ $profileImages = Image::query()->where('type', '=', 'user')
+ ->where('uploaded_to', '=', $user->id)
+ ->get();
+
+ foreach ($profileImages as $image) {
+ $this->imageService->destroy($image);
+ }
+ }
+
/**
* Save an avatar image from an external service.
* @throws Exception
@@ -50,8 +80,16 @@ class UserAvatars
];
$userAvatarUrl = strtr($avatarUrl, $replacements);
- $imageName = str_replace(' ', '-', $user->id . '-avatar.png');
$imageData = $this->getAvatarImageData($userAvatarUrl);
+ return $this->createAvatarImageFromData($user, $imageData, 'png');
+ }
+
+ /**
+ * Creates a new image instance and saves it in the system as a new user avatar image.
+ */
+ protected function createAvatarImageFromData(User $user, string $imageData, string $extension): Image
+ {
+ $imageName = str_replace(' ', '-', $user->id . '-avatar.' . $extension);
$image = $this->imageService->saveNew($imageName, $imageData, 'user', $user->id);
$image->created_by = $user->id;
diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php
index 840dfd630..a04ed4400 100644
--- a/tests/Auth/LdapTest.php
+++ b/tests/Auth/LdapTest.php
@@ -4,7 +4,6 @@ use BookStack\Auth\Access\LdapService;
use BookStack\Auth\Role;
use BookStack\Auth\Access\Ldap;
use BookStack\Auth\User;
-use BookStack\Exceptions\LdapException;
use Mockery\MockInterface;
use Tests\BrowserKitTest;
@@ -35,6 +34,7 @@ class LdapTest extends BrowserKitTest
'services.ldap.user_filter' => '(&(uid=${user}))',
'services.ldap.follow_referrals' => false,
'services.ldap.tls_insecure' => false,
+ 'services.ldap.thumbnail_attribute' => null,
]);
$this->mockLdap = \Mockery::mock(Ldap::class);
$this->app[Ldap::class] = $this->mockLdap;
@@ -668,4 +668,28 @@ class LdapTest extends BrowserKitTest
$this->runFailedAuthLogin();
$this->assertTrue($log->hasWarningThatContains('Failed login for timmyjenkins'));
}
+
+ public function test_thumbnail_attribute_used_as_user_avatar_if_configured()
+ {
+ config()->set(['services.ldap.thumbnail_attribute' => 'jpegPhoto']);
+
+ $this->commonLdapMocks(1, 1, 1, 2, 1);
+ $ldapDn = 'cn=test-user,dc=test' . config('services.ldap.base_dn');
+ $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
+ ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+ ->andReturn(['count' => 1, 0 => [
+ 'cn' => [$this->mockUser->name],
+ 'dn' => $ldapDn,
+ 'jpegphoto' => [base64_decode('/9j/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgGBgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8Q
+EBEQCgwSExIQEw8QEBD/yQALCAABAAEBAREA/8wABgAQEAX/2gAIAQEAAD8A0s8g/9k=')],
+ 'mail' => [$this->mockUser->email]
+ ]]);
+
+ $this->mockUserLogin()
+ ->seePageIs('/');
+
+ $user = User::query()->where('email', '=' , $this->mockUser->email)->first();
+ $this->assertNotNull($user->avatar);
+ $this->assertEquals('8c90748342f19b195b9c6b4eff742ded', md5_file(public_path($user->avatar->path)));
+ }
}