From 989e12cdfbd40de749174ed63a49de1e5192f99b Mon Sep 17 00:00:00 2001 From: gautammanak1 Date: Tue, 11 Aug 2026 00:56:39 +0530 Subject: [PATCH] webaccess: preserve settings is_admin after authenticate (#8110) --- doc/api/hooks_server-side.adoc | 2 +- doc/api/hooks_server-side.md | 2 +- src/node/hooks/express/webaccess.ts | 17 ++++++++++++++++ src/tests/backend/specs/webaccess.ts | 29 ++++++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/doc/api/hooks_server-side.adoc b/doc/api/hooks_server-side.adoc index 25a696b432b..be7f754e636 100644 --- a/doc/api/hooks_server-side.adoc +++ b/doc/api/hooks_server-side.adoc @@ -519,7 +519,7 @@ authnFailure function unless falling back to HTTP basic authentication is appropriate upon authentication failure. This hook is only called if either the `requireAuthentication` setting is true -or the request is for an `/admin` page. +or the request is for `/admin-auth` (admin login / session verification). Calling the provided callback with `[true]` or `[false]` will cause authentication to succeed or fail, respectively. Calling the callback with `[]` diff --git a/doc/api/hooks_server-side.md b/doc/api/hooks_server-side.md index dc94c306993..367334b29d9 100644 --- a/doc/api/hooks_server-side.md +++ b/doc/api/hooks_server-side.md @@ -514,7 +514,7 @@ authnFailure function unless falling back to HTTP basic authentication is appropriate upon authentication failure. This hook is only called if either the `requireAuthentication` setting is true -or the request is for an `/admin` page. +or the request is for `/admin-auth` (admin login / session verification). Calling the provided callback with `[true]` or `[false]` will cause authentication to succeed or fail, respectively. Calling the callback with `[]` diff --git a/src/node/hooks/express/webaccess.ts b/src/node/hooks/express/webaccess.ts index 440cb352553..31ecf8aeb63 100644 --- a/src/node/hooks/express/webaccess.ts +++ b/src/node/hooks/express/webaccess.ts @@ -179,6 +179,14 @@ const checkAccess = async (req:any, res:any, next: Function) => { // user, or a privilege/identity change such as non-admin -> admin), which is // the point at which the session id must be rotated (see below). const prevUser = req.session != null ? req.session.user : null; + // Snapshot is_admin flags before authenticate plugins run. Plugins such as + // ep_hash_auth's hash_dir path replace settings.users[username] on success + // and can drop a pre-declared is_admin: true (issue #8110), which then fails + // the /admin-auth/ authorize check with 403. Restore those flags below. + const preAuthAdminUsers = new Set( + Object.entries(settings.users as Record) + .filter(([, user]) => user != null && !!user.is_admin) + .map(([username]) => username)); // If the HTTP basic auth header is present, extract the username and password so it can be given // to authn plugins. const httpBasicAuth = req.headers.authorization && req.headers.authorization.startsWith('Basic '); @@ -227,6 +235,15 @@ const checkAccess = async (req:any, res:any, next: Function) => { httpLogger.error('authenticate hook failed to add user settings to session'); return res.status(500).send('Internal Server Error'); } + // Restore is_admin when a settings-declared admin was authenticated by a + // plugin that rebuilt the user object without carrying the flag forward. + const authedUsername = req.session.user.username; + if (authedUsername && preAuthAdminUsers.has(authedUsername) && !req.session.user.is_admin) { + req.session.user.is_admin = true; + if (settings.users[authedUsername] != null) { + settings.users[authedUsername].is_admin = true; + } + } // Session fixation defense (GHSA-73h9-c5xp-gfg4): rotate the session id // whenever authentication changed the principal — an anonymous session // becoming authenticated, OR an authenticated session changing identity or diff --git a/src/tests/backend/specs/webaccess.ts b/src/tests/backend/specs/webaccess.ts index 919bb1a4187..89306e1625b 100644 --- a/src/tests/backend/specs/webaccess.ts +++ b/src/tests/backend/specs/webaccess.ts @@ -375,6 +375,35 @@ describe(__filename, function () { await agent.get('/').expect(500); assert.deepEqual(callOrder, ['preAuthorize_0', 'preAuthorize_1', 'authenticate_0']); }); + + // Regression for https://github.com/ether/etherpad/issues/8110 — + // ep_hash_auth (hash_dir path) authenticates by replacing + // settings.users[username] and can drop a pre-declared is_admin flag. + // /admin-auth/ must still admit the user when settings said they were admin. + it('POST /admin-auth/ invokes authenticate and preserves settings is_admin (#8110)', + async function () { + settings.requireAuthentication = false; + settings.users = { + // Classic ep_hash_auth setup: admin declared in settings, credentials + // supplied by the authenticate plugin (no plaintext password here). + hashadmin: {is_admin: true}, + }; + let authenticateCalled = false; + plugins.hooks.authenticate = [makeHook('authenticate', + (hookName: string, context: any, cb: Function) => { + authenticateCalled = true; + assert.equal(context.username, 'hashadmin'); + assert.equal(context.password, 'secret'); + // Mimic ep_hash_auth hash_dir success: rebuild the user object + // without carrying is_admin forward. + settings.users.hashadmin = {username: 'hashadmin', is_admin: false}; + context.req.session.user = settings.users.hashadmin; + return cb([true]); + })]; + await agent.post('/admin-auth/').auth('hashadmin', 'secret').expect(200); + assert.equal(authenticateCalled, true); + assert.equal(settings.users.hashadmin.is_admin, true); + }); }); describe('authorize', function () {