From 2c8f1a28af0b83b37025af54207c0bba3e5929b4 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Sun, 7 Jun 2026 16:17:17 -0400 Subject: [PATCH] fix(auth): self-healing displayName; drop synthetic Member from storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legend showed 'Member 972be1a3' because Authelia does not emit name/preferred_username/email in the ID TOKEN (only at the userinfo endpoint), and @hono/oidc-auth reads ID-token claims only. The real fix is an Authelia claims_policy adding those claims to id_token for the familysync client. App-side robustness so it self-heals once Authelia is fixed (no DB surgery): - deriveDisplayName now returns null (not a synthetic 'Member ') when no real claim is present, so we never persist an ugly sub string; the UI degrades to a generic 'Member'. - upsertUser now tracks the IdP name authoritatively: a non-null displayName that differs from the stored value updates the row (blank/stale 'Member …'/email → real name on next login). A null value never overwrites a good stored name. --- apps/api/src/auth/user.ts | 38 +++++++++++++++++++++++------------ apps/api/src/routes/events.ts | 2 +- apps/api/src/routes/me.ts | 2 +- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/apps/api/src/auth/user.ts b/apps/api/src/auth/user.ts index 1814a62..b2e0c79 100644 --- a/apps/api/src/auth/user.ts +++ b/apps/api/src/auth/user.ts @@ -35,22 +35,29 @@ const claimStr = (v: unknown): string | undefined => * 1. name — full name set by the IdP (most human-friendly) * 2. preferred_username — often the login handle; still readable * 3. email — readable but reveals contact info; acceptable fallback - * 4. `Member ` — sub is always present; never blank + * 4. null — no usable claim; the UI degrades to a generic "Member" + * + * Returns null (NOT a synthetic `Member `) when no real claim is present so + * we never persist an ugly sub-derived string. Whether Authelia emits + * name/preferred_username/email in the ID TOKEN (not just at the userinfo + * endpoint) is an operator config concern: Authelia 4.39+ requires a + * `claims_policies` entry adding those claims to `id_token` for this client, + * because @hono/oidc-auth reads ID-token claims only (no userinfo fetch). Until + * that is set, every claim here is absent and the legend shows "Member". * * Shared by every call site that upserts a user (me.ts, events.ts resolveUserId) - * so a write-path upsert never overwrites a correctly-derived name with a worse - * one. Whether Authelia emits name/preferred_username is an operator config - * concern (userinfo scope + claim mappings) — out of scope here. + * so the write-path upsert agrees with /api/me. */ -export function deriveDisplayName( - claims: { name?: unknown; preferred_username?: unknown; email?: unknown }, - sub: string, -): string { +export function deriveDisplayName(claims: { + name?: unknown + preferred_username?: unknown + email?: unknown +}): string | null { return ( claimStr(claims.name) ?? claimStr(claims.preferred_username) ?? claimStr(claims.email) ?? - `Member ${String(sub).slice(0, 8)}` + null ) } @@ -67,7 +74,7 @@ export function deriveDisplayName( export async function upsertUser( oidcIss: string, oidcSub: string, - displayName?: string, + displayName?: string | null, ) { // 1. Look up by composite identity key (iss + sub) — never email const existing = await db @@ -77,9 +84,14 @@ export async function upsertUser( .limit(1) if (existing[0]) { - // If the existing row has no displayName but the caller supplies one, update it now. - // This corrects rows created before robust claim derivation was in place (BUG 2 fix). - if (!existing[0].displayName && displayName) { + // Track the IdP display name authoritatively: when the caller supplies a + // real (non-null) name that differs from what's stored, update it. This both + // corrects rows created before robust claim derivation (blank → name) and + // self-heals once an operator adds the name/email claims to Authelia's ID + // token (e.g. a stale "Member …"/email → the real name) — no DB surgery. + // A null displayName (no usable claim this request) never overwrites a good + // stored value. + if (displayName != null && displayName !== existing[0].displayName) { await db .update(users) .set({ displayName }) diff --git a/apps/api/src/routes/events.ts b/apps/api/src/routes/events.ts index 5581da0..bc8ae19 100644 --- a/apps/api/src/routes/events.ts +++ b/apps/api/src/routes/events.ts @@ -69,7 +69,7 @@ async function resolveUserId(c: any): Promise { // Derive displayName via the shared helper (name → preferred_username → email // → sub fallback) so the write-path upsert agrees with me.ts and never // overwrites a correctly-derived name with a worse one. - const displayName = deriveDisplayName(auth, sub) + const displayName = deriveDisplayName(auth) const user = await upsertUser(iss, sub, displayName) return user?.id ?? null diff --git a/apps/api/src/routes/me.ts b/apps/api/src/routes/me.ts index 8b6e60e..38ace0c 100644 --- a/apps/api/src/routes/me.ts +++ b/apps/api/src/routes/me.ts @@ -56,7 +56,7 @@ meRouter.get('/', async (c) => { // Derive the best available display name from OIDC claims (name → // preferred_username → email → sub fallback). Shared helper keeps every // upsert call site in agreement (see deriveDisplayName). - const displayName = deriveDisplayName(auth, sub) + const displayName = deriveDisplayName(auth) const user = await upsertUser(iss, sub, displayName)