From 81e60581e4d658063af6bd5174a521819359fe2f Mon Sep 17 00:00:00 2001 From: Pranav Kant Date: Wed, 19 Oct 2016 16:37:28 +0530 Subject: [PATCH 1/8] Add a db index to access token --- appinfo/database.xml | 41 +++++++++++++++++++++++++---------------- appinfo/info.xml | 2 +- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/appinfo/database.xml b/appinfo/database.xml index 722fc1ee..c441ed37 100644 --- a/appinfo/database.xml +++ b/appinfo/database.xml @@ -7,7 +7,7 @@ *dbprefix*richdocuments_session - + es_id text @@ -41,7 +41,7 @@ 64 oC user who created the session - + richdocuments_session_ei_idx true @@ -51,14 +51,14 @@ ascending - +
- + *dbprefix*richdocuments_member - + member_id integer @@ -114,14 +114,14 @@ true 1 - +
- + *dbprefix*richdocuments_op - + seq integer @@ -160,7 +160,7 @@ false json-string - + richdocs_seq_pKey true @@ -181,13 +181,13 @@ ascending - +
*dbprefix*richdocuments_invite - + es_id text @@ -214,13 +214,13 @@ true 4 - +
*dbprefix*richdocuments_revisions - + es_id text @@ -257,7 +257,7 @@ true used to lookup revision in documents folder of member, eg hash.odt - + richdocuments_rev_eis_idx true @@ -270,10 +270,10 @@ ascending - +
- + *dbprefix*richdocuments_wopi @@ -335,6 +335,15 @@ 4 Expiration time of the token + + + richdocuments_wopi_token_idx + true + + token + ascending + +
diff --git a/appinfo/info.xml b/appinfo/info.xml index c79af661..18f56e7b 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -5,7 +5,7 @@ Collabora Online allows you to to work with all kinds of office documents directly in your browser. This application requires Collabora Cloudsuite to be installed on one of your servers, please read the documentation to learn more about that. Edit office documents directly in your browser. AGPL - 1.1.6 + 1.1.7 Collabora Productivity based on work of Frank Karlitschek, Victor Dubiniuk https://github.com/owncloud/richdocuments/issues https://github.com/owncloud/richdocuments.git From 20d2ce582a89e5a9ea50d7c6b02a0e712df93d22 Mon Sep 17 00:00:00 2001 From: Pranav Kant Date: Wed, 19 Oct 2016 00:56:01 +0530 Subject: [PATCH 2/8] Don't ignore non-updatable files Don't discriminate with these files; let them have the right to join the session or generate access token like other sessions --- controller/sessioncontroller.php | 13 +++---------- js/documents.js | 2 ++ lib/db/wopi.php | 2 +- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/controller/sessioncontroller.php b/controller/sessioncontroller.php index 7f63855d..ecdc7345 100644 --- a/controller/sessioncontroller.php +++ b/controller/sessioncontroller.php @@ -99,16 +99,9 @@ class SessionController extends Controller{ $view = \OC\Files\Filesystem::getView(); $path = $view->getPath($fileId); - if ($view->isUpdatable($path)) { - $file = new File($fileId); - $response = Db\Session::start($this->uid, $file); - } else { - $info = $view->getFileInfo($path); - $response = [ - 'permissions' => $info['permissions'], - 'id' => $fileId - ]; - } + $file = new File($fileId); + $response = Db\Session::start($this->uid, $file); + $response = array_merge( $response, [ 'status'=>'success' ] diff --git a/js/documents.js b/js/documents.js index f7495cbb..99c2dd3c 100644 --- a/js/documents.js +++ b/js/documents.js @@ -18,6 +18,7 @@ $.widget('oc.documentGrid', { .then(function(){ that._render(); + // TODO: Handle all of this logic by sending UserCanWrite: false to loolwsd if (!documentsMain.isGuest) { var editGroups = $('#edit_groups').val() .split('|') @@ -658,6 +659,7 @@ var documentsMain = { documentsMain.esId = response.es_id; documentsMain.memberId = response.member_id; + documentsMain.canEdit = response.permissions & OC.PERMISSION_UPDATE; documentsMain.loadDocument(); diff --git a/lib/db/wopi.php b/lib/db/wopi.php index fcff1c54..c826a1a5 100644 --- a/lib/db/wopi.php +++ b/lib/db/wopi.php @@ -49,7 +49,7 @@ class Wopi extends \OCA\Richdocuments\Db{ // Get the virtual path (if the file is shared). $path = $view->getPath($fileId); - if (!$view->is_file($path) || !$view->isUpdatable($path)) { + if (!$view->is_file($path)) { throw new \Exception('Invalid fileId.'); } From 74dd2a6d8a9f9b586c2a9782014277cfdb5d1e5b Mon Sep 17 00:00:00 2001 From: Pranav Kant Date: Wed, 19 Oct 2016 02:29:41 +0530 Subject: [PATCH 3/8] Login as editor, not owner This token belongs to editor, not owner, so its wrong to login as owner. --- controller/documentcontroller.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/controller/documentcontroller.php b/controller/documentcontroller.php index ec43bdc2..e9307dd2 100644 --- a/controller/documentcontroller.php +++ b/controller/documentcontroller.php @@ -476,9 +476,8 @@ class DocumentController extends Controller { } // Login the user to see his mount locations - $this->loginUser($res['owner']); - - $view = new \OC\Files\View('/' . $res['owner'] . '/files'); + $this->loginUser($res['editor']); + $view = new \OC\Files\View('/' . $res['editor'] . '/files'); $info = $view->getFileInfo($res['path']); // Close the session created for user login From 408fe08d5e2697cdd550bd2e6a66b7bdf0c83303 Mon Sep 17 00:00:00 2001 From: Pranav Kant Date: Wed, 19 Oct 2016 02:30:17 +0530 Subject: [PATCH 4/8] Set UserCanWrite WOPI property if file is updatable --- controller/documentcontroller.php | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/controller/documentcontroller.php b/controller/documentcontroller.php index e9307dd2..6d5dedab 100644 --- a/controller/documentcontroller.php +++ b/controller/documentcontroller.php @@ -479,23 +479,47 @@ class DocumentController extends Controller { $this->loginUser($res['editor']); $view = new \OC\Files\View('/' . $res['editor'] . '/files'); $info = $view->getFileInfo($res['path']); + $updatable = (bool)$view->isUpdatable($res['path']); + \OC::$server->getLogger()->debug('File with {fileid} has updatable set to {updatable}', [ 'app' => $this->appName, 'fileid' => $fileId, 'updatable' => $updatable ]); // Close the session created for user login \OC::$server->getSession()->close(); + // Check if the editor (user who is accessing) is in editable group + $editorUid = \OC::$server->getUserManager()->get($res['editor'])->getUID(); + $editGroups = array_filter(explode('|', $this->appConfig->getAppValue('edit_groups'))); + + // UserCanWrite only if + // 1. No edit groups are set or + // 2. if they are set, it is in one of the edit groups + if ($updatable && count($editGroups) > 0) { + $updatable = false; + foreach($editGroups as $editGroup) { + $editorGroup = \OC::$server->getGroupManager()->get($editGroup); + if (sizeof($editorGroup->searchUsers($editorUid)) > 0) { + \OC::$server->getLogger()->debug("Editor {editor} is in edit group {group}", [ + 'app' => $this->appName, + 'editor' => $editorUid, + 'group' => $editGroup + ]); + $updatable = true; + } + } + } + if (!$info) { http_response_code(404); return false; } $editorName = \OC::$server->getUserManager()->get($res['editor'])->getDisplayName(); - \OC::$server->getLogger()->debug('File info: {info}.', [ 'app' => $this->appName, 'info' => $info ]); return array( 'BaseFileName' => $info['name'], 'Size' => $info['size'], 'Version' => $version, 'UserId' => $res['editor'], - 'UserFriendlyName' => $editorName + 'UserFriendlyName' => $editorName, + 'UserCanWrite' => $updatable ); } From 93416e52a429e0fa2c4a283be818bb1bd51c4956 Mon Sep 17 00:00:00 2001 From: Pranav Kant Date: Wed, 19 Oct 2016 02:50:59 +0530 Subject: [PATCH 5/8] Kill edit for specific groups code --- controller/documentcontroller.php | 4 +--- js/documents.js | 24 ------------------------ templates/documents.php | 4 +--- 3 files changed, 2 insertions(+), 30 deletions(-) diff --git a/controller/documentcontroller.php b/controller/documentcontroller.php index 6d5dedab..e93b52df 100644 --- a/controller/documentcontroller.php +++ b/controller/documentcontroller.php @@ -305,9 +305,7 @@ class DocumentController extends Controller { 'uploadMaxHumanFilesize' => \OCP\Util::humanFileSize($maxUploadFilesize), 'allowShareWithLink' => $this->settings->getAppValue('core', 'shareapi_allow_links', 'yes'), 'wopi_url' => $webSocket, - 'edit_groups' => $this->appConfig->getAppValue('edit_groups'), - 'doc_format' => $this->appConfig->getAppValue('doc_format'), - 'usergroups' => $usergroups + 'doc_format' => $this->appConfig->getAppValue('doc_format') ]); $policy = new ContentSecurityPolicy(); diff --git a/js/documents.js b/js/documents.js index 99c2dd3c..9806d00e 100644 --- a/js/documents.js +++ b/js/documents.js @@ -17,30 +17,6 @@ $.widget('oc.documentGrid', { jQuery.when(this._load(fileId)) .then(function(){ that._render(); - - // TODO: Handle all of this logic by sending UserCanWrite: false to loolwsd - if (!documentsMain.isGuest) { - var editGroups = $('#edit_groups').val() - .split('|') - .filter(function(e) { - return e.length !== 0; - }); - var usergroups = $('#usergroups').val() - .split('|') - .filter(function(e) { - return e.length !== 0; - }); - documentsMain.canEdit = (editGroups.length === 0); - if (!documentsMain.canEdit && usergroups.length >= 1) { - for (var idx in usergroups) { - if (editGroups.indexOf(usergroups[idx]) !== -1) { - documentsMain.canEdit = true; - break; - } - } - } - } - documentsMain.renderComplete = true; }); }, diff --git a/templates/documents.php b/templates/documents.php index 3840eafd..1d3556c0 100644 --- a/templates/documents.php +++ b/templates/documents.php @@ -51,6 +51,4 @@ script('files', 'jquery.fileupload'); - - - + \ No newline at end of file From d0589c3e48d17da120dc65b5f632f71d1a0ccb0f Mon Sep 17 00:00:00 2001 From: Pranav Kant Date: Wed, 19 Oct 2016 18:28:38 +0530 Subject: [PATCH 6/8] Second param not needed here --- controller/documentcontroller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller/documentcontroller.php b/controller/documentcontroller.php index e93b52df..50f7af2c 100644 --- a/controller/documentcontroller.php +++ b/controller/documentcontroller.php @@ -624,7 +624,7 @@ class DocumentController extends Controller { // Setup the FS which is needed to emit hooks (versioning). \OC_Util::tearDownFS(); - \OC_Util::setupFS($userid, $root); + \OC_Util::setupFS($userid); $view->file_put_contents($res['path'], $content); From c1588590404054aba41f8dce3e346c79b00f63a9 Mon Sep 17 00:00:00 2001 From: Pranav Kant Date: Wed, 19 Oct 2016 18:40:16 +0530 Subject: [PATCH 7/8] Setup FS during artificial login; new internal method logoutUser --- controller/documentcontroller.php | 41 +++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/controller/documentcontroller.php b/controller/documentcontroller.php index 50f7af2c..155b1615 100644 --- a/controller/documentcontroller.php +++ b/controller/documentcontroller.php @@ -95,6 +95,8 @@ class DocumentController extends Controller { * @param string $userid */ private function loginUser($userid) { + \OC_Util::tearDownFS(); + $users = \OC::$server->getUserManager()->search($userid, 1, 0); if (count($users) > 0) { $user = array_shift($users); @@ -113,6 +115,18 @@ class DocumentController extends Controller { \OC::$server->getUserSession()->setUser($user); } } + + \OC_Util::setupFS(); + } + + /** + * Log out the current user + * This is helpful when we are artifically logged in as someone + */ + private function logoutUser() { + \OC_Util::tearDownFS(); + + \OC::$server->getSession()->close(); } private function responseError($message, $hint = ''){ @@ -475,13 +489,13 @@ class DocumentController extends Controller { // Login the user to see his mount locations $this->loginUser($res['editor']); - $view = new \OC\Files\View('/' . $res['editor'] . '/files'); + $view = \OC\Files\Filesystem::getView(); $info = $view->getFileInfo($res['path']); $updatable = (bool)$view->isUpdatable($res['path']); \OC::$server->getLogger()->debug('File with {fileid} has updatable set to {updatable}', [ 'app' => $this->appName, 'fileid' => $fileId, 'updatable' => $updatable ]); - // Close the session created for user login - \OC::$server->getSession()->close(); + + $this->logoutUser(); // Check if the editor (user who is accessing) is in editable group $editorUid = \OC::$server->getUserManager()->get($res['editor'])->getUID(); @@ -555,10 +569,6 @@ class DocumentController extends Controller { if ($version !== '0') { \OCP\JSON::checkAppEnabled('files_versions'); - // Setup the FS - \OC_Util::tearDownFS(); - \OC_Util::setupFS($ownerid, '/' . $ownerid . '/files'); - list($ownerid, $filename) = \OCA\Files_Versions\Storage::getUidAndFilename($res['path']); $filename = '/files_versions/' . $filename . '.v' . $version; @@ -567,8 +577,7 @@ class DocumentController extends Controller { $filename = '/files' . $res['path']; } - // Close the session created for user login - \OC::$server->getSession()->close(); + $this->logoutUser(); return new DownloadResponse($this->request, $ownerid, $filename); } @@ -612,6 +621,15 @@ class DocumentController extends Controller { // login. This is necessary to make activity app register the // change made to this file under this user's (editorid) name. $this->loginUser($editorid); + $view = \OC\Files\Filesystem::getView(); + if (!$view->isUpdatable($res['path'])) { + \OC::$server->getLogger()->debug('User {editor} has no permission to change the file {fileId}.', [ + 'app' => $this->appName, + 'fileId' => $fileId, + 'editor' => $editorid + ]); + return; + } // Set up the filesystem view for the owner (where the file actually is). $userid = $res['owner']; @@ -628,10 +646,7 @@ class DocumentController extends Controller { $view->file_put_contents($res['path'], $content); - \OC_Util::tearDownFS(); - - // clear any session created before - \OC::$server->getSession()->close(); + $this->logoutUser(); return array( 'status' => 'success' From 7e64777f4a36cd7feefd164be86ab1353334601f Mon Sep 17 00:00:00 2001 From: Pranav Kant Date: Wed, 19 Oct 2016 20:58:10 +0530 Subject: [PATCH 8/8] Store canwrite property in DB Move whether the file is editable or not logic when generating a token and store it in the DB with the access token instead of checking it dynamically in WOPI calls. --- appinfo/database.xml | 7 +++ appinfo/info.xml | 2 +- controller/documentcontroller.php | 85 +++++++++++++++---------------- lib/db/wopi.php | 14 +++-- 4 files changed, 59 insertions(+), 49 deletions(-) diff --git a/appinfo/database.xml b/appinfo/database.xml index c441ed37..fe6c0760 100644 --- a/appinfo/database.xml +++ b/appinfo/database.xml @@ -320,6 +320,13 @@ 512 Relative to storage e.g. /welcome.odt + + canwrite + boolean + false + true + Can make changes to this file + token text diff --git a/appinfo/info.xml b/appinfo/info.xml index 18f56e7b..eeb3a996 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -5,7 +5,7 @@ Collabora Online allows you to to work with all kinds of office documents directly in your browser. This application requires Collabora Cloudsuite to be installed on one of your servers, please read the documentation to learn more about that. Edit office documents directly in your browser. AGPL - 1.1.7 + 1.1.8 Collabora Productivity based on work of Frank Karlitschek, Victor Dubiniuk https://github.com/owncloud/richdocuments/issues https://github.com/owncloud/richdocuments.git diff --git a/controller/documentcontroller.php b/controller/documentcontroller.php index 155b1615..35c3aef1 100644 --- a/controller/documentcontroller.php +++ b/controller/documentcontroller.php @@ -450,8 +450,41 @@ class DocumentController extends Controller { \OC::$server->getLogger()->debug('Generating WOPI Token for file {fileId}, version {version}.', [ 'app' => $this->appName, 'fileId' => $fileId, 'version' => $version ]); + $view = \OC\Files\Filesystem::getView(); + $path = $view->getPath($fileId); + $updatable = (bool)$view->isUpdatable($path); + + // Check if the editor (user who is accessing) is in editable group + // UserCanWrite only if + // 1. No edit groups are set or + // 2. if they are set, it is in one of the edit groups + $editorUid = \OC::$server->getUserSession()->getUser()->getUID(); + $editGroups = array_filter(explode('|', $this->appConfig->getAppValue('edit_groups'))); + if ($updatable && count($editGroups) > 0) { + $updatable = false; + foreach($editGroups as $editGroup) { + $editorGroup = \OC::$server->getGroupManager()->get($editGroup); + if (sizeof($editorGroup->searchUsers($editorUid)) > 0) { + \OC::$server->getLogger()->debug("Editor {editor} is in edit group {group}", [ + 'app' => $this->appName, + 'editor' => $editorUid, + 'group' => $editGroup + ]); + $updatable = true; + } + } + } + + // If token is for some versioned file + if ($version !== '0') { + \OC::$server->getLogger()->debug('setting updatable to false'); + $updatable = false; + } + + \OC::$server->getLogger()->debug('File with {fileid} has updatable set to {updatable}', [ 'app' => $this->appName, 'fileid' => $fileId, 'updatable' => $updatable ]); + $row = new Db\Wopi(); - $token = $row->generateFileToken($fileId, $version); + $token = $row->generateFileToken($fileId, $version, $updatable); // Return the token. return array( @@ -491,34 +524,9 @@ class DocumentController extends Controller { $this->loginUser($res['editor']); $view = \OC\Files\Filesystem::getView(); $info = $view->getFileInfo($res['path']); - $updatable = (bool)$view->isUpdatable($res['path']); - - \OC::$server->getLogger()->debug('File with {fileid} has updatable set to {updatable}', [ 'app' => $this->appName, 'fileid' => $fileId, 'updatable' => $updatable ]); $this->logoutUser(); - // Check if the editor (user who is accessing) is in editable group - $editorUid = \OC::$server->getUserManager()->get($res['editor'])->getUID(); - $editGroups = array_filter(explode('|', $this->appConfig->getAppValue('edit_groups'))); - - // UserCanWrite only if - // 1. No edit groups are set or - // 2. if they are set, it is in one of the edit groups - if ($updatable && count($editGroups) > 0) { - $updatable = false; - foreach($editGroups as $editGroup) { - $editorGroup = \OC::$server->getGroupManager()->get($editGroup); - if (sizeof($editorGroup->searchUsers($editorUid)) > 0) { - \OC::$server->getLogger()->debug("Editor {editor} is in edit group {group}", [ - 'app' => $this->appName, - 'editor' => $editorUid, - 'group' => $editGroup - ]); - $updatable = true; - } - } - } - if (!$info) { http_response_code(404); return false; @@ -531,7 +539,7 @@ class DocumentController extends Controller { 'Version' => $version, 'UserId' => $res['editor'], 'UserFriendlyName' => $editorName, - 'UserCanWrite' => $updatable + 'UserCanWrite' => $res['canwrite'] ? 'true' : 'false' ); } @@ -599,20 +607,18 @@ class DocumentController extends Controller { $version = $arr[1]; } - // Changing a previous version of the file is not possible - // Ignore WOPI put if such a request is encountered - if ($version !== '0') { - return array( - 'status' => 'success' - ); - } - \OC::$server->getLogger()->debug('Putting contents of file {fileId}, version {version} by token {token}.', [ 'app' => $this->appName, 'fileId' => $fileId, 'version' => $version, 'token' => $token ]); $row = new Db\Wopi(); $row->loadBy('token', $token); $res = $row->getPathForToken($fileId, $version, $token); + if (!$res['canwrite']) { + return array( + 'status' => 'error', + 'message' => 'Permission denied' + ); + } // Log-in as the user to regiser the change under her name. $editorid = $res['editor']; @@ -621,15 +627,6 @@ class DocumentController extends Controller { // login. This is necessary to make activity app register the // change made to this file under this user's (editorid) name. $this->loginUser($editorid); - $view = \OC\Files\Filesystem::getView(); - if (!$view->isUpdatable($res['path'])) { - \OC::$server->getLogger()->debug('User {editor} has no permission to change the file {fileId}.', [ - 'app' => $this->appName, - 'fileId' => $fileId, - 'editor' => $editorid - ]); - return; - } // Set up the filesystem view for the owner (where the file actually is). $userid = $res['owner']; diff --git a/lib/db/wopi.php b/lib/db/wopi.php index c826a1a5..c4ea3830 100644 --- a/lib/db/wopi.php +++ b/lib/db/wopi.php @@ -29,8 +29,8 @@ class Wopi extends \OCA\Richdocuments\Db{ protected $tableName = '`*PREFIX*richdocuments_wopi`'; - protected $insertStatement = 'INSERT INTO `*PREFIX*richdocuments_wopi` (`owner_uid`, `editor_uid`, `fileid`, `version`, `path`, `token`, `expiry`) - VALUES (?, ?, ?, ?, ?, ?, ?)'; + protected $insertStatement = 'INSERT INTO `*PREFIX*richdocuments_wopi` (`owner_uid`, `editor_uid`, `fileid`, `version`, `path`, `canwrite`, `token`, `expiry`) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)'; protected $loadStatement = 'SELECT * FROM `*PREFIX*richdocuments_wopi` WHERE `token`= ?'; @@ -41,7 +41,7 @@ class Wopi extends \OCA\Richdocuments\Db{ * its the version number as stored by files_version app * Returns the token. */ - public function generateFileToken($fileId, $version){ + public function generateFileToken($fileId, $version, $updatable){ // Get the FS view of the current user. $view = \OC\Files\Filesystem::getView(); @@ -79,6 +79,7 @@ class Wopi extends \OCA\Richdocuments\Db{ $fileId, $version, $path, + $updatable, $token, time() + self::TOKEN_LIFETIME_SECONDS ]); @@ -120,6 +121,11 @@ class Wopi extends \OCA\Richdocuments\Db{ return false; } - return array('owner' => $row['owner_uid'], 'editor' => $row['editor_uid'], 'path' => $row['path']); + return array( + 'owner' => $row['owner_uid'], + 'editor' => $row['editor_uid'], + 'path' => $row['path'], + 'canwrite' => $row['canwrite'] + ); } }