diff --git a/community/src/api_v0/message/reactions.test.ts b/community/src/api_v0/message/reactions.test.ts index 6ca0587..f32a7bf 100644 --- a/community/src/api_v0/message/reactions.test.ts +++ b/community/src/api_v0/message/reactions.test.ts @@ -7,7 +7,7 @@ import { LinkedReactionUser, ReactionUsersByEmoji, } from './reactions'; -import type { DiscordMessage } from '../../lib/community/type'; +import type { CommunityMessage } from '../../lib/community/type'; const eventMessage: EventMessageRecord = { id: 'event-1', @@ -19,7 +19,7 @@ const eventMessage: EventMessageRecord = { updatedAt: '2026-07-01T00:00:00Z', }; -const discordMessage: DiscordMessage = { +const discordMessage: CommunityMessage = { id: eventMessage.messageId, channelId: eventMessage.channelId, content: eventMessage.content, @@ -27,7 +27,7 @@ const discordMessage: DiscordMessage = { author: { id: '323456789012345678', username: 'bot', - globalName: null, + displayName: null, bot: true, }, reactions: [ @@ -41,15 +41,15 @@ const reactionUsersByEmoji: ReactionUsersByEmoji[] = [ emoji: '✅', count: 2, users: [ - { id: '423456789012345678', username: 'taro', globalName: 'Taro', bot: false }, - { id: '523456789012345678', username: 'hanako', globalName: null, bot: false }, + { id: '423456789012345678', username: 'taro', displayName: 'Taro', bot: false }, + { id: '523456789012345678', username: 'hanako', displayName: null, bot: false }, ], }, { emoji: '🍱', count: 1, users: [ - { id: '423456789012345678', username: 'taro', globalName: 'Taro', bot: false }, + { id: '423456789012345678', username: 'taro', displayName: 'Taro', bot: false }, ], }, ]; @@ -98,58 +98,46 @@ test('buildMessageReactionSummary includes reaction users, linked personal infor assert.equal(summary.eventMessage.id, 'event-1'); assert.equal(summary.discordMessage.id, '223456789012345678'); assert.equal(summary.reactions.length, 2); + // A reaction badge only needs names. The private member fields must not be + // repeated here for every emoji the member reacted with. assert.deepEqual(summary.reactions[0].users[0], { discordUserId: '423456789012345678', discordUsername: 'taro', discordGlobalName: 'Taro', - userId: 'user-1', - userName: 'taro-account', - displayName: '太郎', - email: 'taro-account@example.com', - memberId: 'member-1', memberName: '山田 太郎', - memberStatus: 'active', - displayGrade: 'B2', - studentId: 'S001', - studentEmail: 'taro@example.edu', - emergencyContact: '090-0000-0000', - insurance: true, - someAllergy: false, - allergyDetails: null, - skills: ['TypeScript'], - interests: ['Robotics'], - currentActivities: 'Robot controller', - bio: 'Embedded developer', - discordNickname: 'たろう', - discordRoles: ['Member', 'Developer'], - reactions: ['✅'], + displayName: '太郎', }); assert.deepEqual(summary.reactions[0].users[1], { discordUserId: '523456789012345678', discordUsername: 'hanako', discordGlobalName: null, - userId: null, - userName: null, - displayName: null, - email: null, - memberId: null, memberName: null, - memberStatus: null, - displayGrade: null, - studentId: null, - studentEmail: null, - emergencyContact: null, - insurance: null, - someAllergy: null, - allergyDetails: null, - skills: [], - interests: [], - currentActivities: null, - bio: null, - discordNickname: null, - discordRoles: [], - reactions: ['✅'], + displayName: null, }); assert.deepEqual(summary.members.find(member => member.discordUserId === '423456789012345678')?.reactions, ['✅', '🍱']); assert.equal(summary.members.length, 2); + + // The member list stays complete: it is what the admin table and its CSV + // export read, including the private fields the organiser needs. + const taro = summary.members.find(member => member.discordUserId === '423456789012345678'); + assert.equal(taro?.emergencyContact, '090-0000-0000'); + assert.equal(taro?.studentId, 'S001'); +}); + +test('private member fields are never repeated inside the reaction badges', () => { + const summary = buildMessageReactionSummary( + eventMessage, + discordMessage, + reactionUsersByEmoji, + linkedUsers, + ); + + const serialisedBadges = JSON.stringify(summary.reactions); + for (const secret of ['090-0000-0000', 'S001', 'taro@example.edu', 'taro-account@example.com']) { + assert.equal( + serialisedBadges.includes(secret), + false, + `reaction badges must not carry ${secret}`, + ); + } }); diff --git a/community/src/api_v0/message/reactions.ts b/community/src/api_v0/message/reactions.ts index 588f68e..34f5707 100644 --- a/community/src/api_v0/message/reactions.ts +++ b/community/src/api_v0/message/reactions.ts @@ -1,4 +1,4 @@ -import type { DiscordMessage, DiscordReactionUser } from "../../lib/community/type"; +import type { CommunityMessage, CommunityReactionUser } from "../../lib/community/type"; import type { MemberStatus } from "../../../../share/drizzle/schema"; export type EventMessageRecord = { @@ -38,7 +38,18 @@ export type LinkedReactionUser = { export type ReactionUsersByEmoji = { emoji: string; count: number; - users: DiscordReactionUser[]; + users: CommunityReactionUser[]; +}; + +// What a reaction badge renders. The full record — including the private +// member fields — is returned once per member in `members`, so it is not +// repeated here for every emoji the same member reacted with. +export type ReactionParticipant = { + discordUserId: string; + discordUsername: string; + discordGlobalName: string | null; + memberName: string | null; + displayName: string | null; }; export type ReactionMember = { @@ -76,7 +87,7 @@ export const collectDiscordUserIds = (reactionUsersByEmoji: ReactionUsersByEmoji export const buildMessageReactionSummary = ( eventMessage: EventMessageRecord, - discordMessage: DiscordMessage, + discordMessage: CommunityMessage, reactionUsersByEmoji: ReactionUsersByEmoji[], linkedUsers: LinkedReactionUser[], ) => { @@ -91,7 +102,7 @@ export const buildMessageReactionSummary = ( const reactionMember: ReactionMember = { discordUserId: discordUser.id, discordUsername: discordUser.username, - discordGlobalName: discordUser.globalName, + discordGlobalName: discordUser.displayName, userId: linkedUser?.userId ?? null, userName: linkedUser?.userName ?? null, displayName: linkedUser?.displayName ?? null, @@ -125,10 +136,14 @@ export const buildMessageReactionSummary = ( }); } - return { - ...reactionMember, - reactions: [...reactionMember.reactions], + const participant: ReactionParticipant = { + discordUserId: reactionMember.discordUserId, + discordUsername: reactionMember.discordUsername, + discordGlobalName: reactionMember.discordGlobalName, + memberName: reactionMember.memberName, + displayName: reactionMember.displayName, }; + return participant; }); return { diff --git a/community/src/api_v0/message/schema.ts b/community/src/api_v0/message/schema.ts index f9f9e26..558465a 100644 --- a/community/src/api_v0/message/schema.ts +++ b/community/src/api_v0/message/schema.ts @@ -66,12 +66,23 @@ export const reactionMemberSchema = z.object({ reactions: z.array(z.string()), }).openapi("ReactionMember") +// A reaction badge renders names only. Keeping this separate from +// reactionMemberSchema stops the private member fields from being serialised +// once per member per emoji; they are sent once in `members` instead. +export const reactionParticipantSchema = z.object({ + discordUserId: discordSnowflakeSchema, + discordUsername: z.string(), + discordGlobalName: z.string().nullable(), + memberName: z.string().nullable(), + displayName: z.string().nullable(), +}).openapi("ReactionParticipant") + export const messageReactionSummarySchema = z.object({ eventMessage: eventMessageSchema, reactions: z.array(z.object({ emoji: z.string(), count: z.number(), - users: z.array(reactionMemberSchema), + users: z.array(reactionParticipantSchema), })), members: z.array(reactionMemberSchema), }).openapi("MessageReactionSummary") diff --git a/community/src/api_v0/message/service.ts b/community/src/api_v0/message/service.ts index a4e8d0f..9520bca 100644 --- a/community/src/api_v0/message/service.ts +++ b/community/src/api_v0/message/service.ts @@ -8,6 +8,7 @@ import { listMessagesRoute } from "./schema" import { + appAccounts, communityIdentities, communityMemberships, eventMessages, @@ -141,14 +142,15 @@ export const getMessageReactionsService: RouteHandler 0 + // Everything the domain knows about the members who reacted. The + // authentication subject is already recorded on the identity row, so + // this query stays inside the domain schema. + const domainRows = discordUserIds.length > 0 ? await db.transaction((tx) => tx .select({ discordUserId: communityIdentities.providerAccountId, - userId: authUsers.id, - userName: authUsers.name, - email: authUsers.email, - memberId: authUsers.memberId, + userId: communityIdentities.userId, + memberId: appAccounts.memberId, memberName: members.name, memberStatus: members.memberStatus, displayName: memberDirectoryProfiles.displayName, @@ -167,8 +169,8 @@ export const getMessageReactionsService: RouteHandler row.userId); + const subjects = subjectIds.length > 0 + ? await db.transaction((tx) => tx + .select({ id: authUsers.id, name: authUsers.name, email: authUsers.email }) + .from(authUsers) + .where(inArray(authUsers.id, subjectIds))) + : []; + const subjectById = new Map(subjects.map((subject) => [subject.id, subject])); + + const linkedUsers: LinkedReactionUser[] = domainRows.map((row) => ({ + ...row, + userName: subjectById.get(row.userId)?.name ?? "", + email: subjectById.get(row.userId)?.email ?? "", + })); + const summary = buildMessageReactionSummary( eventMessage, discordMessage, diff --git a/community/src/api_v0/user/me/identity/service.test.ts b/community/src/api_v0/user/me/identity/service.test.ts index 07b6e14..defc067 100644 --- a/community/src/api_v0/user/me/identity/service.test.ts +++ b/community/src/api_v0/user/me/identity/service.test.ts @@ -17,7 +17,7 @@ const linkedAccount = { const oauthUser = { id: linkedAccount.accountId, username: 'test-user', - globalName: 'Test User', + displayName: 'Test User', avatarUrl: null, }; const guildMembership = { @@ -30,7 +30,7 @@ const persisted = { provider: 'discord' as const, providerAccountId: linkedAccount.accountId, username: oauthUser.username, - providerDisplayName: oauthUser.globalName, + providerDisplayName: oauthUser.displayName, avatarUrl: null, oauthVerifiedAt: fixedNow, membership: { diff --git a/community/src/api_v0/user/me/identity/service.ts b/community/src/api_v0/user/me/identity/service.ts index 57d85d6..6ff57fe 100644 --- a/community/src/api_v0/user/me/identity/service.ts +++ b/community/src/api_v0/user/me/identity/service.ts @@ -11,8 +11,8 @@ import { getAuth } from '../../../../auth/better-auth'; import { CommunityProviderError } from '../../../../lib/community/error'; import { getCurrentDiscordUser } from '../../../../lib/community/discord/oauth'; import type { - DiscordGuildMembership, - DiscordOAuthUser, + CommunityAccountProfile, + CommunityMembership, } from '../../../../lib/community/type'; import { verifyDiscordIdentityRoute, @@ -27,9 +27,9 @@ type LinkedDiscordAccount = { type PersistVerificationInput = { appUserId: string; linkedAccount: LinkedDiscordAccount; - oauthUser: DiscordOAuthUser; + oauthUser: CommunityAccountProfile; guildId: string; - guildMembership: DiscordGuildMembership | null; + guildMembership: CommunityMembership | null; verifiedAt: string; }; @@ -43,7 +43,7 @@ type IdentityAuthApi = { type VerificationDependencies = { getAuthApi(c: Context): IdentityAuthApi; - getCurrentDiscordUser(accessToken: string): Promise; + getCurrentDiscordUser(accessToken: string): Promise; findLinkedDiscordAccounts( c: Context, userId: string, @@ -80,7 +80,7 @@ const persistVerification: VerificationDependencies['persistVerification'] = asy provider: 'discord', providerAccountId: input.oauthUser.id, username: input.oauthUser.username, - providerDisplayName: input.oauthUser.globalName, + providerDisplayName: input.oauthUser.displayName, avatarUrl: input.oauthUser.avatarUrl, oauthVerifiedAt: input.verifiedAt, lastSyncedAt: input.verifiedAt, @@ -92,7 +92,7 @@ const persistVerification: VerificationDependencies['persistVerification'] = asy authAccountId: input.linkedAccount.id, providerAccountId: input.oauthUser.id, username: input.oauthUser.username, - providerDisplayName: input.oauthUser.globalName, + providerDisplayName: input.oauthUser.displayName, avatarUrl: input.oauthUser.avatarUrl, oauthVerifiedAt: input.verifiedAt, lastSyncedAt: input.verifiedAt, @@ -222,7 +222,7 @@ export const createVerifyDiscordIdentityService = ( const [linkedAccount] = linkedAccounts; let accessToken: string; - let oauthUser: DiscordOAuthUser; + let oauthUser: CommunityAccountProfile; try { ({ accessToken } = await authApi.getAccessToken({ diff --git a/community/src/api_v0/user/me/service.ts b/community/src/api_v0/user/me/service.ts index ea4ba2d..7945bf0 100644 --- a/community/src/api_v0/user/me/service.ts +++ b/community/src/api_v0/user/me/service.ts @@ -5,14 +5,15 @@ import { getUserMeRoute, updateUserMeRoute } from "./schema"; import { eq } from "drizzle-orm"; import { user } from "../../../../../share/drizzle/schema"; +// The authentication store owns these. Membership and role are domain facts and +// come from the request's already-resolved account, so this service never +// reaches across the boundary. const currentUserSelection = { id: user.id, name: user.name, email: user.email, emailVerified: user.emailVerified, image: user.image, - memberId: user.memberId, - role: user.role, }; const setPrivateNoStore = (c: Context) => { @@ -36,7 +37,7 @@ export const getUserMeService: RouteHandler = return c.json({ error: "User not found in database" }, 404); } - return c.json(dbUser, 200); + return c.json({ ...dbUser, memberId: appUser.memberId, role: appUser.role }, 200); }; export const updateUserMeService: RouteHandler = async (c) => { @@ -58,5 +59,5 @@ export const updateUserMeService: RouteHandler { diff --git a/community/src/auth/better-auth.ts b/community/src/auth/better-auth.ts index 66af1c1..0f7e8a9 100644 --- a/community/src/auth/better-auth.ts +++ b/community/src/auth/better-auth.ts @@ -21,11 +21,24 @@ export const getAuth = (c: Context, database?: AppDatabase) => { trustedOrigins: [ c.env.FRONTEND_URL || "http://localhost:3000" ], - user: { - additionalFields: { - memberId: { type: "string", required: false, input: false }, - role: { type: "string", defaultValue: "user", input: false } - } + // No additionalFields: membership and role belong to the domain and live + // in public.app_accounts, so Better Auth owns its tables outright and + // they can be moved to their own database. + databaseHooks: { + user: { + create: { + // A subject only becomes usable once the domain has its own + // record of it. Creating it here keeps the write in the + // boundary layer instead of a trigger that would make the + // authentication schema depend on the domain again. + after: async (createdUser) => { + await db + .insert(schema.appAccounts) + .values({ userId: createdUser.id }) + .onConflictDoNothing(); + }, + }, + }, }, account: { encryptOAuthTokens: true, @@ -96,18 +109,30 @@ export const getJwtHandler = async (c: Context) => { } try { + // Better Auth proved who the caller is; the domain decides what the JWT + // may claim, so role and membership are read from app_accounts. const authDb = getAuthDatabase(c); + + // The create hook normally provisions this row. Repairing it here costs + // one statement per sign-in and keeps a subject that slipped through — + // a hook that failed, a row seeded outside the flow — from being locked + // out permanently with no path back except editing the database. + await authDb + .insert(schema.appAccounts) + .values({ userId: session.user.id }) + .onConflictDoNothing(); + const [currentUser] = await authDb .select({ - role: schema.user.role, - memberId: schema.user.memberId, + role: schema.appAccounts.role, + memberId: schema.appAccounts.memberId, }) - .from(schema.user) - .where(eq(schema.user.id, session.user.id)) + .from(schema.appAccounts) + .where(eq(schema.appAccounts.userId, session.user.id)) .limit(1); if (!currentUser) { - console.error("[JWT Endpoint] Authenticated user was not found in the application database."); + console.error("[JWT Endpoint] Authenticated user has no application account."); return c.redirect(`${frontendUrl}/login?error=user_not_found`); } diff --git a/community/src/core/auth.test.ts b/community/src/core/auth.test.ts index 6675c49..e18c8c1 100644 --- a/community/src/core/auth.test.ts +++ b/community/src/core/auth.test.ts @@ -9,21 +9,18 @@ test('auth middleware reloads the application user and does not turn downstream let capturedUser: appUser | undefined; let capturedIdentity: unknown; + // The middleware asks the authentication store who the caller is, then asks + // the domain what that subject may do. Two selects, never a join across them. + let selectCall = 0; const transaction = { - select: () => ({ - from: () => ({ - innerJoin: () => ({ - where: () => ({ - limit: async () => [{ - id: 'user-1', - name: 'Test User', - memberId: null, - role: 'user', - }], - }), - }), - }), - }), + select: () => { + selectCall += 1; + const rows = selectCall === 1 + ? [{ id: 'user-1', name: 'Test User' }] + : [{ memberId: null, role: 'user' }]; + const where = () => ({ limit: async () => rows }); + return { from: () => ({ innerJoin: () => ({ where }), where }) }; + }, }; const db = { transaction: (operation: (tx: unknown) => Promise) => operation(transaction), diff --git a/community/src/core/auth.ts b/community/src/core/auth.ts index a373d2e..38b8f6e 100644 --- a/community/src/core/auth.ts +++ b/community/src/core/auth.ts @@ -2,7 +2,7 @@ import type { Context, Next } from 'hono' import { AppContext } from './types' import { verify } from 'hono/jwt' import { and, eq, gt } from 'drizzle-orm' -import { session, user } from '../../../share/drizzle/schema' +import { appAccounts, session, user } from '../../../share/drizzle/schema' export type authUser = { id: string @@ -15,52 +15,74 @@ export type appUser = { role: 'admin' | 'user' } -const userSelection = { +// Who the caller is, according to the authentication store. +const subjectSelection = { id: user.id, name: user.name, - memberId: user.memberId, - role: user.role, } +// What the caller may do, according to the domain. +const accountSelection = { + memberId: appAccounts.memberId, + role: appAccounts.role, +} + +// This is the boundary between the authentication store and the domain, and the +// only place that reads both. The two lookups stay separate rather than joining +// across app_auth and public, so moving the authentication store to its own +// database turns the first one into a remote call and leaves the second alone. const loadAppUser = async ( c: Context, userId: string, sessionId?: string, ): Promise => { - const [currentUser] = await c.get('db').transaction(async (db) => sessionId - ? db - .select(userSelection) - .from(user) - .innerJoin(session, eq(session.userId, user.id)) - .where(and( - eq(user.id, userId), - eq(session.id, sessionId), - gt(session.expiresAt, new Date()), - )) - .limit(1) - : db - .select(userSelection) - .from(user) - .where(eq(user.id, userId)) - .limit(1)); + const resolved = await c.get('db').transaction(async (db) => { + const [subject] = sessionId + ? await db + .select(subjectSelection) + .from(user) + .innerJoin(session, eq(session.userId, user.id)) + .where(and( + eq(user.id, userId), + eq(session.id, sessionId), + gt(session.expiresAt, new Date()), + )) + .limit(1) + : await db + .select(subjectSelection) + .from(user) + .where(eq(user.id, userId)) + .limit(1); - if (!currentUser) { + if (!subject) return null; + + const [account] = await db + .select(accountSelection) + .from(appAccounts) + .where(eq(appAccounts.userId, subject.id)) + .limit(1); + + if (!account) return null; + + return { + id: subject.id, + name: subject.name, + memberId: account.memberId, + role: account.role === 'admin' ? 'admin' as const : 'user' as const, + }; + }); + + if (!resolved) { return null; } - const role = currentUser.role === 'admin' ? 'admin' : 'user'; c.get('db').setIdentity({ - userId: currentUser.id, - memberId: currentUser.memberId, - role, + userId: resolved.id, + memberId: resolved.memberId, + role: resolved.role, }); - return { - id: currentUser.id, - name: currentUser.name, - memberId: currentUser.memberId, - role, - }; + return resolved; } export const authMiddleware = async (c: Context, next: Next) => { diff --git a/community/src/lib/community/discord/main.ts b/community/src/lib/community/discord/main.ts index ba894ad..9d0806e 100644 --- a/community/src/lib/community/discord/main.ts +++ b/community/src/lib/community/discord/main.ts @@ -1,5 +1,5 @@ import type { CommunityProvider } from '../interface'; -import type { DiscordGuildMembership, DiscordMessage, DiscordReactionUser, Role, SendMessageInput, SendMessageResult } from '../type'; +import type { CommunityMembership, CommunityMessage, CommunityReactionUser, CommunityRole, SendMessageInput, SendMessageResult } from '../type'; import { CommunityProviderError } from '../error'; import { getGuildMembershipAPI, listUserRolesAPI } from './role'; import { getMessageAPI, listMessageReactionUsersAPI, sendMessageAPI } from './message'; @@ -31,11 +31,11 @@ export class DiscordProvider implements CommunityProvider { return response.json(); } - async listUserRoles(userId: string): Promise { + async listUserRoles(userId: string): Promise { return listUserRolesAPI(this, userId); } - async getGuildMembership(userId: string): Promise { + async getGuildMembership(userId: string): Promise { return getGuildMembershipAPI(this, userId); } @@ -43,11 +43,11 @@ export class DiscordProvider implements CommunityProvider { return sendMessageAPI(this, input); } - async getMessage(channelId: string, messageId: string): Promise { + async getMessage(channelId: string, messageId: string): Promise { return getMessageAPI(this, channelId, messageId); } - async listMessageReactionUsers(channelId: string, messageId: string, emoji: string): Promise { + async listMessageReactionUsers(channelId: string, messageId: string, emoji: string): Promise { return listMessageReactionUsersAPI(this, channelId, messageId, emoji); } } diff --git a/community/src/lib/community/discord/message.test.ts b/community/src/lib/community/discord/message.test.ts index 4e5aa9a..0a8cd30 100644 --- a/community/src/lib/community/discord/message.test.ts +++ b/community/src/lib/community/discord/message.test.ts @@ -119,7 +119,7 @@ test('listMessageReactionUsersAPI fetches and parses users who reacted to a mess assert.deepEqual(users, [{ id: '623456789012345678', username: 'taro', - globalName: 'Taro', + displayName: 'Taro', bot: false, }]); }); diff --git a/community/src/lib/community/discord/message.ts b/community/src/lib/community/discord/message.ts index d1c6f5a..d831c19 100644 --- a/community/src/lib/community/discord/message.ts +++ b/community/src/lib/community/discord/message.ts @@ -1,29 +1,32 @@ import type { DiscordProvider } from './main'; import { CommunityProviderError } from '../error'; +import type { + CommunityMessage, + CommunityReactionUser, + SendMessageInput, + SendMessageResult +} from '../type'; +import { SendMessageResultSchema } from '../type'; import { - DiscordMessage, + DISCORD_MESSAGE_CONTENT_LIMIT, DiscordMessageSchema, - DiscordReactionUser, DiscordReactionUserSchema, - SendMessageInput, - SendMessageInputSchema, - SendMessageResult, - SendMessageResultSchema -} from '../type'; + DiscordSendMessageInputSchema +} from './schema'; // チャンネルへメッセージを送信し、送信したメッセージIDを返す export async function sendMessageAPI( provider: DiscordProvider, input: SendMessageInput ): Promise { - const parsedInput = SendMessageInputSchema.parse(input); + const parsedInput = DiscordSendMessageInputSchema.parse(input); const roleIds = parsedInput.mentionRoleIds ?? []; // メンション対象のロールをメッセージ本文に埋め込む（<@&ロールID>） const mentions = roleIds.map((id) => `<@&${id}>`).join(' '); const content = mentions ? `${mentions} ${parsedInput.content}` : parsedInput.content; - if (content.length > 2000) { + if (content.length > DISCORD_MESSAGE_CONTENT_LIMIT) { throw new CommunityProviderError('Discord message content is too long', 400, 'discord'); } @@ -46,7 +49,7 @@ export async function getMessageAPI( provider: DiscordProvider, channelId: string, messageId: string -): Promise { +): Promise { const message = await provider.request( 'GET', `/channels/${channelId}/messages/${messageId}` @@ -60,7 +63,7 @@ export async function getMessageAPI( author: { id: message.author.id, username: message.author.username, - globalName: message.author.global_name ?? null, + displayName: message.author.global_name ?? null, bot: message.author.bot ?? false, }, reactions: (message.reactions ?? []).map((reaction: any) => ({ @@ -75,8 +78,8 @@ export async function listMessageReactionUsersAPI( channelId: string, messageId: string, emoji: string -): Promise { - const users: DiscordReactionUser[] = []; +): Promise { + const users: CommunityReactionUser[] = []; let after: string | undefined; while (true) { @@ -92,7 +95,7 @@ export async function listMessageReactionUsersAPI( DiscordReactionUserSchema.parse({ id: user.id, username: user.username, - globalName: user.global_name ?? null, + displayName: user.global_name ?? null, bot: user.bot ?? false, }) ); diff --git a/community/src/lib/community/discord/oauth.test.ts b/community/src/lib/community/discord/oauth.test.ts index 28070ab..2a7fe75 100644 --- a/community/src/lib/community/discord/oauth.test.ts +++ b/community/src/lib/community/discord/oauth.test.ts @@ -26,7 +26,7 @@ test('getCurrentDiscordUser verifies the bearer token and maps the Discord profi assert.deepEqual(user, { id: '123456789012345678', username: 'club-member', - globalName: 'Club Member', + displayName: 'Club Member', avatarUrl: 'https://cdn.discordapp.com/avatars/123456789012345678/avatar-hash.png', }); }); diff --git a/community/src/lib/community/discord/oauth.ts b/community/src/lib/community/discord/oauth.ts index b920937..d97f81a 100644 --- a/community/src/lib/community/discord/oauth.ts +++ b/community/src/lib/community/discord/oauth.ts @@ -1,5 +1,6 @@ import { CommunityProviderError } from '../error'; -import { DiscordOAuthUser, DiscordOAuthUserSchema } from '../type'; +import type { CommunityAccountProfile } from '../type'; +import { DiscordAccountProfileSchema } from './schema'; const DISCORD_API_BASE = 'https://discord.com/api/v10'; @@ -22,7 +23,7 @@ const buildDiscordAvatarUrl = (profile: DiscordOAuthProfile): string | null => { export const getCurrentDiscordUser = async ( accessToken: string, fetcher: typeof fetch = fetch, -): Promise => { +): Promise => { const response = await fetcher(`${DISCORD_API_BASE}/users/@me`, { headers: { Authorization: `Bearer ${accessToken}`, @@ -41,10 +42,10 @@ export const getCurrentDiscordUser = async ( } const profile = await response.json(); - const parsed = DiscordOAuthUserSchema.safeParse({ + const parsed = DiscordAccountProfileSchema.safeParse({ id: profile.id, username: profile.username, - globalName: profile.global_name ?? null, + displayName: profile.global_name ?? null, avatarUrl: buildDiscordAvatarUrl(profile), }); diff --git a/community/src/lib/community/discord/role.ts b/community/src/lib/community/discord/role.ts index 27894a5..acfbcde 100644 --- a/community/src/lib/community/discord/role.ts +++ b/community/src/lib/community/discord/role.ts @@ -1,11 +1,10 @@ import type { DiscordProvider } from './main'; +import type { CommunityMembership, CommunityRole } from '../type'; import { - DiscordGuildMembership, - DiscordGuildMembershipSchema, + DiscordMembershipSchema, + DiscordRoleSchema, DiscordSnowflakeSchema, - Role, - RoleSchema, -} from '../type'; +} from './schema'; import { CommunityProviderError } from '../error'; type DiscordGuildMemberResponse = { @@ -22,7 +21,7 @@ type DiscordRoleResponse = { export async function getGuildMembershipAPI( provider: DiscordProvider, userId: string, -): Promise { +): Promise { const expectedUserId = DiscordSnowflakeSchema.parse(userId); const member = await provider.request( 'GET', @@ -45,12 +44,12 @@ export async function getGuildMembershipAPI( ); const roleMap = new Map( allRoles.map((role) => { - const parsed = RoleSchema.parse(role); + const parsed = DiscordRoleSchema.parse(role); return [parsed.id, parsed] as const; }), ); - return DiscordGuildMembershipSchema.parse({ + return DiscordMembershipSchema.parse({ userId: expectedUserId, nickname: member.nick ?? null, roles: memberRoleIds.flatMap((roleId) => { @@ -60,6 +59,6 @@ export async function getGuildMembershipAPI( }); } -export async function listUserRolesAPI(provider: DiscordProvider, userId: string): Promise { +export async function listUserRolesAPI(provider: DiscordProvider, userId: string): Promise { return (await getGuildMembershipAPI(provider, userId)).roles; } diff --git a/community/src/lib/community/discord/schema.ts b/community/src/lib/community/discord/schema.ts new file mode 100644 index 0000000..95da60c --- /dev/null +++ b/community/src/lib/community/discord/schema.ts @@ -0,0 +1,50 @@ +import { z } from 'zod'; +import { + CommunityAccountProfileSchema, + CommunityMembershipSchema, + CommunityMessageAuthorSchema, + CommunityMessageSchema, + CommunityReactionUserSchema, + CommunityRoleSchema, + SendMessageInputSchema, +} from '../type'; + +// Discord refinements of the neutral port types. Only this adapter knows that +// Discord identifiers are snowflakes, so the format is asserted here rather +// than in the port every other provider would also have to satisfy. +export const DiscordSnowflakeSchema = z.string().regex(/^\d{17,20}$/); + +export const DiscordRoleSchema = CommunityRoleSchema.extend({ + id: DiscordSnowflakeSchema, +}); + +export const DiscordAccountProfileSchema = CommunityAccountProfileSchema.extend({ + id: DiscordSnowflakeSchema, +}); + +export const DiscordMembershipSchema = CommunityMembershipSchema.extend({ + userId: DiscordSnowflakeSchema, + roles: z.array(DiscordRoleSchema), +}); + +export const DiscordSendMessageInputSchema = SendMessageInputSchema.extend({ + channelId: DiscordSnowflakeSchema, + mentionRoleIds: z.array(DiscordSnowflakeSchema).max(100).optional(), +}); + +export const DiscordMessageAuthorSchema = CommunityMessageAuthorSchema.extend({ + id: DiscordSnowflakeSchema, +}); + +export const DiscordMessageSchema = CommunityMessageSchema.extend({ + id: DiscordSnowflakeSchema, + channelId: DiscordSnowflakeSchema, + author: DiscordMessageAuthorSchema, +}); + +export const DiscordReactionUserSchema = CommunityReactionUserSchema.extend({ + id: DiscordSnowflakeSchema, +}); + +// Discord rejects a message body longer than this once mentions are prepended. +export const DISCORD_MESSAGE_CONTENT_LIMIT = 2000; diff --git a/community/src/lib/community/interface.ts b/community/src/lib/community/interface.ts index ab4c7e5..8e1f0b3 100644 --- a/community/src/lib/community/interface.ts +++ b/community/src/lib/community/interface.ts @@ -1,24 +1,26 @@ -import { z } from 'zod'; import { - DiscordGuildMembership, - DiscordMessage, - DiscordReactionUser, - Role, + CommunityMembership, + CommunityMessage, + CommunityReactionUser, + CommunityRole, SendMessageInput, SendMessageResult } from './type'; // --- Interface Definition --- +// The port every community provider implements. It speaks only the neutral +// types from ./type, so replacing the Discord adapter does not reach into the +// callers that depend on this interface. export interface CommunityProvider { // role - listUserRoles(userId: string): Promise; - getGuildMembership(userId: string): Promise; + listUserRoles(userId: string): Promise; + getGuildMembership(userId: string): Promise; // message sendMessage(input: SendMessageInput): Promise; - getMessage(channelId: string, messageId: string): Promise; - listMessageReactionUsers(channelId: string, messageId: string, emoji: string): Promise; + getMessage(channelId: string, messageId: string): Promise; + listMessageReactionUsers(channelId: string, messageId: string, emoji: string): Promise; } diff --git a/community/src/lib/community/type.ts b/community/src/lib/community/type.ts index 159f68b..e68e2e2 100644 --- a/community/src/lib/community/type.ts +++ b/community/src/lib/community/type.ts @@ -1,28 +1,38 @@ import { z } from 'zod'; -export const DiscordSnowflakeSchema = z.string().regex(/^\d{17,20}$/); +// This module is the community port's vocabulary. It must stay free of any one +// provider's shapes so a provider can be replaced without touching callers. +// Provider-specific formats and limits belong in that provider's adapter — see +// discord/schema.ts for the Discord refinements of these types. + +// Identifiers are opaque here. Discord issues snowflakes, another provider will +// not, so only the adapter that issues an ID may constrain its format. +export const CommunityIdSchema = z.string().min(1); // --- role schemas --- -export const RoleSchema = z.object({ - id: DiscordSnowflakeSchema, +export const CommunityRoleSchema = z.object({ + id: CommunityIdSchema, name: z.string(), }); -export type Role = z.infer; +export type CommunityRole = z.infer; -export const DiscordOAuthUserSchema = z.object({ - id: DiscordSnowflakeSchema, +// The profile of a linked community account, as returned by the provider's +// account authorization flow. +export const CommunityAccountProfileSchema = z.object({ + id: CommunityIdSchema, username: z.string().min(1), - globalName: z.string().nullable(), + displayName: z.string().nullable(), avatarUrl: z.string().url().nullable(), }); -export type DiscordOAuthUser = z.infer; +export type CommunityAccountProfile = z.infer; -export const DiscordGuildMembershipSchema = z.object({ - userId: DiscordSnowflakeSchema, +// Evidence that an account belongs to a specific community. +export const CommunityMembershipSchema = z.object({ + userId: CommunityIdSchema, nickname: z.string().nullable(), - roles: z.array(RoleSchema), + roles: z.array(CommunityRoleSchema), }); -export type DiscordGuildMembership = z.infer; +export type CommunityMembership = z.infer; export const GetUserRolesInputSchema = z.object({ userId: z.string(), @@ -32,10 +42,12 @@ export type GetUserRolesInput = z.infer; // --- message schemas --- // イベント通知メッセージの送信に使う入出力定義 export const SendMessageInputSchema = z.object({ - channelId: DiscordSnowflakeSchema, - content: z.string().min(1).max(2000), + channelId: CommunityIdSchema, + // Per-provider length limits are enforced by the adapter, which is the only + // layer that knows what its API accepts. + content: z.string().min(1), // メンションするロールID一覧（省略可） - mentionRoleIds: z.array(DiscordSnowflakeSchema).max(100).optional(), + mentionRoleIds: z.array(CommunityIdSchema).max(100).optional(), }); export type SendMessageInput = z.infer; @@ -44,34 +56,34 @@ export const SendMessageResultSchema = z.object({ }); export type SendMessageResult = z.infer; -export const DiscordMessageAuthorSchema = z.object({ - id: DiscordSnowflakeSchema, +export const CommunityMessageAuthorSchema = z.object({ + id: CommunityIdSchema, username: z.string(), - globalName: z.string().nullable(), + displayName: z.string().nullable(), bot: z.boolean(), }); -export type DiscordMessageAuthor = z.infer; +export type CommunityMessageAuthor = z.infer; -export const DiscordMessageReactionSchema = z.object({ +export const CommunityMessageReactionSchema = z.object({ emoji: z.string(), count: z.number(), }); -export type DiscordMessageReaction = z.infer; +export type CommunityMessageReaction = z.infer; -export const DiscordMessageSchema = z.object({ - id: DiscordSnowflakeSchema, - channelId: DiscordSnowflakeSchema, +export const CommunityMessageSchema = z.object({ + id: CommunityIdSchema, + channelId: CommunityIdSchema, content: z.string(), createdAt: z.string(), - author: DiscordMessageAuthorSchema, - reactions: z.array(DiscordMessageReactionSchema), + author: CommunityMessageAuthorSchema, + reactions: z.array(CommunityMessageReactionSchema), }); -export type DiscordMessage = z.infer; +export type CommunityMessage = z.infer; -export const DiscordReactionUserSchema = z.object({ - id: DiscordSnowflakeSchema, +export const CommunityReactionUserSchema = z.object({ + id: CommunityIdSchema, username: z.string(), - globalName: z.string().nullable(), + displayName: z.string().nullable(), bot: z.boolean(), }); -export type DiscordReactionUser = z.infer; +export type CommunityReactionUser = z.infer; diff --git a/frontend/src/app/event/[id]/detailUtils.test.ts b/frontend/src/app/event/[id]/detailUtils.test.ts index 4c46d23..8080468 100644 --- a/frontend/src/app/event/[id]/detailUtils.test.ts +++ b/frontend/src/app/event/[id]/detailUtils.test.ts @@ -16,58 +16,21 @@ const summary: EventReactionSummary = { { emoji: '✅', count: 2, + // Badges carry names only; the private fields arrive once in `members`. users: [ { discordUserId: '323456789012345678', discordUsername: 'taro', discordGlobalName: 'Taro', - userId: 'user-1', - userName: 'taro-account', - displayName: '太郎', - email: 'account@example.com', - memberId: 'member-1', memberName: '山田 太郎', - memberStatus: 'active', - displayGrade: 'B2', - studentId: 'S001', - studentEmail: 'taro@example.edu', - emergencyContact: '090-0000-0000', - insurance: true, - someAllergy: false, - allergyDetails: null, - skills: ['TypeScript'], - interests: ['Robotics'], - currentActivities: 'Robot controller', - bio: 'Embedded developer', - discordNickname: 'たろう', - discordRoles: ['Member', 'Developer'], - reactions: ['✅'], + displayName: '太郎', }, { discordUserId: '423456789012345678', discordUsername: 'hanako', discordGlobalName: null, - userId: null, - userName: null, - displayName: null, - email: null, - memberId: null, memberName: null, - memberStatus: null, - displayGrade: null, - studentId: null, - studentEmail: null, - emergencyContact: null, - insurance: null, - someAllergy: null, - allergyDetails: null, - skills: [], - interests: [], - currentActivities: null, - bio: null, - discordNickname: null, - discordRoles: [], - reactions: ['✅'], + displayName: null, }, ], }, diff --git a/frontend/src/app/event/[id]/detailUtils.ts b/frontend/src/app/event/[id]/detailUtils.ts index abe15a4..4007162 100644 --- a/frontend/src/app/event/[id]/detailUtils.ts +++ b/frontend/src/app/event/[id]/detailUtils.ts @@ -13,11 +13,21 @@ export type EventReactionSummary = { reactions: Array<{ emoji: string; count: number; - users: Array; + users: Array; }>; members: Array; }; +// A reaction badge only shows names, so the API sends just these. The private +// member fields arrive once per member in `members`. +export type ReactionParticipant = { + discordUserId: string; + discordUsername: string; + discordGlobalName: string | null; + memberName: string | null; + displayName: string | null; +}; + export type ReactionSummaryMember = { discordUserId: string; discordUsername: string; diff --git a/member/src/api_v0/member/join.integration.test.ts b/member/src/api_v0/member/join.integration.test.ts index adec341..1124a4e 100644 --- a/member/src/api_v0/member/join.integration.test.ts +++ b/member/src/api_v0/member/join.integration.test.ts @@ -152,23 +152,31 @@ const setupWorkflowDatabase = async () => { create unique index members_student_id_unique on members(student_id); create unique index members_student_email_unique on members(student_email); - create table "user" ( + create schema app_auth; + create table app_auth."user" ( id text primary key, name text not null, email text not null unique, email_verified boolean not null, image text, created_at timestamp not null, - updated_at timestamp not null, + updated_at timestamp not null + ); + + -- Domain-side account record; user_id is a value, not a foreign key. + create table app_accounts ( + user_id text primary key, member_id uuid unique references members(member_id), - role text not null default 'user' + role text not null default 'user', + created_at timestamptz not null default now(), + updated_at timestamptz not null default now() ); create table account ( id text primary key, account_id text not null, provider_id text not null, - user_id text not null references "user"(id), + user_id text not null references app_auth."user"(id), access_token text, refresh_token text, id_token text, @@ -194,8 +202,8 @@ const setupWorkflowDatabase = async () => { create table community_identities ( identity_id uuid primary key default gen_random_uuid(), - user_id text not null references "user"(id), - auth_account_id text not null unique references account(id), + user_id text not null, + auth_account_id text not null unique, provider text not null, provider_account_id text not null, username text not null, @@ -234,9 +242,12 @@ const setupWorkflowDatabase = async () => { insert into grades(id, code, display_grade, sort_order) values (1, 'B1', 'B1', 10), (2, 'B2', 'B2', 20); - insert into "user"(id, name, email, email_verified, created_at, updated_at, role) values - ('test-admin', 'Admin', 'admin@example.test', true, now(), now(), 'admin'), - ('test-applicant', 'Applicant', 'applicant@example.test', true, now(), now(), 'user'); + insert into app_auth."user"(id, name, email, email_verified, created_at, updated_at) values + ('test-admin', 'Admin', 'admin@example.test', true, now(), now()), + ('test-applicant', 'Applicant', 'applicant@example.test', true, now(), now()); + insert into app_accounts(user_id, role) values + ('test-admin', 'admin'), + ('test-applicant', 'user'); insert into account(id, account_id, provider_id, user_id, created_at, updated_at) values ('discord-account', 'discord-user-1', 'discord', 'test-applicant', now(), now()); insert into community_identities( @@ -275,15 +286,16 @@ const setupWorkflowDatabase = async () => { from member_directory_profiles profile join members member on member.member_id = profile.member_id join grades grade on grade.id = member.grade_id - join "user" app_user on app_user.member_id = member.member_id - join community_identities identity on identity.user_id = app_user.id + join app_accounts account on account.member_id = member.member_id + join community_identities identity on identity.user_id = account.user_id join community_memberships membership on membership.identity_id = identity.identity_id where profile.directory_visible and member.member_status = 'active'; - grant usage on schema public, app_api to app_rls; + grant usage on schema public, app_api, app_auth to app_rls; grant select on grades to app_rls; grant select, insert, update on members, member_directory_profiles to app_rls; - grant select, update on "user" to app_rls; + grant select on app_auth."user" to app_rls; + grant select, insert, update on app_accounts to app_rls; grant select on account, community_identities, community_memberships, member_status_history to app_rls; grant update on community_identities to app_rls; grant insert, update on community_memberships to app_rls; @@ -292,7 +304,8 @@ const setupWorkflowDatabase = async () => { alter table grades enable row level security; alter table members enable row level security; alter table member_directory_profiles enable row level security; - alter table "user" enable row level security; + alter table app_auth."user" enable row level security; + alter table app_accounts enable row level security; alter table account enable row level security; alter table community_identities enable row level security; alter table community_memberships enable row level security; @@ -301,7 +314,8 @@ const setupWorkflowDatabase = async () => { create policy grades_read on grades for select to app_rls using ( current_setting('app.current_user_id', true) <> '' ); - create policy users_all on "user" for all to app_rls using (true) with check (true); + create policy users_all on app_auth."user" for all to app_rls using (true) with check (true); + create policy accounts_all on app_accounts for all to app_rls using (true) with check (true); create policy account_read on account for select to app_rls using (true); create policy members_read on members for select to app_rls using ( current_setting('app.current_user_role', true) = 'admin' @@ -369,7 +383,7 @@ test('membership application uses one versioned row through reject, resubmit, ap const memberId = pending.memberId await client.exec('reset role') - const linked = await client.query<{ member_id: string }>(`select member_id from "user" where id = 'test-applicant'`) + const linked = await client.query<{ member_id: string }>(`select member_id from app_accounts where user_id = 'test-applicant'`) assert.equal(linked.rows[0].member_id, memberId) const initialProfileCount = await client.query<{ count: number }>('select count(*)::int as count from member_directory_profiles') assert.equal(initialProfileCount.rows[0].count, 0) @@ -598,7 +612,7 @@ test('empty RLS context can use auth tables but cannot read business data', asyn select set_config('app.current_user_role', '', true); `) const authUsers = await client.query<{ count: number }>( - 'select count(*)::int as count from "user"', + 'select count(*)::int as count from app_auth."user"', ) const gradeRows = await client.query<{ count: number }>( 'select count(*)::int as count from grades', @@ -735,22 +749,29 @@ test('development auth reloads memberId and downstream failures remain server er await client.exec(` create role app_rls; create table members (member_id uuid primary key); - create table "user" ( + create schema app_auth; + create table app_auth."user" ( id text primary key, - name text not null, + name text not null + ); + create table app_accounts ( + user_id text primary key, member_id uuid references members(member_id), role text default 'user' not null ); - create table session ( + create table app_auth.session ( id text primary key, - user_id text not null references "user"(id), + user_id text not null references app_auth."user"(id), expires_at timestamp not null ); insert into members values ('${memberId}'); - insert into "user" values ('user-1', 'test-user', '${memberId}', 'user'); - insert into session values ('session-1', 'user-1', now() + interval '1 hour'); + insert into app_auth."user" values ('user-1', 'test-user'); + insert into app_accounts values ('user-1', '${memberId}', 'user'); + insert into app_auth.session values ('session-1', 'user-1', now() + interval '1 hour'); grant app_rls to current_user; - grant select on "user", session to app_rls; + grant usage on schema app_auth to app_rls; + grant select on app_auth."user", app_auth.session to app_rls; + grant select on app_accounts to app_rls; set role app_rls; `) const db = createRlsDatabase(drizzle(client) as never) diff --git a/member/src/api_v0/member/service.ts b/member/src/api_v0/member/service.ts index 3f2a704..78c3fe3 100644 --- a/member/src/api_v0/member/service.ts +++ b/member/src/api_v0/member/service.ts @@ -4,12 +4,14 @@ import { and, desc, eq, + inArray, isNull, or, sql, type SQL, } from 'drizzle-orm' import { + appAccounts, grades, memberDirectoryProfiles, members, @@ -74,8 +76,7 @@ const memberSelection = { currentActivities: memberDirectoryProfiles.currentActivities, bio: memberDirectoryProfiles.bio, directoryVisible: memberDirectoryProfiles.directoryVisible, - userId: user.id, - userEmail: user.email, + userId: appAccounts.userId, } const selectMemberRows = async ( @@ -87,7 +88,7 @@ const selectMemberRows = async ( .select(memberSelection) .from(members) .innerJoin(grades, eq(grades.id, members.grade)) - .innerJoin(user, eq(user.memberId, members.memberId)) + .innerJoin(appAccounts, eq(appAccounts.memberId, members.memberId)) .leftJoin(memberDirectoryProfiles, eq(memberDirectoryProfiles.memberId, members.memberId)) .where(where) .orderBy(desc(members.submittedAt), desc(members.memberId)) @@ -147,10 +148,26 @@ const toMemberResponse = (row: MemberRow, evidence: DiscordMembershipEvidence | discord: toDiscordResponse(evidence), }) -const toAdminMemberResponse = (row: MemberRow, evidence: DiscordMembershipEvidence | null) => ({ +// The account email belongs to the authentication store. It is looked up by id +// instead of joined, so it becomes a remote call if that store moves to its own +// database. +const readAuthEmails = async (db: QueryDatabase, userIds: string[]) => { + if (userIds.length === 0) return new Map() + const rows = await db + .select({ id: user.id, email: user.email }) + .from(user) + .where(inArray(user.id, userIds)) + return new Map(rows.map(row => [row.id, row.email])) +} + +const toAdminMemberResponse = ( + row: MemberRow, + evidence: DiscordMembershipEvidence | null, + userEmail: string, +) => ({ ...toMemberResponse(row, evidence), userId: row.userId, - userEmail: row.userEmail, + userEmail, }) const setPrivateNoStore = (c: Context) => { @@ -315,10 +332,10 @@ export const joinMemberService: RouteHandler }) const linkedUsers = await tx - .update(user) - .set({ memberId, updatedAt: new Date() }) - .where(and(eq(user.id, appUser.id), isNull(user.memberId))) - .returning({ id: user.id }) + .update(appAccounts) + .set({ memberId, updatedAt: new Date().toISOString() }) + .where(and(eq(appAccounts.userId, appUser.id), isNull(appAccounts.memberId))) + .returning({ id: appAccounts.userId }) if (linkedUsers.length !== 1) throw new MemberLinkConflictError() return loadMemberRow(tx, memberId) @@ -556,7 +573,7 @@ export const listAdminMembersService: RouteHandler { + const { pageRows, hasNextPage, evidenceByUserId, emailByUserId } = await db.transaction(async (tx) => { const rows = await selectMemberRows( tx, conditions.length ? and(...conditions) : undefined, @@ -572,11 +589,16 @@ export const listAdminMembersService: RouteHandler row.userId), guildId, ), + emailByUserId: await readAuthEmails(tx, page.map(row => row.userId)), } }) return c.json({ - items: pageRows.map(row => toAdminMemberResponse(row, evidenceByUserId.get(row.userId) ?? null)), + items: pageRows.map(row => toAdminMemberResponse( + row, + evidenceByUserId.get(row.userId) ?? null, + emailByUserId.get(row.userId) ?? '', + )), nextCursor: hasNextPage && pageRows.length > 0 ? encodeCursor(pageRows[pageRows.length - 1]) : null, }, 200) } @@ -598,6 +620,7 @@ export const getAdminMemberService: RouteHandler ({ ...item, fromStatus: item.fromStatus as MemberStatus | null, @@ -722,13 +745,13 @@ export const createApproveMemberService = ( if (updated.length !== 1) throw new MemberVersionConflictError() const linked = await tx - .update(user) - .set({ memberId, updatedAt: now }) + .update(appAccounts) + .set({ memberId, updatedAt: now.toISOString() }) .where(and( - eq(user.id, current.userId), - or(isNull(user.memberId), eq(user.memberId, memberId)), + eq(appAccounts.userId, current.userId), + or(isNull(appAccounts.memberId), eq(appAccounts.memberId, memberId)), )) - .returning({ id: user.id }) + .returning({ id: appAccounts.userId }) if (linked.length !== 1) throw new MemberLinkConflictError() await tx.insert(memberDirectoryProfiles).values({ @@ -756,9 +779,13 @@ export const createApproveMemberService = ( throw error } - const approved = await db.transaction((tx) => loadMemberRow(tx, memberId)) + const approved = await db.transaction(async (tx) => { + const row = await loadMemberRow(tx, memberId) + if (!row) return null + return { row, email: (await readAuthEmails(tx, [row.userId])).get(row.userId) ?? '' } + }) if (!approved) throw new Error('Approved member could not be reloaded') - return c.json(toAdminMemberResponse(approved, evidence), 200) + return c.json(toAdminMemberResponse(approved.row, evidence, approved.email), 200) } export const approveMemberService = createApproveMemberService() @@ -881,9 +908,13 @@ export const updateAdminMemberService: RouteHandler loadResponseForMember(tx, memberId, requireGuildId(c))) + const result = await db.transaction(async (tx) => { + const loaded = await loadResponseForMember(tx, memberId, requireGuildId(c)) + if (!loaded) return null + return { ...loaded, email: (await readAuthEmails(tx, [loaded.row.userId])).get(loaded.row.userId) ?? '' } + }) if (!result) throw new Error('Updated member could not be reloaded') - return c.json(toAdminMemberResponse(result.row, result.evidence), 200) + return c.json(toAdminMemberResponse(result.row, result.evidence, result.email), 200) } export const rejectMemberService: RouteHandler = async (c) => { @@ -939,7 +970,11 @@ export const rejectMemberService: RouteHandler loadResponseForMember(tx, memberId, requireGuildId(c))) + const result = await db.transaction(async (tx) => { + const loaded = await loadResponseForMember(tx, memberId, requireGuildId(c)) + if (!loaded) return null + return { ...loaded, email: (await readAuthEmails(tx, [loaded.row.userId])).get(loaded.row.userId) ?? '' } + }) if (!result) throw new Error('Rejected member could not be reloaded') - return c.json(toAdminMemberResponse(result.row, result.evidence), 200) + return c.json(toAdminMemberResponse(result.row, result.evidence, result.email), 200) } diff --git a/member/src/core/auth.ts b/member/src/core/auth.ts index cfba255..ffe4e59 100644 --- a/member/src/core/auth.ts +++ b/member/src/core/auth.ts @@ -2,7 +2,7 @@ import type { Context, Next } from 'hono' import { AppContext } from './types' import { verify } from 'hono/jwt' import { and, eq, gt } from 'drizzle-orm' -import { session, user } from '../../../share/drizzle/schema' +import { appAccounts, session, user } from '../../../share/drizzle/schema' export type authUser = { id: string @@ -15,52 +15,74 @@ export type appUser = { role: 'admin' | 'user' } -const userSelection = { +// Who the caller is, according to the authentication store. +const subjectSelection = { id: user.id, name: user.name, - memberId: user.memberId, - role: user.role, } +// What the caller may do, according to the domain. +const accountSelection = { + memberId: appAccounts.memberId, + role: appAccounts.role, +} + +// This is the boundary between the authentication store and the domain, and the +// only place that reads both. The two lookups stay separate rather than joining +// across app_auth and public, so moving the authentication store to its own +// database turns the first one into a remote call and leaves the second alone. const loadAppUser = async ( c: Context, userId: string, sessionId?: string, ): Promise => { - const [currentUser] = await c.get('db').transaction(async (db) => sessionId - ? db - .select(userSelection) - .from(user) - .innerJoin(session, eq(session.userId, user.id)) - .where(and( - eq(user.id, userId), - eq(session.id, sessionId), - gt(session.expiresAt, new Date()), - )) - .limit(1) - : db - .select(userSelection) - .from(user) - .where(eq(user.id, userId)) - .limit(1)); + const resolved = await c.get('db').transaction(async (db) => { + const [subject] = sessionId + ? await db + .select(subjectSelection) + .from(user) + .innerJoin(session, eq(session.userId, user.id)) + .where(and( + eq(user.id, userId), + eq(session.id, sessionId), + gt(session.expiresAt, new Date()), + )) + .limit(1) + : await db + .select(subjectSelection) + .from(user) + .where(eq(user.id, userId)) + .limit(1); - if (!currentUser) { + if (!subject) return null; + + const [account] = await db + .select(accountSelection) + .from(appAccounts) + .where(eq(appAccounts.userId, subject.id)) + .limit(1); + + if (!account) return null; + + return { + id: subject.id, + name: subject.name, + memberId: account.memberId, + role: account.role === 'admin' ? 'admin' as const : 'user' as const, + }; + }); + + if (!resolved) { return null; } - const role = currentUser.role === 'admin' ? 'admin' : 'user'; c.get('db').setIdentity({ - userId: currentUser.id, - memberId: currentUser.memberId, - role, + userId: resolved.id, + memberId: resolved.memberId, + role: resolved.role, }); - return { - id: currentUser.id, - name: currentUser.name, - memberId: currentUser.memberId, - role, - }; + return resolved; } export const authMiddleware = async (c: Context, next: Next) => { diff --git a/member/src/core/migration.integration.test.ts b/member/src/core/migration.integration.test.ts index 73d4b19..33b3326 100644 --- a/member/src/core/migration.integration.test.ts +++ b/member/src/core/migration.integration.test.ts @@ -13,6 +13,10 @@ const membershipWorkflowMigrationPath = resolve( dirname(fileURLToPath(import.meta.url)), '../../../supabase/migrations/20260716123951_membership_workflow.sql', ) +const splitAuthSchemaMigrationPath = resolve( + dirname(fileURLToPath(import.meta.url)), + '../../../supabase/migrations/20260727000000_split_auth_schema.sql', +) const confirmedTestDataPurgeRunbookPath = resolve( dirname(fileURLToPath(import.meta.url)), '../../../supabase/runbooks/purge_confirmed_test_membership_data.sql', @@ -600,3 +604,59 @@ test('membership workflow fresh replay does not require Supabase API roles', asy await client.close() } }) + +test('splitting the auth schema leaves no reference from the domain into app_auth', async () => { + const client = await PGlite.create() + + try { + await createCurrentSchema(client, false) + await client.exec(await readFile(membershipWorkflowMigrationPath, 'utf8')) + await client.exec(await readFile(splitAuthSchemaMigrationPath, 'utf8')) + + const layout = await client.query<{ + auth_tables: number + public_auth_tables: number + accounts_exists: boolean + user_has_domain_columns: number + }>(` + select + (select count(*)::int from pg_tables + where schemaname = 'app_auth' + and tablename in ('user', 'session', 'account', 'verification')) as auth_tables, + (select count(*)::int from pg_tables + where schemaname = 'public' + and tablename in ('user', 'session', 'account', 'verification')) as public_auth_tables, + to_regclass('public.app_accounts') is not null as accounts_exists, + (select count(*)::int from information_schema.columns + where table_schema = 'app_auth' + and table_name = 'user' + and column_name in ('member_id', 'role')) as user_has_domain_columns + `) + assert.deepEqual(layout.rows[0], { + auth_tables: 4, + public_auth_tables: 0, + accounts_exists: true, + user_has_domain_columns: 0, + }) + + // A foreign key cannot span databases. If one is ever added across this + // boundary, moving the authentication store becomes a schema migration + // again, so the absence is asserted rather than left to review. + const crossings = await client.query<{ constraint_name: string }>(` + select con.conname as constraint_name + from pg_constraint con + join pg_class child on child.oid = con.conrelid + join pg_namespace child_ns on child_ns.oid = child.relnamespace + join pg_class parent on parent.oid = con.confrelid + join pg_namespace parent_ns on parent_ns.oid = parent.relnamespace + where con.contype = 'f' + and ( + (child_ns.nspname = 'public' and parent_ns.nspname = 'app_auth') + or (child_ns.nspname = 'app_auth' and parent_ns.nspname = 'public') + ) + `) + assert.deepEqual(crossings.rows, []) + } finally { + await client.close() + } +}) diff --git a/share/drizzle/relations.ts b/share/drizzle/relations.ts index fa6c703..780a0d5 100644 --- a/share/drizzle/relations.ts +++ b/share/drizzle/relations.ts @@ -1,6 +1,7 @@ import { relations } from "drizzle-orm/relations" import { account, + appAccounts, communityIdentities, communityMemberships, grades, @@ -11,17 +12,14 @@ import { user, } from "./schema" -export const userRelations = relations(user, ({ many, one }) => ({ - member: one(members, { - fields: [user.memberId], - references: [members.memberId], - relationName: "userMember", - }), +// Relations never cross between app_auth and the domain. The authentication +// tables relate only to each other, and the domain reaches an authentication +// subject through app_accounts, which stores the identifier as a plain value. +// See supabase/migrations/20260727000000_split_auth_schema.sql. + +export const userRelations = relations(user, ({ many }) => ({ sessions: many(session), accounts: many(account), - reviewedMembers: many(members, { relationName: "memberReviewer" }), - communityIdentities: many(communityIdentities), - memberStatusChanges: many(memberStatusHistory, { relationName: "memberStatusChangedBy" }), })) export const sessionRelations = relations(session, ({ one }) => ({ @@ -36,7 +34,13 @@ export const accountRelations = relations(account, ({ one }) => ({ fields: [account.userId], references: [user.id], }), - communityIdentity: one(communityIdentities), +})) + +export const appAccountsRelations = relations(appAccounts, ({ one }) => ({ + member: one(members, { + fields: [appAccounts.memberId], + references: [members.memberId], + }), })) export const gradesRelations = relations(grades, ({ many }) => ({ @@ -48,11 +52,7 @@ export const membersRelations = relations(members, ({ many, one }) => ({ fields: [members.grade], references: [grades.id], }), - reviewedBy: one(user, { - fields: [members.reviewedByUserId], - references: [user.id], - relationName: "memberReviewer", - }), + account: one(appAccounts), directoryProfile: one(memberDirectoryProfiles), statusHistory: many(memberStatusHistory), })) @@ -64,15 +64,7 @@ export const memberDirectoryProfilesRelations = relations(memberDirectoryProfile }), })) -export const communityIdentitiesRelations = relations(communityIdentities, ({ many, one }) => ({ - user: one(user, { - fields: [communityIdentities.userId], - references: [user.id], - }), - authAccount: one(account, { - fields: [communityIdentities.authAccountId], - references: [account.id], - }), +export const communityIdentitiesRelations = relations(communityIdentities, ({ many }) => ({ memberships: many(communityMemberships), })) @@ -88,9 +80,4 @@ export const memberStatusHistoryRelations = relations(memberStatusHistory, ({ on fields: [memberStatusHistory.memberId], references: [members.memberId], }), - changedBy: one(user, { - fields: [memberStatusHistory.changedByUserId], - references: [user.id], - relationName: "memberStatusChangedBy", - }), })) diff --git a/share/drizzle/schema.ts b/share/drizzle/schema.ts index 56836ac..61b50c0 100644 --- a/share/drizzle/schema.ts +++ b/share/drizzle/schema.ts @@ -6,6 +6,7 @@ import { foreignKey, index, integer, + pgSchema, pgTable, primaryKey, smallint, @@ -21,8 +22,15 @@ export const memberStatusValues = ["pending", "active", "rejected", "withdrawn"] export type MemberStatus = typeof memberStatusValues[number] // --- Better Auth Tables --- +// +// These live in their own schema and hold no domain column, so the +// authentication store can be moved to its own database. Nothing here may +// reference an application table: see app_accounts below for the link, and +// supabase/migrations/20260727000000_split_auth_schema.sql for the boundary. -export const user = pgTable("user", { +export const appAuth = pgSchema("app_auth") + +export const user = appAuth.table("user", { id: text("id").primaryKey(), name: text("name").notNull(), email: text("email").notNull().unique(), @@ -30,22 +38,9 @@ export const user = pgTable("user", { image: text("image"), createdAt: timestamp("created_at").notNull(), updatedAt: timestamp("updated_at").notNull(), - memberId: uuid("member_id"), - role: text("role", { enum: ["user", "admin"] }).default("user").notNull(), -}, (table) => [ - foreignKey({ - columns: [table.memberId], - foreignColumns: [members.memberId], - name: "user_member_id_fkey", - }).onDelete("set null"), - check("user_role_valid", sql`${table.role} in ('user', 'admin')`), - uniqueIndex("user_member_id_unique") - .on(table.memberId) - .where(sql`${table.memberId} is not null`), - index("user_member_id_idx").on(table.memberId), -]) +}) -export const session = pgTable("session", { +export const session = appAuth.table("session", { id: text("id").primaryKey(), expiresAt: timestamp("expires_at").notNull(), token: text("token").notNull().unique(), @@ -59,7 +54,7 @@ export const session = pgTable("session", { index("session_expires_at_idx").on(table.expiresAt), ]) -export const account = pgTable("account", { +export const account = appAuth.table("account", { id: text("id").primaryKey(), accountId: text("account_id").notNull(), providerId: text("provider_id").notNull(), @@ -78,7 +73,7 @@ export const account = pgTable("account", { index("account_user_id_idx").on(table.userId), ]) -export const verification = pgTable("verification", { +export const verification = appAuth.table("verification", { id: text("id").primaryKey(), identifier: text("identifier").notNull(), value: text("value").notNull(), @@ -91,6 +86,27 @@ export const verification = pgTable("verification", { // --- Application Tables --- +// The domain's own record of an authentication subject, and the only thing that +// links the two sides. userId is an opaque identifier rather than a foreign key +// so the authentication store stays movable. +export const appAccounts = pgTable("app_accounts", { + userId: text("user_id").primaryKey(), + memberId: uuid("member_id"), + role: text("role", { enum: ["user", "admin"] }).default("user").notNull(), + createdAt: timestamp("created_at", { withTimezone: true, mode: "string" }).defaultNow().notNull(), + updatedAt: timestamp("updated_at", { withTimezone: true, mode: "string" }).defaultNow().notNull(), +}, (table) => [ + foreignKey({ + columns: [table.memberId], + foreignColumns: [members.memberId], + name: "app_accounts_member_id_fkey", + }).onDelete("set null"), + unique("app_accounts_member_id_key").on(table.memberId), + check("app_accounts_role_valid", sql`${table.role} in ('user', 'admin')`), + check("app_accounts_user_id_not_blank", sql`btrim(${table.userId}) <> ''`), + index("app_accounts_member_id_idx").on(table.memberId), +]) + export const grades = pgTable("grades", { id: integer("id").primaryKey(), code: text("code").notNull(), @@ -124,7 +140,9 @@ export const members = pgTable("members", { applicationVersion: integer("application_version").default(1).notNull(), submittedAt: timestamp("submitted_at", { withTimezone: true, mode: "string" }).defaultNow().notNull(), reviewedAt: timestamp("reviewed_at", { withTimezone: true, mode: "string" }), - reviewedByUserId: text("reviewed_by_user_id").references((): AnyPgColumn => user.id, { onDelete: "restrict" }), + // Reviewer identity snapshot. Not a foreign key: the authentication store + // may move to its own database. + reviewedByUserId: text("reviewed_by_user_id"), reviewReason: text("review_reason"), createdAt: timestamp("created_at", { withTimezone: true, mode: "string" }).defaultNow().notNull(), updatedAt: timestamp("updated_at", { withTimezone: true, mode: "string" }).defaultNow().notNull(), @@ -212,16 +230,6 @@ export const communityIdentities = pgTable("community_identities", { createdAt: timestamp("created_at", { withTimezone: true, mode: "string" }).defaultNow().notNull(), updatedAt: timestamp("updated_at", { withTimezone: true, mode: "string" }).defaultNow().notNull(), }, (table) => [ - foreignKey({ - columns: [table.userId], - foreignColumns: [user.id], - name: "community_identities_user_id_fkey", - }).onDelete("cascade"), - foreignKey({ - columns: [table.authAccountId], - foreignColumns: [account.id], - name: "community_identities_auth_account_id_fkey", - }).onDelete("cascade"), check("community_identities_provider_not_blank", sql`btrim(${table.provider}) <> ''`), check("community_identities_account_not_blank", sql`btrim(${table.providerAccountId}) <> ''`), check("community_identities_username_not_blank", sql`btrim(${table.username}) <> ''`), diff --git a/supabase/migrations/20260727000000_split_auth_schema.sql b/supabase/migrations/20260727000000_split_auth_schema.sql new file mode 100644 index 0000000..6faa0bc --- /dev/null +++ b/supabase/migrations/20260727000000_split_auth_schema.sql @@ -0,0 +1,267 @@ +-- Separate the authentication tables from the domain. +-- +-- Better Auth owns user/session/account/verification. They move into app_auth, +-- and every reference that crossed between them and the domain is removed, so +-- the authentication store can later be moved to its own database without +-- touching a domain table. A foreign key cannot span databases, which is why +-- the crossings are dropped now rather than when that move happens. +-- +-- The link that remains is public.app_accounts.user_id: a value, not a foreign +-- key. It is the single place that knows an authentication subject exists. + +set statement_timeout = 0; +set lock_timeout = '10s'; + +begin; + +create schema if not exists app_auth; +comment on schema app_auth is + 'Authentication store owned by Better Auth. Nothing in here may reference a domain table.'; + +-- Move the authentication tables. Row level security state, policies and grants +-- follow the table, so they are not restated here. +alter table public."user" set schema app_auth; +alter table public.session set schema app_auth; +alter table public.account set schema app_auth; +alter table public.verification set schema app_auth; + +grant usage on schema app_auth to app_rls; + +-- The directory view reads user.member_id, so it is dropped before that column +-- can go. It is recreated against app_accounts at the end of this migration. +drop view if exists app_api.member_directory_entries; + +-- The domain's own record of an authentication subject. role lives here because +-- the membership trigger authorizes against it, and a trigger cannot read +-- another database. +create table public.app_accounts ( + user_id text primary key, + member_id uuid unique references public.members(member_id) on delete set null, + role text not null default 'user', + created_at timestamptz not null default now(), + updated_at timestamptz not null default now(), + constraint app_accounts_role_valid check (role in ('user', 'admin')), + constraint app_accounts_user_id_not_blank check (btrim(user_id) <> '') +); + +comment on table public.app_accounts is + 'Domain-side account record. user_id is an opaque authentication subject, intentionally not a foreign key.'; +comment on column public.app_accounts.user_id is + 'Authentication subject issued by the identity provider. Deliberately unconstrained so the provider can move to its own database.'; + +create index app_accounts_member_id_idx on public.app_accounts (member_id); + +insert into public.app_accounts (user_id, member_id, role, created_at, updated_at) +select id, member_id, role, created_at, updated_at +from app_auth."user"; + +-- The authentication table no longer carries domain columns, so Better Auth +-- owns its schema outright. +alter table app_auth."user" drop constraint if exists user_member_id_fkey; +alter table app_auth."user" drop constraint if exists user_role_valid; +alter table app_auth."user" drop column if exists member_id; +alter table app_auth."user" drop column if exists role; + +-- Remove the crossings that pointed from the domain into authentication. The +-- actor identifiers stay as snapshots, matching member_status_history, which +-- already stored its actor without a foreign key. +alter table public.members drop constraint if exists members_reviewed_by_user_id_fkey; +alter table public.community_identities drop constraint if exists community_identities_user_id_fkey; +alter table public.community_identities drop constraint if exists community_identities_auth_account_id_fkey; + +comment on column public.members.reviewed_by_user_id is + 'Reviewer identity snapshot. Not a foreign key: the authentication store may live in another database.'; +comment on column public.community_identities.user_id is + 'Authentication subject snapshot. Not a foreign key, for the same reason as members.reviewed_by_user_id.'; +comment on column public.community_identities.auth_account_id is + 'Linked provider account issued by the identity store. Snapshot, not a foreign key.'; + +grant select, insert, update on public.app_accounts to app_rls; +alter table public.app_accounts enable row level security; +alter table public.app_accounts force row level security; + +drop policy if exists app_accounts_app_backend on public.app_accounts; +create policy app_accounts_app_backend on public.app_accounts + for all to app_rls using (true) with check (true); + +-- The membership trigger authorized against the authentication table. It now +-- reads the domain's own account record. +create or replace function app_private.enforce_member_workflow() +returns trigger +language plpgsql +set search_path = pg_catalog, public +as $$ +declare + actor_id text := nullif(current_setting('app.current_user_id', true), ''); + actor_role text := nullif(current_setting('app.current_user_role', true), ''); +begin + if actor_id is null or actor_role not in ('user', 'admin') then + raise exception using + errcode = '23514', + message = 'member workflow requires an authenticated user or admin actor'; + end if; + + if not exists ( + select 1 + from public.app_accounts actor + where actor.user_id = actor_id + and actor.role = actor_role + ) then + raise exception using + errcode = '23514', + message = 'member workflow actor does not match the persisted application role'; + end if; + + if tg_op = 'INSERT' then + if new.member_status <> 'pending' + or new.application_version <> 1 + or new.reviewed_at is not null + or new.reviewed_by_user_id is not null + or new.review_reason is not null + then + raise exception using + errcode = '23514', + message = 'new membership applications must start pending at version 1 without review metadata'; + end if; + + return new; + end if; + + if new.member_id is distinct from old.member_id + or new.created_at is distinct from old.created_at + then + raise exception using + errcode = '23514', + message = 'member identity and creation timestamp are immutable'; + end if; + + if new.application_version <> old.application_version + 1 then + raise exception using + errcode = '23514', + message = 'member application version must increase by exactly one'; + end if; + + if new.member_status is not distinct from old.member_status then + if new.submitted_at is distinct from old.submitted_at + or new.reviewed_at is distinct from old.reviewed_at + or new.reviewed_by_user_id is distinct from old.reviewed_by_user_id + or new.review_reason is distinct from old.review_reason + then + raise exception using + errcode = '23514', + message = 'submission and review metadata can only change during a status transition'; + end if; + elsif old.member_status = 'pending' + and new.member_status in ('active', 'rejected') then + if actor_role <> 'admin' + or new.reviewed_at is null + or new.reviewed_by_user_id is distinct from actor_id + or new.submitted_at is distinct from old.submitted_at + or ( + new.member_status = 'active' + and new.review_reason is not null + ) + or ( + new.member_status = 'rejected' + and btrim(coalesce(new.review_reason, '')) = '' + ) + then + raise exception using + errcode = '23514', + message = 'only an admin may approve or reject a pending application with consistent review metadata'; + end if; + elsif old.member_status = 'rejected' + and new.member_status = 'pending' then + if actor_role not in ('user', 'admin') + or new.reviewed_at is not null + or new.reviewed_by_user_id is not null + or new.review_reason is not null + then + raise exception using + errcode = '23514', + message = 'resubmission must clear all review metadata'; + end if; + elsif old.member_status = 'active' + and new.member_status = 'withdrawn' then + if actor_role <> 'admin' + or new.reviewed_at is null + or new.reviewed_by_user_id is distinct from actor_id + or btrim(coalesce(new.review_reason, '')) = '' + or new.submitted_at is distinct from old.submitted_at + then + raise exception using + errcode = '23514', + message = 'only an admin may withdraw an active member with a reason'; + end if; + else + raise exception using + errcode = '23514', + message = format( + 'invalid member status transition: %s -> %s', + old.member_status, + new.member_status + ); + end if; + + return new; +end +$$; + +-- The directory view reached into the authentication table to find the viewer +-- and the profile owner. Both now resolve through app_accounts. +create view app_api.member_directory_entries +with (security_barrier = true) +as +select + profile.member_id, + profile.display_name, + grade.code as grade_code, + grade.display_grade, + profile.skills, + profile.interests, + profile.current_activities, + profile.bio, + coalesce( + jsonb_agg( + jsonb_build_object( + 'provider', identity.provider, + 'communityId', membership.community_id, + 'nickname', membership.nickname, + 'roles', membership.role_names + ) + order by identity.provider, membership.community_id + ) filter (where membership.membership_status = 'member'), + '[]'::jsonb + ) as communities +from public.member_directory_profiles profile +join public.members member on member.member_id = profile.member_id +join public.grades grade on grade.id = member.grade_id +left join public.app_accounts account on account.member_id = member.member_id +left join public.community_identities identity on identity.user_id = account.user_id +left join public.community_memberships membership on membership.identity_id = identity.identity_id +where profile.directory_visible + and member.member_status = 'active' + and exists ( + select 1 + from public.app_accounts viewer_account + join public.members viewer_member on viewer_member.member_id = viewer_account.member_id + where viewer_account.user_id = current_setting('app.current_user_id', true) + and viewer_member.member_status = 'active' + ) +group by + profile.member_id, + profile.display_name, + grade.code, + grade.display_grade, + profile.skills, + profile.interests, + profile.current_activities, + profile.bio; + +revoke all on app_api.member_directory_entries from public; +grant select on app_api.member_directory_entries to app_rls; + +comment on view app_api.member_directory_entries is + 'Allowlisted active-member directory; intentionally excludes all private member fields.'; + +commit; diff --git a/supabase/runbooks/promote_initial_admin.sql b/supabase/runbooks/promote_initial_admin.sql index 0aa4e01..8271f64 100644 --- a/supabase/runbooks/promote_initial_admin.sql +++ b/supabase/runbooks/promote_initial_admin.sql @@ -37,10 +37,11 @@ begin raise exception 'expected exactly one verified Discord/guild identity, found %', matched_count; end if; - update public."user" + -- role is a domain fact and lives in app_accounts, not in the auth store. + update public.app_accounts set role = 'admin', updated_at = now() - where id = matched_user_id + where user_id = matched_user_id and role = 'user'; if not found then diff --git a/supabase/runbooks/provision_app_runtime_login.sql b/supabase/runbooks/provision_app_runtime_login.sql index 6fe1bdd..9da64da 100644 --- a/supabase/runbooks/provision_app_runtime_login.sql +++ b/supabase/runbooks/provision_app_runtime_login.sql @@ -85,6 +85,13 @@ begin execute format('revoke all on all tables in schema public from %I', runtime_role); execute format('revoke all on all sequences in schema public from %I', runtime_role); execute format('revoke execute on all functions in schema public from %I', runtime_role); + -- app_auth only exists once the authentication tables have been split out, + -- so this stays optional for a database that has not reached that migration. + if exists (select 1 from pg_namespace where nspname = 'app_auth') then + execute format('revoke all on schema app_auth from %I', runtime_role); + execute format('revoke all on all tables in schema app_auth from %I', runtime_role); + execute format('revoke all on all sequences in schema app_auth from %I', runtime_role); + end if; execute format('revoke all on all tables in schema app_private from %I', runtime_role); execute format('revoke all on all sequences in schema app_private from %I', runtime_role); execute format('revoke execute on all functions in schema app_private from %I', runtime_role); diff --git a/supabase/runbooks/purge_confirmed_test_membership_data.sql b/supabase/runbooks/purge_confirmed_test_membership_data.sql index 6385db5..a66e577 100644 --- a/supabase/runbooks/purge_confirmed_test_membership_data.sql +++ b/supabase/runbooks/purge_confirmed_test_membership_data.sql @@ -1,5 +1,10 @@ -- One-off destructive runbook. This is intentionally NOT a migration. -- +-- Ordering matters: this runs before 20260716123951_membership_workflow.sql, +-- which is itself before the authentication tables move out of public in +-- 20260727000000_split_auth_schema.sql. The public references below are +-- therefore correct at the point this file is executed. +-- -- Replace every REPLACE_* value with a pipe-separated, fully audited set. -- Use the literal EMPTY for an expected empty set. Values must not contain "|" -- or "=>". Run as the database migration owner during a maintenance window, diff --git a/supabase/tests/001_reconcile_better_auth_baseline.test.sql b/supabase/tests/001_reconcile_better_auth_baseline.test.sql index 52846d3..2cbae2f 100644 --- a/supabase/tests/001_reconcile_better_auth_baseline.test.sql +++ b/supabase/tests/001_reconcile_better_auth_baseline.test.sql @@ -5,15 +5,15 @@ set local search_path = public, extensions, pg_catalog; select plan(16); -select has_table('public', 'user', 'Better Auth user table exists'); -select has_table('public', 'account', 'Better Auth account table exists'); -select has_table('public', 'session', 'Better Auth session table exists'); -select has_table('public', 'verification', 'Better Auth verification table exists'); +select has_table('app_auth', 'user', 'Better Auth user table exists'); +select has_table('app_auth', 'account', 'Better Auth account table exists'); +select has_table('app_auth', 'session', 'Better Auth session table exists'); +select has_table('app_auth', 'verification', 'Better Auth verification table exists'); select has_table('public', 'grades', 'grade table exists'); select has_table('public', 'members', 'member table exists'); select has_table('public', 'event_messages', 'event message table exists'); -select has_index('public', 'account', 'account_provider_account_unique', 'provider account is unique'); -select has_index('public', 'user', 'user_member_id_unique', 'a member links to at most one app user'); +select has_index('app_auth', 'account', 'account_provider_account_unique', 'provider account is unique'); +select has_index('public', 'app_accounts', 'app_accounts_member_id_key', 'a member links to at most one account'); select ok( to_regclass('public.members_grade_idx') is not null or to_regclass('public.members_grade_id_idx') is not null, @@ -25,10 +25,10 @@ select is( select count(*)::integer from pg_class where oid in ( - 'public."user"'::regclass, - 'public.account'::regclass, - 'public.session'::regclass, - 'public.verification'::regclass, + 'app_auth."user"'::regclass, + 'app_auth.account'::regclass, + 'app_auth.session'::regclass, + 'app_auth.verification'::regclass, 'public.grades'::regclass, 'public.members'::regclass, 'public.event_messages'::regclass @@ -43,10 +43,10 @@ select is( select count(*)::integer from pg_class where oid in ( - 'public."user"'::regclass, - 'public.account'::regclass, - 'public.session'::regclass, - 'public.verification'::regclass, + 'app_auth."user"'::regclass, + 'app_auth.account'::regclass, + 'app_auth.session'::regclass, + 'app_auth.verification'::regclass, 'public.grades'::regclass, 'public.members'::regclass, 'public.event_messages'::regclass diff --git a/supabase/tests/002_membership_foundation.test.sql b/supabase/tests/002_membership_foundation.test.sql index 5aa4d52..ed712fe 100644 --- a/supabase/tests/002_membership_foundation.test.sql +++ b/supabase/tests/002_membership_foundation.test.sql @@ -17,17 +17,24 @@ select hasnt_column('public', 'grades', 'year', 'grade is not year-specific'); select col_is_pk('public', 'member_directory_profiles', 'member_id', 'directory profile is one-to-one'); select has_index('public', 'members', 'members_student_id_unique', 'student ID is individually unique'); select has_index('public', 'members', 'members_student_email_unique', 'student email is individually unique'); -select has_index('public', 'account', 'account_provider_account_unique', 'provider account is unique'); +select has_index('app_auth', 'account', 'account_provider_account_unique', 'provider account is unique'); +-- The reviewer is stored as a snapshot, like member_status_history's actor. A +-- foreign key here would tie the domain to the authentication store and block +-- moving it to its own database. select is( ( - select confdeltype::text - from pg_constraint - where conrelid = 'public.members'::regclass - and conname = 'members_reviewed_by_user_id_fkey' + select count(*)::integer + from pg_constraint constraint_record + join pg_attribute column_record + on column_record.attrelid = constraint_record.conrelid + and column_record.attnum = any (constraint_record.conkey) + where constraint_record.conrelid = 'public.members'::regclass + and constraint_record.contype = 'f' + and column_record.attname = 'reviewed_by_user_id' ), - 'r'::text, - 'reviewed users cannot be deleted while a membership review references them' + 0, + 'reviewer IDs are snapshots without a foreign key into the auth store' ); select is( @@ -85,10 +92,10 @@ select is( select count(*)::integer from pg_class where oid in ( - 'public."user"'::regclass, - 'public.account'::regclass, - 'public.session'::regclass, - 'public.verification'::regclass, + 'app_auth."user"'::regclass, + 'app_auth.account'::regclass, + 'app_auth.session'::regclass, + 'app_auth.verification'::regclass, 'public.grades'::regclass, 'public.members'::regclass, 'public.member_directory_profiles'::regclass, @@ -107,10 +114,10 @@ select is( select count(*)::integer from pg_class where oid in ( - 'public."user"'::regclass, - 'public.account'::regclass, - 'public.session'::regclass, - 'public.verification'::regclass, + 'app_auth."user"'::regclass, + 'app_auth.account'::regclass, + 'app_auth.session'::regclass, + 'app_auth.verification'::regclass, 'public.grades'::regclass, 'public.members'::regclass, 'public.member_directory_profiles'::regclass, diff --git a/supabase/tests/003_membership_rls.test.sql b/supabase/tests/003_membership_rls.test.sql index 526c75a..2c1f64d 100644 --- a/supabase/tests/003_membership_rls.test.sql +++ b/supabase/tests/003_membership_rls.test.sql @@ -3,7 +3,7 @@ begin; create extension if not exists pgtap with schema extensions; set local search_path = public, app_api, extensions, pg_catalog; -select plan(18); +select plan(19); -- Test-only grants are rolled back with this file. Runtime migrations do not -- expose extension functions to the application role. @@ -12,12 +12,17 @@ grant execute on all functions in schema extensions to app_rls; set local role app_rls; -insert into public."user" ( - id, name, email, email_verified, created_at, updated_at, role +insert into app_auth."user" ( + id, name, email, email_verified, created_at, updated_at ) values - ('test-admin', 'Admin', 'admin@example.test', true, now(), now(), 'admin'), - ('test-applicant', 'Applicant', 'applicant@example.test', true, now(), now(), 'user'), - ('test-viewer', 'Viewer', 'viewer@example.test', true, now(), now(), 'user'); + ('test-admin', 'Admin', 'admin@example.test', true, now(), now()), + ('test-applicant', 'Applicant', 'applicant@example.test', true, now(), now()), + ('test-viewer', 'Viewer', 'viewer@example.test', true, now(), now()); + +insert into public.app_accounts (user_id, role) values + ('test-admin', 'admin'), + ('test-applicant', 'user'), + ('test-viewer', 'user'); select set_config('app.current_user_id', 'test-admin', true), @@ -51,9 +56,9 @@ select lives_ok( application_version = application_version + 1 where member_id = '00000000-0000-4000-8000-000000000002'; - update public."user" + update public.app_accounts set member_id = '00000000-0000-4000-8000-000000000002' - where id = 'test-viewer'; + where user_id = 'test-viewer'; insert into public.member_directory_profiles ( member_id, @@ -132,9 +137,9 @@ select lives_ok( 'applicant@student.example' ); - update public."user" + update public.app_accounts set member_id = '00000000-0000-4000-8000-000000000001' - where id = 'test-applicant'; + where user_id = 'test-applicant'; end $body$ $$, @@ -328,15 +333,26 @@ select set_config('app.current_member_id', '', true), set_config('app.current_user_role', 'admin', true); -select throws_ok( - $$delete from public."user" where id = 'test-admin'$$, - '23503', - null, - 'a reviewer account cannot be deleted while reviewed memberships reference it' +-- The reviewer used to be protected by a foreign key. That key crossed into the +-- authentication store and had to go, so the reviewer is now a snapshot: +-- deleting the account succeeds and leaves the review record as written. +select lives_ok( + $$delete from app_auth."user" where id = 'test-admin'$$, + 'a reviewer account can be deleted because the reviewer is only a snapshot' +); + +select is( + ( + select reviewed_by_user_id + from public.members + where member_id = '00000000-0000-4000-8000-000000000002' + ), + 'test-admin'::text, + 'deleting the reviewer account leaves the review snapshot untouched' ); select lives_ok( - $$delete from public."user" where id = 'test-applicant'$$, + $$delete from app_auth."user" where id = 'test-applicant'$$, 'deleting an authentication account does not rewrite immutable actor snapshots' );