Skip to content

Commit 9e0e9ab

Browse files
committed
fix(activity): keep presence avatars without trusting client readerId
Accept an optional https image on updatePresence, return only the public reader card, and resolve readerId from session or socket only.
1 parent 29a1b95 commit 9e0e9ab

10 files changed

Lines changed: 160 additions & 13 deletions

File tree

apps/core/src/modules/activity/activity.controller.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
UpdatePresenceDto,
3838
} from './activity.schema'
3939
import { ActivityService } from './activity.service'
40+
import { toPublicPresenceReader } from './activity.util'
4041
import { ActivitySampleService } from './sample/activity-sample.service'
4142

4243
const ARTICLE_REF_FIELDS = [
@@ -126,7 +127,7 @@ export class ActivityController {
126127
.map((item) => item.readerId)
127128
.filter(Boolean) as string[]
128129
const readerRows = await this.readerService.findReaderInIds(readerIds)
129-
const readers = readerRows.map((item) => ({ ...item, id: item.id }))
130+
const readers = readerRows.map((item) => toPublicPresenceReader(item))
130131

131132
return {
132133
presence: keyBy(

apps/core/src/modules/activity/activity.interface.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,5 @@ export interface ActivityPresence {
1919

2020
ip?: string
2121
readerId?: string
22+
image?: string
2223
}

apps/core/src/modules/activity/activity.schema.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createZodDto } from 'nestjs-zod'
22
import { z } from 'zod'
33

4-
import { zCoerceInt, zEntityId } from '~/common/zod'
4+
import { zCoerceInt, zEntityId, zHttpsUrl } from '~/common/zod'
55
import { BasicPagerSchema } from '~/shared/dto/pager.dto'
66

77
import { Activity } from './activity.constant'
@@ -99,6 +99,7 @@ export const UpdatePresenceSchema = z.object({
9999
displayName: z.string().max(50).optional(),
100100
sid: z.string().min(1).max(64),
101101
readerId: z.string().optional(),
102+
image: z.string().max(2048).pipe(zHttpsUrl).optional(),
102103
})
103104

104105
export class UpdatePresenceDto extends createZodDto(UpdatePresenceSchema) {}

apps/core/src/modules/activity/activity.service.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ import {
4141
extractArticleIdFromRoomName,
4242
isValidRoomName,
4343
parseRoomName,
44+
resolvePresenceReaderId,
45+
toPublicPresenceReader,
4446
} from './activity.util'
4547

4648
interface ActivityPayloadWithRef {
@@ -346,12 +348,10 @@ export class ActivityService implements OnModuleInit, OnModuleDestroy {
346348
const socket = roomSockets[index]
347349
const socketMeta = roomSocketMetas[index]
348350

349-
// Prefer the readerId resolved server-side from the HTTP session cookie
350-
// (RolesGuard runs globally and fills request.readerId before this
351-
// service runs). Fall back to the socket-handshake binding, and finally
352-
// the client-provided value, both of which are less authoritative.
353-
const resolvedReaderId =
354-
RequestContext.currentReaderId() || socketMeta?.readerId || data.readerId
351+
const resolvedReaderId = resolvePresenceReaderId(
352+
RequestContext.currentReaderId(),
353+
socketMeta?.readerId,
354+
)
355355

356356
const presenceData: ActivityPresence = {
357357
...data,
@@ -360,6 +360,7 @@ export class ActivityService implements OnModuleInit, OnModuleDestroy {
360360
updatedAt: Date.now(),
361361
connectedAt: socketMeta?.connectedAt ?? Date.now(),
362362
readerId: resolvedReaderId,
363+
image: data.image,
363364
ip,
364365
}
365366

@@ -370,12 +371,14 @@ export class ActivityService implements OnModuleInit, OnModuleDestroy {
370371
resolvedReaderId,
371372
])
372373
if (reader.length) {
374+
const publicReader = toPublicPresenceReader(reader[0])
373375
Object.assign(serializedPresenceData, {
374-
reader: camelcaseKeys({
375-
...reader[0],
376-
id: reader[0].id.toString(),
377-
}),
376+
reader: camelcaseKeys(publicReader),
378377
})
378+
if (!serializedPresenceData.image && publicReader.image) {
379+
serializedPresenceData.image = publicReader.image
380+
presenceData.image = publicReader.image
381+
}
379382
}
380383
}
381384

apps/core/src/modules/activity/activity.util.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,27 @@
11
const prefix = 'article-'
22

3+
export function resolvePresenceReaderId(
4+
sessionReaderId?: string | null,
5+
socketReaderId?: string | null,
6+
_clientReaderId?: string | null,
7+
): string | undefined {
8+
return sessionReaderId || socketReaderId || undefined
9+
}
10+
11+
export function toPublicPresenceReader(reader: {
12+
id: string | number | bigint
13+
name?: string | null
14+
image?: string | null
15+
handle?: string | null
16+
}) {
17+
return {
18+
id: String(reader.id),
19+
name: reader.name ?? null,
20+
image: reader.image ?? null,
21+
handle: reader.handle ?? null,
22+
}
23+
}
24+
325
export const buildArticleRoomName = (articleId: string) =>
426
`${prefix}${articleId}`
527

apps/core/test/src/modules/activity/activity.controller.spec.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,45 @@ const createController = (opts: {
103103
translationService as any,
104104
)
105105

106-
return { controller, activityService, translationService }
106+
return { controller, activityService, translationService, readerService }
107107
}
108108

109+
describe('ActivityController.getPresence', () => {
110+
it('returns only the public reader card', async () => {
111+
const { controller, activityService, readerService } = createController({})
112+
activityService.getRoomPresence = vi.fn(async () => [
113+
{
114+
identity: 'abcd1234',
115+
readerId: 'reader-1',
116+
ip: '1.2.3.4',
117+
position: 12,
118+
},
119+
])
120+
readerService.findReaderInIds = vi.fn(async () => [
121+
{
122+
id: 'reader-1',
123+
name: 'Magren',
124+
image: 'https://avatars.githubusercontent.com/u/1?v=4',
125+
handle: 'magren',
126+
email: 'hidden@example.com',
127+
emailVerified: true,
128+
role: 'reader',
129+
membership: { status: 'active' },
130+
},
131+
])
132+
133+
const res = await controller.getPresence({ roomName: 'article-1' } as any)
134+
135+
expect(res.readers['reader-1']).toEqual({
136+
id: 'reader-1',
137+
name: 'Magren',
138+
image: 'https://avatars.githubusercontent.com/u/1?v=4',
139+
handle: 'magren',
140+
})
141+
expect(res.presence.abcd1234).not.toHaveProperty('ip')
142+
})
143+
})
144+
109145
describe('ActivityController.getRoomsInfo', () => {
110146
it('returns bare data when lang is absent', async () => {
111147
const { controller } = createController({
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { describe, expect, it } from 'vitest'
2+
3+
import { UpdatePresenceSchema } from '~/modules/activity/activity.schema'
4+
5+
const base = {
6+
identity: 'abcd1234',
7+
roomName: 'article-1',
8+
ts: 1,
9+
position: 10,
10+
sid: 'abcd1234',
11+
}
12+
13+
describe('UpdatePresenceSchema image', () => {
14+
it('accepts an https avatar url', () => {
15+
const parsed = UpdatePresenceSchema.parse({
16+
...base,
17+
image: 'https://avatars.githubusercontent.com/u/1?v=4',
18+
})
19+
expect(parsed.image).toBe('https://avatars.githubusercontent.com/u/1?v=4')
20+
})
21+
22+
it('rejects http and non-url images', () => {
23+
expect(() =>
24+
UpdatePresenceSchema.parse({
25+
...base,
26+
image: 'http://example.com/a.png',
27+
}),
28+
).toThrow()
29+
expect(() =>
30+
UpdatePresenceSchema.parse({
31+
...base,
32+
image: 'data:image/png;base64,abc',
33+
}),
34+
).toThrow()
35+
})
36+
})
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { describe, expect, it } from 'vitest'
2+
3+
import {
4+
resolvePresenceReaderId,
5+
toPublicPresenceReader,
6+
} from '~/modules/activity/activity.util'
7+
8+
describe('resolvePresenceReaderId', () => {
9+
it('prefers the session reader over the socket binding', () => {
10+
expect(resolvePresenceReaderId('session-1', 'socket-1')).toBe('session-1')
11+
})
12+
13+
it('falls back to the socket handshake reader', () => {
14+
expect(resolvePresenceReaderId(null, 'socket-1')).toBe('socket-1')
15+
})
16+
17+
it('ignores a client-supplied reader id', () => {
18+
expect(resolvePresenceReaderId(null, undefined, 'client-1')).toBeUndefined()
19+
})
20+
})
21+
22+
describe('toPublicPresenceReader', () => {
23+
it('keeps only the public card fields', () => {
24+
expect(
25+
toPublicPresenceReader({
26+
id: 133259626412523520n as unknown as string,
27+
name: 'Magren',
28+
image: 'https://avatars.githubusercontent.com/u/1?v=4',
29+
handle: 'magren',
30+
email: 'hidden@example.com',
31+
emailVerified: true,
32+
role: 'reader',
33+
bannedAt: new Date(),
34+
membership: { status: 'active' },
35+
}),
36+
).toEqual({
37+
id: '133259626412523520',
38+
name: 'Magren',
39+
image: 'https://avatars.githubusercontent.com/u/1?v=4',
40+
handle: 'magren',
41+
})
42+
})
43+
})

packages/api-client/controllers/activity.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ export class ActivityController<ResponseWrapper> implements IController {
9494
ts,
9595
displayName,
9696
readerId,
97+
image,
9798
}: {
9899
roomName: string
99100
position: number
@@ -103,6 +104,7 @@ export class ActivityController<ResponseWrapper> implements IController {
103104
displayName?: string
104105
ts?: number
105106
readerId?: string
107+
image?: string
106108
}) {
107109
return this.proxy.presence.update.post({
108110
data: {
@@ -113,6 +115,7 @@ export class ActivityController<ResponseWrapper> implements IController {
113115
sid,
114116
readerId,
115117
displayName,
118+
image,
116119
},
117120
})
118121
}

packages/api-client/models/activity.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export interface ActivityPresence {
1313
readerId?: string
1414

1515
displayName?: string
16+
image?: string
1617
}
1718

1819
export interface RoomOmittedNote {

0 commit comments

Comments
 (0)