PadMessageHandler: Pass session info to handleMessageSecurity hook

Author: rhansen

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
 

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;
 };
 ```

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 || {};

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.