Skip to content

Commit

Permalink
Delete redundant token2author DB save
Browse files Browse the repository at this point in the history
  • Loading branch information
rhansen authored and JohnMcLear committed Sep 5, 2020
1 parent 9962be2 commit 2bf0760
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 27 deletions.
11 changes: 0 additions & 11 deletions src/node/db/AuthorManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions src/node/db/SecurityManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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})
*/
Expand Down
23 changes: 10 additions & 13 deletions src/node/handler/PadMessageHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand Down

0 comments on commit 2bf0760

Please sign in to comment.