import: Replace the `allowAnyoneToImport` check with `userCanModify`

This reduces the number of hoops a user or tool must jump through to
import.

settings.json.template

@@ -497,19 +497,6 @@
    */
   "importMaxFileSize": 52428800, // 50 * 1024 * 1024
 
-
-  /*
-   * From Etherpad 1.8.3 onwards import was restricted to authors who had
-   * content within the pad.
-   *
-   * This setting will override that restriction and allow any user to import
-   * without the requirement to add content to a pad.
-   *
-   * This setting is useful for when you use a plugin for authentication so you
-   * can already trust each user.
-   */
-  "allowAnyoneToImport": false, 
-
   /*
    * From Etherpad 1.9.0 onwards, when Etherpad is in production mode commits from individual users are rate limited
    *

src/locales/en.json

@@ -188,6 +188,5 @@
   "pad.impexp.importfailed": "Import failed",
   "pad.impexp.copypaste": "Please copy paste",
   "pad.impexp.exportdisabled": "Exporting as {{type}} format is disabled. Please contact your system administrator for details.",
-  "pad.impexp.maxFileSize": "File too big. Contact your site administrator to increase the allowed file size for import",
-  "pad.impexp.permission": "Import is disabled because you never contributed to this pad. Please contribute at least once before importing"
+  "pad.impexp.maxFileSize": "File too big. Contact your site administrator to increase the allowed file size for import"
 }

src/node/handler/PadMessageHandler.js

@@ -942,16 +942,6 @@ async function handleClientReady(client, message, authorID)
     });
   }));
 
-  let thisUserHasEditedThisPad = false;
-  if (historicalAuthorData[authorID]) {
-    /*
-     * This flag is set to true when a user contributes to a specific pad for
-     * the first time. It is used for deciding if importing to that pad is
-     * allowed or not.
-     */
-    thisUserHasEditedThisPad = true;
-  }
-
   // glue the clientVars together, send them and tell the other clients that a new one is there
 
   // Check that the client is still here. It might have disconnected between callbacks.
@@ -1135,8 +1125,6 @@ async function handleClientReady(client, message, authorID)
         "percentageToScrollWhenUserPressesArrowUp": settings.scrollWhenFocusLineIsOutOfViewport.percentageToScrollWhenUserPressesArrowUp,
       },
       "initialChangesets": [], // FIXME: REMOVE THIS SHIT
-      "thisUserHasEditedThisPad": thisUserHasEditedThisPad,
-      "allowAnyoneToImport": settings.allowAnyoneToImport
     }
 
     // Add a username to the clientVars if one avaiable

src/node/hooks/express/importexport.js

@@ -8,6 +8,7 @@ var readOnlyManager = require("../../db/ReadOnlyManager");
 var authorManager = require("../../db/AuthorManager");
 const rateLimit = require("express-rate-limit");
 const securityManager = require("../../db/SecurityManager");
+const webaccess = require("./webaccess");
 
 settings.importExportRateLimiting.onLimitReached = function(req, res, options) {
   // when the rate limiter triggers, write a warning in the logs
@@ -63,36 +64,11 @@ exports.expressCreateServer = function (hook_name, args, cb) {
   args.app.use('/p/:pad/import', limiter);
   args.app.post('/p/:pad/import', async function(req, res, next) {
     const {session: {user} = {}} = req;
-    const {accessStatus, authorID} = await securityManager.checkAccess(
+    const {accessStatus} = await securityManager.checkAccess(
         req.params.pad, req.cookies.sessionID, req.cookies.token, req.cookies.password, user);
-    if (accessStatus !== 'grant') return res.status(403).send('Forbidden');
-    assert(authorID);
-
-    /*
-     * Starting from Etherpad 1.8.3 onwards, importing into a pad is allowed
-     * only if a user has his browser opened and connected to the pad (i.e. a
-     * Socket.IO session is estabilished for him) and he has already
-     * contributed to that specific pad.
-     *
-     * Note that this does not have anything to do with the "session", used
-     * for logging into "group pads". That kind of session is not needed here.
-     *
-     * This behaviour does not apply to API requests, only to /p/$PAD$/import
-     *
-     * See: https://github.com/ether/etherpad-lite/pull/3833#discussion_r407490205
-     */
-    if (!settings.allowAnyoneToImport) {
-      const authorsPads = await authorManager.listPadsOfAuthor(authorID);
-      if (!authorsPads) {
-        console.warn(`Unable to import file into "${req.params.pad}". Author "${authorID}" exists but he never contributed to any pad`);
-        return next();
-      }
-      if (authorsPads.padIDs.indexOf(req.params.pad) === -1) {
-        console.warn(`Unable to import file into "${req.params.pad}". Author "${authorID}" exists but he never contributed to this pad`);
-        return next();
-      }
+    if (accessStatus !== 'grant' || !webaccess.userCanModify(req.params.pad, req)) {
+      return res.status(403).send('Forbidden');
     }
-
-    importHandler.doImport(req, res, req.params.pad);
+    await importHandler.doImport(req, res, req.params.pad);
   });
 }

src/node/utils/Settings.js

@@ -385,20 +385,6 @@ exports.commitRateLimiting = {
  */
 exports.importMaxFileSize = 50 * 1024 * 1024;
 
-
-/*
- * From Etherpad 1.8.3 onwards import was restricted to authors who had
- * content within the pad.
- *
- * This setting will override that restriction and allow any user to import
- * without the requirement to add content to a pad.
- *
- * This setting is useful for when you use a plugin for authentication so you
- * can already trust each user.
- */
-exports.allowAnyoneToImport = false,
-
-
 // checks if abiword is avaiable
 exports.abiwordAvailable = function()
 {

src/static/css/pad.css

@@ -71,7 +71,3 @@ input {
 @media (max-width: 800px) {
   .hide-for-mobile { display: none; }
 }
-
-#importmessagepermission {
-  display: none;
-}

src/static/js/collab_client.js

@@ -312,16 +312,6 @@ function getCollabClient(ace2editor, serverVars, initialUserInfo, options, _pad)
     }
     else if (msg.type == "ACCEPT_COMMIT")
     {
-      /*
-       * this is the first time this user contributed to this pad. Let's record
-       * it, because it will be used for allowing import.
-       *
-       * TODO: here, we are changing this variable on the client side only. The
-       *       server has all the informations to make the same deduction, and
-       *       broadcast to the client.
-       */
-      clientVars.thisUserHasEditedThisPad = true;
-
       var newRev = msg.newRev;
       if (msgQueue.length > 0)
       {

src/static/js/pad_editbar.js

@@ -415,17 +415,6 @@ var padeditbar = (function()
 
     toolbar.registerCommand("import_export", function () {
       toolbar.toggleDropDown("import_export", function(){
-
-        if (clientVars.thisUserHasEditedThisPad || clientVars.allowAnyoneToImport) {
-          // the user has edited this pad historically or in this session
-          $('#importform').show();
-          $('#importmessagepermission').hide();
-        } else {
-          // this is the first time this user visits this pad
-          $('#importform').hide();
-          $('#importmessagepermission').show();
-        }
-
         // If Import file input exists then focus on it..
         if($('#importfileinput').length !== 0){
           setTimeout(function(){

src/templates/pad.html

@@ -195,7 +195,6 @@
                       </span>
                   </div>
               </form>
-              <div id="importmessagepermission" data-l10n-id="pad.impexp.permission"></div>
               <% e.end_block(); %>
           </div>
           <div id="exportColumn">

tests/backend/specs/api/importexportGetPost.js

@@ -7,7 +7,10 @@ const common = require('../../common');
 const superagent = require(__dirname+'/../../../../src/node_modules/superagent');
 const fs = require('fs');
 const settings = require(__dirname+'/../../../../src/node/utils/Settings');
+const padManager = require(__dirname+'/../../../../src/node/db/PadManager');
 const path = require('path');
+const plugins = require(__dirname+'/../../../../src/static/js/pluginfw/plugin_defs');
+
 const padText = fs.readFileSync("../tests/backend/specs/api/test.txt");
 const etherpadDoc = fs.readFileSync("../tests/backend/specs/api/test.etherpad");
 const wordDoc = fs.readFileSync("../tests/backend/specs/api/test.doc");
@@ -20,7 +23,8 @@ let agent;
 var apiKey = fs.readFileSync(filePath,  {encoding: 'utf-8'});
 apiKey = apiKey.replace(/\n$/, "");
 var apiVersion = 1;
-var testPadId = makeid();
+const testPadId = makeid();
+const testPadIdEnc = encodeURIComponent(testPadId);
 
 before(async function() { agent = await common.init(); });
 
@@ -67,14 +71,6 @@ Example Curl command for testing import URI:
 */
 
 describe('Imports and Exports', function(){
-  before(function() {
-    if (!settings.allowAnyoneToImport) {
-      console.warn('not anyone can import so not testing -- ' +
-                   'to include this test set allowAnyoneToImport to true in settings.json');
-      this.skip();
-    }
-  });
-
   const backups = {};
 
   beforeEach(async function() {
@@ -219,6 +215,137 @@ describe('Imports and Exports', function(){
         .expect((res) => assert.doesNotMatch(res.text, /FrameCall\('undefined', 'ok'\);/));
   });
 
+  describe('Import authorization checks', function() {
+    let authorize;
+
+    const deleteTestPad = async () => {
+      if (await padManager.doesPadExist(testPadId)) {
+        const pad = await padManager.getPad(testPadId);
+        await pad.remove();
+      }
+    };
+
+    const createTestPad = async (text) => {
+      const pad = await padManager.getPad(testPadId);
+      if (text) await pad.setText(text);
+      return pad;
+    };
+
+    beforeEach(async function() {
+      await deleteTestPad();
+      settings.requireAuthentication = false;
+      settings.requireAuthorization = true;
+      settings.users = {user: {password: 'user-password'}};
+      authorize = () => true;
+      backups.hooks = {};
+      backups.hooks.authorize = plugins.hooks.authorize || [];
+      plugins.hooks.authorize = [{hook_fn: (hookName, {req}, cb) => cb([authorize(req)])}];
+    });
+
+    afterEach(async function() {
+      await deleteTestPad();
+      Object.assign(plugins.hooks, backups.hooks);
+    });
+
+    it('!authn !exist -> create', async function() {
+      await agent.post(`/p/${testPadIdEnc}/import`)
+        .attach('file', padText, {filename: '/test.txt', contentType: 'text/plain'})
+        .expect(200);
+      assert(await padManager.doesPadExist(testPadId));
+      const pad = await padManager.getPad(testPadId);
+      assert.equal(pad.text(), padText.toString());
+    });
+
+    it('!authn exist -> replace', async function() {
+      const pad = await createTestPad('before import');
+      await agent.post(`/p/${testPadIdEnc}/import`)
+        .attach('file', padText, {filename: '/test.txt', contentType: 'text/plain'})
+        .expect(200);
+      assert(await padManager.doesPadExist(testPadId));
+      assert.equal(pad.text(), padText.toString());
+    });
+
+    it('authn anonymous !exist -> fail', async function() {
+      settings.requireAuthentication = true;
+      await agent.post(`/p/${testPadIdEnc}/import`)
+        .attach('file', padText, {filename: '/test.txt', contentType: 'text/plain'})
+        .expect(401);
+      assert(!(await padManager.doesPadExist(testPadId)));
+    });
+
+    it('authn anonymous exist -> fail', async function() {
+      settings.requireAuthentication = true;
+      const pad = await createTestPad('before import\n');
+      await agent.post(`/p/${testPadIdEnc}/import`)
+        .attach('file', padText, {filename: '/test.txt', contentType: 'text/plain'})
+        .expect(401);
+      assert.equal(pad.text(), 'before import\n');
+    });
+
+    it('authn user create !exist -> create', async function() {
+      settings.requireAuthentication = true;
+      await agent.post(`/p/${testPadIdEnc}/import`)
+        .auth('user', 'user-password')
+        .attach('file', padText, {filename: '/test.txt', contentType: 'text/plain'})
+        .expect(200);
+      assert(await padManager.doesPadExist(testPadId));
+      const pad = await padManager.getPad(testPadId);
+      assert.equal(pad.text(), padText.toString());
+    });
+
+    it('authn user modify !exist -> fail', async function() {
+      settings.requireAuthentication = true;
+      authorize = () => 'modify';
+      await agent.post(`/p/${testPadIdEnc}/import`)
+        .auth('user', 'user-password')
+        .attach('file', padText, {filename: '/test.txt', contentType: 'text/plain'})
+        .expect(403);
+      assert(!(await padManager.doesPadExist(testPadId)));
+    });
+
+    it('authn user readonly !exist -> fail', async function() {
+      settings.requireAuthentication = true;
+      authorize = () => 'readOnly';
+      await agent.post(`/p/${testPadIdEnc}/import`)
+        .auth('user', 'user-password')
+        .attach('file', padText, {filename: '/test.txt', contentType: 'text/plain'})
+        .expect(403);
+      assert(!(await padManager.doesPadExist(testPadId)));
+    });
+
+    it('authn user create exist -> replace', async function() {
+      settings.requireAuthentication = true;
+      const pad = await createTestPad('before import\n');
+      await agent.post(`/p/${testPadIdEnc}/import`)
+        .auth('user', 'user-password')
+        .attach('file', padText, {filename: '/test.txt', contentType: 'text/plain'})
+        .expect(200);
+      assert.equal(pad.text(), padText.toString());
+    });
+
+    it('authn user modify exist -> replace', async function() {
+      settings.requireAuthentication = true;
+      authorize = () => 'modify';
+      const pad = await createTestPad('before import\n');
+      await agent.post(`/p/${testPadIdEnc}/import`)
+        .auth('user', 'user-password')
+        .attach('file', padText, {filename: '/test.txt', contentType: 'text/plain'})
+        .expect(200);
+      assert.equal(pad.text(), padText.toString());
+    });
+
+    it('authn user readonly exist -> fail', async function() {
+      const pad = await createTestPad('before import\n');
+      settings.requireAuthentication = true;
+      authorize = () => 'readOnly';
+      await agent.post(`/p/${testPadIdEnc}/import`)
+        .auth('user', 'user-password')
+        .attach('file', padText, {filename: '/test.txt', contentType: 'text/plain'})
+        .expect(403);
+      assert.equal(pad.text(), 'before import\n');
+    });
+  });
+
 }); // End of tests.
 
 

tests/frontend/travis/runnerBackend.sh

@@ -12,11 +12,8 @@ cd "${MY_DIR}/../../../"
 # Set soffice to /usr/bin/soffice
 sed 's#\"soffice\": null,#\"soffice\":\"/usr/bin/soffice\",#g' settings.json.template > settings.json.soffice
 
-# Set allowAnyoneToImport to true
-sed 's/\"allowAnyoneToImport\": false,/\"allowAnyoneToImport\": true,/g' settings.json.soffice > settings.json.allowImport
-
 # Set "max": 10 to 100 to not agressively rate limit
-sed 's/\"max\": 10/\"max\": 100/g' settings.json.allowImport > settings.json.rateLimit
+sed 's/\"max\": 10/\"max\": 100/g' settings.json.soffice > settings.json.rateLimit
 
 # Set "points": 10 to 1000 to not agressively rate limit commits
 sed 's/\"points\": 10/\"points\": 1000/g' settings.json.rateLimit > settings.json