fix(users): a password reset signs out every session begun with the old password (#5427)
#5427 "A password reset doesn't end the sessions already signed in with the old password": users.sessions_since (migration 0038) is set on reset; refresh_session_user ends a session whose login_at is earlier. An admin resetting their own password carries on signed in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -145,6 +145,10 @@ async def refresh_session_user() -> None:
|
|||||||
if user is None or not user.is_active:
|
if user is None or not user.is_active:
|
||||||
session.clear()
|
session.clear()
|
||||||
return
|
return
|
||||||
|
# The password was reset after this session began (#5427).
|
||||||
|
if user.sessions_since is not None and started < user.sessions_since.timestamp():
|
||||||
|
session.clear()
|
||||||
|
return
|
||||||
if session.get("user_role") != user.role.value:
|
if session.get("user_role") != user.role.value:
|
||||||
session["user_role"] = user.role.value
|
session["user_role"] = user.role.value
|
||||||
# The zone times are written in (milestone 490): a change made on the
|
# The zone times are written in (milestone 490): a change made on the
|
||||||
|
|||||||
@@ -0,0 +1,23 @@
|
|||||||
|
"""A password reset ends the account's other sessions (#5427): sessions that
|
||||||
|
began before users.sessions_since are over.
|
||||||
|
|
||||||
|
Revision ID: 0038_user_sessions_since
|
||||||
|
Revises: 0037_user_invites
|
||||||
|
Create Date: 2026-10-08
|
||||||
|
"""
|
||||||
|
from typing import Sequence, Union
|
||||||
|
from alembic import op
|
||||||
|
import sqlalchemy as sa
|
||||||
|
|
||||||
|
revision: str = "0038_user_sessions_since"
|
||||||
|
down_revision: Union[str, None] = "0037_user_invites"
|
||||||
|
branch_labels: Union[str, Sequence[str], None] = None
|
||||||
|
depends_on: Union[str, Sequence[str], None] = None
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
op.add_column("users", sa.Column("sessions_since", sa.DateTime(timezone=True), nullable=True))
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
op.drop_column("users", "sessions_since")
|
||||||
@@ -44,3 +44,6 @@ class User(Base):
|
|||||||
# 495). None follows its role's default (notify.<kind>.<role> in settings).
|
# 495). None follows its role's default (notify.<kind>.<role> in settings).
|
||||||
notify_alerts: Mapped[bool | None] = mapped_column(Boolean, nullable=True)
|
notify_alerts: Mapped[bool | None] = mapped_column(Boolean, nullable=True)
|
||||||
notify_report: Mapped[bool | None] = mapped_column(Boolean, nullable=True)
|
notify_report: Mapped[bool | None] = mapped_column(Boolean, nullable=True)
|
||||||
|
# Sessions that began before this are over (#5427): set when the password
|
||||||
|
# is reset, so a browser signed in with the old password is signed out.
|
||||||
|
sessions_since: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True)
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ so Steward always keeps someone who can get back in. Every change is audited.
|
|||||||
"""
|
"""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import time
|
||||||
from datetime import datetime, timezone
|
from datetime import datetime, timezone
|
||||||
|
|
||||||
import bcrypt
|
import bcrypt
|
||||||
@@ -268,7 +269,7 @@ async def user_invite_revoke(invite_id: int):
|
|||||||
|
|
||||||
|
|
||||||
_DONE = {"created": "Account added.", "saved": "Saved.", "role": "Role changed.",
|
_DONE = {"created": "Account added.", "saved": "Saved.", "role": "Role changed.",
|
||||||
"password": "Password set. It works from their next sign-in.",
|
"password": "Password set. Anywhere they were signed in, they are signed out.",
|
||||||
"disabled": "Account disabled. They are signed out at their next click.",
|
"disabled": "Account disabled. They are signed out at their next click.",
|
||||||
"enabled": "Account enabled."}
|
"enabled": "Account enabled."}
|
||||||
|
|
||||||
@@ -374,8 +375,12 @@ async def user_password(user_id: str):
|
|||||||
problem = f"This account signs in with {SOURCES[user.auth_source]}; it has no Steward password."
|
problem = f"This account signs in with {SOURCES[user.auth_source]}; it has no Steward password."
|
||||||
if problem is None:
|
if problem is None:
|
||||||
user.password_hash = _hash(form.get("password"))
|
user.password_hash = _hash(form.get("password"))
|
||||||
|
# Every browser signed in with the old password is signed out (#5427).
|
||||||
|
user.sessions_since = datetime.now(timezone.utc)
|
||||||
if problem:
|
if problem:
|
||||||
return await _user_page(user_id, problem, 400)
|
return await _user_page(user_id, problem, 400)
|
||||||
|
if user_id == session.get("user_id"):
|
||||||
|
session["login_at"] = time.time() # this session carries on under the new password
|
||||||
await audit_request("user.password_reset", "user", user_id, {"username": user.username})
|
await audit_request("user.password_reset", "user", user_id, {"username": user.username})
|
||||||
return redirect(f"/settings/users/{user_id}?done=password")
|
return redirect(f"/settings/users/{user_id}?done=password")
|
||||||
|
|
||||||
|
|||||||
@@ -72,7 +72,7 @@
|
|||||||
<div class="field">
|
<div class="field">
|
||||||
<label class="field-label" for="u-pw">New password</label>
|
<label class="field-label" for="u-pw">New password</label>
|
||||||
<input class="input" id="u-pw" type="password" name="password" minlength="{{ min_password }}" required autocomplete="new-password">
|
<input class="input" id="u-pw" type="password" name="password" minlength="{{ min_password }}" required autocomplete="new-password">
|
||||||
<span class="field-hint">At least {{ min_password }} characters. Give it to them yourself; Steward sends no email for it.</span>
|
<span class="field-hint">At least {{ min_password }} characters. Give it to them yourself; Steward sends no email for it. Anywhere they're signed in now, they're signed out.</span>
|
||||||
</div>
|
</div>
|
||||||
<div class="field">
|
<div class="field">
|
||||||
<label class="field-label" for="u-pw2">Again</label>
|
<label class="field-label" for="u-pw2">Again</label>
|
||||||
|
|||||||
@@ -161,6 +161,33 @@ def test_a_provider_account_s_role_is_not_edited_here(app):
|
|||||||
asyncio.run(_go())
|
asyncio.run(_go())
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_password_reset_signs_out_the_old_sessions_but_not_the_admin_s_own(app):
|
||||||
|
"""#5427: a browser signed in before the reset is signed out; the admin
|
||||||
|
who resets their own password stays signed in."""
|
||||||
|
async def _go():
|
||||||
|
admin, me = await _client(app)
|
||||||
|
them, uid = await _client(app, role="viewer")
|
||||||
|
async with them.session_transaction() as sess:
|
||||||
|
sess["login_at"] = time.time() - 60 # signed in a minute ago
|
||||||
|
assert (await them.get("/account/")).status_code == 200
|
||||||
|
|
||||||
|
resp = await admin.post(f"/settings/users/{uid}/password", headers=CSRF,
|
||||||
|
form={"password": "battery staple", "confirm": "battery staple"})
|
||||||
|
assert resp.status_code == 302 and "signed out" in await (await admin.get(resp.headers["Location"])).get_data(as_text=True)
|
||||||
|
assert (await them.get("/account/")).status_code == 302 # back to sign-in
|
||||||
|
async with them.session_transaction() as sess:
|
||||||
|
assert "user_id" not in sess
|
||||||
|
|
||||||
|
async with admin.session_transaction() as sess:
|
||||||
|
sess["login_at"] = time.time() - 60
|
||||||
|
resp = await admin.post(f"/settings/users/{me}/password", headers=CSRF,
|
||||||
|
form={"password": "my own new one", "confirm": "my own new one"})
|
||||||
|
assert resp.status_code == 302
|
||||||
|
assert (await admin.get("/account/")).status_code == 200 # still signed in
|
||||||
|
|
||||||
|
asyncio.run(_go())
|
||||||
|
|
||||||
|
|
||||||
def test_only_an_admin_reaches_users(app):
|
def test_only_an_admin_reaches_users(app):
|
||||||
async def _go():
|
async def _go():
|
||||||
client, _ = await _client(app, role="operator")
|
client, _ = await _client(app, role="operator")
|
||||||
|
|||||||
@@ -5,7 +5,8 @@ def test_user_model_fields():
|
|||||||
"""User model has all required fields."""
|
"""User model has all required fields."""
|
||||||
cols = {c.key for c in User.__table__.columns}
|
cols = {c.key for c in User.__table__.columns}
|
||||||
assert cols == {"id", "username", "email", "password_hash", "role", "created_at", "is_active",
|
assert cols == {"id", "username", "email", "password_hash", "role", "created_at", "is_active",
|
||||||
"auth_source", "external_id", "time_zone", "notify_alerts", "notify_report"}
|
"auth_source", "external_id", "time_zone", "notify_alerts", "notify_report",
|
||||||
|
"sessions_since"}
|
||||||
|
|
||||||
|
|
||||||
def test_user_role_enum():
|
def test_user_role_enum():
|
||||||
|
|||||||
Reference in New Issue
Block a user