diff --git a/server/packages/tavolo-app/pyproject.toml b/server/packages/tavolo-app/pyproject.toml index d093db8..bc0764a 100644 --- a/server/packages/tavolo-app/pyproject.toml +++ b/server/packages/tavolo-app/pyproject.toml @@ -34,7 +34,7 @@ dev = [ "httpx-ws", ] otel = [ - "kaya-otel>=0.0.5", + "kaya-otel>=0.0.6", "opentelemetry-instrumentation-asyncpg>=0.66b0", "opentelemetry-instrumentation-redis>=0.66b0", "opentelemetry-instrumentation-system-metrics>=0.66b0", diff --git a/server/packages/tavolo-app/src/tavolo/telemetry.py b/server/packages/tavolo-app/src/tavolo/telemetry.py index dbce9a9..a68a8df 100644 --- a/server/packages/tavolo-app/src/tavolo/telemetry.py +++ b/server/packages/tavolo-app/src/tavolo/telemetry.py @@ -28,12 +28,13 @@ import hashlib import logging from dataclasses import dataclass from logging import getLogger -from typing import TYPE_CHECKING, Any, Dict, Mapping, Optional, Union +from typing import TYPE_CHECKING, Any, Dict, Optional, Union from kaya.core import KayaApp, KayaMixin from kaya.session import InMemorySessionStore, SessionStore from kaya.session.redis import RedisSessionStore from redis.asyncio import Redis +from tavolo.platform.auth import session_user from tavolo.platform.store import GameStore from .config import Settings @@ -108,8 +109,8 @@ def session_tag_hook(span: Any, carrier: Any) -> None: "session.id", hashlib.sha256(session_id.encode()).hexdigest()[:16], ) - user = session.get("oidc_user") - if isinstance(user, Mapping): + user = session_user(session) + if user is not None: sub = user.get("sub") if sub: span.set_attribute("user.id", str(sub)) diff --git a/server/packages/tavolo-platform/pyproject.toml b/server/packages/tavolo-platform/pyproject.toml index 1d6fd56..85ba8d5 100644 --- a/server/packages/tavolo-platform/pyproject.toml +++ b/server/packages/tavolo-platform/pyproject.toml @@ -9,7 +9,7 @@ description = "Game-independent multiplayer game platform (lobby, live play, mat readme = "README.md" requires-python = ">=3.10" dependencies = [ - "kaya-core>=0.0.5", + "kaya-core>=0.0.6", "kaya-session", "kaya-oidc", "kaya-openapi", diff --git a/server/packages/tavolo-platform/src/tavolo/platform/auth.py b/server/packages/tavolo-platform/src/tavolo/platform/auth.py index 5ce2199..d9d4eca 100644 --- a/server/packages/tavolo-platform/src/tavolo/platform/auth.py +++ b/server/packages/tavolo-platform/src/tavolo/platform/auth.py @@ -10,6 +10,28 @@ from typing import Any, Callable, Mapping, Optional from kaya.core import HttpContext, WebSocket from kaya.oidc import OIDCMixin, OIDCUser +from kaya.session import Session + +# kaya-oidc namespaces its session state per provider as +# ``oidc:{provider}:user`` (since 0.0.6), so the lookup cannot assume a +# fixed key or provider name. +_OIDC_PREFIX = "oidc:" +_USER_SUFFIX = ":user" + + +def session_user(session: Session) -> Optional[OIDCUser]: + """Return the authenticated user stored in a session, if any. + + The kaya-oidc mixin writes the userinfo at login under a per-provider + key, so any provider's user is accepted (the configured provider is + ``default`` unless the app says otherwise). + """ + for key in session: + if key.startswith(_OIDC_PREFIX) and key.endswith(_USER_SUFFIX): + claims = session.get(key) + if isinstance(claims, Mapping): + return OIDCUser(claims) + return None def get_ws_user(ws: WebSocket) -> Optional[OIDCUser]: @@ -21,10 +43,7 @@ def get_ws_user(ws: WebSocket) -> Optional[OIDCUser]: session = getattr(ws, "session", None) if session is None: return None - claims = session.get("oidc_user") - if not isinstance(claims, Mapping): - return None - return OIDCUser(claims) + return session_user(session) def display_name(user: OIDCUser) -> str: diff --git a/server/packages/tavolo-platform/tests/test_auth.py b/server/packages/tavolo-platform/tests/test_auth.py new file mode 100644 index 0000000..21f6a40 --- /dev/null +++ b/server/packages/tavolo-platform/tests/test_auth.py @@ -0,0 +1,65 @@ +"""Tests for the auth helpers' OIDC user lookup. + +kaya-oidc (0.0.6) namespaces session state per provider as +``oidc:{provider}:user``; these tests pin that contract so a kaya +upgrade that changes it again cannot silently break websocket +authentication or span tagging. +""" +from __future__ import annotations + +import unittest +from types import SimpleNamespace +from typing import Any + +from kaya.session import Session + +from tavolo.platform.auth import get_ws_user, session_user + + +def _ws(session: Any) -> Any: + return SimpleNamespace(session=session) + + +class SessionUserTest(unittest.TestCase): + def test_finds_default_provider_user(self) -> None: + user = session_user(Session("sid", {"oidc:default:user": {"sub": "alice"}})) + assert user is not None + self.assertEqual("alice", user.sub) + + def test_finds_user_of_any_provider(self) -> None: + user = session_user(Session("sid", {"oidc:google:user": {"sub": "alice"}})) + assert user is not None + self.assertEqual("alice", user.sub) + + def test_ignores_flow_state_and_tokens(self) -> None: + session = Session( + "sid", + { + "oidc:default:state": "state", + "oidc:default:id_token": "token", + "oidc:default:access_token": "token", + }, + ) + self.assertIsNone(session_user(session)) + + def test_ignores_legacy_unprefixed_user_key(self) -> None: + session = Session("sid", {"oidc_user": {"sub": "alice"}}) + self.assertIsNone(session_user(session)) + + def test_ignores_malformed_user_claims(self) -> None: + session = Session("sid", {"oidc:default:user": "alice"}) + self.assertIsNone(session_user(session)) + + +class GetWsUserTest(unittest.TestCase): + def test_returns_user_from_provider_namespaced_session(self) -> None: + session = Session( + "sid", + {"oidc:default:user": {"sub": "alice", "preferred_username": "Alice"}}, + ) + user = get_ws_user(_ws(session)) + assert user is not None + self.assertEqual("alice", user.sub) + + def test_missing_session_returns_none(self) -> None: + self.assertIsNone(get_ws_user(SimpleNamespace())) diff --git a/server/requirements.txt b/server/requirements.txt index 55dc21a..942b13e 100644 --- a/server/requirements.txt +++ b/server/requirements.txt @@ -63,7 +63,7 @@ idna==3.20 # httpx iso8601==2.1.0 # via tortoise-orm -kaya-core==0.0.5 +kaya-core==0.0.6 # via # kaya-cors # kaya-oidc @@ -72,22 +72,22 @@ kaya-core==0.0.5 # kaya-rsgi # kaya-session # tavolo-app (pyproject.toml) -kaya-cors==0.0.5 +kaya-cors==0.0.6 # via tavolo-app (pyproject.toml) -kaya-oidc==0.0.5 +kaya-oidc==0.0.6 # via tavolo-app (pyproject.toml) -kaya-openapi==0.0.5 +kaya-openapi==0.0.6 # via tavolo-app (pyproject.toml) -kaya-otel==0.0.5 +kaya-otel==0.0.6 # via tavolo-app (pyproject.toml) -kaya-rsgi==0.0.5 +kaya-rsgi==0.0.6 # via tavolo-app (pyproject.toml) -kaya-session==0.0.5 +kaya-session==0.0.6 # via # kaya-oidc # kaya-session-redis # tavolo-app (pyproject.toml) -kaya-session-redis==0.0.5 +kaya-session-redis==0.0.6 # via tavolo-app (pyproject.toml) opentelemetry-api==1.45.0 # via diff --git a/server/tests/test_telemetry.py b/server/tests/test_telemetry.py index 8f1e819..713c26d 100644 --- a/server/tests/test_telemetry.py +++ b/server/tests/test_telemetry.py @@ -151,7 +151,7 @@ class SessionTagHookTest(unittest.TestCase): def test_tags_authenticated_session(self) -> None: span = _RecordedSpan() carrier = SimpleNamespace( - session=Session("sid", {"oidc_user": {"sub": "alice"}}) + session=Session("sid", {"oidc:default:user": {"sub": "alice"}}) ) session_tag_hook(span, carrier) self.assertEqual( @@ -160,6 +160,23 @@ class SessionTagHookTest(unittest.TestCase): ) self.assertEqual("alice", span.attributes["user.id"]) + def test_tags_user_of_any_provider(self) -> None: + span = _RecordedSpan() + carrier = SimpleNamespace( + session=Session("sid", {"oidc:google:user": {"sub": "alice"}}) + ) + session_tag_hook(span, carrier) + self.assertEqual("alice", span.attributes["user.id"]) + + def test_ignores_legacy_unprefixed_user_key(self) -> None: + span = _RecordedSpan() + carrier = SimpleNamespace( + session=Session("sid", {"oidc_user": {"sub": "alice"}}) + ) + session_tag_hook(span, carrier) + self.assertIn("session.id", span.attributes) + self.assertNotIn("user.id", span.attributes) + def test_tags_session_without_user(self) -> None: span = _RecordedSpan() session_tag_hook(span, SimpleNamespace(session=Session("sid", {}))) @@ -169,7 +186,7 @@ class SessionTagHookTest(unittest.TestCase): def test_tags_user_when_session_id_not_yet_minted(self) -> None: span = _RecordedSpan() carrier = SimpleNamespace( - session=Session(None, {"oidc_user": {"sub": "alice"}}) + session=Session(None, {"oidc:default:user": {"sub": "alice"}}) ) session_tag_hook(span, carrier) self.assertNotIn("session.id", span.attributes) @@ -182,7 +199,7 @@ class SessionTagHookTest(unittest.TestCase): def test_malformed_user_is_ignored(self) -> None: span = _RecordedSpan() - carrier = SimpleNamespace(session=Session("sid", {"oidc_user": "alice"})) + carrier = SimpleNamespace(session=Session("sid", {"oidc:default:user": "alice"})) session_tag_hook(span, carrier) self.assertIn("session.id", span.attributes) self.assertNotIn("user.id", span.attributes) @@ -224,7 +241,7 @@ class SessionTagIntegrationTest(unittest.TestCase): async def login(ctx: HttpContext) -> None: session = ctx.session assert session is not None - session["oidc_user"] = {"sub": "alice"} + session["oidc:default:user"] = {"sub": "alice"} await ctx.send_str(200, "ok") @app.GET("/me")