fix: prevent data loss in update_permissions and token_refresh
- update_permissions: swap to upsert-first-then-delete-stale so a failed upsert mid-loop rolls back cleanly, leaving the user's permissions intact. Adds Q_DELETE_PERMISSION_SUBSYSTEM for targeted removal. - auth_refresh: generate and persist the new refresh token before blacklisting/clearing the old one, so a failure in generate_tokens doesn't leave the user locked out with no valid refresh token.
This commit is contained in:
@@ -184,13 +184,17 @@ def auth_refresh(_request: Any, body: Any) -> dict[str, Any]:
|
|||||||
if user is None:
|
if user is None:
|
||||||
raise ValueError("User not found")
|
raise ValueError("User not found")
|
||||||
|
|
||||||
|
# Persist new tokens first, then invalidate the old ones.
|
||||||
|
# This prevents data loss if generate_tokens fails mid-way:
|
||||||
|
# the old refresh token remains valid and the user is not locked out.
|
||||||
|
permissions = user["permissions"]
|
||||||
|
tokens = generate_tokens(username, permissions)
|
||||||
|
|
||||||
jti = payload.get("jti")
|
jti = payload.get("jti")
|
||||||
if jti:
|
if jti:
|
||||||
blacklist_token(jti, token_type="refresh")
|
blacklist_token(jti, token_type="refresh")
|
||||||
if username:
|
if username:
|
||||||
_clear_refresh_token_after_rotation(username)
|
_clear_refresh_token_after_rotation(username)
|
||||||
permissions = user["permissions"]
|
|
||||||
tokens = generate_tokens(username, permissions)
|
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"tokens": tokens,
|
"tokens": tokens,
|
||||||
|
|||||||
+12
-2
@@ -16,7 +16,7 @@ from lib.auth import (
|
|||||||
rotate_user_secret,
|
rotate_user_secret,
|
||||||
)
|
)
|
||||||
from lib.db import (
|
from lib.db import (
|
||||||
Q_DELETE_PERMISSIONS,
|
Q_DELETE_PERMISSION_SUBSYSTEM,
|
||||||
Q_DELETE_USER,
|
Q_DELETE_USER,
|
||||||
Q_INSERT_USER,
|
Q_INSERT_USER,
|
||||||
Q_SELECT_PERMISSIONS,
|
Q_SELECT_PERMISSIONS,
|
||||||
@@ -230,9 +230,19 @@ def update_permissions(username: str, permissions: dict[str, str]) -> None:
|
|||||||
|
|
||||||
db = get_db()
|
db = get_db()
|
||||||
with db.in_transaction() as tx:
|
with db.in_transaction() as tx:
|
||||||
tx.run(Q_DELETE_PERMISSIONS, (username,))
|
# Upsert all new permissions first, then remove stale ones.
|
||||||
|
# This order ensures that if an upsert fails mid-loop, the user's
|
||||||
|
# permissions remain intact (transaction rolls back) rather than
|
||||||
|
# being permanently wiped.
|
||||||
|
existing = {
|
||||||
|
row["subsystem"]: row["level"]
|
||||||
|
for row in db.query(Q_SELECT_PERMISSIONS, (username,))
|
||||||
|
}
|
||||||
for subsystem, level in permissions.items():
|
for subsystem, level in permissions.items():
|
||||||
tx.run(Q_UPSERT_PERMISSION, (username, subsystem, level))
|
tx.run(Q_UPSERT_PERMISSION, (username, subsystem, level))
|
||||||
|
for subsystem in existing:
|
||||||
|
if subsystem not in permissions:
|
||||||
|
tx.run(Q_DELETE_PERMISSION_SUBSYSTEM, (username, subsystem))
|
||||||
blacklist_active_refresh_token(username)
|
blacklist_active_refresh_token(username)
|
||||||
rotate_user_secret(username)
|
rotate_user_secret(username)
|
||||||
|
|
||||||
|
|||||||
@@ -40,6 +40,7 @@ Q_DELETE_USER = "delete_user"
|
|||||||
Q_UPSERT_PERMISSION = "upsert_permission"
|
Q_UPSERT_PERMISSION = "upsert_permission"
|
||||||
Q_SELECT_PERMISSIONS = "select_permissions"
|
Q_SELECT_PERMISSIONS = "select_permissions"
|
||||||
Q_DELETE_PERMISSIONS = "delete_permissions"
|
Q_DELETE_PERMISSIONS = "delete_permissions"
|
||||||
|
Q_DELETE_PERMISSION_SUBSYSTEM = "delete_permission_subsystem"
|
||||||
Q_INSERT_BLACKLIST = "insert_blacklist"
|
Q_INSERT_BLACKLIST = "insert_blacklist"
|
||||||
Q_SELECT_BLACKLIST = "select_blacklist_jti"
|
Q_SELECT_BLACKLIST = "select_blacklist_jti"
|
||||||
Q_DELETE_EXPIRED_BLACKLIST = "delete_expired_blacklist"
|
Q_DELETE_EXPIRED_BLACKLIST = "delete_expired_blacklist"
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ from typing import Any, ClassVar
|
|||||||
|
|
||||||
from lib.db import (
|
from lib.db import (
|
||||||
Q_DELETE_EXPIRED_BLACKLIST,
|
Q_DELETE_EXPIRED_BLACKLIST,
|
||||||
|
Q_DELETE_PERMISSION_SUBSYSTEM,
|
||||||
Q_DELETE_PERMISSIONS,
|
Q_DELETE_PERMISSIONS,
|
||||||
Q_DELETE_REFRESH_TOKEN,
|
Q_DELETE_REFRESH_TOKEN,
|
||||||
Q_DELETE_USER,
|
Q_DELETE_USER,
|
||||||
@@ -88,6 +89,9 @@ class SQLiteBackend(Database):
|
|||||||
"SELECT subsystem, level FROM permissions WHERE username = ?"
|
"SELECT subsystem, level FROM permissions WHERE username = ?"
|
||||||
),
|
),
|
||||||
Q_DELETE_PERMISSIONS: ("DELETE FROM permissions WHERE username = ?"),
|
Q_DELETE_PERMISSIONS: ("DELETE FROM permissions WHERE username = ?"),
|
||||||
|
Q_DELETE_PERMISSION_SUBSYSTEM: (
|
||||||
|
"DELETE FROM permissions WHERE username = ? AND subsystem = ?"
|
||||||
|
),
|
||||||
# Token blacklist
|
# Token blacklist
|
||||||
Q_INSERT_BLACKLIST: (
|
Q_INSERT_BLACKLIST: (
|
||||||
"INSERT OR IGNORE INTO token_blacklist (jti, token_type, expires) VALUES (?, ?, ?)"
|
"INSERT OR IGNORE INTO token_blacklist (jti, token_type, expires) VALUES (?, ?, ?)"
|
||||||
|
|||||||
Reference in New Issue
Block a user