fix(auth): self-healing displayName; drop synthetic Member<sub> from storage
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 <sub>') 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.
This commit is contained in:
+25
-13
@@ -35,22 +35,29 @@ const claimStr = (v: unknown): string | undefined =>
|
|||||||
* 1. name — full name set by the IdP (most human-friendly)
|
* 1. name — full name set by the IdP (most human-friendly)
|
||||||
* 2. preferred_username — often the login handle; still readable
|
* 2. preferred_username — often the login handle; still readable
|
||||||
* 3. email — readable but reveals contact info; acceptable fallback
|
* 3. email — readable but reveals contact info; acceptable fallback
|
||||||
* 4. `Member <sub>` — sub is always present; never blank
|
* 4. null — no usable claim; the UI degrades to a generic "Member"
|
||||||
|
*
|
||||||
|
* Returns null (NOT a synthetic `Member <sub>`) 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)
|
* 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
|
* so the write-path upsert agrees with /api/me.
|
||||||
* one. Whether Authelia emits name/preferred_username is an operator config
|
|
||||||
* concern (userinfo scope + claim mappings) — out of scope here.
|
|
||||||
*/
|
*/
|
||||||
export function deriveDisplayName(
|
export function deriveDisplayName(claims: {
|
||||||
claims: { name?: unknown; preferred_username?: unknown; email?: unknown },
|
name?: unknown
|
||||||
sub: string,
|
preferred_username?: unknown
|
||||||
): string {
|
email?: unknown
|
||||||
|
}): string | null {
|
||||||
return (
|
return (
|
||||||
claimStr(claims.name) ??
|
claimStr(claims.name) ??
|
||||||
claimStr(claims.preferred_username) ??
|
claimStr(claims.preferred_username) ??
|
||||||
claimStr(claims.email) ??
|
claimStr(claims.email) ??
|
||||||
`Member ${String(sub).slice(0, 8)}`
|
null
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -67,7 +74,7 @@ export function deriveDisplayName(
|
|||||||
export async function upsertUser(
|
export async function upsertUser(
|
||||||
oidcIss: string,
|
oidcIss: string,
|
||||||
oidcSub: string,
|
oidcSub: string,
|
||||||
displayName?: string,
|
displayName?: string | null,
|
||||||
) {
|
) {
|
||||||
// 1. Look up by composite identity key (iss + sub) — never email
|
// 1. Look up by composite identity key (iss + sub) — never email
|
||||||
const existing = await db
|
const existing = await db
|
||||||
@@ -77,9 +84,14 @@ export async function upsertUser(
|
|||||||
.limit(1)
|
.limit(1)
|
||||||
|
|
||||||
if (existing[0]) {
|
if (existing[0]) {
|
||||||
// If the existing row has no displayName but the caller supplies one, update it now.
|
// Track the IdP display name authoritatively: when the caller supplies a
|
||||||
// This corrects rows created before robust claim derivation was in place (BUG 2 fix).
|
// real (non-null) name that differs from what's stored, update it. This both
|
||||||
if (!existing[0].displayName && displayName) {
|
// 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
|
await db
|
||||||
.update(users)
|
.update(users)
|
||||||
.set({ displayName })
|
.set({ displayName })
|
||||||
|
|||||||
@@ -69,7 +69,7 @@ async function resolveUserId(c: any): Promise<number | null> {
|
|||||||
// Derive displayName via the shared helper (name → preferred_username → email
|
// Derive displayName via the shared helper (name → preferred_username → email
|
||||||
// → sub fallback) so the write-path upsert agrees with me.ts and never
|
// → sub fallback) so the write-path upsert agrees with me.ts and never
|
||||||
// overwrites a correctly-derived name with a worse one.
|
// 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)
|
const user = await upsertUser(iss, sub, displayName)
|
||||||
return user?.id ?? null
|
return user?.id ?? null
|
||||||
|
|||||||
@@ -56,7 +56,7 @@ meRouter.get('/', async (c) => {
|
|||||||
// Derive the best available display name from OIDC claims (name →
|
// Derive the best available display name from OIDC claims (name →
|
||||||
// preferred_username → email → sub fallback). Shared helper keeps every
|
// preferred_username → email → sub fallback). Shared helper keeps every
|
||||||
// upsert call site in agreement (see deriveDisplayName).
|
// upsert call site in agreement (see deriveDisplayName).
|
||||||
const displayName = deriveDisplayName(auth, sub)
|
const displayName = deriveDisplayName(auth)
|
||||||
|
|
||||||
const user = await upsertUser(iss, sub, displayName)
|
const user = await upsertUser(iss, sub, displayName)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user