diff --git a/server/dev/mockoauth/config.json b/server/dev/mockoauth/config.json index c9908d3..5dea3f7 100644 --- a/server/dev/mockoauth/config.json +++ b/server/dev/mockoauth/config.json @@ -11,7 +11,8 @@ "claims": { "sub": "alice", "preferred_username": "alice", - "name": "Alice" + "name": "Alice", + "picture": "https://i.pravatar.cc/128?u=alice" } }, { @@ -20,7 +21,8 @@ "claims": { "sub": "bob", "preferred_username": "bob", - "name": "Bob" + "name": "Bob", + "picture": "https://i.pravatar.cc/128?u=bob" } }, { diff --git a/server/packages/tavolo-platform/src/tavolo/platform/auth.py b/server/packages/tavolo-platform/src/tavolo/platform/auth.py index 1024ffa..edb870e 100644 --- a/server/packages/tavolo-platform/src/tavolo/platform/auth.py +++ b/server/packages/tavolo-platform/src/tavolo/platform/auth.py @@ -60,6 +60,19 @@ def display_name(user: OIDCUser) -> str: return user.sub +def picture(user: OIDCUser) -> Optional[str]: + """The user's avatar URL, when the provider supplies a usable one. + + Only absolute ``http(s)`` URLs are exported: the claim ends up in an + ```` in the SPA, where relative or script-ish schemes have no + business. + """ + value = user.picture + if isinstance(value, str) and value.startswith(("http://", "https://")): + return value + return None + + def current_user(oidc: OIDCProviders, ctx: HttpContext) -> Optional[OIDCUser]: """Return the authenticated user of an HTTP request, if any. diff --git a/server/packages/tavolo-platform/src/tavolo/platform/openapi.py b/server/packages/tavolo-platform/src/tavolo/platform/openapi.py index a789621..32874e9 100644 --- a/server/packages/tavolo-platform/src/tavolo/platform/openapi.py +++ b/server/packages/tavolo-platform/src/tavolo/platform/openapi.py @@ -126,8 +126,9 @@ ME_SCHEMA = _object( { "sub": {"type": "string"}, "name": {"type": "string"}, + "picture": _nullable({"type": "string"}), }, - ("sub", "name"), + ("sub", "name", "picture"), ) AUTH_PROVIDER_SCHEMA = _object( diff --git a/server/packages/tavolo-platform/src/tavolo/platform/routes/me.py b/server/packages/tavolo-platform/src/tavolo/platform/routes/me.py index 285fa8a..46a924e 100644 --- a/server/packages/tavolo-platform/src/tavolo/platform/routes/me.py +++ b/server/packages/tavolo-platform/src/tavolo/platform/routes/me.py @@ -6,7 +6,7 @@ from typing import TYPE_CHECKING from kaya.core import HttpContext, KayaApp from kaya.openapi import operation, schema_ref -from ..auth import current_user, display_name, require_auth +from ..auth import current_user, display_name, picture, require_auth from ..http import send_json from ..openapi import error_response, json_response @@ -18,7 +18,9 @@ def register(app: KayaApp, platform: "Platform") -> None: @app.GET("/api/me") @operation(summary="Current user", description="Returns the authenticated user's identity from the " - "session; 401 when not logged in.", + "session, including the profile picture URL when " + "the provider supplies one; 401 when not logged " + "in.", tags=["auth"], responses={ 200: json_response("The current user", schema_ref("Me")), @@ -28,4 +30,12 @@ def register(app: KayaApp, platform: "Platform") -> None: async def me(ctx: HttpContext) -> None: user = current_user(platform.oidc, ctx) assert user is not None # enforced by @require_auth - await send_json(ctx, 200, {"sub": user.sub, "name": display_name(user)}) + await send_json( + ctx, + 200, + { + "sub": user.sub, + "name": display_name(user), + "picture": picture(user), + }, + ) diff --git a/server/packages/tavolo-platform/tests/test_auth.py b/server/packages/tavolo-platform/tests/test_auth.py index 6967f8c..e7470e9 100644 --- a/server/packages/tavolo-platform/tests/test_auth.py +++ b/server/packages/tavolo-platform/tests/test_auth.py @@ -19,6 +19,7 @@ from tavolo.platform.auth import ( current_user, display_name, get_ws_user, + picture, require_auth, session_user, ) @@ -121,6 +122,23 @@ class DisplayNameTest(unittest.TestCase): self.assertEqual("alice", display_name(user)) +class PictureTest(unittest.TestCase): + """Only absolute http(s) picture claims are exported to the SPA.""" + + def test_uses_an_absolute_http_url(self) -> None: + user = OIDCUser({"sub": "alice", "picture": "https://cdn.example.com/a.png"}) + self.assertEqual("https://cdn.example.com/a.png", picture(user)) + + def test_rejects_relative_and_script_schemes(self) -> None: + for value in ("/relative.png", "javascript:alert(1)", "data:image/png;base64,x"): + with self.subTest(picture=value): + user = OIDCUser({"sub": "alice", "picture": value}) + self.assertIsNone(picture(user)) + + def test_missing_claim_is_none(self) -> None: + self.assertIsNone(picture(OIDCUser({"sub": "alice"}))) + + class CurrentUserTest(unittest.TestCase): def test_anonymous_session_has_no_user(self) -> None: self.assertIsNone(current_user(_providers("default"), _ctx({}))) diff --git a/server/packages/tavolo-platform/tests/test_routes.py b/server/packages/tavolo-platform/tests/test_routes.py index 89c8dc4..08aa30b 100644 --- a/server/packages/tavolo-platform/tests/test_routes.py +++ b/server/packages/tavolo-platform/tests/test_routes.py @@ -310,7 +310,10 @@ class MeRouteTest(unittest.TestCase): with oidc_user(platform.oidc, "alice"): response = await client.get("/api/me") self.assertEqual(200, response.status_code) - self.assertEqual({"sub": "alice", "name": "alice"}, response.json()) + self.assertEqual( + {"sub": "alice", "name": "alice", "picture": None}, + response.json(), + ) @async_test async def test_me_unauthenticated(self) -> None: diff --git a/server/tests/helpers/oidc.py b/server/tests/helpers/oidc.py index 87b98ab..2652395 100644 --- a/server/tests/helpers/oidc.py +++ b/server/tests/helpers/oidc.py @@ -24,25 +24,34 @@ _PROVIDER = next(iter(oidc.providers)) def make_user( - sub: str, name: Optional[str] = None, email: Optional[str] = None + sub: str, + name: Optional[str] = None, + email: Optional[str] = None, + picture: Optional[str] = None, ) -> OIDCUser: claims = {"sub": sub, "preferred_username": name or sub} if email: claims["email"] = email + if picture: + claims["picture"] = picture return OIDCUser(claims) @contextlib.contextmanager def oidc_user( - sub: str, name: Optional[str] = None, email: Optional[str] = None + sub: str, + name: Optional[str] = None, + email: Optional[str] = None, + picture: 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. + and ignores email where present. ``picture`` simulates the profile + scope's avatar claim. """ - user = make_user(sub, name, email) + user = make_user(sub, name, email, picture) patcher = _mock.patch.object(oidc, "get_users", return_value={_PROVIDER: user}) patcher.start() try: diff --git a/server/tests/test_routes_me.py b/server/tests/test_routes_me.py index 7449853..619d0bc 100644 --- a/server/tests/test_routes_me.py +++ b/server/tests/test_routes_me.py @@ -22,8 +22,33 @@ class MeRouteTest(unittest.TestCase): 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()) + # The email claim the IdP returns is neither stored nor exposed; + # no picture claim in this request. + self.assertEqual( + {"sub": "alice", "name": "alice", "picture": None}, + response.json(), + ) + + @async_test + async def test_me_includes_the_profile_picture(self) -> None: + transport = ASGITransport(app=app) + async with AsyncClient(transport=transport, base_url="http://127.0.0.1") as client: + with oidc_user("alice", picture="https://cdn.example.com/alice.png"): + response = await client.get("/api/me") + self.assertEqual(200, response.status_code) + self.assertEqual( + "https://cdn.example.com/alice.png", + response.json()["picture"], + ) + + @async_test + async def test_me_rejects_non_http_picture_claims(self) -> None: + transport = ASGITransport(app=app) + async with AsyncClient(transport=transport, base_url="http://127.0.0.1") as client: + with oidc_user("alice", picture="javascript:alert(1)"): + response = await client.get("/api/me") + self.assertEqual(200, response.status_code) + self.assertIsNone(response.json()["picture"]) @async_test async def test_me_unauthenticated(self) -> None: diff --git a/web/src/components/avatar.rs b/web/src/components/avatar.rs new file mode 100644 index 0000000..2dba59c --- /dev/null +++ b/web/src/components/avatar.rs @@ -0,0 +1,183 @@ +//! User avatar: the provider's `picture` claim when available, a +//! deterministic identicon generated from the OIDC `sub` otherwise. +//! +//! The identicon is a 5x5 mirrored pixel grid derived from an inline +//! FNV-1a hash. The hash is implemented here (instead of +//! `DefaultHasher`) so the picture is stable across Rust versions and +//! the helper functions stay unit-testable on the host, with no DOM. + +use sycamore::prelude::*; + +/// FNV-1a 64-bit: a stable hash for identicon seeds. +fn fnv1a(input: &str) -> u64 { + let mut hash: u64 = 0xcbf2_9ce4_8422_2325; + for byte in input.as_bytes() { + hash ^= u64::from(*byte); + hash = hash.wrapping_mul(0x0000_0100_0000_01b3); + } + hash +} + +/// The identicon hue (0..360) for a seed. +fn hue_for(seed: &str) -> u32 { + (fnv1a(seed) % 360) as u32 +} + +/// The identicon cell grid as 5 columns x 5 rows. Only 15 bits (3 +/// columns) are derived from the hash; the left half is mirrored onto +/// the right so the icon is symmetric. The center cell is forced on so +/// an unlucky hash can never produce a blank icon. +fn cells_for(seed: &str) -> [[bool; 5]; 5] { + let hash = fnv1a(seed); + let mut half = [false; 15]; + for (index, cell) in half.iter_mut().enumerate() { + *cell = (hash >> index) & 1 == 1; + } + if !half.iter().any(|set| *set) { + half[8] = true; // center of the half grid: row 2, column 2 + } + let mut cells = [[false; 5]; 5]; + for row in 0..5 { + for column in 0..3 { + let set = half[row * 3 + column]; + cells[row][column] = set; + if column < 2 { + cells[row][4 - column] = set; + } + } + } + cells +} + +/// Only absolute http(s) picture URLs are rendered. The backend filters +/// the claim the same way; this is the client-side half of the rule. +fn safe_picture_url(url: &str) -> Option { + let trimmed = url.trim(); + let lower = trimmed.to_ascii_lowercase(); + if lower.starts_with("http://") || lower.starts_with("https://") { + Some(trimmed.to_string()) + } else { + None + } +} + +/// The user's avatar: their profile picture, or an identicon generated +/// from the `sub`. A picture that fails to load falls back to the +/// identicon as well. +pub fn avatar(sub: &str, picture: Option<&str>) -> View { + let Some(url) = picture.and_then(safe_picture_url) else { + return identicon(sub); + }; + let seed = sub.to_string(); + let failed = create_signal(false); + view! { + (move || { + if failed.get() { + identicon(&seed) + } else { + let src = url.clone(); + view! { + img(class="avatar", src=src, alt="", + on:error=move |_| failed.set(true)) + } + } + }) + } +} + +/// The generated fallback avatar. +fn identicon(seed: &str) -> View { + let hue = hue_for(seed); + let background = format!("hsl({hue}, 45%, 22%)"); + let foreground = format!("hsl({hue}, 65%, 62%)"); + let cells = cells_for(seed); + let mut squares: Vec = Vec::new(); + for (row, line) in cells.iter().enumerate() { + for (column, set) in line.iter().enumerate() { + if *set { + let x = column.to_string(); + let y = row.to_string(); + let fill = foreground.clone(); + squares.push(view! { + rect(x=x, y=y, width="1", height="1", fill=fill) + }); + } + } + } + view! { + svg(class="avatar", viewBox="0 0 5 5", "aria-hidden"="true") { + rect(x="0", y="0", width="5", height="5", fill=background) + (squares) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn hash_is_deterministic_and_seed_dependent() { + assert_eq!(fnv1a("alice"), fnv1a("alice")); + assert_ne!(fnv1a("alice"), fnv1a("bob")); + // Pinned vectors: guard against accidental changes to the hash + // (it must stay stable for the identicons users see). + assert_eq!(149_266_183, fnv1a("alice") % 1_000_000_000); + assert_eq!(23, hue_for("alice")); + } + + #[test] + fn hue_stays_in_range() { + for seed in ["alice", "bob", "", "auth0|64f0c0ffee"] { + assert!(hue_for(seed) < 360); + } + } + + #[test] + fn pattern_is_symmetric_and_never_blank() { + for seed in ["alice", "bob", "", "auth0|64f0c0ffee"] { + let cells = cells_for(seed); + assert!( + cells.iter().flatten().any(|cell| *cell), + "blank for {seed:?}" + ); + for row in 0..5 { + for column in 0..5 { + assert_eq!( + cells[row][column], + cells[row][4 - column], + "asymmetric for {seed:?} at ({row}, {column})" + ); + } + } + } + } + + #[test] + fn different_seeds_draw_different_patterns() { + assert_ne!(cells_for("alice"), cells_for("bob")); + } + + #[test] + fn only_absolute_http_picture_urls_are_rendered() { + assert_eq!( + Some("https://cdn.example.com/a.png".to_string()), + safe_picture_url("https://cdn.example.com/a.png") + ); + assert_eq!( + Some("https://cdn.example.com/a.png".to_string()), + safe_picture_url(" https://cdn.example.com/a.png ") + ); + assert_eq!( + Some("HTTPS://cdn.example.com/a.png".to_string()), + safe_picture_url("HTTPS://cdn.example.com/a.png") + ); + assert_eq!(None, safe_picture_url("httpz://x")); + assert_eq!(None, safe_picture_url("/relative.png")); + assert_eq!(None, safe_picture_url("javascript:alert(1)")); + assert_eq!( + None, + safe_picture_url("data:image/svg+xml;base64,PHN2ZyB4bW") + ); + } +} diff --git a/web/src/components/mod.rs b/web/src/components/mod.rs index 3f0f0af..757f6ce 100644 --- a/web/src/components/mod.rs +++ b/web/src/components/mod.rs @@ -1,3 +1,4 @@ +pub mod avatar; pub mod briscola; pub mod card; pub mod copy; diff --git a/web/src/model.rs b/web/src/model.rs index d2b80fe..96419f2 100644 --- a/web/src/model.rs +++ b/web/src/model.rs @@ -11,6 +11,8 @@ fn default_true() -> bool { pub struct User { pub sub: String, pub name: String, + #[serde(default)] + pub picture: Option, } #[derive(Debug, Clone, Deserialize)] diff --git a/web/src/pages/lobby.rs b/web/src/pages/lobby.rs index ae2fc31..1ffb9d1 100644 --- a/web/src/pages/lobby.rs +++ b/web/src/pages/lobby.rs @@ -12,6 +12,7 @@ use sycamore::prelude::*; use sycamore_router::navigate; use crate::api; +use crate::components::avatar::avatar; use crate::components::card::card_back; use crate::components::login::login_card; use crate::components::toast::toast; @@ -483,23 +484,30 @@ pub fn LobbyPage() -> View { Some(Some(_me)) => { view! { (crate::components::nav::app_nav("/", move || { - let name = user - .get_clone() - .flatten() - .map(|u| u.name) + let me = user.get_clone().flatten(); + let name = me + .as_ref() + .map(|u| u.name.clone()) .unwrap_or_default(); + let avatar_view = me + .as_ref() + .map(|u| avatar(&u.sub, u.picture.as_deref())) + .unwrap_or_else(|| view! {}); view! { - span(class="navbar-text me-3") { - "Signed in as " strong { (name) } - (move || ratings - .get_clone() - .into_iter() - .find(|r| r.game_type == selected_game.get_clone()) - .map(|r| view! { - span(class="badge text-bg-warning ms-2") { - "Elo " (r.rating) - } - })) + span(class="navbar-text me-3 d-flex align-items-center gap-2") { + (avatar_view) + span() { + "Signed in as " strong { (name) } + (move || ratings + .get_clone() + .into_iter() + .find(|r| r.game_type == selected_game.get_clone()) + .map(|r| view! { + span(class="badge text-bg-warning ms-2") { + "Elo " (r.rating) + } + })) + } } a( class="btn btn-sm btn-outline-light", diff --git a/web/style.css b/web/style.css index 6a85df5..ccc3300 100644 --- a/web/style.css +++ b/web/style.css @@ -33,6 +33,15 @@ body { box-shadow: 0 4px 16px rgba(0, 0, 0, 0.4); } +.avatar { + width: 2rem; + height: 2rem; + border-radius: 50%; + object-fit: cover; + display: block; + flex: 0 0 auto; +} + /* ---------- game table ---------- */ .hud {