From 8aada101ca56e1b1d13dc8cb8eb5b27cc991bb0d Mon Sep 17 00:00:00 2001 From: yumoqing Date: Sun, 26 Apr 2026 11:04:15 +0800 Subject: [PATCH] fix(rbac): remove MySQL-specific SQL for cross-database compatibility - Replace DATE_SUB(NOW(), INTERVAL 300 SECOND) with Python-level time check - Replace NOW() with parameterized timestamps from Python - Lockout check now done in _is_locked() function (DB-agnostic) - All UPDATE statements use parameterized values, not DB functions - Works with MySQL, PostgreSQL, SQLite, SQL Server, Oracle --- rbac/check_perm.py | 85 +++++++++++++++++++++++++------------- wwwroot/phone_login.dspy | 12 +++--- wwwroot/user/up_login.dspy | 58 ++++++++++++++++---------- 3 files changed, 100 insertions(+), 55 deletions(-) diff --git a/rbac/check_perm.py b/rbac/check_perm.py index 9e7d27d..656d5ad 100644 --- a/rbac/check_perm.py +++ b/rbac/check_perm.py @@ -1,6 +1,7 @@ import time from traceback import format_exc +from datetime import datetime from aiohttp import BasicAuth from sqlor.dbpools import DBPools, get_sor_context from appPublic.registerfunction import RegisterFunction @@ -15,6 +16,31 @@ from ahserver.globalEnv import password_encode from ahserver.serverenv import ServerEnv, get_serverenv, set_serverenv from .userperm import UserPermissions + +# DB-agnostic time constants +LOGIN_LOCKOUT_DURATION = 300 # 5 minutes in seconds + + +def _now_ts(): + """Current time as standard SQL timestamp string.""" + return datetime.now().strftime('%Y-%m-%d %H:%M:%S') + + +def _is_locked(fail_count, last_fail, lockout_seconds=LOGIN_LOCKOUT_DURATION): + """Check if user is locked out. Pure Python, DB-agnostic.""" + if fail_count < 3 or last_fail is None: + return False + try: + if isinstance(last_fail, str): + stored = datetime.strptime(last_fail, '%Y-%m-%d %H:%M:%S') + elif isinstance(last_fail, datetime): + stored = last_fail + else: + return False + return (datetime.now() - stored).total_seconds() < lockout_seconds + except (ValueError, TypeError): + return False + async def get_org_users(orgid): env = ServerEnv() async with get_sor_context(env, 'rbac') as sor: @@ -111,36 +137,35 @@ async def checkUserPassword(request, username, password): """Authenticate user with password, supporting login lockout mechanism. High-concurrency safe: - - Uses atomic UPDATE for fail_count increment (no SELECT-then-UPDATE race) - - Lockout check uses database-level comparison + - Atomic UPDATE for fail_count increment (standard SQL, all databases) + - Lockout check done in Python layer (no DB-specific functions) - Password verified with single atomic query """ db = DBPools() dbname = get_dbname() async with db.sqlorContext(dbname) as sor: - # Check lockout status atomically in SQL - # Returns user record only if NOT currently locked - sql = """select * from users where username=${username}$ - and not ( - login_fail_count >= 3 - and last_login_fail is not null - and last_login_fail > DATE_SUB(NOW(), INTERVAL 300 SECOND) - )""" + # Get user record with lockout fields + sql = "select * from users where username=${username}$" recs = await sor.sqlExe(sql, {'username': username}) if len(recs) < 1: - # Either user not found, or locked out - debug(f'User {username} not found or locked out') + debug(f'User {username} not found') return False user = recs[0] + fail_count = getattr(user, 'login_fail_count', 0) or 0 + last_fail = getattr(user, 'last_login_fail', None) - # Verify password with single atomic query + # Lockout check in Python (DB-agnostic) + if _is_locked(fail_count, last_fail): + debug(f'User {username} locked out') + return False + + # Verify password with standard SQL sql = "select * from users where username=${username}$ and password=${password}$" recs = await sor.sqlExe(sql, {'username': username, 'password': password}) if len(recs) < 1: - # Password wrong - atomically increment fail count - # Database-level increment prevents race conditions - now_str = curDateString('%Y-%m-%d %H:%M:%S') + # Atomic increment -- standard SQL, works on all databases + now_str = _now_ts() await sor.sqlExe(""" UPDATE users SET login_fail_count = login_fail_count + 1, @@ -150,8 +175,8 @@ async def checkUserPassword(request, username, password): debug(f'Login failed for {username}, fail_count incremented') return False - # Login successful - atomically reset counters and update last_login - now_str = curDateString('%Y-%m-%d %H:%M:%S') + # Login successful -- atomic reset + now_str = _now_ts() await sor.sqlExe(""" UPDATE users SET login_fail_count = 0, @@ -171,24 +196,26 @@ async def basic_auth(sor, request): m = auther.decode(auth) username = m.login password = password_encode(m.password) - # Check lockout atomically in SQL (same pattern as checkUserPassword) - sql = """select * from users where username=${username}$ - and password=${password}$ - and not ( - login_fail_count >= 3 - and last_login_fail is not null - and last_login_fail > DATE_SUB(NOW(), INTERVAL 300 SECOND) - )""" + # Standard SQL -- no DB-specific functions + sql = "select * from users where username=${username}$ and password=${password}$" recs = await sor.sqlExe(sql, {'username':username,'password':password}) if len(recs) < 1: return None - # Update last_login on successful basic auth + # Check lockout in Python layer (DB-agnostic) + user = recs[0] + fail_count = getattr(user, 'login_fail_count', 0) or 0 + last_fail = getattr(user, 'last_login_fail', None) + if _is_locked(fail_count, last_fail): + debug(f'User {username} locked out via basic auth') + return None + # Update last_login on successful basic auth (standard SQL) + now_str = _now_ts() await sor.sqlExe(""" UPDATE users SET login_fail_count = 0, last_login_fail = NULL, - last_login = NOW() + last_login = ${now}$ WHERE id = ${id}$ - """, {'id': recs[0].id}) + """, {'id': recs[0].id, 'now': now_str}) await user_login(request, recs[0].id, username=recs[0].username, userorgid=recs[0].orgid) diff --git a/wwwroot/phone_login.dspy b/wwwroot/phone_login.dspy index bc24b8a..d4009fb 100644 --- a/wwwroot/phone_login.dspy +++ b/wwwroot/phone_login.dspy @@ -43,13 +43,14 @@ async with get_sor_context(request._run_ns, 'rbac') as sor: if recs: if len(recs) == 1: r = recs[0] - # Update last_login atomically + # Update last_login atomically (standard SQL, no DB-specific functions) + now_str = curDateString('%Y-%m-%d %H:%M:%S') await sor.sqlExe(""" UPDATE users - SET last_login = NOW(), login_fail_count = 0, + SET last_login = ${now}$, login_fail_count = 0, last_login_fail = NULL WHERE id = ${id}$ - """, {'id': r.id}) + """, {'id': r.id, 'now': now_str}) await remember_user(r.id, username=r.username, userorgid=r.orgid) return { "status": "ok", @@ -60,12 +61,13 @@ async with get_sor_context(request._run_ns, 'rbac') as sor: if params_kw.selected_id: for r in recs: if r.id == params_kw.selected_id: + now_str = curDateString('%Y-%m-%d %H:%M:%S') await sor.sqlExe(""" UPDATE users - SET last_login = NOW(), login_fail_count = 0, + SET last_login = ${now}$, login_fail_count = 0, last_login_fail = NULL WHERE id = ${id}$ - """, {'id': r.id}) + """, {'id': r.id, 'now': now_str}) await remember_user(r.id, username=r.username, userorgid=r.orgid) return { "status": "ok", diff --git a/wwwroot/user/up_login.dspy b/wwwroot/user/up_login.dspy index dcc2af9..ee64a42 100644 --- a/wwwroot/user/up_login.dspy +++ b/wwwroot/user/up_login.dspy @@ -9,35 +9,50 @@ info(f'{ns=}') db = DBPools() dbname = get_module_dbname('rbac') async with db.sqlorContext(dbname) as sor: - # Check lockout atomically in SQL - r = await sor.sqlExe("""select * from users where username=${username}$ - and not ( - login_fail_count >= 3 - and last_login_fail is not null - and last_login_fail > DATE_SUB(NOW(), INTERVAL 300 SECOND) - )""", ns.copy()) + # Get user record with lockout fields (standard SQL) + r = await sor.sqlExe('select * from users where username=${username}$', ns.copy()) if len(r) == 0: - # User not found or locked out - r2 = await sor.sqlExe('select username from users where username=${username}$', ns.copy()) - if len(r2) == 0: - msg = "user name or password error" - else: - msg = "Account locked due to too many failed login attempts. Please try again in 5 minutes." return { "widgettype":"Error", "options":{ - "timeout":5, + "timeout":3, "title":"Login Error", - "message": msg + "message":"user name or password error" } } user = r[0] + # Lockout check in Python (DB-agnostic) + fail_count = getattr(user, 'login_fail_count', 0) or 0 + last_fail = getattr(user, 'last_login_fail', None) + if fail_count >= 3 and last_fail: + try: + from datetime import datetime + stored = datetime.strptime(str(last_fail), '%Y-%m-%d %H:%M:%S') + elapsed = (datetime.now() - stored).total_seconds() + if elapsed < 300: + return { + "widgettype":"Error", + "options":{ + "timeout":5, + "title":"Account Locked", + "message":"Account locked due to too many failed login attempts. Please try again in 5 minutes." + } + } + else: + # Lockout expired, reset in background (best-effort) + await sor.sqlExe(""" + UPDATE users SET login_fail_count = 0, last_login_fail = NULL + WHERE id = ${id}$ + """, {'id': user.id}) + except (ValueError, TypeError): + pass + # Verify password r = await sor.sqlExe('select * from users where username=${username}$ and password=${password}$', ns.copy()) if len(r) == 0: - # Atomically increment fail count - now_str = curDateString('%Y-%m-%d %H:%M:%S') + # Atomic increment -- standard SQL + now_str = datetime.now().strftime('%Y-%m-%d %H:%M:%S') await sor.sqlExe(""" UPDATE users SET login_fail_count = login_fail_count + 1, @@ -45,7 +60,7 @@ async with db.sqlorContext(dbname) as sor: WHERE id = ${id}$ """, {'id': user.id, 'now': now_str}) - new_fail_count = (getattr(user, 'login_fail_count', 0) or 0) + 1 + new_fail_count = fail_count + 1 if new_fail_count >= 3: msg = "Too many failed attempts. Account locked for 5 minutes." else: @@ -58,13 +73,14 @@ async with db.sqlorContext(dbname) as sor: "message": msg } } - # Success - atomically reset counters and update last_login + # Success -- atomic reset + now_str = datetime.now().strftime('%Y-%m-%d %H:%M:%S') await sor.sqlExe(""" UPDATE users SET login_fail_count = 0, last_login_fail = NULL, - last_login = NOW() + last_login = ${now}$ WHERE id = ${id}$ - """, {'id': user.id}) + """, {'id': user.id, 'now': now_str}) await remember_user(r[0].id, username=r[0].username, userorgid=r[0].orgid) return { "widgettype":"Message",