diff --git a/CHANGELOG.md b/CHANGELOG.md index 563d8c40c..0179005a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,9 @@ (low-level API) and `ep_etherpad-lite/static/js/AttributeMap` (high-level API). * The `import` server-side hook has a new `ImportError` context property. +* The `handleMessageSecurity` and `handleMessage` server-side hooks have a new + `sessionInfo` context property that includes the user's author ID, the pad ID, + and whether the user only has read-only access. ### Compatibility changes diff --git a/doc/api/hooks_server-side.md b/doc/api/hooks_server-side.md index 648aff224..54192ef3c 100644 --- a/doc/api/hooks_server-side.md +++ b/doc/api/hooks_server-side.md @@ -585,6 +585,12 @@ then the message will not be subject to further processing. Context properties: * `message`: The message being handled. +* `sessionInfo`: Object describing the socket.io session with the following + properties: + * `authorId`: The user's author ID. + * `padId`: The real (not read-only) ID of the pad. + * `readOnly`: Whether the client has read-only access (true) or read/write + access (false). * `socket`: The socket.io Socket object. * `client`: (**Deprecated**; use `socket` instead.) Synonym of `socket`. @@ -620,13 +626,21 @@ Supported return values: Context properties: * `message`: The message being handled. +* `sessionInfo`: Object describing the socket.io connection with the following + properties: + * `authorId`: The user's author ID. + * `padId`: The real (not read-only) ID of the pad. + * `readOnly`: Whether the client has read-only access (true) or read/write + access (false). * `socket`: The socket.io Socket object. * `client`: (**Deprecated**; use `socket` instead.) Synonym of `socket`. Example: ```javascript -exports.handleMessageSecurity = async (hookName, {message, socket}) => { +exports.handleMessageSecurity = async (hookName, context) => { + const {message, sessionInfo: {readOnly}, socket} = context; + if (!readOnly || message.type !== 'COLLABROOM') return; if (shouldGrantWriteAccess(message, socket)) return true; }; ``` diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js index da25b38dd..34db0cbc6 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/src/node/handler/PadMessageHandler.js @@ -235,6 +235,11 @@ exports.handleMessage = async (socket, message) => { padID: message.padId, token: message.token, }; + const padIds = await readOnlyManager.getIds(thisSession.auth.padID); + thisSession.padId = padIds.padId; + thisSession.readOnlyPadId = padIds.readOnlyPadId; + thisSession.readonly = + padIds.readonly || !webaccess.userCanModify(thisSession.auth.padID, socket.client.request); } const auth = thisSession.auth; @@ -273,6 +278,11 @@ exports.handleMessage = async (socket, message) => { // Allow plugins to bypass the readonly message blocker const context = { message, + sessionInfo: { + authorId: thisSession.author, + padId: thisSession.padId, + readOnly: thisSession.readonly, + }, socket, get client() { padutils.warnDeprecated( @@ -793,12 +803,6 @@ const handleClientReady = async (socket, message) => { if (sessionInfo == null) return; assert(sessionInfo.author); - const padIds = await readOnlyManager.getIds(sessionInfo.auth.padID); - sessionInfo.padId = padIds.padId; - sessionInfo.readOnlyPadId = padIds.readOnlyPadId; - sessionInfo.readonly = - padIds.readonly || !webaccess.userCanModify(sessionInfo.auth.padID, socket.client.request); - await hooks.aCallAll('clientReady', message); // Deprecated due to awkward context. let {colorId: authorColorId, name: authorName} = message.userInfo || {}; diff --git a/src/node/hooks/express/webaccess.js b/src/node/hooks/express/webaccess.js index 10e531717..1e1adf41a 100644 --- a/src/node/hooks/express/webaccess.js +++ b/src/node/hooks/express/webaccess.js @@ -36,12 +36,10 @@ exports.userCanModify = (padId, req) => { if (readOnlyManager.isReadOnlyId(padId)) return false; if (!settings.requireAuthentication) return true; const {session: {user} = {}} = req; - assert(user); // If authn required and user == null, the request should have already been denied. - if (user.readOnly) return false; + if (!user || user.readOnly) return false; assert(user.padAuthorizations); // This is populated even if !settings.requireAuthorization. const level = exports.normalizeAuthzLevel(user.padAuthorizations[padId]); - assert(level); // If !level, the request should have already been denied. - return level !== 'readOnly'; + return level && level !== 'readOnly'; }; // Exported so that tests can set this to 0 to avoid unnecessary test slowness.