diff --git a/src/locales/pl.json b/src/locales/pl.json index 77d4fae4b..68f293708 100644 --- a/src/locales/pl.json +++ b/src/locales/pl.json @@ -4,6 +4,7 @@ "DeRudySoulStorm", "Macofe", "Mateon1", + "Matlin", "Pan Cube", "Rezonansowy", "Teeed", @@ -51,6 +52,7 @@ "pad.settings.fontType.normal": "Normalna", "pad.settings.language": "Język:", "pad.settings.about": "O aplikacji", + "pad.settings.poweredBy": "Dostarczane przez $1", "pad.importExport.import_export": "Import/eksport", "pad.importExport.import": "Prześlij dowolny plik tekstowy lub dokument", "pad.importExport.importSuccessful": "Sukces!", diff --git a/src/node/db/SecurityManager.js b/src/node/db/SecurityManager.js index cb7830968..f4c7af880 100644 --- a/src/node/db/SecurityManager.js +++ b/src/node/db/SecurityManager.js @@ -42,240 +42,91 @@ const NEED_PASSWORD = Object.freeze({accessStatus: 'needPassword'}); * 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. + * @param userSettings is the settings.users[username] object (or equivalent from an authn plugin). * @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) +exports.checkAccess = async function(padID, sessionCookie, token, password, userSettings) { if (!padID) { + authLogger.debug('access denied: missing padID'); + return DENY; + } + + // Make sure the user has authenticated if authentication is required. The caller should have + // already performed this check, but it is repeated here just in case. + if (settings.requireAuthentication && userSettings == null) { + authLogger.debug('access denied: authentication is required'); return DENY; } // allow plugins to deny access - var deniedByHook = hooks.callAll("onAccessCheck", {'padID': padID, 'password': password, 'token': token, 'sessionCookie': sessionCookie}).indexOf(false) > -1; - if (deniedByHook) { + const isFalse = (x) => x === false; + if (hooks.callAll('onAccessCheck', {padID, password, token, sessionCookie}).some(isFalse)) { + authLogger.debug('access denied: an onAccessCheck hook function returned false'); return DENY; } - // start to get author for this token - let p_tokenAuthor = authorManager.getAuthor4Token(token); + // start fetching the info we may need + const p_sessionAuthorID = sessionManager.findAuthorID(padID.split('$')[0], sessionCookie); + const p_tokenAuthorID = authorManager.getAuthor4Token(token); + const p_padExists = padManager.doesPadExist(padID); - // start to check if pad exists - let p_padExists = padManager.doesPadExist(padID); + const padExists = await p_padExists; + if (!padExists && settings.editOnly) { + authLogger.debug('access denied: user attempted to create a pad, which is prohibited'); + return DENY; + } - if (settings.requireSession) { - // a valid session is required (api-only mode) - if (!sessionCookie) { - // without sessionCookie, access is denied + const sessionAuthorID = await p_sessionAuthorID; + if (settings.requireSession && !sessionAuthorID) { + authLogger.debug('access denied: HTTP API session is required'); + return DENY; + } + + const grant = { + accessStatus: 'grant', + authorID: (sessionAuthorID != null) ? sessionAuthorID : await p_tokenAuthorID, + }; + + if (!padID.includes('$')) { + // Only group pads can be private or have passwords, so there is nothing more to check for this + // non-group pad. + return grant; + } + + if (!padExists) { + if (sessionAuthorID == null) { + authLogger.debug('access denied: must have an HTTP API session to create a group pad'); return DENY; } - } else { - // a session is not required, so we'll check if it's a public pad - if (padID.indexOf("$") === -1) { - // it's not a group pad, means we can grant access - if (settings.editOnly && !(await p_padExists)) return DENY; - return {accessStatus: 'grant', authorID: await p_tokenAuthor}; - } + // Creating a group pad, so there is no password or public status to check. + return grant; } - let validSession = false; - let sessionAuthor; - let isPublic; - let isPasswordProtected; - let passwordStatus = password == null ? "notGiven" : "wrong"; // notGiven, correct, wrong + const pad = await padManager.getPad(padID); - // get information about all sessions contained in this cookie - if (sessionCookie) { - let groupID = padID.split("$")[0]; - - /* - * Sometimes, RFC 6265-compliant web servers may send back a cookie whose - * value is enclosed in double quotes, such as: - * - * Set-Cookie: sessionCookie="s.37cf5299fbf981e14121fba3a588c02b,s.2b21517bf50729d8130ab85736a11346"; Version=1; Path=/; Domain=localhost; Discard - * - * Where the double quotes at the start and the end of the header value are - * just delimiters. This is perfectly legal: Etherpad parsing logic should - * cope with that, and remove the quotes early in the request phase. - * - * Somehow, this does not happen, and in such cases the actual value that - * sessionCookie ends up having is: - * - * sessionCookie = '"s.37cf5299fbf981e14121fba3a588c02b,s.2b21517bf50729d8130ab85736a11346"' - * - * As quick measure, let's strip the double quotes (when present). - * Note that here we are being minimal, limiting ourselves to just removing - * quotes at the start and the end of the string. - * - * Fixes #3819. - * Also, see #3820. - */ - let sessionIDs = sessionCookie.replace(/^"|"$/g, '').split(','); - - // was previously iterated in parallel using async.forEach - try { - let sessionInfos = await Promise.all(sessionIDs.map(sessionID => { - return sessionManager.getSessionInfo(sessionID); - })); - - // seperated out the iteration of sessioninfos from the (parallel) fetches from the DB - for (let sessionInfo of sessionInfos) { - // is it for this group? - if (sessionInfo.groupID != groupID) { - authLogger.debug("Auth failed: wrong group"); - continue; - } - - // is validUntil still ok? - let now = Math.floor(Date.now() / 1000); - if (sessionInfo.validUntil <= now) { - authLogger.debug("Auth failed: validUntil"); - continue; - } - - // fall-through - there is a valid session - validSession = true; - sessionAuthor = sessionInfo.authorID; - break; - } - } catch (err) { - // skip session if it doesn't exist - if (err.message == "sessionID does not exist") { - authLogger.debug("Auth failed: unknown session"); - } else { - throw err; - } - } + if (!pad.getPublicStatus() && sessionAuthorID == null) { + authLogger.debug('access denied: must have an HTTP API session to access private group pads'); + return DENY; } - let padExists = await p_padExists; - - if (padExists) { - let pad = await padManager.getPad(padID); - - // is it a public pad? - isPublic = pad.getPublicStatus(); - - // is it password protected? - isPasswordProtected = pad.isPasswordProtected(); - - // is password correct? - if (isPasswordProtected && password && pad.isCorrectPassword(password)) { - passwordStatus = "correct"; - } - } - - // - a valid session for this group is avaible AND pad exists - if (validSession && padExists) { - let authorID = sessionAuthor; - let grant = Object.freeze({ accessStatus: "grant", authorID }); - - if (!isPasswordProtected) { - // - the pad is not password protected - - // --> grant access - return grant; - } - - if (settings.sessionNoPassword) { - // - the setting to bypass password validation is set - - // --> grant access - return grant; - } - - if (isPasswordProtected && passwordStatus === "correct") { - // - the pad is password protected and password is correct - - // --> grant access - return grant; - } - - if (isPasswordProtected && passwordStatus === "wrong") { - // - the pad is password protected but wrong password given - - // --> deny access, ask for new password and tell them that the password is wrong - return WRONG_PASSWORD; - } - - if (isPasswordProtected && passwordStatus === "notGiven") { - // - the pad is password protected but no password given - - // --> ask for password + const passwordExempt = settings.sessionNoPassword && sesionAuthorID != null; + const requirePassword = pad.isPasswordProtected() && !passwordExempt; + if (requirePassword) { + if (password == null) { + authLogger.debug('access denied: password required'); return NEED_PASSWORD; } - - throw new Error("Oops, something wrong happend"); - } - - if (validSession && !padExists) { - // - a valid session for this group avaible but pad doesn't exist - - // --> grant access by default - let accessStatus = "grant"; - let authorID = sessionAuthor; - - // --> deny access if user isn't allowed to create the pad - if (settings.editOnly) { - authLogger.debug("Auth failed: valid session & pad does not exist"); - return DENY; - } - - return { accessStatus, authorID }; - } - - if (!validSession && padExists) { - // there is no valid session avaiable AND pad exists - - let authorID = await p_tokenAuthor; - let grant = Object.freeze({ accessStatus: "grant", authorID }); - - if (isPublic && !isPasswordProtected) { - // -- it's public and not password protected - - // --> grant access, with author of token - return grant; - } - - if (isPublic && isPasswordProtected && passwordStatus === "correct") { - // - it's public and password protected and password is correct - - // --> grant access, with author of token - return grant; - } - - if (isPublic && isPasswordProtected && passwordStatus === "wrong") { - // - it's public and the pad is password protected but wrong password given - - // --> deny access, ask for new password and tell them that the password is wrong + if (!password || !pad.isCorrectPassword(password)) { + authLogger.debug('access denied: wrong password'); return WRONG_PASSWORD; } - - if (isPublic && isPasswordProtected && passwordStatus === "notGiven") { - // - it's public and the pad is password protected but no password given - - // --> ask for password - return NEED_PASSWORD; - } - - if (!isPublic) { - // - it's not public - - authLogger.debug("Auth failed: invalid session & pad is not public"); - // --> deny access - return DENY; - } - - throw new Error("Oops, something wrong happend"); } - // there is no valid session avaiable AND pad doesn't exist - authLogger.debug("Auth failed: invalid session & pad does not exist"); - return DENY; -} + return grant; +}; diff --git a/src/node/db/SessionManager.js b/src/node/db/SessionManager.js index 9161205d7..5f7df1e24 100644 --- a/src/node/db/SessionManager.js +++ b/src/node/db/SessionManager.js @@ -19,11 +19,65 @@ */ var customError = require("../utils/customError"); +const promises = require('../utils/promises'); var randomString = require("../utils/randomstring"); var db = require("./DB"); var groupManager = require("./GroupManager"); var authorManager = require("./AuthorManager"); +/** + * Finds the author ID for a session with matching ID and group. + * + * @param groupID identifies the group the session is bound to. + * @param sessionCookie contains a comma-separated list of IDs identifying the sessions to search. + * @return If there is a session that is not expired, has an ID matching one of the session IDs in + * sessionCookie, and is bound to a group with the given ID, then this returns the author ID + * bound to the session. Otherwise, returns undefined. + */ +exports.findAuthorID = async (groupID, sessionCookie) => { + if (!sessionCookie) return undefined; + /* + * Sometimes, RFC 6265-compliant web servers may send back a cookie whose + * value is enclosed in double quotes, such as: + * + * Set-Cookie: sessionCookie="s.37cf5299fbf981e14121fba3a588c02b,s.2b21517bf50729d8130ab85736a11346"; Version=1; Path=/; Domain=localhost; Discard + * + * Where the double quotes at the start and the end of the header value are + * just delimiters. This is perfectly legal: Etherpad parsing logic should + * cope with that, and remove the quotes early in the request phase. + * + * Somehow, this does not happen, and in such cases the actual value that + * sessionCookie ends up having is: + * + * sessionCookie = '"s.37cf5299fbf981e14121fba3a588c02b,s.2b21517bf50729d8130ab85736a11346"' + * + * As quick measure, let's strip the double quotes (when present). + * Note that here we are being minimal, limiting ourselves to just removing + * quotes at the start and the end of the string. + * + * Fixes #3819. + * Also, see #3820. + */ + const sessionIDs = sessionCookie.replace(/^"|"$/g, '').split(','); + const sessionInfoPromises = sessionIDs.map(async (id) => { + try { + return await exports.getSessionInfo(id); + } catch (err) { + if (err.message === 'sessionID does not exist') { + console.debug(`SessionManager getAuthorID: no session exists with ID ${id}`); + } else { + throw err; + } + } + return undefined; + }); + const now = Math.floor(Date.now() / 1000); + const isMatch = (si) => (si != null && si.groupID === groupID && si.validUntil <= now); + const sessionInfo = await promises.firstSatisfies(sessionInfoPromises, isMatch); + if (sessionInfo == null) return undefined; + return sessionInfo.authorID; +}; + exports.doesSessionExist = async function(sessionID) { //check if the database entry of this session exists diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js index fc3b0a19a..624db6829 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/src/node/handler/PadMessageHandler.js @@ -106,7 +106,7 @@ exports.kickSessionsFromPad = function(padID) return; // skip if there is nobody on this pad - if(_getRoomClients(padID).length == 0) + if(_getRoomClients(padID).length === 0) return; // disconnect everyone from this pad @@ -227,32 +227,32 @@ exports.handleMessage = async function(client, message) function finalHandler() { // Check what type of message we get and delegate to the other methods - if (message.type == "CLIENT_READY") { + if (message.type === "CLIENT_READY") { handleClientReady(client, message); - } else if (message.type == "CHANGESET_REQ") { + } else if (message.type === "CHANGESET_REQ") { handleChangesetRequest(client, message); - } else if(message.type == "COLLABROOM") { + } else if(message.type === "COLLABROOM") { if (thisSession.readonly) { messageLogger.warn("Dropped message, COLLABROOM for readonly pad"); - } else if (message.data.type == "USER_CHANGES") { + } else if (message.data.type === "USER_CHANGES") { stats.counter('pendingEdits').inc() padChannels.emit(message.padId, {client: client, message: message}); // add to pad queue - } else if (message.data.type == "USERINFO_UPDATE") { + } else if (message.data.type === "USERINFO_UPDATE") { handleUserInfoUpdate(client, message); - } else if (message.data.type == "CHAT_MESSAGE") { + } else if (message.data.type === "CHAT_MESSAGE") { handleChatMessage(client, message); - } else if (message.data.type == "GET_CHAT_MESSAGES") { + } else if (message.data.type === "GET_CHAT_MESSAGES") { handleGetChatMessages(client, message); - } else if (message.data.type == "SAVE_REVISION") { + } else if (message.data.type === "SAVE_REVISION") { handleSaveRevisionMessage(client, message); - } else if (message.data.type == "CLIENT_MESSAGE" && + } else if (message.data.type === "CLIENT_MESSAGE" && message.data.payload != null && - message.data.payload.type == "suggestUserName") { + message.data.payload.type === "suggestUserName") { handleSuggestUserName(client, message); } else { messageLogger.warn("Dropped message, unknown COLLABROOM Data Type " + message.data.type); } - } else if(message.type == "SWITCH_TO_PAD") { + } else if(message.type === "SWITCH_TO_PAD") { handleSwitchToPad(client, message); } else { messageLogger.warn("Dropped message, unknown Message Type " + message.type); @@ -260,45 +260,47 @@ exports.handleMessage = async function(client, message) } let dropMessage = await handleMessageHook(); - if (!dropMessage) { - if (message.type == "CLIENT_READY") { - // client tried to auth for the first time (first msg from the client) - createSessionInfo(client, message); - } + if (dropMessage) return; - // the session may have been dropped during earlier processing - if (!sessioninfos[client.id]) { - messageLogger.warn("Dropping message from a connection that has gone away.") - return; - } - - // Simulate using the load testing tool - if (!sessioninfos[client.id].auth) { - console.error("Auth was never applied to a session. If you are using the stress-test tool then restart Etherpad and the Stress test tool.") - return; - } - - let auth = sessioninfos[client.id].auth; - - // check if pad is requested via readOnly - let padId = auth.padID; - - if (padId.indexOf("r.") === 0) { - // Pad is readOnly, first get the real Pad ID - padId = await readOnlyManager.getPadId(padId); - } - - let { accessStatus } = await securityManager.checkAccess(padId, auth.sessionID, auth.token, auth.password); - - if (accessStatus !== "grant") { - // no access, send the client a message that tells him why - client.json.send({ accessStatus }); - return; - } - - // access was granted - finalHandler(); + if (message.type === "CLIENT_READY") { + // client tried to auth for the first time (first msg from the client) + createSessionInfoAuth(client, message); } + + // the session may have been dropped during earlier processing + if (!sessioninfos[client.id]) { + messageLogger.warn("Dropping message from a connection that has gone away.") + return; + } + + // Simulate using the load testing tool + if (!sessioninfos[client.id].auth) { + console.error("Auth was never applied to a session. If you are using the stress-test tool then restart Etherpad and the Stress test tool.") + return; + } + + let auth = sessioninfos[client.id].auth; + + // check if pad is requested via readOnly + let padId = auth.padID; + + if (padId.indexOf("r.") === 0) { + // Pad is readOnly, first get the real Pad ID + padId = await readOnlyManager.getPadId(padId); + } + + const {session: {user} = {}} = client.client.request; + const {accessStatus} = + await securityManager.checkAccess(padId, auth.sessionID, auth.token, auth.password, user); + + if (accessStatus !== "grant") { + // no access, send the client a message that tells him why + client.json.send({ accessStatus }); + return; + } + + // access was granted + finalHandler(); } @@ -461,7 +463,7 @@ function handleSuggestUserName(client, message) // search the author and send him this message roomClients.forEach(function(client) { var session = sessioninfos[client.id]; - if (session && session.author == message.data.payload.unnamedId) { + if (session && session.author === message.data.payload.unnamedId) { client.json.send(message); } }); @@ -621,7 +623,7 @@ async function handleUserChanges(data) if (!attr) return; // the empty author is used in the clearAuthorship functionality so this should be the only exception - if ('author' == attr[0] && (attr[1] != thisSession.author && attr[1] != '')) { + if ('author' === attr[0] && (attr[1] !== thisSession.author && attr[1] !== '')) { throw new Error("Trying to submit changes as another author in changeset " + changeset); } }); @@ -660,7 +662,7 @@ async function handleUserChanges(data) // a changeset can be based on an old revision with the same changes in it // prevent eplite from accepting it TODO: better send the client a NEW_CHANGES // of that revision - if (baseRev + 1 == r && c == changeset) { + if (baseRev + 1 === r && c === changeset) { client.json.send({disconnect:"badChangeset"}); stats.meter('failedChangesets').mark(); throw new Error("Won't apply USER_CHANGES, because it contains an already accepted changeset"); @@ -676,7 +678,7 @@ async function handleUserChanges(data) let prevText = pad.text(); - if (Changeset.oldLen(changeset) != prevText.length) { + if (Changeset.oldLen(changeset) !== prevText.length) { client.json.send({disconnect:"badChangeset"}); stats.meter('failedChangesets').mark(); throw new Error("Can't apply USER_CHANGES "+changeset+" with oldLen " + Changeset.oldLen(changeset) + " to document of length " + prevText.length); @@ -696,7 +698,7 @@ async function handleUserChanges(data) } // Make sure the pad always ends with an empty line. - if (pad.text().lastIndexOf("\n") != pad.text().length-1) { + if (pad.text().lastIndexOf("\n") !== pad.text().length-1) { var nlChangeset = Changeset.makeSplice(pad.text(), pad.text().length - 1, 0, "\n"); pad.appendRevision(nlChangeset); } @@ -714,7 +716,7 @@ exports.updatePadClients = async function(pad) // skip this if no-one is on this pad let roomClients = _getRoomClients(pad.id); - if (roomClients.length == 0) { + if (roomClients.length === 0) { return; } @@ -749,7 +751,7 @@ exports.updatePadClients = async function(pad) continue; } - if (author == sessioninfos[sid].author) { + if (author === sessioninfos[sid].author) { client.json.send({ "type": "COLLABROOM", "data":{ type: "ACCEPT_COMMIT", newRev: r }}); } else { let forWire = Changeset.prepareForWire(revChangeset, pad.pool); @@ -794,7 +796,7 @@ function _correctMarkersInPad(atext, apool) { if (hasMarker) { for (var i = 0; i < op.chars; i++) { - if (offset > 0 && text.charAt(offset-1) != '\n') { + if (offset > 0 && text.charAt(offset-1) !== '\n') { badMarkers.push(offset); } offset++; @@ -804,7 +806,7 @@ function _correctMarkersInPad(atext, apool) { } } - if (badMarkers.length == 0) { + if (badMarkers.length === 0) { return null; } @@ -831,7 +833,7 @@ function handleSwitchToPad(client, message) roomClients.forEach(client => { let sinfo = sessioninfos[client.id]; - if (sinfo && sinfo.author == currentSession.author) { + if (sinfo && sinfo.author === currentSession.author) { // fix user's counter, works on page refresh or if user closes browser window and then rejoins sessioninfos[client.id] = {}; client.leave(padId); @@ -839,11 +841,13 @@ function handleSwitchToPad(client, message) }); // start up the new pad - createSessionInfo(client, message); + createSessionInfoAuth(client, message); handleClientReady(client, message); } -function createSessionInfo(client, message) +// Creates/replaces the auth object in the client's session info. Session info for the client must +// already exist. +function createSessionInfoAuth(client, message) { // Remember this information since we won't // have the cookie in further socket.io messages. @@ -883,7 +887,7 @@ async function handleClientReady(client, message) return; } - if (message.protocolVersion != 2) { + if (message.protocolVersion !== 2) { messageLogger.warn("Dropped message, CLIENT_READY Message has a unknown protocolVersion '" + message.protocolVersion + "'!"); return; } @@ -894,8 +898,9 @@ async function handleClientReady(client, message) let padIds = await readOnlyManager.getIds(message.padId); // 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; + const {session: {user} = {}} = client.client.request; + const {accessStatus, authorID} = await securityManager.checkAccess( + padIds.padId, message.sessionID, message.token, message.password, user); // no access, send the client a message that tells him why if (accessStatus !== "grant") { @@ -903,11 +908,9 @@ async function handleClientReady(client, message) return; } - let author = statusObject.authorID; - // get all authordata of this new user - assert(author); - let value = await authorManager.getAuthor(author); + assert(authorID); + let value = await authorManager.getAuthor(authorID); let authorColorId = value.colorId; let authorName = value.name; @@ -933,7 +936,7 @@ async function handleClientReady(client, message) })); let thisUserHasEditedThisPad = false; - if (historicalAuthorData[statusObject.authorID]) { + if (historicalAuthorData[authorID]) { /* * This flag is set to true when a user contributes to a specific pad for * the first time. It is used for deciding if importing to that pad is @@ -954,7 +957,7 @@ async function handleClientReady(client, message) for (let client of roomClients) { let sinfo = sessioninfos[client.id]; - if (sinfo && sinfo.author == author) { + if (sinfo && sinfo.author === authorID) { // fix user's counter, works on page refresh or if user closes browser window and then rejoins sessioninfos[client.id] = {}; client.leave(padIds.padId); @@ -977,7 +980,7 @@ async function handleClientReady(client, message) if (pad.head > 0) { accessLogger.info('[ENTER] Pad "' + padIds.padId + '": Client ' + client.id + ' with IP "' + ip + '" entered the pad'); - } else if (pad.head == 0) { + } else if (pad.head === 0) { accessLogger.info('[CREATE] Pad "' + padIds.padId + '": Client ' + client.id + ' with IP "' + ip + '" created the pad'); } @@ -1038,7 +1041,7 @@ async function handleClientReady(client, message) client.json.send(wireMsg); } - if (startNum == endNum) { + if (startNum === endNum) { var Msg = {"type":"COLLABROOM", "data":{type:"CLIENT_RECONNECT", noChanges: true, @@ -1104,7 +1107,7 @@ async function handleClientReady(client, message) "readOnlyId": padIds.readOnlyPadId, "readonly": padIds.readonly, "serverTimestamp": Date.now(), - "userId": author, + "userId": authorID, "abiwordAvailable": settings.abiwordAvailable(), "sofficeAvailable": settings.sofficeAvailable(), "exportAvailable": settings.exportAvailable(), @@ -1149,7 +1152,7 @@ async function handleClientReady(client, message) // Save the current revision in sessioninfos, should be the same as in clientVars sessioninfos[client.id].rev = pad.getHeadRevisionNumber(); - sessioninfos[client.id].author = author; + sessioninfos[client.id].author = authorID; // prepare the notification for the other users on the pad, that this user joined let messageToTheOtherUsers = { @@ -1160,7 +1163,7 @@ async function handleClientReady(client, message) "ip": "127.0.0.1", "colorId": authorColorId, "userAgent": "Anonymous", - "userId": author + "userId": authorID, } } }; @@ -1178,7 +1181,7 @@ async function handleClientReady(client, message) await Promise.all(_getRoomClients(pad.id).map(async roomClient => { // Jump over, if this session is the connection session - if (roomClient.id == client.id) { + if (roomClient.id === client.id) { return; } @@ -1193,48 +1196,46 @@ async function handleClientReady(client, message) let cached = historicalAuthorData[author]; // reuse previously created cache of author's data - let p = cached ? Promise.resolve(cached) : authorManager.getAuthor(author); + const authorInfo = await (cached ? Promise.resolve(cached) : authorManager.getAuthor(author)); - return p.then(authorInfo => { - // default fallback color to use if authorInfo.colorId is null - const defaultColor = "#daf0b2"; + // default fallback color to use if authorInfo.colorId is null + const defaultColor = "#daf0b2"; - if (!authorInfo) { - console.warn(`handleClientReady(): no authorInfo parameter was received. Default values are going to be used. See issue #3612. This can be caused by a user clicking undo after clearing all authorship colors see #2802`); - authorInfo = {}; - } + if (!authorInfo) { + console.warn(`handleClientReady(): no authorInfo parameter was received. Default values are going to be used. See issue #3612. This can be caused by a user clicking undo after clearing all authorship colors see #2802`); + authorInfo = {}; + } - // For some reason sometimes name isn't set - // Catch this issue here and use a fixed name. - if (!authorInfo.name) { - console.warn(`handleClientReady(): client submitted no author name. Using "Anonymous". See: issue #3612`); - authorInfo.name = "Anonymous"; - } + // For some reason sometimes name isn't set + // Catch this issue here and use a fixed name. + if (!authorInfo.name) { + console.warn(`handleClientReady(): client submitted no author name. Using "Anonymous". See: issue #3612`); + authorInfo.name = "Anonymous"; + } - // For some reason sometimes colorId isn't set - // Catch this issue here and use a fixed color. - if (!authorInfo.colorId) { - console.warn(`handleClientReady(): author "${authorInfo.name}" has no property colorId. Using the default color ${defaultColor}. See issue #3612`); - authorInfo.colorId = defaultColor; - } + // For some reason sometimes colorId isn't set + // Catch this issue here and use a fixed color. + if (!authorInfo.colorId) { + console.warn(`handleClientReady(): author "${authorInfo.name}" has no property colorId. Using the default color ${defaultColor}. See issue #3612`); + authorInfo.colorId = defaultColor; + } - // Send the new User a Notification about this other user - let msg = { - "type": "COLLABROOM", - "data": { - type: "USER_NEWINFO", - userInfo: { - "ip": "127.0.0.1", - "colorId": authorInfo.colorId, - "name": authorInfo.name, - "userAgent": "Anonymous", - "userId": author - } + // Send the new User a Notification about this other user + let msg = { + "type": "COLLABROOM", + "data": { + type: "USER_NEWINFO", + userInfo: { + "ip": "127.0.0.1", + "colorId": authorInfo.colorId, + "name": authorInfo.name, + "userAgent": "Anonymous", + "userId": author } - }; + } + }; - client.json.send(msg); - }); + client.json.send(msg); })); } } @@ -1318,7 +1319,7 @@ async function getChangesetInfo(padId, startNum, endNum, granularity) compositesChangesetNeeded.push({ start, end }); // add the t1 time we need - revTimesNeeded.push(start == 0 ? 0 : start - 1); + revTimesNeeded.push(start === 0 ? 0 : start - 1); // add the t2 time we need revTimesNeeded.push(end - 1); @@ -1373,7 +1374,7 @@ async function getChangesetInfo(padId, startNum, endNum, granularity) let forwards2 = Changeset.moveOpsToNewPool(forwards, pad.apool(), apool); let backwards2 = Changeset.moveOpsToNewPool(backwards, pad.apool(), apool); - let t1 = (compositeStart == 0) ? revisionDate[0] : revisionDate[compositeStart - 1]; + let t1 = (compositeStart === 0) ? revisionDate[0] : revisionDate[compositeStart - 1]; let t2 = revisionDate[compositeEnd - 1]; timeDeltas.push(t2 - t1); diff --git a/src/node/handler/SocketIORouter.js b/src/node/handler/SocketIORouter.js index 077a62beb..a5220d2f4 100644 --- a/src/node/handler/SocketIORouter.js +++ b/src/node/handler/SocketIORouter.js @@ -97,7 +97,9 @@ exports.setSocketIO = function(_socket) { padId = await readOnlyManager.getPadId(message.padId); } - let { accessStatus } = await securityManager.checkAccess(padId, message.sessionID, message.token, message.password); + const {session: {user} = {}} = client.client.request; + const {accessStatus} = await securityManager.checkAccess( + padId, message.sessionID, message.token, message.password, user); if (accessStatus === "grant") { // access was granted, mark the client as authorized and handle the message diff --git a/src/node/hooks/express/importexport.js b/src/node/hooks/express/importexport.js index 85ab6e129..4aa06ecb8 100644 --- a/src/node/hooks/express/importexport.js +++ b/src/node/hooks/express/importexport.js @@ -58,8 +58,9 @@ exports.expressCreateServer = function (hook_name, args, cb) { return next(); } + const {session: {user} = {}} = req; const {accessStatus, authorID} = await securityManager.checkAccess( - req.params.pad, req.cookies.sessionID, req.cookies.token, req.cookies.password); + req.params.pad, req.cookies.sessionID, req.cookies.token, req.cookies.password, user); if (accessStatus !== 'grant') return res.status(403).send('Forbidden'); assert(authorID); diff --git a/src/node/hooks/express/socketio.js b/src/node/hooks/express/socketio.js index 03fa7bbe6..b1a4d4f22 100644 --- a/src/node/hooks/express/socketio.js +++ b/src/node/hooks/express/socketio.js @@ -39,43 +39,36 @@ exports.expressCreateServer = function (hook_name, args, cb) { cookie: false, }); - /* Require an express session cookie to be present, and load the - * session. See http://www.danielbaulig.de/socket-ioexpress for more - * info */ + // REQUIRE a signed express-session cookie to be present, then load the session. See + // http://www.danielbaulig.de/socket-ioexpress for more info. After the session is loaded, ensure + // that the user has authenticated (if authentication is required). + // + // !!!WARNING!!! Requests to /socket.io are NOT subject to the checkAccess middleware in + // webaccess.js. If this handler fails to check for a signed express-session cookie or fails to + // 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(function(socket, accept) { + io.use((socket, next) => { var data = socket.request; - // Use a setting if we want to allow load Testing - - // Sometimes browsers might not have cookies at all, for example Safari in iFrames Cross domain - // https://github.com/ether/etherpad-lite/issues/4031 - // if requireSession is false we can allow them to still get on the pad. - // Note that this does make security less tight because any socketIO connection can be established without - // any logic on the client to do any handshaking.. I am not concerned about this though, the real solution - // here is to implement rateLimiting on SocketIO ACCEPT_COMMIT messages. - - if(!data.headers.cookie && (settings.loadTest || !settings.requireSession)){ - accept(null, true); - }else{ - if (!data.headers.cookie) return accept('No session cookie transmitted.', false); + if (!data.headers.cookie && settings.loadTest) { + console.warn('bypassing socket.io authentication check due to settings.loadTest'); + return next(null, true); } - if(data.headers.cookie){ - cookieParserFn(data, {}, function(err){ - if(err) { - console.error(err); - accept("Couldn't parse request cookies. ", false); - return; + 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'); } - - data.sessionID = data.signedCookies.express_sid; - args.app.sessionStore.get(data.sessionID, function (err, session) { - if (err || !session) return accept('Bad session / session has expired', false); - data.session = new sessionModule.Session(data, session); - accept(null, true); - }); + next(null, true); }); - } + }); }); // var socketIOLogger = log4js.getLogger("socket.io"); diff --git a/src/node/hooks/express/webaccess.js b/src/node/hooks/express/webaccess.js index 761a46ab8..e8caab566 100644 --- a/src/node/hooks/express/webaccess.js +++ b/src/node/hooks/express/webaccess.js @@ -17,18 +17,21 @@ exports.checkAccess = (req, res, next) => { // This may be called twice per access: once before authentication is checked and once after (if // settings.requireAuthorization is true). - const authorize = (cb) => { + const authorize = (fail) => { // Do not require auth for static paths and the API...this could be a bit brittle - if (req.path.match(/^\/(static|javascripts|pluginfw|api)/)) return cb(true); + if (req.path.match(/^\/(static|javascripts|pluginfw|api)/)) return next(); if (req.path.toLowerCase().indexOf('/admin') !== 0) { - if (!settings.requireAuthentication) return cb(true); - if (!settings.requireAuthorization && req.session && req.session.user) return cb(true); + if (!settings.requireAuthentication) return next(); + if (!settings.requireAuthorization && req.session && req.session.user) return next(); } - if (req.session && req.session.user && req.session.user.is_admin) return cb(true); + if (req.session && req.session.user && req.session.user.is_admin) return next(); - hooks.aCallFirst('authorize', {req, res, next, resource: req.path}, hookResultMangle(cb)); + hooks.aCallFirst('authorize', {req, res, next, resource: req.path}, hookResultMangle((ok) => { + if (ok) return next(); + return fail(); + )); }; /* Authentication OR authorization failed. */ @@ -59,12 +62,7 @@ exports.checkAccess = (req, res, next) => { let step1PreAuthenticate, step2Authenticate, step3Authorize; - step1PreAuthenticate = () => { - authorize((ok) => { - if (ok) return next(); - step2Authenticate(); - }); - }; + step1PreAuthenticate = () => authorize(step2Authenticate); step2Authenticate = () => { const ctx = {req, res, next}; @@ -98,12 +96,7 @@ exports.checkAccess = (req, res, next) => { })); }; - step3Authorize = () => { - authorize((ok) => { - if (ok) return next(); - failure(); - }); - }; + step3Authorize = () => authorize(failure); step1PreAuthenticate(); }; diff --git a/src/node/padaccess.js b/src/node/padaccess.js index 3449f7d16..6e294403e 100644 --- a/src/node/padaccess.js +++ b/src/node/padaccess.js @@ -3,7 +3,9 @@ var securityManager = require('./db/SecurityManager'); // checks for padAccess module.exports = async function (req, res) { try { - let accessObj = await securityManager.checkAccess(req.params.pad, req.cookies.sessionID, req.cookies.token, req.cookies.password); + const {session: {user} = {}} = req; + const accessObj = await securityManager.checkAccess( + req.params.pad, req.cookies.sessionID, req.cookies.token, req.cookies.password, user); if (accessObj.accessStatus === "grant") { // there is access, continue diff --git a/src/node/utils/promises.js b/src/node/utils/promises.js index 4fb2e8318..a754823a0 100644 --- a/src/node/utils/promises.js +++ b/src/node/utils/promises.js @@ -2,7 +2,40 @@ * Helpers to manipulate promises (like async but for promises). */ -var timesLimit = function (ltMax, concurrency, promiseCreator) { +// Returns a Promise that resolves to the first resolved value from `promises` that satisfies +// `predicate`. Resolves to `undefined` if none of the Promises satisfy `predicate`, or if +// `promises` is empty. If `predicate` is nullish, the truthiness of the resolved value is used as +// the predicate. +exports.firstSatisfies = (promises, predicate) => { + if (predicate == null) predicate = (x) => x; + + // Transform each original Promise into a Promise that never resolves if the original resolved + // value does not satisfy `predicate`. These transformed Promises will be passed to Promise.race, + // yielding the first resolved value that satisfies `predicate`. + const newPromises = promises.map( + (p) => new Promise((resolve, reject) => p.then((v) => predicate(v) && resolve(v), reject))); + + // If `promises` is an empty array or if none of them resolve to a value that satisfies + // `predicate`, then `Promise.race(newPromises)` will never resolve. To handle that, add another + // Promise that resolves to `undefined` after all of the original Promises resolve. + // + // Note: If all of the original Promises simultaneously resolve to a value that satisfies + // `predicate` (perhaps they were already resolved when this function was called), then this + // Promise will resolve too, and with a value of `undefined`. There is no concern that this + // Promise will win the race and thus cause an erroneous `undefined` result. This is because + // a resolved Promise's `.then()` function is scheduled for execution -- not executed right away + // -- and ES guarantees in-order execution of the enqueued invocations. Each of the above + // transformed Promises has a `.then()` chain of length one, while the Promise added here has a + // `.then()` chain of length two or more (at least one `.then()` that is internal to + // `Promise.all()`, plus the `.then()` function added here). By the time the `.then()` function + // added here executes, all of the above transformed Promises will have already resolved and one + // will have been chosen as the winner. + newPromises.push(Promise.all(promises).then(() => {})); + + return Promise.race(newPromises); +}; + +exports.timesLimit = function(ltMax, concurrency, promiseCreator) { var done = 0 var current = 0 @@ -26,7 +59,3 @@ var timesLimit = function (ltMax, concurrency, promiseCreator) { addAnother() } } - -module.exports = { - timesLimit: timesLimit -} diff --git a/src/node/utils/tar.json b/src/node/utils/tar.json index 1c1102f46..80d645dd0 100644 --- a/src/node/utils/tar.json +++ b/src/node/utils/tar.json @@ -6,8 +6,9 @@ , "pad_cookie.js" , "pad_editor.js" , "pad_editbar.js" - , "pad_docbar.js" + , "vendors/nice-select.js" , "pad_modals.js" + , "pad_automatic_reconnect.js" , "ace.js" , "collab_client.js" , "pad_userlist.js" @@ -19,6 +20,7 @@ , "$tinycon/tinycon.js" , "excanvas.js" , "farbtastic.js" + , "skin_variants.js" ] , "timeslider.js": [ "timeslider.js" @@ -29,8 +31,9 @@ , "pad_cookie.js" , "pad_editor.js" , "pad_editbar.js" - , "pad_docbar.js" + , "vendors/nice-select.js" , "pad_modals.js" + , "pad_automatic_reconnect.js" , "pad_savedrevs.js" , "pad_impexp.js" , "AttributePool.js" @@ -52,12 +55,14 @@ , "cssmanager.js" , "colorutils.js" , "undomodule.js" - , "$unorm.js" + , "$unorm/lib/unorm.js" , "contentcollector.js" , "changesettracker.js" , "linestylefilter.js" , "domline.js" , "AttributeManager.js" + , "scroll.js" + , "caretPosition.js" ] , "ace2_common.js": [ "ace2_common.js" diff --git a/src/static/css/iframe_editor.css b/src/static/css/iframe_editor.css index a68ca3527..7267375a4 100644 --- a/src/static/css/iframe_editor.css +++ b/src/static/css/iframe_editor.css @@ -25,7 +25,8 @@ html.inner-editor { /* ACE-PAD Container (i.e. where the text is displayed) */ #innerdocbody { - padding: 15px; + padding-left: 15px; + padding-right: 15px; overflow: hidden; background-color: white; line-height: 1.6; @@ -39,6 +40,13 @@ html.inner-editor { overflow-wrap: break-word; } +#innerdocbody, #sidediv { + /* Both must have the same top padding to line up line numbers */ + padding-top: 15px; + /* Some space when we scroll to the bottom */ + padding-bottom: 15px; +} + #innerdocbody a { color: #2e96f3; } @@ -144,4 +152,4 @@ body.mozilla, body.safari { } body.grayedout { background-color: #eee !important -} \ No newline at end of file +} diff --git a/src/static/js/pad.js b/src/static/js/pad.js index cfc58f18e..2e774a38a 100644 --- a/src/static/js/pad.js +++ b/src/static/js/pad.js @@ -156,7 +156,8 @@ function sendClientReady(isReconnect, messageType) createCookie("token", token, 60); } - var sessionID = decodeURIComponent(readCookie("sessionID")); + var encodedSessionID = readCookie('sessionID'); + var sessionID = encodedSessionID == null ? null : decodeURIComponent(encodedSessionID); var password = readCookie("password"); var msg = { diff --git a/tests/frontend/specs/responsiveness.js b/tests/frontend/specs/responsiveness.js index 3eb7a1919..e6552642f 100644 --- a/tests/frontend/specs/responsiveness.js +++ b/tests/frontend/specs/responsiveness.js @@ -23,7 +23,12 @@ describe('Responsiveness of Editor', function() { // And the test needs to be fixed to work in Firefox 52 on Windows 7. I am not sure why it fails on this specific platform // The errors show this.timeout... then crash the browser but I am sure something is actually causing the stack trace and // I just need to narrow down what, offers to help accepted. - xit('Fast response to keypress in pad with large amount of contents', function(done) { + it('Fast response to keypress in pad with large amount of contents', function(done) { + + //skip on Windows Firefox 52.0 + if(window.bowser && window.bowser.windows && window.bowser.firefox && window.bowser.version == "52.0") { + this.skip(); + } var inner$ = helper.padInner$; var chrome$ = helper.padChrome$; var chars = '0000000000'; // row of placeholder chars @@ -72,7 +77,7 @@ describe('Responsiveness of Editor', function() { var end = Date.now(); // get the current time var delay = end - start; // get the delay as the current time minus the start time - expect(delay).to.be.below(300); + expect(delay).to.be.below(400); done(); }, 1000);