feat(oidc): use the picture claim as avatar when none is uploaded

When a user signs in via OIDC and hasn't uploaded a custom avatar, their
`picture` claim is now used as their avatar. The users.avatar column holds
either an uploaded file name or an absolute https URL from the claim, and a
single resolver on each side (server avatarUrl, client avatarSrc) renders
both. An uploaded avatar always wins and is never overwritten; the picture
refreshes on each login otherwise. Only https URLs are stored, matching the
image CSP.

All the scattered /uploads/avatars/ builders now go through the resolvers,
which also fixes collection member avatars that were rendering a bare file
name.

Discussion #1399

server/src/nest/oidc/oidc.controller.ts

@@ -128,6 +128,12 @@ export class OidcController {
         return f('/login?oidc_error=subject_mismatch');
       }
 
+      // Some IdPs put `picture` only in the id_token, not the userinfo response —
+      // fall back to it so the avatar (#1399) still populates.
+      if (!userInfo.picture && typeof idVerify.claims.picture === 'string') {
+        userInfo.picture = idVerify.claims.picture;
+      }
+
       const result = this.oidc.findOrCreateUser(userInfo, config, pending.inviteToken);
       if ('error' in result) return f('/login?oidc_error=' + result.error);
 

server/src/services/adminService.ts

@@ -1,4 +1,5 @@
 import bcrypt from 'bcryptjs';
+import { avatarUrl } from './avatarUrl';
 import crypto from 'crypto';
 import path from 'path';
 import fs from 'fs';
@@ -71,7 +72,7 @@ export function listUsers() {
   } catch { /* */ }
   return users.map(u => ({
     ...u,
-    avatar_url: u.avatar ? `/uploads/avatars/${u.avatar}` : null,
+    avatar_url: avatarUrl(u),
     created_at: utcSuffix(u.created_at),
     updated_at: utcSuffix(u.updated_at as string),
     last_login: utcSuffix(u.last_login),

server/src/services/authService.ts

@@ -711,7 +711,9 @@ export function getSettings(userId: number): { error?: string; status?: number;
 
 export async function saveAvatar(userId: number, filename: string) {
   const current = db.prepare('SELECT avatar FROM users WHERE id = ?').get(userId) as { avatar: string | null } | undefined;
-  if (current && current.avatar) {
+  // Only a locally uploaded file has something to clean up. An OIDC picture URL
+  // (#1399) has no file on disk, so skip the rm — path.join on a URL is meaningless.
+  if (current?.avatar && !/^https:\/\//i.test(current.avatar)) {
     // Fire-and-forget: leftover files are harmless; the DB update is
     // the source of truth for which avatar is current.
     const oldPath = path.join(avatarDir, current.avatar);
@@ -726,7 +728,8 @@ export async function saveAvatar(userId: number, filename: string) {
 
 export async function deleteAvatar(userId: number) {
   const current = db.prepare('SELECT avatar FROM users WHERE id = ?').get(userId) as { avatar: string | null } | undefined;
-  if (current && current.avatar) {
+  // An OIDC picture URL (#1399) has no local file — only rm an uploaded one.
+  if (current?.avatar && !/^https:\/\//i.test(current.avatar)) {
     const filePath = path.join(avatarDir, current.avatar);
     await fs.promises.rm(filePath, { force: true }).catch(() => {});
   }

server/src/services/avatarUrl.ts

@@ -1,3 +1,12 @@
+/**
+ * Resolve a user's stored avatar reference to a renderable URL.
+ *
+ * The value is either an uploaded file name (served from /uploads/avatars) or,
+ * for SSO users who never uploaded one, an absolute https URL taken from their
+ * OIDC `picture` claim (#1399) — that one is passed through untouched.
+ */
 export function avatarUrl(user: { avatar?: string | null }): string | null {
-  return user.avatar ? `/uploads/avatars/${user.avatar}` : null;
+  if (!user.avatar) return null;
+  if (/^https:\/\//i.test(user.avatar)) return user.avatar;
+  return `/uploads/avatars/${user.avatar}`;
 }

server/src/services/fileService.ts

@@ -1,4 +1,5 @@
 import path from 'path';
+import { avatarUrl } from './avatarUrl';
 import fs from 'fs';
 import type { Request } from 'express';
 import { db } from '../db/database';
@@ -66,7 +67,7 @@ export function formatFile(file: TripFile & { trip_id?: number; uploaded_by_avat
   return {
     ...file,
     url: `/api/trips/${tripId}/files/${file.id}/download`,
-    uploaded_by_avatar: file.uploaded_by_avatar ? `/uploads/avatars/${file.uploaded_by_avatar}` : null,
+    uploaded_by_avatar: avatarUrl({ avatar: file.uploaded_by_avatar }),
   };
 }
 

server/src/services/inAppNotifications.ts

@@ -1,4 +1,5 @@
 import { db } from '../db/database';
+import { avatarUrl } from './avatarUrl';
 import { broadcastToUser } from '../websocket';
 import { getAction } from './inAppNotificationActions';
 import { isEnabledForEvent, type NotifEventType } from './notificationPreferencesService';
@@ -171,7 +172,7 @@ function createNotification(input: NotificationInput): number[] {
       notification: {
         ...row,
         sender_username: sender?.username ?? null,
-        sender_avatar: sender?.avatar ? `/uploads/avatars/${sender.avatar}` : null,
+        sender_avatar: avatarUrl({ avatar: sender?.avatar }),
       },
     });
   }
@@ -232,7 +233,7 @@ export function createNotificationForRecipient(
       ...row,
       created_at: toUtcIso(row.created_at),
       sender_username: sender?.username ?? null,
-      sender_avatar: sender?.avatar ? `/uploads/avatars/${sender.avatar}` : null,
+      sender_avatar: avatarUrl({ avatar: sender?.avatar }),
     },
   });
 
@@ -265,7 +266,7 @@ function getNotifications(
   const mapped = rows.map(r => ({
     ...r,
     created_at: toUtcIso(r.created_at),
-    sender_avatar: r.sender_avatar ? `/uploads/avatars/${r.sender_avatar}` : null,
+    sender_avatar: avatarUrl({ avatar: r.sender_avatar }),
   }));
 
   return { notifications: mapped, total, unread_count };
@@ -347,7 +348,7 @@ async function respondToBoolean(
 
   const mappedUpdated = {
     ...updated,
-    sender_avatar: updated.sender_avatar ? `/uploads/avatars/${updated.sender_avatar}` : null,
+    sender_avatar: avatarUrl({ avatar: updated.sender_avatar }),
   };
 
   broadcastToUser(userId, { type: 'notification:updated', notification: mappedUpdated });

server/src/services/journeyService.ts

@@ -1,4 +1,5 @@
 import { db, canAccessTrip } from '../db/database';
+import { avatarUrl } from './avatarUrl';
 import type { Journey, JourneyEntry, JourneyPhoto, JourneyContributor } from '../types';
 import { broadcastToUser } from '../websocket';
 import {
@@ -214,7 +215,7 @@ export function getJourneyFull(journeyId: number, userId: number) {
     .all(journeyId) as any[];
   const contributors = contributorsRaw.map((c) => ({
     ...c,
-    avatar_url: c.avatar ? `/uploads/avatars/${c.avatar}` : null,
+    avatar_url: avatarUrl(c),
   }));
 
   // stats

server/src/services/oidcService.ts

@@ -32,6 +32,8 @@ export interface OidcUserInfo {
   email_verified?: boolean | string;
   name?: string;
   preferred_username?: string;
+  // Standard OIDC profile claim: URL of the user's profile picture.
+  picture?: string;
   groups?: string[];
   roles?: string[];
   [key: string]: unknown;
@@ -355,6 +357,17 @@ export async function verifyIdToken(
 // Find or create user by OIDC sub / email
 // ---------------------------------------------------------------------------
 
+// Sanitize the OIDC `picture` claim before we store it as the avatar. Only https
+// URLs are usable: the app's CSP allows https image sources but not http, and we
+// render the value directly. Non-strings, non-https and oversized values (e.g. a
+// large data: URI) are ignored so a user payload never carries junk. #1399
+function safeOidcPicture(picture: unknown): string | null {
+  if (typeof picture !== 'string') return null;
+  const url = picture.trim();
+  if (!url || url.length > 1024) return null;
+  return /^https:\/\//i.test(url) ? url : null;
+}
+
 export function findOrCreateUser(
   userInfo: OidcUserInfo,
   config: OidcConfig,
@@ -363,6 +376,7 @@ export function findOrCreateUser(
   const email = userInfo.email!.trim().toLowerCase();
   const name = userInfo.name || userInfo.preferred_username || email.split('@')[0];
   const sub = userInfo.sub;
+  const picture = safeOidcPicture(userInfo.picture);
 
   // Try to find existing user by sub, then by email
   let user = db.prepare('SELECT * FROM users WHERE oidc_sub = ? AND oidc_issuer = ?').get(sub, config.issuer) as User | undefined;
@@ -402,6 +416,14 @@ export function findOrCreateUser(
         }
       }
     }
+    // Keep the avatar in sync with the OIDC picture, but never clobber a custom
+    // upload: only fill it when empty or when the current value is itself an OIDC
+    // picture URL, so the picture refreshes on each login without overriding an
+    // uploaded one. #1399
+    if (picture && picture !== user.avatar && (!user.avatar || /^https:\/\//i.test(user.avatar))) {
+      db.prepare('UPDATE users SET avatar = ? WHERE id = ?').run(picture, user.id);
+      user = { ...user, avatar: picture } as User;
+    }
     return { user };
   }
 
@@ -452,11 +474,11 @@ export function findOrCreateUser(
         if (updated.changes === 0) throw inviteRaceError;
       }
       return db.prepare(
-        'INSERT INTO users (username, email, password_hash, role, oidc_sub, oidc_issuer, first_seen_version, login_count) VALUES (?, ?, ?, ?, ?, ?, ?, 0)',
-      ).run(username, email, hash, role, sub, config.issuer, process.env.APP_VERSION || '0.0.0');
+        'INSERT INTO users (username, email, password_hash, role, oidc_sub, oidc_issuer, avatar, first_seen_version, login_count) VALUES (?, ?, ?, ?, ?, ?, ?, ?, 0)',
+      ).run(username, email, hash, role, sub, config.issuer, picture, process.env.APP_VERSION || '0.0.0');
     });
     const result = createUser() as { lastInsertRowid: number | bigint };
-    user = { id: Number(result.lastInsertRowid), username, email, role } as User;
+    user = { id: Number(result.lastInsertRowid), username, email, role, avatar: picture } as User;
     return { user };
   } catch (err) {
     if (err === inviteRaceError) {

server/src/services/tripService.ts

@@ -1,4 +1,5 @@
 import path from 'path';
+import { avatarUrl } from './avatarUrl';
 import fs from 'fs';
 import { randomUUID } from 'crypto';
 import { db, isOwner } from '../db/database';
@@ -365,8 +366,8 @@ export function listMembers(tripId: string | number, tripOwnerId: number) {
   const owner = db.prepare('SELECT id, username, email, avatar FROM users WHERE id = ?').get(tripOwnerId) as Pick<User, 'id' | 'username' | 'email' | 'avatar'>;
 
   return {
-    owner: { ...owner, role: 'owner', is_guest: false, avatar_url: owner.avatar ? `/uploads/avatars/${owner.avatar}` : null },
-    members: members.map(m => ({ ...m, is_guest: !!m.is_guest, avatar_url: m.avatar ? `/uploads/avatars/${m.avatar}` : null })),
+    owner: { ...owner, role: 'owner', is_guest: false, avatar_url: avatarUrl(owner) },
+    members: members.map(m => ({ ...m, is_guest: !!m.is_guest, avatar_url: avatarUrl(m) })),
   };
 }
 
@@ -398,7 +399,7 @@ export function addMember(tripId: string | number, identifier: string, tripOwner
   const tripInfo = db.prepare('SELECT title FROM trips WHERE id = ?').get(tripId) as { title: string } | undefined;
 
   return {
-    member: { ...target, role: 'member', avatar_url: target.avatar ? `/uploads/avatars/${target.avatar}` : null },
+    member: { ...target, role: 'member', avatar_url: avatarUrl(target) },
     targetUserId: target.id,
     tripTitle: tripInfo?.title || 'Untitled',
   };