From 253a8e37fd6f7a81bd0e7659919890a36dad3f23 Mon Sep 17 00:00:00 2001 From: mluto Date: Sat, 30 Mar 2013 20:28:46 +0100 Subject: [PATCH 1/5] Added debug-output to SecurityManager.checkAccess to indicate *why* an auth-try failed. --- src/node/db/SecurityManager.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/node/db/SecurityManager.js b/src/node/db/SecurityManager.js index 4289e39ca..074728b58 100644 --- a/src/node/db/SecurityManager.js +++ b/src/node/db/SecurityManager.js @@ -134,12 +134,14 @@ exports.checkAccess = function (padID, sessionCookie, token, password, callback) //is it for this group? if(sessionInfo.groupID != groupID) { + console.debug("Auth failed: wrong group"); callback(); return; } //is validUntil still ok? if(sessionInfo.validUntil <= now){ + console.debug("Auth failed: validUntil"); callback(); return; } @@ -234,7 +236,11 @@ exports.checkAccess = function (padID, sessionCookie, token, password, callback) //--> grant access statusObject = {accessStatus: "grant", authorID: sessionAuthor}; //--> deny access if user isn't allowed to create the pad - if(settings.editOnly) statusObject.accessStatus = "deny"; + if(settings.editOnly) + { + console.debug("Auth failed: valid session & pad does not exist"); + statusObject.accessStatus = "deny"; + } } // there is no valid session avaiable AND pad exists else if(!validSession && padExists) @@ -266,6 +272,7 @@ exports.checkAccess = function (padID, sessionCookie, token, password, callback) //- its not public else if(!isPublic) { + console.debug("Auth failed: invalid session & pad is not public"); //--> deny access statusObject = {accessStatus: "deny"}; } @@ -277,6 +284,7 @@ exports.checkAccess = function (padID, sessionCookie, token, password, callback) // there is no valid session avaiable AND pad doesn't exists else { + console.debug("Auth failed: invalid session & pad does not exist"); //--> deny access statusObject = {accessStatus: "deny"}; } From 7e3b288afc53e8908c0550c9d0944af6183c161a Mon Sep 17 00:00:00 2001 From: mluto Date: Sat, 30 Mar 2013 20:46:56 +0100 Subject: [PATCH 2/5] log things the log4js-way; put all the brackets on a new line --- src/node/db/SecurityManager.js | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/node/db/SecurityManager.js b/src/node/db/SecurityManager.js index 074728b58..d3a98446d 100644 --- a/src/node/db/SecurityManager.js +++ b/src/node/db/SecurityManager.js @@ -27,6 +27,8 @@ var padManager = require("./PadManager"); var sessionManager = require("./SessionManager"); var settings = require("../utils/Settings"); var randomString = require('ep_etherpad-lite/static/js/pad_utils').randomString; +var log4js = require('log4js'); +var authLogger = log4js.getLogger("auth"); /** * This function controlls the access to a pad, it checks if the user can access a pad. @@ -117,14 +119,17 @@ exports.checkAccess = function (padID, sessionCookie, token, password, callback) //get information about all sessions contained in this cookie function(callback) { - if (!sessionCookie) { + if (!sessionCookie) + { callback(); return; } var sessionIDs = sessionCookie.split(','); - async.forEach(sessionIDs, function(sessionID, callback) { - sessionManager.getSessionInfo(sessionID, function(err, sessionInfo) { + async.forEach(sessionIDs, function(sessionID, callback) + { + sessionManager.getSessionInfo(sessionID, function(err, sessionInfo) + { //skip session if it doesn't exist if(err && err.message == "sessionID does not exist") return; @@ -133,15 +138,17 @@ exports.checkAccess = function (padID, sessionCookie, token, password, callback) var now = Math.floor(new Date().getTime()/1000); //is it for this group? - if(sessionInfo.groupID != groupID) { - console.debug("Auth failed: wrong group"); + if(sessionInfo.groupID != groupID) + { + authLogger.debug("Auth failed: wrong group"); callback(); return; } //is validUntil still ok? - if(sessionInfo.validUntil <= now){ - console.debug("Auth failed: validUntil"); + if(sessionInfo.validUntil <= now) + { + authLogger.debug("Auth failed: validUntil"); callback(); return; } @@ -238,7 +245,7 @@ exports.checkAccess = function (padID, sessionCookie, token, password, callback) //--> deny access if user isn't allowed to create the pad if(settings.editOnly) { - console.debug("Auth failed: valid session & pad does not exist"); + authLogger.debug("Auth failed: valid session & pad does not exist"); statusObject.accessStatus = "deny"; } } @@ -272,7 +279,7 @@ exports.checkAccess = function (padID, sessionCookie, token, password, callback) //- its not public else if(!isPublic) { - console.debug("Auth failed: invalid session & pad is not public"); + authLogger.debug("Auth failed: invalid session & pad is not public"); //--> deny access statusObject = {accessStatus: "deny"}; } @@ -284,7 +291,7 @@ exports.checkAccess = function (padID, sessionCookie, token, password, callback) // there is no valid session avaiable AND pad doesn't exists else { - console.debug("Auth failed: invalid session & pad does not exist"); + authLogger.debug("Auth failed: invalid session & pad does not exist"); //--> deny access statusObject = {accessStatus: "deny"}; } From 30cae9097f2979048a2491b2ec032aa9548121a2 Mon Sep 17 00:00:00 2001 From: mluto Date: Sun, 31 Mar 2013 10:27:21 +0200 Subject: [PATCH 3/5] When there is just one session and this one is invalid SecurityManager.checkAccess would cause the request to hang forever because the callback is omitted for each invalid session, this fixes this issue. validSession still remains false so this does not cause issues further down. --- src/node/db/SecurityManager.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/node/db/SecurityManager.js b/src/node/db/SecurityManager.js index d3a98446d..410204e01 100644 --- a/src/node/db/SecurityManager.js +++ b/src/node/db/SecurityManager.js @@ -131,7 +131,11 @@ exports.checkAccess = function (padID, sessionCookie, token, password, callback) sessionManager.getSessionInfo(sessionID, function(err, sessionInfo) { //skip session if it doesn't exist - if(err && err.message == "sessionID does not exist") return; + if(err && err.message == "sessionID does not exist") + { + callback(); + return; + } if(ERR(err, callback)) return; From 911bfb30e4d1f11ea569cc0d922085821dd23158 Mon Sep 17 00:00:00 2001 From: mluto Date: Sun, 31 Mar 2013 10:56:14 +0200 Subject: [PATCH 4/5] Log when a sessionID in checkAccess is not found --- src/node/db/SecurityManager.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node/db/SecurityManager.js b/src/node/db/SecurityManager.js index 410204e01..06e58bc4b 100644 --- a/src/node/db/SecurityManager.js +++ b/src/node/db/SecurityManager.js @@ -133,6 +133,7 @@ exports.checkAccess = function (padID, sessionCookie, token, password, callback) //skip session if it doesn't exist if(err && err.message == "sessionID does not exist") { + authLogger.debug("Auth failed: unknown session"); callback(); return; } From 1793e93ea1b831329b580c7a5aa4fb982c665417 Mon Sep 17 00:00:00 2001 From: mluto Date: Sun, 31 Mar 2013 11:30:01 +0200 Subject: [PATCH 5/5] Decode the sessionID before sending it to the server since our separator ',' gets encoded --- src/static/js/pad.js | 2 +- src/static/js/timeslider.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/static/js/pad.js b/src/static/js/pad.js index e5e6e95f3..5cc02d87e 100644 --- a/src/static/js/pad.js +++ b/src/static/js/pad.js @@ -191,7 +191,7 @@ function handshake() createCookie("token", token, 60); } - var sessionID = readCookie("sessionID"); + var sessionID = decodeURIComponent(readCookie("sessionID")); var password = readCookie("password"); var msg = { diff --git a/src/static/js/timeslider.js b/src/static/js/timeslider.js index eb3703d9f..cd98deadd 100644 --- a/src/static/js/timeslider.js +++ b/src/static/js/timeslider.js @@ -116,7 +116,7 @@ function init() { //sends a message over the socket function sendSocketMsg(type, data) { - var sessionID = readCookie("sessionID"); + var sessionID = decodeURIComponent(readCookie("sessionID")); var password = readCookie("password"); var msg = { "component" : "pad", // FIXME: Remove this stupidity!