From c3b2e68dad5939f661b64bbb1078a651f897a5de Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 7 Sep 2020 12:20:57 -0400 Subject: [PATCH] Revert "Delete redundant token2author DB save" Something's weird here; this change shouldn't have any effect. I'll have to squint at the code some more. This reverts commit 2bf076043f6c4d49fdc18f4e3163390aacb8d314. Fixes #4262 --- src/node/db/AuthorManager.js | 11 +++++++++++ src/node/db/SecurityManager.js | 8 +++----- src/node/handler/PadMessageHandler.js | 23 +++++++++++++---------- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/node/db/AuthorManager.js b/src/node/db/AuthorManager.js index a17952248..af867ec0d 100644 --- a/src/node/db/AuthorManager.js +++ b/src/node/db/AuthorManager.js @@ -77,6 +77,17 @@ exports.createAuthorIfNotExistsFor = async function(authorMapper, name) return author; }; +/** + * Sets the token <> AuthorID relationship. + * Discussion at https://github.com/ether/etherpad-lite/issues/4006 + * @param {String} token The token (generated by a client) + * @param {String} authorID The authorID (returned by the Security Manager) + */ +exports.setToken2Author = async function(token, authorID) +{ + await db.set("token2author:"+token, authorID); +} + /** * Returns the AuthorID for a mapper. We can map using a mapperkey, * so far this is token2author and mapper2author diff --git a/src/node/db/SecurityManager.js b/src/node/db/SecurityManager.js index c94ca09ef..0ca67669f 100644 --- a/src/node/db/SecurityManager.js +++ b/src/node/db/SecurityManager.js @@ -30,12 +30,10 @@ var authLogger = log4js.getLogger("auth"); * This function controlls the access to a pad, it checks if the user can access a pad. * @param padID the pad the user wants to access * @param sessionCookie the session the user has (set via api) - * @param token A random token representing the author, of the form t.randomstring_of_length_20. - * The random string is generated by the client. + * @param token a random token representing the author, of the form + * t.randomstring_of_lenght_20. The random string is generated by + * the client. * Used for every pad in the web UI. Not used for the HTTP API. - * If there is not already an author associated with this token, and access is not - * denied, an author object will be created (including generating an author ID) and - * saved in the DB. * @param password the password the user has given to access this pad, can be null * @return {accessStatus: grant|deny|wrongPassword|needPassword, authorID: a.xxxxxx}) */ diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js index a1557217d..af5ba4d84 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/src/node/handler/PadMessageHandler.js @@ -36,7 +36,6 @@ var hooks = require("ep_etherpad-lite/static/js/pluginfw/hooks.js"); var channels = require("channels"); var stats = require('../stats'); var remoteAddress = require("../utils/RemoteAddress").remoteAddress; -const assert = require('assert').strict; const nodeify = require("nodeify"); const { RateLimiterMemory } = require('rate-limiter-flexible'); @@ -902,14 +901,10 @@ async function handleClientReady(client, message) // Get ro/rw id:s let padIds = await readOnlyManager.getIds(message.padId); - // Check permissions. Notes: - // * If there is not already an author associated with the client-generated token, and access is - // not denied, checkAccess will create an author object (including generating an author ID) - // and save it in the DB. - // * Tokens must be kept secret, otherwise users will able to impersonate each other (which - // might allow them to gain privileges). - // * message.sessionID is an entierly different kind of session from the sessions we use here! - // Beware! + // check permissions + + // Note: message.sessionID is an entierly different kind of + // session from the sessions we use here! Beware! // FIXME: Call our "sessions" "connections". // FIXME: Use a hook instead // FIXME: Allow to override readwrite access with readonly @@ -925,11 +920,19 @@ async function handleClientReady(client, message) let author = statusObject.authorID; // get all authordata of this new user - assert(author); let value = await authorManager.getAuthor(author); let authorColorId = value.colorId; let authorName = value.name; + /* + * Here we know authorID, token and session. We should ?always? store it.. + * TODO: I fear that this might allow a user to pass a token for an authorID + * meaning that they could in theory "imitate" another author? + * Perhaps the fix to this is check to see if it exists first and if it + * does then abort.. Details: https://github.com/ether/etherpad-lite/issues/4006 + */ + await authorManager.setToken2Author(message.token, statusObject.authorID) + // load the pad-object from the database let pad = await padManager.getPad(padIds.padId);