diff --git a/src/dashboard_auth.py b/src/dashboard_auth.py index 66ed097..10ea841 100644 --- a/src/dashboard_auth.py +++ b/src/dashboard_auth.py @@ -236,7 +236,11 @@ async def revoke_all_sessions() -> None: 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: diff --git a/src/main.py b/src/main.py index c5c0ece..f2a75ee 100644 --- a/src/main.py +++ b/src/main.py @@ -4027,7 +4027,9 @@ async def revoke_active_device( ) try: 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: return JSONResponse( @@ -4075,7 +4077,7 @@ async def sign_out_all_devices( headers={"Cache-Control": "no-store"}, ) try: - await asyncio.to_thread( + management_ids = await asyncio.to_thread( _passkey_store().revoke_all_access, principal_id=request.state.dashboard_session.principal_id, ) @@ -4085,7 +4087,7 @@ async def sign_out_all_devices( status_code=503, 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: await asyncio.to_thread(journal.finalize, operation_id) except SecurityEventStoreError: diff --git a/src/passkey_store.py b/src/passkey_store.py index da69877..2116904 100644 --- a/src/passkey_store.py +++ b/src/passkey_store.py @@ -322,13 +322,14 @@ class PasskeyStore: except (OSError, sqlite3.Error) as 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.""" try: with self._connect() as connection: session = connection.execute( - "SELECT session_hash FROM active_sessions WHERE management_id = ?", - (management_id,), + "SELECT session_hash FROM active_sessions " + "WHERE management_id = ? AND principal_id = ?", + (management_id, principal_id), ).fetchone() if session is None: return False @@ -336,19 +337,31 @@ class PasskeyStore: "DELETE FROM step_up_grants WHERE session_hash = ?", (session[0],) ) 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( - "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 except (OSError, sqlite3.Error) as 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.""" try: 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( "DELETE FROM passkey_credentials WHERE principal_id = ?", (principal_id,), @@ -362,5 +375,6 @@ class PasskeyStore: "DELETE FROM active_sessions WHERE principal_id = ?", (principal_id,), ) + return management_ids except (OSError, sqlite3.Error) as exc: raise SessionStoreError("Session registry is temporarily unavailable") from exc diff --git a/src/push_subscription_store.py b/src/push_subscription_store.py index 54847e8..e70b215 100644 --- a/src/push_subscription_store.py +++ b/src/push_subscription_store.py @@ -133,6 +133,9 @@ class DisabledPushSubscriptionStore: def delete_session(self, *args, **kwargs) -> None: return None + def delete_sessions(self, *args, **kwargs) -> None: + return None + def delete_all(self, *args, **kwargs) -> None: return None @@ -442,6 +445,17 @@ class PushSubscriptionStore: with self._connect() as connection: 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: with self._connect() as connection: connection.execute("DELETE FROM push_subscriptions") diff --git a/src/session_store.py b/src/session_store.py index 7123728..8e4b614 100644 --- a/src/session_store.py +++ b/src/session_store.py @@ -259,16 +259,18 @@ class SessionStore: except (OSError, sqlite3.Error) as 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()) current_hash = self._digest(current_session_id) try: with self._connect() as connection: rows = connection.execute( "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", - (now,), + (now, principal_id), ).fetchall() except (OSError, sqlite3.Error) as exc: raise SessionStoreError("Session registry is temporarily unavailable") from exc diff --git a/tests/test_access_revocation.py b/tests/test_access_revocation.py index 0ca4bba..aa3e54d 100644 --- a/tests/test_access_revocation.py +++ b/tests/test_access_revocation.py @@ -99,6 +99,8 @@ def test_device_access_revocation_rolls_back_target_and_preserves_other_devices( 2_000, management_id=f"{slug}-management-id", device_label=label, + principal_id=42, + principal_login="timmy", ) sessions.mint_step_up( f"{slug}-session", @@ -122,7 +124,7 @@ def test_device_access_revocation_rolls_back_target_and_preserves_other_devices( ) 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: 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" ).fetchall() == [("laptop-management-id",), ("phone-management-id",)] 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",), + ] diff --git a/tests/test_push_notifications.py b/tests/test_push_notifications.py index 53e02f0..82d5672 100644 --- a/tests/test_push_notifications.py +++ b/tests/test_push_notifications.py @@ -1985,3 +1985,21 @@ async def test_subscription_fails_closed_when_existing_unread_baseline_is_unavai assert raised.value.status_code == 503 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 diff --git a/tests/test_session_store.py b/tests/test_session_store.py index 7e6ec9d..545cfd5 100644 --- a/tests/test_session_store.py +++ b/tests/test_session_store.py @@ -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) == {} +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): now = [1_000.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): store = SessionStore(tmp_path / "sessions.sqlite3", clock=lambda: 1_000.0) - store.activate("phone-session-secret", 2_000, device_label="Pixel 9") - store.activate("laptop-session-secret", 3_000, device_label="Work laptop") + store.activate( + "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.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): store = SessionStore(tmp_path / "sessions.sqlite3", clock=lambda: 1_000.0) - store.activate("phone", 2_000, device_label="Phone") - store.activate("laptop", 2_000, device_label="Laptop") - phone = next(device for device in store.list_active("laptop") if device.device_label == "Phone") + store.activate("phone", 2_000, device_label="Phone", principal_id=42) + store.activate("laptop", 2_000, device_label="Laptop", principal_id=42) + 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.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") assert store.is_active("existing-session", 2_000) is True - devices = store.list_active("existing-session") - assert len(devices) == 2 - assert next(device for device in devices if device.current).device_label == "Existing device" + devices = store.list_active("existing-session", principal_id=42) + assert devices == [] 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): 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( "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.consume_step_up(