diff --git a/backend/alembic/versions/0085_users_last_login_at.py b/backend/alembic/versions/0085_users_last_login_at.py new file mode 100644 index 00000000..fb050e91 --- /dev/null +++ b/backend/alembic/versions/0085_users_last_login_at.py @@ -0,0 +1,26 @@ +"""add users.last_login_at — when the account last signed in (E22 S1) + +Revision ID: 0085_users_last_login_at +Revises: 0084_dl_job_error_detail +Create Date: 2026-09-16 + +Stamped by the one sign-in path (`routes/auth._establish_session`), which the Google callback and +password login share. No backfill is possible: the signal exists only at sign-in, so existing rows +read NULL until each account signs in again, and the admin UI says so rather than inventing a date. +""" + +import sqlalchemy as sa +from alembic import op + +revision: str = "0085_users_last_login_at" +down_revision: str | None = "0084_dl_job_error_detail" +branch_labels: str | None = None +depends_on: str | None = None + + +def upgrade() -> None: + op.add_column("users", sa.Column("last_login_at", sa.DateTime(timezone=True), nullable=True)) + + +def downgrade() -> None: + op.drop_column("users", "last_login_at") diff --git a/backend/app/models.py b/backend/app/models.py index f29695d9..94203c9b 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -102,6 +102,10 @@ class User(Base, TimestampMixin, CanonicalEmailMixin): # current_user rejects any cookie whose epoch is stale, so a stolen/copied cookie dies on those # events (client-side signed cookies otherwise stay valid until they expire). session_epoch: Mapped[int] = mapped_column(Integer, default=0, server_default="0") + # When this account last signed in (Google or password), stamped by the one sign-in path, + # routes/auth._establish_session. NULL = not since the column existed (0085; no backfill is + # possible). The shared demo account is never stamped — its "last login" would mean nobody. + last_login_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) display_name: Mapped[str | None] = mapped_column(String(255)) avatar_url: Mapped[str | None] = mapped_column(String(1024)) role: Mapped[str] = mapped_column(String(16), default="user", server_default="user") diff --git a/backend/app/routes/admin.py b/backend/app/routes/admin.py index 86ad5ade..63e070da 100644 --- a/backend/app/routes/admin.py +++ b/backend/app/routes/admin.py @@ -408,6 +408,8 @@ def _serialize_user(u: User) -> dict: # confusing sign-in. "google_email": u.google_email if u.google_email != u.email else None, "created_at": iso(u.created_at), + # NULL = no sign-in since the column existed (0085), or the shared demo account. + "last_login_at": iso(u.last_login_at), } diff --git a/backend/app/routes/auth.py b/backend/app/routes/auth.py index 18c25e67..97bae871 100644 --- a/backend/app/routes/auth.py +++ b/backend/app/routes/auth.py @@ -479,16 +479,8 @@ async def callback( db.flush() _store_token(db, user, token) + _establish_session(request, user) db.commit() - - # Multi-session: remember every account that has authenticated in this browser so the - # user can switch between them without going through Google again. Only accounts that - # actually completed OAuth here land in this list (it's the proof they may be switched to). - accounts = [a for a in (request.session.get("account_ids") or []) if a != user.id] - accounts.append(user.id) - request.session["account_ids"] = accounts[-MAX_SESSION_ACCOUNTS:] - request.session["user_id"] = user.id - _remember_epoch(request, user) log.info("Login: %s (id=%s, role=%s)", email, user.id, user.role) return RedirectResponse(url="/") @@ -675,13 +667,18 @@ def demo_login(body: EmailIn, request: Request, db: Session = Depends(get_db)) - def _establish_session(request: Request, user: User) -> None: - """Sign `user` in for this browser session, mirroring the Google callback's multi-account - handling so password sign-in joins the same account switcher.""" + """Sign `user` in for this browser session — the ONE sign-in path, shared by the Google callback + and password login, so both join the same account switcher and both stamp `last_login_at` (the + caller commits). Multi-session: every account that authenticated in this browser is remembered so + the user can switch between them; only accounts that actually proved themselves HERE land in that + wallet (it is the proof they may be switched to). The demo entry deliberately does not come + through here: it clears the wallet instead, and its shared account has no meaningful last login.""" accounts = [a for a in (request.session.get("account_ids") or []) if a != user.id] accounts.append(user.id) request.session["account_ids"] = accounts[-MAX_SESSION_ACCOUNTS:] request.session["user_id"] = user.id _remember_epoch(request, user) + user.last_login_at = datetime.now(timezone.utc) def _app_base() -> str: @@ -1051,6 +1048,7 @@ def password_login( if not user.is_active: raise _login_refused(403, "pending_approval", user) _establish_session(request, user) + db.commit() log.info("Password login: %s (id=%s, role=%s)", email, user.id, user.role) return {"ok": True} diff --git a/backend/tests/test_last_login.py b/backend/tests/test_last_login.py new file mode 100644 index 00000000..b2a48d6d --- /dev/null +++ b/backend/tests/test_last_login.py @@ -0,0 +1,97 @@ +"""`users.last_login_at` (E22 S1), DB-lane. + +The roadmap assumed stamping it in `_establish_session` would cover Google and password alike. It +would not have: only password login called it, and the Google callback carried its own copy of the +same four lines. So these pin BOTH doors through the real routes — plus the two that must not stamp +(a refused password, the shared demo account) and the admin list that shows it. +""" + +from datetime import datetime, timedelta, timezone + +from app.config import settings +from app.models import DemoWhitelist +from app.routes import auth as auth_routes + +_PW = "correct horse battery" + + +def _recent(ts: datetime | None) -> bool: + return ts is not None and datetime.now(timezone.utc) - ts < timedelta(minutes=1) + + +def test_a_password_sign_in_stamps_last_login(client, db, make_user): + user = make_user(email="pw@example.com", password=_PW) + assert user.last_login_at is None + + r = client.post("/auth/password-login", json={"email": "pw@example.com", "password": _PW}) + + assert r.status_code == 200 + db.refresh(user) + assert _recent(user.last_login_at) + + +def test_a_refused_password_does_not_stamp(client, db, make_user): + user = make_user(email="wrong@example.com", password=_PW) + + r = client.post( + "/auth/password-login", json={"email": "wrong@example.com", "password": "not it"} + ) + + assert r.status_code == 401 + db.refresh(user) + assert user.last_login_at is None + + +class _FakeGoogle: + """authlib's client, reduced to the one call the callback makes after Google redirects back.""" + + def __init__(self, userinfo: dict): + self._userinfo = userinfo + + async def authorize_access_token(self, _request): + return {"userinfo": self._userinfo, "access_token": "at", "scope": "openid email profile"} + + +def test_a_google_sign_in_stamps_last_login(client, db, make_user, monkeypatch): + from cryptography.fernet import Fernet + + from app import security + + # The callback stores the OAuth grant encrypted; a throwaway key stands in for TOKEN_ENCRYPTION_KEY. + monkeypatch.setattr(security, "_fernet", Fernet(Fernet.generate_key())) + user = make_user(email="g@example.com", password=None, google_sub="sub-g") + monkeypatch.setattr(settings, "admin_emails", "g@example.com") # the bootstrap allow-list + fake = _FakeGoogle({"sub": "sub-g", "email": "g@example.com", "email_verified": True}) + monkeypatch.setattr(auth_routes, "google_oauth", lambda _db: fake) + + r = client.get("/auth/callback", follow_redirects=False) + + assert r.status_code in (302, 307) and r.headers["location"] == "/" + db.refresh(user) + assert _recent(user.last_login_at) + # …and it is a real sign-in, not only a stamp: the session now answers as this account. + assert client.get("/api/me").json()["id"] == user.id + + +def test_the_demo_entry_does_not_stamp_the_shared_account(client, db): + db.add(DemoWhitelist(email="visitor@example.com")) + db.commit() + + r = client.post("/auth/demo", json={"email": "visitor@example.com"}) + + assert r.json() == {"authenticated": True} + demo = auth_routes.get_or_create_demo_user(db) + db.refresh(demo) + assert demo.last_login_at is None + + +def test_the_admin_user_list_carries_last_login(as_user, db, make_user): + admin = make_user(email="boss@example.com", role="admin") + never = make_user(email="never@example.com") + admin.last_login_at = datetime(2026, 9, 16, 8, 30, tzinfo=timezone.utc) + db.commit() + + rows = {u["email"]: u for u in as_user(admin).get("/api/admin/users").json()} + + assert rows["boss@example.com"]["last_login_at"].startswith("2026-09-16T08:30") + assert rows[never.email]["last_login_at"] is None diff --git a/frontend/src/lib/api/admin.ts b/frontend/src/lib/api/admin.ts index a9ae05e7..8b9b82b9 100644 --- a/frontend/src/lib/api/admin.ts +++ b/frontend/src/lib/api/admin.ts @@ -82,6 +82,8 @@ export interface AdminUserRow { /** The Google account's address, ONLY when it differs from the account address (E9.5 S3). */ google_email: string | null; created_at: string | null; + /** Last sign-in (Google or password). Null = not since 0.88 (no backfill), or the demo account. */ + last_login_at: string | null; } /** The account panel's view of why someone can or cannot get in (E12 S5). */