From f878ea96067ff634800a06bff7ecafdfa09e040a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 12 Mar 2020 17:38:18 +0100 Subject: [PATCH 1/2] Remove user name from public sharing page Signed-off-by: Joas Schilling --- apps/files_sharing/css/public.scss | 6 ++++ .../lib/Controller/ShareController.php | 30 +++++++++++++++---- apps/files_sharing/templates/public.php | 13 +++++--- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/apps/files_sharing/css/public.scss b/apps/files_sharing/css/public.scss index a8207167c2..b5b3f4f1f4 100644 --- a/apps/files_sharing/css/public.scss +++ b/apps/files_sharing/css/public.scss @@ -123,6 +123,12 @@ thead { opacity: 1; } +#public-upload #emptycontent #displayavatar .icon-folder { + height: 48px; + width: 48px; + background-size: 48px; +} + #public-upload #emptycontent .button { display: inline-block; height: auto; diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index a7c936f28a..616b975463 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -48,6 +48,7 @@ use OC\Security\CSP\ContentSecurityPolicy; use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\Files_Sharing\Activity\Providers\Downloads; use OCA\Viewer\Event\LoadViewer; +use OCP\Accounts\IAccountManager; use OCP\AppFramework\AuthPublicShareController; use OCP\AppFramework\Http\NotFoundResponse; use OCP\AppFramework\Http\Template\ExternalShareMenuAction; @@ -66,6 +67,7 @@ use OCP\IPreview; use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; +use OCP\IUser; use OCP\IUserManager; use OCP\Share; use OCP\Share\Exceptions\ShareNotFound; @@ -95,6 +97,8 @@ class ShareController extends AuthPublicShareController { protected $rootFolder; /** @var FederatedShareProvider */ protected $federatedShareProvider; + /** @var IAccountManager */ + protected $accountManager; /** @var EventDispatcherInterface */ protected $eventDispatcher; /** @var IL10N */ @@ -120,6 +124,7 @@ class ShareController extends AuthPublicShareController { * @param IPreview $previewManager * @param IRootFolder $rootFolder * @param FederatedShareProvider $federatedShareProvider + * @param IAccountManager $accountManager * @param EventDispatcherInterface $eventDispatcher * @param IL10N $l10n * @param Defaults $defaults @@ -136,6 +141,7 @@ class ShareController extends AuthPublicShareController { IPreview $previewManager, IRootFolder $rootFolder, FederatedShareProvider $federatedShareProvider, + IAccountManager $accountManager, EventDispatcherInterface $eventDispatcher, IL10N $l10n, Defaults $defaults) { @@ -148,6 +154,7 @@ class ShareController extends AuthPublicShareController { $this->previewManager = $previewManager; $this->rootFolder = $rootFolder; $this->federatedShareProvider = $federatedShareProvider; + $this->accountManager = $accountManager; $this->eventDispatcher = $eventDispatcher; $this->l10n = $l10n; $this->defaults = $defaults; @@ -328,8 +335,20 @@ class ShareController extends AuthPublicShareController { } $shareTmpl = []; - $shareTmpl['displayName'] = $this->userManager->get($share->getShareOwner())->getDisplayName(); - $shareTmpl['owner'] = $share->getShareOwner(); + $shareTmpl['owner'] = ''; + $shareTmpl['shareOwner'] = ''; + + $owner = $this->userManager->get($share->getShareOwner()); + if ($owner instanceof IUser) { + $ownerAccount = $this->accountManager->getAccount($owner); + + $ownerName = $ownerAccount->getProperty(IAccountManager::PROPERTY_DISPLAYNAME); + if ($ownerName->getScope() === IAccountManager::VISIBILITY_PUBLIC) { + $shareTmpl['owner'] = $owner->getUID(); + $shareTmpl['shareOwner'] = $owner->getDisplayName(); + } + } + $shareTmpl['filename'] = $shareNode->getName(); $shareTmpl['directory_path'] = $share->getTarget(); $shareTmpl['note'] = $share->getNote(); @@ -396,7 +415,6 @@ class ShareController extends AuthPublicShareController { $shareTmpl['showgridview'] = false; $shareTmpl['hideFileList'] = $hideFileList; - $shareTmpl['shareOwner'] = $this->userManager->get($share->getShareOwner())->getDisplayName(); $shareTmpl['downloadURL'] = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.downloadShare', ['token' => $this->getToken()]); $shareTmpl['shareUrl'] = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.showShare', ['token' => $this->getToken()]); $shareTmpl['maxSizeAnimateGif'] = $this->config->getSystemValue('max_filesize_animated_gifs_public_sharing', 10); @@ -479,7 +497,9 @@ class ShareController extends AuthPublicShareController { $response = new PublicTemplateResponse($this->appName, 'public', $shareTmpl); $response->setHeaderTitle($shareTmpl['filename']); - $response->setHeaderDetails($this->l10n->t('shared by %s', [$shareTmpl['displayName']])); + if ($shareTmpl['shareOwner'] !== '') { + $response->setHeaderDetails($this->l10n->t('shared by %s', [$shareTmpl['shareOwner']])); + } $isNoneFileDropFolder = $shareIsFolder === false || $share->getPermissions() !== \OCP\Constants::PERMISSION_CREATE; @@ -491,7 +511,7 @@ class ShareController extends AuthPublicShareController { $download = new SimpleMenuAction('download', $this->l10n->t('Download'), 'icon-download', $shareTmpl['downloadURL'], 10, $shareTmpl['fileSize']); $downloadAll = new SimpleMenuAction('download', $this->l10n->t('Download all files'), 'icon-download', $shareTmpl['downloadURL'], 10, $shareTmpl['fileSize']); $directLink = new LinkMenuAction($this->l10n->t('Direct link'), 'icon-public', $shareTmpl['previewURL']); - $externalShare = new ExternalShareMenuAction($this->l10n->t('Add to your Nextcloud'), 'icon-external', $shareTmpl['owner'], $shareTmpl['displayName'], $shareTmpl['filename']); + $externalShare = new ExternalShareMenuAction($this->l10n->t('Add to your Nextcloud'), 'icon-external', $shareTmpl['owner'], $shareTmpl['shareOwner'], $shareTmpl['filename']); $responseComposer = []; diff --git a/apps/files_sharing/templates/public.php b/apps/files_sharing/templates/public.php index a9cfaae9df..ce251028ee 100644 --- a/apps/files_sharing/templates/public.php +++ b/apps/files_sharing/templates/public.php @@ -54,7 +54,7 @@ $maxUploadFilesize = min($upload_max_filesize, $post_max_size); - +
@@ -88,9 +88,14 @@ $maxUploadFilesize = min($upload_max_filesize, $post_max_size);
-
-

t('Upload files to %s', [$_['shareOwner']])) ?>

-

+ +
+

t('Upload files to %s', [$_['shareOwner']])) ?>

+

+ +
+

t('Upload files to %s', [$_['filename']])) ?>

+

t('Note')); ?>

From 1c4422ca3d7b5175d365162a1e7347ae12f3a9fa Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 1 Apr 2020 16:02:34 +0200 Subject: [PATCH 2/2] Add a test and fix the existing ones Signed-off-by: Joas Schilling --- .../tests/Controller/ShareControllerTest.php | 249 ++++++++++++++++-- 1 file changed, 220 insertions(+), 29 deletions(-) diff --git a/apps/files_sharing/tests/Controller/ShareControllerTest.php b/apps/files_sharing/tests/Controller/ShareControllerTest.php index c9a187116b..240a0c7faa 100644 --- a/apps/files_sharing/tests/Controller/ShareControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareControllerTest.php @@ -39,6 +39,9 @@ use OC\Files\Filesystem; use OC\Files\Node\Folder; use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\Files_Sharing\Controller\ShareController; +use OCP\Accounts\IAccount; +use OCP\Accounts\IAccountManager; +use OCP\Accounts\IAccountProperty; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\Template\ExternalShareMenuAction; use OCP\AppFramework\Http\Template\LinkMenuAction; @@ -62,6 +65,10 @@ use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IShare; use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use OCP\Activity\IManager; +use OCP\Files\IRootFolder; +use OCP\Defaults; +use OC\Share20\Manager; /** * @group DB @@ -79,21 +86,23 @@ class ShareControllerTest extends \Test\TestCase { private $appName = 'files_sharing'; /** @var ShareController */ private $shareController; - /** @var IURLGenerator | \PHPUnit_Framework_MockObject_MockObject */ + /** @var IURLGenerator|MockObject */ private $urlGenerator; - /** @var ISession | \PHPUnit_Framework_MockObject_MockObject */ + /** @var ISession|MockObject */ private $session; - /** @var \OCP\IPreview | \PHPUnit_Framework_MockObject_MockObject */ + /** @var \OCP\IPreview|MockObject */ private $previewManager; - /** @var \OCP\IConfig | \PHPUnit_Framework_MockObject_MockObject */ + /** @var \OCP\IConfig|MockObject */ private $config; - /** @var \OC\Share20\Manager | \PHPUnit_Framework_MockObject_MockObject */ + /** @var \OC\Share20\Manager|MockObject */ private $shareManager; - /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject */ + /** @var IUserManager|MockObject */ private $userManager; - /** @var FederatedShareProvider | \PHPUnit_Framework_MockObject_MockObject */ + /** @var FederatedShareProvider|MockObject */ private $federatedShareProvider; - /** @var EventDispatcherInterface | \PHPUnit_Framework_MockObject_MockObject */ + /** @var IAccountManager|MockObject */ + private $accountManager; + /** @var EventDispatcherInterface|MockObject */ private $eventDispatcher; /** @var IL10N */ private $l10n; @@ -102,37 +111,38 @@ class ShareControllerTest extends \Test\TestCase { parent::setUp(); $this->appName = 'files_sharing'; - $this->shareManager = $this->getMockBuilder('\OC\Share20\Manager')->disableOriginalConstructor()->getMock(); - $this->urlGenerator = $this->getMockBuilder(IURLGenerator::class)->getMock(); - $this->session = $this->getMockBuilder(ISession::class)->getMock(); - $this->previewManager = $this->getMockBuilder(IPreview::class)->getMock(); - $this->config = $this->getMockBuilder(IConfig::class)->getMock(); - $this->userManager = $this->getMockBuilder(IUserManager::class)->getMock(); - $this->federatedShareProvider = $this->getMockBuilder('OCA\FederatedFileSharing\FederatedShareProvider') - ->disableOriginalConstructor()->getMock(); + $this->shareManager = $this->createMock(Manager::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->session = $this->createMock(ISession::class); + $this->previewManager = $this->createMock(IPreview::class); + $this->config = $this->createMock(IConfig::class); + $this->userManager = $this->createMock(IUserManager::class); + $this->federatedShareProvider = $this->createMock(FederatedShareProvider::class); $this->federatedShareProvider->expects($this->any()) ->method('isOutgoingServer2serverShareEnabled')->willReturn(true); $this->federatedShareProvider->expects($this->any()) ->method('isIncomingServer2serverShareEnabled')->willReturn(true); - $this->eventDispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')->getMock(); + $this->accountManager = $this->createMock(IAccountManager::class); + $this->eventDispatcher = $this->createMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); $this->l10n = $this->createMock(IL10N::class); $this->shareController = new \OCA\Files_Sharing\Controller\ShareController( $this->appName, - $this->getMockBuilder(IRequest::class)->getMock(), + $this->createMock(IRequest::class), $this->config, $this->urlGenerator, $this->userManager, - $this->getMockBuilder(ILogger::class)->getMock(), - $this->getMockBuilder('\OCP\Activity\IManager')->getMock(), + $this->createMock(ILogger::class), + $this->createMock(IManager::class), $this->shareManager, $this->session, $this->previewManager, - $this->getMockBuilder('\OCP\Files\IRootFolder')->getMock(), + $this->createMock(IRootFolder::class), $this->federatedShareProvider, + $this->accountManager, $this->eventDispatcher, $this->l10n, - $this->getMockBuilder('\OCP\Defaults')->getMock() + $this->createMock(Defaults::class) ); @@ -220,6 +230,18 @@ class ShareControllerTest extends \Test\TestCase { $file->method('isReadable')->willReturn(true); $file->method('isShareable')->willReturn(true); + $accountName = $this->createMock(IAccountProperty::class); + $accountName->method('getScope') + ->willReturn(IAccountManager::VISIBILITY_PUBLIC); + $account = $this->createMock(IAccount::class); + $account->method('getProperty') + ->with(IAccountManager::PROPERTY_DISPLAYNAME) + ->willReturn($accountName); + $this->accountManager->expects($this->once()) + ->method('getAccount') + ->with($owner) + ->willReturn($account); + $share = \OC::$server->getShareManager()->newShare(); $share->setId(42); $share->setPassword('password') @@ -289,7 +311,6 @@ class ShareControllerTest extends \Test\TestCase { $response = $this->shareController->showShare(); $sharedTmplParams = [ - 'displayName' => 'ownerDisplay', 'owner' => 'ownerUID', 'filename' => 'file1.txt', 'directory_path' => '/file1.txt', @@ -323,12 +344,160 @@ class ShareControllerTest extends \Test\TestCase { $expectedResponse = new PublicTemplateResponse($this->appName, 'public', $sharedTmplParams); $expectedResponse->setContentSecurityPolicy($csp); $expectedResponse->setHeaderTitle($sharedTmplParams['filename']); - $expectedResponse->setHeaderDetails('shared by ' . $sharedTmplParams['displayName']); + $expectedResponse->setHeaderDetails('shared by ' . $sharedTmplParams['shareOwner']); $expectedResponse->setHeaderActions([ new SimpleMenuAction('download', $this->l10n->t('Download'), 'icon-download-white', $sharedTmplParams['downloadURL'], 0), new SimpleMenuAction('download', $this->l10n->t('Download'), 'icon-download', $sharedTmplParams['downloadURL'], 10, $sharedTmplParams['fileSize']), new LinkMenuAction($this->l10n->t('Direct link'), 'icon-public', $sharedTmplParams['previewURL']), - new ExternalShareMenuAction($this->l10n->t('Add to your Nextcloud'), 'icon-external', $sharedTmplParams['owner'], $sharedTmplParams['displayName'], $sharedTmplParams['filename']), + new ExternalShareMenuAction($this->l10n->t('Add to your Nextcloud'), 'icon-external', $sharedTmplParams['owner'], $sharedTmplParams['shareOwner'], $sharedTmplParams['filename']), + ]); + + $this->assertEquals($expectedResponse, $response); + } + + public function testShowShareWithPrivateName() { + + $note = 'personal note'; + + $this->shareController->setToken('token'); + + $owner = $this->createMock(IUser::class); + $owner->method('getDisplayName')->willReturn('ownerDisplay'); + $owner->method('getUID')->willReturn('ownerUID'); + $owner->method('isEnabled')->willReturn(true); + + $initiator = $this->createMock(IUser::class); + $initiator->method('getDisplayName')->willReturn('initiatorDisplay'); + $initiator->method('getUID')->willReturn('initiatorUID'); + $initiator->method('isEnabled')->willReturn(true); + + $file = $this->createMock(File::class); + $file->method('getName')->willReturn('file1.txt'); + $file->method('getMimetype')->willReturn('text/plain'); + $file->method('getSize')->willReturn(33); + $file->method('isReadable')->willReturn(true); + $file->method('isShareable')->willReturn(true); + + $accountName = $this->createMock(IAccountProperty::class); + $accountName->method('getScope') + ->willReturn(IAccountManager::VISIBILITY_PRIVATE); + $account = $this->createMock(IAccount::class); + $account->method('getProperty') + ->with(IAccountManager::PROPERTY_DISPLAYNAME) + ->willReturn($accountName); + $this->accountManager->expects($this->once()) + ->method('getAccount') + ->with($owner) + ->willReturn($account); + + $share = \OC::$server->getShareManager()->newShare(); + $share->setId(42); + $share->setPassword('password') + ->setShareOwner('ownerUID') + ->setSharedBy('initiatorUID') + ->setNode($file) + ->setNote($note) + ->setTarget('/file1.txt'); + + $this->session->method('exists')->with('public_link_authenticated')->willReturn(true); + $this->session->method('get')->with('public_link_authenticated')->willReturn('42'); + + $this->urlGenerator->expects($this->at(0)) + ->method('linkToRouteAbsolute') + ->with('files_sharing.sharecontroller.downloadShare', ['token' => 'token']) + ->willReturn('downloadURL'); + + $this->previewManager->method('isMimeSupported')->with('text/plain')->willReturn(true); + + $this->config->method('getSystemValue') + ->willReturnMap( + [ + ['max_filesize_animated_gifs_public_sharing', 10, 10], + ['enable_previews', true, true], + ['preview_max_x', 1024, 1024], + ['preview_max_y', 1024, 1024], + ] + ); + $shareTmpl['maxSizeAnimateGif'] = $this->config->getSystemValue('max_filesize_animated_gifs_public_sharing', 10); + $shareTmpl['previewEnabled'] = $this->config->getSystemValue('enable_previews', true); + + $this->shareManager + ->expects($this->once()) + ->method('getShareByToken') + ->with('token') + ->willReturn($share); + $this->config + ->expects($this->once()) + ->method('getAppValue') + ->with('core', 'shareapi_public_link_disclaimertext', null) + ->willReturn('My disclaimer text'); + + $this->userManager->method('get')->willReturnCallback(function(string $uid) use ($owner, $initiator) { + if ($uid === 'ownerUID') { + return $owner; + } + if ($uid === 'initiatorUID') { + return $initiator; + } + return null; + }); + + $this->eventDispatcher->expects($this->once()) + ->method('dispatch') + ->with( + 'OCA\Files_Sharing::loadAdditionalScripts', + $this->callback(function($event) use ($share) { + return $event->getArgument('share') === $share; + }) + ); + + $this->l10n->expects($this->any()) + ->method('t') + ->will($this->returnCallback(function($text, $parameters) { + return vsprintf($text, $parameters); + })); + + $response = $this->shareController->showShare(); + $sharedTmplParams = [ + 'owner' => '', + 'filename' => 'file1.txt', + 'directory_path' => '/file1.txt', + 'mimetype' => 'text/plain', + 'dirToken' => 'token', + 'sharingToken' => 'token', + 'server2serversharing' => true, + 'protected' => 'true', + 'dir' => '', + 'downloadURL' => 'downloadURL', + 'fileSize' => '33 B', + 'nonHumanFileSize' => 33, + 'maxSizeAnimateGif' => 10, + 'previewSupported' => true, + 'previewEnabled' => true, + 'previewMaxX' => 1024, + 'previewMaxY' => 1024, + 'hideFileList' => false, + 'shareOwner' => '', + 'disclaimer' => 'My disclaimer text', + 'shareUrl' => null, + 'previewImage' => null, + 'previewURL' => 'downloadURL', + 'note' => $note, + 'hideDownload' => false, + 'showgridview' => false + ]; + + $csp = new \OCP\AppFramework\Http\ContentSecurityPolicy(); + $csp->addAllowedFrameDomain('\'self\''); + $expectedResponse = new PublicTemplateResponse($this->appName, 'public', $sharedTmplParams); + $expectedResponse->setContentSecurityPolicy($csp); + $expectedResponse->setHeaderTitle($sharedTmplParams['filename']); + $expectedResponse->setHeaderDetails(''); + $expectedResponse->setHeaderActions([ + new SimpleMenuAction('download', $this->l10n->t('Download'), 'icon-download-white', $sharedTmplParams['downloadURL'], 0), + new SimpleMenuAction('download', $this->l10n->t('Download'), 'icon-download', $sharedTmplParams['downloadURL'], 10, $sharedTmplParams['fileSize']), + new LinkMenuAction($this->l10n->t('Direct link'), 'icon-public', $sharedTmplParams['previewURL']), + new ExternalShareMenuAction($this->l10n->t('Add to your Nextcloud'), 'icon-external', $sharedTmplParams['owner'], $sharedTmplParams['shareOwner'], $sharedTmplParams['filename']), ]); $this->assertEquals($expectedResponse, $response); @@ -356,6 +525,18 @@ class ShareControllerTest extends \Test\TestCase { $file->method('isReadable')->willReturn(true); $file->method('isShareable')->willReturn(true); + $accountName = $this->createMock(IAccountProperty::class); + $accountName->method('getScope') + ->willReturn(IAccountManager::VISIBILITY_PUBLIC); + $account = $this->createMock(IAccount::class); + $account->method('getProperty') + ->with(IAccountManager::PROPERTY_DISPLAYNAME) + ->willReturn($accountName); + $this->accountManager->expects($this->once()) + ->method('getAccount') + ->with($owner) + ->willReturn($account); + $share = \OC::$server->getShareManager()->newShare(); $share->setId(42); $share->setPassword('password') @@ -429,7 +610,6 @@ class ShareControllerTest extends \Test\TestCase { $response = $this->shareController->showShare(); $sharedTmplParams = [ - 'displayName' => 'ownerDisplay', 'owner' => 'ownerUID', 'filename' => 'file1.txt', 'directory_path' => '/file1.txt', @@ -463,7 +643,7 @@ class ShareControllerTest extends \Test\TestCase { $expectedResponse = new PublicTemplateResponse($this->appName, 'public', $sharedTmplParams); $expectedResponse->setContentSecurityPolicy($csp); $expectedResponse->setHeaderTitle($sharedTmplParams['filename']); - $expectedResponse->setHeaderDetails('shared by ' . $sharedTmplParams['displayName']); + $expectedResponse->setHeaderDetails('shared by ' . $sharedTmplParams['shareOwner']); $expectedResponse->setHeaderActions([]); $this->assertEquals($expectedResponse, $response); @@ -506,6 +686,18 @@ class ShareControllerTest extends \Test\TestCase { $folder->method('get')->with('')->willReturn($folder); $folder->method('getSize')->willReturn(1337); + $accountName = $this->createMock(IAccountProperty::class); + $accountName->method('getScope') + ->willReturn(IAccountManager::VISIBILITY_PUBLIC); + $account = $this->createMock(IAccount::class); + $account->method('getProperty') + ->with(IAccountManager::PROPERTY_DISPLAYNAME) + ->willReturn($accountName); + $this->accountManager->expects($this->once()) + ->method('getAccount') + ->with($owner) + ->willReturn($account); + $share = \OC::$server->getShareManager()->newShare(); $share->setId(42); $share->setPermissions(Constants::PERMISSION_CREATE) @@ -543,7 +735,6 @@ class ShareControllerTest extends \Test\TestCase { $response->setParams($responseParams); $sharedTmplParams = [ - 'displayName' => 'ownerDisplay', 'owner' => 'ownerUID', 'filename' => '/fileDrop', 'directory_path' => '/fileDrop', @@ -577,7 +768,7 @@ class ShareControllerTest extends \Test\TestCase { $expectedResponse = new PublicTemplateResponse($this->appName, 'public', $sharedTmplParams); $expectedResponse->setContentSecurityPolicy($csp); $expectedResponse->setHeaderTitle($sharedTmplParams['filename']); - $expectedResponse->setHeaderDetails('shared by ' . $sharedTmplParams['displayName']); + $expectedResponse->setHeaderDetails('shared by ' . $sharedTmplParams['shareOwner']); self::assertEquals($expectedResponse, $response); }