PadMessageHandler: Avoid redundant access checks

This commit is contained in:
Richard Hansen 2020-09-02 21:21:40 -04:00 committed by John McLear
parent 1e3aa9edff
commit 6ed11b7605
2 changed files with 34 additions and 67 deletions

View file

@ -218,13 +218,13 @@ exports.handleMessage = async function(client, message)
padId = await readOnlyManager.getPadId(padId);
}
// FIXME: Allow to override readwrite access with readonly
const {session: {user} = {}} = client.client.request;
const {accessStatus, authorID} =
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 });
if (accessStatus !== 'grant') {
// Access denied. Send the reason to the user.
client.json.send({accessStatus});
return;
}
if (thisSession.author != null && thisSession.author !== authorID) {
@ -259,7 +259,7 @@ exports.handleMessage = async function(client, message)
// Check what type of message we get and delegate to the other methods
if (message.type === "CLIENT_READY") {
await handleClientReady(client, message);
await handleClientReady(client, message, authorID);
} else if (message.type === "CHANGESET_REQ") {
await handleChangesetRequest(client, message);
} else if(message.type === "COLLABROOM") {
@ -284,7 +284,7 @@ exports.handleMessage = async function(client, message)
messageLogger.warn("Dropped message, unknown COLLABROOM Data Type " + message.data.type);
}
} else if(message.type === "SWITCH_TO_PAD") {
await handleSwitchToPad(client, message);
await handleSwitchToPad(client, message, authorID);
} else {
messageLogger.warn("Dropped message, unknown Message Type " + message.type);
}
@ -818,11 +818,30 @@ function _correctMarkersInPad(atext, apool) {
return builder.toString();
}
async function handleSwitchToPad(client, message)
async function handleSwitchToPad(client, message, _authorID)
{
// clear the session and leave the room
const currentSessionInfo = sessioninfos[client.id];
const padId = currentSessionInfo.padId;
// Check permissions for the new pad.
const newPadIds = await readOnlyManager.getIds(message.padId);
const {session: {user} = {}} = client.client.request;
const {accessStatus, authorID} = await securityManager.checkAccess(
newPadIds.padId, message.sessionID, message.token, message.password, user);
if (accessStatus !== 'grant') {
// Access denied. Send the reason to the user.
client.json.send({accessStatus});
return;
}
// The same token and session ID were passed to checkAccess in handleMessage, so this second call
// to checkAccess should return the same author ID.
assert(authorID === _authorID);
assert(authorID === currentSessionInfo.author);
// Check if the connection dropped during the access check.
if (sessioninfos[client.id] !== currentSessionInfo) return;
// clear the session and leave the room
_getRoomClients(padId).forEach(client => {
let sinfo = sessioninfos[client.id];
if (sinfo && sinfo.author === currentSessionInfo.author) {
@ -835,7 +854,7 @@ async function handleSwitchToPad(client, message)
// start up the new pad
const newSessionInfo = sessioninfos[client.id];
createSessionInfoAuth(newSessionInfo, message);
await handleClientReady(client, message);
await handleClientReady(client, message, authorID);
}
// Creates/replaces the auth object in the given session info.
@ -860,7 +879,7 @@ function createSessionInfoAuth(sessionInfo, message)
* @param client the client that send this message
* @param message the message from the client
*/
async function handleClientReady(client, message)
async function handleClientReady(client, message, authorID)
{
// check if all ok
if (!message.token) {
@ -888,17 +907,6 @@ async function handleClientReady(client, message)
// Get ro/rw id:s
let padIds = await readOnlyManager.getIds(message.padId);
// FIXME: Allow to override readwrite access with readonly
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") {
client.json.send({ accessStatus });
return;
}
// get all authordata of this new user
assert(authorID);
let value = await authorManager.getAuthor(authorID);

View file

@ -65,8 +65,6 @@ exports.setSocketIO = function(_socket) {
remoteAddress[client.id] = client.handshake.address;
}
var clientAuthorized = false;
// wrap the original send function to log the messages
client._send = client.send;
client.send = function(message) {
@ -84,37 +82,12 @@ exports.setSocketIO = function(_socket) {
messageLogger.warn("Protocolversion header is not correct:" + stringifyWithoutPassword(message));
return;
}
if (clientAuthorized) {
// client is authorized, everything ok
await handleMessage(client, message);
} else {
// try to authorize the client
if (message.padId !== undefined && message.sessionID !== undefined && message.token !== undefined && message.password !== undefined) {
// check for read-only pads
let padId = message.padId;
if (padId.indexOf("r.") === 0) {
padId = await readOnlyManager.getPadId(message.padId);
}
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
clientAuthorized = true;
await handleMessage(client, message);
} else {
// no access, send the client a message that tells him why
messageLogger.warn("Authentication try failed:" + stringifyWithoutPassword(message));
client.json.send({ accessStatus });
}
} else {
// drop message
messageLogger.warn("Dropped message because of bad permissions:" + stringifyWithoutPassword(message));
}
if (!message.component || !components[message.component]) {
messageLogger.error("Can't route the message:" + stringifyWithoutPassword(message));
return;
}
messageLogger.debug("from " + client.id + ": " + stringifyWithoutPassword(message));
await components[message.component].handleMessage(client, message);
});
client.on('disconnect', function() {
@ -126,20 +99,6 @@ exports.setSocketIO = function(_socket) {
});
}
// try to handle the message of this client
async function handleMessage(client, message)
{
if (message.component && components[message.component]) {
// check if component is registered in the components array
if (components[message.component]) {
messageLogger.debug("from " + client.id + ": " + stringifyWithoutPassword(message));
await components[message.component].handleMessage(client, message);
}
} else {
messageLogger.error("Can't route the message:" + stringifyWithoutPassword(message));
}
}
// returns a stringified representation of a message, removes the password
// this ensures there are no passwords in the log
function stringifyWithoutPassword(message)