Merge pull request #21067 from owncloud/fix_20296

don't allow to create a federated share if source and target are the same

apps/files_sharing/ajax/external.php

@@ -40,6 +40,7 @@ if (OCA\Files_Sharing\Helper::isIncomingServer2serverShareEnabled() === false) {
 $token = $_POST['token'];
 $remote = $_POST['remote'];
 $owner = $_POST['owner'];
+$ownerDisplayName = $_POST['ownerDisplayName'];
 $name = $_POST['name'];
 $password = $_POST['password'];
 
@@ -49,6 +50,14 @@ if(!\OCP\Util::isValidFileName($name)) {
 	exit();
 }
 
+$currentUser = \OC::$server->getUserSession()->getUser()->getUID();
+$currentServer = \OC::$server->getURLGenerator()->getAbsoluteURL('/');
+if (\OC\Share\Helper::isSameUserOnSameServer($owner, $remote, $currentUser, $currentServer )) {
+	\OCP\JSON::error(array('data' => array('message' => $l->t('Not allowed to create a federated share with the same user server'))));
+	exit();
+}
+
+
 $externalManager = new \OCA\Files_Sharing\External\Manager(
 		\OC::$server->getDatabaseConnection(),
 		\OC\Files\Filesystem::getMountManager(),
@@ -68,7 +77,7 @@ if (substr($remote, 0, 5) === 'https') {
 	}
 }
 
-$mount = $externalManager->addShare($remote, $token, $password, $name, $owner, true);
+$mount = $externalManager->addShare($remote, $token, $password, $name, $ownerDisplayName, true);
 
 /**
  * @var \OCA\Files_Sharing\External\Storage $storage

apps/files_sharing/js/external.js

@@ -19,7 +19,7 @@
 	 */
 	OCA.Sharing.showAddExternalDialog = function (share, passwordProtected, callback) {
 		var remote = share.remote;
-		var owner = share.owner;
+		var owner = share.ownerDisplayName || share.owner;
 		var name = share.name;
 		var remoteClean = (remote.substr(0, 8) === 'https://') ? remote.substr(8) : remote.substr(7);
 
@@ -92,6 +92,7 @@
 							remote: share.remote,
 							token: share.token,
 							owner: share.owner,
+							ownerDisplayName: share.ownerDisplayName || share.owner,
 							name: share.name,
 							password: password}, function(result) {
 							if (result.status === 'error') {

apps/files_sharing/js/public.js

@@ -242,9 +242,10 @@ OCA.Sharing.PublicApp = {
 			var remote = $(this).find('input[type="text"]').val();
 			var token = $('#sharingToken').val();
 			var owner = $('#save').data('owner');
+			var ownerDisplayName = $('#save').data('owner-display-name');
 			var name = $('#save').data('name');
 			var isProtected = $('#save').data('protected') ? 1 : 0;
-			OCA.Sharing.PublicApp._saveToOwnCloud(remote, token, owner, name, isProtected);
+			OCA.Sharing.PublicApp._saveToOwnCloud(remote, token, owner, ownerDisplayName, name, isProtected);
 		});
 
 		$('#remote_address').on("keyup paste", function() {
@@ -291,7 +292,7 @@ OCA.Sharing.PublicApp = {
 		this.fileList.changeDirectory(params.path || params.dir, false, true);
 	},
 
-	_saveToOwnCloud: function (remote, token, owner, name, isProtected) {
+	_saveToOwnCloud: function (remote, token, owner, ownerDisplayName, name, isProtected) {
 		var location = window.location.protocol + '//' + window.location.host + OC.webroot;
 		
 		if(remote.substr(-1) !== '/') {
@@ -299,7 +300,7 @@ OCA.Sharing.PublicApp = {
 		};
 
 		var url = remote + 'index.php/apps/files#' + 'remote=' + encodeURIComponent(location) // our location is the remote for the other server
-			+ "&token=" + encodeURIComponent(token) + "&owner=" + encodeURIComponent(owner) + "&name=" + encodeURIComponent(name) + "&protected=" + isProtected;
+			+ "&token=" + encodeURIComponent(token) + "&owner=" + encodeURIComponent(owner) +"&ownerDisplayName=" + encodeURIComponent(ownerDisplayName) + "&name=" + encodeURIComponent(name) + "&protected=" + isProtected;
 
 
 		if (remote.indexOf('://') > 0) {

apps/files_sharing/lib/controllers/sharecontroller.php

@@ -181,6 +181,7 @@ class ShareController extends Controller {
 
 		$shareTmpl = [];
 		$shareTmpl['displayName'] = User::getDisplayName($shareOwner);
+		$shareTmpl['owner'] = $shareOwner;
 		$shareTmpl['filename'] = $file;
 		$shareTmpl['directory_path'] = $linkItem['file_target'];
 		$shareTmpl['mimetype'] = Filesystem::getMimeType($originalSharePath);

apps/files_sharing/templates/public.php

@@ -72,7 +72,7 @@ $thumbSize = 1024;
 				if ($_['server2serversharing']) {
 					?>
 					<span id="save" data-protected="<?php p($_['protected']) ?>"
-						  data-owner="<?php p($_['displayName']) ?>" data-name="<?php p($_['filename']) ?>">
+						  data-owner-display-name="<?php p($_['displayName']) ?>" data-owner="<?php p($_['owner']) ?>" data-name="<?php p($_['filename']) ?>">
 					<button id="save-button"><?php p($l->t('Add to your ownCloud')) ?></button>
 					<form class="save-form hidden" action="#">
 						<input type="text" id="remote_address" placeholder="example.com/owncloud"/>

apps/files_sharing/tests/controller/sharecontroller.php

@@ -168,6 +168,7 @@ class ShareControllerTest extends \Test\TestCase {
 		$response = $this->shareController->showShare($this->token);
 		$sharedTmplParams = array(
 			'displayName' => $this->user,
+			'owner' => $this->user,
 			'filename' => 'file1.txt',
 			'directory_path' => '/file1.txt',
 			'mimetype' => 'text/plain',

apps/files_sharing/tests/js/externalSpec.js

@@ -67,6 +67,7 @@ describe('OCA.Sharing external tests', function() {
 				remote: 'http://example.com/owncloud',
 				token: 'abcdefg',
 				owner: 'theowner',
+				ownerDisplayName: 'The Generous Owner',
 				name: 'the share name'
 			};
 		});
@@ -88,6 +89,7 @@ describe('OCA.Sharing external tests', function() {
 				remote: 'http://example.com/owncloud',
 				token: 'abcdefg',
 				owner: 'theowner',
+				ownerDisplayName: 'The Generous Owner',
 				name: 'the share name',
 				password: ''
 			});
@@ -104,6 +106,7 @@ describe('OCA.Sharing external tests', function() {
 				remote: 'http://example.com/owncloud',
 				token: 'abcdefg',
 				owner: 'theowner',
+				ownerDisplayName: 'The Generous Owner',
 				name: 'the share name',
 				password: 'thepassword'
 			});
@@ -148,6 +151,7 @@ describe('OCA.Sharing external tests', function() {
 				remote: 'http://example.com/owncloud',
 				token: 'abcdefg',
 				owner: 'theowner',
+				ownerDisplayName: 'The Generous Owner',
 				name: 'the share name'
 			};
 		});

lib/private/share/helper.php

@@ -289,4 +289,38 @@ class Helper extends \OC\Share\Constants {
 		$hint = $l->t('Invalid Federated Cloud ID');
 		throw new HintException('Invalid Fededrated Cloud ID', $hint);
 	}
+
+	/**
+	 * check if two federated cloud IDs refer to the same user
+	 *
+	 * @param string $user1
+	 * @param string $server1
+	 * @param string $user2
+	 * @param string $server2
+	 * @return bool true if both users and servers are the same
+	 */
+	public static function isSameUserOnSameServer($user1, $server1, $user2, $server2) {
+		$normalizedServer1 = strtolower(\OC\Share\Share::removeProtocolFromUrl($server1));
+		$normalizedServer2 = strtolower(\OC\Share\Share::removeProtocolFromUrl($server2));
+
+		if (rtrim($normalizedServer1, '/') === rtrim($normalizedServer2, '/')) {
+			// FIXME this should be a method in the user management instead
+			\OCP\Util::emitHook(
+					'\OCA\Files_Sharing\API\Server2Server',
+					'preLoginNameUsedAsUserName',
+					array('uid' => &$user1)
+			);
+			\OCP\Util::emitHook(
+					'\OCA\Files_Sharing\API\Server2Server',
+					'preLoginNameUsedAsUserName',
+					array('uid' => &$user2)
+			);
+
+			if ($user1 === $user2) {
+				return true;
+			}
+		}
+
+		return false;
+	}
 }

lib/private/share/share.php

@@ -849,11 +849,20 @@ class Share extends Constants {
 					throw new \Exception($message_t);
 			}
 
+			// don't allow federated shares if source and target server are the same
+			list($user, $remote) = Helper::splitUserRemote($shareWith);
+			$currentServer = self::removeProtocolFromUrl(\OC::$server->getURLGenerator()->getAbsoluteURL('/'));
+			$currentUser = \OC::$server->getUserSession()->getUser()->getUID();
+			if (Helper::isSameUserOnSameServer($user, $remote, $currentUser, $currentServer)) {
+				$message = 'Not allowed to create a federated share with the same user.';
+				$message_t = $l->t('Not allowed to create a federated share with the same user');
+				\OCP\Util::writeLog('OCP\Share', $message, \OCP\Util::DEBUG);
+				throw new \Exception($message_t);
+			}
 
 			$token = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(self::TOKEN_LENGTH, \OCP\Security\ISecureRandom::CHAR_LOWER . \OCP\Security\ISecureRandom::CHAR_UPPER .
 				\OCP\Security\ISecureRandom::CHAR_DIGITS);
 
-			list($user, $remote) = Helper::splitUserRemote($shareWith);
 			$shareWith = $user . '@' . $remote;
 			$shareId = self::put($itemType, $itemSource, $shareType, $shareWith, $uidOwner, $permissions, null, $token, $itemSourceName);
 
@@ -2510,7 +2519,7 @@ class Share extends Constants {
 	 * @param string $url
 	 * @return string
 	 */
-	private static function removeProtocolFromUrl($url) {
+	public static function removeProtocolFromUrl($url) {
 		if (strpos($url, 'https://') === 0) {
 			return substr($url, strlen('https://'));
 		} else if (strpos($url, 'http://') === 0) {

tests/lib/share/helper.php

@@ -19,6 +19,10 @@
 * License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 */
 
+/**
+ * @group DB
+ * Class Test_Share_Helper
+ */
 class Test_Share_Helper extends \Test\TestCase {
 
 	public function expireDateProvider() {
@@ -121,4 +125,37 @@ class Test_Share_Helper extends \Test\TestCase {
 	public function testSplitUserRemoteError($id) {
 		\OC\Share\Helper::splitUserRemote($id);
 	}
+
+	/**
+	 * @dataProvider dataTestCompareServerAddresses
+	 *
+	 * @param string $server1
+	 * @param string $server2
+	 * @param bool $expected
+	 */
+	public function testIsSameUserOnSameServer($user1, $server1, $user2, $server2, $expected) {
+		$this->assertSame($expected,
+			\OC\Share\Helper::isSameUserOnSameServer($user1, $server1, $user2, $server2)
+		);
+	}
+
+	public function dataTestCompareServerAddresses() {
+		return [
+			['user1', 'http://server1', 'user1', 'http://server1', true],
+			['user1', 'https://server1', 'user1', 'http://server1', true],
+			['user1', 'http://serVer1', 'user1', 'http://server1', true],
+			['user1', 'http://server1/',  'user1', 'http://server1', true],
+			['user1', 'server1', 'user1', 'http://server1', true],
+			['user1', 'http://server1', 'user1', 'http://server2', false],
+			['user1', 'https://server1', 'user1', 'http://server2', false],
+			['user1', 'http://serVer1', 'user1', 'http://serer2', false],
+			['user1', 'http://server1/', 'user1', 'http://server2', false],
+			['user1', 'server1', 'user1', 'http://server2', false],
+			['user1', 'http://server1', 'user2', 'http://server1', false],
+			['user1', 'https://server1', 'user2', 'http://server1', false],
+			['user1', 'http://serVer1', 'user2', 'http://server1', false],
+			['user1', 'http://server1/',  'user2', 'http://server1', false],
+			['user1', 'server1', 'user2', 'http://server1', false],
+		];
+	}
 }