From f45f826b5262aefdeb97eb127ff65b647b19578b Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 22 Mar 2021 12:08:53 +0100 Subject: [PATCH] Add new v2-private account scope Added new v2-private account manager scope that restricts the scope further by excluding public link access. Avatars with v2-private account scope are now showing the guest avatar instead of the real avatar. Signed-off-by: Vincent Petry --- apps/settings/js/federationscopemenu.js | 28 ++++++++-- apps/settings/js/federationsettingsview.js | 20 ++++++- lib/private/Avatar/AvatarManager.php | 36 ++++++++++++- lib/private/Server.php | 4 +- lib/public/Accounts/IAccountManager.php | 49 +++++++++++++++-- tests/lib/Avatar/AvatarManagerTest.php | 61 +++++++++++++++++++++- 6 files changed, 185 insertions(+), 13 deletions(-) diff --git a/apps/settings/js/federationscopemenu.js b/apps/settings/js/federationscopemenu.js index 170aec15a8..1c854a7ee5 100644 --- a/apps/settings/js/federationscopemenu.js +++ b/apps/settings/js/federationscopemenu.js @@ -15,6 +15,8 @@ * Construct a new FederationScopeMenu instance * @constructs FederationScopeMenu * @memberof OC.Settings + * @param {object} options + * @param {array.} [options.excludedScopes] array of excluded scopes */ var FederationScopeMenu = OC.Backbone.View.extend({ tagName: 'div', @@ -26,8 +28,15 @@ this.field = options.field; this._scopes = [ { - name: 'private', + name: 'v2-private', displayName: t('settings', 'Private'), + tooltip: t('settings', "Don't show via public link"), + iconClass: 'icon-password', + active: false + }, + { + name: 'private', + displayName: t('settings', 'Local'), tooltip: t('settings', "Don't synchronize to servers"), iconClass: 'icon-password', active: false @@ -41,12 +50,18 @@ }, { name: 'public', - displayName: t('settings', 'Public'), + displayName: t('settings', 'Published'), tooltip: t('settings', 'Synchronize to trusted servers and the global and public address book'), iconClass: 'icon-link', active: false } ]; + + if (options.excludedScopes) { + this._scopes = this._scopes.filter(function(scopeEntry) { + return options.excludedScopes.indexOf(scopeEntry.name) === -1; + }) + } }, /** @@ -106,15 +121,18 @@ } switch (currentlyActiveValue) { - case 'private': + case 'v2-private': this._scopes[0].active = true; break; - case 'contacts': + case 'private': this._scopes[1].active = true; break; - case 'public': + case 'contacts': this._scopes[2].active = true; break; + case 'public': + this._scopes[3].active = true; + break; } this.render(); diff --git a/apps/settings/js/federationsettingsview.js b/apps/settings/js/federationsettingsview.js index 9cefaf132f..293989baa0 100644 --- a/apps/settings/js/federationsettingsview.js +++ b/apps/settings/js/federationsettingsview.js @@ -61,9 +61,26 @@ render: function() { var self = this; + var fieldsWithV2Private = [ + 'avatar', + 'phone', + 'twitter', + 'website', + 'address', + ]; + _.each(this._inputFields, function(field) { var $icon = self.$('#' + field + 'form h3 > .federation-menu'); - var scopeMenu = new OC.Settings.FederationScopeMenu({field: field}); + var excludedScopes = null + + if (fieldsWithV2Private.indexOf(field) === -1) { + excludedScopes = ['v2-private'] + } + + var scopeMenu = new OC.Settings.FederationScopeMenu({ + field: field, + excludedScopes: excludedScopes, + }); self.listenTo(scopeMenu, 'select:scope', function(scope) { self._onScopeChanged(field, scope); @@ -208,6 +225,7 @@ switch (scope) { case 'private': + case 'v2-private': $icon.addClass('icon-password'); $icon.removeClass('hidden'); break; diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index 5102396224..03f3d89e5f 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -36,6 +36,7 @@ namespace OC\Avatar; use OC\User\Manager; use OC\User\NoUserException; +use OCP\Accounts\IAccountManager; use OCP\Files\IAppData; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; @@ -44,12 +45,16 @@ use OCP\IAvatarManager; use OCP\IConfig; use OCP\IL10N; use OCP\ILogger; +use OCP\IUserSession; /** * This class implements methods to access Avatar functionality */ class AvatarManager implements IAvatarManager { + /** @var IUserSession */ + private $userSession; + /** @var Manager */ private $userManager; @@ -65,6 +70,9 @@ class AvatarManager implements IAvatarManager { /** @var IConfig */ private $config; + /** @var IAccountManager */ + private $accountManager; + /** * AvatarManager constructor. * @@ -73,18 +81,23 @@ class AvatarManager implements IAvatarManager { * @param IL10N $l * @param ILogger $logger * @param IConfig $config + * @param IUserSession $userSession */ public function __construct( + IUserSession $userSession, Manager $userManager, IAppData $appData, IL10N $l, ILogger $logger, - IConfig $config) { + IConfig $config, + IAccountManager $accountManager) { + $this->userSession = $userSession; $this->userManager = $userManager; $this->appData = $appData; $this->l = $l; $this->logger = $logger; $this->config = $config; + $this->accountManager = $accountManager; } /** @@ -104,6 +117,27 @@ class AvatarManager implements IAvatarManager { // sanitize userID - fixes casing issue (needed for the filesystem stuff that is done below) $userId = $user->getUID(); + $requestingUser = null; + if ($this->userSession !== null) { + $requestingUser = $this->userSession->getUser(); + } + + $canShowRealAvatar = true; + + // requesting in public page + if ($requestingUser === null) { + $account = $this->accountManager->getAccount($user); + $avatarProperties = $account->getProperty(IAccountManager::PROPERTY_AVATAR); + $avatarScope = $avatarProperties->getScope(); + + // v2-private scope hides the avatar from public access + if ($avatarScope === IAccountManager::SCOPE_PRIVATE) { + // FIXME: guest avatar is re-generated every time, use a cache instead + // see how UserAvatar caches the generated one + return $this->getGuestAvatar($userId); + } + } + try { $folder = $this->appData->getFolder($userId); } catch (NotFoundException $e) { diff --git a/lib/private/Server.php b/lib/private/Server.php index c0d6afbaaf..93ad3b3899 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -720,11 +720,13 @@ class Server extends ServerContainer implements IServerContainer { $this->registerService(AvatarManager::class, function (Server $c) { return new AvatarManager( + $c->get(IUserSession::class), $c->get(\OC\User\Manager::class), $c->getAppDataDir('avatar'), $c->getL10N('lib'), $c->get(ILogger::class), - $c->get(\OCP\IConfig::class) + $c->get(\OCP\IConfig::class), + $c->get(IAccountManager::class) ); }); $this->registerAlias(IAvatarManager::class, AvatarManager::class); diff --git a/lib/public/Accounts/IAccountManager.php b/lib/public/Accounts/IAccountManager.php index ae70d8963b..9d720ba9e5 100644 --- a/lib/public/Accounts/IAccountManager.php +++ b/lib/public/Accounts/IAccountManager.php @@ -38,11 +38,54 @@ use OCP\IUser; */ interface IAccountManager { - /** nobody can see my account details */ + /** + * Contact details visible locally only + * + * @since 21.0.1 + */ + public const SCOPE_PRIVATE = 'v2-private'; + + /** + * Contact details visible locally and through public link access on local instance + * + * @since 21.0.1 + */ + public const SCOPE_LOCAL = 'private'; + + /** + * Contact details visible locally, through public link access and on trusted federated servers. + * + * @since 21.0.1 + */ + public const SCOPE_FEDERATED = 'federated'; + + /** + * Contact details visible locally, through public link access, on trusted federated servers + * and published to the public lookup server. + * + * @since 21.0.1 + */ + public const SCOPE_PUBLISHED = 'public'; + + /** + * Contact details only visible locally + * + * @deprecated 21.0.1 + */ public const VISIBILITY_PRIVATE = 'private'; - /** only contacts, especially trusted servers can see my contact details */ + + /** + * Contact details visible on trusted federated servers. + * + * @deprecated 21.0.1 + */ public const VISIBILITY_CONTACTS_ONLY = 'contacts'; - /** every body ca see my contact detail, will be published to the lookup server */ + + /** + * Contact details visible on trusted federated servers and in the public lookup server. + * + * @deprecated 21.0.1 + */ public const VISIBILITY_PUBLIC = 'public'; public const PROPERTY_AVATAR = 'avatar'; diff --git a/tests/lib/Avatar/AvatarManagerTest.php b/tests/lib/Avatar/AvatarManagerTest.php index 5a061cd10e..0ce0e75256 100644 --- a/tests/lib/Avatar/AvatarManagerTest.php +++ b/tests/lib/Avatar/AvatarManagerTest.php @@ -25,19 +25,26 @@ namespace Test\Avatar; use OC\Avatar\AvatarManager; +use OC\Avatar\GuestAvatar; use OC\Avatar\UserAvatar; use OC\User\Manager; +use OCP\Accounts\IAccount; +use OCP\Accounts\IAccountManager; +use OCP\Accounts\IAccountProperty; use OCP\Files\IAppData; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\IConfig; use OCP\IL10N; use OCP\ILogger; use OCP\IUser; +use OCP\IUserSession; /** * Class AvatarManagerTest */ class AvatarManagerTest extends \Test\TestCase { + /** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */ + private $userSession; /** @var Manager|\PHPUnit\Framework\MockObject\MockObject */ private $userManager; /** @var IAppData|\PHPUnit\Framework\MockObject\MockObject */ @@ -48,28 +55,33 @@ class AvatarManagerTest extends \Test\TestCase { private $logger; /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ private $config; + /** @var IAccountManager|\PHPUnit\Framework\MockObject\MockObject */ + private $accountManager; /** @var AvatarManager | \PHPUnit\Framework\MockObject\MockObject */ private $avatarManager; protected function setUp(): void { parent::setUp(); + $this->userSession = $this->createMock(IUserSession::class); $this->userManager = $this->createMock(Manager::class); $this->appData = $this->createMock(IAppData::class); $this->l10n = $this->createMock(IL10N::class); $this->logger = $this->createMock(ILogger::class); $this->config = $this->createMock(IConfig::class); + $this->accountManager = $this->createMock(IAccountManager::class); $this->avatarManager = new AvatarManager( + $this->userSession, $this->userManager, $this->appData, $this->l10n, $this->logger, - $this->config + $this->config, + $this->accountManager ); } - public function testGetAvatarInvalidUser() { $this->expectException(\Exception::class); $this->expectExceptionMessage('user does not exist'); @@ -84,6 +96,15 @@ class AvatarManagerTest extends \Test\TestCase { } public function testGetAvatarValidUser() { + // requesting user + $this->userSession->expects($this->once()) + ->method('getUser') + ->willReturn($this->createMock(IUser::class)); + + // we skip account scope check for logged in user + $this->accountManager->expects($this->never()) + ->method('getAccount'); + $user = $this->createMock(IUser::class); $user ->expects($this->once()) @@ -126,4 +147,40 @@ class AvatarManagerTest extends \Test\TestCase { $expected = new UserAvatar($folder, $this->l10n, $user, $this->logger, $this->config); $this->assertEquals($expected, $this->avatarManager->getAvatar('vaLid-USER')); } + + public function testGetAvatarPrivateScope() { + $user = $this->createMock(IUser::class); + $user + ->expects($this->once()) + ->method('getUID') + ->willReturn('valid-user'); + $this->userManager + ->expects($this->once()) + ->method('get') + ->with('valid-user') + ->willReturn($user); + $folder = $this->createMock(ISimpleFolder::class); + $this->appData + ->expects($this->never()) + ->method('getFolder'); + + $account = $this->createMock(IAccount::class); + $this->accountManager->expects($this->once()) + ->method('getAccount') + ->with($user) + ->willReturn($account); + + $property = $this->createMock(IAccountProperty::class); + $account->expects($this->once()) + ->method('getProperty') + ->with(IAccountManager::PROPERTY_AVATAR) + ->willReturn($property); + + $property->expects($this->once()) + ->method('getScope') + ->willReturn(IAccountManager::SCOPE_PRIVATE); + + $expected = new GuestAvatar('valid-user', $this->createMock(ILogger::class)); + $this->assertEquals($expected, $this->avatarManager->getAvatar('valid-user')); + } }