fix(permissions): deny space permissions to non-members (invite-bypass)
computePermissions() returned the space @everyone role's permissions without verifying the caller had joined the space. Because CREATE_INVITE is in DEFAULT_EVERYONE_PERMISSIONS, any authenticated user could mint an invite code for a request-only space — whose id is listed by /api/spaces/explore — and then self-join via /api/spaces/:id/join, bypassing the join-request approval flow. The same gap let non-members read message history and search default channels. Root cause: - computePermissions now returns 0n for non-members (space owner and instance admin still short-circuit first, so they are unaffected). Defense in depth (request-only spaces are approval-gated, never invite-joinable): - both invite-code join endpoints reject visibility='request' (private stays invite-joinable — its only entry path; public too). - POST /api/spaces/:id/invite refuses to hand out a code for request spaces. - POST /api/dm/space-invite refuses to card a local request space, checked by space id against the local table so a spoofed spaceInstanceOrigin can't slip past it. - InviteModal hides the invite affordances for request spaces. Also removes the unused computeCategoryPermissions(), which duplicated the resolution algorithm without the membership gate. Adds unit + route + component tests covering non-member/member/owner/admin resolution and the request/private/public visibility matrix. Reported-by: BadAtCaptchas (#2)
This commit is contained in:
Jannis Braun
@@ -0,0 +1,127 @@
|
||||
import { describe, it, expect, beforeEach, vi } from 'vitest';
|
||||
import Database from 'better-sqlite3';
|
||||
import { drizzle } from 'drizzle-orm/better-sqlite3';
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import * as schema from '../db/schema.js';
|
||||
import {
|
||||
PermissionBits,
|
||||
ALL_PERMISSIONS,
|
||||
DEFAULT_EVERYONE_PERMISSIONS,
|
||||
permissionsToString,
|
||||
} from '@backspace/shared/src/permissions.js';
|
||||
|
||||
const __dirname = path.dirname(fileURLToPath(import.meta.url));
|
||||
|
||||
type TestDb = ReturnType<typeof drizzle<typeof schema>>;
|
||||
let sqlite: Database.Database;
|
||||
let testDb: TestDb;
|
||||
|
||||
vi.mock('../db/index.js', () => ({
|
||||
getDb: () => testDb,
|
||||
getRawDb: () => sqlite,
|
||||
schema,
|
||||
}));
|
||||
|
||||
function applyMigrations(db: Database.Database): void {
|
||||
const migrationsDir = path.resolve(__dirname, '../../drizzle');
|
||||
const files = fs.readdirSync(migrationsDir).filter(f => f.endsWith('.sql')).sort();
|
||||
for (const f of files) {
|
||||
const sqlText = fs.readFileSync(path.join(migrationsDir, f), 'utf8');
|
||||
const statements = sqlText.split(/-->\s*statement-breakpoint/);
|
||||
for (const stmt of statements) {
|
||||
const clean = stmt.trim();
|
||||
if (clean) db.exec(clean);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const OWNER_ID = 'owner-1';
|
||||
const MEMBER_ID = 'member-1';
|
||||
const OUTSIDER_ID = 'outsider-1';
|
||||
const ADMIN_ID = 'admin-1';
|
||||
const SPACE_ID = 'space-1';
|
||||
const now = 1_700_000_000_000;
|
||||
|
||||
function seed(): void {
|
||||
for (const [id, isAdmin] of [
|
||||
[OWNER_ID, 0],
|
||||
[MEMBER_ID, 0],
|
||||
[OUTSIDER_ID, 0],
|
||||
[ADMIN_ID, 1],
|
||||
] as const) {
|
||||
testDb.insert(schema.users).values({
|
||||
id,
|
||||
username: id,
|
||||
passwordHash: 'x',
|
||||
isAdmin,
|
||||
createdAt: now,
|
||||
}).run();
|
||||
}
|
||||
|
||||
testDb.insert(schema.spaces).values({
|
||||
id: SPACE_ID,
|
||||
name: 'Test Space',
|
||||
ownerId: OWNER_ID,
|
||||
inviteCode: 'code-1',
|
||||
visibility: 'request',
|
||||
createdAt: now,
|
||||
}).run();
|
||||
|
||||
// @everyone role (id === spaceId) carries the default member permissions.
|
||||
testDb.insert(schema.roles).values({
|
||||
id: SPACE_ID,
|
||||
spaceId: SPACE_ID,
|
||||
name: '@everyone',
|
||||
permissions: permissionsToString(DEFAULT_EVERYONE_PERMISSIONS),
|
||||
createdAt: now,
|
||||
}).run();
|
||||
|
||||
// Owner and one ordinary member are enrolled; OUTSIDER and ADMIN are not.
|
||||
testDb.insert(schema.spaceMembers).values({ spaceId: SPACE_ID, userId: OWNER_ID, joinedAt: now }).run();
|
||||
testDb.insert(schema.spaceMembers).values({ spaceId: SPACE_ID, userId: MEMBER_ID, joinedAt: now }).run();
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
sqlite = new Database(':memory:');
|
||||
sqlite.pragma('foreign_keys = ON');
|
||||
applyMigrations(sqlite);
|
||||
testDb = drizzle(sqlite, { schema });
|
||||
seed();
|
||||
});
|
||||
|
||||
describe('computePermissions', () => {
|
||||
it('returns 0n for a user who is not a member of the space', async () => {
|
||||
const { computePermissions } = await import('./permissions.js');
|
||||
expect(computePermissions(OUTSIDER_ID, SPACE_ID)).toBe(0n);
|
||||
});
|
||||
|
||||
it('does not grant CREATE_INVITE to a non-member via @everyone', async () => {
|
||||
const { hasPermission } = await import('./permissions.js');
|
||||
expect(hasPermission(OUTSIDER_ID, SPACE_ID, PermissionBits.CREATE_INVITE)).toBe(false);
|
||||
});
|
||||
|
||||
it('does not grant read access to a non-member via @everyone', async () => {
|
||||
const { hasPermission } = await import('./permissions.js');
|
||||
const read = PermissionBits.VIEW_CHANNEL | PermissionBits.READ_MESSAGE_HISTORY;
|
||||
expect(hasPermission(OUTSIDER_ID, SPACE_ID, read)).toBe(false);
|
||||
});
|
||||
|
||||
it('grants @everyone permissions to an enrolled member', async () => {
|
||||
const { computePermissions } = await import('./permissions.js');
|
||||
const perms = computePermissions(MEMBER_ID, SPACE_ID);
|
||||
expect(perms).toBe(DEFAULT_EVERYONE_PERMISSIONS);
|
||||
expect(perms & PermissionBits.CREATE_INVITE).toBe(PermissionBits.CREATE_INVITE);
|
||||
});
|
||||
|
||||
it('grants ALL_PERMISSIONS to the space owner', async () => {
|
||||
const { computePermissions } = await import('./permissions.js');
|
||||
expect(computePermissions(OWNER_ID, SPACE_ID)).toBe(ALL_PERMISSIONS);
|
||||
});
|
||||
|
||||
it('grants ALL_PERMISSIONS to an instance admin even when not a member', async () => {
|
||||
const { computePermissions } = await import('./permissions.js');
|
||||
expect(computePermissions(ADMIN_ID, SPACE_ID)).toBe(ALL_PERMISSIONS);
|
||||
});
|
||||
});
|
||||
@@ -36,6 +36,13 @@ export function computePermissions(userId: string, spaceId: string, channelId?:
|
||||
const userRow = db.select().from(schema.users).where(eq(schema.users.id, userId)).get();
|
||||
if (userRow?.isAdmin === 1) return ALL_PERMISSIONS;
|
||||
|
||||
// 1c. Membership gate — a user who has not joined the space has NO permissions
|
||||
// in it. Without this, the @everyone role below leaks default member rights
|
||||
// (VIEW_CHANNEL, READ_MESSAGE_HISTORY, CREATE_INVITE, …) to any authenticated
|
||||
// non-member, which allowed reading channels and minting invite codes for
|
||||
// spaces the caller never joined. Owner and instance admin are handled above.
|
||||
if (!getMember(spaceId, userId)) return 0n;
|
||||
|
||||
// 2. Base permissions from @everyone role (id === spaceId)
|
||||
const everyoneRole = db.select().from(schema.roles)
|
||||
.where(and(eq(schema.roles.id, spaceId), eq(schema.roles.spaceId, spaceId)))
|
||||
@@ -146,67 +153,6 @@ export function computePermissions(userId: string, spaceId: string, channelId?:
|
||||
return base;
|
||||
}
|
||||
|
||||
/**
|
||||
* Compute permissions at the category level (no channel step).
|
||||
* Used for determining if a category is "private" for a user.
|
||||
*/
|
||||
export function computeCategoryPermissions(userId: string, spaceId: string, categoryId: string): bigint {
|
||||
const db = getDb();
|
||||
|
||||
const space = db.select().from(schema.spaces).where(eq(schema.spaces.id, spaceId)).get();
|
||||
if (!space) return 0n;
|
||||
if (space.ownerId === userId) return ALL_PERMISSIONS;
|
||||
|
||||
const userRow = db.select().from(schema.users).where(eq(schema.users.id, userId)).get();
|
||||
if (userRow?.isAdmin === 1) return ALL_PERMISSIONS;
|
||||
|
||||
const everyoneRole = db.select().from(schema.roles)
|
||||
.where(and(eq(schema.roles.id, spaceId), eq(schema.roles.spaceId, spaceId)))
|
||||
.get();
|
||||
let base = everyoneRole ? stringToPermissions(everyoneRole.permissions) : 0n;
|
||||
|
||||
const memberRoleRows = db.select().from(schema.memberRoles)
|
||||
.where(and(eq(schema.memberRoles.spaceId, spaceId), eq(schema.memberRoles.userId, userId)))
|
||||
.all();
|
||||
const assignedRoleIds = memberRoleRows.map(mr => mr.roleId);
|
||||
|
||||
for (const roleId of assignedRoleIds) {
|
||||
const role = db.select().from(schema.roles).where(eq(schema.roles.id, roleId)).get();
|
||||
if (role) base |= stringToPermissions(role.permissions);
|
||||
}
|
||||
|
||||
if ((base & PermissionBits.ADMINISTRATOR) !== 0n) return ALL_PERMISSIONS;
|
||||
|
||||
const catOverrides = db.select().from(schema.categoryOverrides)
|
||||
.where(eq(schema.categoryOverrides.categoryId, categoryId))
|
||||
.all();
|
||||
|
||||
if (catOverrides.length === 0) return base;
|
||||
|
||||
const everyoneOverride = catOverrides.find(o => o.targetType === 'role' && o.targetId === spaceId);
|
||||
if (everyoneOverride) {
|
||||
base = (base & ~stringToPermissions(everyoneOverride.deny)) | stringToPermissions(everyoneOverride.allow);
|
||||
}
|
||||
|
||||
let combinedAllow = 0n;
|
||||
let combinedDeny = 0n;
|
||||
for (const roleId of assignedRoleIds) {
|
||||
const roleOverride = catOverrides.find(o => o.targetType === 'role' && o.targetId === roleId);
|
||||
if (roleOverride) {
|
||||
combinedAllow |= stringToPermissions(roleOverride.allow);
|
||||
combinedDeny |= stringToPermissions(roleOverride.deny);
|
||||
}
|
||||
}
|
||||
base = (base & ~combinedDeny) | combinedAllow;
|
||||
|
||||
const memberOverride = catOverrides.find(o => o.targetType === 'member' && o.targetId === userId);
|
||||
if (memberOverride) {
|
||||
base = (base & ~stringToPermissions(memberOverride.deny)) | stringToPermissions(memberOverride.allow);
|
||||
}
|
||||
|
||||
return base;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a user has a specific permission in a space/channel.
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user