diff --git a/foundations/core/packages/core/src/__tests__/externalAvatarUrl.test.ts b/foundations/core/packages/core/src/__tests__/externalAvatarUrl.test.ts new file mode 100644 index 00000000000..8020c058bdf --- /dev/null +++ b/foundations/core/packages/core/src/__tests__/externalAvatarUrl.test.ts @@ -0,0 +1,44 @@ +// +// Copyright © 2026 Hardcore Engineering Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import { normalizeExternalAvatarUrl } from '../utils' + +describe('normalizeExternalAvatarUrl', () => { + it('accepts absolute http(s) URLs', () => { + expect(normalizeExternalAvatarUrl('https://idp.example.com/avatar.png')).toBe('https://idp.example.com/avatar.png') + expect(normalizeExternalAvatarUrl(' http://idp.example.com/a?s=96 ')).toBe('http://idp.example.com/a?s=96') + }) + + it('rejects non-string and empty values', () => { + expect(normalizeExternalAvatarUrl(undefined)).toBeUndefined() + expect(normalizeExternalAvatarUrl(null)).toBeUndefined() + expect(normalizeExternalAvatarUrl(42)).toBeUndefined() + expect(normalizeExternalAvatarUrl({ url: 'https://idp.example.com/a.png' })).toBeUndefined() + expect(normalizeExternalAvatarUrl(' ')).toBeUndefined() + }) + + it('rejects relative URLs and other protocols', () => { + expect(normalizeExternalAvatarUrl('/avatar.png')).toBeUndefined() + expect(normalizeExternalAvatarUrl('javascript:alert(1)')).toBeUndefined() + expect(normalizeExternalAvatarUrl('data:image/png;base64,AAAA')).toBeUndefined() + expect(normalizeExternalAvatarUrl('file:///etc/passwd')).toBeUndefined() + expect(normalizeExternalAvatarUrl('ftp://idp.example.com/a.png')).toBeUndefined() + }) + + it('rejects URLs with credentials or excessive length', () => { + expect(normalizeExternalAvatarUrl('https://user:pass@idp.example.com/a.png')).toBeUndefined() + expect(normalizeExternalAvatarUrl('https://idp.example.com/' + 'a'.repeat(3000))).toBeUndefined() + }) +}) diff --git a/foundations/core/packages/core/src/classes.ts b/foundations/core/packages/core/src/classes.ts index 84a9555caa2..fc3cf47774d 100644 --- a/foundations/core/packages/core/src/classes.ts +++ b/foundations/core/packages/core/src/classes.ts @@ -639,6 +639,8 @@ export interface Person { uuid: PersonUuid firstName: string lastName: string + /** External avatar URL provided by an identity provider (OIDC/OAuth), if any. */ + avatarUrl?: string } export interface PersonInfo extends BasePerson { diff --git a/foundations/core/packages/core/src/utils.ts b/foundations/core/packages/core/src/utils.ts index 568c34a415e..bed800ff035 100644 --- a/foundations/core/packages/core/src/utils.ts +++ b/foundations/core/packages/core/src/utils.ts @@ -1018,3 +1018,28 @@ export function toRank (str: string | undefined): Rank | undefined { export function escapeRegExp (value: string): string { return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') } + +const MAX_EXTERNAL_AVATAR_URL_LENGTH = 2048 + +/** + * Returns a normalized http(s) URL suitable for an external avatar, or undefined + * when the value is not a string, not an absolute http(s) URL, carries credentials + * or is unreasonably long. The URL is never fetched here. + */ +export function normalizeExternalAvatarUrl (value: unknown): string | undefined { + if (typeof value !== 'string') return undefined + const trimmed = value.trim() + if (trimmed === '' || trimmed.length > MAX_EXTERNAL_AVATAR_URL_LENGTH) return undefined + + let url: URL + try { + url = new URL(trimmed) + } catch { + return undefined + } + if (url.protocol !== 'https:' && url.protocol !== 'http:') return undefined + if (url.username !== '' || url.password !== '') return undefined + + const normalized = url.toString() + return normalized.length > MAX_EXTERNAL_AVATAR_URL_LENGTH ? undefined : normalized +} diff --git a/models/contact/src/index.ts b/models/contact/src/index.ts index e4736288baa..0c8fbc56462 100644 --- a/models/contact/src/index.ts +++ b/models/contact/src/index.ts @@ -914,7 +914,7 @@ export function createModel (builder: Builder): void { type: AvatarType.EXTERNAL, getUrl: contact.function.GetExternalUrl }, - contact.avatarProvider.Color + contact.avatarProvider.External ) builder.createDoc( diff --git a/plugins/contact-resources/src/index.ts b/plugins/contact-resources/src/index.ts index b7e2193834f..06153fc31d2 100644 --- a/plugins/contact-resources/src/index.ts +++ b/plugins/contact-resources/src/index.ts @@ -15,6 +15,7 @@ // /* eslint-disable @typescript-eslint/no-unused-vars */ import { + getExternalAvatarUrl, getGravatarUrl, getName, type AvatarInfo, @@ -458,7 +459,7 @@ export default async (): Promise => ({ }), GetExternalUrl: async (person: Data>, name: string) => ({ color: getPersonColor(person, name), - url: person.avatarProps?.url + url: getExternalAvatarUrl(person) }), EmployeeSort: employeeSort, FilterChannelInResult: filterChannelInResult, diff --git a/plugins/contact/src/__tests__/providerAvatar.test.ts b/plugins/contact/src/__tests__/providerAvatar.test.ts new file mode 100644 index 00000000000..937b8244565 --- /dev/null +++ b/plugins/contact/src/__tests__/providerAvatar.test.ts @@ -0,0 +1,90 @@ +// +// Copyright © 2026 Hardcore Engineering Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import contact, { AvatarType } from '..' +import { getAvatarProviderId, getExternalAvatarUrl, getProviderAvatarUpdate } from '../utils' + +const url = 'https://idp.example.com/avatar.png' +const external = { avatarType: AvatarType.EXTERNAL, avatarProps: { url } } + +describe('getProviderAvatarUpdate', () => { + it('uses the provider avatar for a new person', () => { + expect(getProviderAvatarUpdate(undefined, url)).toEqual(external) + }) + + it('ignores missing or non-http avatar urls', () => { + expect(getProviderAvatarUpdate(undefined, undefined)).toBeUndefined() + expect(getProviderAvatarUpdate(undefined, 'javascript:alert(1)')).toBeUndefined() + expect(getProviderAvatarUpdate({ avatarType: AvatarType.COLOR }, 'ftp://idp.example.com/a.png')).toBeUndefined() + }) + + it('replaces the default color avatar', () => { + expect(getProviderAvatarUpdate({ avatarType: AvatarType.COLOR }, url)).toEqual(external) + expect(getProviderAvatarUpdate({ avatarType: AvatarType.COLOR, avatar: null, avatarProps: {} }, url)).toEqual( + external + ) + }) + + it('refreshes a previously synced external avatar', () => { + expect( + getProviderAvatarUpdate( + { avatarType: AvatarType.EXTERNAL, avatarProps: { url: 'https://old.example.com/a' } }, + url + ) + ).toEqual(external) + expect(getProviderAvatarUpdate(external, url)).toBeUndefined() + }) + + it('keeps an avatar chosen by the user', () => { + expect( + getProviderAvatarUpdate({ avatarType: AvatarType.COLOR, avatarProps: { color: 'Sky' } }, url) + ).toBeUndefined() + expect(getProviderAvatarUpdate({ avatarType: AvatarType.IMAGE, avatar: 'blob' as any }, url)).toBeUndefined() + expect( + getProviderAvatarUpdate({ avatarType: AvatarType.GRAVATAR, avatarProps: { url: 'gravatar-id' } }, url) + ).toBeUndefined() + }) +}) + +describe('getAvatarProviderId', () => { + it('resolves every avatar type to its own provider', () => { + expect(getAvatarProviderId(AvatarType.COLOR)).toBe(contact.avatarProvider.Color) + expect(getAvatarProviderId(AvatarType.IMAGE)).toBe(contact.avatarProvider.Image) + expect(getAvatarProviderId(AvatarType.GRAVATAR)).toBe(contact.avatarProvider.Gravatar) + expect(getAvatarProviderId(AvatarType.EXTERNAL)).toBe(contact.avatarProvider.External) + }) + + it('uses distinct provider ids', () => { + const ids = Object.values(AvatarType).map((type) => getAvatarProviderId(type)) + expect(new Set(ids).size).toBe(ids.length) + }) +}) + +describe('getExternalAvatarUrl', () => { + it('returns http(s) avatar urls', () => { + expect(getExternalAvatarUrl({ avatarProps: { url } })).toBe(url) + expect(getExternalAvatarUrl({ avatarProps: { url: 'http://avatars.example.com/u/1?v=4' } })).toBe( + 'http://avatars.example.com/u/1?v=4' + ) + }) + + it('never returns unsafe or missing urls', () => { + expect(getExternalAvatarUrl({})).toBeUndefined() + expect(getExternalAvatarUrl({ avatarProps: {} })).toBeUndefined() + expect(getExternalAvatarUrl({ avatarProps: { url: 'javascript:alert(1)' } })).toBeUndefined() + expect(getExternalAvatarUrl({ avatarProps: { url: 'data:image/svg+xml;base64,AAAA' } })).toBeUndefined() + expect(getExternalAvatarUrl({ avatarProps: { url: '/files/avatar.png' } })).toBeUndefined() + }) +}) diff --git a/plugins/contact/src/index.ts b/plugins/contact/src/index.ts index b600532a804..812dd77bcab 100644 --- a/plugins/contact/src/index.ts +++ b/plugins/contact/src/index.ts @@ -305,7 +305,8 @@ export const contactPlugin = plugin(contactId, { avatarProvider: { Color: '' as Ref, Image: '' as Ref, - Gravatar: '' as Ref + Gravatar: '' as Ref, + External: '' as Ref }, function: { GetColorUrl: '' as Resource, diff --git a/plugins/contact/src/utils.ts b/plugins/contact/src/utils.ts index eceb5ae392a..f37f45c3da1 100644 --- a/plugins/contact/src/utils.ts +++ b/plugins/contact/src/utils.ts @@ -26,6 +26,7 @@ import { generateId, Hierarchy, MeasureContext, + normalizeExternalAvatarUrl, notEmpty, Person as GlobalPerson, PersonId, @@ -39,6 +40,7 @@ import { import platform, { getMetadata, PlatformError } from '@hcengineering/platform' import { ColorDefinition } from '@hcengineering/ui' import contact, { + AvatarInfo, AvatarProvider, AvatarType, Channel, @@ -137,6 +139,8 @@ export function getAvatarProviderId (kind: AvatarType): Ref | un return contact.avatarProvider.Gravatar case AvatarType.COLOR: return contact.avatarProvider.Color + case AvatarType.EXTERNAL: + return contact.avatarProvider.External } return contact.avatarProvider.Image } @@ -415,6 +419,40 @@ export async function getAllUserAccounts (client: Client): Promise it.personUuid).filter(notEmpty) } +/** + * Returns the image URL of an external avatar, or undefined when the stored value is not + * an absolute http(s) URL (so e.g. javascript: or data: values are never used as image source). + */ +export function getExternalAvatarUrl (person: Pick): string | undefined { + return normalizeExternalAvatarUrl(person.avatarProps?.url) +} + +/** + * Returns the avatar fields to apply for an avatar URL provided by an identity provider + * (OIDC/OAuth), or undefined when nothing should change. Only the default color avatar and a + * previously synced external avatar are replaced; an uploaded image, Gravatar or an explicitly + * chosen color are kept. Pass undefined as person for a person that is about to be created. + */ +export function getProviderAvatarUpdate ( + person: Pick | undefined, + avatarUrl: string | undefined +): Pick | undefined { + const url = normalizeExternalAvatarUrl(avatarUrl) + if (url === undefined) return undefined + const update = { avatarType: AvatarType.EXTERNAL, avatarProps: { url } } + if (person === undefined) return update + + if (person.avatarType === AvatarType.EXTERNAL) { + return person.avatarProps?.url === url ? undefined : update + } + + const isDefaultColor = + (person.avatarType ?? AvatarType.COLOR) === AvatarType.COLOR && + person.avatar == null && + (person.avatarProps?.color ?? '') === '' + return isDefaultColor ? update : undefined +} + export async function ensureEmployee ( ctx: MeasureContext, me: Account, @@ -462,7 +500,8 @@ export async function ensureEmployeeForPerson ( personUuid: person.uuid, name: combineName(globalPerson.firstName, globalPerson.lastName), city: '', - avatarType: AvatarType.COLOR + avatarType: AvatarType.COLOR, + ...getProviderAvatarUpdate(undefined, globalPerson.avatarUrl) } personRef = generateId() @@ -480,6 +519,14 @@ export async function ensureEmployeeForPerson ( personUuid: person.uuid }) await client.tx(updatePersonTx) + } else { + // Keep the avatar in sync with the identity provider unless the user has chosen one + const avatarUpdate = getProviderAvatarUpdate(personByUuid, globalPerson?.avatarUrl) + if (avatarUpdate !== undefined) { + await client.tx( + txFactory.createTxUpdateDoc(contact.class.Person, contact.space.Contacts, personByUuid._id, avatarUpdate) + ) + } } const existingIdentifiers = toIdMap( diff --git a/pods/authProviders/src/__tests__/providerAvatar.test.ts b/pods/authProviders/src/__tests__/providerAvatar.test.ts new file mode 100644 index 00000000000..c5fa97b2cb3 --- /dev/null +++ b/pods/authProviders/src/__tests__/providerAvatar.test.ts @@ -0,0 +1,48 @@ +// +// Copyright © 2026 Hardcore Engineering Inc. +// +// Licensed under the Eclipse Public License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. You may +// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import { getProviderAvatarUrl } from '../utils' + +describe('getProviderAvatarUrl', () => { + test('reads the OIDC picture claim', () => { + expect(getProviderAvatarUrl({ sub: 'user', picture: 'https://idp.example.com/u/1.png' })).toBe( + 'https://idp.example.com/u/1.png' + ) + }) + + test('reads the first passport profile photo (Google, GitHub)', () => { + expect( + getProviderAvatarUrl({ + photos: [{ value: 'https://avatars.example.com/u/1?v=4' }, { value: 'https://other.example.com/2' }] + }) + ).toBe('https://avatars.example.com/u/1?v=4') + }) + + test('falls back to photos when the picture claim is not a usable url', () => { + expect( + getProviderAvatarUrl({ picture: 'javascript:alert(1)', photos: [{ value: 'http://idp.example.com/a' }] }) + ).toBe('http://idp.example.com/a') + }) + + test('returns undefined when there is no usable avatar', () => { + expect(getProviderAvatarUrl(undefined)).toBeUndefined() + expect(getProviderAvatarUrl('user')).toBeUndefined() + expect(getProviderAvatarUrl({})).toBeUndefined() + expect(getProviderAvatarUrl({ picture: '' })).toBeUndefined() + expect(getProviderAvatarUrl({ picture: { url: 'https://idp.example.com/a' } })).toBeUndefined() + expect(getProviderAvatarUrl({ photos: 'https://idp.example.com/a' })).toBeUndefined() + expect(getProviderAvatarUrl({ photos: [{ value: 'data:image/png;base64,AAAA' }] })).toBeUndefined() + }) +}) diff --git a/pods/authProviders/src/utils.ts b/pods/authProviders/src/utils.ts index 32f86c812c9..615a6e70b71 100644 --- a/pods/authProviders/src/utils.ts +++ b/pods/authProviders/src/utils.ts @@ -13,7 +13,14 @@ // limitations under the License. // import { AccountDB, joinWithProvider, LoginInfo, loginOrSignUpWithProvider } from '@hcengineering/account' -import { BrandingMap, concatLink, getBranding, MeasureContext, SocialKey } from '@hcengineering/core' +import { + BrandingMap, + concatLink, + getBranding, + MeasureContext, + normalizeExternalAvatarUrl, + SocialKey +} from '@hcengineering/core' import { IncomingHttpHeaders } from 'http' import qs from 'querystringify' @@ -59,6 +66,20 @@ export function encodeState (ctx: any, brandings: BrandingMap): string { return encodeURIComponent(JSON.stringify(state)) } +/** + * Picks the avatar URL from a provider profile: the OIDC `picture` claim, or the first + * entry of the passport `photos` list (Google, GitHub). Only http(s) URLs are returned. + */ +export function getProviderAvatarUrl (user: any): string | undefined { + if (user == null || typeof user !== 'object') return undefined + const candidates = [user.picture, Array.isArray(user.photos) ? user.photos[0]?.value : undefined] + for (const candidate of candidates) { + const url = normalizeExternalAvatarUrl(candidate) + if (url !== undefined) return url + } + return undefined +} + export async function handleProviderAuth ( measureCtx: MeasureContext, db: AccountDB, @@ -78,6 +99,7 @@ export async function handleProviderAuth ( let loginInfo: LoginInfo | null const state = safeParseAuthState(rawState) const branding = getBranding(brandings, state?.branding) + const avatarUrl = getProviderAvatarUrl(user) if (state.inviteId != null && state.inviteId !== '' && state.autoJoin !== true) { loginInfo = await joinWithProvider( @@ -89,7 +111,8 @@ export async function handleProviderAuth ( last, state.inviteId as any, socialKey, - signUpDisabled + signUpDisabled, + avatarUrl ) } else { loginInfo = await loginOrSignUpWithProvider( @@ -100,7 +123,8 @@ export async function handleProviderAuth ( first, last, socialKey, - signUpDisabled === true || state.autoJoin === true + signUpDisabled === true || state.autoJoin === true, + avatarUrl ) } diff --git a/server-plugins/notification-resources/src/push.ts b/server-plugins/notification-resources/src/push.ts index a1d5890d1d7..43f7072860b 100644 --- a/server-plugins/notification-resources/src/push.ts +++ b/server-plugins/notification-resources/src/push.ts @@ -44,6 +44,7 @@ import { encodeObjectURI } from '@hcengineering/view' import contact, { type AvatarInfo, getAvatarProviderId, + getExternalAvatarUrl, getGravatarUrl, Person, PersonSpace @@ -189,6 +190,8 @@ export async function createPushNotification ( } } else if (provider === contact.avatarProvider.Gravatar && senderAvatar.avatarProps?.url !== undefined) { data.icon = getGravatarUrl(senderAvatar.avatarProps?.url, 512) + } else if (provider === contact.avatarProvider.External) { + data.icon = getExternalAvatarUrl(senderAvatar) } } diff --git a/server/account/src/__tests__/operations.test.ts b/server/account/src/__tests__/operations.test.ts index 67ea04e5544..ea3dd7b3803 100644 --- a/server/account/src/__tests__/operations.test.ts +++ b/server/account/src/__tests__/operations.test.ts @@ -54,6 +54,7 @@ import { requestPasswordReset, changePassword, getPerson, + getUserProfile, getSocialIds, getAccountInfo, createAccessLink, @@ -3247,6 +3248,47 @@ describe('account operations', () => { }) }) + describe('getUserProfile', () => { + const ownerUuid = 'profile-owner-uuid' as PersonUuid + const profileDb = { + person: { + findOne: jest.fn() + }, + userProfile: { + findOne: jest.fn() + } + } as unknown as AccountDB + + beforeEach(() => { + ;(profileDb.person.findOne as jest.Mock).mockResolvedValue({ + uuid: ownerUuid, + firstName: 'John', + lastName: 'Doe', + avatarUrl: 'https://idp.example.com/avatar.png?signature=secret' + }) + ;(profileDb.userProfile.findOne as jest.Mock).mockResolvedValue({ + personUuid: ownerUuid, + bio: 'Hello', + isPublic: true + }) + }) + + test('should not expose the provider avatar url in a public profile', async () => { + const result = await getUserProfile(mockCtx, profileDb, mockBranding, undefined, { personUuid: ownerUuid }) + + expect(result).toEqual({ uuid: ownerUuid, firstName: 'John', lastName: 'Doe', bio: 'Hello', isPublic: true }) + expect(result).not.toHaveProperty('avatarUrl') + }) + + test('should not expose the provider avatar url in the own profile', async () => { + ;(decodeTokenVerbose as jest.Mock).mockReturnValue({ account: ownerUuid }) + + const result = await getUserProfile(mockCtx, profileDb, mockBranding, mockToken, {}) + + expect(result).not.toHaveProperty('avatarUrl') + }) + }) + describe('getAccountInfo', () => { const callerAccount = 'caller-account-uuid' as AccountUuid const otherAccount = 'other-account-uuid' as AccountUuid diff --git a/server/account/src/__tests__/utils.test.ts b/server/account/src/__tests__/utils.test.ts index ea1130854a0..53b7957dc43 100644 --- a/server/account/src/__tests__/utils.test.ts +++ b/server/account/src/__tests__/utils.test.ts @@ -1910,6 +1910,69 @@ describe('account utils', () => { expect(result).toBeNull() }) + + describe('provider avatar url', () => { + const personUuid = 'existing-person' as PersonUuid + + function mockExistingAccount (storedAvatarUrl?: string): void { + ;(mockDb.socialId.findOne as jest.Mock).mockResolvedValue({ + _id: 'social-id', + key: 'google:test-id', + personUuid, + verifiedOn: 1 + }) + ;(mockDb.person.findOne as jest.Mock).mockResolvedValue({ + uuid: personUuid, + firstName: 'John', + lastName: 'Doe', + avatarUrl: storedAvatarUrl + }) + ;(mockDb.account.findOne as jest.Mock).mockResolvedValue({ uuid: personUuid }) + } + + async function login (avatarUrl?: string): Promise { + await loginOrSignUpWithProvider( + mockCtx, + mockDb, + mockBranding, + 'test@example.com', + 'John', + 'Doe', + mockSocialId, + false, + avatarUrl + ) + } + + test('should store an http(s) avatar url from the provider', async () => { + mockExistingAccount() + + await login('https://idp.example.com/avatar.png') + + expect(mockDb.person.update).toHaveBeenCalledWith( + { uuid: personUuid }, + { avatarUrl: 'https://idp.example.com/avatar.png' } + ) + }) + + test('should not update an unchanged avatar url', async () => { + mockExistingAccount('https://idp.example.com/avatar.png') + + await login('https://idp.example.com/avatar.png') + + expect(mockDb.person.update).not.toHaveBeenCalled() + }) + + test('should ignore missing and non-http avatar urls', async () => { + mockExistingAccount('https://idp.example.com/avatar.png') + + await login(undefined) + await login('javascript:alert(1)') + await login('data:image/png;base64,AAAA') + + expect(mockDb.person.update).not.toHaveBeenCalled() + }) + }) }) describe('getWorkspaceInvite', () => { diff --git a/server/account/src/collections/postgres/migrations.ts b/server/account/src/collections/postgres/migrations.ts index ddc74cc1dbc..e593681e857 100644 --- a/server/account/src/collections/postgres/migrations.ts +++ b/server/account/src/collections/postgres/migrations.ts @@ -84,7 +84,8 @@ export function getMigrations (ns: string, flavor: DBFlavor): [string, string][] getV24Migration(ns, flavor), getV25Migration(ns, flavor), getV26Migration(ns, flavor), - getV27Migration(ns, flavor) + getV27Migration(ns, flavor), + getV28Migration(ns, flavor) ] } @@ -840,3 +841,15 @@ function getV27Migration (ns: string, flavor: DBFlavor): [string, string] { ` ] } + +function getV28Migration (ns: string, flavor: DBFlavor): [string, string] { + const types = dbTypes[flavor] + return [ + 'account_db_v28_add_person_avatar_url', + ` + -- External avatar URL provided by an identity provider (OIDC/OAuth) on login. + ALTER TABLE ${ns}.person + ADD COLUMN IF NOT EXISTS avatar_url ${types.string}; + ` + ] +} diff --git a/server/account/src/operations.ts b/server/account/src/operations.ts index 0e8953515d0..cbb97adcac8 100644 --- a/server/account/src/operations.ts +++ b/server/account/src/operations.ts @@ -3285,7 +3285,7 @@ async function setMyProfile ( } } -async function getUserProfile ( +export async function getUserProfile ( ctx: MeasureContext, db: AccountDB, branding: Branding | null, @@ -3331,8 +3331,11 @@ async function getUserProfile ( // 1. Requesting own profile (authenticated user requesting their own) // 2. Profile is public if (requestingAccount === targetPersonUuid || profile.isPublic) { + // The identity provider avatar URL may carry signed query parameters; it is only + // meant for the person's own workspace avatar sync and is never part of a profile. + const { avatarUrl, ...profilePerson } = person const res: PersonWithProfile = { - ...person, + ...profilePerson, ...profile } diff --git a/server/account/src/utils.ts b/server/account/src/utils.ts index 5c5115c1c94..4997d0ea508 100644 --- a/server/account/src/utils.ts +++ b/server/account/src/utils.ts @@ -21,6 +21,7 @@ import { groupByArray, isActiveMode, type MeasureContext, + normalizeExternalAvatarUrl, type Person, type PersonId, type PersonUuid, @@ -1630,7 +1631,8 @@ export async function loginOrSignUpWithProvider ( first: string, last: string, socialId: SocialKey, - signUpDisabled = false + signUpDisabled = false, + avatarUrl?: string ): Promise { try { const normalizedEmail = email != null ? cleanEmail(email) : '' @@ -1676,6 +1678,13 @@ export async function loginOrSignUpWithProvider ( await db.person.update({ uuid: personUuid }, { firstName: first, lastName: last }) } + // Keep the provider avatar URL up to date. It is only stored as a link and never fetched here; + // workspaces decide whether to show it (see ensureEmployee in @hcengineering/contact). + const providerAvatarUrl = normalizeExternalAvatarUrl(avatarUrl) + if (providerAvatarUrl !== undefined && person.avatarUrl !== providerAvatarUrl) { + await db.person.update({ uuid: personUuid }, { avatarUrl: providerAvatarUrl }) + } + // We should check and reset password if there's an account with password but no social ids have been // confirmed yet const confirmedSocialId = await db.socialId.findOne({ personUuid, verifiedOn: { $gt: 0 } }) @@ -1732,7 +1741,8 @@ export async function joinWithProvider ( last: string, inviteId: string, socialId: SocialKey, - signUpDisabled = false + signUpDisabled = false, + avatarUrl?: string ): Promise { const normalizedEmail = email != null ? cleanEmail(email) : '' const invite = await getWorkspaceInvite(db, inviteId) @@ -1755,7 +1765,8 @@ export async function joinWithProvider ( first, last, socialId, - signUpDisabled + signUpDisabled, + avatarUrl ) if (loginInfo == null) {