fix: harden RBAC, add security headers, and prevent permission escalation
- Add permission escalation check to role create/update endpoints so users cannot assign permissions they do not possess - Replace permissive `settings.view` with `settings.read` across cache, health, and metrics routes - Require authentication and `playlists.create` for community playlist downloads (was `optionalAuth`) - Add `X-Content-Type-Options: nosniff` and `X-Frame-Options` headers to game content responses - Enable Content-Security-Policy via helmet with restrictive default-src and frame-ancestors directives - Set `server.maxConnections = 500` as defense-in-depth limit - Delete role atomically with its permissions in a transaction and invalidate permission cache afterward - Unify registration error messages to prevent username/email enumeration - Disable permission management for system roles in RoleTable - Encode share tokens and game IDs in frontend API URLs - Add `optionalAuth` to statistics route - Remove unused `rateLimitAuth` export
This commit is contained in:
@@ -289,6 +289,8 @@ export class GameZipServer {
|
||||
|
||||
const settings = ConfigManager.getSettings();
|
||||
setCorsHeaders(res, settings);
|
||||
res.setHeader('X-Content-Type-Options', 'nosniff');
|
||||
res.setHeader('X-Frame-Options', 'SAMEORIGIN');
|
||||
|
||||
res.setHeader('Content-Type', contentType);
|
||||
res.setHeader('Content-Length', fileData.length);
|
||||
@@ -529,6 +531,7 @@ export class GameZipServer {
|
||||
|
||||
const settings = ConfigManager.getSettings();
|
||||
setCorsHeaders(res, settings);
|
||||
res.setHeader('X-Content-Type-Options', 'nosniff');
|
||||
|
||||
res.setHeader('Content-Type', 'text/plain');
|
||||
res.writeHead(statusCode);
|
||||
|
||||
@@ -139,6 +139,7 @@ export class ProxyRequestHandler {
|
||||
const statusCode = result.statusCode || 200;
|
||||
|
||||
setCorsHeaders(res, settings);
|
||||
res.setHeader('X-Content-Type-Options', 'nosniff');
|
||||
|
||||
res.setHeader('Cache-Control', `public, max-age=${CACHE_MAX_AGE_SECONDS}`);
|
||||
if (process.env.NODE_ENV !== 'production') {
|
||||
@@ -233,6 +234,7 @@ export class ProxyRequestHandler {
|
||||
|
||||
const settings = ConfigManager.getSettings();
|
||||
setCorsHeaders(res, settings);
|
||||
res.setHeader('X-Content-Type-Options', 'nosniff');
|
||||
|
||||
res.setHeader('Content-Type', 'text/plain');
|
||||
res.writeHead(statusCode);
|
||||
|
||||
@@ -17,11 +17,3 @@ export const rateLimitStrict = rateLimit({
|
||||
standardHeaders: true,
|
||||
legacyHeaders: false,
|
||||
});
|
||||
|
||||
export const rateLimitAuth = rateLimit({
|
||||
windowMs: 60 * 1000,
|
||||
max: 30,
|
||||
message: 'Too many requests, please slow down',
|
||||
standardHeaders: true,
|
||||
legacyHeaders: false,
|
||||
});
|
||||
|
||||
@@ -10,7 +10,7 @@ const router = Router();
|
||||
router.get(
|
||||
'/stats',
|
||||
authenticate,
|
||||
requirePermission('settings.view'),
|
||||
requirePermission('settings.read'),
|
||||
asyncHandler(async (req, res) => {
|
||||
const gameSearchStats = GameSearchCache.getStats();
|
||||
|
||||
|
||||
@@ -6,7 +6,8 @@ import {
|
||||
} from '../services/CommunityPlaylistService';
|
||||
import { AppError } from '../middleware/errorHandler';
|
||||
import { asyncHandler } from '../middleware/asyncHandler';
|
||||
import { optionalAuth } from '../middleware/auth';
|
||||
import { optionalAuth, authenticate } from '../middleware/auth';
|
||||
import { requirePermission } from '../middleware/rbac';
|
||||
import { logActivity } from '../middleware/activityLogger';
|
||||
|
||||
const router = Router();
|
||||
@@ -36,7 +37,8 @@ router.get(
|
||||
|
||||
router.post(
|
||||
'/download',
|
||||
optionalAuth,
|
||||
authenticate,
|
||||
requirePermission('playlists.create'),
|
||||
logActivity('playlists.community.download', 'community_playlists', (req, res) => ({
|
||||
downloadUrl: req.body.downloadUrl,
|
||||
playlistTitle: res.locals.playlistTitle || null,
|
||||
|
||||
@@ -22,7 +22,7 @@ router.get('/', (req, res) => {
|
||||
router.get(
|
||||
'/detailed',
|
||||
authenticate,
|
||||
requirePermission('settings.view'),
|
||||
requirePermission('settings.read'),
|
||||
asyncHandler(async (req, res) => {
|
||||
const startTime = performance.now();
|
||||
|
||||
|
||||
@@ -9,7 +9,7 @@ const router = Router();
|
||||
router.get(
|
||||
'/summary',
|
||||
authenticate,
|
||||
requirePermission('settings.view'),
|
||||
requirePermission('settings.read'),
|
||||
asyncHandler(async (req, res) => {
|
||||
const summary = PerformanceMetrics.getSummary();
|
||||
|
||||
@@ -23,7 +23,7 @@ router.get(
|
||||
router.get(
|
||||
'/endpoints',
|
||||
authenticate,
|
||||
requirePermission('settings.view'),
|
||||
requirePermission('settings.read'),
|
||||
asyncHandler(async (req, res) => {
|
||||
const stats = PerformanceMetrics.getAllEndpointStats();
|
||||
|
||||
@@ -37,7 +37,7 @@ router.get(
|
||||
router.get(
|
||||
'/endpoints/slowest',
|
||||
authenticate,
|
||||
requirePermission('settings.view'),
|
||||
requirePermission('settings.read'),
|
||||
asyncHandler(async (req, res) => {
|
||||
const limit = Math.min(parseInt(req.query.limit as string) || 10, 50);
|
||||
const slowest = PerformanceMetrics.getSlowestEndpoints(limit);
|
||||
@@ -52,7 +52,7 @@ router.get(
|
||||
router.get(
|
||||
'/caches',
|
||||
authenticate,
|
||||
requirePermission('settings.view'),
|
||||
requirePermission('settings.read'),
|
||||
asyncHandler(async (req, res) => {
|
||||
const cacheStats = PerformanceMetrics.getCacheStats();
|
||||
|
||||
@@ -66,7 +66,7 @@ router.get(
|
||||
router.get(
|
||||
'/queries',
|
||||
authenticate,
|
||||
requirePermission('settings.view'),
|
||||
requirePermission('settings.read'),
|
||||
asyncHandler(async (req, res) => {
|
||||
const queryStats = PerformanceMetrics.getQueryStats();
|
||||
|
||||
|
||||
@@ -77,6 +77,13 @@ router.post(
|
||||
asyncHandler(async (req, res) => {
|
||||
const data = createRoleSchema.parse(req.body);
|
||||
|
||||
if (data.permissionIds && data.permissionIds.length > 0) {
|
||||
await roleService.validatePermissionEscalation(
|
||||
data.permissionIds,
|
||||
req.user?.permissions || []
|
||||
);
|
||||
}
|
||||
|
||||
const role = await roleService.createRole(
|
||||
data.name,
|
||||
data.description,
|
||||
@@ -120,6 +127,8 @@ router.put(
|
||||
|
||||
const data = updatePermissionsSchema.parse(req.body);
|
||||
|
||||
await roleService.validatePermissionEscalation(data.permissionIds, req.user?.permissions || []);
|
||||
|
||||
await roleService.updateRolePermissions(id, data.permissionIds);
|
||||
const role = await roleService.getRoleById(id);
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import { Router } from 'express';
|
||||
import { StatisticsService } from '../services/StatisticsService';
|
||||
import { requireFeature } from '../middleware/featureFlags';
|
||||
import { asyncHandler } from '../middleware/asyncHandler';
|
||||
import { optionalAuth } from '../middleware/auth';
|
||||
|
||||
const router = Router();
|
||||
const statisticsService = new StatisticsService();
|
||||
@@ -10,6 +11,7 @@ router.use(requireFeature('enableStatistics'));
|
||||
|
||||
router.get(
|
||||
'/',
|
||||
optionalAuth,
|
||||
asyncHandler(async (req, res) => {
|
||||
const statistics = await statisticsService.getStatistics();
|
||||
res.json(statistics);
|
||||
|
||||
@@ -79,7 +79,12 @@ async function startServer() {
|
||||
|
||||
app.use(
|
||||
helmet({
|
||||
contentSecurityPolicy: false,
|
||||
contentSecurityPolicy: {
|
||||
directives: {
|
||||
defaultSrc: ["'none'"],
|
||||
frameAncestors: ["'none'"],
|
||||
},
|
||||
},
|
||||
crossOriginEmbedderPolicy: false,
|
||||
frameguard: { action: 'deny' },
|
||||
})
|
||||
@@ -269,7 +274,7 @@ async function startServer() {
|
||||
server.keepAliveTimeout = 65000; // 65s - slightly above common load balancer timeout
|
||||
server.headersTimeout = 66000; // Must be > keepAliveTimeout
|
||||
server.timeout = 120000; // 2 min max for any request (including game file streaming)
|
||||
// Note: Connection limiting is handled by the reverse proxy (nginx)
|
||||
server.maxConnections = 500; // Defense-in-depth; nginx handles primary limiting
|
||||
|
||||
const playTrackingService = new PlayTrackingService();
|
||||
const playSessionCleanupInterval = setInterval(
|
||||
|
||||
@@ -108,12 +108,12 @@ export class AuthService {
|
||||
const registerTransaction = db.transaction(() => {
|
||||
const existingUser = db.prepare('SELECT id FROM users WHERE username = ?').get(data.username);
|
||||
if (existingUser) {
|
||||
throw new AppError(409, 'Username already exists');
|
||||
throw new AppError(409, 'An account with that username or email already exists');
|
||||
}
|
||||
|
||||
const existingEmail = db.prepare('SELECT id FROM users WHERE email = ?').get(data.email);
|
||||
if (existingEmail) {
|
||||
throw new AppError(409, 'Email already exists');
|
||||
throw new AppError(409, 'An account with that username or email already exists');
|
||||
}
|
||||
|
||||
// Check if this is initial setup (no users exist)
|
||||
|
||||
@@ -102,6 +102,30 @@ export class RoleService {
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates that the user has all permissions they're trying to assign
|
||||
* Prevents permission escalation attacks
|
||||
*/
|
||||
async validatePermissionEscalation(
|
||||
permissionIds: number[],
|
||||
userPermissions: string[]
|
||||
): Promise<void> {
|
||||
if (permissionIds.length === 0) return;
|
||||
|
||||
const allPermissions = await this.getPermissions();
|
||||
const permissionMap = new Map(allPermissions.map((p) => [p.id, p.name]));
|
||||
|
||||
const unauthorized = permissionIds.filter((id) => {
|
||||
const name = permissionMap.get(id);
|
||||
return !name || !userPermissions.includes(name);
|
||||
});
|
||||
|
||||
if (unauthorized.length > 0) {
|
||||
const names = unauthorized.map((id) => permissionMap.get(id) || `unknown(${id})`).join(', ');
|
||||
throw new AppError(403, `Cannot assign permissions you do not possess: ${names}`);
|
||||
}
|
||||
}
|
||||
|
||||
private getRolePermissions(roleId: number): Permission[] {
|
||||
return UserDatabaseService.all(
|
||||
`SELECT p.id, p.name, p.description, p.resource, p.action
|
||||
@@ -256,6 +280,15 @@ export class RoleService {
|
||||
throw new AppError(409, `Cannot delete role: ${userCount} users are assigned to this role`);
|
||||
}
|
||||
|
||||
UserDatabaseService.run('DELETE FROM roles WHERE id = ?', [id]);
|
||||
// Delete role_permissions and role in a transaction for atomicity
|
||||
const db = UserDatabaseService.getDatabase();
|
||||
db.transaction(() => {
|
||||
db.prepare('DELETE FROM role_permissions WHERE role_id = ?').run(id);
|
||||
db.prepare('DELETE FROM roles WHERE id = ?').run(id);
|
||||
})();
|
||||
|
||||
// Invalidate permission cache for this role
|
||||
PermissionCache.invalidateRole(id);
|
||||
logger.info(`[RoleService] Deleted role ${id}, cache invalidated`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,6 +19,9 @@ import {
|
||||
DropdownMenuTrigger,
|
||||
} from '../ui/dropdown-menu';
|
||||
|
||||
/** System role IDs from the initial migration — admin=1, user=2, guest=3 */
|
||||
const SYSTEM_ROLE_IDS = new Set([1, 2, 3]);
|
||||
|
||||
interface RoleTableProps {
|
||||
onEdit: (role: Role) => void;
|
||||
onManagePermissions: (role: Role) => void;
|
||||
@@ -99,7 +102,7 @@ export function RoleTable({ onEdit, onManagePermissions }: RoleTableProps) {
|
||||
header: 'Actions',
|
||||
cell: ({ row }) => {
|
||||
const role = row.original;
|
||||
const isSystemRole = role.id <= 3;
|
||||
const isSystemRole = SYSTEM_ROLE_IDS.has(role.id);
|
||||
|
||||
return (
|
||||
<div className="flex items-center gap-2">
|
||||
@@ -119,7 +122,10 @@ export function RoleTable({ onEdit, onManagePermissions }: RoleTableProps) {
|
||||
</DropdownMenuItem>
|
||||
</RoleGuard>
|
||||
<RoleGuard permission="roles.update">
|
||||
<DropdownMenuItem onClick={() => onManagePermissions(role)}>
|
||||
<DropdownMenuItem
|
||||
onClick={() => onManagePermissions(role)}
|
||||
disabled={isSystemRole}
|
||||
>
|
||||
Manage Permissions
|
||||
</DropdownMenuItem>
|
||||
</RoleGuard>
|
||||
|
||||
@@ -4,26 +4,33 @@ import type { Game } from '@/types/game';
|
||||
|
||||
export const sharedPlaylistsApi = {
|
||||
getByToken: async (shareToken: string): Promise<SharedPlaylist> => {
|
||||
const { data } = await apiClient.get<SharedPlaylist>(`/playlists/shared/${shareToken}`);
|
||||
const { data } = await apiClient.get<SharedPlaylist>(
|
||||
`/playlists/shared/${encodeURIComponent(shareToken)}`
|
||||
);
|
||||
return data;
|
||||
},
|
||||
|
||||
getGames: async (shareToken: string): Promise<Game[]> => {
|
||||
const { data } = await apiClient.get<Game[]>(`/playlists/shared/${shareToken}/games`);
|
||||
const { data } = await apiClient.get<Game[]>(
|
||||
`/playlists/shared/${encodeURIComponent(shareToken)}/games`
|
||||
);
|
||||
return data;
|
||||
},
|
||||
|
||||
clonePlaylist: async (shareToken: string, newTitle?: string): Promise<UserPlaylist> => {
|
||||
const { data } = await apiClient.post<UserPlaylist>(`/playlists/shared/${shareToken}/clone`, {
|
||||
newTitle,
|
||||
});
|
||||
const { data } = await apiClient.post<UserPlaylist>(
|
||||
`/playlists/shared/${encodeURIComponent(shareToken)}/clone`,
|
||||
{
|
||||
newTitle,
|
||||
}
|
||||
);
|
||||
return data;
|
||||
},
|
||||
|
||||
validateGameAccess: async (shareToken: string, gameId: string): Promise<boolean> => {
|
||||
try {
|
||||
const { data } = await apiClient.get<{ valid: boolean }>(
|
||||
`/playlists/shared/${shareToken}/games/${gameId}/validate`
|
||||
`/playlists/shared/${encodeURIComponent(shareToken)}/games/${encodeURIComponent(gameId)}/validate`
|
||||
);
|
||||
return data.valid;
|
||||
} catch {
|
||||
@@ -39,7 +46,9 @@ export const sharedPlaylistsApi = {
|
||||
expiresIn: number;
|
||||
playlistId: number;
|
||||
}> => {
|
||||
const { data } = await apiClient.post(`/playlists/shared/${shareToken}/generate-access-token`);
|
||||
const { data } = await apiClient.post(
|
||||
`/playlists/shared/${encodeURIComponent(shareToken)}/generate-access-token`
|
||||
);
|
||||
return data;
|
||||
},
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user