Merge pull request #22416 from owncloud/fix_22402

Return proper error string if sharing for this user is disabled

2 files changed, 15 insertions, 42 deletions

diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php
index 7a10d6cba55..4cff3dc818b 100644
--- a/lib/private/share20/manager.php
+++ b/lib/private/share20/manager.php
@@ -455,18 +455,16 @@ class Manager implements IManager {
 	 * Check if the user that is sharing can actually share
 	 *
 	 * @param \OCP\Share\IShare $share
-	 * @return bool
+	 * @throws \Exception
 	 */
 	protected function canShare(\OCP\Share\IShare $share) {
 		if (!$this->shareApiEnabled()) {
-			return false;
+			throw new \Exception('The share API is disabled');
 		}
 
 		if ($this->sharingDisabledForUser($share->getSharedBy())) {
-			return false;
+			throw new \Exception('You are not allowed to share');
 		}
-
-		return true;
 	}
 
 	/**
@@ -479,9 +477,7 @@ class Manager implements IManager {
 	 * TODO: handle link share permissions or check them
 	 */
 	public function createShare(\OCP\Share\IShare $share) {
-		if (!$this->canShare($share)) {
-			throw new \Exception('The Share API is disabled');
-		}
+		$this->canShare($share);
 
 		$this->generalCreateChecks($share);
 
@@ -592,9 +588,7 @@ class Manager implements IManager {
 	public function updateShare(\OCP\Share\IShare $share) {
 		$expirationDateUpdated = false;
 
-		if (!$this->canShare($share)) {
-			throw new \Exception('The Share API is disabled');
-		}
+		$this->canShare($share);
 
 		try {
 			$originalShare = $this->getShareById($share->getFullId());
diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php
index bb91ed0d51e..fe94b72c4e6 100644
--- a/tests/lib/share20/managertest.php
+++ b/tests/lib/share20/managertest.php
@@ -1404,28 +1404,21 @@ class ManagerTest extends \Test\TestCase {
 			->setMethods(['sharingDisabledForUser'])
 			->getMock();
 
-		$manager->method('sharingDisabledForUser')->willReturn($disabledForUser);
+		$manager->method('sharingDisabledForUser')
+			->with('user')
+			->willReturn($disabledForUser);
 
-		$user = $this->getMock('\OCP\IUser');
 		$share = $this->manager->newShare();
 		$share->setSharedBy('user');
 
-		$res = $this->invokePrivate($manager, 'canShare', [$share]);
-		$this->assertEquals($expected, $res);
-	}
-
-	/**
-	 * @expectedException Exception
-	 * @expectedExceptionMessage The Share API is disabled
-	 */
-	public function testCreateShareCantShare() {
-		$manager = $this->createManagerMock()
-			->setMethods(['canShare'])
-			->getMock();
+		$exception = false;
+		try {
+			$res = $this->invokePrivate($manager, 'canShare', [$share]);
+		} catch (\Exception $e) {
+			$exception = true;
+		}
 
-		$manager->expects($this->once())->method('canShare')->willReturn(false);
-		$share = $this->manager->newShare();
-		$manager->createShare($share);
+		$this->assertEquals($expected, !$exception);
 	}
 
 	public function testCreateShareUser() {
@@ -1945,20 +1938,6 @@ class ManagerTest extends \Test\TestCase {
 
 	/**
 	 * @expectedException Exception
-	 * @expectedExceptionMessage The Share API is disabled
-	 */
-	public function testUpdateShareCantShare() {
-		$manager = $this->createManagerMock()
-			->setMethods(['canShare'])
-			->getMock();
-
-		$manager->expects($this->once())->method('canShare')->willReturn(false);
-		$share = $this->manager->newShare();
-		$manager->updateShare($share);
-	}
-
-	/**
-	 * @expectedException Exception
 	 * @expectedExceptionMessage Can't change share type
 	 */
 	public function testUpdateShareCantChangeShareType() {