Fix admin authorization on recent Etherpad

As of Etherpad 1.8.7 (and possibly earlier), the authorize hook did not seem to be called anymore. According to the documentation, it is not called for admin paths, and recent Etherpads allow admin access only to admin users anyway. Thus, this commit moves the admin check to be part of authentication. This has the disadvantage that admin sessions will stay valid even if a user is removed from an admin group, which is now documented in the README.
tykeal · Mar 14, 2021 · 5dfbfc1 · 5dfbfc1
1 parent 42cd54c
commit 5dfbfc1
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 83 deletions.
diff --git a/README.md b/README.md
@@ -26,7 +26,9 @@ Add to settings.json:
     },
 
 Users who are in the matches group have *admin* access to
-etherpad-lite.
+etherpad-lite. Note that once they gain access to an admin session once,
+they can use this session in the future to retain admin access even if
+they are removed from the admin group.
 
 ## Using with FreeIPA
 

diff --git a/ep.json b/ep.json
@@ -4,7 +4,6 @@
       "name": "ep_ldapauth",
       "hooks": {
         "authenticate": "ep_ldapauth/ep_ldapauth",
-        "authorize": "ep_ldapauth/ep_ldapauth",
         "handleMessage": "ep_ldapauth/ep_ldapauth"
       }
     }

diff --git a/ep_ldapauth.js b/ep_ldapauth.js
@@ -78,101 +78,60 @@ exports.authenticate = function(hook_name, context, cb) {
       });
       authenticateLDAP = null;
       console.debug('ep_ldapauth.authenticate: successful authentication');
-      return cb([true]);
+
+      checkAdmin(context, cb.bind(null, [true]));
     });
   } else {
     console.debug('ep_ldapauth.authenticate: failed authentication no auth headers');
     return cb([false]);
   }
 };
 
-exports.authorize = function(hook_name, context, cb) {
-  console.debug('ep_ldapauth.authorize');
-
-  if(settings.users.ldapauth.anonymousReadonly &&
-      /^\/(p\/r\..{16}|locales.json|static|javascripts|pluginfw|favicon.ico)/.test(context.resource)) {
-    console.debug('ep_ldapauth.authorize.anonymous: authorizing static path %s', context.resource);
-    return cb([true]);
-  }
-
-  userDN = null;
-
-  if (typeof(context.req.session.user) !== 'undefined' &&
-    typeof(context.req.session.user.username) !== 'undefined') {
-    username = context.req.session.user.username;
-    if (typeof(context.req.session.user.userDN) !== 'undefined') {
-      userDN = context.req.session.user.userDN;
-    }
-  } else {
-    console.debug('ep_ldapauth.authorize: no username in user object');
-    return cb([false]);
+function checkAdmin(context, cb) {
+  console.debug('ep_ldapauth.checkAdmin');
+  var username = context.req.session.user.username;
+  var userDN = context.req.session.user.userDN;
+
+  var myLdapAuthOpts = {
+    url: settings.users.ldapauth.url,
+    adminDn: settings.users.ldapauth.searchDN,
+    adminPassword: settings.users.ldapauth.searchPWD,
+    searchBase: settings.users.ldapauth.accountBase,
+    searchFilter: settings.users.ldapauth.accountPattern,
+    groupSearchBase: settings.users.ldapauth.groupSearchBase,
+    groupAttribute: settings.users.ldapauth.groupAttribute,
+    groupAttributeIsDN: settings.users.ldapauth.groupAttributeIsDN,
+    searchScope: settings.users.ldapauth.searchScope,
+    groupSearch: settings.users.ldapauth.groupSearch,
+    cache: true
+  };
+
+  if (typeof(settings.users.ldapauth.tls_ca_file) !== 'undefined') {
+    myLdapAuthOpts.tls_ca = fs.readFileSync(settings.users.ldapauth.tls_ca_file);
   }
 
-  if (/^\/(static|javascripts|pluginfw|favicon.ico|api)/.test(context.resource)) {
-    console.debug('ep_ldapauth.authorize: authorizing static path %s', context.resource);
-    return cb([true]);
-  } else if (context.resource.match(/^\/admin/)) {
-    console.debug('ep_ldapauth.authorize: attempting to authorize along administrative path %s', context.resource);
-    var myLdapAuthOpts = {
-      url: settings.users.ldapauth.url,
-      adminDn: settings.users.ldapauth.searchDN,
-      adminPassword: settings.users.ldapauth.searchPWD,
-      searchBase: settings.users.ldapauth.accountBase,
-      searchFilter: settings.users.ldapauth.accountPattern,
-      groupSearchBase: settings.users.ldapauth.groupSearchBase,
-      groupAttribute: settings.users.ldapauth.groupAttribute,
-      groupAttributeIsDN: settings.users.ldapauth.groupAttributeIsDN,
-      searchScope: settings.users.ldapauth.searchScope,
-      groupSearch: settings.users.ldapauth.groupSearch,
-      cache: true
-    };
-
-    if (typeof(settings.users.ldapauth.tls_ca_file) !== 'undefined') {
-      myLdapAuthOpts.tls_ca = fs.readFileSync(settings.users.ldapauth.tls_ca_file);
-    }
+  var authorizeLDAP = new MyLdapAuth(myLdapAuthOpts);
 
-    var authorizeLDAP = new MyLdapAuth(myLdapAuthOpts);
+  // fallback
+  context.req.session.user.is_admin = false;
 
-    authorizeLDAP.groupsearch(username, userDN, function(err, groups) {
+  authorizeLDAP.groupsearch(username, userDN, function(err, groups) {
+    authorizeLDAP.close(function (err) {
       if (err) {
-        console.error('ep_ldapauth.authorize: LDAP groupsearch error: %s', err);
-        authorizeLDAP.close(function (err) {
-          if (err) {
-            console.error('ep_ldapauth.authorize: LDAP close error: %s', err);
-          }
-        });
-        authorizeLDAP = null;
-        return cb([false]);
-      }
-
-      // We've recieved back group(s) that the user matches
-      // Given our current auth scheme (only checking on admin) we'll auth
-      if (groups) {
-        context.req.session.user.is_admin = true;
-        authorizeLDAP.close(function (err) {
-          if (err) {
-            console.error('ep_ldapauth.authorize: LDAP close error: %s', err);
-          }
-        });
-        authorizeLDAP = null;
-        console.debug('ep_ldapauth.authorize: successful authorization');
-        return cb([true]);
-      } else {
-        context.req.session.user.is_admin = false;
-        authorizeLDAP.close(function (err) {
-          if (err) {
-            console.error('ep_ldapauth.authorize: LDAP close error: %s', err);
-          }
-        });
-        authorizeLDAP = null;
-        console.debug('ep_ldapauth.authorize: failed authorization');
-        return cb([false]);
+        console.error('ep_ldapauth.checkAdmin: LDAP close error: %s', err);
       }
     });
-  } else {
-    console.debug('ep_ldapauth.authorize: passing authorize along for path %s', context.resource);
-    return cb([false]);
-  }
+    if (err) {
+      console.error('ep_ldapauth.checkAdmin: LDAP groupsearch error: %s', err);
+      return cb(false);
+    }
+
+    // We've received back group(s) that the user matches
+    // Given our current auth scheme (only checking on admin) we'll auth
+    context.req.session.user.is_admin = Boolean(groups);
+    console.debug('ep_ldapauth.checkAdmin: ' + (context.req.session.user.is_admin ? 'successful' : 'failed') + ' authorization');
+    return cb(context.req.session.user.is_admin);
+  });
 };
 
 exports.handleMessage = function(hook_name, context, cb) {