mirror of
https://github.com/wekan/wekan.git
synced 2026-11-05 21:44:59 -05:00
Let the login-cookie refresh limit be raised, and stop browser tests signing themselves out.
Meteor answers more than 30 refreshes of the HttpOnly login cookie per 10 seconds from one client address with 429, and every page load is one. The whole browser suite comes from localhost, so a quick run signed its own test users out: 18-rtl-layout "list pages" landed on the sign-in page on its third full navigation in one of two runs with two workers. TODO Later recorded this on 2026-10-05. ACCOUNTS_COOKIE_REFRESH_RATE_LIMIT now sets the allowance per 10 seconds; anything but a positive whole number keeps Meteor's default, and the limiter is never switched off. The browser test server sets it to 100000. It also helps an install where many users reach WeKan through a proxy that is not trusted for the client address and so share one address; documented beside the login settings. loginWithToken additionally waits out a 429 on the cookie refresh and resumes once, and only for that. tests/cookieRefreshRateLimit.test.cjs runs the parser with valid and invalid values and pins the server, build.sh, docs and helper wiring. Thanks to xet7 !
This commit is contained in:
1 parent
4ba3e10bb0
commit
63a7fba465
5 files changed
+108
-7
No files matched your search
@@ -1373,6 +1373,10 @@ function run_all_tests(){
|
||||
# The Docker and Snap default, so the browser tests see the framing policy
|
||||
# (FrameBleed) the way a real install sends it.
|
||||
export BROWSER_POLICY_ENABLED=true
|
||||
# Every page load refreshes the login cookie, and Meteor allows 30 per 10
|
||||
# seconds per address; the whole browser suite comes from localhost, so a
|
||||
# quick run signed its own test users out (server/accounts-common.js).
|
||||
export ACCOUNTS_COOKIE_REFRESH_RATE_LIMIT="${ACCOUNTS_COOKIE_REFRESH_RATE_LIMIT:-100000}"
|
||||
if [ "${WEKAN_TEST_SERVER_MODE:-bundle}" = source ]; then
|
||||
export WEKAN_PLAYWRIGHT_PROBE=0
|
||||
local WRITABLE_ABS="$WEKAN_DIR/.tools/test-writable"
|
||||
|
||||
@@ -27,6 +27,13 @@ Under them, the Login pane's own environment variables, each overridable:
|
||||
`PASSWORD_LOGIN_ENABLED`, `DEFAULT_AUTHENTICATION_METHOD` and
|
||||
`ACCOUNTS_COMMON_LOGIN_EXPIRATION_IN_DAYS` (applied when WeKan starts).
|
||||
|
||||
`ACCOUNTS_COOKIE_REFRESH_RATE_LIMIT` is environment-only and also applied when
|
||||
WeKan starts: how many login-cookie refreshes one client address may make per
|
||||
10 seconds (Meteor's default is 30; every page load is one). Raise it when many
|
||||
users reach WeKan through a proxy that WeKan does not trust for the client
|
||||
address (`HTTP_FORWARDED_COUNT`), because they then share one address. The
|
||||
browser tests set it, as they all come from localhost.
|
||||
|
||||
## Provider settings
|
||||
|
||||
Every login environment variable can be overridden in the Admin Panel, in the
|
||||
|
||||
@@ -10,6 +10,20 @@ import { authEnv, authConfigReady } from '/server/lib/authConfig';
|
||||
// change applies from the next start; the cookie below uses the same value.
|
||||
let loginExpirationDays = 90;
|
||||
|
||||
// Meteor answers more than 30 refreshes of the HttpOnly login cookie per 10
|
||||
// seconds from one client address with 429, and every page load is one. That
|
||||
// is per ADDRESS: behind a reverse proxy that is not trusted for the client
|
||||
// address (HTTP_FORWARDED_COUNT), every user shares one, and the browser tests
|
||||
// all come from localhost, where a quick run signs itself out mid-test.
|
||||
// ACCOUNTS_COOKIE_REFRESH_RATE_LIMIT raises the per-10-seconds allowance; a
|
||||
// value that is not a positive whole number leaves Meteor's default.
|
||||
export function cookieRefreshRateLimit(value) {
|
||||
const max = Number(value);
|
||||
return Number.isSafeInteger(max) && max > 0
|
||||
? { httpOnlyCookieRateLimit: { max, windowMs: 10000 } }
|
||||
: {};
|
||||
}
|
||||
|
||||
Meteor.startup(async () => {
|
||||
await authConfigReady;
|
||||
loginExpirationDays = Number(authEnv('ACCOUNTS_COMMON_LOGIN_EXPIRATION_IN_DAYS')) || 90;
|
||||
@@ -24,6 +38,7 @@ Meteor.startup(async () => {
|
||||
// over a trillion values, and a code now lives 15 minutes.
|
||||
tokenSequenceLength: 10,
|
||||
loginTokenExpirationHours: 0.25,
|
||||
...cookieRefreshRateLimit(process.env.ACCOUNTS_COOKIE_REFRESH_RATE_LIMIT),
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
'use strict';
|
||||
// Meteor's HttpOnly login cookie refresh is limited to 30 per 10 seconds per
|
||||
// client address. ACCOUNTS_COOKIE_REFRESH_RATE_LIMIT raises it; the browser
|
||||
// test server sets it, because every spec comes from localhost and a quick run
|
||||
// signed its own users out (the 18-rtl-layout "list pages" flake).
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const vm = require('node:vm');
|
||||
|
||||
const root = path.resolve(__dirname, '..');
|
||||
const source = fs.readFileSync(path.join(root, 'server/accounts-common.js'), 'utf8');
|
||||
let passed = 0;
|
||||
function test(name, fn) { fn(); passed += 1; console.log(' ok -', name); }
|
||||
|
||||
const fnSource = source.match(/export function cookieRefreshRateLimit\(value\) \{[\s\S]*?\n\}/)[0]
|
||||
.replace('export function', 'function');
|
||||
const cookieRefreshRateLimit = vm.runInNewContext(`${fnSource}; cookieRefreshRateLimit`);
|
||||
|
||||
test('a positive whole number raises the per-10-seconds allowance', () => {
|
||||
assert.deepEqual(JSON.parse(JSON.stringify(cookieRefreshRateLimit('100000'))),
|
||||
{ httpOnlyCookieRateLimit: { max: 100000, windowMs: 10000 } });
|
||||
assert.deepEqual(JSON.parse(JSON.stringify(cookieRefreshRateLimit('31'))).httpOnlyCookieRateLimit.max, 31);
|
||||
});
|
||||
|
||||
test('negative: anything else leaves Meteor\'s default rather than disabling the limit', () => {
|
||||
for (const value of [undefined, '', '0', '-5', '1.5', 'abc', 'false', 'Infinity']) {
|
||||
assert.deepEqual(JSON.parse(JSON.stringify(cookieRefreshRateLimit(value))), {}, String(value));
|
||||
}
|
||||
});
|
||||
|
||||
test('Accounts.config receives it, and only from the server environment', () => {
|
||||
assert.match(source, /\.\.\.cookieRefreshRateLimit\(process\.env\.ACCOUNTS_COOKIE_REFRESH_RATE_LIMIT\),/);
|
||||
// false would switch Meteor's limiter off entirely; never pass it.
|
||||
assert.doesNotMatch(source, /httpOnlyCookieRateLimit:\s*false/);
|
||||
});
|
||||
|
||||
test('the browser test server raises it, and documentation names it', () => {
|
||||
const build = fs.readFileSync(path.join(root, 'build.sh'), 'utf8');
|
||||
assert.match(build, /export ACCOUNTS_COOKIE_REFRESH_RATE_LIMIT="\$\{ACCOUNTS_COOKIE_REFRESH_RATE_LIMIT:-100000\}"/);
|
||||
const docs = fs.readFileSync(path.join(root, 'docs/Features/Admin-Panel/People/Login.md'), 'utf8');
|
||||
assert.match(docs, /ACCOUNTS_COOKIE_REFRESH_RATE_LIMIT/);
|
||||
});
|
||||
|
||||
test('loginWithToken waits out a 429 on the cookie refresh, and only that', () => {
|
||||
const auth = fs.readFileSync(path.join(root, 'tests/playwright/helpers/auth.js'), 'utf8');
|
||||
assert.match(auth, /response\.status\(\) === 429 && response\.url\(\)\.includes\('\/_accounts\/cookie\/refresh'\)/);
|
||||
assert.match(auth, /if \(!throttled\) throw error;/);
|
||||
});
|
||||
|
||||
console.log(`\ncookieRefreshRateLimit: all ${passed} tests passed`);
|
||||
@@ -70,13 +70,37 @@ async function loginWithToken(page, userId, token) {
|
||||
.evaluate(() => typeof Meteor !== 'undefined' && typeof Meteor.subscribe === 'function')
|
||||
.catch(() => false);
|
||||
if (!onLoadedApp) {
|
||||
await page.goto(`${BASE_URL}/sign-in`, { waitUntil: 'commit' });
|
||||
await waitForMeteor(page);
|
||||
// Cookie retrieval precedes Accounts.loggingIn(). Wait for the resume itself,
|
||||
// rather than interpreting that initial false value as a finished login.
|
||||
await page.waitForFunction(
|
||||
id => Meteor.userId() === id && !Meteor.loggingIn(), userId, { timeout: 30_000 },
|
||||
);
|
||||
// Meteor answers more than 30 refreshes of the HttpOnly login cookie per 10
|
||||
// seconds from one address with 429, and every test runs from localhost, so
|
||||
// a run that switches users quickly can hit it: the resume then never lands
|
||||
// and the page stays on sign-in. Notice exactly that answer, and only after
|
||||
// it wait the window out and load once more; any other failure still fails.
|
||||
let throttled = false;
|
||||
const onResponse = response => {
|
||||
if (response.status() === 429 && response.url().includes('/_accounts/cookie/refresh')) throttled = true;
|
||||
};
|
||||
// Real pages always have events; the unit tests' stand-in page does not.
|
||||
const listens = typeof page.on === 'function' && typeof page.off === 'function';
|
||||
if (listens) page.on('response', onResponse);
|
||||
const resume = async () => {
|
||||
await page.goto(`${BASE_URL}/sign-in`, { waitUntil: 'commit' });
|
||||
await waitForMeteor(page);
|
||||
// Cookie retrieval precedes Accounts.loggingIn(). Wait for the resume itself,
|
||||
// rather than interpreting that initial false value as a finished login.
|
||||
await page.waitForFunction(
|
||||
id => Meteor.userId() === id && !Meteor.loggingIn(), userId, { timeout: 30_000 },
|
||||
);
|
||||
};
|
||||
try {
|
||||
await resume();
|
||||
} catch (error) {
|
||||
if (!throttled) throw error;
|
||||
throttled = false;
|
||||
await page.waitForTimeout(11_000);
|
||||
await resume();
|
||||
} finally {
|
||||
if (listens) page.off('response', onResponse);
|
||||
}
|
||||
}
|
||||
|
||||
// Wait for any stored-session resume that is still in flight to finish, so the
|
||||
|
||||
Reference in new issue
Block a user