From 2bf076043f6c4d49fdc18f4e3163390aacb8d314 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 2 Sep 2020 17:16:02 -0400 Subject: [PATCH] Delete redundant token2author DB save See: https://github.com/ether/etherpad-lite/pull/4012#issuecomment-686005563 https://github.com/ether/etherpad-lite/issues/4006 --- src/node/db/AuthorManager.js | 11 ----------- src/node/db/SecurityManager.js | 8 +++++--- src/node/handler/PadMessageHandler.js | 23 ++++++++++------------- 3 files changed, 15 insertions(+), 27 deletions(-) diff --git a/src/node/db/AuthorManager.js b/src/node/db/AuthorManager.js index af867ec0d..a17952248 100644 --- a/src/node/db/AuthorManager.js +++ b/src/node/db/AuthorManager.js @@ -77,17 +77,6 @@ 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 d0fc6645a..5b790aba0 100644 --- a/src/node/db/SecurityManager.js +++ b/src/node/db/SecurityManager.js @@ -30,10 +30,12 @@ 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_lenght_20. The random string is generated by - * the client. + * @param token A random token representing the author, of the form t.randomstring_of_length_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 5db1ff955..69053ecaa 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/src/node/handler/PadMessageHandler.js @@ -36,6 +36,7 @@ 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'); @@ -914,10 +915,14 @@ async function handleClientReady(client, message) // Get ro/rw id:s let padIds = await readOnlyManager.getIds(message.padId); - // check permissions - - // Note: message.sessionID is an entierly different kind of - // session from the sessions we use here! Beware! + // 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! // FIXME: Call our "sessions" "connections". // FIXME: Use a hook instead // FIXME: Allow to override readwrite access with readonly @@ -933,19 +938,11 @@ 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);