Merge pull request #838 from nextcloud/fix/835/set-avatar-on-login

Fix setting avatar on login
nextcloud · Jul 23, 2024 · c89d2b3 · c89d2b3
2 parents 7324192 + 5ae5b98
commit c89d2b3
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 22 deletions.
diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml
@@ -22,9 +22,6 @@ jobs:
         databases: ['sqlite', 'mysql', 'pgsql']
         server-versions: ['master']
         include:
-            - php-versions: 8.1
-              databases: mysql
-              server-versions: stable25
             - php-versions: 8.1
               databases: mysql
               server-versions: stable26
@@ -34,6 +31,9 @@ jobs:
             - php-versions: 8.1
               databases: mysql
               server-versions: stable28
+            - php-versions: 8.1
+              databases: mysql
+              server-versions: stable29
             - php-versions: 8.1
               databases: mysql
               server-versions: master

diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml
@@ -19,12 +19,14 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        php-versions: ['7.4', '8.0', '8.1']
+        php-versions: ['7.4', '8.0', '8.1', '8.2']
         databases: ['mysql']
-        server-versions: ['stable25', 'stable26', 'stable27', 'stable28', 'master']
+        server-versions: ['stable26', 'stable27', 'stable28', 'stable29', 'master']
         exclude:
           - php-versions: 7.4
             server-versions: master
+          - php-versions: 7.4
+            server-versions: stable29
           - php-versions: 7.4
             server-versions: stable28
           - php-versions: 7.4
@@ -34,21 +36,9 @@ jobs:
           - php-versions: 8.0
             server-versions: master
         include:
-          - php-versions: 8.2
-            databases: mysql
-            server-versions: stable26
-          - php-versions: 8.2
-            databases: mysql
-            server-versions: stable27
-          - php-versions: 8.2
-            databases: mysql
-            server-versions: stable28
           - php-versions: 8.3
             databases: mysql
             server-versions: stable28
-          - php-versions: 8.2
-            databases: mysql
-            server-versions: master
           - php-versions: 8.3
             databases: mysql
             server-versions: master

diff --git a/README.md b/README.md
@@ -47,6 +47,12 @@ To skip the confirmation, use `--force`.
 ***Warning***: be careful with the deletion of a provider because in some setup, this invalidates access to all
 NextCloud accounts associated with this provider.
 
+#### Avatar support
+
+The avatar attribute on your IdP side may contain a URL pointing to an image file or directly a base64 encoded image.
+The base64 should start with `data:image/png;base64,` or `data:image/jpeg;base64,`.
+The image should be in JPG or PNG format and have the same width and height.
+
 ### Disable default claims
 
 Even if you don't map any attribute for quota, display name, email or groups, this application will

diff --git a/appinfo/info.xml b/appinfo/info.xml
@@ -4,7 +4,7 @@
 	<name>OpenID Connect user backend</name>
 	<summary>Use an OpenID Connect backend to login to your Nextcloud</summary>
 	<description>Allows flexible configuration of an OIDC server as Nextcloud login user backend.</description>
-	<version>5.0.3</version>
+	<version>6.0.0</version>
 	<licence>agpl</licence>
 	<author>Roeland Jago Douma</author>
 	<author>Julius Härtl</author>
@@ -19,7 +19,7 @@
 	<bugs>https://github.com/nextcloud/user_oidc/issues</bugs>
 	<repository>https://github.com/nextcloud/user_oidc</repository>
 	<dependencies>
-		<nextcloud min-version="25" max-version="30"/>
+		<nextcloud min-version="26" max-version="30"/>
 	</dependencies>
 	<settings>
 		<admin>OCA\UserOIDC\Settings\AdminSettings</admin>

diff --git a/lib/Service/ProvisioningService.php b/lib/Service/ProvisioningService.php
@@ -7,11 +7,15 @@
 use OCP\Accounts\IAccountManager;
 use OCP\DB\Exception;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Http\Client\IClientService;
+use OCP\IAvatarManager;
 use OCP\IGroupManager;
+use OCP\Image;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\User\Events\UserChangedEvent;
 use Psr\Log\LoggerInterface;
+use Throwable;
 
 class ProvisioningService {
 	/** @var UserMapper */
@@ -37,6 +41,14 @@ class ProvisioningService {
 
 	/** @var IAccountManager */
 	private $accountManager;
+	/**
+	 * @var IClientService
+	 */
+	private $clientService;
+	/**
+	 * @var IAvatarManager
+	 */
+	private $avatarManager;
 
 
 	public function __construct(
@@ -47,7 +59,9 @@ public function __construct(
 		IGroupManager $groupManager,
 		IEventDispatcher $eventDispatcher,
 		LoggerInterface $logger,
-		IAccountManager $accountManager
+		IAccountManager $accountManager,
+		IClientService $clientService,
+		IAvatarManager $avatarManager
 	) {
 		$this->idService = $idService;
 		$this->providerService = $providerService;
@@ -57,6 +71,8 @@ public function __construct(
 		$this->eventDispatcher = $eventDispatcher;
 		$this->logger = $logger;
 		$this->accountManager = $accountManager;
+		$this->clientService = $clientService;
+		$this->avatarManager = $avatarManager;
 	}
 
 	/**
@@ -247,7 +263,7 @@ public function provisionUser(string $tokenUserId, int $providerId, object $idTo
 		$this->eventDispatcher->dispatchTyped($event);
 		$this->logger->debug('Avatar mapping event dispatched');
 		if ($event->hasValue()) {
-			$account->setProperty('avatar', $avatar, $scope, '1', '');
+			$this->setUserAvatar($user->getUID(), $avatar);
 		}
 
 		// Update twitter/X
@@ -310,6 +326,59 @@ public function provisionUser(string $tokenUserId, int $providerId, object $idTo
 		return $user;
 	}
 
+	/**
+	 * @param string $userId
+	 * @param string $avatarAttribute
+	 * @return void
+	 */
+	private function setUserAvatar(string $userId, string $avatarAttribute): void {
+		if (filter_var($avatarAttribute, FILTER_VALIDATE_URL)) {
+			$client = $this->clientService->newClient();
+			try {
+				$avatarContent = $client->get($avatarAttribute)->getBody();
+			} catch (Throwable $e) {
+				$this->logger->warning('Failed to get remote avatar for user ' . $userId, ['avatar_attribute' => $avatarAttribute]);
+				return;
+			}
+		} elseif (str_starts_with($avatarAttribute, 'data:image/png;base64,')) {
+			$avatarContent = base64_decode(str_replace('data:image/png;base64,', '', $avatarAttribute));
+		} elseif (str_starts_with($avatarAttribute, 'data:image/jpeg;base64,')) {
+			$avatarContent = base64_decode(str_replace('data:image/jpeg;base64,', '', $avatarAttribute));
+		}
+
+		try {
+			// inspired from OC\Core\Controller\AvatarController::postAvatar()
+			$image = new Image();
+			$image->loadFromData($avatarContent);
+			$image->readExif($avatarContent);
+			$image->fixOrientation();
+
+			if ($image->valid()) {
+				$mimeType = $image->mimeType();
+				if ($mimeType !== 'image/jpeg' && $mimeType !== 'image/png') {
+					$this->logger->warning('Failed to set remote avatar for user ' . $userId, ['error' => 'Unknown filetype']);
+					return;
+				}
+
+				if ($image->width() === $image->height()) {
+					try {
+						$avatar = $this->avatarManager->getAvatar($userId);
+						$avatar->set($image);
+						return;
+					} catch (Throwable $e) {
+						$this->logger->error('Failed to set remote avatar for user ' . $userId, ['exception' => $e]);
+						return;
+					}
+				}
+				$this->logger->warning('Failed to set remote avatar for user ' . $userId, ['error' => 'Image is not square']);
+			} else {
+				$this->logger->warning('Failed to set remote avatar for user ' . $userId, ['error' => 'Invalid image']);
+			}
+		} catch (\Exception $e) {
+			$this->logger->error('Failed to set remote avatar for user ' . $userId, ['exception' => $e]);
+		}
+	}
+
 	public function provisionUserGroups(IUser $user, int $providerId, object $idTokenPayload): void {
 		$groupsAttribute = $this->providerService->getSetting($providerId, ProviderService::SETTING_MAPPING_GROUPS, 'groups');
 		$groupsData = $idTokenPayload->{$groupsAttribute} ?? null;

diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml
@@ -1,6 +1,9 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="5.24.0@462c80e31c34e58cc4f750c656be3927e80e550e">
+<files psalm-version="5.25.0@01a8eb06b9e9cc6cfb6a320bf9fb14331919d505">
   <file src="lib/Service/ProvisioningService.php">
+    <MissingDependency>
+      <code><![CDATA[Image]]></code>
+    </MissingDependency>
     <RedundantCondition>
       <code><![CDATA[isset($group->displayName)]]></code>
     </RedundantCondition>

diff --git a/tests/unit/Service/ProvisioningServiceTest.php b/tests/unit/Service/ProvisioningServiceTest.php
@@ -8,6 +8,8 @@
 use OCA\UserOIDC\Service\ProvisioningService;
 use OCP\Accounts\IAccountManager;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Http\Client\IClientService;
+use OCP\IAvatarManager;
 use OCP\IGroup;
 use OCP\IGroupManager;
 use OCP\IUser;
@@ -44,6 +46,12 @@ class ProvisioningServiceTest extends TestCase {
 	/** @var IAccountManager | MockObject */
 	private $accountManager;
 
+	/** @var IClientService | MockObject */
+	private $clientService;
+
+	/** @var IAvatarManager | MockObject */
+	private $avatarManager;
+
 	public function setUp(): void {
 		parent::setUp();
 		$this->idService = $this->createMock(LocalIdService::class);
@@ -55,6 +63,8 @@ public function setUp(): void {
 		$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->accountManager = $this->createMock(IAccountManager::class);
+		$this->clientService = $this->createMock(IClientService::class);
+		$this->avatarManager = $this->createMock(IAvatarManager::class);
 
 		$this->provisioningService = new ProvisioningService(
 			$this->idService,
@@ -65,6 +75,8 @@ public function setUp(): void {
 			$this->eventDispatcher,
 			$this->logger,
 			$this->accountManager,
+			$this->clientService,
+			$this->avatarManager
 		);
 	}