From a9eb97e6e1a0a651837efd4583d53d8fad585465 Mon Sep 17 00:00:00 2001 From: Walter Oggioni Date: Wed, 14 Oct 2026 15:56:29 +0000 Subject: [PATCH] Stop requesting the user email from the OIDC provider OIDCConfig now asks for the openid and profile scopes only: kaya-oidc's default would also request email, which the platform never stores or exposes. The display-name fallback drops the email claim too (preferred_username, name, then the sub). Tests pin the configured scopes and cover an IdP that still returns an email claim: the claim is ignored and absent from /api/me. --- server/packages/tavolo-app/src/tavolo/app.py | 3 +++ .../src/tavolo/platform/auth.py | 4 ++-- .../tavolo-platform/tests/test_auth.py | 6 +++-- server/tests/helpers/oidc.py | 22 ++++++++++++++----- server/tests/test_routes_auth.py | 9 +++++++- server/tests/test_routes_me.py | 3 ++- 6 files changed, 36 insertions(+), 11 deletions(-) diff --git a/server/packages/tavolo-app/src/tavolo/app.py b/server/packages/tavolo-app/src/tavolo/app.py index 76dfe16..c76ce6f 100644 --- a/server/packages/tavolo-app/src/tavolo/app.py +++ b/server/packages/tavolo-app/src/tavolo/app.py @@ -203,6 +203,9 @@ oidc = OIDCProviders( redirect_uri=provider.redirect_uri, post_login_redirect=provider.post_login_redirect, post_logout_redirect=provider.post_logout_redirect, + # kaya-oidc defaults to openid+email+profile; the app neither + # needs nor stores the email claim (profile covers the names). + scopes=("openid", "profile"), fetch_userinfo=True, ) for provider in settings.oidc_providers diff --git a/server/packages/tavolo-platform/src/tavolo/platform/auth.py b/server/packages/tavolo-platform/src/tavolo/platform/auth.py index c33d3ec..1024ffa 100644 --- a/server/packages/tavolo-platform/src/tavolo/platform/auth.py +++ b/server/packages/tavolo-platform/src/tavolo/platform/auth.py @@ -51,9 +51,9 @@ def display_name(user: OIDCUser) -> str: ``preferred_username`` is preferred: it is the account's stable handle, while ``name`` is a free-form display name a user may change (and two - accounts can share). ``email`` is a last resort before the ``sub``. + accounts can share). The ``sub`` is the last resort. """ - for key in ("preferred_username", "name", "email"): + for key in ("preferred_username", "name"): value = user.get(key) if isinstance(value, str) and value: return value diff --git a/server/packages/tavolo-platform/tests/test_auth.py b/server/packages/tavolo-platform/tests/test_auth.py index 382186d..6967f8c 100644 --- a/server/packages/tavolo-platform/tests/test_auth.py +++ b/server/packages/tavolo-platform/tests/test_auth.py @@ -106,9 +106,11 @@ class DisplayNameTest(unittest.TestCase): user = OIDCUser({"sub": "alice", "name": "Alice"}) self.assertEqual("Alice", display_name(user)) - def test_falls_back_to_email(self) -> None: + def test_ignores_email(self) -> None: + # The app requests only the openid and profile scopes; an email + # claim the provider still returns must not leak into the name. user = OIDCUser({"sub": "alice", "email": "alice@example.com"}) - self.assertEqual("alice@example.com", display_name(user)) + self.assertEqual("alice", display_name(user)) def test_falls_back_to_sub(self) -> None: user = OIDCUser({"sub": "alice"}) diff --git a/server/tests/helpers/oidc.py b/server/tests/helpers/oidc.py index 8f866f0..87b98ab 100644 --- a/server/tests/helpers/oidc.py +++ b/server/tests/helpers/oidc.py @@ -23,14 +23,26 @@ from tavolo.platform import auth _PROVIDER = next(iter(oidc.providers)) -def make_user(sub: str, name: Optional[str] = None) -> OIDCUser: - return OIDCUser({"sub": sub, "preferred_username": name or sub}) +def make_user( + sub: str, name: Optional[str] = None, email: Optional[str] = None +) -> OIDCUser: + claims = {"sub": sub, "preferred_username": name or sub} + if email: + claims["email"] = email + return OIDCUser(claims) @contextlib.contextmanager -def oidc_user(sub: str, name: Optional[str] = None) -> Iterator[OIDCUser]: - """Context manager: make ``oidc.get_users`` return this user.""" - user = make_user(sub, name) +def oidc_user( + sub: str, name: Optional[str] = None, email: Optional[str] = None +) -> Iterator[OIDCUser]: + """Context manager: make ``oidc.get_users`` return this user. + + ``email`` is optional so tests can simulate an IdP that still returns + the claim: the app requests only the ``openid`` and ``profile`` scopes + and ignores email where present. + """ + user = make_user(sub, name, email) patcher = _mock.patch.object(oidc, "get_users", return_value={_PROVIDER: user}) patcher.start() try: diff --git a/server/tests/test_routes_auth.py b/server/tests/test_routes_auth.py index 8863a6d..6fd58f6 100644 --- a/server/tests/test_routes_auth.py +++ b/server/tests/test_routes_auth.py @@ -10,7 +10,7 @@ import unittest from httpx import ASGITransport, AsyncClient -from tavolo.app import app +from tavolo.app import app, oidc from tests.helpers import async_test @@ -33,3 +33,10 @@ class AuthRoutesTest(unittest.TestCase): response = await client.get("/auth/login", follow_redirects=False) self.assertEqual(302, response.status_code) self.assertEqual("/auth/default/login", response.headers["location"]) + + def test_authorization_scopes_exclude_email(self) -> None: + # Only the scopes the app uses are requested: openid (sub) and + # profile (preferred_username/name). kaya-oidc's default would also + # request email, which the app neither needs nor stores. + mixin = oidc.provider("default") + self.assertEqual(("openid", "profile"), tuple(mixin._config.scopes)) diff --git a/server/tests/test_routes_me.py b/server/tests/test_routes_me.py index 0c84ef1..7449853 100644 --- a/server/tests/test_routes_me.py +++ b/server/tests/test_routes_me.py @@ -19,9 +19,10 @@ class MeRouteTest(unittest.TestCase): async def test_me_authenticated(self) -> None: transport = ASGITransport(app=app) async with AsyncClient(transport=transport, base_url="http://127.0.0.1") as client: - with oidc_user("alice"): + with oidc_user("alice", email="alice@example.com"): response = await client.get("/api/me") self.assertEqual(200, response.status_code) + # The email claim the IdP returns is neither stored nor exposed. self.assertEqual({"sub": "alice", "name": "alice"}, response.json()) @async_test