From c18ece510c45d3ed584db1acce1ca52f0973ae20 Mon Sep 17 00:00:00 2001 From: yufoxda Date: Mon, 27 Jul 2026 15:23:17 +0900 Subject: [PATCH 1/5] refactor(community): give the provider port vendor-neutral types The port was generic in name only. Its methods returned DiscordGuildMembership, DiscordMessage and DiscordReactionUser, and every identifier was validated against the Discord snowflake format, so the identifier regex reached callers that have no reason to know what a snowflake is. Replacing the provider would have meant editing the interface and both api_v0 services rather than swapping an adapter. The port now speaks CommunityRole, CommunityMembership, CommunityMessage, CommunityReactionUser and CommunityAccountProfile, treats identifiers as opaque strings, and drops the Discord message length limit. Discord's snowflake format, its 2000-character limit, and the global_name field it returns are refinements applied in discord/schema.ts, which is the only layer that issues those values. The provider-specific field name is mapped to displayName at the adapter boundary, matching the provider_display_name column it is stored in. Behaviour is unchanged: the adapter still rejects a malformed provider response and a guild member response for another user, both of which depend on the snowflake assertion that moved. Co-Authored-By: Claude Opus 4.8 --- .../src/api_v0/message/reactions.test.ts | 12 +-- community/src/api_v0/message/reactions.ts | 8 +- .../api_v0/user/me/identity/service.test.ts | 4 +- .../src/api_v0/user/me/identity/service.ts | 16 ++-- community/src/lib/community/discord/main.ts | 10 +-- .../src/lib/community/discord/message.test.ts | 2 +- .../src/lib/community/discord/message.ts | 31 ++++---- .../src/lib/community/discord/oauth.test.ts | 2 +- community/src/lib/community/discord/oauth.ts | 9 ++- community/src/lib/community/discord/role.ts | 17 ++--- community/src/lib/community/discord/schema.ts | 50 +++++++++++++ community/src/lib/community/interface.ts | 20 ++--- community/src/lib/community/type.ts | 74 +++++++++++-------- 13 files changed, 161 insertions(+), 94 deletions(-) create mode 100644 community/src/lib/community/discord/schema.ts diff --git a/community/src/api_v0/message/reactions.test.ts b/community/src/api_v0/message/reactions.test.ts index 6ca0587..7130d47 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 }, ], }, ]; diff --git a/community/src/api_v0/message/reactions.ts b/community/src/api_v0/message/reactions.ts index 588f68e..b3f1f37 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,7 @@ export type LinkedReactionUser = { export type ReactionUsersByEmoji = { emoji: string; count: number; - users: DiscordReactionUser[]; + users: CommunityReactionUser[]; }; export type ReactionMember = { @@ -76,7 +76,7 @@ export const collectDiscordUserIds = (reactionUsersByEmoji: ReactionUsersByEmoji export const buildMessageReactionSummary = ( eventMessage: EventMessageRecord, - discordMessage: DiscordMessage, + discordMessage: CommunityMessage, reactionUsersByEmoji: ReactionUsersByEmoji[], linkedUsers: LinkedReactionUser[], ) => { @@ -91,7 +91,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, 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/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; From f2650c9bc0b786efd4d1c46ca8faa4aeb4ec1803 Mon Sep 17 00:00:00 2001 From: yufoxda Date: Mon, 27 Jul 2026 15:33:49 +0900 Subject: [PATCH 2/5] fix(message): stop repeating private member fields in reaction badges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reaction summary returned the complete member record — student ID, student email, emergency contact, insurance and allergy details — inside every reaction's user list as well as in `members`. A member who reacted with three emoji had those fields serialised four times, so the private data on the wire grew with the number of reactions rather than the number of members. The badges only ever rendered names: the client maps that list through getDisplayName and reads nothing else from it. They now carry a ReactionParticipant with the identity and name fields, while `members` keeps the full record the admin table and its CSV export need, including the emergency contact and allergy details an organiser relies on. Adds a regression test asserting no private field appears in the badge payload. Co-Authored-By: Claude Opus 4.8 --- .../src/api_v0/message/reactions.test.ts | 68 ++++++++----------- community/src/api_v0/message/reactions.ts | 21 +++++- community/src/api_v0/message/schema.ts | 13 +++- .../src/app/event/[id]/detailUtils.test.ts | 43 +----------- frontend/src/app/event/[id]/detailUtils.ts | 12 +++- 5 files changed, 72 insertions(+), 85 deletions(-) diff --git a/community/src/api_v0/message/reactions.test.ts b/community/src/api_v0/message/reactions.test.ts index 7130d47..f32a7bf 100644 --- a/community/src/api_v0/message/reactions.test.ts +++ b/community/src/api_v0/message/reactions.test.ts @@ -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 b3f1f37..34f5707 100644 --- a/community/src/api_v0/message/reactions.ts +++ b/community/src/api_v0/message/reactions.ts @@ -41,6 +41,17 @@ export type ReactionUsersByEmoji = { 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 = { discordUserId: string; discordUsername: string; @@ -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/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; From a7e6ef8af7e31194275aa241b620e14ec37fd98f Mon Sep 17 00:00:00 2001 From: yufoxda Date: Mon, 27 Jul 2026 16:15:48 +0900 Subject: [PATCH 3/5] refactor(db): move the authentication store into its own schema Better Auth's tables move to app_auth, and every reference that crossed between them and the domain is removed, so the authentication store can be lifted into its own database without touching a domain table. Moving the tables alone would not have achieved that: a foreign key cannot span databases, and three of them crossed this boundary, one of which only surfaced when the new assertion ran. The domain now keeps its own account record in public.app_accounts, holding the membership link and the application role. role has to live on this side because the membership trigger authorizes against it, and a trigger cannot read another database. user_id is stored as a value rather than a foreign key; the reviewer and community identity references become snapshots too, matching member_status_history, which already recorded its actor that way. Losing those foreign keys costs the guarantee that a reviewed user cannot be deleted. That is the price of the move, and it is now stated in the schema comments and asserted in the pgTAP suite rather than left implicit. The auth middleware is the only place that reads both sides. It resolves the subject from the authentication store and the account from the domain in two separate queries, so a future split turns the first into a remote call and leaves the second untouched. The admin views that display an account email do the same thing in batch instead of joining. A replay test asserts the tables sit in app_auth, that "user" carries no domain column, and that no foreign key crosses the boundary in either direction. Co-Authored-By: Claude Opus 4.8 --- community/src/api_v0/message/service.ts | 34 ++- community/src/api_v0/user/me/service.ts | 9 +- community/src/auth/better-auth.test.ts | 10 +- community/src/auth/better-auth.ts | 35 ++- community/src/core/auth.test.ts | 25 +- community/src/core/auth.ts | 84 ++++-- .../api_v0/member/join.integration.test.ts | 69 +++-- member/src/api_v0/member/service.ts | 83 ++++-- member/src/core/auth.ts | 84 ++++-- member/src/core/migration.integration.test.ts | 60 ++++ share/drizzle/relations.ts | 45 ++- share/drizzle/schema.ts | 66 +++-- .../20260727000000_split_auth_schema.sql | 267 ++++++++++++++++++ supabase/runbooks/promote_initial_admin.sql | 5 +- .../runbooks/provision_app_runtime_login.sql | 7 + .../purge_confirmed_test_membership_data.sql | 5 + ...01_reconcile_better_auth_baseline.test.sql | 28 +- .../tests/002_membership_foundation.test.sql | 37 ++- supabase/tests/003_membership_rls.test.sql | 27 +- 19 files changed, 729 insertions(+), 251 deletions(-) create mode 100644 supabase/migrations/20260727000000_split_auth_schema.sql 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/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..23d4598 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,20 @@ 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); 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/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..1e5b761 100644 --- a/supabase/tests/003_membership_rls.test.sql +++ b/supabase/tests/003_membership_rls.test.sql @@ -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$ $$, @@ -329,14 +334,14 @@ select set_config('app.current_user_role', 'admin', true); select throws_ok( - $$delete from public."user" where id = 'test-admin'$$, + $$delete from app_auth."user" where id = 'test-admin'$$, '23503', null, 'a reviewer account cannot be deleted while reviewed memberships reference it' ); 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' ); From 6bc0b0f3f2d7949360cfb9008e08bac6f6977811 Mon Sep 17 00:00:00 2001 From: yufoxda Date: Mon, 27 Jul 2026 16:30:56 +0900 Subject: [PATCH 4/5] fix(db): assert the reviewer snapshot instead of the dropped foreign key The RLS suite still required a reviewed account to be undeletable, which was the guarantee the dropped foreign key provided. That key crossed into the authentication store, so the assertion now states what replaced it: the delete succeeds and the review record keeps the reviewer it was written with. Caught by the pgTAP job, which the membership work added and which is the only thing that executes these suites. Co-Authored-By: Claude Opus 4.8 --- supabase/tests/003_membership_rls.test.sql | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/supabase/tests/003_membership_rls.test.sql b/supabase/tests/003_membership_rls.test.sql index 1e5b761..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. @@ -333,11 +333,22 @@ select set_config('app.current_member_id', '', true), set_config('app.current_user_role', 'admin', true); -select throws_ok( +-- 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'$$, - '23503', - null, - 'a reviewer account cannot be deleted while reviewed memberships reference it' + '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( From 2e778236c5c6ca8e683a7f73fb4710865a250ddd Mon Sep 17 00:00:00 2001 From: yufoxda Date: Mon, 27 Jul 2026 16:40:56 +0900 Subject: [PATCH 5/5] fix(auth): repair a missing domain account at sign-in Sign-in now requires a public.app_accounts row, which the Better Auth create hook provisions. If that hook ever fails, or a subject is seeded outside the flow, the account exists in the authentication store with nothing on the domain side, and the JWT endpoint and the auth middleware both reject it. The user is then locked out permanently with no path back except editing the database. The sign-in path now inserts the row when it is absent, which costs one statement per sign-in and makes the missing row self-correcting. Co-Authored-By: Claude Opus 4.8 --- community/src/auth/better-auth.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/community/src/auth/better-auth.ts b/community/src/auth/better-auth.ts index 23d4598..0f7e8a9 100644 --- a/community/src/auth/better-auth.ts +++ b/community/src/auth/better-auth.ts @@ -112,6 +112,16 @@ export const getJwtHandler = async (c: Context) => { // 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.appAccounts.role,