From 2692b5a3ca3a16b9579a968be5fcf12e1ded814d Mon Sep 17 00:00:00 2001 From: BayerC Date: Fri, 17 Jul 2026 14:31:55 +0200 Subject: [PATCH 1/4] cookie solution --- README.md | 8 +++++++ conftest.py | 14 +++++++++++ src/open_cups/session_state.py | 41 ++++++++++++++++++++++++++++---- tests/unit/test_session_state.py | 25 +++++++++++++++++++ 4 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_session_state.py diff --git a/README.md b/README.md index ac1b6a1..a0d2f5f 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,14 @@ The presenter sees an aggregate view of the responses and can adjust the present +## Privacy + +The app stores a single functional cookie (`OPEN_CUPS_SESSION_ID`) holding a +random identifier. It keeps you attached to your room when your device +reconnects (e.g. after a phone locks) and is never used for tracking or shared +with third parties. As a strictly necessary cookie it does not require a +consent banner under GDPR/ePrivacy. + ## Run locally 1. `git clone https://github.com/BayerC/open_cups.git` diff --git a/conftest.py b/conftest.py index 7d4fa81..9eb2935 100644 --- a/conftest.py +++ b/conftest.py @@ -9,6 +9,20 @@ ] +@pytest.fixture(autouse=True) +def isolate_browser_cookies(monkeypatch: pytest.MonkeyPatch) -> None: + """Bypass the browser-backed cookie during tests. + + ``AppTest`` has no browser, so each simulated session gets a fresh identity + by minting a new id (empty cookie), matching how distinct browsers behave. + """ + monkeypatch.setattr("open_cups.session_state._read_session_cookie", lambda: None) + monkeypatch.setattr( + "open_cups.session_state._write_session_cookie", + lambda session_id: None, # noqa: ARG005 + ) + + class MockTime: def __init__(self, initial_time: float | None = None) -> None: self._current_time = initial_time if initial_time is not None else time.time() diff --git a/src/open_cups/session_state.py b/src/open_cups/session_state.py index 626c159..4677be5 100644 --- a/src/open_cups/session_state.py +++ b/src/open_cups/session_state.py @@ -1,21 +1,52 @@ import uuid import streamlit as st +import streamlit.components.v1 as components + +COOKIE_NAME = "OPEN_CUPS_SESSION_ID" +COOKIE_MAX_AGE_SECONDS = 60 * 60 * 24 * 30 # 30 days class SessionState: """Per-user session state wrapper. - See https://docs.streamlit.io/develop/api-reference/caching-and-state/st.session_state - for more details. + The session identity is persisted in a browser cookie so it survives + Streamlit session loss (a websocket drop while a phone is locked, a page + reload). The cookie is scoped to the browser and never travels in the URL, + so sharing a room link cannot leak a user's identity. + + See https://github.com/streamlit/streamlit/issues/10041 for the technique. """ def __init__(self) -> None: if "session_id" not in st.session_state: - existing = st.query_params.get("session_id") - st.session_state.session_id = existing or str(uuid.uuid4()) - st.query_params["session_id"] = st.session_state.session_id + st.session_state.session_id = _load_or_create_session_id() @property def session_id(self) -> str: return str(st.session_state.session_id) + + +def _load_or_create_session_id() -> str: + if existing := _read_session_cookie(): + return existing + session_id = str(uuid.uuid4()) + _write_session_cookie(session_id) + return session_id + + +def _read_session_cookie() -> str | None: # pragma: no cover + return st.context.cookies.get(COOKIE_NAME) + + +def _write_session_cookie(session_id: str) -> None: # pragma: no cover + # st.html() strips ', + height=0, + ) diff --git a/tests/unit/test_session_state.py b/tests/unit/test_session_state.py new file mode 100644 index 0000000..55656f5 --- /dev/null +++ b/tests/unit/test_session_state.py @@ -0,0 +1,25 @@ +import pytest + +from open_cups import session_state + + +def test_load_returns_existing_cookie(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(session_state, "_read_session_cookie", lambda: "existing-id") + assert session_state._load_or_create_session_id() == "existing-id" # noqa: SLF001 + + +def test_load_mints_and_persists_when_cookie_absent( + monkeypatch: pytest.MonkeyPatch, +) -> None: + written: list[str] = [] + monkeypatch.setattr(session_state, "_read_session_cookie", lambda: None) + monkeypatch.setattr( + session_state, + "_write_session_cookie", + written.append, + ) + + session_id = session_state._load_or_create_session_id() # noqa: SLF001 + + assert session_id + assert written == [session_id] From f28530ce9ae06d7e279e06d60af3a4763defb428 Mon Sep 17 00:00:00 2001 From: BayerC <42061449+BayerC@users.noreply.github.com> Date: Fri, 17 Jul 2026 14:42:17 +0200 Subject: [PATCH 2/4] change title such we can see we use the right branch --- src/open_cups/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/open_cups/app.py b/src/open_cups/app.py index ac4fc10..3e7ae15 100644 --- a/src/open_cups/app.py +++ b/src/open_cups/app.py @@ -32,7 +32,7 @@ def show_room_selection_screen(lobby: LobbyState) -> None: left, right = st.columns([2, 1]) with left: - st.title("Welcome to OpenCups") + st.title("Welcome to OpenCups with Cookie") st.write("Host or join a room to share feedback.") with right: st.image("assets/logo.png", width="content") From 82299a06b58f8d8a6b7fcd4a9608a03fe1032257 Mon Sep 17 00:00:00 2001 From: BayerC <42061449+BayerC@users.noreply.github.com> Date: Fri, 17 Jul 2026 15:25:17 +0200 Subject: [PATCH 3/4] debug panel --- src/open_cups/app.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/open_cups/app.py b/src/open_cups/app.py index 3e7ae15..fc619a7 100644 --- a/src/open_cups/app.py +++ b/src/open_cups/app.py @@ -5,6 +5,7 @@ from streamlit_autorefresh import st_autorefresh from open_cups.plots import show_room_statistics, show_status_history_chart +from open_cups.session_state import COOKIE_NAME from open_cups.state_provider import ( ClientState, HostState, @@ -32,7 +33,7 @@ def show_room_selection_screen(lobby: LobbyState) -> None: left, right = st.columns([2, 1]) with left: - st.title("Welcome to OpenCups with Cookie") + st.title("Welcome to OpenCups") st.write("Host or join a room to share feedback.") with right: st.image("assets/logo.png", width="content") @@ -256,10 +257,28 @@ def handle_question_submit() -> None: show_open_questions(client_state) +def _show_cookie_debug_panel(session_id: str) -> None: # pragma: no cover + try: + cookies = dict(st.context.cookies) + cookies_note = "" + except Exception as error: # noqa: BLE001 + cookies = {} + cookies_note = f" (read failed: {error})" + cookie_value = cookies.get(COOKIE_NAME) + st.code( + "DEBUG (temporary)\n" + f"session_id : {session_id}\n" + f"cookie : {cookie_value}{cookies_note}\n" + f"reused : {cookie_value == session_id}\n" + f"cookie_keys: {sorted(cookies)}", + ) + + def run() -> None: st_autorefresh(interval=AUTOREFRESH_INTERVAL_MS, key="data_refresh") state_provider = StateProvider() + _show_cookie_debug_panel(state_provider.context.session_state.session_id) cleanup = state_provider.get_cleanup(USER_REMOVAL_TIMEOUT_SECONDS) cleanup.cleanup_all() From 6ffab342b9570d7928650ea556ff7ebdcd622626 Mon Sep 17 00:00:00 2001 From: BayerC <42061449+BayerC@users.noreply.github.com> Date: Sat, 18 Jul 2026 10:39:24 +0200 Subject: [PATCH 4/4] fix --- README.md | 8 ----- conftest.py | 14 -------- src/open_cups/app.py | 19 ---------- src/open_cups/application_state.py | 6 ++++ src/open_cups/room.py | 8 +++++ src/open_cups/session_state.py | 57 ++++++++++++------------------ src/open_cups/state_provider.py | 2 +- tests/unit/test_room.py | 35 ++++++++++++++++++ tests/unit/test_session_state.py | 51 ++++++++++++++++---------- 9 files changed, 106 insertions(+), 94 deletions(-) diff --git a/README.md b/README.md index a0d2f5f..ac1b6a1 100644 --- a/README.md +++ b/README.md @@ -45,14 +45,6 @@ The presenter sees an aggregate view of the responses and can adjust the present -## Privacy - -The app stores a single functional cookie (`OPEN_CUPS_SESSION_ID`) holding a -random identifier. It keeps you attached to your room when your device -reconnects (e.g. after a phone locks) and is never used for tracking or shared -with third parties. As a strictly necessary cookie it does not require a -consent banner under GDPR/ePrivacy. - ## Run locally 1. `git clone https://github.com/BayerC/open_cups.git` diff --git a/conftest.py b/conftest.py index 9eb2935..7d4fa81 100644 --- a/conftest.py +++ b/conftest.py @@ -9,20 +9,6 @@ ] -@pytest.fixture(autouse=True) -def isolate_browser_cookies(monkeypatch: pytest.MonkeyPatch) -> None: - """Bypass the browser-backed cookie during tests. - - ``AppTest`` has no browser, so each simulated session gets a fresh identity - by minting a new id (empty cookie), matching how distinct browsers behave. - """ - monkeypatch.setattr("open_cups.session_state._read_session_cookie", lambda: None) - monkeypatch.setattr( - "open_cups.session_state._write_session_cookie", - lambda session_id: None, # noqa: ARG005 - ) - - class MockTime: def __init__(self, initial_time: float | None = None) -> None: self._current_time = initial_time if initial_time is not None else time.time() diff --git a/src/open_cups/app.py b/src/open_cups/app.py index fc619a7..ac4fc10 100644 --- a/src/open_cups/app.py +++ b/src/open_cups/app.py @@ -5,7 +5,6 @@ from streamlit_autorefresh import st_autorefresh from open_cups.plots import show_room_statistics, show_status_history_chart -from open_cups.session_state import COOKIE_NAME from open_cups.state_provider import ( ClientState, HostState, @@ -257,28 +256,10 @@ def handle_question_submit() -> None: show_open_questions(client_state) -def _show_cookie_debug_panel(session_id: str) -> None: # pragma: no cover - try: - cookies = dict(st.context.cookies) - cookies_note = "" - except Exception as error: # noqa: BLE001 - cookies = {} - cookies_note = f" (read failed: {error})" - cookie_value = cookies.get(COOKIE_NAME) - st.code( - "DEBUG (temporary)\n" - f"session_id : {session_id}\n" - f"cookie : {cookie_value}{cookies_note}\n" - f"reused : {cookie_value == session_id}\n" - f"cookie_keys: {sorted(cookies)}", - ) - - def run() -> None: st_autorefresh(interval=AUTOREFRESH_INTERVAL_MS, key="data_refresh") state_provider = StateProvider() - _show_cookie_debug_panel(state_provider.context.session_state.session_id) cleanup = state_provider.get_cleanup(USER_REMOVAL_TIMEOUT_SECONDS) cleanup.cleanup_all() diff --git a/src/open_cups/application_state.py b/src/open_cups/application_state.py index 1d21dd6..cb0653e 100644 --- a/src/open_cups/application_state.py +++ b/src/open_cups/application_state.py @@ -15,6 +15,12 @@ def get_session_room(self, session_id: str) -> Room | None: return room return None + def is_session_live(self, session_id: str, timeout_seconds: float) -> bool: + room = self.get_session_room(session_id) + if room is None: + return False + return room.is_session_live(session_id, timeout_seconds) + def create_room(self, room_id: str, session_id: str) -> None: room = Room(room_id, session_id) self.rooms[room_id] = room diff --git a/src/open_cups/room.py b/src/open_cups/room.py index fb55ca5..f4054c6 100644 --- a/src/open_cups/room.py +++ b/src/open_cups/room.py @@ -47,6 +47,14 @@ def has_session(self, session_id: str) -> bool: return True return bool(self.is_host(session_id)) + def is_session_live(self, session_id: str, timeout_seconds: float) -> bool: + current_time = time.time() + if self.is_host(session_id): + return current_time - self._host_last_seen <= timeout_seconds + if session_id not in self._sessions: + return False + return current_time - self._sessions[session_id].last_seen <= timeout_seconds + def get_participants_by_activity( self, inactivity_timeout_seconds: float, diff --git a/src/open_cups/session_state.py b/src/open_cups/session_state.py index 4677be5..3b9ad53 100644 --- a/src/open_cups/session_state.py +++ b/src/open_cups/session_state.py @@ -1,52 +1,41 @@ import uuid import streamlit as st -import streamlit.components.v1 as components -COOKIE_NAME = "OPEN_CUPS_SESSION_ID" -COOKIE_MAX_AGE_SECONDS = 60 * 60 * 24 * 30 # 30 days +from open_cups.application_state import ApplicationState + +# A live session refreshes its last_seen every autorefresh tick (~2s), so this +# threshold tolerates a couple of missed ticks before treating it as stale. +SESSION_LIVENESS_TIMEOUT_SECONDS = 10 class SessionState: """Per-user session state wrapper. - The session identity is persisted in a browser cookie so it survives - Streamlit session loss (a websocket drop while a phone is locked, a page - reload). The cookie is scoped to the browser and never travels in the URL, - so sharing a room link cannot leak a user's identity. - - See https://github.com/streamlit/streamlit/issues/10041 for the technique. + The session id is kept in the URL so it survives a websocket drop (a phone + locking, a page reload). To stop a copy-pasted link from cloning identity, + a session id from the URL is only adopted when that session is currently + stale (a genuine reconnect); if it is still live, the visitor is treated as + a new user. See https://github.com/BayerC/open_cups/issues/165. """ - def __init__(self) -> None: + def __init__(self, application_state: ApplicationState) -> None: if "session_id" not in st.session_state: - st.session_state.session_id = _load_or_create_session_id() + st.session_state.session_id = _resolve_session_id(application_state) + st.query_params["session_id"] = st.session_state.session_id @property def session_id(self) -> str: return str(st.session_state.session_id) -def _load_or_create_session_id() -> str: - if existing := _read_session_cookie(): - return existing - session_id = str(uuid.uuid4()) - _write_session_cookie(session_id) - return session_id - - -def _read_session_cookie() -> str | None: # pragma: no cover - return st.context.cookies.get(COOKIE_NAME) - - -def _write_session_cookie(session_id: str) -> None: # pragma: no cover - # st.html() strips ', - height=0, - ) +def _resolve_session_id(application_state: ApplicationState) -> str: + url_session_id = st.query_params.get("session_id") + if url_session_id is None: + return str(uuid.uuid4()) + if application_state.is_session_live( + url_session_id, + SESSION_LIVENESS_TIMEOUT_SECONDS, + ): + return str(uuid.uuid4()) + return url_session_id diff --git a/src/open_cups/state_provider.py b/src/open_cups/state_provider.py index d93b0f3..fb1f538 100644 --- a/src/open_cups/state_provider.py +++ b/src/open_cups/state_provider.py @@ -105,7 +105,7 @@ def cleanup_all(self) -> None: class Context: def __init__(self) -> None: self.application_state: ApplicationState = self._get_application_state() - self.session_state = SessionState() + self.session_state = SessionState(self.application_state) @staticmethod @st.cache_resource diff --git a/tests/unit/test_room.py b/tests/unit/test_room.py index 73b77e7..b194842 100644 --- a/tests/unit/test_room.py +++ b/tests/unit/test_room.py @@ -104,6 +104,41 @@ def test_integration_with_stats_tracker(monkeypatch: pytest.MonkeyPatch) -> None assert room.get_status_history() == [] +def test_is_session_live_true_for_recent_host(mock_time: MockTime) -> None: + mock_time.current_time = 0.0 + room = Room("room-id", "host-id") + + mock_time.current_time = 5.0 + + assert room.is_session_live("host-id", timeout_seconds=10) + + +def test_is_session_live_true_for_recent_client(mock_time: MockTime) -> None: + mock_time.current_time = 0.0 + room = Room("room-id", "host-id") + room.set_session_status("user", UserStatus.GREEN) + + mock_time.current_time = 5.0 + + assert room.is_session_live("user", timeout_seconds=10) + + +def test_is_session_live_false_for_stale_client(mock_time: MockTime) -> None: + mock_time.current_time = 0.0 + room = Room("room-id", "host-id") + room.set_session_status("user", UserStatus.GREEN) + + mock_time.current_time = 20.0 + + assert not room.is_session_live("user", timeout_seconds=10) + + +def test_is_session_live_false_for_unknown_session() -> None: + room = Room("room-id", "host-id") + + assert not room.is_session_live("unknown-id", timeout_seconds=10) + + def test_get_participants_by_activity_separates_active_and_inactive( mock_time: MockTime, ) -> None: diff --git a/tests/unit/test_session_state.py b/tests/unit/test_session_state.py index 55656f5..fbd2afb 100644 --- a/tests/unit/test_session_state.py +++ b/tests/unit/test_session_state.py @@ -1,25 +1,40 @@ -import pytest +from streamlit.testing.v1 import AppTest -from open_cups import session_state +from tests.bdd.fixture import run_wrapper +from tests.bdd.test_helper import get_room_id -def test_load_returns_existing_cookie(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr(session_state, "_read_session_cookie", lambda: "existing-id") - assert session_state._load_or_create_session_id() == "existing-id" # noqa: SLF001 +def _resolved_session_id(app: AppTest) -> str: + value = app.query_params["session_id"] + resolved = value[0] if isinstance(value, list) else value + return str(resolved) -def test_load_mints_and_persists_when_cookie_absent( - monkeypatch: pytest.MonkeyPatch, -) -> None: - written: list[str] = [] - monkeypatch.setattr(session_state, "_read_session_cookie", lambda: None) - monkeypatch.setattr( - session_state, - "_write_session_cookie", - written.append, - ) +def test_fresh_visitor_gets_a_session_id() -> None: + app = AppTest.from_function(run_wrapper) + app.run() - session_id = session_state._load_or_create_session_id() # noqa: SLF001 + assert _resolved_session_id(app) - assert session_id - assert written == [session_id] + +def test_url_session_id_adopted_when_not_live() -> None: + app = AppTest.from_function(run_wrapper) + app.query_params["session_id"] = "stale-or-unknown-id" + app.run() + + assert _resolved_session_id(app) == "stale-or-unknown-id" + + +def test_copy_pasted_live_session_forks_to_new_user() -> None: + host = AppTest.from_function(run_wrapper) + host.run() + host.button(key="start_room").click().run() + host_session_id = _resolved_session_id(host) + room_id = get_room_id(host) + + visitor = AppTest.from_function(run_wrapper) + visitor.query_params["room_id"] = room_id + visitor.query_params["session_id"] = host_session_id + visitor.run() + + assert _resolved_session_id(visitor) != host_session_id