From 2d44b0191cb80b2b83ceb18cbe8e4de5828f6270 Mon Sep 17 00:00:00 2001 From: Rick Bruyninckx Date: Tue, 4 Jul 2023 10:05:13 +0200 Subject: [PATCH] feat: Populate avatar from SAML attribute Signed-off-by: Rick Bruyninckx Signed-off-by: Carl Schwan --- lib/SAMLSettings.php | 1 + lib/Settings/Admin.php | 5 ++ lib/UserBackend.php | 95 ++++++++++++++++++++++++++- psalm.xml | 1 + tests/stubs/oc_files_setupmanager.php | 86 ++++++++++++++++++++++++ tests/unit/Settings/AdminTest.php | 5 ++ tests/unit/UserBackendTest.php | 20 ++++++ 7 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 tests/stubs/oc_files_setupmanager.php diff --git a/lib/SAMLSettings.php b/lib/SAMLSettings.php index a13eae48e..5cdcdff4d 100644 --- a/lib/SAMLSettings.php +++ b/lib/SAMLSettings.php @@ -61,6 +61,7 @@ class SAMLSettings { 'saml-attribute-mapping-mfa_mapping', 'saml-attribute-mapping-user_id_ldap_mapping', 'saml-attribute-mapping-group_mapping_prefix', + 'saml-attribute-mapping-avatar_mapping', 'saml-user-filter-reject_groups', 'saml-user-filter-require_groups', 'sp-entityId', diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php index 7ddd2d18c..8e6c31804 100644 --- a/lib/Settings/Admin.php +++ b/lib/Settings/Admin.php @@ -150,6 +150,11 @@ public function getForm(): TemplateResponse { 'type' => 'line', 'required' => false, ], + 'avatar_mapping' => [ + 'text' => $this->l10n->t('Attribute to map the users avatar to.'), + 'type' => 'line', + 'required' => false, + ], 'user_id_ldap_mapping' => [ 'text' => $this->l10n->t('Attribute to map the users to an existing LDAP user'), 'type' => 'line', diff --git a/lib/UserBackend.php b/lib/UserBackend.php index c71eb7324..d71760cb4 100644 --- a/lib/UserBackend.php +++ b/lib/UserBackend.php @@ -9,15 +9,20 @@ namespace OCA\User_SAML; +use OC\Files\SetupManager; use OC\Security\CSRF\CsrfTokenManager; use OCA\User_SAML\Model\SessionData; use OCP\AppFramework\Services\IAppConfig; use OCP\Authentication\IApacheBackend; +use OCP\Config\IUserConfig; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\IRootFolder; use OCP\Files\NotPermittedException; +use OCP\IAvatarManager; use OCP\IConfig; use OCP\IDBConnection; +use OCP\IImage; +use OCP\Image; use OCP\ISession; use OCP\IURLGenerator; use OCP\IUser; @@ -30,6 +35,7 @@ use OCP\User\Backend\IGetDisplayNameBackend; use OCP\User\Backend\IGetHomeBackend; use OCP\User\Backend\ILimitAwareCountUsersBackend; +use OCP\User\Backend\IProvideAvatarBackend; use OCP\User\Backend\IProvideEnabledStateBackend; use OCP\User\Backend\ISetDisplayNameBackend; use OCP\User\Events\UserChangedEvent; @@ -38,12 +44,23 @@ use Override; use Psr\Log\LoggerInterface; -class UserBackend extends ABackend implements IApacheBackend, IUserBackend, IGetDisplayNameBackend, ILimitAwareCountUsersBackend, IGetHomeBackend, ICustomLogout, ISetDisplayNameBackend, IProvideEnabledStateBackend { +class UserBackend extends ABackend implements + IApacheBackend, + IUserBackend, + IGetDisplayNameBackend, + ILimitAwareCountUsersBackend, + IGetHomeBackend, + ICustomLogout, + ISetDisplayNameBackend, + IProvideEnabledStateBackend, + IProvideAvatarBackend { /** @var \OCP\UserInterface[] */ private static array $backends = []; + /** @psalm-suppress UndefinedClass */ public function __construct( private readonly IConfig $config, + private readonly IUserConfig $userConfig, private readonly IAppConfig $appConfig, private readonly IURLGenerator $urlGenerator, private readonly ISession $session, @@ -55,9 +72,20 @@ public function __construct( private readonly UserData $userData, private readonly IEventDispatcher $eventDispatcher, private readonly string $serverRoot, + private readonly IAvatarManager $avatarManager, + private readonly SetupManager $setupManager, // TODO: replace with ISetupManager once we depends on NC34 ) { } + #[Override] + public function canChangeAvatar($uid): bool { + try { + return empty(trim($this->getAttributeKeys('saml-attribute-mapping-avatar_mapping')[0])); + } catch (\InvalidArgumentException $e) { + return true; + } + } + /** * Whether $uid exists in the database */ @@ -523,6 +551,14 @@ public function updateAttributes(string $uid): void { $newGroups = null; } + try { + $newAvatar = $this->getAttributeValue('saml-attribute-mapping-avatar_mapping', $attributes); + $this->logger->debug('Avatar attribute content: {avatar}', ['avatar' => $newAvatar]); + } catch (\InvalidArgumentException $e) { + $this->logger->debug('Failed to fetch avatar attribute: {exception}', ['exception' => $e->getMessage()]); + $newAvatar = null; + } + if ($user !== null) { $this->logger->debug('Updating attributes for existing user', ['app' => 'user_saml', 'user' => $user->getUID()]); $currentEmail = (string)$user->getSystemEMailAddress(); @@ -570,7 +606,64 @@ public function updateAttributes(string $uid): void { 'user' => $user->getUID(), 'groups' => $newGroups, ]); + + if ($newAvatar !== null) { + /** + * @psalm-suppress MissingDependency + * @var IImage $image + */ + $image = new Image(); + $fileData = file_get_contents($newAvatar); + if ($fileData === false) { + $this->logger->warning('Unable to read content from avatar'); + return; + } + + $image->loadFromData($fileData); + $data = $image->data(); + if ($data === null) { + $this->logger->warning('Image data is invalid'); + return; + } + + $checksum = md5($data); + if ($checksum !== $this->userConfig->getValueString($uid, 'user_saml', 'lastAvatarChecksum')) { + // use the checksum before modifications + if ($this->setAvatarFromSamlProvider($user, $image)) { + // save checksum only after successful setting + $this->userConfig->setValueString($uid, 'user_saml', 'lastAvatarChecksum', $checksum); + } + } + } + } + } + + private function setAvatarFromSamlProvider(IUser $user, IImage $image): bool { + if (!$image->valid()) { + $this->logger->debug('avatar image data from LDAP invalid for ' . $user->getUID()); + return false; + } + + //make sure it is a square and not bigger than 128x128 + $size = min([$image->width(), $image->height(), 128]); + if (!$image->centerCrop($size)) { + $this->logger->debug('croping image for avatar failed for ' . $user->getUID()); + return false; + } + + /** @psalm-suppress UndefinedClass */ + $this->setupManager->setupForUser($user); + + try { + $avatar = $this->avatarManager->getAvatar($user->getUID()); + $avatar->set($image); + return true; + } catch (\Exception $e) { + $this->logger->info('Could not set avatar for ' . $user->getUID(), [ + 'exception' => $e, + ]); } + return false; } #[\Override] diff --git a/psalm.xml b/psalm.xml index 8c040b92e..57f8d3cbe 100644 --- a/psalm.xml +++ b/psalm.xml @@ -24,6 +24,7 @@ + diff --git a/tests/stubs/oc_files_setupmanager.php b/tests/stubs/oc_files_setupmanager.php new file mode 100644 index 000000000..add940807 --- /dev/null +++ b/tests/stubs/oc_files_setupmanager.php @@ -0,0 +1,86 @@ +[] $providers + */ + public function dropPartialMountsForUser(IUser $user, array $providers = []): void { + } +} diff --git a/tests/unit/Settings/AdminTest.php b/tests/unit/Settings/AdminTest.php index 7de3f07a0..7b79fefc0 100644 --- a/tests/unit/Settings/AdminTest.php +++ b/tests/unit/Settings/AdminTest.php @@ -178,6 +178,11 @@ public function formDataProvider(): array { 'type' => 'line', 'required' => false, ], + 'avatar_mapping' => [ + 'text' => $this->l10n->t('Attribute to map the users avatar to.'), + 'type' => 'line', + 'required' => false, + ], ]; $userFilterSettings = [ diff --git a/tests/unit/UserBackendTest.php b/tests/unit/UserBackendTest.php index e91a37c8f..3488f5a0b 100644 --- a/tests/unit/UserBackendTest.php +++ b/tests/unit/UserBackendTest.php @@ -9,12 +9,15 @@ namespace OCA\User_SAML\Tests\Settings; +use OC\Files\SetupManager; use OCA\User_SAML\GroupManager; use OCA\User_SAML\SAMLSettings; use OCA\User_SAML\UserBackend; use OCA\User_SAML\UserData; use OCP\AppFramework\Services\IAppConfig; +use OCP\Config\IUserConfig; use OCP\EventDispatcher\IEventDispatcher; +use OCP\IAvatarManager; use OCP\IConfig; use OCP\IDBConnection; use OCP\ISession; @@ -45,6 +48,10 @@ class UserBackendTest extends TestCase { private SAMLSettings&MockObject $SAMLSettings; private LoggerInterface&MockObject $logger; private IEventDispatcher&MockObject $eventDispatcher; + private IAvatarManager&MockObject $avatarManager; + private IUserConfig&MockObject $userConfig; + /** @psalm-suppress UndefinedClass */ + private SetupManager&MockObject $setupManager; #[Override] protected function setUp(): void { @@ -61,6 +68,13 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->userData = $this->createMock(UserData::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->avatarManager = $this->createMock(IAvatarManager::class); + $this->userConfig = $this->createMock(IUserConfig::class); + /** + * @psalm-suppress UndefinedClass + * @psalm-suppress PropertyTypeCoercion + */ + $this->setupManager = $this->createMock(SetupManager::class); } /** @@ -70,6 +84,7 @@ public function getMockedBuilder(array $mockedFunctions = []): UserBackend&MockO return $this->getMockBuilder(UserBackend::class) ->setConstructorArgs([ $this->config, + $this->userConfig, $this->appConfig, $this->urlGenerator, $this->session, @@ -81,6 +96,8 @@ public function getMockedBuilder(array $mockedFunctions = []): UserBackend&MockO $this->userData, $this->eventDispatcher, 'serverRoot', + $this->avatarManager, + $this->setupManager, ]) ->onlyMethods($mockedFunctions) ->getMock(); @@ -89,6 +106,7 @@ public function getMockedBuilder(array $mockedFunctions = []): UserBackend&MockO public function getRealUserBackend(): UserBackend { return new UserBackend( $this->config, + $this->userConfig, $this->appConfig, $this->urlGenerator, $this->session, @@ -100,6 +118,8 @@ public function getRealUserBackend(): UserBackend { $this->userData, $this->eventDispatcher, 'serverRoot', + $this->avatarManager, + $this->setupManager, ); }