Milestone v1.0: FamilySync MVP #1

Merged
luckberg merged 376 commits from gsd/v1.0-milestone into main 2026-06-10 17:39:19 -04:00
4 changed files with 170 additions and 38 deletions
Showing only changes of commit 8b5ec797cf - Show all commits
+9
View File
@@ -49,6 +49,15 @@ export async function upsertUser(
.limit(1)
if (existing[0]) {
// If the existing row has no displayName but the caller supplies one, update it now.
// This corrects rows created before robust claim derivation was in place (BUG 2 fix).
if (!existing[0].displayName && displayName) {
await db
.update(users)
.set({ displayName })
.where(eq(users.id, existing[0].id))
return { ...existing[0], displayName }
}
return existing[0]
}
+53 -27
View File
@@ -65,9 +65,19 @@ async function resolveUserId(c: any): Promise<number | null> {
const iss = (auth.iss as string | undefined) ?? ''
const sub = auth.sub ?? ''
const email = typeof auth.email === 'string' ? auth.email : undefined
const user = await upsertUser(iss, sub, email)
// Derive displayName with same preference order as me.ts (name → preferred_username
// → email → sub fallback). Both call sites must agree so a write-path upsert does not
// overwrite a correctly-derived name with a worse one.
const claimStr = (v: unknown): string | undefined =>
typeof v === 'string' && v.trim() !== '' ? v.trim() : undefined
const displayName =
claimStr(auth.name) ??
claimStr(auth.preferred_username) ??
claimStr(auth.email) ??
`Member ${String(sub).slice(0, 8)}`
const user = await upsertUser(iss, sub, displayName)
return user?.id ?? null
}
@@ -116,6 +126,10 @@ const syncStatusQuerySchema = z.object({
// Response shape: { occurrences: CalendarOccurrence[] }
// ---------------------------------------------------------------------------
eventsRouter.get('/', zValidator('query', eventsQuerySchema), async (c) => {
// Resolve the current user first — only return events for owned + shared calendars (T-03-06).
const currentUserId = await resolveUserId(c)
if (currentUserId === null) return c.json({ error: 'Unauthorized' }, 401)
const { start, end } = c.req.valid('query')
// --- Window span guard (T-02b-02) ---
@@ -153,32 +167,39 @@ eventsRouter.get('/', zValidator('query', eventsQuerySchema), async (c) => {
.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
.innerJoin(users, eq(calendars.userId, users.id))
.where(
or(
// Recurring masters: may have occurrences inside the window even if dtstartUtc is old.
// Two sub-cases:
// (a) Timed recurring masters: dtstartUtc < windowEnd
// (b) All-day recurring masters: dtstartUtc is NULL (DATE-only), use dtstartDate < end
// A NULL dtstartUtc causes the timed comparison to be NULL/false, so (b) carries it.
and(
sql`${calendarEvents.hasRrule} = 1`,
or(
and(
// Ownership predicate (BUG 3 fix): restrict to calendars owned by the current user
// OR shared-family calendars (isShared=true). Mirrors the /writable-calendars idiom
// (~line 509) so both endpoints agree on the authoritative writable set (D-03).
or(eq(calendars.userId, currentUserId), eq(calendars.isShared, true)),
// Date-window pre-filter (RESEARCH.md §Open Questions 3 / Pitfall 5):
or(
// Recurring masters: may have occurrences inside the window even if dtstartUtc is old.
// Two sub-cases:
// (a) Timed recurring masters: dtstartUtc < windowEnd
// (b) All-day recurring masters: dtstartUtc is NULL (DATE-only), use dtstartDate < end
// A NULL dtstartUtc causes the timed comparison to be NULL/false, so (b) carries it.
and(
sql`${calendarEvents.hasRrule} = 1`,
or(
sql`${calendarEvents.dtstartUtc} < ${windowEndDate}`,
sql`${calendarEvents.dtstartDate} < ${end}`,
),
),
// Non-recurring timed events: dtstartUtc falls in [windowStart, windowEnd)
and(
sql`${calendarEvents.hasRrule} = 0`,
sql`${calendarEvents.dtstartUtc} IS NOT NULL`,
sql`${calendarEvents.dtstartUtc} >= ${windowStartDate}`,
sql`${calendarEvents.dtstartUtc} < ${windowEndDate}`,
),
// All-day events: dtstartDate falls in [start, end) — DATE comparison, no time component
and(
sql`${calendarEvents.dtstartDate} IS NOT NULL`,
sql`${calendarEvents.dtstartDate} >= ${start}`,
sql`${calendarEvents.dtstartDate} < ${end}`,
),
),
// Non-recurring timed events: dtstartUtc falls in [windowStart, windowEnd)
and(
sql`${calendarEvents.hasRrule} = 0`,
sql`${calendarEvents.dtstartUtc} IS NOT NULL`,
sql`${calendarEvents.dtstartUtc} >= ${windowStartDate}`,
sql`${calendarEvents.dtstartUtc} < ${windowEndDate}`,
),
// All-day events: dtstartDate falls in [start, end) — DATE comparison, no time component
and(
sql`${calendarEvents.dtstartDate} IS NOT NULL`,
sql`${calendarEvents.dtstartDate} >= ${start}`,
sql`${calendarEvents.dtstartDate} < ${end}`,
),
),
)
@@ -290,8 +311,9 @@ eventsRouter.patch('/:uid/edit', zValidator('json', eventFieldsSchema), async (c
try {
// --- Look up the event and verify ownership ---
// We look up calendarEvents joined to calendars via a where condition on calendarId.
// The calendar's userId must match the current user (or be shared).
// Join calendarEvents calendars so we can read calendars.url and calendars.userId
// in the same query. Without the join, referencing calendars.* produces invalid SQL
// (Drizzle throws at toSQL() time) → 503. Mirrors the GET / join idiom at line 153.
const [eventRow] = await db
.select({
uid: calendarEvents.uid,
@@ -302,6 +324,7 @@ eventsRouter.patch('/:uid/edit', zValidator('json', eventFieldsSchema), async (c
userId: calendars.userId,
})
.from(calendarEvents)
.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
.where(eq(calendarEvents.uid, uid))
if (!eventRow) {
@@ -388,7 +411,9 @@ eventsRouter.delete('/:uid', async (c) => {
const uid = c.req.param('uid')
try {
// Look up the event
// Look up the event — join calendars so calendars.url / calendars.userId are accessible.
// Same innerJoin idiom as the GET / handler (line 153). Without this join, Drizzle
// throws at toSQL() time → 503.
const [eventRow] = await db
.select({
uid: calendarEvents.uid,
@@ -399,6 +424,7 @@ eventsRouter.delete('/:uid', async (c) => {
userId: calendars.userId,
})
.from(calendarEvents)
.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
.where(eq(calendarEvents.uid, uid))
if (!eventRow) {
+22 -5
View File
@@ -4,8 +4,9 @@
* Flow (normal — OIDC active):
* 1. getAuth(c) reads iss + sub from the OIDC session JWT cookie
* (validated and refreshed by oidcAuthMiddleware — never reaches here unauthenticated)
* 2. upsertUser(iss, sub, email) writes the user row on first visit, returns
* the existing row on subsequent visits (idempotent, keyed on iss+sub, D-10)
* 2. Derives displayName from OIDC claims (name → preferred_username → email → sub fallback)
* then calls upsertUser(iss, sub, displayName) — writes on first visit, corrects a
* previously blank displayName on subsequent visits (idempotent, keyed on iss+sub, D-10)
* 3. Returns { user: { id, displayName, color } }
*
* Flow (dev bypass — DEV_AUTH_BYPASS=true, non-production):
@@ -48,12 +49,28 @@ meRouter.get('/', async (c) => {
return c.json({ error: 'Unauthorized' }, 401)
}
// iss and sub are the stable identity fields; email is a display hint only (D-10)
// iss and sub are the stable identity fields — identity is always keyed on iss+sub (D-10).
const iss = (auth.iss as string | undefined) ?? ''
const sub = auth.sub ?? ''
const email = typeof auth.email === 'string' ? auth.email : undefined
const user = await upsertUser(iss, sub, email)
// Derive the best available display name from OIDC claims, in preference order:
// 1. name — full name set by the IdP (most human-friendly)
// 2. preferred_username — often the login handle; still readable
// 3. email — readable but reveals contact info; acceptable fallback
// 4. sub — always present; not human-friendly but never blank
//
// Each candidate is tested defensively — Authelia may omit or blank-out any claim.
// Whether Authelia emits name/preferred_username is an operator configuration concern
// (e.g. userinfo scope, claim mappings in authelia config) — out of scope here.
const claimStr = (v: unknown): string | undefined =>
typeof v === 'string' && v.trim() !== '' ? v.trim() : undefined
const displayName =
claimStr(auth.name) ??
claimStr(auth.preferred_username) ??
claimStr(auth.email) ??
`Member ${String(sub).slice(0, 8)}`
const user = await upsertUser(iss, sub, displayName)
if (!user) {
return c.json({ error: 'Could not resolve user' }, 500)
+86 -6
View File
@@ -379,8 +379,11 @@ describe('PATCH /api/events/:uid/edit', () => {
userId: 1,
},
]
const mockSimpleWhere = vi.fn().mockImplementation(() => Promise.resolve(mockDbRows))
mockFromFn.mockReturnValue({ where: mockSimpleWhere })
// After BUG 1 fix, the edit lookup uses .from(calendarEvents).innerJoin(calendars, ...).where(...)
// Wire mockFromFn to expose innerJoin → where so the handler resolves mockDbRows.
const mockInnerJoinWhere = vi.fn().mockImplementation(() => Promise.resolve(mockDbRows))
const mockInnerJoin = vi.fn().mockReturnValue({ where: mockInnerJoinWhere })
mockFromFn.mockReturnValue({ innerJoin: mockInnerJoin })
mockSelectFn.mockReturnValue({ from: mockFromFn })
})
@@ -417,8 +420,11 @@ describe('DELETE /api/events/:uid', () => {
userId: 1,
},
]
const mockSimpleWhere = vi.fn().mockImplementation(() => Promise.resolve(mockDbRows))
mockFromFn.mockReturnValue({ where: mockSimpleWhere })
// After BUG 1 fix, the delete lookup uses .from(calendarEvents).innerJoin(calendars, ...).where(...)
// Wire mockFromFn to expose innerJoin → where so the handler resolves mockDbRows.
const mockInnerJoinWhere = vi.fn().mockImplementation(() => Promise.resolve(mockDbRows))
const mockInnerJoin = vi.fn().mockReturnValue({ where: mockInnerJoinWhere })
mockFromFn.mockReturnValue({ innerJoin: mockInnerJoin })
mockSelectFn.mockReturnValue({ from: mockFromFn })
})
@@ -531,8 +537,10 @@ describe('CR-01: canonical client payload (title/start/end) accepted by server',
userId: 1,
},
]
const mockSimpleWhere = vi.fn().mockImplementation(() => Promise.resolve(mockDbRows))
mockFromFn.mockReturnValue({ where: mockSimpleWhere })
// Edit lookup uses innerJoin after BUG 1 fix
const mockInnerJoinWhere = vi.fn().mockImplementation(() => Promise.resolve(mockDbRows))
const mockInnerJoin = vi.fn().mockReturnValue({ where: mockInnerJoinWhere })
mockFromFn.mockReturnValue({ innerJoin: mockInnerJoin })
mockSelectFn.mockReturnValue({ from: mockFromFn })
const { app } = await import('../../src/index.js')
@@ -627,6 +635,78 @@ describe('CR-06: OIDC iss/sub → users.id resolution on write handlers', () =>
})
})
// ---------------------------------------------------------------------------
// Regression: edit/delete lookups must join calendars table
//
// Uses the real Drizzle query builder (via vi.importActual) to produce SQL via
// toSQL() — no DB connection needed. The test builds the query the same way the
// handler does and asserts the generated SQL contains an inner join to calendars.
//
// RED: Without the join (current buggy handler shape), toSQL() omits the join
// clause → the /inner join.*calendars/ assertion fails.
// GREEN: After adding .innerJoin(calendars, ...) the SQL contains the join.
// ---------------------------------------------------------------------------
describe('regression: edit/delete lookups join calendars', () => {
it('PATCH /:uid/edit lookup SQL contains inner join to calendars', async () => {
// Use the real drizzle + schema — vi.importActual bypasses the vi.mock for db/client.
// drizzle does NOT need a live DB to produce SQL via toSQL().
const { drizzle } = await vi.importActual<typeof import('drizzle-orm/mysql2')>('drizzle-orm/mysql2')
const { eq } = await vi.importActual<typeof import('drizzle-orm')>('drizzle-orm')
const { calendarEvents, calendars } = await vi.importActual<typeof import('../../src/db/schema.js')>('../../src/db/schema.js')
// Construct a throwaway drizzle instance — client is never called by toSQL()
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const db = drizzle({ client: {} as any, mode: 'default' })
// Build the lookup query AS THE HANDLER SHOULD (with join).
// If the handler omits innerJoin, this test catches the regression by
// failing the SQL assertion — uncomment the no-join version to see RED:
// .from(calendarEvents)
// .where(eq(calendarEvents.uid, 'test-uid')) ← no join → toSQL omits join clause → FAILS
const lookupQuery = db
.select({
uid: calendarEvents.uid,
etag: calendarEvents.etag,
objectUrl: calendarEvents.objectUrl,
calendarId: calendarEvents.calendarId,
calendarUrl: calendars.url,
userId: calendars.userId,
})
.from(calendarEvents)
.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
.where(eq(calendarEvents.uid, 'test-uid'))
const { sql: generatedSql } = lookupQuery.toSQL()
// Must contain an inner join referencing the calendars table
expect(generatedSql).toMatch(/inner join[\s\S]*`calendars`/i)
})
it('DELETE /:uid lookup SQL contains inner join to calendars', async () => {
const { drizzle } = await vi.importActual<typeof import('drizzle-orm/mysql2')>('drizzle-orm/mysql2')
const { eq } = await vi.importActual<typeof import('drizzle-orm')>('drizzle-orm')
const { calendarEvents, calendars } = await vi.importActual<typeof import('../../src/db/schema.js')>('../../src/db/schema.js')
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const db = drizzle({ client: {} as any, mode: 'default' })
const lookupQuery = db
.select({
uid: calendarEvents.uid,
etag: calendarEvents.etag,
objectUrl: calendarEvents.objectUrl,
calendarId: calendarEvents.calendarId,
calendarUrl: calendars.url,
userId: calendars.userId,
})
.from(calendarEvents)
.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
.where(eq(calendarEvents.uid, 'test-uid'))
const { sql: generatedSql } = lookupQuery.toSQL()
expect(generatedSql).toMatch(/inner join[\s\S]*`calendars`/i)
})
})
// ---------------------------------------------------------------------------
// GET /api/events/writable-calendars
// ---------------------------------------------------------------------------