diff --git a/bin/createUserSession.js b/bin/createUserSession.js index 5a9fb5ccf..0cba0d8e3 100644 --- a/bin/createUserSession.js +++ b/bin/createUserSession.js @@ -2,77 +2,46 @@ * A tool for generating a test user session which can be used for debugging configs * that require sessions. */ +const m = (f) => __dirname + '/../' + f; -const request = require('../src/node_modules/request'); -const settings = require(__dirname+'/../tests/container/loadSettings').loadSettings(); -const supertest = require(__dirname+'/../src/node_modules/supertest'); -const api = supertest('http://'+settings.ip+":"+settings.port); -const path = require('path'); const fs = require('fs'); +const path = require('path'); +const querystring = require('querystring'); +const request = require(m('src/node_modules/request')); +const settings = require(m('src/node/utils/Settings')); +const supertest = require(m('src/node_modules/supertest')); -// get the API Key -var filePath = path.join(__dirname, '../APIKEY.txt'); -var apikey = fs.readFileSync(filePath, {encoding: 'utf-8'}); +(async () => { + const api = supertest('http://'+settings.ip+':'+settings.port); -// Set apiVersion to base value, we change this later. -var apiVersion = 1; + const filePath = path.join(__dirname, '../APIKEY.txt'); + const apikey = fs.readFileSync(filePath, {encoding: 'utf-8'}); -// Update the apiVersion -api.get('/api/') - .expect(function(res){ - apiVersion = res.body.currentVersion; - if (!res.body.currentVersion) throw new Error("No version set in API"); - return; - }) - .end(function(err, res){ - // Now we know the latest API version, let's create a group - var uri = '/api/'+apiVersion+'/createGroup?apikey='+apikey; - api.post(uri) - .expect(function(res){ - if (res.body.code === 1){ - console.error("Error creating group", res.body); - }else{ - var groupID = res.body.data.groupID; - console.log("groupID", groupID); + let res; - // creating a group pad - api.post('/api/'+apiVersion+'/createGroupPad?apikey='+apikey+'&groupID='+groupID) - .expect(function(res){ - if (res.body.code === 1){ - console.error("Error creating author", res.body); - }else{ - console.log("Test Pad ID ====> ", res.body.data.padID) - } - }).end(function(){}) + res = await api.get('/api/'); + const apiVersion = res.body.currentVersion; + if (!apiVersion) throw new Error('No version set in API'); + const uri = (cmd, args) => `/api/${apiVersion}/${cmd}?${querystring.stringify(args)}`; - // create an author - api.post('/api/'+apiVersion+'/createAuthor?apikey='+apikey) - .expect(function(res){ - if (res.body.code === 1){ - console.error("Error creating author", res.body); - }else{ - console.log("authorID", res.body.data.authorID) - var authorID = res.body.data.authorID; - // create a session for this authorID - var validUntil = Math.floor(new Date() / 1000) + 60000; - console.log("validUntil", validUntil) - api.post('/api/'+apiVersion+'/createSession?apikey='+apikey + '&groupID='+groupID+'&authorID='+authorID+'&validUntil='+validUntil) - .expect(function(res){ - if (res.body.code === 1){ - console.error("Error creating author", res.body); - }else{ - console.log("Session made: ====> create a cookie named sessionID and set it's value to ", res.body.data.sessionID); - } - }) - .end(function(){}) // I shouldn't have nested but here we are.. it's not too ugly :P + res = await api.post(uri('createGroup', {apikey})); + if (res.body.code === 1) throw new Error(`Error creating group: ${res.body}`); + const groupID = res.body.data.groupID; + console.log('groupID', groupID); - } - }) - .end(function(){}) + res = await api.post(uri('createGroupPad', {apikey, groupID})); + if (res.body.code === 1) throw new Error(`Error creating group pad: ${res.body}`); + console.log('Test Pad ID ====> ', res.body.data.padID); - } - return; - }) - .end(function(){}) - }); -// end + res = await api.post(uri('createAuthor', {apikey})); + if (res.body.code === 1) throw new Error(`Error creating author: ${res.body}`); + const authorID = res.body.data.authorID; + console.log('authorID', authorID); + + const validUntil = Math.floor(new Date() / 1000) + 60000; + console.log('validUntil', validUntil); + res = await api.post(uri('createSession', {apikey, groupID, authorID, validUntil})); + if (res.body.code === 1) throw new Error(`Error creating session: ${res.body}`); + console.log('Session made: ====> create a cookie named sessionID and set the value to', + res.body.data.sessionID); +})(); 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 08df5ad0e..cb7830968 100644 --- a/src/node/db/SecurityManager.js +++ b/src/node/db/SecurityManager.js @@ -31,15 +31,25 @@ const WRONG_PASSWORD = Object.freeze({accessStatus: 'wrongPassword'}); const NEED_PASSWORD = Object.freeze({accessStatus: 'needPassword'}); /** - * 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. - * Used for every pad in the web UI. Not used for the HTTP API. - * @param password the password the user has given to access this pad, can be null - * @return {accessStatus: grant|deny|wrongPassword|needPassword, authorID: a.xxxxxx}) + * Determines whether the user can access a pad. + * + * @param padID identifies the pad the user wants to access. + * @param sessionCookie identifies the sessions the user created via the HTTP API, if any. + * Note: The term "session" used here is unrelated to express-session. + * @param token is a random token of the form t.randomstring_of_length_20 generated by the client + * when using the web UI (not the HTTP API). This token is only used if settings.requireSession + * is false and the user is accessing a public pad. If there is not an author already associated + * with this token then a new author object is created (including generating an author ID) and + * associated with this token. + * @param password is the password the user has given to access this pad. It can be null. + * @return {accessStatus: grant|deny|wrongPassword|needPassword, authorID: a.xxxxxx}. The caller + * must use the author ID returned in this object when making any changes associated with the + * author. + * + * WARNING: Tokens and session IDs MUST be kept secret, otherwise users will be able to impersonate + * each other (which might allow them to gain privileges). + * + * TODO: Add a hook so that plugins can make access decisions. */ exports.checkAccess = async function(padID, sessionCookie, token, password) { diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js index 14540820f..fc3b0a19a 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'); @@ -260,20 +261,11 @@ exports.handleMessage = async function(client, message) let dropMessage = await handleMessageHook(); if (!dropMessage) { - - // check permissions - if (message.type == "CLIENT_READY") { // client tried to auth for the first time (first msg from the client) createSessionInfo(client, message); } - // Note: message.sessionID is an entirely 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 - // the session may have been dropped during earlier processing if (!sessioninfos[client.id]) { messageLogger.warn("Dropping message from a connection that has gone away.") @@ -901,12 +893,6 @@ 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! - // FIXME: Call our "sessions" "connections". - // FIXME: Use a hook instead // FIXME: Allow to override readwrite access with readonly let statusObject = await securityManager.checkAccess(padIds.padId, message.sessionID, message.token, message.password); let accessStatus = statusObject.accessStatus; @@ -920,19 +906,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); diff --git a/src/node/hooks/express/importexport.js b/src/node/hooks/express/importexport.js index 9ddefe8a0..85ab6e129 100644 --- a/src/node/hooks/express/importexport.js +++ b/src/node/hooks/express/importexport.js @@ -1,3 +1,4 @@ +const assert = require('assert').strict; var hasPadAccess = require("../../padaccess"); var settings = require('../../utils/Settings'); var exportHandler = require('../../handler/ExportHandler'); @@ -5,6 +6,7 @@ var importHandler = require('../../handler/ImportHandler'); var padManager = require("../../db/PadManager"); var authorManager = require("../../db/AuthorManager"); const rateLimit = require("express-rate-limit"); +const securityManager = require("../../db/SecurityManager"); settings.importExportRateLimiting.onLimitReached = function(req, res, options) { // when the rate limiter triggers, write a warning in the logs @@ -51,57 +53,41 @@ exports.expressCreateServer = function (hook_name, args, cb) { // handle import requests args.app.use('/p/:pad/import', limiter); args.app.post('/p/:pad/import', async function(req, res, next) { - if (await hasPadAccess(req, res)) { - let exists = await padManager.doesPadExists(req.params.pad); - if (!exists) { - console.warn(`Someone tried to import into a pad that doesn't exist (${req.params.pad})`); - return next(); - } - - /* - * Starting from Etherpad 1.8.3 onwards, importing into a pad is allowed - * only if a user has his browser opened and connected to the pad (i.e. a - * Socket.IO session is estabilished for him) and he has already - * contributed to that specific pad. - * - * Note that this does not have anything to do with the "session", used - * for logging into "group pads". That kind of session is not needed here. - * - * This behaviour does not apply to API requests, only to /p/$PAD$/import - * - * See: https://github.com/ether/etherpad-lite/pull/3833#discussion_r407490205 - */ - if (!req.cookies && !settings.allowAnyoneToImport) { - console.warn(`Unable to import file into "${req.params.pad}". No cookies included in request`); - return next(); - } - - if (!req.cookies.token && !settings.allowAnyoneToImport) { - console.warn(`Unable to import file into "${req.params.pad}". No token in the cookies`); - return next(); - } - - let author = await authorManager.getAuthor4Token(req.cookies.token); - // author is of the form: "a.g2droBYw1prY7HW9" - if (!author && !settings.allowAnyoneToImport) { - console.warn(`Unable to import file into "${req.params.pad}". No Author found for token ${req.cookies.token}`); - - return next(); - } - - let authorsPads = await authorManager.listPadsOfAuthor(author); - if (!authorsPads && !settings.allowAnyoneToImport) { - console.warn(`Unable to import file into "${req.params.pad}". Author "${author}" exists but he never contributed to any pad`); - return next(); - } - - let authorsPadIDs = authorsPads.padIDs; - if ( (authorsPadIDs.indexOf(req.params.pad) === -1) && !settings.allowAnyoneToImport) { - console.warn(`Unable to import file into "${req.params.pad}". Author "${author}" exists but he never contributed to this pad`); - return next(); - } - - importHandler.doImport(req, res, req.params.pad); + if (!(await padManager.doesPadExists(req.params.pad))) { + console.warn(`Someone tried to import into a pad that doesn't exist (${req.params.pad})`); + return next(); } + + const {accessStatus, authorID} = await securityManager.checkAccess( + req.params.pad, req.cookies.sessionID, req.cookies.token, req.cookies.password); + if (accessStatus !== 'grant') return res.status(403).send('Forbidden'); + assert(authorID); + + /* + * Starting from Etherpad 1.8.3 onwards, importing into a pad is allowed + * only if a user has his browser opened and connected to the pad (i.e. a + * Socket.IO session is estabilished for him) and he has already + * contributed to that specific pad. + * + * Note that this does not have anything to do with the "session", used + * for logging into "group pads". That kind of session is not needed here. + * + * This behaviour does not apply to API requests, only to /p/$PAD$/import + * + * See: https://github.com/ether/etherpad-lite/pull/3833#discussion_r407490205 + */ + if (!settings.allowAnyoneToImport) { + const authorsPads = await authorManager.listPadsOfAuthor(authorID); + if (!authorsPads) { + console.warn(`Unable to import file into "${req.params.pad}". Author "${authorID}" exists but he never contributed to any pad`); + return next(); + } + if (authorsPads.padIDs.indexOf(req.params.pad) === -1) { + console.warn(`Unable to import file into "${req.params.pad}". Author "${authorID}" exists but he never contributed to this pad`); + return next(); + } + } + + importHandler.doImport(req, res, req.params.pad); }); }