Browse Source

Fix shares read permissions (#16761)

Fix shares read permissions
tags/v18.0.0beta1
John Molakvoæ 4 years ago
parent
commit
2169c26195
No account linked to committer's email address

+ 60
- 60
.drone.yml View File

@@ -1123,20 +1123,20 @@ trigger:

---
kind: pipeline
name: integration-sharing-v1
name: integration-checksums-v1

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-sharing-v1
image: nextcloudci/integration-php7.1:2
- name: integration-checksums-v1
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh features/sharing-v1.feature
- ./run.sh features/checksums.feature

trigger:
branch:
@@ -1148,20 +1148,20 @@ trigger:

---
kind: pipeline
name: integration-sharing-v1-part2
name: integration-external-storage

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-sharing-v1-part2
image: nextcloudci/integration-php7.1:2
- name: integration-external-storage
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh features/sharing-v1-part2.feature
- ./run.sh features/external-storage.feature

trigger:
branch:
@@ -1173,20 +1173,20 @@ trigger:

---
kind: pipeline
name: integration-sharing-v1-part3
name: integration-provisioning-v1

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-sharing-v1-part3
image: nextcloudci/integration-php7.1:2
- name: integration-provisioning-v1
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh features/sharing-v1-part3.feature
- ./run.sh features/provisioning-v1.feature

trigger:
branch:
@@ -1198,20 +1198,20 @@ trigger:

---
kind: pipeline
name: integration-checksums-v1
name: integration-tags

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-checksums-v1
image: nextcloudci/integration-php7.1:2
- name: integration-tags
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh features/checksums.feature
- ./run.sh features/tags.feature

trigger:
branch:
@@ -1223,20 +1223,20 @@ trigger:

---
kind: pipeline
name: integration-external-storage
name: integration-caldav

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-external-storage
image: nextcloudci/integration-php7.1:2
- name: integration-caldav
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh features/external-storage.feature
- ./run.sh features/caldav.feature

trigger:
branch:
@@ -1248,20 +1248,20 @@ trigger:

---
kind: pipeline
name: integration-provisioning-v1
name: integration-comments

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-provisioning-v1
image: nextcloudci/integration-php7.1:2
- name: integration-comments
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh features/provisioning-v1.feature
- ./run.sh features/comments.feature

trigger:
branch:
@@ -1273,20 +1273,20 @@ trigger:

---
kind: pipeline
name: integration-tags
name: integration-comments-search

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-tags
image: nextcloudci/integration-php7.1:2
- name: integration-comments-search
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh features/tags.feature
- ./run.sh features/comments-search.feature

trigger:
branch:
@@ -1298,20 +1298,20 @@ trigger:

---
kind: pipeline
name: integration-caldav
name: integration-favorites

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-caldav
image: nextcloudci/integration-php7.1:2
- name: integration-favorites
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh features/caldav.feature
- ./run.sh features/favorites.feature

trigger:
branch:
@@ -1323,20 +1323,20 @@ trigger:

---
kind: pipeline
name: integration-comments
name: integration-provisioning-v2

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-comments
image: nextcloudci/integration-php7.1:2
- name: integration-provisioning-v2
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh features/comments.feature
- ./run.sh features/provisioning-v2.feature

trigger:
branch:
@@ -1348,20 +1348,20 @@ trigger:

---
kind: pipeline
name: integration-comments-search
name: integration-webdav-related

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-comments-search
image: nextcloudci/integration-php7.1:2
- name: integration-webdav-related
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh features/comments-search.feature
- ./run.sh features/webdav-related.feature

trigger:
branch:
@@ -1373,20 +1373,20 @@ trigger:

---
kind: pipeline
name: integration-favorites
name: integration-sharees-features

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-favorites
image: nextcloudci/integration-php7.1:2
- name: integration-sharees-features
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh features/favorites.feature
- ./run.sh sharees_features/sharees.feature

trigger:
branch:
@@ -1398,20 +1398,20 @@ trigger:

---
kind: pipeline
name: integration-provisioning-v2
name: integration-sharees-v2-features

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-provisioning-v2
image: nextcloudci/integration-php7.1:2
- name: integration-sharees-v2-features
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh features/provisioning-v2.feature
- ./run.sh sharees_features/sharees_provisioningapiv2.feature

trigger:
branch:
@@ -1423,20 +1423,20 @@ trigger:

---
kind: pipeline
name: integration-webdav-related
name: integration-sharing-v1

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-webdav-related
image: nextcloudci/integration-php7.1:2
- name: integration-sharing-v1
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh features/webdav-related.feature
- ./run.sh sharing_features/sharing-v1.feature

trigger:
branch:
@@ -1448,20 +1448,20 @@ trigger:

---
kind: pipeline
name: integration-sharees-features
name: integration-sharing-v1-part2

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-sharees-features
image: nextcloudci/integration-php7.1:2
- name: integration-sharing-v1-part2
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh sharees_features/sharees.feature
- ./run.sh sharing_features/sharing-v1-part2.feature

trigger:
branch:
@@ -1473,20 +1473,20 @@ trigger:

---
kind: pipeline
name: integration-sharees-v2-features
name: integration-sharing-v1-part3

steps:
- name: submodules
image: docker:git
commands:
- git submodule update --init
- name: integration-sharees-v2-features
image: nextcloudci/integration-php7.1:2
- name: integration-sharing-v1-part3
image: nextcloudci/integration-php7.1:1
commands:
- bash tests/drone-run-integration-tests.sh || exit 0
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
- cd build/integration
- ./run.sh sharees_features/sharees_provisioningapiv2.feature
- ./run.sh sharing_features/sharing-v1-part3.feature

trigger:
branch:

+ 138
- 15
apps/files_sharing/lib/Controller/ShareAPIController.php View File

@@ -305,13 +305,13 @@ class ShareAPIController extends OCSController {
throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist'));
}

if ($this->canAccessShare($share)) {
try {
try {
if ($this->canAccessShare($share)) {
$share = $this->formatShare($share);
return new DataResponse([$share]);
} catch (NotFoundException $e) {
//Fall trough
}
} catch (NotFoundException $e) {
// Fall trough
}

throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist'));
@@ -336,21 +336,24 @@ class ShareAPIController extends OCSController {
try {
$this->lock($share->getNode());
} catch (LockedException $e) {
throw new OCSNotFoundException($this->l->t('could not delete share'));
throw new OCSNotFoundException($this->l->t('Could not delete share'));
}

if (!$this->canAccessShare($share)) {
throw new OCSNotFoundException($this->l->t('Could not delete share'));
throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist'));
}

if ((
$share->getShareType() === Share::SHARE_TYPE_GROUP
|| $share->getShareType() === Share::SHARE_TYPE_ROOM
)
&& $share->getShareOwner() !== $this->currentUser
&& $share->getSharedBy() !== $this->currentUser) {
// if it's a group share or a room share
// we don't delete the share, but only the
// mount point. Allowing it to be restored
// from the deleted shares
if ($this->canDeleteShareFromSelf($share)) {
$this->shareManager->deleteFromSelf($share, $this->currentUser);
} else {
if (!$this->canDeleteShare($share)) {
throw new OCSForbiddenException($this->l->t('Could not delete share'));
}

$this->shareManager->deleteShare($share);
}

@@ -500,7 +503,6 @@ class ShareAPIController extends OCSController {
}
}


if ($sendPasswordByTalk === 'true') {
if (!$this->appManager->isEnabledForUser('spreed')) {
throw new OCSForbiddenException($this->l->t('Sharing %s sending the password by Nextcloud Talk failed because Nextcloud Talk is not enabled', [$path->getPath()]));
@@ -823,7 +825,7 @@ class ShareAPIController extends OCSController {
throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist'));
}

if ($share->getShareOwner() !== $this->currentUser && $share->getSharedBy() !== $this->currentUser) {
if (!$this->canEditShare($share)) {
throw new OCSForbiddenException('You are not allowed to edit incoming shares');
}

@@ -981,6 +983,13 @@ class ShareAPIController extends OCSController {
}

/**
* Does the user have read permission on the share
*
* @param \OCP\Share\IShare $share the share to check
* @param boolean $checkGroups check groups as well?
* @return boolean
* @throws NotFoundException
*
* @suppress PhanUndeclaredClassMethod
*/
protected function canAccessShare(\OCP\Share\IShare $share, bool $checkGroups = true): bool {
@@ -995,12 +1004,21 @@ class ShareAPIController extends OCSController {
return true;
}

// If the share is shared with you (or a group you are a member of)
// If the share is shared with you, you can access it!
if ($share->getShareType() === Share::SHARE_TYPE_USER
&& $share->getSharedWith() === $this->currentUser) {
return true;
}

// Have reshare rights on the shared file/folder ?
// Does the currentUser have access to the shared file?
$userFolder = $this->rootFolder->getUserFolder($this->currentUser);
$files = $userFolder->getById($share->getNodeId());
if (!empty($files) && $this->shareProviderResharingRights($this->currentUser, $share, $files[0])) {
return true;
}

// If in the recipient group, you can see the share
if ($checkGroups && $share->getShareType() === Share::SHARE_TYPE_GROUP) {
$sharedWith = $this->groupManager->get($share->getSharedWith());
$user = $this->userManager->get($this->currentUser);
@@ -1025,6 +1043,110 @@ class ShareAPIController extends OCSController {
return false;
}

/**
* Does the user have edit permission on the share
*
* @param \OCP\Share\IShare $share the share to check
* @return boolean
*/
protected function canEditShare(\OCP\Share\IShare $share): bool {
// A file with permissions 0 can't be accessed by us. So Don't show it
if ($share->getPermissions() === 0) {
return false;
}

// The owner of the file and the creator of the share
// can always edit the share
if ($share->getShareOwner() === $this->currentUser ||
$share->getSharedBy() === $this->currentUser
) {
return true;
}

//! we do NOT support some kind of `admin` in groups.
//! You cannot edit shares shared to a group you're
//! a member of if you're not the share owner or the file owner!

return false;
}

/**
* Does the user have delete permission on the share
*
* @param \OCP\Share\IShare $share the share to check
* @return boolean
*/
protected function canDeleteShare(\OCP\Share\IShare $share): bool {
// A file with permissions 0 can't be accessed by us. So Don't show it
if ($share->getPermissions() === 0) {
return false;
}

// if the user is the recipient, i can unshare
// the share with self
if ($share->getShareType() === Share::SHARE_TYPE_USER &&
$share->getSharedWith() === $this->currentUser
) {
return true;
}

// The owner of the file and the creator of the share
// can always delete the share
if ($share->getShareOwner() === $this->currentUser ||
$share->getSharedBy() === $this->currentUser
) {
return true;
}

return false;
}

/**
* Does the user have delete permission on the share
* This differs from the canDeleteShare function as it only
* remove the share for the current user. It does NOT
* completely delete the share but only the mount point.
* It can then be restored from the deleted shares section.
*
* @param \OCP\Share\IShare $share the share to check
* @return boolean
*
* @suppress PhanUndeclaredClassMethod
*/
protected function canDeleteShareFromSelf(\OCP\Share\IShare $share): bool {
if ($share->getShareType() !== Share::SHARE_TYPE_GROUP &&
$share->getShareType() !== Share::SHARE_TYPE_ROOM
) {
return false;
}

if ($share->getShareOwner() === $this->currentUser ||
$share->getSharedBy() === $this->currentUser
) {
// Delete the whole share, not just for self
return false;
}

// If in the recipient group, you can delete the share from self
if ($share->getShareType() === Share::SHARE_TYPE_GROUP) {
$sharedWith = $this->groupManager->get($share->getSharedWith());
$user = $this->userManager->get($this->currentUser);
if ($user !== null && $sharedWith !== null && $sharedWith->inGroup($user)) {
return true;
}
}

if ($share->getShareType() === Share::SHARE_TYPE_ROOM) {
try {
return $this->getRoomShareHelper()->canAccessShare($share, $this->currentUser);
} catch (QueryException $e) {
return false;
}
}

return false;
}

/**
* Make sure that the passed date is valid ISO 8601
* So YYYY-MM-DD
@@ -1201,4 +1323,5 @@ class ShareAPIController extends OCSController {

return false;
}

}

+ 265
- 3
apps/files_sharing/tests/Controller/ShareAPIControllerTest.php View File

@@ -213,19 +213,20 @@ class ShareAPIControllerTest extends TestCase {

/**
* @expectedException \OCP\AppFramework\OCS\OCSNotFoundException
* @expectedExceptionMessage could not delete share
* @expectedExceptionMessage Could not delete share
*/
public function testDeleteShareLocked() {
$node = $this->getMockBuilder(File::class)->getMock();

$share = $this->newShare();
$share->setSharedBy($this->currentUser)
->setNode($node);
$share->setNode($node);
$this->shareManager
->expects($this->once())
->method('getShareById')
->with('ocinternal:42')
->willReturn($share);

$this->shareManager
->expects($this->never())
->method('deleteShare')
@@ -235,6 +236,223 @@ class ShareAPIControllerTest extends TestCase {
->method('lock')
->with(\OCP\Lock\ILockingProvider::LOCK_SHARED)
->will($this->throwException(new LockedException('mypath')));
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteFromSelf', [$share]));
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteShare', [$share]));

$this->ocs->deleteShare(42);
}

/**
* You can always remove a share that was shared with you
*/
public function testDeleteShareWithMe() {
$node = $this->getMockBuilder(File::class)->getMock();

$share = $this->newShare();
$share->setSharedWith($this->currentUser)
->setShareType(\OCP\Share::SHARE_TYPE_USER)
->setNode($node);
$this->shareManager
->expects($this->once())
->method('getShareById')
->with('ocinternal:42')
->willReturn($share);

$this->shareManager
->expects($this->once())
->method('deleteShare')
->with($share);

$node->expects($this->once())
->method('lock')
->with(\OCP\Lock\ILockingProvider::LOCK_SHARED);
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteFromSelf', [$share]));
$this->assertTrue($this->invokePrivate($this->ocs, 'canDeleteShare', [$share]));

$this->ocs->deleteShare(42);
}

/**
* You can always delete a share you own
*/
public function testDeleteShareOwner() {
$node = $this->getMockBuilder(File::class)->getMock();

$share = $this->newShare();
$share->setSharedBy($this->currentUser)
->setNode($node);

$this->shareManager
->expects($this->once())
->method('getShareById')
->with('ocinternal:42')
->willReturn($share);

$this->shareManager
->expects($this->once())
->method('deleteShare')
->with($share);

$node->expects($this->once())
->method('lock')
->with(\OCP\Lock\ILockingProvider::LOCK_SHARED);
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteFromSelf', [$share]));
$this->assertTrue($this->invokePrivate($this->ocs, 'canDeleteShare', [$share]));

$this->ocs->deleteShare(42);
}

/**
* You can always delete a share when you own
* the file path it belong to
*/
public function testDeleteShareFileOwner() {
$node = $this->getMockBuilder(File::class)->getMock();

$share = $this->newShare();
$share->setShareOwner($this->currentUser)
->setNode($node);

$this->shareManager
->expects($this->once())
->method('getShareById')
->with('ocinternal:42')
->willReturn($share);

$this->shareManager
->expects($this->once())
->method('deleteShare')
->with($share);

$node->expects($this->once())
->method('lock')
->with(\OCP\Lock\ILockingProvider::LOCK_SHARED);
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteFromSelf', [$share]));
$this->assertTrue($this->invokePrivate($this->ocs, 'canDeleteShare', [$share]));

$this->ocs->deleteShare(42);
}

/**
* You can remove (the mountpoint, not the share)
* a share if you're in the group the share is shared with
*/
public function testDeleteSharedWithMyGroup() {
$node = $this->getMockBuilder(File::class)->getMock();

$share = $this->newShare();
$share->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
->setSharedWith('group')
->setNode($node);

$this->shareManager
->expects($this->once())
->method('getShareById')
->with('ocinternal:42')
->willReturn($share);

// canDeleteShareFromSelf
$user = $this->createMock(IUser::class);
$group = $this->getMockBuilder('OCP\IGroup')->getMock();
$this->groupManager
->method('get')
->with('group')
->willReturn($group);
$this->userManager
->method('get')
->with($this->currentUser)
->willReturn($user);
$group->method('inGroup')
->with($user)
->willReturn(true);

$node->expects($this->once())
->method('lock')
->with(\OCP\Lock\ILockingProvider::LOCK_SHARED);

$userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock();
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);

$userFolder->method('getById')
->with($share->getNodeId())
->willReturn([$share->getNode()]);

$this->shareManager->expects($this->once())
->method('deleteFromSelf')
->with($share, $this->currentUser);
$this->shareManager->expects($this->never())
->method('deleteShare');
$this->assertTrue($this->invokePrivate($this->ocs, 'canDeleteShareFromSelf', [$share]));
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteShare', [$share]));

$this->ocs->deleteShare(42);
}

/**
* You cannot remove a share if you're not
* in the group the share is shared with
* @expectedException \OCP\AppFramework\OCS\OCSNotFoundException
* @expectedExceptionMessage Wrong share ID, share doesn't exist
*/
public function testDeleteSharedWithGroupIDontBelongTo() {
$node = $this->getMockBuilder(File::class)->getMock();

$share = $this->newShare();
$share->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
->setSharedWith('group')
->setNode($node);

$this->shareManager
->expects($this->once())
->method('getShareById')
->with('ocinternal:42')
->willReturn($share);

// canDeleteShareFromSelf
$user = $this->createMock(IUser::class);
$group = $this->getMockBuilder('OCP\IGroup')->getMock();
$this->groupManager
->method('get')
->with('group')
->willReturn($group);
$this->userManager
->method('get')
->with($this->currentUser)
->willReturn($user);
$group->method('inGroup')
->with($user)
->willReturn(false);

$node->expects($this->once())
->method('lock')
->with(\OCP\Lock\ILockingProvider::LOCK_SHARED);

$userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock();
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);

$userFolder->method('getById')
->with($share->getNodeId())
->willReturn([$share->getNode()]);

$this->shareManager->expects($this->never())
->method('deleteFromSelf');
$this->shareManager->expects($this->never())
->method('deleteShare');
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteShareFromSelf', [$share]));
$this->assertFalse($this->invokePrivate($this->ocs, 'canDeleteShare', [$share]));

$this->ocs->deleteShare(42);
}
@@ -558,6 +776,11 @@ class ShareAPIControllerTest extends TestCase {
->with('ocinternal:42', 'currentUser')
->willReturn($share);

$userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock();
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);

$this->ocs->getShare(42);
}

@@ -575,6 +798,27 @@ class ShareAPIControllerTest extends TestCase {
$share->method('getSharedWith')->willReturn($this->currentUser);
$this->assertTrue($this->invokePrivate($this->ocs, 'canAccessShare', [$share]));

$file = $this->getMockBuilder(File::class)->getMock();

$userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock();
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);

$userFolder->method('getById')
->with($share->getNodeId())
->willReturn([$file]);

$file->method('getPermissions')
->will($this->onConsecutiveCalls(\OCP\Constants::PERMISSION_SHARE, \OCP\Constants::PERMISSION_READ));

// getPermissions -> share
$share = $this->getMockBuilder(IShare::class)->getMock();
$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_USER);
$share->method('getSharedWith')->willReturn($this->getMockBuilder(IUser::class)->getMock());
$this->assertTrue($this->invokePrivate($this->ocs, 'canAccessShare', [$share]));

// getPermissions -> read
$share = $this->getMockBuilder(IShare::class)->getMock();
$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_USER);
$share->method('getSharedWith')->willReturn($this->getMockBuilder(IUser::class)->getMock());
@@ -652,6 +896,15 @@ class ShareAPIControllerTest extends TestCase {
* @param bool canAccessShareByHelper
*/
public function testCanAccessRoomShare(bool $expected, \OCP\Share\IShare $share, bool $helperAvailable, bool $canAccessShareByHelper) {
$userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock();
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);

$userFolder->method('getById')
->with($share->getNodeId())
->willReturn([$share->getNode()]);

if (!$helperAvailable) {
$this->appManager->method('isEnabledForUser')
->with('spreed')
@@ -1527,6 +1780,15 @@ class ShareAPIControllerTest extends TestCase {

$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);

$userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock();
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);

$userFolder->method('getById')
->with($share->getNodeId())
->willReturn([$share->getNode()]);

$this->ocs->updateShare(42);
}


+ 4
- 4
apps/oauth2/js/oauth2.js
File diff suppressed because it is too large
View File


+ 1
- 1
apps/oauth2/js/oauth2.js.map
File diff suppressed because it is too large
View File


+ 10
- 0
build/integration/config/behat.yml View File

@@ -55,6 +55,16 @@ default:
- admin
- admin
regular_user_password: 123456
sharing:
paths:
- "%paths.base%/../sharing_features"
contexts:
- SharingContext:
baseUrl: http://localhost:8080/ocs/
admin:
- admin
- admin
regular_user_password: 123456
setup:
paths:
- "%paths.base%/../setup_features"

+ 0
- 2
build/integration/features/bootstrap/Sharing.php View File

@@ -218,8 +218,6 @@ trait Sharing {
} catch (\GuzzleHttp\Exception\ClientException $ex) {
$this->response = $ex->getResponse();
}

Assert::assertEquals(200, $this->response->getStatusCode());
}

public function createShare($user,

+ 40
- 0
build/integration/features/bootstrap/SharingContext.php View File

@@ -0,0 +1,40 @@
<?php
/**
*
*
* @author Joas Schilling <coding@schilljs.com>
* @author Roeland Jago Douma <roeland@famdouma.nl>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
use Behat\Behat\Context\Context;
use Behat\Behat\Context\SnippetAcceptingContext;
use PHPUnit\Framework\Assert;
use Psr\Http\Message\ResponseInterface;

require __DIR__ . '/../../vendor/autoload.php';


/**
* Features context.
*/
class SharingContext implements Context, SnippetAcceptingContext {
use Sharing;
use AppConfiguration;

protected function resetAppConfigs() {}
}

+ 261
- 0
build/integration/run-docker.sh View File

@@ -0,0 +1,261 @@
#!/usr/bin/env bash

# @copyright Copyright (c) 2017, Daniel Calviño Sánchez (danxuliu@gmail.com)
# @copyright Copyright (c) 2018, Daniel Calviño Sánchez (danxuliu@gmail.com)
#
# @license GNU AGPL version 3 or any later version
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License as
# published by the Free Software Foundation, either version 3 of the
# License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU Affero General Public License for more details.
#
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

# Helper script to run the integration tests on a fresh Nextcloud server through
# Docker.
#
# The integration tests are run in its own Docker container; the grandparent
# directory of the integration tests directory (that is, the root directory of
# the Nextcloud server) is copied to the container and the integration tests are
# run inside it; in the container the configuration/data from the original
# Nextcloud server is ignored, and a new server installation is performed inside
# the container instead. Once the tests end the container is stopped.
#
# To perform its job, the script requires the "docker" command to be available.
#
# The Docker Command Line Interface (the "docker" command) requires special
# permissions to talk to the Docker daemon, and those permissions are typically
# available only to the root user. Please see the Docker documentation to find
# out how to give access to a regular user to the Docker daemon:
# https://docs.docker.com/engine/installation/linux/linux-postinstall/
#
# Note, however, that being able to communicate with the Docker daemon is the
# same as being able to get root privileges for the system. Therefore, you must
# give access to the Docker daemon (and thus run this script as) ONLY to trusted
# and secure users:
# https://docs.docker.com/engine/security/security/#docker-daemon-attack-surface
#
# Finally, take into account that this script will automatically remove the
# Docker containers named "database-nextcloud-local-test-integration" and
# "nextcloud-local-test-integration", even if the script did not create them
# (probably you will not have containers nor images with that name, but just in
# case).

# Sets the variables that abstract the differences in command names and options
# between operating systems.
#
# Switches between mktemp on GNU/Linux and gmktemp on macOS.
function setOperatingSystemAbstractionVariables() {
case "$OSTYPE" in
darwin*)
if [ "$(which gtimeout)" == "" ]; then
echo "Please install coreutils (brew install coreutils)"
exit 1
fi

MKTEMP=gmktemp
TIMEOUT=gtimeout
;;
linux*)
MKTEMP=mktemp
TIMEOUT=timeout
;;
*)
echo "Operating system ($OSTYPE) not supported"
exit 1
;;
esac
}

# Launches the database server in a Docker container.
#
# No server is started if "SQLite" is being used; in other cases the database
# is set up as needed and generic "$DATABASE_NAME/USER/PASSWORD" variables
# (independent of the database type) are set to be used when installing the
# Nextcloud server.
#
# The Docker container started here will be automatically stopped when the
# script exits (see cleanUp). If the database server can not be started then the
# script will be exited immediately with an error state.
function prepareDatabase() {
if [ "$DATABASE" = "sqlite" ]; then
return
fi

DATABASE_CONTAINER=database-nextcloud-local-test-integration

DATABASE_NAME=oc_autotest
DATABASE_USER=oc_autotest
DATABASE_PASSWORD=nextcloud

DATABASE_CONTAINER_OPTIONS="--env MYSQL_ROOT_PASSWORD=nextcloud_root --env MYSQL_USER=$DATABASE_USER --env MYSQL_PASSWORD=$DATABASE_PASSWORD --env MYSQL_DATABASE=$DATABASE_NAME"
if [ "$DATABASE" = "pgsql" ]; then
DATABASE_CONTAINER_OPTIONS=" --env POSTGRES_USER=$DATABASE_USER --env POSTGRES_PASSWORD=$DATABASE_PASSWORD --env POSTGRES_DB=${DATABASE_NAME}_dummy"
fi

echo "Starting database server"
docker run --detach --name=$DATABASE_CONTAINER $DATABASE_CONTAINER_OPTIONS $DATABASE_IMAGE

DATABASE_IP=$(docker inspect --format='{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' $DATABASE_CONTAINER)

DATABASE_PORT=3306
if [ "$DATABASE" = "pgsql" ]; then
DATABASE_PORT=5432
fi

echo "Waiting for database server to be ready"
if ! $TIMEOUT 600s bash -c "while ! (</dev/tcp/$DATABASE_IP/$DATABASE_PORT) >/dev/null 2>&1; do sleep 1; done"; then
echo "Could not start database server after 600 seconds" >&2

exit 1
fi
}

# Creates a Docker container to run the integration tests.
#
# This function starts a Docker container with a copy of the Nextcloud code from
# the grandparent directory, although ignoring any configuration or data that it
# may provide (for example, if that directory was used directly to deploy a
# Nextcloud instance in a web server). As the Nextcloud code is copied to the
# container instead of referenced the original code can be modified while the
# integration tests are running without interfering in them.
function prepareDocker() {
NEXTCLOUD_LOCAL_CONTAINER=nextcloud-local-test-integration

NEXTCLOUD_LOCAL_CONTAINER_NETWORK_OPTIONS=""
if [ -n "$DATABASE_CONTAINER" ]; then
# The network stack is shared between the database and the Nextcloud
# container, so the Nextcloud server can access the database directly on
# 127.0.0.1.
NEXTCLOUD_LOCAL_CONTAINER_NETWORK_OPTIONS="--network=container:$DATABASE_CONTAINER"
fi

echo "Starting the Nextcloud container"
# When using "nextcloudci/phpX.Y" images the container exits immediately if
# no command is given, so a Bash session is created to prevent that.
docker run --detach --name=$NEXTCLOUD_LOCAL_CONTAINER $NEXTCLOUD_LOCAL_CONTAINER_NETWORK_OPTIONS --interactive --tty $NEXTCLOUD_LOCAL_IMAGE bash

# Use the $TMPDIR or, if not set, fall back to /tmp.
NEXTCLOUD_LOCAL_TAR="$($MKTEMP --tmpdir="${TMPDIR:-/tmp}" --suffix=.tar nextcloud-local-XXXXXXXXXX)"

# Setting the user and group of files in the tar would be superfluous, as
# "docker cp" does not take them into account (the extracted files are set
# to root).
echo "Copying local Git working directory of Nextcloud to the container"
tar --create --file="$NEXTCLOUD_LOCAL_TAR" \
--exclude=".git" \
--exclude="./config/config.php" \
--exclude="./data" \
--exclude="./data-autotest" \
--exclude="./tests" \
--exclude="node_modules" \
--directory=../../ \
.

docker exec $NEXTCLOUD_LOCAL_CONTAINER mkdir /nextcloud
docker cp - $NEXTCLOUD_LOCAL_CONTAINER:/nextcloud/ < "$NEXTCLOUD_LOCAL_TAR"

# Database options are needed only when a database other than SQLite is
# used.
NEXTCLOUD_LOCAL_CONTAINER_INSTALL_DATABASE_OPTIONS=""
if [ -n "$DATABASE_CONTAINER" ]; then
NEXTCLOUD_LOCAL_CONTAINER_INSTALL_DATABASE_OPTIONS="--database=$DATABASE --database-name=$DATABASE_NAME --database-user=$DATABASE_USER --database-pass=$DATABASE_PASSWORD --database-host=127.0.0.1"
fi

echo "Installing Nextcloud in the container"
docker exec $NEXTCLOUD_LOCAL_CONTAINER bash -c "cd nextcloud && php occ maintenance:install --admin-pass=admin $NEXTCLOUD_LOCAL_CONTAINER_INSTALL_DATABASE_OPTIONS"
}

# Removes/stops temporal elements created/started by this script.
function cleanUp() {
# Disable (yes, "+" disables) exiting immediately on errors to ensure that
# all the cleanup commands are executed (well, no errors should occur during
# the cleanup anyway, but just in case).
set +o errexit

echo "Cleaning up"

if [ -f "$NEXTCLOUD_LOCAL_TAR" ]; then
echo "Removing $NEXTCLOUD_LOCAL_TAR"
rm $NEXTCLOUD_LOCAL_TAR
fi

# The name filter must be specified as "^/XXX$" to get an exact match; using
# just "XXX" would match every name that contained "XXX".
if [ -n "$(docker ps --all --quiet --filter name="^/$NEXTCLOUD_LOCAL_CONTAINER$")" ]; then
echo "Removing Docker container $NEXTCLOUD_LOCAL_CONTAINER"
docker rm --volumes --force $NEXTCLOUD_LOCAL_CONTAINER
fi

if [ -n "$DATABASE_CONTAINER" -a -n "$(docker ps --all --quiet --filter name="^/$DATABASE_CONTAINER$")" ]; then
echo "Removing Docker container $DATABASE_CONTAINER"
docker rm --volumes --force $DATABASE_CONTAINER
fi
}

# Exit immediately on errors.
set -o errexit

# Execute cleanUp when the script exits, either normally or due to an error.
trap cleanUp EXIT

# Ensure working directory is script directory, as some actions (like copying
# the Git working directory to the container) expect that.
cd "$(dirname $0)"

# "--image XXX" option can be provided to set the Docker image to use to run
# the integration tests (one of the "nextcloudci/phpX.Y:phpX.Y-Z" images).
NEXTCLOUD_LOCAL_IMAGE="nextcloudci/php7.1:php7.1-15"
if [ "$1" = "--image" ]; then
NEXTCLOUD_LOCAL_IMAGE=$2

shift 2
fi

# "--database XXX" option can be provided to set the database to use to run the
# integration tests (one of "sqlite", "mysql" or "pgsql"; "sqlite" is used
# by default).
DATABASE="sqlite"
if [ "$1" = "--database" ]; then
DATABASE=$2

shift 2
fi

if [ "$DATABASE" != "sqlite" ] && [ "$DATABASE" != "mysql" ] && [ "$DATABASE" != "pgsql" ]; then
echo "--database must be followed by one of: sqlite, mysql or pgsql"

exit 1
fi

# "--database-image XXX" option can be provided to set the Docker image to use
# for the database container (ignored when using "sqlite").
if [ "$DATABASE" = "mysql" ]; then
DATABASE_IMAGE="mysql:5.7"
elif [ "$DATABASE" = "pgsql" ]; then
DATABASE_IMAGE="postgres:10"
fi
if [ "$1" = "--database-image" ]; then
DATABASE_IMAGE=$2

shift 2
fi

# If no parameter is provided to this script all the integration tests are run.
SCENARIO_TO_RUN=$1

setOperatingSystemAbstractionVariables

prepareDatabase
prepareDocker

echo "Running tests"
# --tty is needed to get colourful output.
docker exec --tty $NEXTCLOUD_LOCAL_CONTAINER bash -c "cd nextcloud/build/integration && ./run.sh $SCENARIO_TO_RUN"

build/integration/features/sharing-v1-part2.feature → build/integration/sharing_features/sharing-v1-part2.feature View File

@@ -321,6 +321,36 @@ Feature: sharing
Then the OCS status code should be "404"
And the HTTP status code should be "200"

Scenario: Get a share with a user with resharing rights
Given user "user0" exists
And user "user1" exists
And user "user2" exists
And file "textfile0.txt" of user "user0" is shared with user "user1"
And file "textfile0.txt" of user "user0" is shared with user "user2"
And As an "user1"
When Getting info of last share
Then the OCS status code should be "100"
And the HTTP status code should be "200"
And Share fields of last share match with
| id | A_NUMBER |
| item_type | file |
| item_source | A_NUMBER |
| share_type | 0 |
| share_with | user2 |
| file_source | A_NUMBER |
| file_target | /textfile0.txt |
| path | /textfile0 (2).txt |
| permissions | 19 |
| stime | A_NUMBER |
| storage | A_NUMBER |
| mail_send | 0 |
| uid_owner | user0 |
| storage_id | shared::/textfile0 (2).txt |
| file_parent | A_NUMBER |
| share_with_displayname | user2 |
| displayname_owner | user0 |
| mimetype | text/plain |

Scenario: Share of folder and sub-folder to same user - core#20645
Given As an "admin"
And user "user0" exists
@@ -409,6 +439,27 @@ Feature: sharing
Then the OCS status code should be "100"
And the HTTP status code should be "200"

Scenario: delete a share with a user that didn't receive the share
Given user "user0" exists
And user "user1" exists
And user "user2" exists
And file "textfile0.txt" of user "user0" is shared with user "user1"
And As an "user2"
When Deleting last share
Then the OCS status code should be "404"
And the HTTP status code should be "200"

Scenario: delete a share with a user with resharing rights that didn't receive the share
Given user "user0" exists
And user "user1" exists
And user "user2" exists
And file "textfile0.txt" of user "user0" is shared with user "user1"
And file "textfile0.txt" of user "user0" is shared with user "user2"
And As an "user1"
When Deleting last share
Then the OCS status code should be "403"
And the HTTP status code should be "401"

Scenario: Keep usergroup shares (#22143)
Given As an "admin"
And user "user0" exists
@@ -455,6 +506,7 @@ Feature: sharing
When Updating last share with
| permissions | 1 |
Then the OCS status code should be "100"
And the HTTP status code should be "200"

Scenario: Do not allow reshare to exceed permissions
Given user "user0" exists
@@ -476,6 +528,7 @@ Feature: sharing
When Updating last share with
| permissions | 31 |
Then the OCS status code should be "404"
And the HTTP status code should be "200"

Scenario: Do not allow sub reshare to exceed permissions
Given user "user0" exists
@@ -498,6 +551,7 @@ Feature: sharing
When Updating last share with
| permissions | 31 |
Then the OCS status code should be "404"
And the HTTP status code should be "200"

Scenario: Only allow 1 link share per file/folder
Given user "user0" exists

build/integration/features/sharing-v1-part3.feature → build/integration/sharing_features/sharing-v1-part3.feature View File

@@ -127,6 +127,30 @@ Feature: sharing
Then the OCS status code should be "997"
And the HTTP status code should be "401"

Scenario: Deleting a group share as its owner
Given As an "admin"
And user "user0" exists
And user "user1" exists
And group "group1" exists
And user "user0" belongs to group "group1"
And user "user1" belongs to group "group1"
And As an "user0"
And creating a share with
| path | welcome.txt |
| shareType | 1 |
| shareWith | group1 |
When As an "user0"
And Deleting last share
Then the OCS status code should be "100"
And the HTTP status code should be "200"
And Getting info of last share
And the OCS status code should be "404"
And the HTTP status code should be "200"
And As an "user1"
And Getting info of last share
And the OCS status code should be "404"
And the HTTP status code should be "200"

Scenario: Deleting a group share as user
Given As an "admin"
And user "user0" exists
@@ -337,6 +361,46 @@ Feature: sharing
Then etag of element "/" of user "user1" has changed
And etag of element "/PARENT" of user "user0" has not changed

Scenario: do not allow to increase permissions on received share
Given As an "admin"
And user "user0" exists
And user "user1" exists
And user "user0" created a folder "/TMP"
And As an "user0"
And creating a share with
| path | TMP |
| shareType | 0 |
| shareWith | user1 |
| permissions | 17 |
When As an "user1"
And Updating last share with
| permissions | 19 |
Then the OCS status code should be "403"
And the HTTP status code should be "401"

Scenario: do not allow to increase permissions on non received share with user with resharing rights
Given As an "admin"
And user "user0" exists
And user "user1" exists
And user "user2" exists
And user "user0" created a folder "/TMP"
And As an "user0"
And creating a share with
| path | TMP |
| shareType | 0 |
| shareWith | user1 |
| permissions | 31 |
And creating a share with
| path | TMP |
| shareType | 0 |
| shareWith | user2 |
| permissions | 17 |
When As an "user1"
And Updating last share with
| permissions | 19 |
Then the OCS status code should be "403"
And the HTTP status code should be "401"

Scenario: do not allow to increase link share permissions on reshare
Given As an "admin"
And user "user0" exists

build/integration/features/sharing-v1.feature → build/integration/sharing_features/sharing-v1.feature View File

@@ -118,6 +118,7 @@ Feature: sharing
And Updating last share with
| expireDate | +3 days |
And the OCS status code should be "100"
And the HTTP status code should be "200"
And Getting info of last share
Then the OCS status code should be "100"
And the HTTP status code should be "200"
@@ -150,6 +151,7 @@ Feature: sharing
And Updating last share with
| password | publicpw |
And the OCS status code should be "100"
And the HTTP status code should be "200"
And Getting info of last share
Then the OCS status code should be "100"
And the HTTP status code should be "200"
@@ -181,6 +183,7 @@ Feature: sharing
And Updating last share with
| permissions | 7 |
And the OCS status code should be "100"
And the HTTP status code should be "200"
And Getting info of last share
Then the OCS status code should be "100"
And the HTTP status code should be "200"
@@ -212,6 +215,7 @@ Feature: sharing
And Updating last share with
| permissions | 4 |
And the OCS status code should be "100"
And the HTTP status code should be "200"
And Getting info of last share
Then the OCS status code should be "100"
And the HTTP status code should be "200"
@@ -243,6 +247,7 @@ Feature: sharing
And Updating last share with
| publicUpload | true |
And the OCS status code should be "100"
And the HTTP status code should be "200"
And Getting info of last share
Then the OCS status code should be "100"
And the HTTP status code should be "200"
@@ -300,4 +305,19 @@ Feature: sharing
And User "user2" should be included in the response
And User "user3" should not be included in the response

Scenario: getting all shares of a file with a user with resharing rights
Given user "user0" exists
And user "user1" exists
And user "user2" exists
And user "user3" exists
And file "textfile0.txt" of user "user0" is shared with user "user1"
And file "textfile0.txt" of user "user0" is shared with user "user2"
And As an "user1"
When sending "GET" to "/apps/files_sharing/api/v1/shares?path=textfile0 (2).txt&reshares=true"
Then the OCS status code should be "100"
And the HTTP status code should be "200"
And User "user1" should be included in the response
And User "user2" should be included in the response
And User "user3" should not be included in the response

# See sharing-v1-part2.feature

Loading…
Cancel
Save