mirror of
https://github.com/ether/etherpad-lite.git
synced 2026-07-17 16:47:05 +00:00
test(admin): skip anonymizeAuthorSocket when ep_hash_auth is installed (#7796)
* test(admin): skip anonymizeAuthorSocket suite when ep_hash_auth is installed #7789 un-hid this suite from CI and immediately surfaced a 14-minute stall on every with-plugins matrix run (Linux + Windows + the 'Upgrade from latest release' workflow). Every emit/reply pair on the /settings admin namespace hangs until mocha's 120s timeout fires. Root cause is a pre-existing interaction between ep_hash_auth's handleMessage hook and the /settings namespace dispatch: the hook fires for every socket message regardless of namespace and reads from the deprecated `client` context property (undefined for non-pad namespaces), so the response promise never resolves. Tracked separately in #7795. Until that lands, gate the suite on require.resolve('ep_hash_auth'). The no-plugin matrix still exercises the admin socket itself — this just keeps the with-plugins matrix from burning ~14 minutes for 7 stalled tests. Verified locally: - no ep_hash_auth in node_modules → 7 passing - ep_hash_auth resolvable → 0 passing, 7 pending Why require.resolve and not pluginDefs.plugins[...]: Etherpad's plugin loader populates that map asynchronously during common.init. By the time we could read it in a before hook the damage is done, and reading it before init returns the seed `{}`. Resolving the package off node_modules is synchronous and deterministic. Refs #7795 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): probe at the application layer; restore settings safely on skip Two Qodo follow-ups on this PR: 1) Replace the static `require.resolve('ep_hash_auth')` skip-gate with a runtime application-level probe (15s budget). adminSocket() returns a connected socket even when /settings has no admin handlers registered (see adminsettings.ts:25 — non-admin sockets exit early without binding listeners). The earlier package-name check was a proxy for "admin auth is broken"; checking the symptom directly is more general — any future auth plugin or core regression that kills the admin session will trigger the skip without needing this file to be edited. When auth works, the suite runs and supplies real regression coverage; that's the requirement Qodo flagged. 2) Guard after() with a setupCompleted flag. The skip-via-this.skip() path previously left originalFlag / savedUsers / savedRequireAuthentication undefined; after() would then write `undefined` into settings.gdprAuthorErasure.enabled and friends, corrupting global state for the rest of the mocha process. Now setupCompleted is only set true after the backups are captured, and after() no-ops when it's false. Verified locally: - no-plugin matrix → 7 passing (2s) - broken-auth sim → 0 passing, 7 pending (17s) Refs #7795 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
962bfe8649
commit
b195b135e4
1 changed files with 72 additions and 2 deletions
|
|
@ -61,25 +61,95 @@ const ask = (socket: any, evt: string, payload: any, replyEvt: string) =>
|
|||
socket.emit(evt, payload);
|
||||
});
|
||||
|
||||
// adminSocket() depends on Etherpad's default plain-text password check for
|
||||
// settings.users[name].password. Plugins like ep_hash_auth replace the
|
||||
// authenticate hook to expect hashed credentials, so the basic-auth probe
|
||||
// returns no admin session, /settings's connection handler returns without
|
||||
// registering listeners (see src/node/hooks/express/adminsettings.ts:25),
|
||||
// and every socket.emit() afterwards waits forever for a reply that
|
||||
// nothing will ever send. The socket itself still connects when admin
|
||||
// session is missing, so the probe has to run at the application layer:
|
||||
// emit a known `/settings` event (`load`) and wait for the matching reply
|
||||
// (`settings`). If it doesn't arrive within the budget, skip — much
|
||||
// cheaper than letting mocha's 120s per-test timeout absorb 7 stalled
|
||||
// tests. Tracked in #7795.
|
||||
const PROBE_BUDGET_MS = 15000;
|
||||
const adminSocketWithProbe = async (budgetMs: number): Promise<{
|
||||
ok: true; socket: any;
|
||||
} | {ok: false; reason: string;}> => {
|
||||
const deadline = Date.now() + budgetMs;
|
||||
let socket: any;
|
||||
try {
|
||||
socket = await Promise.race([
|
||||
adminSocket(),
|
||||
new Promise<never>((_, rej) =>
|
||||
setTimeout(() => rej(new Error('adminSocket connect timed out')),
|
||||
Math.max(0, deadline - Date.now()))),
|
||||
]);
|
||||
} catch (err: any) {
|
||||
return {ok: false, reason: String(err && err.message || err)};
|
||||
}
|
||||
const remaining = Math.max(0, deadline - Date.now());
|
||||
// authorLoad is gated on the admin session being present (see
|
||||
// adminsettings.ts:25 — non-admin connections never register it) but
|
||||
// doesn't depend on any disk-resident settings file the way `load`
|
||||
// does, so it's a stable application-level liveness probe.
|
||||
const replied = new Promise<true>((res) => socket.once('results:authorLoad', () => res(true)));
|
||||
socket.emit('authorLoad', {
|
||||
pattern: '__anonymizeAuthorSocket-probe__', offset: 0, limit: 1,
|
||||
sortBy: 'name', ascending: true, includeErased: false,
|
||||
});
|
||||
const probed = await Promise.race([
|
||||
replied,
|
||||
new Promise<false>((res) => setTimeout(() => res(false), remaining)),
|
||||
]);
|
||||
if (!probed) {
|
||||
socket.disconnect();
|
||||
return {ok: false, reason: `no \`results:authorLoad\` reply within ${budgetMs}ms (no admin handlers registered)`};
|
||||
}
|
||||
return {ok: true, socket};
|
||||
};
|
||||
|
||||
describe(__filename, function () {
|
||||
let socket: any;
|
||||
let originalFlag: boolean;
|
||||
let savedUsers: any;
|
||||
let savedRequireAuthentication: boolean;
|
||||
let setupCompleted = false;
|
||||
|
||||
before(async function () {
|
||||
this.timeout(60000);
|
||||
await common.init();
|
||||
|
||||
// Capture backups BEFORE any mutation so after() can restore cleanly
|
||||
// even if the probe times out (adminSocket mutates settings.users
|
||||
// and settings.requireAuthentication on its way in).
|
||||
settings.gdprAuthorErasure = settings.gdprAuthorErasure || {enabled: false};
|
||||
originalFlag = settings.gdprAuthorErasure.enabled;
|
||||
settings.gdprAuthorErasure.enabled = true;
|
||||
savedUsers = settings.users;
|
||||
savedRequireAuthentication = settings.requireAuthentication;
|
||||
socket = await adminSocket();
|
||||
settings.gdprAuthorErasure.enabled = true;
|
||||
setupCompleted = true;
|
||||
|
||||
const probe = await adminSocketWithProbe(PROBE_BUDGET_MS);
|
||||
if (!probe.ok) {
|
||||
console.warn(
|
||||
`[anonymizeAuthorSocket] admin socket probe failed (${probe.reason}); ` +
|
||||
'skipping suite — likely an authenticate-hook plugin (e.g. ep_hash_auth) ' +
|
||||
'rejecting the test\'s plain-text admin credentials. Tracked in #7795.');
|
||||
this.skip();
|
||||
return;
|
||||
}
|
||||
socket = probe.socket;
|
||||
});
|
||||
|
||||
after(function () {
|
||||
if (socket) socket.disconnect();
|
||||
// before() may have called this.skip() before capturing backups (e.g.
|
||||
// a common.init() failure), so guard against writing undefined into
|
||||
// settings. Once setupCompleted flips true the backup variables are
|
||||
// safe to read.
|
||||
if (!setupCompleted) return;
|
||||
settings.gdprAuthorErasure.enabled = originalFlag;
|
||||
// savedUsers and settings.users point at the same object — restoring
|
||||
// the reference is a no-op against the in-place mutation. Delete the
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue