Skip to content

Commit 6731f8b

Browse files
authored
Merge pull request #103 from amar-python/feat/m2a2-login-lockout
feat(auth): lock accounts after repeated failed sign-ins (#102)
2 parents de01ed7 + d19bd1d commit 6731f8b

8 files changed

Lines changed: 115 additions & 8 deletions

File tree

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
"""Lock accounts after repeated failed sign-ins (#102, BR-36).
2+
3+
Shared-only, like 0004: users live in the control schema, not in each tenant.
4+
5+
Revision ID: 0005
6+
Revises: 0004
7+
Create Date: 2026-10-26
8+
"""
9+
10+
from alembic import context, op
11+
12+
from api.config import settings
13+
14+
revision = "0005"
15+
down_revision = "0004"
16+
branch_labels = None
17+
depends_on = None
18+
19+
20+
def _skip() -> bool:
21+
return context.config.attributes.get("scope") == "tenant"
22+
23+
24+
def _users() -> str:
25+
return op.get_bind().dialect.identifier_preparer.quote(settings.CONTROL_SCHEMA) + ".users"
26+
27+
28+
def upgrade() -> None:
29+
if _skip():
30+
return
31+
op.execute(f"""ALTER TABLE {_users()}
32+
ADD COLUMN failed_logins INTEGER NOT NULL DEFAULT 0,
33+
ADD COLUMN first_failed_at TIMESTAMPTZ,
34+
ADD COLUMN locked_until TIMESTAMPTZ""")
35+
36+
37+
def downgrade() -> None:
38+
if _skip():
39+
return
40+
op.execute(f"""ALTER TABLE {_users()}
41+
DROP COLUMN failed_logins, DROP COLUMN first_failed_at, DROP COLUMN locked_until""")

‎api/config.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ class Settings:
1616
CONTROL_SCHEMA: str = os.environ.get("CONTROL_SCHEMA", "control")
1717
# Lifetime of a sign-in session, in hours.
1818
SESSION_TTL_HOURS: int = int(os.environ.get("SESSION_TTL_HOURS", "12"))
19+
# #102: lock an account after this many failed sign-ins within the window,
20+
# for the same number of minutes.
21+
LOGIN_MAX_FAILURES: int = int(os.environ.get("LOGIN_MAX_FAILURES", "10"))
22+
LOGIN_LOCKOUT_MINUTES: int = int(os.environ.get("LOGIN_LOCKOUT_MINUTES", "15"))
1923

2024
# Schema holding the fixed T&E tables (deploy_all.sh dev creates te_dev).
2125
TE_SCHEMA: str = os.environ.get("TE_SCHEMA", "te_dev")

‎api/services/accounts.py‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,19 +68,25 @@ def add_membership(cur, user_id: int, tenant_id: int, role: str) -> None:
6868
def login(cur, tenant_slug: str, username: str, password: str) -> dict | None:
6969
"""Return a new session for an active member of the tenant, else None.
7070
71-
The caller commits. Wrong tenant, unknown user, wrong password and inactive
72-
user all return None, so a caller cannot tell which one failed.
71+
The caller commits, including on failure so failed attempts are counted.
72+
Wrong tenant, unknown user, wrong password, inactive user and locked
73+
account (#102) all return None, so a caller cannot tell which one failed.
7374
"""
7475
cur.execute(_q(
75-
"SELECT u.id, u.password_hash, u.is_active, t.id, m.role "
76+
"SELECT u.id, u.password_hash, u.is_active, t.id, m.role, "
77+
"COALESCE(u.locked_until > now(), false) "
7678
"FROM {c}.users u "
7779
"LEFT JOIN {c}.tenants t ON t.slug = %s "
7880
"LEFT JOIN {c}.memberships m ON m.user_id = u.id AND m.tenant_id = t.id "
7981
"WHERE u.username = %s"), (tenant_slug, username))
8082
row = cur.fetchone()
8183
ok = verify_password(password, row[1] if row else _DUMMY_HASH)
82-
if not (row and ok and row[2] and row[4]):
84+
if row and not ok:
85+
_record_failure(cur, row[0])
86+
if not (row and ok and row[2] and row[4]) or row[5]:
8387
return None
88+
cur.execute(_q("UPDATE {c}.users SET failed_logins = 0, first_failed_at = NULL, "
89+
"locked_until = NULL WHERE id = %s"), (row[0],))
8490
user_id, tenant_id, role = row[0], row[3], row[4]
8591
token = secrets.token_urlsafe(32)
8692
cur.execute(_q(
@@ -92,6 +98,21 @@ def login(cur, tenant_slug: str, username: str, password: str) -> dict | None:
9298
"tenant": tenant_slug, "role": role}
9399

94100

101+
def _record_failure(cur, user_id: int) -> None:
102+
"""Count a wrong password; lock the account once the window's limit is reached (#102)."""
103+
cur.execute(_q(
104+
"UPDATE {c}.users SET "
105+
"failed_logins = CASE WHEN first_failed_at > now() - make_interval(mins => %(m)s) "
106+
" THEN failed_logins + 1 ELSE 1 END, "
107+
"first_failed_at = CASE WHEN first_failed_at > now() - make_interval(mins => %(m)s) "
108+
" THEN first_failed_at ELSE now() END "
109+
"WHERE id = %(id)s RETURNING failed_logins"),
110+
{"m": settings.LOGIN_LOCKOUT_MINUTES, "id": user_id})
111+
if cur.fetchone()[0] >= settings.LOGIN_MAX_FAILURES:
112+
cur.execute(_q("UPDATE {c}.users SET locked_until = now() + make_interval(mins => %s) "
113+
"WHERE id = %s"), (settings.LOGIN_LOCKOUT_MINUTES, user_id))
114+
115+
95116
def session_for(cur, token: str) -> dict | None:
96117
"""The live session for a token, with its user, tenant and role, else None."""
97118
cur.execute(_q(

‎docs/ENV_VARIABLES.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ Read by `api/config.py`. Set these in the shell before running
4141
| `CSV_UPLOADS_SCHEMA` | `csv_uploads` | No | Schema where per-CSV tables and the file registry live. |
4242
| `CONTROL_SCHEMA` | `control` | No | Schema for tenants, users, memberships and sign-in sessions (M2, #92). |
4343
| `SESSION_TTL_HOURS` | `12` | No | How long a sign-in session lasts before the user must sign in again. |
44+
| `LOGIN_MAX_FAILURES` | `10` | No | Failed sign-ins within the window that lock an account (#102). |
45+
| `LOGIN_LOCKOUT_MINUTES` | `15` | No | Length of the failure window and of the lock. |
4446
| `TE_SCHEMA` | `te_dev` | No | Schema holding the 12 fixed T&E tables. Switch to `te_test`, `te_staging`, or `te_prod` per environment. |
4547
| `MAX_UPLOAD_BYTES` | `52428800` (50 MB) | No | Maximum file size accepted by the upload endpoint. |
4648

‎scripts/lint.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ PY_SRC=(
1818
alembic/versions/0002_registry_unique.py
1919
alembic/versions/0003_import_counts.py
2020
alembic/versions/0004_control_schema.py
21+
alembic/versions/0005_login_lockout.py
2122
api/auth.py
2223
api/services/accounts.py
2324
api/routers/auth_routes.py

‎tests/test_accounts.py‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@ def test_malformed_hash_fails_closed(self):
4040
self.assertFalse(accounts.verify_password(_PW, bad), bad)
4141

4242

43-
@pytest.mark.integration
44-
class SignIn(unittest.TestCase):
43+
class _Accounts(unittest.TestCase):
44+
"""Shared setup: one tenant with one uploader, plus an unrelated tenant."""
45+
4546
@classmethod
4647
def setUpClass(cls):
4748
cls.ctx = TestClient(app)
@@ -78,6 +79,9 @@ def _login(self, tenant=None, username=None, password=_PW):
7879
def _me(self, token: str):
7980
return self.client.get("/api/auth/me", headers={"Authorization": f"Bearer {token}"})
8081

82+
83+
@pytest.mark.integration
84+
class SignIn(_Accounts):
8185
def test_login_returns_a_session_for_the_tenant_and_role(self):
8286
res = self._login()
8387
self.assertEqual(res.status_code, 200, res.text)
@@ -133,3 +137,37 @@ def test_tokens_are_stored_only_as_hashes(self):
133137
stored = [r[0] for r in cur.fetchall()]
134138
self.assertEqual(len(stored), 1)
135139
self.assertNotIn(token, stored)
140+
141+
142+
@pytest.mark.integration
143+
class Lockout(_Accounts):
144+
"""Accounts lock after LOGIN_MAX_FAILURES wrong passwords in the window (#102)."""
145+
146+
def _fail(self, n: int) -> None:
147+
for _ in range(n):
148+
self.assertEqual(self._login(password="wrong").status_code, 401)
149+
150+
def test_eleventh_attempt_is_refused_even_with_the_right_password(self):
151+
self._fail(settings.LOGIN_MAX_FAILURES)
152+
res = self._login()
153+
self.assertEqual((res.status_code, res.json()["detail"]),
154+
(401, "Invalid tenant, username or password"))
155+
156+
def test_one_short_of_the_limit_still_signs_in_and_resets_the_count(self):
157+
self._fail(settings.LOGIN_MAX_FAILURES - 1)
158+
self.assertEqual(self._login().status_code, 200)
159+
self._fail(settings.LOGIN_MAX_FAILURES - 1)
160+
self.assertEqual(self._login().status_code, 200, "a success must reset the count")
161+
162+
def test_sign_in_works_again_after_the_lock_expires(self):
163+
self._fail(settings.LOGIN_MAX_FAILURES)
164+
self._exec("UPDATE {c}.users SET locked_until = now() - interval '1 second' WHERE id = %s",
165+
self.user_id)
166+
self.assertEqual(self._login().status_code, 200)
167+
168+
def test_failures_outside_the_window_start_a_new_count(self):
169+
self._fail(settings.LOGIN_MAX_FAILURES - 1)
170+
self._exec("UPDATE {c}.users SET first_failed_at = now() - make_interval(mins => %s) "
171+
"WHERE id = %s", settings.LOGIN_LOCKOUT_MINUTES + 1, self.user_id)
172+
self._fail(1)
173+
self.assertEqual(self._login().status_code, 200)

‎tests/test_migrations.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ def setUp(self):
148148
def test_bootstrap_brings_the_schema_to_head(self):
149149
with Conn() as conn, conn.cursor() as cur:
150150
cur.execute(f'SELECT version_num FROM "{settings.UPLOADS_SCHEMA}".alembic_version')
151-
self.assertEqual(cur.fetchone()[0], "0004")
151+
self.assertEqual(cur.fetchone()[0], "0005")
152152

153153
def test_duplicate_hash_is_rejected_by_the_database(self):
154154
insert = (f'INSERT INTO "{settings.UPLOADS_SCHEMA}".csv_files (file_name, file_hash, table_name, mode) '

‎tests/test_tenant_isolation.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ def test_tenant_schema_is_at_head_with_its_own_registry(self):
150150
with Conn() as conn, conn.cursor() as cur:
151151
cur.execute(sql.SQL("SELECT version_num FROM {}.alembic_version").format(
152152
sql.Identifier(schema_for(self.slugs["b"]))))
153-
self.assertEqual(cur.fetchone()[0], "0004")
153+
self.assertEqual(cur.fetchone()[0], "0005")
154154
cur.execute("SELECT table_name FROM information_schema.tables WHERE table_schema = %s",
155155
(schema_for(self.slugs["b"]),))
156156
self.assertTrue({"csv_files", "audit_log"} <= {r[0] for r in cur.fetchall()})

0 commit comments

Comments
 (0)