Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions apps/api/src/__tests__/messaging-channels-permissions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { describe, it, expect } from 'vitest';
import type { Kysely } from 'kysely';
import type { Database } from '@vencore/db';
import type { RequestHandler } from 'express';
import { createChannelsRouter } from '../routes/messaging/channels';
import { MESSAGING_MODULE } from '@vencore/modules';

/**
* Tags each gate handler with the permission it was built from, so the assertions
* can read the gate off the router without standing up Express + a database.
*/
function taggingRequirePermission(permission: string): RequestHandler {
const handler = (() => undefined) as unknown as RequestHandler;
(handler as unknown as { __permission: string }).__permission = permission;
return handler;
}

function permissionsForRoute(router: unknown, method: string, path: string): string[] {
const stack = (router as { stack: unknown[] }).stack;
const perms: string[] = [];
for (const layer of stack as Array<{
route?: { path: string; methods: Record<string, boolean>; stack: Array<{ handle: { __permission?: string } }> };
}>) {
if (!layer.route) continue;
if (layer.route.path !== path) continue;
if (!layer.route.methods[method]) continue;
for (const h of layer.route.stack) {
const p = h.handle.__permission;
if (p) perms.push(p);
}
}
return perms;
}

describe('messaging channels route permissions', () => {
const db = {} as Kysely<Database>;

it('gates channel creation on create_channel, not the admin-only manage', () => {
const router = createChannelsRouter(db, taggingRequirePermission);

const perms = permissionsForRoute(router, 'post', '/');

expect(perms).toContain('messaging:create_channel');
expect(perms).not.toContain('messaging:manage');
});

it('still gates rename and archive on manage', () => {
const router = createChannelsRouter(db, taggingRequirePermission);

expect(permissionsForRoute(router, 'patch', '/:id')).toContain('messaging:manage');
expect(permissionsForRoute(router, 'delete', '/:id')).toContain('messaging:manage');
});

it('keeps listing and reading channels on view', () => {
const router = createChannelsRouter(db, taggingRequirePermission);

expect(permissionsForRoute(router, 'get', '/')).toContain('messaging:view');
expect(permissionsForRoute(router, 'get', '/:id')).toContain('messaging:view');
});
});

describe('messaging module permission definitions', () => {
const byKey = new Map(MESSAGING_MODULE.permissions.map(p => [p.key, p]));

it('declares messaging:create_channel', () => {
expect(byKey.has('messaging:create_channel')).toBe(true);
});

it('grants channel creation to members, so they are not locked out', () => {
expect(byKey.get('messaging:create_channel')?.defaultRoles).toContain('member');
expect(byKey.get('messaging:create_channel')?.defaultRoles).toContain('admin');
});

it('keeps messaging:manage admin-only', () => {
expect(byKey.get('messaging:manage')?.defaultRoles).toEqual(['admin']);
});
});
26 changes: 24 additions & 2 deletions apps/api/src/routes/messaging/channels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,18 +141,40 @@ export function createChannelsRouter(
}
}

// DM rows are all stored as name='dm'; the display name is built from the
// participants. Without members here the sidebar renders every DM
// identically, so attach them for dm/group_dm rows only.
const dmIds = rows.filter(r => r.type === 'dm' || r.type === 'group_dm').map(r => r.id);
const dmMembers = dmIds.length
? await db
.selectFrom('channel_members')
.innerJoin('users', 'users.id', 'channel_members.user_id')
.where('channel_members.channel_id', 'in', dmIds)
.select(['channel_members.channel_id', 'channel_members.user_id', 'users.name', 'users.email'])
.execute()
: [];

const membersByChannel = new Map<string, { user_id: string; name: string; email: string }[]>();
for (const m of dmMembers) {
const list = membersByChannel.get(m.channel_id) ?? [];
list.push({ user_id: m.user_id, name: m.name, email: m.email });
membersByChannel.set(m.channel_id, list);
}

const channels = rows.map(r => ({
...r,
unread_count: unreadCounts[r.id] ?? 0,
last_message: latestMap.get(r.id) ?? null,
...(membersByChannel.has(r.id) ? { members: membersByChannel.get(r.id) } : {}),
}));

res.json({ data: channels, error: null });
} catch (err) { next(err); }
});

// Create channel
router.post('/', requirePermission('messaging:manage'), async (req, res, next) => {
// Create channel. Gated on create_channel, not manage — 'manage' is admin-only
// and would stop ordinary members from ever creating a channel.
router.post('/', requirePermission('messaging:create_channel'), async (req, res, next) => {
try {
const { workspace, user } = req as unknown as AuthenticatedRequest;
const body = createChannelSchema.parse(req.body);
Expand Down
2 changes: 1 addition & 1 deletion apps/api/src/routes/tasks-unified.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export function createUnifiedTasksRouter(db: Kysely<Database>, requirePermission
return
}
const { status, source, priority, show_all, q, owner_id } = parsed.data
const showAll = show_all && user.role === 'admin'
const showAll = show_all && isAdmin

// ── 1. CRM tasks ────────────────────────────────────────────────────────
let crmQ = db
Expand Down
16 changes: 13 additions & 3 deletions apps/web/modules/messaging/components/ChannelSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,20 @@ import { ContextMenu, useContextMenu, type ContextMenuItem } from '@/modules/sha
import { useConfirm } from '@/modules/shared/components/ui/ConfirmDialog';
import { useApiToken } from '@/modules/shared/lib/useApiToken';
import { listChannels, createChannel, updateChannel, archiveChannel, markChannelRead } from '../lib/messaging';
import { channelDisplayName, type ChannelMemberSummary } from '../lib/dm-name';
import { useAuth } from '@/modules/shared/lib/AuthContext';
import { NewDMModal } from './NewDMModal';
import type { Channel, Message } from '@vencore/types';

type ChannelWithMeta = Channel & { unread_count: number; last_message: Message | null };
type ChannelWithMeta = Channel & {
unread_count: number;
last_message: Message | null;
members?: ChannelMemberSummary[];
};

export function ChannelSidebar() {
const getToken = useApiToken();
const { user } = useAuth();
const router = useRouter();
const params = useParams();
const activeId = params?.['channelId'] as string | undefined;
Expand Down Expand Up @@ -163,6 +170,7 @@ export function ChannelSidebar() {
active={activeId === ch.id}
onClick={() => router.push(`/messaging/${ch.id}`)}
onMarkRead={ch.last_message ? () => markRead.mutate({ id: ch.id, messageId: ch.last_message!.id }) : undefined}
displayName={channelDisplayName(ch, user?.id ?? '')}
isDm
/>
))}
Expand Down Expand Up @@ -232,12 +240,14 @@ function SectionLabel({ label }: { label: string }) {
}

function ChannelRow({
channel, active, onClick, isDm, onOpenDetails, onRename, onArchive, onMarkRead,
channel, active, onClick, isDm, displayName, onOpenDetails, onRename, onArchive, onMarkRead,
}: {
channel: ChannelWithMeta;
active: boolean;
onClick: () => void;
isDm?: boolean;
/** DMs are stored as name='dm'; the row shows the other participant instead. */
displayName?: string;
onOpenDetails?: () => void;
onRename?: (name: string) => void;
onArchive?: () => void;
Expand Down Expand Up @@ -323,7 +333,7 @@ function ChannelRow({
{isDm ? '●' : channel.is_private ? <Icon name="lock" size={13} /> : '#'}
</span>
<span style={{ flex: 1, overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap' }}>
{channel.name}
{displayName ?? channel.name}
</span>
{hasUnread && !active && (
<span style={{
Expand Down
5 changes: 3 additions & 2 deletions apps/web/modules/messaging/components/ChannelView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { ChannelSettings } from './ChannelSettings';
import { useChat } from '../hooks/useChat';
import { useApiToken } from '@/modules/shared/lib/useApiToken';
import { getChannel, type PendingAttachment } from '../lib/messaging';
import { channelDisplayName } from '../lib/dm-name';
import { useAuth } from '@/modules/shared/lib/AuthContext';
import type { Message } from '@vencore/types';

Expand Down Expand Up @@ -84,7 +85,7 @@ export function ChannelView({ channelId }: Props) {
{isDm ? '●' : channelData?.is_private ? <Icon name="lock" size={14} /> : '#'}
</span>
<span style={{ fontSize: 14, fontWeight: 600, color: 'var(--text)' }}>
{channelData?.name ?? '…'}
{channelData ? channelDisplayName(channelData, user?.id ?? '') : '…'}
</span>
{channelData?.topic && (
<>
Expand Down Expand Up @@ -131,7 +132,7 @@ export function ChannelView({ channelId }: Props) {
<MessageInput
onSend={handleSend}
onTyping={sendTyping}
placeholder={`Message ${channelData?.name ? (isDm ? channelData.name : `#${channelData.name}`) : '…'}`}
placeholder={`Message ${channelData ? (isDm ? channelDisplayName(channelData, user?.id ?? '') : `#${channelData.name}`) : '…'}`}
disabled={!wsReady && messages.length === 0}
/>
</div>
Expand Down
61 changes: 61 additions & 0 deletions apps/web/modules/messaging/lib/dm-name.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { describe, it, expect } from 'vitest';
import { channelDisplayName } from './dm-name';

const ME = 'user-me';

describe('channelDisplayName', () => {
it('returns the stored name for a regular channel', () => {
expect(channelDisplayName({ name: 'general', type: 'channel' }, ME)).toBe('general');
});

it('ignores members on a regular channel', () => {
const channel = {
name: 'engineering',
type: 'channel',
members: [{ user_id: 'user-other', name: 'Admin' }],
};
expect(channelDisplayName(channel, ME)).toBe('engineering');
});

it('names a DM after the other participant, not "dm"', () => {
const channel = {
name: 'dm',
type: 'dm',
members: [
{ user_id: ME, name: 'Claude Test' },
{ user_id: 'user-other', name: 'Admin' },
],
};
expect(channelDisplayName(channel, ME)).toBe('Admin');
});

it('joins all other participants for a group DM', () => {
const channel = {
name: 'dm',
type: 'group_dm',
members: [
{ user_id: ME, name: 'Claude Test' },
{ user_id: 'u2', name: 'Admin' },
{ user_id: 'u3', name: 'Dana' },
],
};
expect(channelDisplayName(channel, ME)).toBe('Admin, Dana');
});

it('falls back to a readable label when members are not loaded', () => {
expect(channelDisplayName({ name: 'dm', type: 'dm' }, ME)).toBe('Direct message');
});
Comment on lines +45 to +47

it('falls back when the only member is yourself', () => {
const channel = {
name: 'dm',
type: 'dm',
members: [{ user_id: ME, name: 'Claude Test' }],
};
expect(channelDisplayName(channel, ME)).toBe('Direct message');
});

it('returns empty string for a missing channel', () => {
expect(channelDisplayName(null, ME)).toBe('');
});
});
33 changes: 33 additions & 0 deletions apps/web/modules/messaging/lib/dm-name.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
export interface ChannelMemberSummary {
user_id: string;
name: string;
email?: string;
}

export interface NameableChannel {
name: string;
type?: string | null;
members?: ChannelMemberSummary[] | undefined;
}

/**
* DM channels are all stored with name='dm' — the display name is derived from
* the other participants. Regular channels keep their stored name.
*/
export function channelDisplayName(
channel: NameableChannel | null | undefined,
currentUserId: string,
): string {
if (!channel) return '';

const isDm = channel.type === 'dm' || channel.type === 'group_dm';
if (!isDm) return channel.name;

const others = (channel.members ?? []).filter(m => m.user_id !== currentUserId);
Comment on lines +23 to +26
if (others.length === 0) {
// Members not loaded yet, or a DM with only yourself left in it.
return channel.name === 'dm' ? 'Direct message' : channel.name;
}

return others.map(m => m.name).join(', ');
}
4 changes: 4 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ services:

api:
image: ${API_IMAGE:-ghcr.io/vencorehq/vencore-api:latest}
# Published so a locally-run `pnpm dev` web server can reach the API through
# the Next.js rewrite (and so WS upgrades can bypass Next entirely).
ports:
- "127.0.0.1:3001:3001"
env_file: .env
environment:
DATABASE_URL: postgresql://${DB_USER:-vencore}:${DB_PASSWORD:-vencore}@db:5432/${DB_NAME:-vencore}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { type Kysely, sql } from 'kysely';

// Adding a permission to a ModuleDefinition only reaches NEW workspaces:
// seedWorkspaceRoles grants module defaults when a workspace's Member role is
// first created and never runs again. Existing workspaces therefore need an
// explicit backfill, or members stay locked out of channel creation.
//
// Backfill rule: any role that can already send messages can create channels.
// Roles with grants_all are unaffected — they short-circuit to superuser and
// never consult role_permissions.
export async function up(db: Kysely<unknown>): Promise<void> {
await sql`
insert into role_permissions (workspace_id, role_id, permission)
select workspace_id, role_id, 'messaging:create_channel'
from role_permissions
where permission = 'messaging:send'
on conflict (role_id, permission) do nothing
`.execute(db);
}

export async function down(db: Kysely<unknown>): Promise<void> {
await sql`
delete from role_permissions where permission = 'messaging:create_channel'
`.execute(db);
}
Comment on lines +21 to +25
10 changes: 7 additions & 3 deletions packages/modules/src/messaging/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ export const MESSAGING_MODULE: ModuleDefinition = {
icon: 'MessageSquare',
defaultEnabled: true,
permissions: [
{ key: 'messaging:view', label: 'View channels and messages', defaultRoles: ['admin', 'member'] },
{ key: 'messaging:send', label: 'Send messages and upload files', defaultRoles: ['admin', 'member'] },
{ key: 'messaging:manage', label: 'Manage channels and delete messages', defaultRoles: ['admin'] },
{ key: 'messaging:view', label: 'View channels and messages', defaultRoles: ['admin', 'member'] },
{ key: 'messaging:send', label: 'Send messages and upload files', defaultRoles: ['admin', 'member'] },
// Creating a channel is a normal member action; renaming/archiving someone
// else's channel is not. Keeping them separate means members are not locked
// out of creating channels by the admin-only 'manage' gate.
{ key: 'messaging:create_channel', label: 'Create channels', defaultRoles: ['admin', 'member'] },
{ key: 'messaging:manage', label: 'Manage channels and delete messages', defaultRoles: ['admin'] },
],
nav: [{ label: 'Messaging', path: '/messaging', icon: 'MessageSquare' }],
apiPrefixes: ['/messaging'],
Expand Down
Loading