From 0a157de1628ed714efd3b5a55c7e3d2abdcd7289 Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Wed, 25 Feb 2026 22:46:50 +0100 Subject: [PATCH] refactor: purge legacy server_members.role column, single source of truth via member_roles Remove the legacy TEXT role column ('owner'/'admin'/'member') from server_members and make the bitwise RBAC member_roles junction table the sole authority for role assignments. Owner detection now uses servers.ownerId exclusively. - Remove MemberRole type and role field from shared types - Remove role from Drizzle schema, raw SQL CREATE TABLE, and seed data - Rewrite PATCH /members/:uid to accept { roleIds: string[] } - Fix GET /members to populate roles array (was TODO) - Replace member.role === 'owner' guard with isServerOwner() - Remove getMemberRole() helper and legacy bridge code - MemberSidebar groups by highest-positioned role instead of legacy string - ServerSettings replaces admin/member dropdown with role checkboxes - Message.tsx derives color from roles[] with owner fallback via ownerId - Existing DBs keep vestigial column (Drizzle ignores it); new DBs omit it --- packages/server/src/db/index.ts | 1 - packages/server/src/db/schema.ts | 1 - packages/server/src/db/seed.ts | 1 - packages/server/src/routes/servers.ts | 153 ++++++++++------- packages/server/src/utils/permissions.ts | 6 - packages/server/src/ws/handler.ts | 1 - packages/shared/src/types.ts | 5 +- packages/web/src/components/chat/Message.tsx | 26 ++- .../src/components/layout/MemberSidebar.tsx | 88 +++++++--- .../src/components/modals/ServerSettings.tsx | 154 ++++++++++++++---- 10 files changed, 295 insertions(+), 141 deletions(-) diff --git a/packages/server/src/db/index.ts b/packages/server/src/db/index.ts index d7be0100..8a2e8c6e 100644 --- a/packages/server/src/db/index.ts +++ b/packages/server/src/db/index.ts @@ -38,7 +38,6 @@ function createTables(db: Database.Database): void { CREATE TABLE IF NOT EXISTS server_members ( server_id TEXT NOT NULL REFERENCES servers(id) ON DELETE CASCADE, user_id TEXT NOT NULL REFERENCES users(id) ON DELETE CASCADE, - role TEXT DEFAULT 'member', nickname TEXT, joined_at INTEGER NOT NULL, PRIMARY KEY (server_id, user_id) diff --git a/packages/server/src/db/schema.ts b/packages/server/src/db/schema.ts index f7cc6603..63eaa17a 100644 --- a/packages/server/src/db/schema.ts +++ b/packages/server/src/db/schema.ts @@ -23,7 +23,6 @@ export const servers = sqliteTable('servers', { export const serverMembers = sqliteTable('server_members', { serverId: text('server_id').notNull().references(() => servers.id, { onDelete: 'cascade' }), userId: text('user_id').notNull().references(() => users.id, { onDelete: 'cascade' }), - role: text('role').default('member'), nickname: text('nickname'), joinedAt: integer('joined_at').notNull(), }, (table) => ({ diff --git a/packages/server/src/db/seed.ts b/packages/server/src/db/seed.ts index 3a4c337d..5a41e810 100644 --- a/packages/server/src/db/seed.ts +++ b/packages/server/src/db/seed.ts @@ -39,7 +39,6 @@ export async function seedDatabase(): Promise { db.insert(schema.serverMembers).values({ serverId: serverId, userId: adminId, - role: 'owner', joinedAt: Date.now(), }).run(); diff --git a/packages/server/src/routes/servers.ts b/packages/server/src/routes/servers.ts index 25d88e40..38904beb 100644 --- a/packages/server/src/routes/servers.ts +++ b/packages/server/src/routes/servers.ts @@ -95,7 +95,6 @@ export async function serverRoutes(app: FastifyInstance): Promise { tx.insert(schema.serverMembers).values({ serverId, userId: request.userId, - role: 'owner', joinedAt: now, }).run(); @@ -219,7 +218,6 @@ export async function serverRoutes(app: FastifyInstance): Promise { return { serverId: m.serverId, userId: m.userId, - role: (m.role ?? 'member') as MemberWithUser['role'], nickname: m.nickname, joinedAt: m.joinedAt, user: sanitizeUser(user), @@ -387,7 +385,6 @@ export async function serverRoutes(app: FastifyInstance): Promise { db.insert(schema.serverMembers).values({ serverId: id, userId: request.userId, - role: 'member', joinedAt: now, }).run(); @@ -422,7 +419,6 @@ export async function serverRoutes(app: FastifyInstance): Promise { db.insert(schema.serverMembers).values({ serverId: server.id, userId: request.userId, - role: 'member', joinedAt: now, }).run(); @@ -460,18 +456,44 @@ export async function serverRoutes(app: FastifyInstance): Promise { const userMap = new Map(users.map(u => [u.id, u])); + const roles = db.select() + .from(schema.roles) + .where(eq(schema.roles.serverId, id)) + .orderBy(schema.roles.position) + .all(); + + const memberRoleRows = db.select() + .from(schema.memberRoles) + .where(eq(schema.memberRoles.serverId, id)) + .all(); + const members: MemberWithUser[] = memberRows .map(m => { const user = userMap.get(m.userId); if (!user) return null; + + const assignedRoleIds = memberRoleRows + .filter(mr => mr.userId === m.userId) + .map(mr => mr.roleId); + + const assignedRoles = roles + .filter(r => assignedRoleIds.includes(r.id)) + .map(r => ({ + id: r.id, + serverId: r.serverId, + name: r.name, + color: r.color ?? '#b9bbbe', + position: r.position ?? 0, + createdAt: r.createdAt, + })); + return { serverId: m.serverId, userId: m.userId, - role: (m.role ?? 'member') as MemberWithUser['role'], nickname: m.nickname, joinedAt: m.joinedAt, user: sanitizeUser(user), - roles: [] as Role[], // TODO: Fetch member roles + roles: assignedRoles, }; }) .filter((m): m is MemberWithUser => m !== null); @@ -479,12 +501,12 @@ export async function serverRoutes(app: FastifyInstance): Promise { return reply.code(200).send(members); }); - // PATCH /api/servers/:id/members/:uid - Update member role (owner only) + // PATCH /api/servers/:id/members/:uid - Update member roles app.patch<{ Params: { id: string; uid: string }; Body: UpdateMemberRequest }>('/api/servers/:id/members/:uid', { preHandler: authenticate, }, async (request, reply) => { const { id, uid } = request.params; - const { role } = request.body; + const { roleIds } = request.body; const db = getDb(); const server = db.select().from(schema.servers).where(eq(schema.servers.id, id)).get(); @@ -497,11 +519,16 @@ export async function serverRoutes(app: FastifyInstance): Promise { } if (uid === request.userId) { - return reply.code(400).send({ error: 'You cannot change your own role', statusCode: 400 }); + return reply.code(400).send({ error: 'You cannot change your own roles', statusCode: 400 }); } - if (!role || !['admin', 'member'].includes(role)) { - return reply.code(400).send({ error: 'Role must be "admin" or "member"', statusCode: 400 }); + if (!Array.isArray(roleIds)) { + return reply.code(400).send({ error: 'roleIds must be an array of role IDs', statusCode: 400 }); + } + + // Cannot modify the server owner's roles unless you are the owner + if (isServerOwner(id, uid) && !isServerOwner(id, request.userId)) { + return reply.code(403).send({ error: 'Only the server owner can modify their own roles', statusCode: 403 }); } const member = db.select() @@ -516,62 +543,49 @@ export async function serverRoutes(app: FastifyInstance): Promise { return reply.code(404).send({ error: 'Member not found', statusCode: 404 }); } - db.update(schema.serverMembers) - .set({ role }) - .where(and( - eq(schema.serverMembers.serverId, id), - eq(schema.serverMembers.userId, uid), - )) - .run(); + // Validate all roleIds belong to this server and are not @everyone + if (roleIds.length > 0) { + const serverRoles = db.select() + .from(schema.roles) + .where(eq(schema.roles.serverId, id)) + .all(); - // Bridge legacy role string to bitwise member_roles - if (role === 'admin') { - // Find or create Admin role for this server (matches migrate.ts convention) - const adminPerms = permissionsToString(ALL_PERMISSIONS); - let adminRole = db.select().from(schema.roles) - .where(and( - eq(schema.roles.serverId, id), - eq(schema.roles.name, 'Admin'), - eq(schema.roles.permissions, adminPerms), - )) - .get(); + const serverRoleIds = new Set(serverRoles.map(r => r.id)); - if (!adminRole) { - const adminRoleId = `${id}-admin`; - db.insert(schema.roles).values({ - id: adminRoleId, - serverId: id, - name: 'Admin', - color: '#e74c3c', - position: 1, - permissions: adminPerms, - createdAt: Date.now(), - }).run(); - adminRole = db.select().from(schema.roles).where(eq(schema.roles.id, adminRoleId)).get(); + for (const roleId of roleIds) { + if (!serverRoleIds.has(roleId)) { + return reply.code(400).send({ error: `Role ${roleId} does not belong to this server`, statusCode: 400 }); + } + if (roleId === id) { + return reply.code(400).send({ error: '@everyone role is implicit and cannot be assigned', statusCode: 400 }); + } } + } - if (adminRole) { - // Assign the Admin role (no-op if already assigned) - db.insert(schema.memberRoles).values({ - serverId: id, - userId: uid, - roleId: adminRole.id, - }).onConflictDoNothing().run(); - } - } else if (role === 'member') { - // Remove all explicit role assignments (demote to @everyone only) - // @everyone is implicit via computePermissions, never stored in member_roles - db.delete(schema.memberRoles) + // Atomically replace member's role assignments + db.transaction((tx) => { + // Remove all existing role assignments for this member in this server + tx.delete(schema.memberRoles) .where(and( eq(schema.memberRoles.serverId, id), eq(schema.memberRoles.userId, uid), )) .run(); - } + + // Insert new role assignments + for (const roleId of roleIds) { + tx.insert(schema.memberRoles).values({ + serverId: id, + userId: uid, + roleId, + }).run(); + } + }); // Force target user's client to re-sync with their new permissions connectionManager.pushReadyPayload(uid); + // Build response with populated roles const updatedMember = db.select() .from(schema.serverMembers) .where(and( @@ -589,14 +603,39 @@ export async function serverRoutes(app: FastifyInstance): Promise { return reply.code(500).send({ error: 'User not found', statusCode: 500 }); } + const updatedRoleRows = db.select() + .from(schema.memberRoles) + .where(and( + eq(schema.memberRoles.serverId, id), + eq(schema.memberRoles.userId, uid), + )) + .all(); + + const updatedRoleIds = updatedRoleRows.map(r => r.roleId); + const allRoles = db.select() + .from(schema.roles) + .where(eq(schema.roles.serverId, id)) + .orderBy(schema.roles.position) + .all(); + + const memberRoles = allRoles + .filter(r => updatedRoleIds.includes(r.id)) + .map(r => ({ + id: r.id, + serverId: r.serverId, + name: r.name, + color: r.color ?? '#b9bbbe', + position: r.position ?? 0, + createdAt: r.createdAt, + })); + const result: MemberWithUser = { serverId: updatedMember.serverId, userId: updatedMember.userId, - role: (updatedMember.role ?? 'member') as MemberWithUser['role'], nickname: updatedMember.nickname, joinedAt: updatedMember.joinedAt, user: sanitizeUser(user), - roles: [] as Role[], // TODO: Fetch member roles + roles: memberRoles, }; return reply.code(200).send(result); @@ -640,7 +679,7 @@ export async function serverRoutes(app: FastifyInstance): Promise { } // Cannot kick the owner - if (member.role === 'owner') { + if (isServerOwner(id, uid)) { return reply.code(400).send({ error: 'Cannot remove the server owner', statusCode: 400 }); } diff --git a/packages/server/src/utils/permissions.ts b/packages/server/src/utils/permissions.ts index fc45d920..62230866 100644 --- a/packages/server/src/utils/permissions.ts +++ b/packages/server/src/utils/permissions.ts @@ -1,6 +1,5 @@ import { eq, and } from 'drizzle-orm'; import { getDb, schema } from '../db/index.js'; -import type { MemberRole } from '@opencord/shared'; import { PermissionBits, ALL_PERMISSIONS, @@ -134,11 +133,6 @@ export function isMember(serverId: string, userId: string): boolean { return getMember(serverId, userId) !== undefined; } -export function getMemberRole(serverId: string, userId: string): MemberRole | null { - const member = getMember(serverId, userId); - return member ? (member.role as MemberRole) : null; -} - export function isServerOwner(serverId: string, userId: string): boolean { const db = getDb(); const server = db.select().from(schema.servers).where(eq(schema.servers.id, serverId)).get(); diff --git a/packages/server/src/ws/handler.ts b/packages/server/src/ws/handler.ts index 344913cb..21a4b66a 100644 --- a/packages/server/src/ws/handler.ts +++ b/packages/server/src/ws/handler.ts @@ -651,7 +651,6 @@ function buildReadyPayload(userId: string): { return { serverId: m.serverId, userId: m.userId, - role: (m.role ?? 'member') as MemberWithUser['role'], nickname: m.nickname, joinedAt: m.joinedAt, user: sanitizeUser(u), diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index 77783076..47b3b798 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -36,12 +36,9 @@ export interface ServerWithChannelsAndMembers extends Server { // ─── Member Types ─────────────────────────────────────────────────────────── -export type MemberRole = 'owner' | 'admin' | 'member'; - export interface Member { serverId: string; userId: string; - role: MemberRole; nickname: string | null; joinedAt: number; } @@ -288,7 +285,7 @@ export interface UpdateUserRequest { } export interface UpdateMemberRequest { - role: MemberRole; + roleIds: string[]; } export interface CreateMessageRequest { diff --git a/packages/web/src/components/chat/Message.tsx b/packages/web/src/components/chat/Message.tsx index ee19edc3..f01a037b 100644 --- a/packages/web/src/components/chat/Message.tsx +++ b/packages/web/src/components/chat/Message.tsx @@ -123,26 +123,24 @@ export function Message({ message, isCompact, isFirstInGroup }: MessageProps) { const displayName = message.user.displayName ?? message.user.username; - const roleColor = (() => { - const member = members.find(m => m.userId === message.userId); - if (member?.roles && member.roles.length > 0) { - return { color: member.roles[0]!.color }; - } - if (member?.role === 'owner') return { color: '#f23f43' }; - if (member?.role === 'admin') return { color: '#5865f2' }; - return { color: '#dcdcdf' }; - })(); + const servers = useServerStore((s) => s.servers); + const currentServerId = useServerStore((s) => s.currentServerId); + const ownerId = servers.find(s => s.id === currentServerId)?.ownerId; - const replyRoleColor = (msg: any) => { - const member = members.find(m => m.userId === msg.userId); + const getMemberDisplayColor = (userId: string) => { + const member = members.find(m => m.userId === userId); if (member?.roles && member.roles.length > 0) { - return { color: member.roles[0]!.color }; + const sorted = [...member.roles].sort((a, b) => b.position - a.position); + return { color: sorted[0]!.color }; } - if (member?.role === 'owner') return { color: '#f23f43' }; - if (member?.role === 'admin') return { color: '#5865f2' }; + if (ownerId && userId === ownerId) return { color: '#f23f43' }; return { color: '#dcdcdf' }; }; + const roleColor = getMemberDisplayColor(message.userId); + + const replyRoleColor = (msg: { userId: string }) => getMemberDisplayColor(msg.userId); + const content = (
= { owner: 0, admin: 1, member: 2 }; -const ROLE_LABELS: Record = { owner: 'OWNER', admin: 'ADMIN', member: 'MEMBER' }; +/** + * Derives the display group for a member based on their highest-positioned role + * or owner status. Returns { key, label, color, position }. + */ +function getMemberGroup(member: MemberWithUser, ownerId: string | undefined) { + if (ownerId && member.userId === ownerId) { + // Owner always sorts first — position Infinity so it's above all roles + const ownerRole = member.roles?.find(r => r.position > 0); + return { + key: '__owner__', + label: 'OWNER', + color: ownerRole?.color ?? '#f23f43', + position: Infinity, + }; + } + if (member.roles && member.roles.length > 0) { + // Sort by position descending — highest position = most important role + const sorted = [...member.roles].sort((a, b) => b.position - a.position); + const top = sorted[0]!; + return { + key: top.id, + label: top.name.toUpperCase(), + color: top.color, + position: top.position, + }; + } + // No explicit roles — just @everyone + return { + key: '__online__', + label: 'ONLINE', + color: undefined, + position: -1, + }; +} export function MemberSidebar() { const members = useServerStore((s) => s.members); + const servers = useServerStore((s) => s.servers); + const currentServerId = useServerStore((s) => s.currentServerId); const memberListOpen = useUIStore((s) => s.memberListOpen); const openUserProfile = useUIStore((s) => s.openUserProfile); + const server = servers.find(s => s.id === currentServerId); + const ownerId = server?.ownerId; + const { roleGroups, offlineMembers } = useMemo(() => { const online = members.filter(m => m.user.status !== 'offline'); const offline = members.filter(m => m.user.status === 'offline'); - // Group online members by role - const groups = new Map(); + // Group online members by their highest role + const groups = new Map(); for (const m of online) { - const role = m.role || 'member'; - if (!groups.has(role)) groups.set(role, []); - groups.get(role)!.push(m); + const group = getMemberGroup(m, ownerId); + if (!groups.has(group.key)) { + groups.set(group.key, { label: group.label, color: group.color, position: group.position, members: [] }); + } + groups.get(group.key)!.members.push(m); } - // Sort groups by role hierarchy + // Sort groups by position descending (highest role first), then ONLINE last const sorted = [...groups.entries()].sort( - (a, b) => (ROLE_ORDER[a[0]] ?? 99) - (ROLE_ORDER[b[0]] ?? 99) + (a, b) => b[1].position - a[1].position ); return { roleGroups: sorted, offlineMembers: offline }; - }, [members]); + }, [members, ownerId]); if (!memberListOpen) return null; - const roleColors: Record = { - owner: 'text-discord-red', - admin: 'text-discord-blurple', - member: 'text-discord-text-primary', - }; - - const getMemberColor = (member: MemberWithUser) => { + const getMemberColor = (member: MemberWithUser): React.CSSProperties | undefined => { if (member.roles && member.roles.length > 0) { - return { color: member.roles[0]!.color }; + const sorted = [...member.roles].sort((a, b) => b.position - a.position); + return { color: sorted[0]!.color }; + } + if (ownerId && member.userId === ownerId) { + return { color: '#f23f43' }; } return undefined; }; - const handleMemberClick = (e: React.MouseEvent, user: any) => { + const handleMemberClick = (e: React.MouseEvent, user: MemberWithUser['user']) => { e.stopPropagation(); const rect = e.currentTarget.getBoundingClientRect(); openUserProfile(user, { @@ -58,6 +95,7 @@ export function MemberSidebar() { const renderMember = (member: MemberWithUser, isOffline = false) => { const displayName = member.user.displayName ?? member.user.username; + const colorStyle = isOffline ? undefined : getMemberColor(member); return (
{displayName}
@@ -90,12 +128,12 @@ export function MemberSidebar() {
{/* Role-based groups */} - {roleGroups.map(([role, groupMembers]) => ( -
+ {roleGroups.map(([key, group]) => ( +

- {ROLE_LABELS[role] ?? role.toUpperCase()} — {groupMembers.length} + {group.label} — {group.members.length}

- {groupMembers.map((m) => renderMember(m))} + {group.members.map((m) => renderMember(m))}
))} diff --git a/packages/web/src/components/modals/ServerSettings.tsx b/packages/web/src/components/modals/ServerSettings.tsx index dc665724..93315a5d 100644 --- a/packages/web/src/components/modals/ServerSettings.tsx +++ b/packages/web/src/components/modals/ServerSettings.tsx @@ -6,7 +6,6 @@ import { useAuthStore } from '../../stores/authStore'; import { Avatar } from '../ui/Avatar'; import { api } from '../../api/client'; import { useNavigate } from 'react-router-dom'; -import type { MemberRole } from '@opencord/shared'; import { hasPermissionBit, PermissionBits } from '../../utils/permissions'; export function ServerSettingsModal() { @@ -15,6 +14,7 @@ export function ServerSettingsModal() { const currentServerId = useServerStore((s) => s.currentServerId); const servers = useServerStore((s) => s.servers); const members = useServerStore((s) => s.members); + const roles = useServerStore((s) => s.roles); const updateServer = useServerStore((s) => s.updateServer); const deleteServer = useServerStore((s) => s.deleteServer); const loadServerDetail = useServerStore((s) => s.loadServerDetail); @@ -26,6 +26,7 @@ export function ServerSettingsModal() { const [error, setError] = useState(''); const [isLoading, setIsLoading] = useState(false); const [confirmDelete, setConfirmDelete] = useState(false); + const [pendingRoleChanges, setPendingRoleChanges] = useState>>(new Map()); const serverPermissions = useServerStore((s) => s.serverPermissions); @@ -34,6 +35,10 @@ export function ServerSettingsModal() { const isOwnerUser = server?.ownerId === currentUser?.id; const myServerPerms = currentServerId ? serverPermissions.get(currentServerId) : undefined; const canManageServer = hasPermissionBit(myServerPerms, PermissionBits.MANAGE_SERVER); + const canManageRoles = hasPermissionBit(myServerPerms, PermissionBits.MANAGE_ROLES); + + // Assignable roles: exclude @everyone (where role.id === serverId) + const assignableRoles = roles.filter(r => r.id !== currentServerId); React.useEffect(() => { if (server) { @@ -69,15 +74,48 @@ export function ServerSettingsModal() { } }; - const handleRoleChange = async (userId: string, role: MemberRole) => { + const getMemberRoleIds = (member: typeof members[number]): Set => { + // Check for pending (unsaved) changes first + const pending = pendingRoleChanges.get(member.userId); + if (pending) return pending; + return new Set(member.roles?.map(r => r.id) ?? []); + }; + + const handleRoleToggle = (userId: string, roleId: string, currentRoleIds: Set) => { + const updated = new Set(currentRoleIds); + if (updated.has(roleId)) { + updated.delete(roleId); + } else { + updated.add(roleId); + } + setPendingRoleChanges(prev => new Map(prev).set(userId, updated)); + }; + + const handleSaveRoles = async (userId: string) => { + const roleIds = pendingRoleChanges.get(userId); + if (!roleIds) return; + try { - await api.servers.updateMember(currentServerId, userId, { role }); + await api.servers.updateMember(currentServerId, userId, { roleIds: Array.from(roleIds) }); + setPendingRoleChanges(prev => { + const next = new Map(prev); + next.delete(userId); + return next; + }); await loadServerDetail(currentServerId); } catch (err) { - setError(err instanceof Error ? err.message : 'Failed to update role'); + setError(err instanceof Error ? err.message : 'Failed to update roles'); } }; + const handleCancelRoleChange = (userId: string) => { + setPendingRoleChanges(prev => { + const next = new Map(prev); + next.delete(userId); + return next; + }); + }; + const handleKick = async (userId: string) => { try { await api.servers.removeMember(currentServerId, userId); @@ -159,37 +197,91 @@ export function ServerSettingsModal() {
{members.map((member) => { const displayName = member.user.displayName ?? member.user.username; + const isOwner = member.userId === server.ownerId; + const memberRoleIds = getMemberRoleIds(member); + const hasPendingChanges = pendingRoleChanges.has(member.userId); + return ( -
-
- -
-
{displayName}
-
{member.role}
+
+
+
+ +
+
{displayName}
+
+ {isOwner && ( + + Owner + + )} + {member.roles?.filter(r => r.id !== currentServerId).map(r => ( + + {r.name} + + ))} + {!isOwner && (!member.roles || member.roles.filter(r => r.id !== currentServerId).length === 0) && ( + No roles + )} +
+
+ + {canManageRoles && member.userId !== currentUser?.id && !isOwner && ( +
+ +
+ )}
- {isOwnerUser && member.userId !== currentUser?.id && ( -
- - + {/* Role checkboxes — shown for non-self, non-owner members when user can manage roles */} + {canManageRoles && member.userId !== currentUser?.id && !isOwner && assignableRoles.length > 0 && ( +
+ {assignableRoles.map(role => ( + + ))} + {hasPendingChanges && ( +
+ + +
+ )}
)}