Aug 26 - Update password detect against off field 2
CI / Python lint (flake8) (push) Has been cancelled
CI / Python syntax check (push) Has been cancelled
CI / Alembic migration chain (push) Has been cancelled
CI / JavaScript syntax check (push) Has been cancelled
CI / Pytest (push) Has been cancelled
CI / Build extension zip (push) Has been cancelled
CI / Python lint (flake8) (push) Has been cancelled
CI / Python syntax check (push) Has been cancelled
CI / Alembic migration chain (push) Has been cancelled
CI / JavaScript syntax check (push) Has been cancelled
CI / Pytest (push) Has been cancelled
CI / Build extension zip (push) Has been cancelled
This commit is contained in:
@@ -112,6 +112,64 @@ check('click fallback registered', /addEventListener\(\s*"click"/.test(SRC), tru
|
||||
check('keydown fallback registered', /addEventListener\(\s*"keydown"/.test(SRC), true);
|
||||
check('submit listener retained', /addEventListener\(\s*"submit"/.test(SRC), true);
|
||||
|
||||
// ── _isTrustworthyOrigin: where credentials may be filled ──────────────────
|
||||
// Extracted from the real source and evaluated against a stubbed `location`,
|
||||
// so this exercises the shipped function rather than a copy of its rules.
|
||||
{
|
||||
const fnSrc = SRC.match(
|
||||
/function _isTrustworthyOrigin\(\) \{[\s\S]*?\n \}/,
|
||||
);
|
||||
if (!fnSrc) {
|
||||
failures++;
|
||||
console.log('FAIL could not extract _isTrustworthyOrigin from content.js');
|
||||
} else {
|
||||
const make = new Function(
|
||||
'location',
|
||||
`${fnSrc[0]}; return _isTrustworthyOrigin();`,
|
||||
);
|
||||
const at = (protocol, hostname) => make({ protocol, hostname });
|
||||
|
||||
// HTTPS is always fine.
|
||||
check('https is trustworthy', at('https:', 'example.com'), true);
|
||||
|
||||
// Local devices over plain HTTP — routers, NAS, printers. Dropping
|
||||
// http://*/* entirely would break exactly these.
|
||||
for (const host of ['localhost', '127.0.0.1', '::1', 'router.local',
|
||||
'10.0.0.1', '192.168.1.1', '172.16.5.4', '172.31.0.1',
|
||||
'169.254.1.1', 'nas.lan', 'box.home']) {
|
||||
check(`http://${host} is treated as local`, at('http:', host), true);
|
||||
}
|
||||
|
||||
// Plaintext on the public internet must warn.
|
||||
for (const host of ['example.com', 'bank.co.uk', '8.8.8.8',
|
||||
'172.15.0.1', '172.32.0.1', '11.0.0.1',
|
||||
'192.169.1.1', 'evil-localhost.com',
|
||||
'localhost.evil.com', '127.0.0.1.evil.com']) {
|
||||
check(`http://${host} is NOT trusted`, at('http:', host), false);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ── Autologin must not click a control that discards the login ────────────
|
||||
{
|
||||
const m = SRC.match(/var _NEGATIVE_CONTROL = (\/[^\n]+\/[a-z]*);/);
|
||||
if (!m) {
|
||||
failures++;
|
||||
console.log('FAIL could not find _NEGATIVE_CONTROL in content.js');
|
||||
} else {
|
||||
// eslint-disable-next-line no-eval
|
||||
const NEG = eval(m[1]);
|
||||
for (const label of ['Cancel', 'Reset', 'Go back', 'Forgot password?',
|
||||
'Register', 'Sign up', 'Create account']) {
|
||||
check(`autologin skips "${label}"`, NEG.test(label), true);
|
||||
}
|
||||
for (const label of ['Sign In', 'Log in', 'Submit', 'Continue', 'OK']) {
|
||||
check(`autologin allows "${label}"`, NEG.test(label), false);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ── Summary (must stay last so every block above is counted) ───────────────
|
||||
if (failures) {
|
||||
console.log(`\n${failures} failure(s)`);
|
||||
process.exit(1);
|
||||
|
||||
@@ -0,0 +1,168 @@
|
||||
"""
|
||||
Regression tests for the account lockout (two related problems).
|
||||
|
||||
1. Disclosure — a locked account answered 429 "Account temporarily locked. Try
|
||||
again in N minute(s)", confirming the address had an account. That is the same
|
||||
leak /register was fixed for, reachable by anyone willing to send five wrong
|
||||
passwords.
|
||||
|
||||
2. Denial of service — the lockout was global to the account, so anyone who knew
|
||||
an address could lock the real owner out for 15 minutes, repeatedly and
|
||||
indefinitely. Locking someone out of their password manager is a serious harm
|
||||
on its own.
|
||||
|
||||
Lockout is now scoped to (account, source IP): an attacker locks out only
|
||||
themselves, and every failure mode returns one identical response.
|
||||
"""
|
||||
from app import db
|
||||
from app.models.audit_log import AuditLog
|
||||
from app.models.login_attempt import LoginAttempt
|
||||
from app.models.user import User
|
||||
from tests.conftest import make_user, register
|
||||
|
||||
ATTACKER = '203.0.113.9'
|
||||
OWNER = '198.51.100.4'
|
||||
|
||||
|
||||
def _login(client, ip, email='user@example.com', auth_hash='AUTH-HASH-V1'):
|
||||
# ProxyFix(x_for=1) makes the rightmost XFF hop the client address, which is
|
||||
# what client_ip() and the lockout key off.
|
||||
return client.post('/api/auth/login',
|
||||
json={'email': email, 'auth_hash': auth_hash},
|
||||
headers={'X-Forwarded-For': ip})
|
||||
|
||||
|
||||
def _lock_out(client, ip, email='user@example.com'):
|
||||
for _ in range(LoginAttempt.MAX_FAILED):
|
||||
_login(client, ip, email, 'WRONG-HASH')
|
||||
|
||||
|
||||
# ── Disclosure ──────────────────────────────────────────────────────────────
|
||||
|
||||
def test_locked_response_matches_wrong_password(client, app):
|
||||
make_user(client)
|
||||
_lock_out(client, ATTACKER)
|
||||
|
||||
locked = _login(client, ATTACKER, auth_hash='WRONG-HASH')
|
||||
wrong_on_fresh_ip = _login(client, '203.0.113.77', auth_hash='WRONG-HASH')
|
||||
|
||||
assert locked.status_code == wrong_on_fresh_ip.status_code == 401
|
||||
assert locked.get_json() == wrong_on_fresh_ip.get_json(), (
|
||||
'the locked response is distinguishable, so it discloses the account'
|
||||
)
|
||||
|
||||
|
||||
def test_locked_response_matches_unknown_account(client, app):
|
||||
make_user(client)
|
||||
_lock_out(client, ATTACKER)
|
||||
|
||||
locked = _login(client, ATTACKER, auth_hash='WRONG-HASH')
|
||||
unknown = _login(client, ATTACKER, 'nobody@example.com', 'WRONG-HASH')
|
||||
|
||||
assert locked.status_code == unknown.status_code == 401
|
||||
assert locked.get_json() == unknown.get_json()
|
||||
|
||||
|
||||
def test_response_never_names_the_lockout(client, app):
|
||||
make_user(client)
|
||||
_lock_out(client, ATTACKER)
|
||||
body = _login(client, ATTACKER, auth_hash='WRONG-HASH').get_json()
|
||||
text = ' '.join(str(v) for v in body.values()).lower()
|
||||
|
||||
for leak in ('locked', 'lockout', 'minute(s) remaining', 'too many'):
|
||||
assert leak not in text, f'response leaks lockout state via {leak!r}: {body}'
|
||||
|
||||
|
||||
# ── Denial of service ───────────────────────────────────────────────────────
|
||||
|
||||
def test_attacker_cannot_lock_the_owner_out(client, app):
|
||||
"""The core DoS fix: the victim's own IP is untouched."""
|
||||
make_user(client)
|
||||
_lock_out(client, ATTACKER)
|
||||
|
||||
assert _login(client, ATTACKER, auth_hash='WRONG-HASH').status_code == 401
|
||||
res = _login(client, OWNER)
|
||||
assert res.status_code == 200, 'the owner was locked out by someone else'
|
||||
assert 'access_token' in res.get_json()
|
||||
|
||||
|
||||
def test_lockout_actually_applies_to_the_offending_ip(client, app):
|
||||
"""The DoS fix must not have removed brute-force protection."""
|
||||
make_user(client)
|
||||
_lock_out(client, ATTACKER)
|
||||
|
||||
# Even the CORRECT password is refused from a locked-out IP.
|
||||
assert _login(client, ATTACKER).status_code == 401
|
||||
|
||||
row = LoginAttempt.query.filter_by(ip_address=ATTACKER).first()
|
||||
assert row is not None and row.locked_until is not None
|
||||
|
||||
|
||||
def test_each_ip_gets_its_own_budget(client, app):
|
||||
make_user(client)
|
||||
_lock_out(client, ATTACKER)
|
||||
|
||||
# A second IP is still four failures away from its own lockout.
|
||||
for _ in range(LoginAttempt.MAX_FAILED - 1):
|
||||
_login(client, '203.0.113.55', auth_hash='WRONG-HASH')
|
||||
assert _login(client, '203.0.113.55').status_code == 200
|
||||
|
||||
assert LoginAttempt.query.filter_by(ip_address=ATTACKER).first().locked_until
|
||||
|
||||
|
||||
def test_successful_login_clears_that_ips_history(client, app):
|
||||
make_user(client)
|
||||
for _ in range(LoginAttempt.MAX_FAILED - 1):
|
||||
_login(client, OWNER, auth_hash='WRONG-HASH')
|
||||
|
||||
assert _login(client, OWNER).status_code == 200
|
||||
assert LoginAttempt.query.filter_by(ip_address=OWNER).first() is None, (
|
||||
'failure history survived a successful login, so the next typo locks out'
|
||||
)
|
||||
|
||||
|
||||
# ── Bookkeeping ─────────────────────────────────────────────────────────────
|
||||
|
||||
def test_lockout_is_audited(client, app):
|
||||
make_user(client)
|
||||
_lock_out(client, ATTACKER)
|
||||
|
||||
entry = (AuditLog.query.filter_by(action='auth.account_locked')
|
||||
.order_by(AuditLog.id.desc()).first())
|
||||
assert entry is not None
|
||||
assert entry.ip_address == ATTACKER
|
||||
|
||||
_login(client, ATTACKER, auth_hash='WRONG-HASH')
|
||||
blocked = (AuditLog.query.filter_by(action='auth.login_blocked')
|
||||
.order_by(AuditLog.id.desc()).first())
|
||||
assert blocked is not None, 'blocked attempts are not recorded'
|
||||
|
||||
|
||||
def test_aggregate_counter_still_tracks_all_ips(client, app):
|
||||
"""users.failed_login_count no longer gates login but stays informative."""
|
||||
make_user(client)
|
||||
_login(client, ATTACKER, auth_hash='WRONG-HASH')
|
||||
_login(client, '203.0.113.55', auth_hash='WRONG-HASH')
|
||||
|
||||
assert User.query.filter_by(email='user@example.com').first().failed_login_count == 2
|
||||
|
||||
|
||||
def test_cleanup_prunes_stale_rows(client, app):
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
make_user(client)
|
||||
_login(client, ATTACKER, auth_hash='WRONG-HASH')
|
||||
row = LoginAttempt.query.filter_by(ip_address=ATTACKER).first()
|
||||
row.updated_at = (datetime.now(timezone.utc).replace(tzinfo=None)
|
||||
- timedelta(hours=LoginAttempt.RETENTION_HOURS + 1))
|
||||
db.session.commit()
|
||||
|
||||
assert LoginAttempt.cleanup_expired() == 1
|
||||
db.session.commit()
|
||||
assert LoginAttempt.query.filter_by(ip_address=ATTACKER).first() is None
|
||||
|
||||
|
||||
def test_unknown_account_creates_no_rows(client, app):
|
||||
"""No user, nothing to track — must not be a way to grow the table."""
|
||||
_login(client, ATTACKER, 'nobody@example.com', 'WRONG-HASH')
|
||||
assert LoginAttempt.query.count() == 0
|
||||
Reference in New Issue
Block a user