diff --git a/client/src/utils/avatarSrc.test.ts b/client/src/utils/avatarSrc.test.ts
new file mode 100644
index 000000000..c041272ed
--- /dev/null
+++ b/client/src/utils/avatarSrc.test.ts
@@ -0,0 +1,18 @@
+import { describe, it, expect } from 'vitest'
+import { avatarSrc } from './avatarSrc'
+
+describe('avatarSrc', () => {
+ it('prefixes an uploaded file name with the avatars path', () => {
+ expect(avatarSrc('abc123.jpg')).toBe('/uploads/avatars/abc123.jpg')
+ })
+
+ it('passes an absolute https URL (OIDC picture) through untouched', () => {
+ expect(avatarSrc('https://idp.example.com/u/pic.png')).toBe('https://idp.example.com/u/pic.png')
+ })
+
+ it('returns null for empty input', () => {
+ expect(avatarSrc(null)).toBeNull()
+ expect(avatarSrc(undefined)).toBeNull()
+ expect(avatarSrc('')).toBeNull()
+ })
+})
diff --git a/client/src/utils/avatarSrc.ts b/client/src/utils/avatarSrc.ts
new file mode 100644
index 000000000..b5693c4f9
--- /dev/null
+++ b/client/src/utils/avatarSrc.ts
@@ -0,0 +1,12 @@
+/**
+ * Resolve a user's raw `avatar` field to an
![]()
.
+ *
+ * The value is either an uploaded file name (served from /uploads/avatars) or,
+ * for SSO users who never uploaded one, an absolute https URL from their OIDC
+ * `picture` claim (#1399), which is used as-is. Server responses usually expose a
+ * pre-resolved `avatar_url`; use this only where components read the raw field.
+ */
+export function avatarSrc(avatar?: string | null): string | null {
+ if (!avatar) return null
+ return /^https:\/\//i.test(avatar) ? avatar : `/uploads/avatars/${avatar}`
+}
diff --git a/server/src/nest/oidc/oidc.controller.ts b/server/src/nest/oidc/oidc.controller.ts
index 8d783603f..2dc6be5ad 100644
--- a/server/src/nest/oidc/oidc.controller.ts
+++ b/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);
diff --git a/server/src/services/adminService.ts b/server/src/services/adminService.ts
index c6ef3a6af..ff3a4e611 100644
--- a/server/src/services/adminService.ts
+++ b/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),
diff --git a/server/src/services/authService.ts b/server/src/services/authService.ts
index 3e368c224..ec212e731 100644
--- a/server/src/services/authService.ts
+++ b/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(() => {});
}
diff --git a/server/src/services/avatarUrl.ts b/server/src/services/avatarUrl.ts
index d3919f5f0..069effdc1 100644
--- a/server/src/services/avatarUrl.ts
+++ b/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}`;
}
diff --git a/server/src/services/fileService.ts b/server/src/services/fileService.ts
index 072fecc82..e4fdc0068 100644
--- a/server/src/services/fileService.ts
+++ b/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 }),
};
}
diff --git a/server/src/services/inAppNotifications.ts b/server/src/services/inAppNotifications.ts
index ea972b4d9..fbccd6569 100644
--- a/server/src/services/inAppNotifications.ts
+++ b/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 });
diff --git a/server/src/services/journeyService.ts b/server/src/services/journeyService.ts
index 22dddcc50..ff1b54646 100644
--- a/server/src/services/journeyService.ts
+++ b/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
diff --git a/server/src/services/oidcService.ts b/server/src/services/oidcService.ts
index 546ce91c6..aa273dd1b 100644
--- a/server/src/services/oidcService.ts
+++ b/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) {
diff --git a/server/src/services/tripService.ts b/server/src/services/tripService.ts
index 1b0b0c3b1..8df373a69 100644
--- a/server/src/services/tripService.ts
+++ b/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
;
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',
};
diff --git a/server/tests/unit/services/avatarUrl.test.ts b/server/tests/unit/services/avatarUrl.test.ts
new file mode 100644
index 000000000..5a4e1c21f
--- /dev/null
+++ b/server/tests/unit/services/avatarUrl.test.ts
@@ -0,0 +1,25 @@
+/**
+ * Unit tests for avatarUrl — AVATAR-URL-001 through AVATAR-URL-003.
+ * The stored value is either an uploaded file name or an absolute https URL from
+ * an OIDC `picture` claim (#1399); the helper resolves both to a renderable src.
+ */
+import { describe, it, expect } from 'vitest';
+import { avatarUrl } from '../../../src/services/avatarUrl';
+
+describe('avatarUrl', () => {
+ it('AVATAR-URL-001: prefixes an uploaded file name with the avatars path', () => {
+ expect(avatarUrl({ avatar: 'abc123.jpg' })).toBe('/uploads/avatars/abc123.jpg');
+ });
+
+ it('AVATAR-URL-002: passes an absolute https URL (OIDC picture) through untouched', () => {
+ expect(avatarUrl({ avatar: 'https://idp.example.com/u/pic.png' })).toBe(
+ 'https://idp.example.com/u/pic.png',
+ );
+ });
+
+ it('AVATAR-URL-003: returns null when no avatar is set', () => {
+ expect(avatarUrl({ avatar: null })).toBeNull();
+ expect(avatarUrl({ avatar: undefined })).toBeNull();
+ expect(avatarUrl({})).toBeNull();
+ });
+});
diff --git a/server/tests/unit/services/oidcService.test.ts b/server/tests/unit/services/oidcService.test.ts
index 6a86b2cc1..0b22c5158 100644
--- a/server/tests/unit/services/oidcService.test.ts
+++ b/server/tests/unit/services/oidcService.test.ts
@@ -464,6 +464,63 @@ describe('findOrCreateUser', () => {
const token = testDb.prepare("SELECT used_count FROM invite_tokens WHERE token = 'tok-full'").get() as any;
expect(token.used_count).toBe(1);
});
+
+ // ── OIDC picture claim → avatar (#1399) ──────────────────────────────────
+
+ it('OIDC-SVC-040: new user stores the https picture claim as their avatar', () => {
+ const result = findOrCreateUser(
+ { sub: 'sub-pic-1', email: 'pic1@example.com', name: 'Pic One', picture: 'https://idp.example.com/u/pic1.png' },
+ MOCK_CONFIG
+ );
+ expect('user' in result).toBe(true);
+ const row = testDb.prepare("SELECT avatar FROM users WHERE email = 'pic1@example.com'").get() as any;
+ expect(row.avatar).toBe('https://idp.example.com/u/pic1.png');
+ });
+
+ it('OIDC-SVC-041: new user with a non-https picture claim stores no avatar', () => {
+ findOrCreateUser(
+ { sub: 'sub-pic-2', email: 'pic2@example.com', name: 'Pic Two', picture: 'http://idp.example.com/u/pic2.png' },
+ MOCK_CONFIG
+ );
+ const row = testDb.prepare("SELECT avatar FROM users WHERE email = 'pic2@example.com'").get() as any;
+ expect(row.avatar).toBeNull();
+ });
+
+ it('OIDC-SVC-042: existing user with no avatar gets the OIDC picture', () => {
+ const { user } = createUser(testDb, { email: 'pic3@example.com' });
+ testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ?, avatar = NULL WHERE id = ?')
+ .run('sub-pic-3', MOCK_CONFIG.issuer, user.id);
+ findOrCreateUser(
+ { sub: 'sub-pic-3', email: 'pic3@example.com', name: 'Pic Three', picture: 'https://idp.example.com/u/pic3.png' },
+ MOCK_CONFIG
+ );
+ const row = testDb.prepare('SELECT avatar FROM users WHERE id = ?').get(user.id) as any;
+ expect(row.avatar).toBe('https://idp.example.com/u/pic3.png');
+ });
+
+ it('OIDC-SVC-043: a custom uploaded avatar is never overwritten by the OIDC picture', () => {
+ const { user } = createUser(testDb, { email: 'pic4@example.com' });
+ testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ?, avatar = ? WHERE id = ?')
+ .run('sub-pic-4', MOCK_CONFIG.issuer, 'uploaded-abc.jpg', user.id);
+ findOrCreateUser(
+ { sub: 'sub-pic-4', email: 'pic4@example.com', name: 'Pic Four', picture: 'https://idp.example.com/u/pic4.png' },
+ MOCK_CONFIG
+ );
+ const row = testDb.prepare('SELECT avatar FROM users WHERE id = ?').get(user.id) as any;
+ expect(row.avatar).toBe('uploaded-abc.jpg');
+ });
+
+ it('OIDC-SVC-044: a previously stored OIDC picture URL is refreshed on next login', () => {
+ const { user } = createUser(testDb, { email: 'pic5@example.com' });
+ testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ?, avatar = ? WHERE id = ?')
+ .run('sub-pic-5', MOCK_CONFIG.issuer, 'https://idp.example.com/u/old.png', user.id);
+ findOrCreateUser(
+ { sub: 'sub-pic-5', email: 'pic5@example.com', name: 'Pic Five', picture: 'https://idp.example.com/u/new.png' },
+ MOCK_CONFIG
+ );
+ const row = testDb.prepare('SELECT avatar FROM users WHERE id = ?').get(user.id) as any;
+ expect(row.avatar).toBe('https://idp.example.com/u/new.png');
+ });
});
// ── exchangeCodeForToken ──────────────────────────────────────────────────────