Fix read only pad access with authentication

Before this commit, webaccess.checkAccess saved the authorization in
user.padAuthorizations[padId] with padId being the read-only pad ID,
however later stages, e.g. in PadMessageHandler, use the real pad ID for
access checks. This led to authorization being denied.

This commit fixes it by only storing and comparing the real pad IDs and
not read-only pad IDs.

This fixes test case "authn user readonly pad -> 200, ok" in
src/tests/backend/specs/socketio.js.

doc/api/hooks_server-side.md

@@ -156,11 +156,13 @@ Called from: src/node/db/SecurityManager.js
 
 Things in context:
 
-1. padID - the pad the user wants to access
+1. padID - the real ID (never the read-only ID) of the pad the user wants to
+   access
 2. token - the token of the author
 3. sessionCookie - the session the use has
 
-This hook gets called when the access to the concrete pad is being checked. Return `false` to deny access.
+This hook gets called when the access to the concrete pad is being checked.
+Return `false` to deny access.
 
 ## padCreate
 Called from: src/node/db/Pad.js

src/node/db/SecurityManager.js

@@ -22,6 +22,7 @@
 const authorManager = require('./AuthorManager');
 const hooks = require('../../static/js/pluginfw/hooks.js');
 const padManager = require('./PadManager');
+const readOnlyManager = require('./ReadOnlyManager');
 const sessionManager = require('./SessionManager');
 const settings = require('../utils/Settings');
 const webaccess = require('../hooks/express/webaccess');
@@ -56,6 +57,15 @@ exports.checkAccess = async (padID, sessionCookie, token, userSettings) => {
 
   let canCreate = !settings.editOnly;
 
+  if (readOnlyManager.isReadOnlyId(padID)) {
+    canCreate = false;
+    padID = await readOnlyManager.getPadId(padID);
+    if (padID == null) {
+      authLogger.debug('access denied: read-only pad ID for a pad that does not exist');
+      return DENY;
+    }
+  }
+
   // Authentication and authorization checks.
   if (settings.loadTest) {
     console.warn(

src/node/handler/PadMessageHandler.js

@@ -218,17 +218,9 @@ exports.handleMessage = async (socket, message) => {
     return;
   }
 
-  // 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} = {}} = socket.client.request;
   const {accessStatus, authorID} =
-      await securityManager.checkAccess(padId, auth.sessionID, auth.token, user);
+      await securityManager.checkAccess(auth.padID, auth.sessionID, auth.token, user);
   if (accessStatus !== 'grant') {
     // Access denied. Send the reason to the user.
     socket.json.send({accessStatus});

src/node/hooks/express/webaccess.js

@@ -70,14 +70,19 @@ const checkAccess = async (req, res, next) => {
   // This helper is used in steps 2 and 4 below, so it may be called twice per access: once before
   // authentication is checked and once after (if settings.requireAuthorization is true).
   const authorize = async () => {
-    const grant = (level) => {
+    const grant = async (level) => {
       level = exports.normalizeAuthzLevel(level);
       if (!level) return false;
       const user = req.session.user;
       if (user == null) return true; // This will happen if authentication is not required.
       const encodedPadId = (req.path.match(/^\/p\/([^/]*)/) || [])[1];
       if (encodedPadId == null) return true;
-      const padId = decodeURIComponent(encodedPadId);
+      let padId = decodeURIComponent(encodedPadId);
+      if (readOnlyManager.isReadOnlyId(padId)) {
+        // pad is read-only, first get the real pad ID
+        padId = await readOnlyManager.getPadId(padId);
+        if (padId == null) return false;
+      }
       // The user was granted access to a pad. Remember the authorization level in the user's
       // settings so that SecurityManager can approve or deny specific actions.
       if (user.padAuthorizations == null) user.padAuthorizations = {};
@@ -85,13 +90,13 @@ const checkAccess = async (req, res, next) => {
       return true;
     };
     const isAuthenticated = req.session && req.session.user;
-    if (isAuthenticated && req.session.user.is_admin) return grant('create');
+    if (isAuthenticated && req.session.user.is_admin) return await grant('create');
     const requireAuthn = requireAdmin || settings.requireAuthentication;
-    if (!requireAuthn) return grant('create');
-    if (!isAuthenticated) return grant(false);
-    if (requireAdmin && !req.session.user.is_admin) return grant(false);
-    if (!settings.requireAuthorization) return grant('create');
-    return grant(await aCallFirst0('authorize', {req, res, next, resource: req.path}));
+    if (!requireAuthn) return await grant('create');
+    if (!isAuthenticated) return await grant(false);
+    if (requireAdmin && !req.session.user.is_admin) return await grant(false);
+    if (!settings.requireAuthorization) return await grant('create');
+    return await grant(await aCallFirst0('authorize', {req, res, next, resource: req.path}));
   };
 
   // ///////////////////////////////////////////////////////////////////////////////////////////////