fix: harden permissions system after code review
- Gate permissions in /app-config behind optionalAuth so unauthenticated requests don't receive admin configuration - Fix trip_delete isMember parameter (was hardcoded false) - Return skipped keys from savePermissions for admin visibility - Add disabled prop to CustomSelect, use in BudgetPanel currency picker - Fix CollabChat reaction handler returning false instead of void - Pass canUploadFiles as prop to NoteFormModal instead of internal store read - Make edit-only NoteFormModal props optional (onDeleteFile, note, tripId) - Add missing trailing newlines to .gitignore and it.ts
This commit is contained in:
2
.gitignore
vendored
2
.gitignore
vendored
@@ -55,4 +55,4 @@ coverage
|
|||||||
.eslintcache
|
.eslintcache
|
||||||
.cache
|
.cache
|
||||||
*.tsbuildinfo
|
*.tsbuildinfo
|
||||||
*.tgz
|
*.tgz
|
||||||
|
|||||||
@@ -649,7 +649,8 @@ export default function BudgetPanel({ tripId, tripMembers = [] }: BudgetPanelPro
|
|||||||
<div style={{ marginBottom: 12 }}>
|
<div style={{ marginBottom: 12 }}>
|
||||||
<CustomSelect
|
<CustomSelect
|
||||||
value={currency}
|
value={currency}
|
||||||
onChange={canEdit ? setCurrency : () => {}}
|
onChange={setCurrency}
|
||||||
|
disabled={!canEdit}
|
||||||
options={CURRENCIES.map(c => ({ value: c, label: `${c} (${SYMBOLS[c] || c})` }))}
|
options={CURRENCIES.map(c => ({ value: c, label: `${c} (${SYMBOLS[c] || c})` }))}
|
||||||
searchable
|
searchable
|
||||||
/>
|
/>
|
||||||
|
|||||||
@@ -740,7 +740,7 @@ export default function CollabChat({ tripId, currentUser }: CollabChatProps) {
|
|||||||
{msg.reactions.map(r => {
|
{msg.reactions.map(r => {
|
||||||
const myReaction = r.users.some(u => String(u.user_id) === String(currentUser.id))
|
const myReaction = r.users.some(u => String(u.user_id) === String(currentUser.id))
|
||||||
return (
|
return (
|
||||||
<ReactionBadge key={r.emoji} reaction={r} currentUserId={currentUser.id} onReact={() => canEdit && handleReact(msg.id, r.emoji)} />
|
<ReactionBadge key={r.emoji} reaction={r} currentUserId={currentUser.id} onReact={() => { if (canEdit) handleReact(msg.id, r.emoji) }} />
|
||||||
)
|
)
|
||||||
})}
|
})}
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -218,19 +218,17 @@ function UserAvatar({ user, size = 14 }: UserAvatarProps) {
|
|||||||
interface NoteFormModalProps {
|
interface NoteFormModalProps {
|
||||||
onClose: () => void
|
onClose: () => void
|
||||||
onSubmit: (data: { title: string; content: string; category: string; website: string; files?: File[] }) => Promise<void>
|
onSubmit: (data: { title: string; content: string; category: string; website: string; files?: File[] }) => Promise<void>
|
||||||
onDeleteFile: (noteId: number, fileId: number) => Promise<void>
|
onDeleteFile?: (noteId: number, fileId: number) => Promise<void>
|
||||||
existingCategories: string[]
|
existingCategories: string[]
|
||||||
categoryColors: Record<string, string>
|
categoryColors: Record<string, string>
|
||||||
getCategoryColor: (category: string) => string
|
getCategoryColor: (category: string) => string
|
||||||
note: CollabNote | null
|
note?: CollabNote | null
|
||||||
tripId: number
|
tripId?: number
|
||||||
t: (key: string) => string
|
t: (key: string) => string
|
||||||
|
canUploadFiles?: boolean
|
||||||
}
|
}
|
||||||
|
|
||||||
function NoteFormModal({ onClose, onSubmit, onDeleteFile, existingCategories, categoryColors, getCategoryColor, note, tripId, t }: NoteFormModalProps) {
|
function NoteFormModal({ onClose, onSubmit, onDeleteFile, existingCategories, categoryColors, getCategoryColor, note, tripId, t, canUploadFiles = true }: NoteFormModalProps) {
|
||||||
const can = useCanDo()
|
|
||||||
const tripObj = useTripStore((s) => s.trip)
|
|
||||||
const canUploadFiles = can('file_upload', tripObj)
|
|
||||||
const isEdit = !!note
|
const isEdit = !!note
|
||||||
const allCategories = [...new Set([...existingCategories, ...Object.keys(categoryColors || {})])].filter(Boolean)
|
const allCategories = [...new Set([...existingCategories, ...Object.keys(categoryColors || {})])].filter(Boolean)
|
||||||
|
|
||||||
@@ -889,6 +887,7 @@ export default function CollabNotes({ tripId, currentUser }: CollabNotesProps) {
|
|||||||
const can = useCanDo()
|
const can = useCanDo()
|
||||||
const trip = useTripStore((s) => s.trip)
|
const trip = useTripStore((s) => s.trip)
|
||||||
const canEdit = can('collab_edit', trip)
|
const canEdit = can('collab_edit', trip)
|
||||||
|
const canUploadFiles = can('file_upload', trip)
|
||||||
const [notes, setNotes] = useState([])
|
const [notes, setNotes] = useState([])
|
||||||
const [loading, setLoading] = useState(true)
|
const [loading, setLoading] = useState(true)
|
||||||
const [showNewModal, setShowNewModal] = useState(false)
|
const [showNewModal, setShowNewModal] = useState(false)
|
||||||
@@ -1343,6 +1342,7 @@ export default function CollabNotes({ tripId, currentUser }: CollabNotesProps) {
|
|||||||
existingCategories={categories}
|
existingCategories={categories}
|
||||||
categoryColors={categoryColors}
|
categoryColors={categoryColors}
|
||||||
getCategoryColor={getCategoryColor}
|
getCategoryColor={getCategoryColor}
|
||||||
|
canUploadFiles={canUploadFiles}
|
||||||
t={t}
|
t={t}
|
||||||
/>
|
/>
|
||||||
)}
|
)}
|
||||||
@@ -1358,6 +1358,7 @@ export default function CollabNotes({ tripId, currentUser }: CollabNotesProps) {
|
|||||||
existingCategories={categories}
|
existingCategories={categories}
|
||||||
categoryColors={categoryColors}
|
categoryColors={categoryColors}
|
||||||
getCategoryColor={getCategoryColor}
|
getCategoryColor={getCategoryColor}
|
||||||
|
canUploadFiles={canUploadFiles}
|
||||||
t={t}
|
t={t}
|
||||||
/>
|
/>
|
||||||
)}
|
)}
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ interface CustomSelectProps {
|
|||||||
searchable?: boolean
|
searchable?: boolean
|
||||||
style?: React.CSSProperties
|
style?: React.CSSProperties
|
||||||
size?: 'sm' | 'md'
|
size?: 'sm' | 'md'
|
||||||
|
disabled?: boolean
|
||||||
}
|
}
|
||||||
|
|
||||||
export default function CustomSelect({
|
export default function CustomSelect({
|
||||||
@@ -29,6 +30,7 @@ export default function CustomSelect({
|
|||||||
searchable = false,
|
searchable = false,
|
||||||
style = {},
|
style = {},
|
||||||
size = 'md',
|
size = 'md',
|
||||||
|
disabled = false,
|
||||||
}: CustomSelectProps) {
|
}: CustomSelectProps) {
|
||||||
const [open, setOpen] = useState(false)
|
const [open, setOpen] = useState(false)
|
||||||
const [search, setSearch] = useState('')
|
const [search, setSearch] = useState('')
|
||||||
@@ -83,17 +85,19 @@ export default function CustomSelect({
|
|||||||
{/* Trigger */}
|
{/* Trigger */}
|
||||||
<button
|
<button
|
||||||
type="button"
|
type="button"
|
||||||
onClick={() => { setOpen(o => !o); setSearch('') }}
|
disabled={disabled}
|
||||||
|
onClick={() => { if (!disabled) { setOpen(o => !o); setSearch('') } }}
|
||||||
style={{
|
style={{
|
||||||
width: '100%', display: 'flex', alignItems: 'center', gap: 8,
|
width: '100%', display: 'flex', alignItems: 'center', gap: 8,
|
||||||
padding: sm ? '8px 12px' : '8px 14px', borderRadius: 10,
|
padding: sm ? '8px 12px' : '8px 14px', borderRadius: 10,
|
||||||
border: '1px solid var(--border-primary)',
|
border: '1px solid var(--border-primary)',
|
||||||
background: 'var(--bg-input)', color: 'var(--text-primary)',
|
background: 'var(--bg-input)', color: 'var(--text-primary)',
|
||||||
fontSize: 13, fontWeight: 500, fontFamily: 'inherit',
|
fontSize: 13, fontWeight: 500, fontFamily: 'inherit',
|
||||||
cursor: 'pointer', outline: 'none', textAlign: 'left',
|
cursor: disabled ? 'default' : 'pointer', outline: 'none', textAlign: 'left',
|
||||||
transition: 'border-color 0.15s', overflow: 'hidden', minWidth: 0,
|
transition: 'border-color 0.15s', overflow: 'hidden', minWidth: 0,
|
||||||
|
opacity: disabled ? 0.5 : 1,
|
||||||
}}
|
}}
|
||||||
onMouseEnter={e => e.currentTarget.style.borderColor = 'var(--text-faint)'}
|
onMouseEnter={e => { if (!disabled) e.currentTarget.style.borderColor = 'var(--text-faint)' }}
|
||||||
onMouseLeave={e => { if (!open) e.currentTarget.style.borderColor = 'var(--border-primary)' }}
|
onMouseLeave={e => { if (!open) e.currentTarget.style.borderColor = 'var(--border-primary)' }}
|
||||||
>
|
>
|
||||||
{selected?.icon && <span style={{ display: 'flex', flexShrink: 0 }}>{selected.icon}</span>}
|
{selected?.icon && <span style={{ display: 'flex', flexShrink: 0 }}>{selected.icon}</span>}
|
||||||
|
|||||||
@@ -1470,4 +1470,4 @@ const it: Record<string, string | { name: string; category: string }[]> = {
|
|||||||
'perm.actionHint.share_manage': 'Chi può creare o eliminare link di condivisione pubblici',
|
'perm.actionHint.share_manage': 'Chi può creare o eliminare link di condivisione pubblici',
|
||||||
}
|
}
|
||||||
|
|
||||||
export default it
|
export default it
|
||||||
|
|||||||
@@ -177,7 +177,7 @@ router.put('/permissions', (req: Request, res: Response) => {
|
|||||||
if (!permissions || typeof permissions !== 'object') {
|
if (!permissions || typeof permissions !== 'object') {
|
||||||
return res.status(400).json({ error: 'permissions object required' });
|
return res.status(400).json({ error: 'permissions object required' });
|
||||||
}
|
}
|
||||||
savePermissions(permissions);
|
const { skipped } = savePermissions(permissions);
|
||||||
writeAudit({
|
writeAudit({
|
||||||
userId: authReq.user.id,
|
userId: authReq.user.id,
|
||||||
action: 'admin.permissions_update',
|
action: 'admin.permissions_update',
|
||||||
@@ -185,7 +185,7 @@ router.put('/permissions', (req: Request, res: Response) => {
|
|||||||
ip: getClientIp(req),
|
ip: getClientIp(req),
|
||||||
details: permissions,
|
details: permissions,
|
||||||
});
|
});
|
||||||
res.json({ success: true, permissions: getAllPermissions() });
|
res.json({ success: true, permissions: getAllPermissions(), ...(skipped.length ? { skipped } : {}) });
|
||||||
});
|
});
|
||||||
|
|
||||||
router.get('/audit-log', (req: Request, res: Response) => {
|
router.get('/audit-log', (req: Request, res: Response) => {
|
||||||
|
|||||||
@@ -10,13 +10,13 @@ import fetch from 'node-fetch';
|
|||||||
import { authenticator } from 'otplib';
|
import { authenticator } from 'otplib';
|
||||||
import QRCode from 'qrcode';
|
import QRCode from 'qrcode';
|
||||||
import { db } from '../db/database';
|
import { db } from '../db/database';
|
||||||
import { authenticate, demoUploadBlock } from '../middleware/auth';
|
import { authenticate, optionalAuth, demoUploadBlock } from '../middleware/auth';
|
||||||
import { JWT_SECRET } from '../config';
|
import { JWT_SECRET } from '../config';
|
||||||
import { encryptMfaSecret, decryptMfaSecret } from '../services/mfaCrypto';
|
import { encryptMfaSecret, decryptMfaSecret } from '../services/mfaCrypto';
|
||||||
import { getAllPermissions } from '../services/permissions';
|
import { getAllPermissions } from '../services/permissions';
|
||||||
import { randomBytes, createHash } from 'crypto';
|
import { randomBytes, createHash } from 'crypto';
|
||||||
import { revokeUserSessions } from '../mcp';
|
import { revokeUserSessions } from '../mcp';
|
||||||
import { AuthRequest, User } from '../types';
|
import { AuthRequest, OptionalAuthRequest, User } from '../types';
|
||||||
import { writeAudit, getClientIp } from '../services/auditLog';
|
import { writeAudit, getClientIp } from '../services/auditLog';
|
||||||
import { decrypt_api_key, maybe_encrypt_api_key } from '../services/apiKeyCrypto';
|
import { decrypt_api_key, maybe_encrypt_api_key } from '../services/apiKeyCrypto';
|
||||||
import { startTripReminders } from '../scheduler';
|
import { startTripReminders } from '../scheduler';
|
||||||
@@ -171,7 +171,7 @@ function generateToken(user: { id: number | bigint }) {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
router.get('/app-config', (_req: Request, res: Response) => {
|
router.get('/app-config', optionalAuth, (req: Request, res: Response) => {
|
||||||
const userCount = (db.prepare('SELECT COUNT(*) as count FROM users').get() as { count: number }).count;
|
const userCount = (db.prepare('SELECT COUNT(*) as count FROM users').get() as { count: number }).count;
|
||||||
const setting = db.prepare("SELECT value FROM app_settings WHERE key = 'allow_registration'").get() as { value: string } | undefined;
|
const setting = db.prepare("SELECT value FROM app_settings WHERE key = 'allow_registration'").get() as { value: string } | undefined;
|
||||||
const allowRegistration = userCount === 0 || (setting?.value ?? 'true') === 'true';
|
const allowRegistration = userCount === 0 || (setting?.value ?? 'true') === 'true';
|
||||||
@@ -210,7 +210,7 @@ router.get('/app-config', (_req: Request, res: Response) => {
|
|||||||
timezone: process.env.TZ || Intl.DateTimeFormat().resolvedOptions().timeZone || 'UTC',
|
timezone: process.env.TZ || Intl.DateTimeFormat().resolvedOptions().timeZone || 'UTC',
|
||||||
notification_channel: notifChannel,
|
notification_channel: notifChannel,
|
||||||
trip_reminders_enabled: tripRemindersEnabled,
|
trip_reminders_enabled: tripRemindersEnabled,
|
||||||
permissions: getAllPermissions(),
|
permissions: (req as OptionalAuthRequest).user ? getAllPermissions() : undefined,
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -294,7 +294,8 @@ router.delete('/:id', authenticate, (req: Request, res: Response) => {
|
|||||||
const trip = db.prepare('SELECT user_id FROM trips WHERE id = ?').get(req.params.id) as { user_id: number } | undefined;
|
const trip = db.prepare('SELECT user_id FROM trips WHERE id = ?').get(req.params.id) as { user_id: number } | undefined;
|
||||||
if (!trip) return res.status(404).json({ error: 'Trip not found' });
|
if (!trip) return res.status(404).json({ error: 'Trip not found' });
|
||||||
const tripOwnerId = trip.user_id;
|
const tripOwnerId = trip.user_id;
|
||||||
if (!checkPermission('trip_delete', authReq.user.role, tripOwnerId, authReq.user.id, false))
|
const isMemberDel = tripOwnerId !== authReq.user.id;
|
||||||
|
if (!checkPermission('trip_delete', authReq.user.role, tripOwnerId, authReq.user.id, isMemberDel))
|
||||||
return res.status(403).json({ error: 'No permission to delete this trip' });
|
return res.status(403).json({ error: 'No permission to delete this trip' });
|
||||||
const deletedTripId = Number(req.params.id);
|
const deletedTripId = Number(req.params.id);
|
||||||
const delTrip = db.prepare('SELECT title, user_id FROM trips WHERE id = ?').get(req.params.id) as { title: string; user_id: number } | undefined;
|
const delTrip = db.prepare('SELECT title, user_id FROM trips WHERE id = ?').get(req.params.id) as { title: string; user_id: number } | undefined;
|
||||||
|
|||||||
@@ -95,18 +95,22 @@ export function getAllPermissions(): Record<string, PermissionLevel> {
|
|||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function savePermissions(settings: Record<string, string>): void {
|
export function savePermissions(settings: Record<string, string>): { skipped: string[] } {
|
||||||
|
const skipped: string[] = [];
|
||||||
const upsert = db.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)");
|
const upsert = db.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)");
|
||||||
const txn = db.transaction(() => {
|
const txn = db.transaction(() => {
|
||||||
for (const [actionKey, level] of Object.entries(settings)) {
|
for (const [actionKey, level] of Object.entries(settings)) {
|
||||||
const action = ACTIONS_MAP.get(actionKey);
|
const action = ACTIONS_MAP.get(actionKey);
|
||||||
if (!action) continue;
|
if (!action || !action.allowedLevels.includes(level as PermissionLevel)) {
|
||||||
if (!action.allowedLevels.includes(level as PermissionLevel)) continue;
|
skipped.push(actionKey);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
upsert.run(`perm_${actionKey}`, level);
|
upsert.run(`perm_${actionKey}`, level);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
txn();
|
txn();
|
||||||
invalidatePermissionsCache();
|
invalidatePermissionsCache();
|
||||||
|
return { skipped };
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user