From 53fd0b4f98bacbd075c4b8a1315d45dc3769c727 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Wed, 26 Aug 2020 22:08:07 -0400
Subject: [PATCH 01/12] webaccess: Return 401 for authn failure, 403 for authz
 failure

This makes it possible for reverse proxies to transform 403 errors
into something like "upgrade to a premium account to access this
pad".

Also add some webaccess tests.
---
 doc/api/hooks_server-side.md        |   3 +-
 src/node/hooks/express/webaccess.js |  34 +++---
 src/node/server.js                  |   2 +-
 tests/backend/specs/webaccess.js    | 166 ++++++++++++++++++++++++++++
 4 files changed, 188 insertions(+), 17 deletions(-)
 create mode 100644 tests/backend/specs/webaccess.js

diff --git a/doc/api/hooks_server-side.md b/doc/api/hooks_server-side.md
index dd0b8599e..370e782ed 100644
--- a/doc/api/hooks_server-side.md
+++ b/doc/api/hooks_server-side.md
@@ -370,7 +370,8 @@ A plugin's authFailure function is only called if all of the following are true:
 Calling the provided callback with `[true]` tells Etherpad that the failure was
 handled and no further error handling is required. Calling the callback with
 `[]` or `undefined` defers error handling to the next authFailure plugin (if
-any, otherwise fall back to HTTP basic authentication).
+any, otherwise fall back to HTTP basic authentication for an authentication
+failure or a generic 403 page for an authorization failure).
 
 Example:
 
diff --git a/src/node/hooks/express/webaccess.js b/src/node/hooks/express/webaccess.js
index b83fbbd00..fe5a40535 100644
--- a/src/node/hooks/express/webaccess.js
+++ b/src/node/hooks/express/webaccess.js
@@ -58,19 +58,6 @@ exports.checkAccess = (req, res, next) => {
     hooks.aCallFirst('authorize', {req, res, next, resource: req.path}, hookResultMangle(grant));
   };
 
-  /* Authentication OR authorization failed. */
-  const failure = () => {
-    return hooks.aCallFirst('authFailure', {req, res, next}, hookResultMangle((ok) => {
-      if (ok) return;
-      // No plugin handled the authn/authz failure. Fall back to basic authentication.
-      res.header('WWW-Authenticate', 'Basic realm="Protected Area"');
-      // Delay the error response for 1s to slow down brute force attacks.
-      setTimeout(() => {
-        res.status(401).send('Authentication Required');
-      }, 1000);
-    }));
-  };
-
   // Access checking is done in three steps:
   //
   // 1) Try to just access the thing. If access fails (perhaps authentication has not yet completed,
@@ -78,7 +65,7 @@ exports.checkAccess = (req, res, next) => {
   // 2) Try to authenticate. (Or, if already logged in, reauthenticate with different credentials if
   //    supported by the authn scheme.) If authentication fails, give the user a 401 error to
   //    request new credentials. Otherwise, go to the next step.
-  // 3) Try to access the thing again. If this fails, give the user a 401 error.
+  // 3) Try to access the thing again. If this fails, give the user a 403 error.
   //
   // Plugins can use the 'next' callback (from the hook's context) to break out at any point (e.g.,
   // to process an OAuth callback). Plugins can use the authFailure hook to override the default
@@ -103,6 +90,17 @@ exports.checkAccess = (req, res, next) => {
     }
     hooks.aCallFirst('authenticate', ctx, hookResultMangle((ok) => {
       if (!ok) {
+        const failure = () => {
+          return hooks.aCallFirst('authFailure', {req, res, next}, hookResultMangle((ok) => {
+            if (ok) return;
+            // No plugin handled the authentication failure. Fall back to basic authentication.
+            res.header('WWW-Authenticate', 'Basic realm="Protected Area"');
+            // Delay the error response for 1s to slow down brute force attacks.
+            setTimeout(() => {
+              res.status(401).send('Authentication Required');
+            }, 1000);
+          }));
+        };
         // Fall back to HTTP basic auth.
         if (!httpBasicAuth) return failure();
         if (!(ctx.username in settings.users)) {
@@ -126,7 +124,13 @@ exports.checkAccess = (req, res, next) => {
     }));
   };
 
-  step3Authorize = () => authorize(failure);
+  step3Authorize = () => authorize(() => {
+    return hooks.aCallFirst('authFailure', {req, res, next}, hookResultMangle((ok) => {
+      if (ok) return;
+      // No plugin handled the authorization failure.
+      res.status(403).send('Forbidden');
+    }));
+  });
 
   step1PreAuthenticate();
 };
diff --git a/src/node/server.js b/src/node/server.js
index c9ef33cc9..a8a567179 100755
--- a/src/node/server.js
+++ b/src/node/server.js
@@ -45,7 +45,7 @@ let started = false;
 let stopped = false;
 
 exports.start = async () => {
-  if (started) return;
+  if (started) return express.server;
   started = true;
   if (stopped) throw new Error('restart not supported');
 
diff --git a/tests/backend/specs/webaccess.js b/tests/backend/specs/webaccess.js
new file mode 100644
index 000000000..8367429f0
--- /dev/null
+++ b/tests/backend/specs/webaccess.js
@@ -0,0 +1,166 @@
+function m(mod) { return __dirname + '/../../../src/' + mod; }
+
+const assert = require('assert').strict;
+const log4js = require(m('node_modules/log4js'));
+const plugins = require(m('static/js/pluginfw/plugin_defs'));
+const server = require(m('node/server'));
+const settings = require(m('node/utils/Settings'));
+const supertest = require(m('node_modules/supertest'));
+
+let agent;
+const logger = log4js.getLogger('test');
+
+before(async function() {
+  settings.port = 0;
+  settings.ip = 'localhost';
+  const httpServer = await server.start();
+  const baseUrl = `http://localhost:${httpServer.address().port}`;
+  logger.debug(`HTTP server at ${baseUrl}`);
+  agent = supertest(baseUrl);
+});
+
+after(async function() {
+  await server.stop();
+});
+
+describe('webaccess without any plugins', function() {
+  const backup = {};
+
+  before(async function() {
+    Object.assign(backup, settings);
+    settings.users = {
+      admin: {password: 'admin-password', is_admin: true},
+      user: {password: 'user-password'},
+    };
+  });
+
+  after(async function() {
+    Object.assign(settings, backup);
+  });
+
+  it('!authn !authz anonymous / -> 200', async function() {
+    settings.requireAuthentication = false;
+    settings.requireAuthorization = false;
+    await agent.get('/').expect(200);
+  });
+  it('!authn !authz anonymous /admin/ -> 401', async function() {
+    settings.requireAuthentication = false;
+    settings.requireAuthorization = false;
+    await agent.get('/admin/').expect(401);
+  });
+  it('authn !authz anonymous / -> 401', async function() {
+    settings.requireAuthentication = true;
+    settings.requireAuthorization = false;
+    await agent.get('/').expect(401);
+  });
+  it('authn !authz user / -> 200', async function() {
+    settings.requireAuthentication = true;
+    settings.requireAuthorization = false;
+    await agent.get('/').auth('user', 'user-password').expect(200);
+  });
+  it('authn !authz user /admin/ -> 403', async function() {
+    settings.requireAuthentication = true;
+    settings.requireAuthorization = false;
+    await agent.get('/admin/').auth('user', 'user-password').expect(403);
+  });
+  it('authn !authz admin / -> 200', async function() {
+    settings.requireAuthentication = true;
+    settings.requireAuthorization = false;
+    await agent.get('/').auth('admin', 'admin-password').expect(200);
+  });
+  it('authn !authz admin /admin/ -> 200', async function() {
+    settings.requireAuthentication = true;
+    settings.requireAuthorization = false;
+    await agent.get('/admin/').auth('admin', 'admin-password').expect(200);
+  });
+  it('authn authz user / -> 403', async function() {
+    settings.requireAuthentication = true;
+    settings.requireAuthorization = true;
+    await agent.get('/').auth('user', 'user-password').expect(403);
+  });
+  it('authn authz user /admin/ -> 403', async function() {
+    settings.requireAuthentication = true;
+    settings.requireAuthorization = true;
+    await agent.get('/admin/').auth('user', 'user-password').expect(403);
+  });
+  it('authn authz admin / -> 200', async function() {
+    settings.requireAuthentication = true;
+    settings.requireAuthorization = true;
+    await agent.get('/').auth('admin', 'admin-password').expect(200);
+  });
+  it('authn authz admin /admin/ -> 200', async function() {
+    settings.requireAuthentication = true;
+    settings.requireAuthorization = true;
+    await agent.get('/admin/').auth('admin', 'admin-password').expect(200);
+  });
+});
+
+describe('webaccess with authFailure plugin', function() {
+  let handle, returnUndef, status, called;
+  const authFailure = (hookName, context, cb) => {
+    assert.equal(hookName, 'authFailure');
+    assert(context != null);
+    assert(context.req != null);
+    assert(context.res != null);
+    assert(context.next != null);
+    assert(!called);
+    called = true;
+    if (handle) {
+      context.res.status(status).send('injected content');
+      return cb([true]);
+    }
+    if (returnUndef) return cb();
+    return cb([]);
+  };
+
+  const settingsBackup = {};
+  let authFailureHooksBackup;
+  before(function() {
+    Object.assign(settingsBackup, settings);
+    authFailureHooksBackup = plugins.hooks.authFailure;
+    plugins.hooks.authFailure = [{hook_fn: authFailure}];
+    settings.requireAuthentication = true;
+    settings.requireAuthorization = true;
+    settings.users = {
+      admin: {password: 'admin-password', is_admin: true},
+      user: {password: 'user-password'},
+    };
+  });
+  after(function() {
+    Object.assign(settings, settingsBackup);
+    plugins.hooks.authFailure = authFailureHooksBackup;
+  });
+
+  beforeEach(function() {
+    handle = false;
+    returnUndef = false;
+    status = 200;
+    called = false;
+  });
+  afterEach(function() {
+    assert(called);
+  });
+
+  it('authn fail, hook handles -> 200', async function() {
+    handle = true;
+    await agent.get('/').expect(200, /injected content/);
+  });
+  it('authn fail, hook defers (undefined) -> 401', async function() {
+    returnUndef = true;
+    await agent.get('/').expect(401);
+  });
+  it('authn fail, hook defers (empty list) -> 401', async function() {
+    await agent.get('/').expect(401);
+  });
+  it('authz fail, hook handles -> 200', async function() {
+    handle = true;
+    await agent.get('/').auth('user', 'user-password').expect(200, /injected content/);
+  });
+  it('authz fail, hook defers (undefined) -> 403', async function() {
+    returnUndef = true;
+    await agent.get('/').auth('user', 'user-password').expect(403);
+  });
+  it('authz fail, hook defers (empty list) -> 403', async function() {
+    await agent.get('/').auth('user', 'user-password').expect(403);
+  });
+});

From 94f944160dcf46d161f44aee264656d77bfb169c Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Wed, 23 Sep 2020 17:13:07 -0400
Subject: [PATCH 02/12] security: Don't require express_sid if authn not
 required

This should make it possible to embed a pad in an iframe from another
site as long as `settings.requireAuthentication` is false.
---
 src/node/hooks/express/socketio.js | 43 ++++++++++++++++--------------
 tests/backend/specs/socketio.js    | 10 +++++--
 2 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/src/node/hooks/express/socketio.js b/src/node/hooks/express/socketio.js
index b1406afd2..ffc280b5c 100644
--- a/src/node/hooks/express/socketio.js
+++ b/src/node/hooks/express/socketio.js
@@ -8,6 +8,7 @@ var padMessageHandler = require("../../handler/PadMessageHandler");
 
 var cookieParser = require('cookie-parser');
 var sessionModule = require('express-session');
+const util = require('util');
 
 exports.expressCreateServer = function (hook_name, args, cb) {
   //init socket.io and redirect all requests to the MessageHandler
@@ -48,32 +49,34 @@ exports.expressCreateServer = function (hook_name, args, cb) {
   // check whether the user has authenticated, then any random person on the Internet can read,
   // modify, or create any pad (unless the pad is password protected or an HTTP API session is
   // required).
-  var cookieParserFn = cookieParser(webaccess.secret, {});
-  io.use((socket, next) => {
-    var data = socket.request;
-    if (!data.headers.cookie) {
+  const cookieParserFn = util.promisify(cookieParser(webaccess.secret, {}));
+  const getSession = util.promisify(args.app.sessionStore.get).bind(args.app.sessionStore);
+  io.use(async (socket, next) => {
+    const req = socket.request;
+    if (!req.headers.cookie) {
       // socketio.js-client on node.js doesn't support cookies (see https://git.io/JU8u9), so the
       // token and express_sid cookies have to be passed via a query parameter for unit tests.
-      data.headers.cookie = socket.handshake.query.cookie;
+      req.headers.cookie = socket.handshake.query.cookie;
     }
-    if (!data.headers.cookie && settings.loadTest) {
+    if (!req.headers.cookie && settings.loadTest) {
       console.warn('bypassing socket.io authentication check due to settings.loadTest');
       return next(null, true);
     }
-    const fail = (msg) => { return next(new Error(msg), false); };
-    cookieParserFn(data, {}, function(err) {
-      if (err) return fail('access denied: unable to parse express_sid cookie');
-      const expressSid = data.signedCookies.express_sid;
-      if (!expressSid) return fail ('access denied: signed express_sid cookie is required');
-      args.app.sessionStore.get(expressSid, (err, session) => {
-        if (err || !session) return fail('access denied: bad session or session has expired');
-        data.session = new sessionModule.Session(data, session);
-        if (settings.requireAuthentication && data.session.user == null) {
-          return fail('access denied: authentication required');
-        }
-        next(null, true);
-      });
-    });
+    try {
+      await cookieParserFn(req, {});
+      const expressSid = req.signedCookies.express_sid;
+      const needAuthn = settings.requireAuthentication;
+      if (needAuthn && !expressSid) throw new Error('signed express_sid cookie is required');
+      if (expressSid) {
+        const session = await getSession(expressSid);
+        if (!session) throw new Error('bad session or session has expired');
+        req.session = new sessionModule.Session(req, session);
+        if (needAuthn && req.session.user == null) throw new Error('authentication required');
+      }
+    } catch (err) {
+      return next(new Error(`access denied: ${err}`), false);
+    }
+    return next(null, true);
   });
 
   // var socketIOLogger = log4js.getLogger("socket.io");
diff --git a/tests/backend/specs/socketio.js b/tests/backend/specs/socketio.js
index ba639a79c..8dfd8aecc 100644
--- a/tests/backend/specs/socketio.js
+++ b/tests/backend/specs/socketio.js
@@ -130,13 +130,19 @@ describe('socket.io access checks', () => {
   });
 
   // Normal accesses.
-  it('!authn anonymous /p/pad -> 200, ok', async () => {
+  it('!authn anonymous cookie /p/pad -> 200, ok', async () => {
     const res = await client.get('/p/pad').expect(200);
     // Should not throw.
     socket = await connect(res);
     const clientVars = await handshake(socket, 'pad');
     assert.equal(clientVars.type, 'CLIENT_VARS');
   });
+  it('!authn !cookie -> ok', async () => {
+    // Should not throw.
+    socket = await connect(null);
+    const clientVars = await handshake(socket, 'pad');
+    assert.equal(clientVars.type, 'CLIENT_VARS');
+  });
   it('!authn user /p/pad -> 200, ok', async () => {
     const res = await client.get('/p/pad').auth('user', 'user-password').expect(200);
     // Should not throw.
@@ -160,7 +166,7 @@ describe('socket.io access checks', () => {
     // Despite the 401, try to create the pad via a socket.io connection anyway.
     await assert.rejects(connect(res), {message: /authentication required/i});
   });
-  it('socket.io connection without express-session cookie -> error', async () => {
+  it('authn !cookie -> error', async () => {
     settings.requireAuthentication = true;
     await assert.rejects(connect(null), {message: /signed express_sid cookie is required/i});
   });

From 6cde6f5a9897fb210e68b7d833154804abf51b17 Mon Sep 17 00:00:00 2001
From: "translatewiki.net" <l10n-bot@translatewiki.net>
Date: Thu, 24 Sep 2020 15:54:49 +0200
Subject: [PATCH 03/12] Localisation updates from https://translatewiki.net.

---
 src/locales/diq.json     | 2 ++
 src/locales/fr.json      | 2 ++
 src/locales/mk.json      | 2 ++
 src/locales/pms.json     | 2 ++
 src/locales/pt-br.json   | 2 ++
 src/locales/sv.json      | 2 ++
 src/locales/tr.json      | 3 +++
 src/locales/zh-hant.json | 2 ++
 8 files changed, 17 insertions(+)

diff --git a/src/locales/diq.json b/src/locales/diq.json
index 8ba96f358..c21ffb62a 100644
--- a/src/locales/diq.json
+++ b/src/locales/diq.json
@@ -85,6 +85,8 @@
 	"pad.modals.deleted.explanation": "Ena ped wedariye",
 	"pad.modals.rateLimited": "Nısbeto kemeyeyın",
 	"pad.modals.rateLimited.explanation": "Na pad re ßıma vêşi mesac rışto, coki ra irtibat bıriyayo.",
+	"pad.modals.rejected.explanation": "Server, terefê browseri ra rışiyaye yew mesac red kerdo.",
+	"pad.modals.rejected.cause": "Şıma wexto ke ped weyniyayış de server belka biyo rocane ya ziEtherpad de yew xeta bena. Pela reyna bar kerê.",
 	"pad.modals.disconnected": "İrtibata şıma reyê",
 	"pad.modals.disconnected.explanation": "Rovıteri ya irtibata şıma reyyê",
 	"pad.modals.disconnected.cause": "Qay rovıtero nêkarên o.  Ena xerpey deqam kena se idarekaranê sistemiya irtibat kewê",
diff --git a/src/locales/fr.json b/src/locales/fr.json
index 6b567dcbb..e8c9bd910 100644
--- a/src/locales/fr.json
+++ b/src/locales/fr.json
@@ -104,6 +104,8 @@
 	"pad.modals.deleted.explanation": "Ce bloc-notes a été supprimé.",
 	"pad.modals.rateLimited": "Taux limité.",
 	"pad.modals.rateLimited.explanation": "Vous avez envoyé trop de messages à ce bloc, il vous a donc déconnecté.",
+	"pad.modals.rejected.explanation": "Le serveur a rejeté un message qui a été envoyé par votre navigateur.",
+	"pad.modals.rejected.cause": "Le serveur peut avoir été mis à jour pendant que vous regardiez le bloc, ou il y a peut-être un bogue dans Etherpad. Essayez de recharger la page.",
 	"pad.modals.disconnected": "Vous avez été déconnecté.",
 	"pad.modals.disconnected.explanation": "La connexion au serveur a échoué.",
 	"pad.modals.disconnected.cause": "Il se peut que le serveur soit indisponible. Si le problème persiste, veuillez en informer l’administrateur du service.",
diff --git a/src/locales/mk.json b/src/locales/mk.json
index b5d79d1f8..3f492c7d0 100644
--- a/src/locales/mk.json
+++ b/src/locales/mk.json
@@ -81,6 +81,8 @@
 	"pad.modals.deleted.explanation": "Оваа тетратка е отстранета.",
 	"pad.modals.rateLimited": "Ограничено по стапка.",
 	"pad.modals.rateLimited.explanation": "Испративте премногу пораки на тетраткава, па затоа таа ве исклучи.",
+	"pad.modals.rejected.explanation": "Опслужувачот ја отфрли пораката што му беше испратена од вашиот прелистувач.",
+	"pad.modals.rejected.cause": "Опслужувачот може да бил подновен додека ја гледавте тетратката, или пак Etherpad има некоја грешка. Пробајте со превчитување на страницата.",
 	"pad.modals.disconnected": "Врската е прекината.",
 	"pad.modals.disconnected.explanation": "Врската со опслужувачот е прекината",
 	"pad.modals.disconnected.cause": "Опслужувачот може да е недостапен. Известете го администраторот ако ова продолжи да ви се случува.",
diff --git a/src/locales/pms.json b/src/locales/pms.json
index 05677597a..8ca0b3ff7 100644
--- a/src/locales/pms.json
+++ b/src/locales/pms.json
@@ -78,6 +78,8 @@
 	"pad.modals.deleted.explanation": "Ës feuj a l'é stàit eliminà.",
 	"pad.modals.rateLimited": "Tass limità.",
 	"pad.modals.rateLimited.explanation": "A l'ha mandà tròpi mëssagi a 's blòch-sì, antlora a l'ha dëscolegalo.",
+	"pad.modals.rejected.explanation": "Ël servent a l'ha arpossà un mëssagi mandà da sò navigador.",
+	"pad.modals.rejected.cause": "Ël servent a podrìa esse stàit agiornà antramentre che chiel a beicava ël blòch, o peul desse ch'a-i é un givo an Etherpad. Ch'a preuva a carié torna la pàgina.",
 	"pad.modals.disconnected": "A l'é stàit dëscolegà",
 	"pad.modals.disconnected.explanation": "La conession al servent a l'é perdusse",
 	"pad.modals.disconnected.cause": "Ël servent a podrìa esse indisponìbil. Për piasì, ch'a anforma l'aministrator dël servissi si ël problema a persist.",
diff --git a/src/locales/pt-br.json b/src/locales/pt-br.json
index ebcb3be44..a458de3d0 100644
--- a/src/locales/pt-br.json
+++ b/src/locales/pt-br.json
@@ -9,6 +9,7 @@
 			"Lpagliari",
 			"Luckas",
 			"Macofe",
+			"Nsharkey",
 			"Prilopes",
 			"Rafaelff",
 			"Rodrigo codignoli",
@@ -94,6 +95,7 @@
 	"pad.modals.deleted.explanation": "Esta nota foi removida.",
 	"pad.modals.rateLimited": "Limitado.",
 	"pad.modals.rateLimited.explanation": "Você enviou muitas mensagens para este pad por isso será desconectado.",
+	"pad.modals.rejected.explanation": "O servidor rejeitou uma mensagem que foi enviada pelo seu navegador.",
 	"pad.modals.disconnected": "Você foi desconectado.",
 	"pad.modals.disconnected.explanation": "A conexão com o servidor foi perdida",
 	"pad.modals.disconnected.cause": "O servidor pode estar indisponível. Por favor, notifique o administrador caso isso continue.",
diff --git a/src/locales/sv.json b/src/locales/sv.json
index fcef0a10c..3065ac079 100644
--- a/src/locales/sv.json
+++ b/src/locales/sv.json
@@ -83,6 +83,8 @@
 	"pad.modals.deleted.explanation": "Detta block har tagits bort.",
 	"pad.modals.rateLimited": "Begränsad frekvens.",
 	"pad.modals.rateLimited.explanation": "Du skickade för många meddelanden till detta block så du blev frånkopplad.",
+	"pad.modals.rejected.explanation": "Servern avvisade ett meddelande som skickades av din webbläsare.",
+	"pad.modals.rejected.cause": "Servern kan ha uppdaterats medan du visade blocket, eller så finns det kanske en bugg i Etherpad. Försök att ladda om sidan.",
 	"pad.modals.disconnected": "Du har blivit frånkopplad.",
 	"pad.modals.disconnected.explanation": "Anslutningen till servern avbröts",
 	"pad.modals.disconnected.cause": "Servern kanske är otillgänglig. Var god meddela tjänstadministratören om detta fortsätter att hända.",
diff --git a/src/locales/tr.json b/src/locales/tr.json
index 6031cfd2c..7b5dfac0a 100644
--- a/src/locales/tr.json
+++ b/src/locales/tr.json
@@ -9,6 +9,7 @@
 			"Joseph",
 			"McAang",
 			"Meelo",
+			"MuratTheTurkish",
 			"Trockya",
 			"Vito Genovese"
 		]
@@ -88,6 +89,8 @@
 	"pad.modals.deleted.explanation": "Bu bloknot kaldırılmış.",
 	"pad.modals.rateLimited": "Oran Sınırlı.",
 	"pad.modals.rateLimited.explanation": "Bu pad'e çok fazla mesaj gönderdiniz, böylece bağlantı kesildi.",
+	"pad.modals.rejected.explanation": "Sunucu, tarayıcınız tarafından gönderilen bir mesajı reddetti.",
+	"pad.modals.rejected.cause": "Siz pedi görüntülerken sunucu güncellenmiş olabilir veya Etherpad'de bir hata olabilir. Sayfayı yeniden yüklemeyi deneyin.",
 	"pad.modals.disconnected": "Bağlantınız koptu.",
 	"pad.modals.disconnected.explanation": "Sunucu bağlantısı kaybedildi",
 	"pad.modals.disconnected.cause": "Sunucu kullanılamıyor olabilir. Bunun devam etmesi durumunda servis yöneticisine bildirin.",
diff --git a/src/locales/zh-hant.json b/src/locales/zh-hant.json
index 79adb64f0..bd4eb6d8d 100644
--- a/src/locales/zh-hant.json
+++ b/src/locales/zh-hant.json
@@ -87,6 +87,8 @@
 	"pad.modals.deleted.explanation": "此記事本已被移除。",
 	"pad.modals.rateLimited": "比例限制。",
 	"pad.modals.rateLimited.explanation": "您發送太多訊息到此記事本，因此中斷了您的連結。",
+	"pad.modals.rejected.explanation": "伺服器拒絕了由您的瀏覽器發送的訊息。",
+	"pad.modals.rejected.cause": "當您在檢視記事本時伺服器可能正在更新，或是在 Etherpad 裡有臭蟲。請嘗試重新載入頁面。",
 	"pad.modals.disconnected": "您已中斷連線。",
 	"pad.modals.disconnected.explanation": "伺服器連接曾中斷",
 	"pad.modals.disconnected.cause": "伺服器可能無法使用。若此情況持續發生，請通知伺服器管理員。",

From 0bb8d73ba2969bc2c871c5095de19188a75b2b54 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Sat, 12 Sep 2020 23:35:41 -0400
Subject: [PATCH 04/12] PadMessageHandler: Always save the author ID in the
 session info

Before, the author ID was only saved in the session info during the
initial CLIENT_READY, not when the client sent a CLIENT_READY due to a
reconnect. This caused the handling of subsequent messages to use an
undefined author ID.
---
 src/node/handler/PadMessageHandler.js | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js
index e311e9dbf..8f3547920 100644
--- a/src/node/handler/PadMessageHandler.js
+++ b/src/node/handler/PadMessageHandler.js
@@ -53,7 +53,7 @@ const rateLimiter = new RateLimiterMemory({
  *   readonlyPadId = The readonly pad id of the pad
  *   readonly = Wether the client has only read access (true) or read/write access (false)
  *   rev = That last revision that was send to this client
- *   author = the author name of this session
+ *   author = the author ID used for this session
  */
 var sessioninfos = {};
 exports.sessioninfos = sessioninfos;
@@ -219,7 +219,7 @@ exports.handleMessage = async function(client, message)
   }
 
   const {session: {user} = {}} = client.client.request;
-  const {accessStatus} =
+  const {accessStatus, authorID} =
       await securityManager.checkAccess(padId, auth.sessionID, auth.token, auth.password, user);
 
   if (accessStatus !== "grant") {
@@ -227,6 +227,19 @@ exports.handleMessage = async function(client, message)
     client.json.send({ accessStatus });
     return;
   }
+  if (thisSession.author != null && thisSession.author !== authorID) {
+    messageLogger.warn(
+        'Rejecting message from client because the author ID changed mid-session.' +
+        ' Bad or missing token or sessionID?' +
+        ` socket:${client.id}` +
+        ` IP:${settings.disableIPlogging ? ANONYMOUS : remoteAddress[client.id]}` +
+        ` originalAuthorID:${thisSession.author}` +
+        ` newAuthorID:${authorID}` +
+        ` message:${message}`);
+    client.json.send({disconnect: 'rejected'});
+    return;
+  }
+  thisSession.author = authorID;
 
   // Allow plugins to bypass the readonly message blocker
   if ((await hooks.aCallAll('handleMessageSecurity', {client, message})).some((w) => w === true)) {
@@ -1124,8 +1137,6 @@ async function handleClientReady(client, message)
     // Save the current revision in sessioninfos, should be the same as in clientVars
     sessionInfo.rev = pad.getHeadRevisionNumber();
 
-    sessionInfo.author = authorID;
-
     // prepare the notification for the other users on the pad, that this user joined
     let messageToTheOtherUsers = {
        "type": "COLLABROOM",

From 89de03795a928450159234147b994f2f4a90df12 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Thu, 24 Sep 2020 19:14:10 -0400
Subject: [PATCH 05/12] tests: Delete unused imports and code

---
 .../backend/specs/api/importexportGetPost.js  | 35 -------------------
 1 file changed, 35 deletions(-)

diff --git a/tests/backend/specs/api/importexportGetPost.js b/tests/backend/specs/api/importexportGetPost.js
index f678f7de7..693295f85 100644
--- a/tests/backend/specs/api/importexportGetPost.js
+++ b/tests/backend/specs/api/importexportGetPost.js
@@ -9,7 +9,6 @@ const settings = require(__dirname+'/../../../../src/node/utils/Settings');
 const host = 'http://127.0.0.1:'+settings.port;
 const api = supertest('http://'+settings.ip+":"+settings.port);
 const path = require('path');
-const async = require(__dirname+'/../../../../src/node_modules/async');
 const request = require(__dirname+'/../../../../src/node_modules/request');
 const padText = fs.readFileSync("../tests/backend/specs/api/test.txt");
 const etherpadDoc = fs.readFileSync("../tests/backend/specs/api/test.etherpad");
@@ -23,8 +22,6 @@ var apiKey = fs.readFileSync(filePath,  {encoding: 'utf-8'});
 apiKey = apiKey.replace(/\n$/, "");
 var apiVersion = 1;
 var testPadId = makeid();
-var lastEdited = "";
-var text = generateLongText();
 
 describe('Connectivity', function(){
   it('can connect', function(done) {
@@ -376,35 +373,3 @@ function makeid()
   }
   return text;
 }
-
-function generateLongText(){
-  var text = "";
-  var possible = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
-
-  for( var i=0; i < 80000; i++ ){
-    text += possible.charAt(Math.floor(Math.random() * possible.length));
-  }
-  return text;
-}
-
-// Need this to compare arrays (listSavedRevisions test)
-Array.prototype.equals = function (array) {
-    // if the other array is a falsy value, return
-    if (!array)
-        return false;
-    // compare lengths - can save a lot of time
-    if (this.length != array.length)
-        return false;
-    for (var i = 0, l=this.length; i < l; i++) {
-        // Check if we have nested arrays
-        if (this[i] instanceof Array && array[i] instanceof Array) {
-            // recurse into the nested arrays
-            if (!this[i].equals(array[i]))
-                return false;
-        } else if (this[i] != array[i]) {
-            // Warning - two different object instances will never be equal: {x:20} != {x:20}
-            return false;
-        }
-    }
-    return true;
-}

From 668373b80f1d0c89b64e22ef2ee4a302b02d74b8 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Thu, 24 Sep 2020 19:21:07 -0400
Subject: [PATCH 06/12] tests: Fix abiword/soffice check

---
 .../backend/specs/api/importexportGetPost.js  | 40 +++++++++++++++----
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/tests/backend/specs/api/importexportGetPost.js b/tests/backend/specs/api/importexportGetPost.js
index 693295f85..368359449 100644
--- a/tests/backend/specs/api/importexportGetPost.js
+++ b/tests/backend/specs/api/importexportGetPost.js
@@ -109,7 +109,10 @@ describe('Imports and Exports', function(){
   // TODO: fix support for .doc files..
   it('Tries to import .doc that uses soffice or abiword', function(done) {
     if(!settings.allowAnyoneToImport) return done();
-    if((settings.abiword && settings.abiword.indexOf("/" === -1)) && (settings.office && settings.soffice.indexOf("/" === -1))) return done();
+    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
+        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      return done();
+    }
 
     var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
       if (err) {
@@ -132,7 +135,10 @@ describe('Imports and Exports', function(){
 
   it('exports DOC', function(done) {
     if(!settings.allowAnyoneToImport) return done();
-    if((settings.abiword && settings.abiword.indexOf("/" === -1)) && (settings.office && settings.soffice.indexOf("/" === -1))) return done();
+    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
+        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      return done();
+    }
     try{
       request(host + '/p/'+testPadId+'/export/doc', function (err, res, body) {
         // TODO: At some point checking that the contents is correct would be suitable
@@ -149,7 +155,10 @@ describe('Imports and Exports', function(){
 
   it('Tries to import .docx that uses soffice or abiword', function(done) {
     if(!settings.allowAnyoneToImport) return done();
-    if((settings.abiword && settings.abiword.indexOf("/" === -1)) && (settings.office && settings.soffice.indexOf("/" === -1))) return done();
+    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
+        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      return done();
+    }
 
     var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
       if (err) {
@@ -172,7 +181,10 @@ describe('Imports and Exports', function(){
 
   it('exports DOC from imported DOCX', function(done) {
     if(!settings.allowAnyoneToImport) return done();
-    if((settings.abiword && settings.abiword.indexOf("/" === -1)) && (settings.office && settings.soffice.indexOf("/" === -1))) return done();
+    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
+        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      return done();
+    }
     request(host + '/p/'+testPadId+'/export/doc', function (err, res, body) {
       // TODO: At some point checking that the contents is correct would be suitable
       if(body.length >= 9100){
@@ -185,7 +197,10 @@ describe('Imports and Exports', function(){
 
   it('Tries to import .pdf that uses soffice or abiword', function(done) {
     if(!settings.allowAnyoneToImport) return done();
-    if((settings.abiword && settings.abiword.indexOf("/" === -1)) && (settings.office && settings.soffice.indexOf("/" === -1))) return done();
+    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
+        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      return done();
+    }
 
     var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
       if (err) {
@@ -208,7 +223,10 @@ describe('Imports and Exports', function(){
 
   it('exports PDF', function(done) {
     if(!settings.allowAnyoneToImport) return done();
-    if((settings.abiword && settings.abiword.indexOf("/" === -1)) && (settings.office && settings.soffice.indexOf("/" === -1))) return done();
+    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
+        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      return done();
+    }
     request(host + '/p/'+testPadId+'/export/pdf', function (err, res, body) {
       // TODO: At some point checking that the contents is correct would be suitable
       if(body.length >= 1000){
@@ -221,7 +239,10 @@ describe('Imports and Exports', function(){
 
   it('Tries to import .odt that uses soffice or abiword', function(done) {
     if(!settings.allowAnyoneToImport) return done();
-    if((settings.abiword && settings.abiword.indexOf("/" === -1)) && (settings.office && settings.soffice.indexOf("/" === -1))) return done();
+    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
+        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      return done();
+    }
 
     var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
       if (err) {
@@ -244,7 +265,10 @@ describe('Imports and Exports', function(){
 
   it('exports ODT', function(done) {
     if(!settings.allowAnyoneToImport) return done();
-    if((settings.abiword && settings.abiword.indexOf("/" === -1)) && (settings.office && settings.soffice.indexOf("/" === -1))) return done();
+    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
+        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      return done();
+    }
     request(host + '/p/'+testPadId+'/export/odt', function (err, res, body) {
       // TODO: At some point checking that the contents is correct would be suitable
       if(body.length >= 7000){

From c148e673a8a6fefcc6a85dd556c9a911a048f888 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Thu, 24 Sep 2020 19:25:51 -0400
Subject: [PATCH 07/12] tests: Use `this.skip()` when skipping tests

---
 .../backend/specs/api/importexportGetPost.js  | 28 +++++++++++++------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/tests/backend/specs/api/importexportGetPost.js b/tests/backend/specs/api/importexportGetPost.js
index 368359449..831266ec0 100644
--- a/tests/backend/specs/api/importexportGetPost.js
+++ b/tests/backend/specs/api/importexportGetPost.js
@@ -73,6 +73,7 @@ describe('Imports and Exports', function(){
   it('creates a new Pad, imports content to it, checks that content', function(done) {
     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();
       done();
     }else{
       api.get(endPoint('createPad')+"&padID="+testPadId)
@@ -108,9 +109,10 @@ describe('Imports and Exports', function(){
   // For some reason word import does not work in testing..
   // TODO: fix support for .doc files..
   it('Tries to import .doc that uses soffice or abiword', function(done) {
-    if(!settings.allowAnyoneToImport) return done();
+    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
         (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      this.skip();
       return done();
     }
 
@@ -134,9 +136,10 @@ describe('Imports and Exports', function(){
   });
 
   it('exports DOC', function(done) {
-    if(!settings.allowAnyoneToImport) return done();
+    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
         (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      this.skip();
       return done();
     }
     try{
@@ -154,9 +157,10 @@ describe('Imports and Exports', function(){
   })
 
   it('Tries to import .docx that uses soffice or abiword', function(done) {
-    if(!settings.allowAnyoneToImport) return done();
+    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
         (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      this.skip();
       return done();
     }
 
@@ -180,9 +184,10 @@ describe('Imports and Exports', function(){
   });
 
   it('exports DOC from imported DOCX', function(done) {
-    if(!settings.allowAnyoneToImport) return done();
+    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
         (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      this.skip();
       return done();
     }
     request(host + '/p/'+testPadId+'/export/doc', function (err, res, body) {
@@ -196,9 +201,10 @@ describe('Imports and Exports', function(){
   })
 
   it('Tries to import .pdf that uses soffice or abiword', function(done) {
-    if(!settings.allowAnyoneToImport) return done();
+    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
         (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      this.skip();
       return done();
     }
 
@@ -222,9 +228,10 @@ describe('Imports and Exports', function(){
   });
 
   it('exports PDF', function(done) {
-    if(!settings.allowAnyoneToImport) return done();
+    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
         (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      this.skip();
       return done();
     }
     request(host + '/p/'+testPadId+'/export/pdf', function (err, res, body) {
@@ -238,9 +245,10 @@ describe('Imports and Exports', function(){
   })
 
   it('Tries to import .odt that uses soffice or abiword', function(done) {
-    if(!settings.allowAnyoneToImport) return done();
+    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
         (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      this.skip();
       return done();
     }
 
@@ -264,9 +272,10 @@ describe('Imports and Exports', function(){
   });
 
   it('exports ODT', function(done) {
-    if(!settings.allowAnyoneToImport) return done();
+    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
         (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+      this.skip();
       return done();
     }
     request(host + '/p/'+testPadId+'/export/odt', function (err, res, body) {
@@ -280,7 +289,7 @@ describe('Imports and Exports', function(){
   })
 
   it('Tries to import .etherpad', function(done) {
-    if(!settings.allowAnyoneToImport) return done();
+    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
 
     var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
       if (err) {
@@ -353,6 +362,7 @@ describe('Imports and Exports', function(){
   it('Tries to import unsupported file type', function(done) {
     if(settings.allowUnknownFileEnds === true){
       console.log("allowing unknown file ends so skipping this test");
+      this.skip();
       return done();
     }
 

From 1c3c5b744cff66ac6d1906770d032fc838ba8349 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Thu, 24 Sep 2020 19:28:32 -0400
Subject: [PATCH 08/12] tests: Skip all import/export tests if
 `!allowAnyoneToImport`

Three of the four tests fail if `settings.allowAnyoneToImport` is
false. The fourth ("tries to import Plain Text to a pad that does not
exist") isn't particularly useful when `settings.allowAnyoneToImport`
is false: That test tests an import failure mode, and when
`settings.allowAnyoneToImport` is false the failure could be caused by
that instead of the expected cause.
---
 tests/backend/specs/api/importexportGetPost.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/backend/specs/api/importexportGetPost.js b/tests/backend/specs/api/importexportGetPost.js
index 831266ec0..e372a9a85 100644
--- a/tests/backend/specs/api/importexportGetPost.js
+++ b/tests/backend/specs/api/importexportGetPost.js
@@ -311,6 +311,7 @@ describe('Imports and Exports', function(){
   });
 
   it('exports Etherpad', function(done) {
+    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     request(host + '/p/'+testPadId+'/export/etherpad', function (err, res, body) {
       // TODO: At some point checking that the contents is correct would be suitable
       if(body.indexOf("hello") !== -1){
@@ -323,6 +324,7 @@ describe('Imports and Exports', function(){
   })
 
   it('exports HTML for this Etherpad file', function(done) {
+    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     request(host + '/p/'+testPadId+'/export/html', function (err, res, body) {
 
       // broken pre fix export -- <ul class="bullet"></li><ul class="bullet"></ul></li></ul>
@@ -338,6 +340,7 @@ describe('Imports and Exports', function(){
   })
 
   it('tries to import Plain Text to a pad that does not exist', function(done) {
+    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     var req = request.post(host + '/p/'+testPadId+testPadId+testPadId+'/import', function (err, res, body) {
       if (res.statusCode === 200) {
         throw new Error("Was able to import to a pad that doesn't exist");
@@ -360,6 +363,7 @@ describe('Imports and Exports', function(){
   });
 
   it('Tries to import unsupported file type', function(done) {
+    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     if(settings.allowUnknownFileEnds === true){
       console.log("allowing unknown file ends so skipping this test");
       this.skip();

From 54c999fe83700487be1b3228c7186716a140f0df Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Fri, 25 Sep 2020 13:55:19 -0400
Subject: [PATCH 09/12] tests: Factor out common skip checks

---
 .../backend/specs/api/importexportGetPost.js  | 352 ++++++++----------
 1 file changed, 153 insertions(+), 199 deletions(-)

diff --git a/tests/backend/specs/api/importexportGetPost.js b/tests/backend/specs/api/importexportGetPost.js
index e372a9a85..7d84bc71a 100644
--- a/tests/backend/specs/api/importexportGetPost.js
+++ b/tests/backend/specs/api/importexportGetPost.js
@@ -70,227 +70,185 @@ Example Curl command for testing import URI:
 */
 
 describe('Imports and Exports', function(){
-  it('creates a new Pad, imports content to it, checks that content', function(done) {
-    if(!settings.allowAnyoneToImport){
-      console.warn("not anyone can import so not testing -- to include this test set allowAnyoneToImport to true in settings.json");
+  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();
-      done();
-    }else{
-      api.get(endPoint('createPad')+"&padID="+testPadId)
-      .expect(function(res){
-        if(res.body.code !== 0) throw new Error("Unable to create new Pad");
-
-        var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
-         if (err) {
-            throw new Error("Failed to import", err);
-          } else {
-            api.get(endPoint('getText')+"&padID="+testPadId)
-            .expect(function(res){
-              if(res.body.data.text !== padText.toString()){
-                throw new Error("text is wrong on export");
-              }
-            })
-          }
-        });
-
-        let form = req.form();
-
-        form.append('file', padText, {
-          filename: '/test.txt',
-          contentType: 'text/plain'
-        });
-
-      })
-      .expect('Content-Type', /json/)
-      .expect(200, done)
     }
   });
 
-  // For some reason word import does not work in testing..
-  // TODO: fix support for .doc files..
-  it('Tries to import .doc that uses soffice or abiword', function(done) {
-    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
-    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
-        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
-      this.skip();
-      return done();
-    }
+  it('creates a new Pad, imports content to it, checks that content', function(done) {
+    api.get(endPoint('createPad') + "&padID=" + testPadId)
+        .expect(function(res) {
+          if (res.body.code !== 0) throw new Error("Unable to create new Pad");
 
-    var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
-      if (err) {
-        throw new Error("Failed to import", err);
-      } else {
-        if(res.body.indexOf("FrameCall('undefined', 'ok');") === -1){
-          throw new Error("Failed DOC import", testPadId);
-        }else{
-          done();
-        }
+          var req = request.post(host + '/p/' + testPadId + '/import', function(err, res, body) {
+            if (err) {
+              throw new Error("Failed to import", err);
+            } else {
+              api.get(endPoint('getText')+"&padID="+testPadId)
+                  .expect(function(res){
+                    if(res.body.data.text !== padText.toString()){
+                      throw new Error("text is wrong on export");
+                    }
+                  })
+            }
+          });
+
+          let form = req.form();
+
+          form.append('file', padText, {
+            filename: '/test.txt',
+            contentType: 'text/plain'
+          });
+
+        })
+        .expect('Content-Type', /json/)
+        .expect(200, done)
+  });
+
+  describe('Import/Export tests requiring AbiWord/LibreOffice', function() {
+    before(function() {
+      if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
+          (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
+        this.skip();
       }
     });
 
-    let form = req.form();
-    form.append('file', wordDoc, {
-      filename: '/test.doc',
-      contentType: 'application/msword'
-    });
-  });
+    // For some reason word import does not work in testing..
+    // TODO: fix support for .doc files..
+    it('Tries to import .doc that uses soffice or abiword', function(done) {
+      var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
+        if (err) {
+          throw new Error("Failed to import", err);
+        } else {
+          if(res.body.indexOf("FrameCall('undefined', 'ok');") === -1){
+            throw new Error("Failed DOC import", testPadId);
+          }else{
+            done();
+          }
+        }
+      });
 
-  it('exports DOC', function(done) {
-    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
-    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
-        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
-      this.skip();
-      return done();
-    }
-    try{
+      let form = req.form();
+      form.append('file', wordDoc, {
+        filename: '/test.doc',
+        contentType: 'application/msword'
+      });
+    });
+
+    it('exports DOC', function(done) {
+      try{
+        request(host + '/p/'+testPadId+'/export/doc', function (err, res, body) {
+          // TODO: At some point checking that the contents is correct would be suitable
+          if(body.length >= 9000){
+            done();
+          }else{
+            throw new Error("Word Document export length is not right");
+          }
+        })
+      }catch(e){
+        throw new Error(e);
+      }
+    });
+
+    it('Tries to import .docx that uses soffice or abiword', function(done) {
+      var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
+        if (err) {
+          throw new Error("Failed to import", err);
+        } else {
+          if(res.body.indexOf("FrameCall('undefined', 'ok');") === -1){
+            throw new Error("Failed DOCX import");
+          }else{
+            done();
+          }
+        }
+      });
+
+      let form = req.form();
+      form.append('file', wordXDoc, {
+        filename: '/test.docx',
+        contentType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'
+      });
+    });
+
+    it('exports DOC from imported DOCX', function(done) {
       request(host + '/p/'+testPadId+'/export/doc', function (err, res, body) {
         // TODO: At some point checking that the contents is correct would be suitable
-        if(body.length >= 9000){
+        if(body.length >= 9100){
           done();
         }else{
           throw new Error("Word Document export length is not right");
         }
       })
-    }catch(e){
-      throw new Error(e);
-    }
-  })
+    });
 
-  it('Tries to import .docx that uses soffice or abiword', function(done) {
-    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
-    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
-        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
-      this.skip();
-      return done();
-    }
-
-    var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
-      if (err) {
-        throw new Error("Failed to import", err);
-      } else {
-        if(res.body.indexOf("FrameCall('undefined', 'ok');") === -1){
-          throw new Error("Failed DOCX import");
-        }else{
-          done();
+    it('Tries to import .pdf that uses soffice or abiword', function(done) {
+      var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
+        if (err) {
+          throw new Error("Failed to import", err);
+        } else {
+          if(res.body.indexOf("FrameCall('undefined', 'ok');") === -1){
+            throw new Error("Failed PDF import");
+          }else{
+            done();
+          }
         }
-      }
+      });
+
+      let form = req.form();
+      form.append('file', pdfDoc, {
+        filename: '/test.pdf',
+        contentType: 'application/pdf'
+      });
     });
 
-    let form = req.form();
-    form.append('file', wordXDoc, {
-      filename: '/test.docx',
-      contentType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'
-    });
-  });
-
-  it('exports DOC from imported DOCX', function(done) {
-    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
-    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
-        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
-      this.skip();
-      return done();
-    }
-    request(host + '/p/'+testPadId+'/export/doc', function (err, res, body) {
-      // TODO: At some point checking that the contents is correct would be suitable
-      if(body.length >= 9100){
-        done();
-      }else{
-        throw new Error("Word Document export length is not right");
-      }
-    })
-  })
-
-  it('Tries to import .pdf that uses soffice or abiword', function(done) {
-    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
-    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
-        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
-      this.skip();
-      return done();
-    }
-
-    var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
-      if (err) {
-        throw new Error("Failed to import", err);
-      } else {
-        if(res.body.indexOf("FrameCall('undefined', 'ok');") === -1){
-          throw new Error("Failed PDF import");
-        }else{
+    it('exports PDF', function(done) {
+      request(host + '/p/'+testPadId+'/export/pdf', function (err, res, body) {
+        // TODO: At some point checking that the contents is correct would be suitable
+        if(body.length >= 1000){
           done();
-        }
-      }
-    });
-
-    let form = req.form();
-    form.append('file', pdfDoc, {
-      filename: '/test.pdf',
-      contentType: 'application/pdf'
-    });
-  });
-
-  it('exports PDF', function(done) {
-    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
-    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
-        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
-      this.skip();
-      return done();
-    }
-    request(host + '/p/'+testPadId+'/export/pdf', function (err, res, body) {
-      // TODO: At some point checking that the contents is correct would be suitable
-      if(body.length >= 1000){
-        done();
-      }else{
-        throw new Error("PDF Document export length is not right");
-      }
-    })
-  })
-
-  it('Tries to import .odt that uses soffice or abiword', function(done) {
-    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
-    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
-        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
-      this.skip();
-      return done();
-    }
-
-    var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
-      if (err) {
-        throw new Error("Failed to import", err);
-      } else {
-        if(res.body.indexOf("FrameCall('undefined', 'ok');") === -1){
-          throw new Error("Failed ODT import", testPadId);
         }else{
-          done();
+          throw new Error("PDF Document export length is not right");
         }
-      }
+      })
     });
 
-    let form = req.form();
-    form.append('file', odtDoc, {
-      filename: '/test.odt',
-      contentType: 'application/odt'
-    });
-  });
+    it('Tries to import .odt that uses soffice or abiword', function(done) {
+      var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
+        if (err) {
+          throw new Error("Failed to import", err);
+        } else {
+          if(res.body.indexOf("FrameCall('undefined', 'ok');") === -1){
+            throw new Error("Failed ODT import", testPadId);
+          }else{
+            done();
+          }
+        }
+      });
 
-  it('exports ODT', function(done) {
-    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
-    if ((!settings.abiword || settings.abiword.indexOf('/') === -1) &&
-        (!settings.soffice || settings.soffice.indexOf('/') === -1)) {
-      this.skip();
-      return done();
-    }
-    request(host + '/p/'+testPadId+'/export/odt', function (err, res, body) {
-      // TODO: At some point checking that the contents is correct would be suitable
-      if(body.length >= 7000){
-        done();
-      }else{
-        throw new Error("ODT Document export length is not right");
-      }
-    })
-  })
+      let form = req.form();
+      form.append('file', odtDoc, {
+        filename: '/test.odt',
+        contentType: 'application/odt'
+      });
+    });
+
+    it('exports ODT', function(done) {
+      request(host + '/p/'+testPadId+'/export/odt', function (err, res, body) {
+        // TODO: At some point checking that the contents is correct would be suitable
+        if(body.length >= 7000){
+          done();
+        }else{
+          throw new Error("ODT Document export length is not right");
+        }
+      })
+    });
+
+  }); // End of AbiWord/LibreOffice tests.
 
   it('Tries to import .etherpad', function(done) {
-    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
-
     var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
       if (err) {
         throw new Error("Failed to import", err);
@@ -311,7 +269,6 @@ describe('Imports and Exports', function(){
   });
 
   it('exports Etherpad', function(done) {
-    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     request(host + '/p/'+testPadId+'/export/etherpad', function (err, res, body) {
       // TODO: At some point checking that the contents is correct would be suitable
       if(body.indexOf("hello") !== -1){
@@ -321,10 +278,9 @@ describe('Imports and Exports', function(){
         throw new Error("Etherpad Document does not include hello");
       }
     })
-  })
+  });
 
   it('exports HTML for this Etherpad file', function(done) {
-    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     request(host + '/p/'+testPadId+'/export/html', function (err, res, body) {
 
       // broken pre fix export -- <ul class="bullet"></li><ul class="bullet"></ul></li></ul>
@@ -337,10 +293,9 @@ describe('Imports and Exports', function(){
         throw new Error("Exported HTML nested list items is not right", body);
       }
     })
-  })
+  });
 
   it('tries to import Plain Text to a pad that does not exist', function(done) {
-    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     var req = request.post(host + '/p/'+testPadId+testPadId+testPadId+'/import', function (err, res, body) {
       if (res.statusCode === 200) {
         throw new Error("Was able to import to a pad that doesn't exist");
@@ -363,7 +318,6 @@ describe('Imports and Exports', function(){
   });
 
   it('Tries to import unsupported file type', function(done) {
-    if (!settings.allowAnyoneToImport) { this.skip(); return done(); }
     if(settings.allowUnknownFileEnds === true){
       console.log("allowing unknown file ends so skipping this test");
       this.skip();

From 23131a501c35e192d8c71bdb775390522da1037b Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Thu, 24 Sep 2020 19:32:02 -0400
Subject: [PATCH 10/12] tests: Rewrite import/export tests to use async and
 supertest

---
 src/package-lock.json                         |  71 ++++
 src/package.json                              |   1 +
 .../backend/specs/api/importexportGetPost.js  | 341 +++++-------------
 3 files changed, 172 insertions(+), 241 deletions(-)

diff --git a/src/package-lock.json b/src/package-lock.json
index 6b9b9b0c8..7ff64c3b1 100644
--- a/src/package-lock.json
+++ b/src/package-lock.json
@@ -8235,6 +8235,77 @@
       "requires": {
         "methods": "^1.1.2",
         "superagent": "^3.8.3"
+      },
+      "dependencies": {
+        "debug": {
+          "version": "3.2.6",
+          "resolved": "https://registry.npmjs.org/debug/-/debug-3.2.6.tgz",
+          "integrity": "sha512-mel+jf7nrtEl5Pn1Qx46zARXKDpBbvzezse7p7LqINmdoIk8PYP5SySaxEmYv6TZ0JyEKA1hsCId6DIhgITtWQ==",
+          "dev": true,
+          "requires": {
+            "ms": "^2.1.1"
+          }
+        },
+        "isarray": {
+          "version": "1.0.0",
+          "resolved": "https://registry.npmjs.org/isarray/-/isarray-1.0.0.tgz",
+          "integrity": "sha1-u5NdSFgsuhaMBoNJV6VKPgcSTxE=",
+          "dev": true
+        },
+        "ms": {
+          "version": "2.1.2",
+          "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz",
+          "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==",
+          "dev": true
+        },
+        "readable-stream": {
+          "version": "2.3.7",
+          "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.7.tgz",
+          "integrity": "sha512-Ebho8K4jIbHAxnuxi7o42OrZgF/ZTNcsZj6nRKyUmkhLFq8CHItp/fy6hQZuZmP/n3yZ9VBUbp4zz/mX8hmYPw==",
+          "dev": true,
+          "requires": {
+            "core-util-is": "~1.0.0",
+            "inherits": "~2.0.3",
+            "isarray": "~1.0.0",
+            "process-nextick-args": "~2.0.0",
+            "safe-buffer": "~5.1.1",
+            "string_decoder": "~1.1.1",
+            "util-deprecate": "~1.0.1"
+          }
+        },
+        "safe-buffer": {
+          "version": "5.1.2",
+          "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.1.2.tgz",
+          "integrity": "sha512-Gd2UZBJDkXlY7GbJxfsE8/nvKkUEU1G38c1siN6QP6a9PT9MmHB8GnpscSmMJSoF8LOIrt8ud/wPtojys4G6+g==",
+          "dev": true
+        },
+        "string_decoder": {
+          "version": "1.1.1",
+          "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.1.1.tgz",
+          "integrity": "sha512-n/ShnvDi6FHbbVfviro+WojiFzv+s8MPMHBczVePfUpDJLwoLT0ht1l4YwBCbi8pJAveEEdnkHyPyTP/mzRfwg==",
+          "dev": true,
+          "requires": {
+            "safe-buffer": "~5.1.0"
+          }
+        },
+        "superagent": {
+          "version": "3.8.3",
+          "resolved": "https://registry.npmjs.org/superagent/-/superagent-3.8.3.tgz",
+          "integrity": "sha512-GLQtLMCoEIK4eDv6OGtkOoSMt3D+oq0y3dsxMuYuDvaNUvuT8eFBuLmfR0iYYzHC1e8hpzC6ZsxbuP6DIalMFA==",
+          "dev": true,
+          "requires": {
+            "component-emitter": "^1.2.0",
+            "cookiejar": "^2.1.0",
+            "debug": "^3.1.0",
+            "extend": "^3.0.0",
+            "form-data": "^2.3.1",
+            "formidable": "^1.2.0",
+            "methods": "^1.1.1",
+            "mime": "^1.4.1",
+            "qs": "^6.5.1",
+            "readable-stream": "^2.3.5"
+          }
+        }
       }
     },
     "supports-color": {
diff --git a/src/package.json b/src/package.json
index 9575ab117..ee45fa9a6 100644
--- a/src/package.json
+++ b/src/package.json
@@ -79,6 +79,7 @@
     "mocha-froth": "^0.2.10",
     "nyc": "15.0.1",
     "set-cookie-parser": "^2.4.6",
+    "superagent": "^3.8.3",
     "supertest": "4.0.2",
     "wd": "1.12.1"
   },
diff --git a/tests/backend/specs/api/importexportGetPost.js b/tests/backend/specs/api/importexportGetPost.js
index 7d84bc71a..aa72f7072 100644
--- a/tests/backend/specs/api/importexportGetPost.js
+++ b/tests/backend/specs/api/importexportGetPost.js
@@ -2,14 +2,14 @@
  * Import and Export tests for the /p/whateverPadId/import and /p/whateverPadId/export endpoints.
  */
 
-const assert = require('assert');
+const assert = require('assert').strict;
+const superagent = require(__dirname+'/../../../../src/node_modules/superagent');
 const supertest = require(__dirname+'/../../../../src/node_modules/supertest');
 const fs = require('fs');
 const settings = require(__dirname+'/../../../../src/node/utils/Settings');
 const host = 'http://127.0.0.1:'+settings.port;
-const api = supertest('http://'+settings.ip+":"+settings.port);
+const agent = supertest(`http://${settings.ip}:${settings.port}`);
 const path = require('path');
-const request = require(__dirname+'/../../../../src/node_modules/request');
 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");
@@ -24,22 +24,18 @@ var apiVersion = 1;
 var testPadId = makeid();
 
 describe('Connectivity', function(){
-  it('can connect', function(done) {
-    api.get('/api/')
-    .expect('Content-Type', /json/)
-    .expect(200, done)
+  it('can connect', async function() {
+    await agent.get('/api/')
+        .expect(200)
+        .expect('Content-Type', /json/);
   });
 })
 
 describe('API Versioning', function(){
-  it('finds the version tag', function(done) {
-    api.get('/api/')
-    .expect(function(res){
-      apiVersion = res.body.currentVersion;
-      if (!res.body.currentVersion) throw new Error("No version set in API");
-      return;
-    })
-    .expect(200, done)
+  it('finds the version tag', async function() {
+    await agent.get('/api/')
+        .expect(200)
+        .expect((res) => assert(res.body.currentVersion));
   });
 })
 
@@ -78,34 +74,17 @@ describe('Imports and Exports', function(){
     }
   });
 
-  it('creates a new Pad, imports content to it, checks that content', function(done) {
-    api.get(endPoint('createPad') + "&padID=" + testPadId)
-        .expect(function(res) {
-          if (res.body.code !== 0) throw new Error("Unable to create new Pad");
-
-          var req = request.post(host + '/p/' + testPadId + '/import', function(err, res, body) {
-            if (err) {
-              throw new Error("Failed to import", err);
-            } else {
-              api.get(endPoint('getText')+"&padID="+testPadId)
-                  .expect(function(res){
-                    if(res.body.data.text !== padText.toString()){
-                      throw new Error("text is wrong on export");
-                    }
-                  })
-            }
-          });
-
-          let form = req.form();
-
-          form.append('file', padText, {
-            filename: '/test.txt',
-            contentType: 'text/plain'
-          });
-
-        })
+  it('creates a new Pad, imports content to it, checks that content', async function() {
+    await agent.get(endPoint('createPad') + `&padID=${testPadId}`)
+        .expect(200)
         .expect('Content-Type', /json/)
-        .expect(200, done)
+        .expect((res) => assert.equal(res.body.code, 0));
+    await agent.post(`/p/${testPadId}/import`)
+        .attach('file', padText, {filename: '/test.txt', contentType: 'text/plain'})
+        .expect(200);
+    await agent.get(endPoint('getText') + `&padID=${testPadId}`)
+        .expect(200)
+        .expect((res) => assert.equal(res.body.data.text, padText.toString()));
   });
 
   describe('Import/Export tests requiring AbiWord/LibreOffice', function() {
@@ -118,233 +97,113 @@ describe('Imports and Exports', function(){
 
     // For some reason word import does not work in testing..
     // TODO: fix support for .doc files..
-    it('Tries to import .doc that uses soffice or abiword', function(done) {
-      var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
-        if (err) {
-          throw new Error("Failed to import", err);
-        } else {
-          if(res.body.indexOf("FrameCall('undefined', 'ok');") === -1){
-            throw new Error("Failed DOC import", testPadId);
-          }else{
-            done();
-          }
-        }
-      });
-
-      let form = req.form();
-      form.append('file', wordDoc, {
-        filename: '/test.doc',
-        contentType: 'application/msword'
-      });
+    it('Tries to import .doc that uses soffice or abiword', async function() {
+      await agent.post(`/p/${testPadId}/import`)
+          .attach('file', wordDoc, {filename: '/test.doc', contentType: 'application/msword'})
+          .expect(200)
+          .expect(/FrameCall\('undefined', 'ok'\);/);
     });
 
-    it('exports DOC', function(done) {
-      try{
-        request(host + '/p/'+testPadId+'/export/doc', function (err, res, body) {
-          // TODO: At some point checking that the contents is correct would be suitable
-          if(body.length >= 9000){
-            done();
-          }else{
-            throw new Error("Word Document export length is not right");
-          }
-        })
-      }catch(e){
-        throw new Error(e);
-      }
+    it('exports DOC', async function() {
+      await agent.get(`/p/${testPadId}/export/doc`)
+          .buffer(true).parse(superagent.parse['application/octet-stream'])
+          .expect(200)
+          .expect((res) => assert(res.body.length >= 9000));
     });
 
-    it('Tries to import .docx that uses soffice or abiword', function(done) {
-      var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
-        if (err) {
-          throw new Error("Failed to import", err);
-        } else {
-          if(res.body.indexOf("FrameCall('undefined', 'ok');") === -1){
-            throw new Error("Failed DOCX import");
-          }else{
-            done();
-          }
-        }
-      });
-
-      let form = req.form();
-      form.append('file', wordXDoc, {
-        filename: '/test.docx',
-        contentType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'
-      });
+    it('Tries to import .docx that uses soffice or abiword', async function() {
+      await agent.post(`/p/${testPadId}/import`)
+          .attach('file', wordXDoc, {
+            filename: '/test.docx',
+            contentType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
+          })
+          .expect(200)
+          .expect(/FrameCall\('undefined', 'ok'\);/);
     });
 
-    it('exports DOC from imported DOCX', function(done) {
-      request(host + '/p/'+testPadId+'/export/doc', function (err, res, body) {
-        // TODO: At some point checking that the contents is correct would be suitable
-        if(body.length >= 9100){
-          done();
-        }else{
-          throw new Error("Word Document export length is not right");
-        }
-      })
+    it('exports DOC from imported DOCX', async function() {
+      await agent.get(`/p/${testPadId}/export/doc`)
+          .buffer(true).parse(superagent.parse['application/octet-stream'])
+          .expect(200)
+          .expect((res) => assert(res.body.length >= 9100));
     });
 
-    it('Tries to import .pdf that uses soffice or abiword', function(done) {
-      var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
-        if (err) {
-          throw new Error("Failed to import", err);
-        } else {
-          if(res.body.indexOf("FrameCall('undefined', 'ok');") === -1){
-            throw new Error("Failed PDF import");
-          }else{
-            done();
-          }
-        }
-      });
-
-      let form = req.form();
-      form.append('file', pdfDoc, {
-        filename: '/test.pdf',
-        contentType: 'application/pdf'
-      });
+    it('Tries to import .pdf that uses soffice or abiword', async function() {
+      await agent.post(`/p/${testPadId}/import`)
+          .attach('file', pdfDoc, {filename: '/test.pdf', contentType: 'application/pdf'})
+          .expect(200)
+          .expect(/FrameCall\('undefined', 'ok'\);/);
     });
 
-    it('exports PDF', function(done) {
-      request(host + '/p/'+testPadId+'/export/pdf', function (err, res, body) {
-        // TODO: At some point checking that the contents is correct would be suitable
-        if(body.length >= 1000){
-          done();
-        }else{
-          throw new Error("PDF Document export length is not right");
-        }
-      })
+    it('exports PDF', async function() {
+      await agent.get(`/p/${testPadId}/export/pdf`)
+          .buffer(true).parse(superagent.parse['application/octet-stream'])
+          .expect(200)
+          .expect((res) => assert(res.body.length >= 1000));
     });
 
-    it('Tries to import .odt that uses soffice or abiword', function(done) {
-      var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
-        if (err) {
-          throw new Error("Failed to import", err);
-        } else {
-          if(res.body.indexOf("FrameCall('undefined', 'ok');") === -1){
-            throw new Error("Failed ODT import", testPadId);
-          }else{
-            done();
-          }
-        }
-      });
-
-      let form = req.form();
-      form.append('file', odtDoc, {
-        filename: '/test.odt',
-        contentType: 'application/odt'
-      });
+    it('Tries to import .odt that uses soffice or abiword', async function() {
+      await agent.post(`/p/${testPadId}/import`)
+          .attach('file', odtDoc, {filename: '/test.odt', contentType: 'application/odt'})
+          .expect(200)
+          .expect(/FrameCall\('undefined', 'ok'\);/);
     });
 
-    it('exports ODT', function(done) {
-      request(host + '/p/'+testPadId+'/export/odt', function (err, res, body) {
-        // TODO: At some point checking that the contents is correct would be suitable
-        if(body.length >= 7000){
-          done();
-        }else{
-          throw new Error("ODT Document export length is not right");
-        }
-      })
+    it('exports ODT', async function() {
+      await agent.get(`/p/${testPadId}/export/odt`)
+          .buffer(true).parse(superagent.parse['application/octet-stream'])
+          .expect(200)
+          .expect((res) => assert(res.body.length >= 7000));
     });
 
   }); // End of AbiWord/LibreOffice tests.
 
-  it('Tries to import .etherpad', function(done) {
-    var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
-      if (err) {
-        throw new Error("Failed to import", err);
-      } else {
-        if(res.body.indexOf("FrameCall(\'true\', \'ok\');") === -1){
-          throw new Error("Failed Etherpad import", err, testPadId);
-        }else{
-          done();
-        }
-      }
-    });
-
-    let form = req.form();
-    form.append('file', etherpadDoc, {
-      filename: '/test.etherpad',
-      contentType: 'application/etherpad'
-    });
+  it('Tries to import .etherpad', async function() {
+    await agent.post(`/p/${testPadId}/import`)
+        .attach('file', etherpadDoc, {
+          filename: '/test.etherpad',
+          contentType: 'application/etherpad',
+        })
+        .expect(200)
+        .expect(/FrameCall\('true', 'ok'\);/);
   });
 
-  it('exports Etherpad', function(done) {
-    request(host + '/p/'+testPadId+'/export/etherpad', function (err, res, body) {
-      // TODO: At some point checking that the contents is correct would be suitable
-      if(body.indexOf("hello") !== -1){
-        done();
-      }else{
-        console.error("body");
-        throw new Error("Etherpad Document does not include hello");
-      }
-    })
+  it('exports Etherpad', async function() {
+    await agent.get(`/p/${testPadId}/export/etherpad`)
+        .buffer(true).parse(superagent.parse.text)
+        .expect(200)
+        .expect(/hello/);
   });
 
-  it('exports HTML for this Etherpad file', function(done) {
-    request(host + '/p/'+testPadId+'/export/html', function (err, res, body) {
-
-      // broken pre fix export -- <ul class="bullet"></li><ul class="bullet"></ul></li></ul>
-      var expectedHTML = '<ul class="bullet"><li><ul class="bullet"><li>hello</ul></li></ul>';
-      // expect body to include
-      if(body.indexOf(expectedHTML) !== -1){
-        done();
-      }else{
-        console.error(body);
-        throw new Error("Exported HTML nested list items is not right", body);
-      }
-    })
+  it('exports HTML for this Etherpad file', async function() {
+    await agent.get(`/p/${testPadId}/export/html`)
+        .expect(200)
+        .expect('content-type', 'text/html; charset=utf-8')
+        .expect(/<ul class="bullet"><li><ul class="bullet"><li>hello<\/ul><\/li><\/ul>/);
   });
 
-  it('tries to import Plain Text to a pad that does not exist', function(done) {
-    var req = request.post(host + '/p/'+testPadId+testPadId+testPadId+'/import', function (err, res, body) {
-      if (res.statusCode === 200) {
-        throw new Error("Was able to import to a pad that doesn't exist");
-      }else{
-          // Wasn't able to write to a pad that doesn't exist, this is expected behavior
-          api.get(endPoint('getText')+"&padID="+testPadId+testPadId+testPadId)
-          .expect(function(res){
-            if(res.body.code !== 1) throw new Error("Pad Exists");
-          })
-          .expect(200, done)
-      }
-
-      let form = req.form();
-
-      form.append('file', padText, {
-        filename: '/test.txt',
-        contentType: 'text/plain'
-      });
-    })
+  it('tries to import Plain Text to a pad that does not exist', async function() {
+    const padId = testPadId + testPadId + testPadId;
+    await agent.post(`/p/${padId}/import`)
+        .attach('file', padText, {filename: '/test.txt', contentType: 'text/plain'})
+        .expect(405);
+    await agent.get(endPoint('getText') + `&padID=${padId}`)
+        .expect(200)
+        .expect((res) => assert.equal(res.body.code, 1));
   });
 
-  it('Tries to import unsupported file type', function(done) {
-    if(settings.allowUnknownFileEnds === true){
-      console.log("allowing unknown file ends so skipping this test");
-      this.skip();
-      return done();
+  it('Tries to import unsupported file type', async function() {
+    if (settings.allowUnknownFileEnds === true) {
+      console.log('skipping test because allowUnknownFileEnds is true');
+      return this.skip();
     }
-
-    var req = request.post(host + '/p/'+testPadId+'/import', function (err, res, body) {
-      if (err) {
-        throw new Error("Failed to import", err);
-      } else {
-        if(res.body.indexOf("FrameCall('undefined', 'ok');") !== -1){
-          console.log("worked");
-          throw new Error("You shouldn't be able to import this file", testPadId);
-        }
-        return done();
-      }
-    });
-
-    let form = req.form();
-    form.append('file', padText, {
-      filename: '/test.xasdasdxx',
-      contentType: 'weirdness/jobby'
-    });
+    await agent.post(`/p/${testPadId}/import`)
+        .attach('file', padText, {filename: '/test.xasdasdxx', contentType: 'weirdness/jobby'})
+        .expect(200)
+        .expect((res) => assert.doesNotMatch(res.text, /FrameCall\('undefined', 'ok'\);/));
   });
 
-// end of tests
-})
+}); // End of tests.
 
 
 
@@ -352,7 +211,7 @@ describe('Imports and Exports', function(){
 
 var endPoint = function(point, version){
   version = version || apiVersion;
-  return '/api/'+version+'/'+point+'?apikey='+apiKey;
+  return `/api/${version}/${point}?apikey=${apiKey}`;
 }
 
 function makeid()

From 3c9ae57bb33fbf047aa4872b33b1493d2ebc1109 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Fri, 4 Sep 2020 17:35:42 -0400
Subject: [PATCH 11/12] PadMessageHandler: Block Promise resolution until
 message is handled

Benefits:
  * More functions are now async which makes it possible for future
    changes to use await in those functions.
  * This will help keep the server from drowning in too many messages
    if we ever add acknowledgements or if WebSocket backpressure ever
    becomes reality.
  * This might make tests less flaky because changes triggered by a
    message will complete before the Promise resolves.
---
 src/node/handler/PadMessageHandler.js | 33 +++++++++++++++------------
 src/node/handler/SocketIORouter.js    |  8 +++----
 2 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js
index 8f3547920..a106bc252 100644
--- a/src/node/handler/PadMessageHandler.js
+++ b/src/node/handler/PadMessageHandler.js
@@ -259,9 +259,9 @@ exports.handleMessage = async function(client, message)
 
   // Check what type of message we get and delegate to the other methods
   if (message.type === "CLIENT_READY") {
-    handleClientReady(client, message);
+    await handleClientReady(client, message);
   } else if (message.type === "CHANGESET_REQ") {
-    handleChangesetRequest(client, message);
+    await handleChangesetRequest(client, message);
   } else if(message.type === "COLLABROOM") {
     if (thisSession.readonly) {
       messageLogger.warn("Dropped message, COLLABROOM for readonly pad");
@@ -269,13 +269,13 @@ exports.handleMessage = async function(client, message)
       stats.counter('pendingEdits').inc()
       padChannels.emit(message.padId, {client: client, message: message}); // add to pad queue
     } else if (message.data.type === "USERINFO_UPDATE") {
-      handleUserInfoUpdate(client, message);
+      await handleUserInfoUpdate(client, message);
     } else if (message.data.type === "CHAT_MESSAGE") {
-      handleChatMessage(client, message);
+      await handleChatMessage(client, message);
     } else if (message.data.type === "GET_CHAT_MESSAGES") {
-      handleGetChatMessages(client, message);
+      await handleGetChatMessages(client, message);
     } else if (message.data.type === "SAVE_REVISION") {
-      handleSaveRevisionMessage(client, message);
+      await handleSaveRevisionMessage(client, message);
     } else if (message.data.type === "CLIENT_MESSAGE" &&
                message.data.payload != null &&
                message.data.payload.type === "suggestUserName") {
@@ -284,7 +284,7 @@ exports.handleMessage = async function(client, message)
       messageLogger.warn("Dropped message, unknown COLLABROOM Data  Type " + message.data.type);
     }
   } else if(message.type === "SWITCH_TO_PAD") {
-    handleSwitchToPad(client, message);
+    await handleSwitchToPad(client, message);
   } else {
     messageLogger.warn("Dropped message, unknown Message Type " + message.type);
   }
@@ -347,14 +347,14 @@ exports.handleCustomMessage = function(padID, msgString) {
  * @param client the client that send this message
  * @param message the message from the client
  */
-function handleChatMessage(client, message)
+async function handleChatMessage(client, message)
 {
   var time = Date.now();
   var userId = sessioninfos[client.id].author;
   var text = message.data.text;
   var padId = sessioninfos[client.id].padId;
 
-  exports.sendChatMessageToPadClients(time, userId, text, padId);
+  await exports.sendChatMessageToPadClients(time, userId, text, padId);
 }
 
 /**
@@ -463,7 +463,7 @@ function handleSuggestUserName(client, message)
  * @param client the client that send this message
  * @param message the message from the client
  */
-function handleUserInfoUpdate(client, message)
+async function handleUserInfoUpdate(client, message)
 {
   // check if all ok
   if (message.data.userInfo == null) {
@@ -494,8 +494,10 @@ function handleUserInfoUpdate(client, message)
   }
 
   // Tell the authorManager about the new attributes
-  authorManager.setAuthorColorId(author, message.data.userInfo.colorId);
-  authorManager.setAuthorName(author, message.data.userInfo.name);
+  const p = Promise.all([
+    authorManager.setAuthorColorId(author, message.data.userInfo.colorId),
+    authorManager.setAuthorName(author, message.data.userInfo.name),
+  ]);
 
   var padId = session.padId;
 
@@ -517,6 +519,9 @@ function handleUserInfoUpdate(client, message)
 
   // Send the other clients on the pad the update message
   client.broadcast.to(padId).json.send(infoMsg);
+
+  // Block until the authorManager has stored the new attributes.
+  await p;
 }
 
 /**
@@ -813,7 +818,7 @@ function _correctMarkersInPad(atext, apool) {
   return builder.toString();
 }
 
-function handleSwitchToPad(client, message)
+async function handleSwitchToPad(client, message)
 {
   // clear the session and leave the room
   const currentSessionInfo = sessioninfos[client.id];
@@ -830,7 +835,7 @@ function handleSwitchToPad(client, message)
   // start up the new pad
   const newSessionInfo = sessioninfos[client.id];
   createSessionInfoAuth(newSessionInfo, message);
-  handleClientReady(client, message);
+  await handleClientReady(client, message);
 }
 
 // Creates/replaces the auth object in the given session info.
diff --git a/src/node/handler/SocketIORouter.js b/src/node/handler/SocketIORouter.js
index a5220d2f4..23f3e459c 100644
--- a/src/node/handler/SocketIORouter.js
+++ b/src/node/handler/SocketIORouter.js
@@ -87,7 +87,7 @@ exports.setSocketIO = function(_socket) {
 
       if (clientAuthorized) {
         // client is authorized, everything ok
-        handleMessage(client, message);
+        await handleMessage(client, message);
       } else {
         // try to authorize the client
         if (message.padId !== undefined && message.sessionID !== undefined && message.token !== undefined && message.password !== undefined) {
@@ -104,7 +104,7 @@ exports.setSocketIO = function(_socket) {
           if (accessStatus === "grant") {
             // access was granted, mark the client as authorized and handle the message
             clientAuthorized = true;
-            handleMessage(client, message);
+            await handleMessage(client, message);
           } else {
             // no access, send the client a message that tells him why
             messageLogger.warn("Authentication try failed:" + stringifyWithoutPassword(message));
@@ -127,13 +127,13 @@ exports.setSocketIO = function(_socket) {
 }
 
 // try to handle the message of this client
-function handleMessage(client, message)
+async function handleMessage(client, message)
 {
   if (message.component && components[message.component]) {
     // check if component is registered in the components array
     if (components[message.component]) {
       messageLogger.debug("from " + client.id + ": " + stringifyWithoutPassword(message));
-      components[message.component].handleMessage(client, message);
+      await components[message.component].handleMessage(client, message);
     }
   } else {
     messageLogger.error("Can't route the message:" + stringifyWithoutPassword(message));

From 72ed1816ecae16e442a15f68fbbb1a73ccf1bb81 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Thu, 24 Sep 2020 13:59:07 -0400
Subject: [PATCH 12/12] security: Fix authz check for pad names with encoded
 characters

Also:
  * Minor test cleanups (`function` instead of arrow functions, etc.).
  * Add a test for a case that was previously not covered.
---
 src/node/hooks/express/webaccess.js |  7 ++-
 tests/backend/specs/socketio.js     | 92 ++++++++++++++++++++---------
 2 files changed, 69 insertions(+), 30 deletions(-)

diff --git a/src/node/hooks/express/webaccess.js b/src/node/hooks/express/webaccess.js
index fe5a40535..fd8c935e6 100644
--- a/src/node/hooks/express/webaccess.js
+++ b/src/node/hooks/express/webaccess.js
@@ -39,12 +39,13 @@ exports.checkAccess = (req, res, next) => {
       if (!level) return fail();
       const user = req.session.user;
       if (user == null) return next(); // This will happen if authentication is not required.
-      const padID = (req.path.match(/^\/p\/(.*)$/) || [])[1];
-      if (padID == null) return next();
+      const encodedPadId = (req.path.match(/^\/p\/(.*)$/) || [])[1];
+      if (encodedPadId == null) return next();
+      const padId = decodeURIComponent(encodedPadId);
       // The user was granted access to a pad. Remember the authorization level in the user's
       // settings so that SecurityManager can approve or deny specific actions.
       if (user.padAuthorizations == null) user.padAuthorizations = {};
-      user.padAuthorizations[padID] = level;
+      user.padAuthorizations[padId] = level;
       return next();
     };
 
diff --git a/tests/backend/specs/socketio.js b/tests/backend/specs/socketio.js
index 8dfd8aecc..b685e1f51 100644
--- a/tests/backend/specs/socketio.js
+++ b/tests/backend/specs/socketio.js
@@ -11,19 +11,19 @@ const settings = require(m('node/utils/Settings'));
 const supertest = require(m('node_modules/supertest'));
 
 const logger = log4js.getLogger('test');
-let client;
+let agent;
 let baseUrl;
 
-before(async () => {
+before(async function() {
   settings.port = 0;
   settings.ip = 'localhost';
   const httpServer = await server.start();
   baseUrl = `http://localhost:${httpServer.address().port}`;
   logger.debug(`HTTP server at ${baseUrl}`);
-  client = supertest(baseUrl);
+  agent = supertest(baseUrl);
 });
 
-after(async () => {
+after(async function() {
   await server.stop();
 });
 
@@ -107,10 +107,22 @@ const handshake = async (socket, padID) => {
   return msg;
 };
 
-describe('socket.io access checks', () => {
+describe('socket.io access checks', function() {
+  let authorize;
+  let authorizeHooksBackup;
+  const cleanUpPads = async () => {
+    const padIds = ['pad', 'other-pad', 'päd'];
+    await Promise.all(padIds.map(async (padId) => {
+      if (await padManager.doesPadExist(padId)) {
+        const pad = await padManager.getPad(padId);
+        await pad.remove();
+      }
+    }));
+  };
   const settingsBackup = {};
   let socket;
-  beforeEach(async () => {
+
+  beforeEach(async function() {
     Object.assign(settingsBackup, settings);
     assert(socket == null);
     settings.requireAuthentication = false;
@@ -119,67 +131,93 @@ describe('socket.io access checks', () => {
       admin: {password: 'admin-password', is_admin: true},
       user: {password: 'user-password'},
     };
-    Promise.all(['pad', 'other-pad'].map(async (pad) => {
-      if (await padManager.doesPadExist(pad)) (await padManager.getPad(pad)).remove();
-    }));
+    authorize = () => true;
+    authorizeHooksBackup = plugins.hooks.authorize;
+    plugins.hooks.authorize = [{hook_fn: (hookName, {req}, cb) => {
+      if (req.session.user == null) return cb([]); // Hasn't authenticated yet.
+      return cb([authorize(req)]);
+    }}];
+    await cleanUpPads();
   });
-  afterEach(async () => {
+  afterEach(async function() {
     Object.assign(settings, settingsBackup);
     if (socket) socket.close();
     socket = null;
+    plugins.hooks.authorize = authorizeHooksBackup;
+    await cleanUpPads();
   });
 
   // Normal accesses.
-  it('!authn anonymous cookie /p/pad -> 200, ok', async () => {
-    const res = await client.get('/p/pad').expect(200);
+  it('!authn anonymous cookie /p/pad -> 200, ok', async function() {
+    const res = await agent.get('/p/pad').expect(200);
     // Should not throw.
     socket = await connect(res);
     const clientVars = await handshake(socket, 'pad');
     assert.equal(clientVars.type, 'CLIENT_VARS');
   });
-  it('!authn !cookie -> ok', async () => {
+  it('!authn !cookie -> ok', async function() {
     // Should not throw.
     socket = await connect(null);
     const clientVars = await handshake(socket, 'pad');
     assert.equal(clientVars.type, 'CLIENT_VARS');
   });
-  it('!authn user /p/pad -> 200, ok', async () => {
-    const res = await client.get('/p/pad').auth('user', 'user-password').expect(200);
+  it('!authn user /p/pad -> 200, ok', async function() {
+    const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200);
     // Should not throw.
     socket = await connect(res);
     const clientVars = await handshake(socket, 'pad');
     assert.equal(clientVars.type, 'CLIENT_VARS');
   });
-  it('authn user /p/pad -> 200, ok', async () => {
+  it('authn user /p/pad -> 200, ok', async function() {
     settings.requireAuthentication = true;
-    const res = await client.get('/p/pad').auth('user', 'user-password').expect(200);
+    const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200);
     // Should not throw.
     socket = await connect(res);
     const clientVars = await handshake(socket, 'pad');
     assert.equal(clientVars.type, 'CLIENT_VARS');
   });
+  it('authz user /p/pad -> 200, ok', async function() {
+    settings.requireAuthentication = true;
+    settings.requireAuthorization = true;
+    const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200);
+    // Should not throw.
+    socket = await connect(res);
+    const clientVars = await handshake(socket, 'pad');
+    assert.equal(clientVars.type, 'CLIENT_VARS');
+  });
+  it('supports pad names with characters that must be percent-encoded', async function() {
+    settings.requireAuthentication = true;
+    // requireAuthorization is set to true here to guarantee that the user's padAuthorizations
+    // object is populated. Technically this isn't necessary because the user's padAuthorizations is
+    // currently populated even if requireAuthorization is false, but setting this to true ensures
+    // the test remains useful if the implementation ever changes.
+    settings.requireAuthorization = true;
+    const encodedPadId = encodeURIComponent('päd');
+    const res = await agent.get(`/p/${encodedPadId}`).auth('user', 'user-password').expect(200);
+    // Should not throw.
+    socket = await connect(res);
+    const clientVars = await handshake(socket, 'päd');
+    assert.equal(clientVars.type, 'CLIENT_VARS');
+  });
 
   // Abnormal access attempts.
-  it('authn anonymous /p/pad -> 401, error', async () => {
+  it('authn anonymous /p/pad -> 401, error', async function() {
     settings.requireAuthentication = true;
-    const res = await client.get('/p/pad').expect(401);
+    const res = await agent.get('/p/pad').expect(401);
     // Despite the 401, try to create the pad via a socket.io connection anyway.
     await assert.rejects(connect(res), {message: /authentication required/i});
   });
-  it('authn !cookie -> error', async () => {
+  it('authn !cookie -> error', async function() {
     settings.requireAuthentication = true;
     await assert.rejects(connect(null), {message: /signed express_sid cookie is required/i});
   });
-  it('authorization bypass attempt -> error', async () => {
-    plugins.hooks.authorize = [{hook_fn: (hookName, {req}, cb) => {
-      if (req.session.user == null) return cb([]); // Hasn't authenticated yet.
-      // Only allowed to access /p/pad.
-      return cb([req.path === '/p/pad']);
-    }}];
+  it('authorization bypass attempt -> error', async function() {
+    // Only allowed to access /p/pad.
+    authorize = (req) => req.path === '/p/pad';
     settings.requireAuthentication = true;
     settings.requireAuthorization = true;
     // First authenticate and establish a session.
-    const res = await client.get('/p/pad').auth('user', 'user-password').expect(200);
+    const res = await agent.get('/p/pad').auth('user', 'user-password').expect(200);
     // Connecting should work because the user successfully authenticated.
     socket = await connect(res);
     // Accessing /p/other-pad should fail, despite the successful fetch of /p/pad.