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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"})
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user