fix(aim): enforce direct-room membership in Convex queries, skip message query until room id, harden logout
- commit
- a2662b96b5528a728a0bb95afc78398528e6548f
- author
- Kit · · 2026-09-27 12:50 UTC
- parent
- 995a83f
- signature
- SSH Unverified
SHA256:H91DFuEG4jpk5edzLA4aUKkpv4zv9Q339ipRmkph6TkThe key is unknown
5 files · +41 -11
M src/convex/aim.ts +9 -0
| @@ -190,6 +190,9 @@ export const getRoomMessages = authQuery({ | |||
| 190 | 190 | export const getRoomTyping = authQuery({ | |
| 191 | 191 | args: { roomId: v.id('chatRooms') }, | |
| 192 | 192 | handler: async (ctx, args) => { | |
| 193 | + | const room = await ctx.db.get(args.roomId); | |
| 194 | + | if (!room) return []; | |
| 195 | + | await assertRoomAccess(ctx, room, ctx.user._id); | |
| 193 | 196 | const since = Date.now() - TYPING_TTL_MS; | |
| 194 | 197 | const rows = await ctx.db | |
| 195 | 198 | .query('typing') | |
| @@ -335,6 +338,9 @@ export const createGroupRoom = authMutation({ | |||
| 335 | 338 | export const markRoomRead = authMutation({ | |
| 336 | 339 | args: { roomId: v.id('chatRooms') }, | |
| 337 | 340 | handler: async (ctx, args) => { | |
| 341 | + | const room = await ctx.db.get(args.roomId); | |
| 342 | + | if (!room) throw new Error('Chat room not found'); | |
| 343 | + | await assertRoomAccess(ctx, room, ctx.user._id); | |
| 338 | 344 | const now = Date.now(); | |
| 339 | 345 | const existing = await ctx.db | |
| 340 | 346 | .query('roomReads') | |
| @@ -358,6 +364,9 @@ export const markRoomRead = authMutation({ | |||
| 358 | 364 | export const setTyping = authMutation({ | |
| 359 | 365 | args: { roomId: v.id('chatRooms'), typing: v.boolean() }, | |
| 360 | 366 | handler: async (ctx, args) => { | |
| 367 | + | const room = await ctx.db.get(args.roomId); | |
| 368 | + | if (!room) throw new Error('Chat room not found'); | |
| 369 | + | await assertRoomAccess(ctx, room, ctx.user._id); | |
| 361 | 370 | const existing = await ctx.db | |
| 362 | 371 | .query('typing') | |
| 363 | 372 | .withIndex('by_user_room', (q) => q.eq('userId', ctx.user._id).eq('roomId', args.roomId)) | |
| @@ -190,6 +190,9 @@ export const getRoomMessages = authQuery({ | |||||
| 190 | export const getRoomTyping = authQuery({ | 190 | export const getRoomTyping = authQuery({ | ||
| 191 | args: { roomId: v.id('chatRooms') }, | 191 | args: { roomId: v.id('chatRooms') }, | ||
| 192 | handler: async (ctx, args) => { | 192 | handler: async (ctx, args) => { | ||
| 193 | + | const room = await ctx.db.get(args.roomId); | |||
| 194 | + | if (!room) return []; | |||
| 195 | + | await assertRoomAccess(ctx, room, ctx.user._id); | |||
| 193 | const since = Date.now() - TYPING_TTL_MS; | 196 | const since = Date.now() - TYPING_TTL_MS; | ||
| 194 | const rows = await ctx.db | 197 | const rows = await ctx.db | ||
| 195 | .query('typing') | 198 | .query('typing') | ||
| @@ -335,6 +338,9 @@ export const createGroupRoom = authMutation({ | |||||
| 335 | export const markRoomRead = authMutation({ | 338 | export const markRoomRead = authMutation({ | ||
| 336 | args: { roomId: v.id('chatRooms') }, | 339 | args: { roomId: v.id('chatRooms') }, | ||
| 337 | handler: async (ctx, args) => { | 340 | handler: async (ctx, args) => { | ||
| 341 | + | const room = await ctx.db.get(args.roomId); | |||
| 342 | + | if (!room) throw new Error('Chat room not found'); | |||
| 343 | + | await assertRoomAccess(ctx, room, ctx.user._id); | |||
| 338 | const now = Date.now(); | 344 | const now = Date.now(); | ||
| 339 | const existing = await ctx.db | 345 | const existing = await ctx.db | ||
| 340 | .query('roomReads') | 346 | .query('roomReads') | ||
| @@ -358,6 +364,9 @@ export const markRoomRead = authMutation({ | |||||
| 358 | export const setTyping = authMutation({ | 364 | export const setTyping = authMutation({ | ||
| 359 | args: { roomId: v.id('chatRooms'), typing: v.boolean() }, | 365 | args: { roomId: v.id('chatRooms'), typing: v.boolean() }, | ||
| 360 | handler: async (ctx, args) => { | 366 | handler: async (ctx, args) => { | ||
| 367 | + | const room = await ctx.db.get(args.roomId); | |||
| 368 | + | if (!room) throw new Error('Chat room not found'); | |||
| 369 | + | await assertRoomAccess(ctx, room, ctx.user._id); | |||
| 361 | const existing = await ctx.db | 370 | const existing = await ctx.db | ||
| 362 | .query('typing') | 371 | .query('typing') | ||
| 363 | .withIndex('by_user_room', (q) => q.eq('userId', ctx.user._id).eq('roomId', args.roomId)) | 372 | .withIndex('by_user_room', (q) => q.eq('userId', ctx.user._id).eq('roomId', args.roomId)) | ||
M src/convex/queries.ts +13 -1
| @@ -1,7 +1,7 @@ | |||
| 1 | 1 | import { query } from './_generated/server'; | |
| 2 | 2 | import { authQuery } from './auth'; | |
| 3 | 3 | import { v } from 'convex/values'; | |
| 4 | - | import { toPublicUser } from './aim'; | |
| 4 | + | import { toPublicUser, assertRoomAccess } from './aim'; | |
| 5 | 5 | ||
| 6 | 6 | // Timeout threshold for marking users as offline (2 minutes) | |
| 7 | 7 | const ONLINE_TIMEOUT_MS = 2 * 60 * 1000; | |
| @@ -36,6 +36,10 @@ export const getMessagesPublic = query({ | |||
| 36 | 36 | limit: v.optional(v.number()) | |
| 37 | 37 | }, | |
| 38 | 38 | handler: async (ctx, args) => { | |
| 39 | + | // Direct (IM) rooms are never public | |
| 40 | + | const room = await ctx.db.get(args.roomId); | |
| 41 | + | if (!room || room.type === 'direct') return []; | |
| 42 | + | ||
| 39 | 43 | // Enforce max limit and clamp to a safe positive integer | |
| 40 | 44 | const rawLimit = args.limit ?? PUBLIC_MESSAGE_LIMIT; | |
| 41 | 45 | const limit = Math.max(1, Math.min(PUBLIC_MESSAGE_LIMIT, Math.floor(rawLimit))); | |
| @@ -84,6 +88,10 @@ export const getMessagesPublicPage = query({ | |||
| 84 | 88 | beforeTimestamp: v.optional(v.number()) | |
| 85 | 89 | }, | |
| 86 | 90 | handler: async (ctx, args) => { | |
| 91 | + | // Direct (IM) rooms are never public | |
| 92 | + | const room = await ctx.db.get(args.roomId); | |
| 93 | + | if (!room || room.type === 'direct') return { messages: [], hasMore: false }; | |
| 94 | + | ||
| 87 | 95 | const rawLimit = args.limit ?? 50; | |
| 88 | 96 | const limit = Math.max(1, Math.min(PUBLIC_MESSAGE_LIMIT, Math.floor(rawLimit))); | |
| 89 | 97 | ||
| @@ -167,6 +175,10 @@ export const getMessages = authQuery({ | |||
| 167 | 175 | limit: v.optional(v.number()) | |
| 168 | 176 | }, | |
| 169 | 177 | handler: async (ctx, args) => { | |
| 178 | + | const room = await ctx.db.get(args.roomId); | |
| 179 | + | if (!room) return []; | |
| 180 | + | await assertRoomAccess(ctx, room, ctx.user._id); | |
| 181 | + | ||
| 170 | 182 | const MAX_AUTH_MESSAGE_LIMIT = 200; | |
| 171 | 183 | const rawLimit = args.limit ?? 100; | |
| 172 | 184 | const limit = Math.max(1, Math.min(MAX_AUTH_MESSAGE_LIMIT, Math.floor(rawLimit))); | |
| @@ -1,7 +1,7 @@ | |||||
| 1 | import { query } from './_generated/server'; | 1 | import { query } from './_generated/server'; | ||
| 2 | import { authQuery } from './auth'; | 2 | import { authQuery } from './auth'; | ||
| 3 | import { v } from 'convex/values'; | 3 | import { v } from 'convex/values'; | ||
| 4 | - | import { toPublicUser } from './aim'; | 4 | + | import { toPublicUser, assertRoomAccess } from './aim'; |
| 5 | 5 | ||||
| 6 | // Timeout threshold for marking users as offline (2 minutes) | 6 | // Timeout threshold for marking users as offline (2 minutes) | ||
| 7 | const ONLINE_TIMEOUT_MS = 2 * 60 * 1000; | 7 | const ONLINE_TIMEOUT_MS = 2 * 60 * 1000; | ||
| @@ -36,6 +36,10 @@ export const getMessagesPublic = query({ | |||||
| 36 | limit: v.optional(v.number()) | 36 | limit: v.optional(v.number()) | ||
| 37 | }, | 37 | }, | ||
| 38 | handler: async (ctx, args) => { | 38 | handler: async (ctx, args) => { | ||
| 39 | + | // Direct (IM) rooms are never public | |||
| 40 | + | const room = await ctx.db.get(args.roomId); | |||
| 41 | + | if (!room || room.type === 'direct') return []; | |||
| 42 | + | ||||
| 39 | // Enforce max limit and clamp to a safe positive integer | 43 | // Enforce max limit and clamp to a safe positive integer | ||
| 40 | const rawLimit = args.limit ?? PUBLIC_MESSAGE_LIMIT; | 44 | const rawLimit = args.limit ?? PUBLIC_MESSAGE_LIMIT; | ||
| 41 | const limit = Math.max(1, Math.min(PUBLIC_MESSAGE_LIMIT, Math.floor(rawLimit))); | 45 | const limit = Math.max(1, Math.min(PUBLIC_MESSAGE_LIMIT, Math.floor(rawLimit))); | ||
| @@ -84,6 +88,10 @@ export const getMessagesPublicPage = query({ | |||||
| 84 | beforeTimestamp: v.optional(v.number()) | 88 | beforeTimestamp: v.optional(v.number()) | ||
| 85 | }, | 89 | }, | ||
| 86 | handler: async (ctx, args) => { | 90 | handler: async (ctx, args) => { | ||
| 91 | + | // Direct (IM) rooms are never public | |||
| 92 | + | const room = await ctx.db.get(args.roomId); | |||
| 93 | + | if (!room || room.type === 'direct') return { messages: [], hasMore: false }; | |||
| 94 | + | ||||
| 87 | const rawLimit = args.limit ?? 50; | 95 | const rawLimit = args.limit ?? 50; | ||
| 88 | const limit = Math.max(1, Math.min(PUBLIC_MESSAGE_LIMIT, Math.floor(rawLimit))); | 96 | const limit = Math.max(1, Math.min(PUBLIC_MESSAGE_LIMIT, Math.floor(rawLimit))); | ||
| 89 | 97 | ||||
| @@ -167,6 +175,10 @@ export const getMessages = authQuery({ | |||||
| 167 | limit: v.optional(v.number()) | 175 | limit: v.optional(v.number()) | ||
| 168 | }, | 176 | }, | ||
| 169 | handler: async (ctx, args) => { | 177 | handler: async (ctx, args) => { | ||
| 178 | + | const room = await ctx.db.get(args.roomId); | |||
| 179 | + | if (!room) return []; | |||
| 180 | + | await assertRoomAccess(ctx, room, ctx.user._id); | |||
| 181 | + | ||||
| 170 | const MAX_AUTH_MESSAGE_LIMIT = 200; | 182 | const MAX_AUTH_MESSAGE_LIMIT = 200; | ||
| 171 | const rawLimit = args.limit ?? 100; | 183 | const rawLimit = args.limit ?? 100; | ||
| 172 | const limit = Math.max(1, Math.min(MAX_AUTH_MESSAGE_LIMIT, Math.floor(rawLimit))); | 184 | const limit = Math.max(1, Math.min(MAX_AUTH_MESSAGE_LIMIT, Math.floor(rawLimit))); | ||
M src/lib/components/chat-room.svelte +4 -6
| @@ -70,12 +70,10 @@ | |||
| 70 | 70 | const groupRoomsQuery = isClient ? useQuery(convexApi.aim.getGroupRooms, {}) : null; | |
| 71 | 71 | const roomTopic = $derived(groupRoomsQuery?.data?.find((r) => r.id === roomId)?.topic ?? null); | |
| 72 | 72 | ||
| 73 | - | // Subscribe to messages - only when room ID is available | |
| 74 | - | const messagesQuery = $derived( | |
| 75 | - | roomId && isClient | |
| 76 | - | ? useQuery(convexApi.queries.getMessagesPublic, () => ({ roomId: roomId! })) | |
| 77 | - | : null | |
| 78 | - | ); | |
| 73 | + | // Subscribe to messages - skipped until the room ID is available | |
| 74 | + | const messagesQuery = isClient | |
| 75 | + | ? useQuery(convexApi.queries.getMessagesPublic, () => (roomId ? { roomId } : 'skip')) | |
| 76 | + | : null; | |
| 79 | 77 | ||
| 80 | 78 | const convexClient = isClient ? useConvexClient() : null; | |
| 81 | 79 | ||
| @@ -70,12 +70,10 @@ | |||||
| 70 | const groupRoomsQuery = isClient ? useQuery(convexApi.aim.getGroupRooms, {}) : null; | 70 | const groupRoomsQuery = isClient ? useQuery(convexApi.aim.getGroupRooms, {}) : null; | ||
| 71 | const roomTopic = $derived(groupRoomsQuery?.data?.find((r) => r.id === roomId)?.topic ?? null); | 71 | const roomTopic = $derived(groupRoomsQuery?.data?.find((r) => r.id === roomId)?.topic ?? null); | ||
| 72 | 72 | ||||
| 73 | - | // Subscribe to messages - only when room ID is available | 73 | + | // Subscribe to messages - skipped until the room ID is available |
| 74 | - | const messagesQuery = $derived( | 74 | + | const messagesQuery = isClient |
| 75 | - | roomId && isClient | 75 | + | ? useQuery(convexApi.queries.getMessagesPublic, () => (roomId ? { roomId } : 'skip')) |
| 76 | - | ? useQuery(convexApi.queries.getMessagesPublic, () => ({ roomId: roomId! })) | 76 | + | : null; |
| 77 | - | : null | |||
| 78 | - | ); | |||
| 79 | 77 | ||||
| 80 | const convexClient = isClient ? useConvexClient() : null; | 78 | const convexClient = isClient ? useConvexClient() : null; | ||
| 81 | 79 | ||||
M src/routes/+layout.svelte +3 -1
| @@ -196,7 +196,9 @@ | |||
| 196 | 196 | $effect(() => { | |
| 197 | 197 | if (!browser) return; | |
| 198 | 198 | ||
| 199 | - | const currentUserJson = data.user ? JSON.stringify(data.user) : null; | |
| 199 | + | const currentUserJson = data.user | |
| 200 | + | ? JSON.stringify({ user: data.user, sessionId: data.session?.id ?? null }) | |
| 201 | + | : null; | |
| 200 | 202 | if (currentUserJson === lastUserUpdate) { | |
| 201 | 203 | console.debug('Skipping duplicate user update'); | |
| 202 | 204 | return; | |
| @@ -196,7 +196,9 @@ | |||||
| 196 | $effect(() => { | 196 | $effect(() => { | ||
| 197 | if (!browser) return; | 197 | if (!browser) return; | ||
| 198 | 198 | ||||
| 199 | - | const currentUserJson = data.user ? JSON.stringify(data.user) : null; | 199 | + | const currentUserJson = data.user |
| 200 | + | ? JSON.stringify({ user: data.user, sessionId: data.session?.id ?? null }) | |||
| 201 | + | : null; | |||
| 200 | if (currentUserJson === lastUserUpdate) { | 202 | if (currentUserJson === lastUserUpdate) { | ||
| 201 | console.debug('Skipping duplicate user update'); | 203 | console.debug('Skipping duplicate user update'); | ||
| 202 | return; | 204 | return; | ||
M src/routes/api/session/logout/+server.ts +12 -3
| @@ -7,18 +7,27 @@ import { createLogger } from '$lib/utils/logger.server'; | |||
| 7 | 7 | const log = createLogger('logout-server'); | |
| 8 | 8 | ||
| 9 | 9 | export const POST: RequestHandler = async ({ cookies, locals }) => { | |
| 10 | + | let success = true; | |
| 10 | 11 | if (locals.session) { | |
| 11 | 12 | try { | |
| 12 | - | await users.updateStatus(locals.session.userId, 'offline'); | |
| 13 | 13 | await invalidateSession(locals.session.id); | |
| 14 | 14 | } catch (err) { | |
| 15 | - | log.error('Logout cleanup failed', { | |
| 15 | + | success = false; | |
| 16 | + | log.error('Session invalidation failed', { | |
| 17 | + | error: err instanceof Error ? err.message : 'Unknown error' | |
| 18 | + | }); | |
| 19 | + | } | |
| 20 | + | try { | |
| 21 | + | await users.updateStatus(locals.session.userId, 'offline'); | |
| 22 | + | } catch (err) { | |
| 23 | + | log.warn('Offline status update failed on logout', { | |
| 16 | 24 | error: err instanceof Error ? err.message : 'Unknown error' | |
| 17 | 25 | }); | |
| 18 | 26 | } | |
| 19 | 27 | } | |
| 20 | 28 | deleteSessionTokenCookie({ cookies }); | |
| 21 | - | return new Response(JSON.stringify({ success: true }), { | |
| 29 | + | return new Response(JSON.stringify({ success }), { | |
| 30 | + | status: success ? 200 : 500, | |
| 22 | 31 | headers: { 'Content-Type': 'application/json' } | |
| 23 | 32 | }); | |
| 24 | 33 | }; | |
| @@ -7,18 +7,27 @@ import { createLogger } from '$lib/utils/logger.server'; | |||||
| 7 | const log = createLogger('logout-server'); | 7 | const log = createLogger('logout-server'); | ||
| 8 | 8 | ||||
| 9 | export const POST: RequestHandler = async ({ cookies, locals }) => { | 9 | export const POST: RequestHandler = async ({ cookies, locals }) => { | ||
| 10 | + | let success = true; | |||
| 10 | if (locals.session) { | 11 | if (locals.session) { | ||
| 11 | try { | 12 | try { | ||
| 12 | - | await users.updateStatus(locals.session.userId, 'offline'); | |||
| 13 | await invalidateSession(locals.session.id); | 13 | await invalidateSession(locals.session.id); | ||
| 14 | } catch (err) { | 14 | } catch (err) { | ||
| 15 | - | log.error('Logout cleanup failed', { | 15 | + | success = false; |
| 16 | + | log.error('Session invalidation failed', { | |||
| 17 | + | error: err instanceof Error ? err.message : 'Unknown error' | |||
| 18 | + | }); | |||
| 19 | + | } | |||
| 20 | + | try { | |||
| 21 | + | await users.updateStatus(locals.session.userId, 'offline'); | |||
| 22 | + | } catch (err) { | |||
| 23 | + | log.warn('Offline status update failed on logout', { | |||
| 16 | error: err instanceof Error ? err.message : 'Unknown error' | 24 | error: err instanceof Error ? err.message : 'Unknown error' | ||
| 17 | }); | 25 | }); | ||
| 18 | } | 26 | } | ||
| 19 | } | 27 | } | ||
| 20 | deleteSessionTokenCookie({ cookies }); | 28 | deleteSessionTokenCookie({ cookies }); | ||
| 21 | - | return new Response(JSON.stringify({ success: true }), { | 29 | + | return new Response(JSON.stringify({ success }), { |
| 30 | + | status: success ? 200 : 500, | |||
| 22 | headers: { 'Content-Type': 'application/json' } | 31 | headers: { 'Content-Type': 'application/json' } | ||
| 23 | }); | 32 | }); | ||
| 24 | }; | 33 | }; | ||