diff --git a/alembic/versions/0024_add_session_version.py b/alembic/versions/0024_add_session_version.py new file mode 100644 index 0000000..f3a9356 --- /dev/null +++ b/alembic/versions/0024_add_session_version.py @@ -0,0 +1,16 @@ +"""Add session_version to users for session invalidation on credential change.""" + +from alembic import op + +revision = "0024" +down_revision = "0023" + + +def upgrade() -> None: + op.execute( + "ALTER TABLE users ADD COLUMN IF NOT EXISTS session_version INTEGER NOT NULL DEFAULT 1" + ) + + +def downgrade() -> None: + op.execute("ALTER TABLE users DROP COLUMN IF EXISTS session_version") diff --git a/src/fabledassistant/auth.py b/src/fabledassistant/auth.py index cac2515..3006219 100644 --- a/src/fabledassistant/auth.py +++ b/src/fabledassistant/auth.py @@ -15,6 +15,9 @@ def _check_auth(f, required_role: str | None = None): if not user: session.clear() return jsonify({"error": "Authentication required"}), 401 + if session.get("session_version") != user.session_version: + session.clear() + return jsonify({"error": "Session expired. Please log in again."}), 401 if required_role and user.role != required_role: return jsonify({"error": "Admin access required"}), 403 g.user = user diff --git a/src/fabledassistant/models/user.py b/src/fabledassistant/models/user.py index 7fc7133..2db482a 100644 --- a/src/fabledassistant/models/user.py +++ b/src/fabledassistant/models/user.py @@ -1,4 +1,4 @@ -from sqlalchemy import Index, Text +from sqlalchemy import Index, Integer, Text from sqlalchemy.orm import Mapped, mapped_column from fabledassistant.models import Base @@ -14,6 +14,7 @@ class User(Base, CreatedAtMixin): password_hash: Mapped[str | None] = mapped_column(Text, nullable=True) oauth_sub: Mapped[str | None] = mapped_column(Text, unique=True, nullable=True) role: Mapped[str] = mapped_column(Text, nullable=False, default="user") + session_version: Mapped[int] = mapped_column(Integer, nullable=False, default=1) __table_args__ = ( Index("ix_users_username", "username"), diff --git a/src/fabledassistant/routes/auth.py b/src/fabledassistant/routes/auth.py index 7b4ccaf..434e991 100644 --- a/src/fabledassistant/routes/auth.py +++ b/src/fabledassistant/routes/auth.py @@ -76,6 +76,7 @@ async def register(): return jsonify({"error": "Username already taken"}), 409 session["user_id"] = user.id + session["session_version"] = user.session_version await log_audit("register", user_id=user.id, username=user.username, ip_address=_client_ip()) return jsonify(user.to_dict()), 201 @@ -106,6 +107,7 @@ async def login(): return jsonify({"error": "Invalid username or password"}), 401 session["user_id"] = user.id + session["session_version"] = user.session_version await log_audit("login", user_id=user.id, username=user.username, ip_address=_client_ip()) asyncio.create_task(notify_security_event( user.id, "login", @@ -150,10 +152,12 @@ async def update_password(): return jsonify({"error": "New password must be at least 8 characters"}), 400 uid = get_current_user_id() - success = await change_password(uid, current, new_pw) - if not success: + new_version = await change_password(uid, current, new_pw) + if new_version is None: return jsonify({"error": "Current password is incorrect"}), 403 + # Keep the current session alive with the new version (other sessions are now invalidated) + session["session_version"] = new_version await log_audit("password_change", user_id=uid, username=g.user.username, ip_address=_client_ip()) asyncio.create_task(notify_security_event( uid, "password_change", @@ -279,6 +283,7 @@ async def register_with_invite(): return jsonify({"error": "Invalid or expired invitation"}), 400 session["user_id"] = user.id + session["session_version"] = user.session_version await log_audit( "register_with_invite", user_id=user.id, @@ -346,5 +351,6 @@ async def oauth_callback(): user = await find_or_create_oauth_user(sub, email, preferred_username) session["user_id"] = user.id + session["session_version"] = user.session_version await log_audit("oauth_login", user_id=user.id, username=user.username, ip_address=_client_ip()) return redirect("/") diff --git a/src/fabledassistant/services/auth.py b/src/fabledassistant/services/auth.py index e290381..57f8beb 100644 --- a/src/fabledassistant/services/auth.py +++ b/src/fabledassistant/services/auth.py @@ -109,18 +109,19 @@ async def get_user_by_username(username: str) -> User | None: return result.scalars().first() -async def change_password(user_id: int, current_password: str, new_password: str) -> bool: - """Change a user's password. Returns True on success, False if current password is wrong.""" +async def change_password(user_id: int, current_password: str, new_password: str) -> int | None: + """Change a user's password. Returns new session_version on success, None if current password is wrong.""" async with async_session() as session: user = await session.get(User, user_id) if not user: - return False + return None if not verify_password(current_password, user.password_hash): - return False + return None user.password_hash = hash_password(new_password) + user.session_version += 1 await session.commit() logger.info("Password changed for user %d (%s)", user_id, user.username) - return True + return user.session_version async def is_registration_open() -> bool: @@ -249,6 +250,7 @@ async def reset_password_with_token(raw_token: str, new_password: str) -> int | return None user.password_hash = hash_password(new_password) + user.session_version += 1 reset_token.used = True await session.commit() diff --git a/summary.md b/summary.md index d7ee859..78fa089 100644 --- a/summary.md +++ b/summary.md @@ -12,7 +12,21 @@ > Include file-level details in the commit body when the change is non-trivial. ## Last Updated -2026-03-06 — DRY refactoring pass: TimestampMixin, route helpers, shared composables, ConfirmDialog, shared CSS. +2026-03-06 — Session invalidation on credential change (password reset + password change). + +**Session invalidation on credential change:** +- `models/user.py`: added `session_version: Mapped[int]` column (default 1). `Integer` import added. +- `alembic/versions/0024_add_session_version.py`: migration adds `session_version INTEGER NOT NULL DEFAULT 1` to `users`. +- `auth.py` (`_check_auth`): after loading user, checks `session["session_version"] == user.session_version`; mismatch → `session.clear()` + 401 "Session expired. Please log in again." +- `services/auth.py`: + - `change_password` return type changed from `bool` to `int | None` (returns new `session_version` on success, `None` on failure). Increments `user.session_version` before commit. + - `reset_password_with_token`: increments `user.session_version` before commit (invalidates all live sessions on password reset). +- `routes/auth.py`: + - `login`, `register`, `register_with_invite`, `oauth_callback`: now also set `session["session_version"] = user.session_version` alongside `session["user_id"]`. + - `update_password`: receives `new_version` from `change_password`; updates `session["session_version"] = new_version` so the current user's own session stays valid while all other sessions are invalidated. +- **Effect:** All existing sessions will require re-login once after the migration runs (existing cookies lack `session_version`, so they'll mismatch `1`). Subsequent credential changes immediately evict all other active sessions. + +--- **DRY refactoring pass (backend):** - `src/fabledassistant/models/base.py` (new): `TimestampMixin` (`created_at` + `updated_at`) and `CreatedAtMixin` (`created_at` only) — SQLAlchemy 2.0 `Mapped[]` mixins. Applied to all 10+ models (`Note`, `Project`, `Milestone`, `Conversation`, `Message`, `User`, `PushSubscription`, `TaskLog`, `NoteDraft`, `NoteVersion`), removing ~60 lines of duplicated column definitions.