From 5e905f8db2f5965f52ec363cd02e50284107b759 Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Mon, 4 May 2026 17:30:04 +0000 Subject: [PATCH] =?UTF-8?q?fix(auth):=20#29=20OAuth=20display-name=20casca?= =?UTF-8?q?de=20=E2=80=94=20add=20user=5Fname=20+=20preferred=5Fusername?= =?UTF-8?q?=20tiers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub OAuth puts the user's @handle in user_metadata.user_name, not .name. The previous cascade (full_name > name > email_prefix) skipped user_name entirely, so GitHub users with no display name set on GitHub fell through to email prefix even though the @handle was a meaningful identifier already provided by the OAuth flow. OIDC providers using preferred_username had the same issue. Cascade is now: full_name > name > user_name > preferred_username > email_prefix > "Anonymous User" Fixed in two places: 1. src/lib/auth/oauth-utils.ts extractOAuthDisplayName() — runtime path, called by populateOAuthProfile() during the OAuth callback. Each tier now trims whitespace; whitespace-only metadata fields fall through instead of producing a whitespace-only display name. Non-string values (e.g. accidental nulls in metadata) are skipped without throwing. 2. supabase/migrations/20251006_complete_monolithic_setup.sql PART 9 one-time UPDATE — mirrors the JS cascade so the SQL bootstrap path produces the same result. NULLIF(TRIM(...), '') handles whitespace- only fields the same way. Note on runtime behavior: create_user_profile() (the on_auth_user_created trigger at line 357) does NOT set display_name — it only inserts (id, created_at, updated_at). At signup display_name starts NULL and populateOAuthProfile() is the sole authoritative populator. The PART 9 UPDATE handles the one-time bootstrap for users who existed before that runtime path landed; idempotent (only fires when display_name IS NULL). Tests: 10 new cases in oauth-utils.test.ts cover the GitHub @handle fallthrough, OIDC preferred_username, whitespace handling, non-string metadata, and realistic Google/GitHub fixture shapes. Verification: - pnpm run type-check: clean - pnpm run lint: clean - pnpm test: 3247/3247 pass (was 3237 before — 10 new tests, all passing) - UPDATE applied to dev Supabase via Management API: 0 rows affected (no existing OAuth users on dev), confirming the WHERE clause is idempotent and selective. Closes #29 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/auth/oauth-utils.test.ts | 107 ++++++++++++++++++ src/lib/auth/oauth-utils.ts | 33 ++++-- .../20251006_complete_monolithic_setup.sql | 32 +++++- 3 files changed, 158 insertions(+), 14 deletions(-) diff --git a/src/lib/auth/oauth-utils.test.ts b/src/lib/auth/oauth-utils.test.ts index f2cae01f..834dfda5 100644 --- a/src/lib/auth/oauth-utils.test.ts +++ b/src/lib/auth/oauth-utils.test.ts @@ -105,6 +105,113 @@ describe('extractOAuthDisplayName', () => { }); expect(extractOAuthDisplayName(user)).toBe('José García 🚀'); }); + + // Issue #29: GitHub puts the user's @handle in user_name, not name. Without + // this tier in the cascade, GitHub users with no display name set would + // fall through to email prefix even though the @handle is a meaningful + // identifier provided by the OAuth flow. + describe('issue #29 — extended cascade for GitHub / OIDC providers', () => { + it('returns user_name when full_name and name are absent (GitHub @handle)', () => { + const user = createMockUser({ + user_metadata: { user_name: 'octocat' }, + }); + expect(extractOAuthDisplayName(user)).toBe('octocat'); + }); + + it('prefers name over user_name (GitHub user with display name set)', () => { + const user = createMockUser({ + user_metadata: { name: 'The Octocat', user_name: 'octocat' }, + }); + expect(extractOAuthDisplayName(user)).toBe('The Octocat'); + }); + + it('returns preferred_username when nothing higher is set (OIDC)', () => { + const user = createMockUser({ + user_metadata: { preferred_username: 'jsmith' }, + }); + expect(extractOAuthDisplayName(user)).toBe('jsmith'); + }); + + it('prefers user_name over preferred_username when both present', () => { + const user = createMockUser({ + user_metadata: { + user_name: 'octocat', + preferred_username: 'fallback', + }, + }); + expect(extractOAuthDisplayName(user)).toBe('octocat'); + }); + + it('skips whitespace-only metadata fields and falls through', () => { + const user = createMockUser({ + email: 'jsmith@example.com', + user_metadata: { + full_name: ' ', + name: '', + user_name: ' ', + }, + }); + expect(extractOAuthDisplayName(user)).toBe('jsmith'); + }); + + it('trims surrounding whitespace from a populated tier', () => { + const user = createMockUser({ + user_metadata: { full_name: ' Jon Pohlner ' }, + }); + expect(extractOAuthDisplayName(user)).toBe('Jon Pohlner'); + }); + + // Realistic Google fixture: full_name + name + avatar_url, no user_name + it('handles Google OAuth metadata shape', () => { + const user = createMockUser({ + email: 'jpohlner@gmail.com', + user_metadata: { + full_name: 'Jon Pohlner', + name: 'Jon Pohlner', + avatar_url: 'https://lh3.googleusercontent.com/a/abc', + email_verified: true, + }, + }); + expect(extractOAuthDisplayName(user)).toBe('Jon Pohlner'); + }); + + // Realistic GitHub fixture: name set to display name, user_name to handle + it('handles GitHub OAuth metadata shape with display name', () => { + const user = createMockUser({ + email: 'octocat@users.noreply.github.com', + user_metadata: { + name: 'The Octocat', + user_name: 'octocat', + avatar_url: 'https://avatars.githubusercontent.com/u/583231', + }, + }); + expect(extractOAuthDisplayName(user)).toBe('The Octocat'); + }); + + // Realistic GitHub fixture: handle only (user has no display name set on GitHub) + it('handles GitHub OAuth metadata shape with handle only', () => { + const user = createMockUser({ + email: 'octocat@users.noreply.github.com', + user_metadata: { + user_name: 'octocat', + avatar_url: 'https://avatars.githubusercontent.com/u/583231', + }, + }); + expect(extractOAuthDisplayName(user)).toBe('octocat'); + }); + + it('ignores non-string metadata values without throwing', () => { + const user = createMockUser({ + email: 'jsmith@example.com', + user_metadata: { + full_name: 42 as unknown as string, + name: null as unknown as string, + user_name: undefined as unknown as string, + }, + }); + expect(extractOAuthDisplayName(user)).toBe('jsmith'); + }); + }); }); describe('extractOAuthAvatarUrl', () => { diff --git a/src/lib/auth/oauth-utils.ts b/src/lib/auth/oauth-utils.ts index 5f5b524a..502f2e09 100644 --- a/src/lib/auth/oauth-utils.ts +++ b/src/lib/auth/oauth-utils.ts @@ -8,8 +8,18 @@ import { createClient } from '@/lib/supabase/client'; import { createLogger } from '@/lib/logger'; /** - * Extract display name from OAuth user metadata using fallback cascade - * Priority: full_name > name > email prefix > "Anonymous User" + * Extract display name from OAuth user metadata using fallback cascade. + * Priority: full_name > name > user_name > preferred_username > email prefix > "Anonymous User" + * + * Provider-specific notes: + * - Google sets `full_name` and `name` + * - GitHub sets `name` (the user's display name) and `user_name` (the GitHub + * handle). The handle is preferred over email prefix because users with + * no display name set on GitHub still have a meaningful identifier. + * - Other providers may use `preferred_username` (OIDC standard claim). + * + * Trims whitespace at each tier so a metadata field of " " falls through + * to the next tier instead of producing a whitespace-only display name. * * @param user - Supabase User object * @returns Display name string, never null @@ -17,12 +27,19 @@ import { createLogger } from '@/lib/logger'; export function extractOAuthDisplayName(user: User | null): string { if (!user) return 'Anonymous User'; - // Fallback cascade per FR-005 - const fullName = user.user_metadata?.full_name; - if (fullName) return fullName; - - const name = user.user_metadata?.name; - if (name) return name; + const meta = user.user_metadata ?? {}; + const tiers = [ + meta.full_name, + meta.name, + meta.user_name, + meta.preferred_username, + ]; + for (const tier of tiers) { + if (typeof tier === 'string') { + const trimmed = tier.trim(); + if (trimmed.length > 0) return trimmed; + } + } // Email prefix fallback const email = user.email; diff --git a/supabase/migrations/20251006_complete_monolithic_setup.sql b/supabase/migrations/20251006_complete_monolithic_setup.sql index eae1cd8e..f9029961 100644 --- a/supabase/migrations/20251006_complete_monolithic_setup.sql +++ b/supabase/migrations/20251006_complete_monolithic_setup.sql @@ -2290,16 +2290,36 @@ END $seed_admin_profile$; -- ============================================================================ --- Feature 004: Populate OAuth user profiles (one-time migration) --- Only updates NULL display_name for OAuth users --- Idempotent: Safe to run multiple times (FR-006) +-- Feature 004 / Issue #29: Populate OAuth user profiles (one-time backfill) +-- Only updates NULL display_name for OAuth users. +-- Idempotent: Safe to run multiple times (FR-006). +-- +-- Cascade mirrors src/lib/auth/oauth-utils.ts extractOAuthDisplayName(): +-- full_name > name > user_name > preferred_username > email prefix > 'Anonymous User' +-- +-- Provider notes: +-- - Google sets full_name AND name +-- - GitHub sets name (display name) AND user_name (the @handle) — without +-- user_name in the cascade, GitHub users with no GitHub display name +-- would fall through to email prefix even though the @handle is a +-- better identifier +-- - Other OIDC providers may set preferred_username +-- +-- Note on runtime behavior: create_user_profile() (the on_auth_user_created +-- trigger) does NOT set display_name — it inserts only (id, created_at, +-- updated_at). So at signup display_name is NULL, and the runtime +-- populateOAuthProfile() in src/lib/auth/oauth-utils.ts is the sole +-- authoritative populator going forward. This UPDATE handles only the +-- one-time bootstrap for users who existed before that runtime path landed. -- ============================================================================ UPDATE public.user_profiles p SET display_name = COALESCE( - u.raw_user_meta_data->>'full_name', - u.raw_user_meta_data->>'name', - split_part(u.email, '@', 1), + NULLIF(TRIM(u.raw_user_meta_data->>'full_name'), ''), + NULLIF(TRIM(u.raw_user_meta_data->>'name'), ''), + NULLIF(TRIM(u.raw_user_meta_data->>'user_name'), ''), + NULLIF(TRIM(u.raw_user_meta_data->>'preferred_username'), ''), + NULLIF(split_part(u.email, '@', 1), ''), 'Anonymous User' ), avatar_url = COALESCE(p.avatar_url, u.raw_user_meta_data->>'avatar_url')