security: scope device revocation by principal (Closes #1454)
This commit is contained in:
parent
7a09452bef
commit
0328e197ac
|
|
@ -236,7 +236,11 @@ async def revoke_all_sessions() -> None:
|
||||||
|
|
||||||
|
|
||||||
async def active_devices(session: Session):
|
async def active_devices(session: Session):
|
||||||
return await asyncio.to_thread(_session_store().list_active, session.session_id)
|
return await asyncio.to_thread(
|
||||||
|
_session_store().list_active,
|
||||||
|
session.session_id,
|
||||||
|
principal_id=session.principal_id,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
async def session_management_id(session: Session) -> str:
|
async def session_management_id(session: Session) -> str:
|
||||||
|
|
|
||||||
|
|
@ -4027,7 +4027,9 @@ async def revoke_active_device(
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
revoked = await asyncio.to_thread(
|
revoked = await asyncio.to_thread(
|
||||||
_passkey_store().revoke_device_access, management_id
|
_passkey_store().revoke_device_access,
|
||||||
|
management_id,
|
||||||
|
principal_id=request.state.dashboard_session.principal_id,
|
||||||
)
|
)
|
||||||
except dashboard_auth.SessionStoreError:
|
except dashboard_auth.SessionStoreError:
|
||||||
return JSONResponse(
|
return JSONResponse(
|
||||||
|
|
@ -4075,7 +4077,7 @@ async def sign_out_all_devices(
|
||||||
headers={"Cache-Control": "no-store"},
|
headers={"Cache-Control": "no-store"},
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
await asyncio.to_thread(
|
management_ids = await asyncio.to_thread(
|
||||||
_passkey_store().revoke_all_access,
|
_passkey_store().revoke_all_access,
|
||||||
principal_id=request.state.dashboard_session.principal_id,
|
principal_id=request.state.dashboard_session.principal_id,
|
||||||
)
|
)
|
||||||
|
|
@ -4085,7 +4087,7 @@ async def sign_out_all_devices(
|
||||||
status_code=503,
|
status_code=503,
|
||||||
headers={"Cache-Control": "no-store"},
|
headers={"Cache-Control": "no-store"},
|
||||||
)
|
)
|
||||||
await asyncio.to_thread(_push_subscription_store.delete_all)
|
await asyncio.to_thread(_push_subscription_store.delete_sessions, management_ids)
|
||||||
try:
|
try:
|
||||||
await asyncio.to_thread(journal.finalize, operation_id)
|
await asyncio.to_thread(journal.finalize, operation_id)
|
||||||
except SecurityEventStoreError:
|
except SecurityEventStoreError:
|
||||||
|
|
|
||||||
|
|
@ -322,13 +322,14 @@ class PasskeyStore:
|
||||||
except (OSError, sqlite3.Error) as exc:
|
except (OSError, sqlite3.Error) as exc:
|
||||||
raise SessionStoreError("Passkey registry is temporarily unavailable") from exc
|
raise SessionStoreError("Passkey registry is temporarily unavailable") from exc
|
||||||
|
|
||||||
def revoke_device_access(self, management_id: str) -> bool:
|
def revoke_device_access(self, management_id: str, *, principal_id: int) -> bool:
|
||||||
"""Atomically remove one active session, its grants, and its linked passkey."""
|
"""Atomically remove one active session, its grants, and its linked passkey."""
|
||||||
try:
|
try:
|
||||||
with self._connect() as connection:
|
with self._connect() as connection:
|
||||||
session = connection.execute(
|
session = connection.execute(
|
||||||
"SELECT session_hash FROM active_sessions WHERE management_id = ?",
|
"SELECT session_hash FROM active_sessions "
|
||||||
(management_id,),
|
"WHERE management_id = ? AND principal_id = ?",
|
||||||
|
(management_id, principal_id),
|
||||||
).fetchone()
|
).fetchone()
|
||||||
if session is None:
|
if session is None:
|
||||||
return False
|
return False
|
||||||
|
|
@ -336,19 +337,31 @@ class PasskeyStore:
|
||||||
"DELETE FROM step_up_grants WHERE session_hash = ?", (session[0],)
|
"DELETE FROM step_up_grants WHERE session_hash = ?", (session[0],)
|
||||||
)
|
)
|
||||||
connection.execute(
|
connection.execute(
|
||||||
"DELETE FROM passkey_credentials WHERE management_id = ?", (management_id,)
|
"DELETE FROM passkey_credentials "
|
||||||
|
"WHERE management_id = ? AND principal_id = ?",
|
||||||
|
(management_id, principal_id),
|
||||||
)
|
)
|
||||||
cursor = connection.execute(
|
cursor = connection.execute(
|
||||||
"DELETE FROM active_sessions WHERE management_id = ?", (management_id,)
|
"DELETE FROM active_sessions "
|
||||||
|
"WHERE management_id = ? AND principal_id = ?",
|
||||||
|
(management_id, principal_id),
|
||||||
)
|
)
|
||||||
return cursor.rowcount == 1
|
return cursor.rowcount == 1
|
||||||
except (OSError, sqlite3.Error) as exc:
|
except (OSError, sqlite3.Error) as exc:
|
||||||
raise SessionStoreError("Session registry is temporarily unavailable") from exc
|
raise SessionStoreError("Session registry is temporarily unavailable") from exc
|
||||||
|
|
||||||
def revoke_all_access(self, *, principal_id: int) -> None:
|
def revoke_all_access(self, *, principal_id: int) -> list[str]:
|
||||||
"""Atomically remove one principal's passkeys, grants, and active sessions."""
|
"""Atomically remove one principal's passkeys, grants, and active sessions."""
|
||||||
try:
|
try:
|
||||||
with self._connect() as connection:
|
with self._connect() as connection:
|
||||||
|
management_ids = [
|
||||||
|
row[0]
|
||||||
|
for row in connection.execute(
|
||||||
|
"SELECT management_id FROM active_sessions "
|
||||||
|
"WHERE principal_id = ? ORDER BY management_id",
|
||||||
|
(principal_id,),
|
||||||
|
).fetchall()
|
||||||
|
]
|
||||||
connection.execute(
|
connection.execute(
|
||||||
"DELETE FROM passkey_credentials WHERE principal_id = ?",
|
"DELETE FROM passkey_credentials WHERE principal_id = ?",
|
||||||
(principal_id,),
|
(principal_id,),
|
||||||
|
|
@ -362,5 +375,6 @@ class PasskeyStore:
|
||||||
"DELETE FROM active_sessions WHERE principal_id = ?",
|
"DELETE FROM active_sessions WHERE principal_id = ?",
|
||||||
(principal_id,),
|
(principal_id,),
|
||||||
)
|
)
|
||||||
|
return management_ids
|
||||||
except (OSError, sqlite3.Error) as exc:
|
except (OSError, sqlite3.Error) as exc:
|
||||||
raise SessionStoreError("Session registry is temporarily unavailable") from exc
|
raise SessionStoreError("Session registry is temporarily unavailable") from exc
|
||||||
|
|
|
||||||
|
|
@ -133,6 +133,9 @@ class DisabledPushSubscriptionStore:
|
||||||
def delete_session(self, *args, **kwargs) -> None:
|
def delete_session(self, *args, **kwargs) -> None:
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
def delete_sessions(self, *args, **kwargs) -> None:
|
||||||
|
return None
|
||||||
|
|
||||||
def delete_all(self, *args, **kwargs) -> None:
|
def delete_all(self, *args, **kwargs) -> None:
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
@ -442,6 +445,17 @@ class PushSubscriptionStore:
|
||||||
with self._connect() as connection:
|
with self._connect() as connection:
|
||||||
connection.execute("DELETE FROM push_subscriptions WHERE session_id = ?", (session_id,))
|
connection.execute("DELETE FROM push_subscriptions WHERE session_id = ?", (session_id,))
|
||||||
|
|
||||||
|
def delete_sessions(self, session_ids) -> None:
|
||||||
|
requested = set(session_ids)
|
||||||
|
if not requested:
|
||||||
|
return
|
||||||
|
placeholders = ",".join("?" for _ in requested)
|
||||||
|
with self._connect() as connection:
|
||||||
|
connection.execute(
|
||||||
|
f"DELETE FROM push_subscriptions WHERE session_id IN ({placeholders})",
|
||||||
|
tuple(requested),
|
||||||
|
)
|
||||||
|
|
||||||
def delete_all(self) -> None:
|
def delete_all(self) -> None:
|
||||||
with self._connect() as connection:
|
with self._connect() as connection:
|
||||||
connection.execute("DELETE FROM push_subscriptions")
|
connection.execute("DELETE FROM push_subscriptions")
|
||||||
|
|
|
||||||
|
|
@ -259,16 +259,18 @@ class SessionStore:
|
||||||
except (OSError, sqlite3.Error) as exc:
|
except (OSError, sqlite3.Error) as exc:
|
||||||
raise SessionStoreError("Session registry is temporarily unavailable") from exc
|
raise SessionStoreError("Session registry is temporarily unavailable") from exc
|
||||||
|
|
||||||
def list_active(self, current_session_id: str) -> list[ActiveDevice]:
|
def list_active(
|
||||||
|
self, current_session_id: str, *, principal_id: int
|
||||||
|
) -> list[ActiveDevice]:
|
||||||
now = int(self.clock())
|
now = int(self.clock())
|
||||||
current_hash = self._digest(current_session_id)
|
current_hash = self._digest(current_session_id)
|
||||||
try:
|
try:
|
||||||
with self._connect() as connection:
|
with self._connect() as connection:
|
||||||
rows = connection.execute(
|
rows = connection.execute(
|
||||||
"SELECT management_id, device_label, created_at, expires_at, session_hash "
|
"SELECT management_id, device_label, created_at, expires_at, session_hash "
|
||||||
"FROM active_sessions WHERE expires_at > ? "
|
"FROM active_sessions WHERE expires_at > ? AND principal_id = ? "
|
||||||
"ORDER BY expires_at DESC, created_at DESC",
|
"ORDER BY expires_at DESC, created_at DESC",
|
||||||
(now,),
|
(now, principal_id),
|
||||||
).fetchall()
|
).fetchall()
|
||||||
except (OSError, sqlite3.Error) as exc:
|
except (OSError, sqlite3.Error) as exc:
|
||||||
raise SessionStoreError("Session registry is temporarily unavailable") from exc
|
raise SessionStoreError("Session registry is temporarily unavailable") from exc
|
||||||
|
|
|
||||||
|
|
@ -99,6 +99,8 @@ def test_device_access_revocation_rolls_back_target_and_preserves_other_devices(
|
||||||
2_000,
|
2_000,
|
||||||
management_id=f"{slug}-management-id",
|
management_id=f"{slug}-management-id",
|
||||||
device_label=label,
|
device_label=label,
|
||||||
|
principal_id=42,
|
||||||
|
principal_login="timmy",
|
||||||
)
|
)
|
||||||
sessions.mint_step_up(
|
sessions.mint_step_up(
|
||||||
f"{slug}-session",
|
f"{slug}-session",
|
||||||
|
|
@ -122,7 +124,7 @@ def test_device_access_revocation_rolls_back_target_and_preserves_other_devices(
|
||||||
)
|
)
|
||||||
|
|
||||||
with pytest.raises(SessionStoreError):
|
with pytest.raises(SessionStoreError):
|
||||||
passkeys.revoke_device_access("phone-management-id")
|
passkeys.revoke_device_access("phone-management-id", principal_id=42)
|
||||||
|
|
||||||
with sqlite3.connect(database) as connection:
|
with sqlite3.connect(database) as connection:
|
||||||
assert connection.execute(
|
assert connection.execute(
|
||||||
|
|
@ -132,3 +134,69 @@ def test_device_access_revocation_rolls_back_target_and_preserves_other_devices(
|
||||||
"SELECT management_id FROM passkey_credentials ORDER BY management_id"
|
"SELECT management_id FROM passkey_credentials ORDER BY management_id"
|
||||||
).fetchall() == [("laptop-management-id",), ("phone-management-id",)]
|
).fetchall() == [("laptop-management-id",), ("phone-management-id",)]
|
||||||
assert connection.execute("SELECT COUNT(*) FROM step_up_grants").fetchone() == (2,)
|
assert connection.execute("SELECT COUNT(*) FROM step_up_grants").fetchone() == (2,)
|
||||||
|
|
||||||
|
|
||||||
|
def test_device_access_revocation_cannot_cross_principal_boundary(tmp_path):
|
||||||
|
database = tmp_path / "sessions.sqlite3"
|
||||||
|
sessions = SessionStore(database, clock=lambda: 1_000.0)
|
||||||
|
passkeys = PasskeyStore(database, clock=lambda: 1_000.0)
|
||||||
|
for principal_id in (42, 84):
|
||||||
|
sessions.activate(
|
||||||
|
f"session-{principal_id}",
|
||||||
|
2_000,
|
||||||
|
management_id=f"management-{principal_id}",
|
||||||
|
principal_id=principal_id,
|
||||||
|
principal_login=f"operator-{principal_id}",
|
||||||
|
)
|
||||||
|
sessions.mint_step_up(
|
||||||
|
f"session-{principal_id}",
|
||||||
|
action="close_issue",
|
||||||
|
target="stackchain/dashboard#1",
|
||||||
|
ttl_seconds=90,
|
||||||
|
)
|
||||||
|
passkeys.register(
|
||||||
|
credential_id=f"credential-{principal_id}".encode(),
|
||||||
|
public_key=f"public-key-{principal_id}".encode(),
|
||||||
|
sign_count=0,
|
||||||
|
device_label=f"Device {principal_id}",
|
||||||
|
management_id=f"management-{principal_id}",
|
||||||
|
principal_id=principal_id,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert passkeys.revoke_device_access("management-84", principal_id=42) is False
|
||||||
|
assert passkeys.revoke_device_access("management-42", principal_id=42) is True
|
||||||
|
|
||||||
|
with sqlite3.connect(database) as connection:
|
||||||
|
assert connection.execute(
|
||||||
|
"SELECT principal_id FROM active_sessions ORDER BY principal_id"
|
||||||
|
).fetchall() == [(84,)]
|
||||||
|
assert connection.execute(
|
||||||
|
"SELECT principal_id FROM passkey_credentials ORDER BY principal_id"
|
||||||
|
).fetchall() == [(84,)]
|
||||||
|
assert connection.execute("SELECT COUNT(*) FROM step_up_grants").fetchone() == (1,)
|
||||||
|
|
||||||
|
|
||||||
|
def test_global_access_revocation_returns_only_affected_management_ids(tmp_path):
|
||||||
|
database = tmp_path / "sessions.sqlite3"
|
||||||
|
sessions = SessionStore(database, clock=lambda: 1_000.0)
|
||||||
|
passkeys = PasskeyStore(database, clock=lambda: 1_000.0)
|
||||||
|
for principal_id in (42, 84):
|
||||||
|
for suffix in ("phone", "laptop"):
|
||||||
|
sessions.activate(
|
||||||
|
f"session-{principal_id}-{suffix}",
|
||||||
|
2_000,
|
||||||
|
management_id=f"management-{principal_id}-{suffix}",
|
||||||
|
principal_id=principal_id,
|
||||||
|
principal_login=f"operator-{principal_id}",
|
||||||
|
)
|
||||||
|
|
||||||
|
affected = passkeys.revoke_all_access(principal_id=42)
|
||||||
|
|
||||||
|
assert affected == ["management-42-laptop", "management-42-phone"]
|
||||||
|
with sqlite3.connect(database) as connection:
|
||||||
|
assert connection.execute(
|
||||||
|
"SELECT management_id FROM active_sessions ORDER BY management_id"
|
||||||
|
).fetchall() == [
|
||||||
|
("management-84-laptop",),
|
||||||
|
("management-84-phone",),
|
||||||
|
]
|
||||||
|
|
|
||||||
|
|
@ -1985,3 +1985,21 @@ async def test_subscription_fails_closed_when_existing_unread_baseline_is_unavai
|
||||||
|
|
||||||
assert raised.value.status_code == 503
|
assert raised.value.status_code == 503
|
||||||
assert store.is_subscribed("device-a") is False
|
assert store.is_subscribed("device-a") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_sessions_removes_only_the_requested_device_push_state(tmp_path):
|
||||||
|
store = PushSubscriptionStore(tmp_path / "push.sqlite3")
|
||||||
|
for session_id in ("management-42-phone", "management-42-laptop", "management-84-phone"):
|
||||||
|
store.upsert(
|
||||||
|
session_id,
|
||||||
|
{
|
||||||
|
"endpoint": f"https://push.example/{session_id}",
|
||||||
|
"keys": {"p256dh": "public-key", "auth": "auth-secret"},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
store.delete_sessions(["management-42-phone", "management-42-laptop"])
|
||||||
|
|
||||||
|
assert store.is_subscribed("management-42-phone") is False
|
||||||
|
assert store.is_subscribed("management-42-laptop") is False
|
||||||
|
assert store.is_subscribed("management-84-phone") is True
|
||||||
|
|
|
||||||
|
|
@ -201,6 +201,38 @@ def test_managed_session_statuses_skips_database_for_an_empty_batch(tmp_path, mo
|
||||||
assert store.managed_statuses([], idle_timeout_seconds=900) == {}
|
assert store.managed_statuses([], idle_timeout_seconds=900) == {}
|
||||||
|
|
||||||
|
|
||||||
|
def test_active_devices_are_scoped_to_the_signed_in_principal(tmp_path):
|
||||||
|
store = SessionStore(tmp_path / "sessions.sqlite3", clock=lambda: 1_000.0)
|
||||||
|
store.activate(
|
||||||
|
"matching-session",
|
||||||
|
3_000,
|
||||||
|
management_id="matching-device",
|
||||||
|
device_label="My phone",
|
||||||
|
principal_id=42,
|
||||||
|
principal_login="timmy",
|
||||||
|
)
|
||||||
|
store.activate(
|
||||||
|
"other-session",
|
||||||
|
3_000,
|
||||||
|
management_id="other-device",
|
||||||
|
device_label="Other phone",
|
||||||
|
principal_id=84,
|
||||||
|
principal_login="other",
|
||||||
|
)
|
||||||
|
store.activate(
|
||||||
|
"legacy-session",
|
||||||
|
3_000,
|
||||||
|
management_id="legacy-device",
|
||||||
|
device_label="Legacy phone",
|
||||||
|
)
|
||||||
|
|
||||||
|
devices = store.list_active("matching-session", principal_id=42)
|
||||||
|
|
||||||
|
assert [(device.management_id, device.current) for device in devices] == [
|
||||||
|
("matching-device", True)
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
def test_touch_extends_only_the_matching_live_session(tmp_path):
|
def test_touch_extends_only_the_matching_live_session(tmp_path):
|
||||||
now = [1_000.0]
|
now = [1_000.0]
|
||||||
store = SessionStore(tmp_path / "sessions.sqlite3", clock=lambda: now[0])
|
store = SessionStore(tmp_path / "sessions.sqlite3", clock=lambda: now[0])
|
||||||
|
|
@ -229,10 +261,14 @@ def test_touch_cannot_revive_a_session_at_the_idle_boundary(tmp_path):
|
||||||
|
|
||||||
def test_active_devices_are_listed_without_exposing_session_secrets(tmp_path):
|
def test_active_devices_are_listed_without_exposing_session_secrets(tmp_path):
|
||||||
store = SessionStore(tmp_path / "sessions.sqlite3", clock=lambda: 1_000.0)
|
store = SessionStore(tmp_path / "sessions.sqlite3", clock=lambda: 1_000.0)
|
||||||
store.activate("phone-session-secret", 2_000, device_label="Pixel 9")
|
store.activate(
|
||||||
store.activate("laptop-session-secret", 3_000, device_label="Work laptop")
|
"phone-session-secret", 2_000, device_label="Pixel 9", principal_id=42
|
||||||
|
)
|
||||||
|
store.activate(
|
||||||
|
"laptop-session-secret", 3_000, device_label="Work laptop", principal_id=42
|
||||||
|
)
|
||||||
|
|
||||||
devices = store.list_active("phone-session-secret")
|
devices = store.list_active("phone-session-secret", principal_id=42)
|
||||||
|
|
||||||
assert [device.device_label for device in devices] == ["Work laptop", "Pixel 9"]
|
assert [device.device_label for device in devices] == ["Work laptop", "Pixel 9"]
|
||||||
assert [device.current for device in devices] == [False, True]
|
assert [device.current for device in devices] == [False, True]
|
||||||
|
|
@ -243,9 +279,13 @@ def test_active_devices_are_listed_without_exposing_session_secrets(tmp_path):
|
||||||
|
|
||||||
def test_revoke_managed_device_removes_only_the_selected_session(tmp_path):
|
def test_revoke_managed_device_removes_only_the_selected_session(tmp_path):
|
||||||
store = SessionStore(tmp_path / "sessions.sqlite3", clock=lambda: 1_000.0)
|
store = SessionStore(tmp_path / "sessions.sqlite3", clock=lambda: 1_000.0)
|
||||||
store.activate("phone", 2_000, device_label="Phone")
|
store.activate("phone", 2_000, device_label="Phone", principal_id=42)
|
||||||
store.activate("laptop", 2_000, device_label="Laptop")
|
store.activate("laptop", 2_000, device_label="Laptop", principal_id=42)
|
||||||
phone = next(device for device in store.list_active("laptop") if device.device_label == "Phone")
|
phone = next(
|
||||||
|
device
|
||||||
|
for device in store.list_active("laptop", principal_id=42)
|
||||||
|
if device.device_label == "Phone"
|
||||||
|
)
|
||||||
|
|
||||||
assert store.revoke_managed(phone.management_id) is True
|
assert store.revoke_managed(phone.management_id) is True
|
||||||
assert store.is_active("phone", 2_000) is False
|
assert store.is_active("phone", 2_000) is False
|
||||||
|
|
@ -266,9 +306,8 @@ def test_existing_session_registry_migrates_without_invalidating_sessions(tmp_pa
|
||||||
store.activate("new-session", 3_000, device_label="New phone")
|
store.activate("new-session", 3_000, device_label="New phone")
|
||||||
|
|
||||||
assert store.is_active("existing-session", 2_000) is True
|
assert store.is_active("existing-session", 2_000) is True
|
||||||
devices = store.list_active("existing-session")
|
devices = store.list_active("existing-session", principal_id=42)
|
||||||
assert len(devices) == 2
|
assert devices == []
|
||||||
assert next(device for device in devices if device.current).device_label == "Existing device"
|
|
||||||
|
|
||||||
|
|
||||||
def test_idle_status_migrates_existing_registry_and_starts_legacy_idle_clock_now(tmp_path):
|
def test_idle_status_migrates_existing_registry_and_starts_legacy_idle_clock_now(tmp_path):
|
||||||
|
|
@ -388,11 +427,11 @@ def test_step_up_grants_expire_and_are_removed_with_parent_session(tmp_path):
|
||||||
|
|
||||||
def test_managed_session_revocation_invalidates_its_outstanding_grants(tmp_path):
|
def test_managed_session_revocation_invalidates_its_outstanding_grants(tmp_path):
|
||||||
store = SessionStore(tmp_path / "sessions.sqlite3", clock=lambda: 1_000.0)
|
store = SessionStore(tmp_path / "sessions.sqlite3", clock=lambda: 1_000.0)
|
||||||
store.activate("phone-session", 2_000, device_label="Phone")
|
store.activate("phone-session", 2_000, device_label="Phone", principal_id=42)
|
||||||
grant = store.mint_step_up(
|
grant = store.mint_step_up(
|
||||||
"phone-session", action="merge_pull", target="stackchain/api#7", ttl_seconds=90
|
"phone-session", action="merge_pull", target="stackchain/api#7", ttl_seconds=90
|
||||||
)
|
)
|
||||||
phone = store.list_active("phone-session")[0]
|
phone = store.list_active("phone-session", principal_id=42)[0]
|
||||||
|
|
||||||
assert store.revoke_managed(phone.management_id) is True
|
assert store.revoke_managed(phone.management_id) is True
|
||||||
assert store.consume_step_up(
|
assert store.consume_step_up(
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user