From af88549ad89858d0215ba67f059325de7b5cf5c8 Mon Sep 17 00:00:00 2001 From: Steve Dogiakos Date: Wed, 18 Mar 2026 23:31:23 -0600 Subject: [PATCH] Modal scroll fix; per-account editor/viewer roles - Fix account settings modal overflow: add max-height to .modal, make .modal-body flex/scrollable, widen #acct-settings-modal to 620px - Add role column to user_accounts (editor|viewer) with migration; existing assignments promoted to editor - New isEditorForAccount() in auth middleware for per-account write checks - Replace global requireEditor with per-account checks in checks.js, deposits.js, pdf.js, deposit-pdf.js, qbo-import.js - GET /api/accounts now returns user_role per account - users.js returns {account_id, role} per assignment; POST/PUT accept accounts as [{id, role}] - Frontend: state.accountRole tracks effective role for active account; applyRoleUI and renderRow use it; user management shows role dropdown per account assignment --- public/css/style.css | 7 ++++++ public/js/app.js | 47 ++++++++++++++++++++++++++++----------- src/app.js | 16 +++++++------ src/db/database.js | 17 ++++++++++++++ src/db/schema.sql | 1 + src/middleware/auth.js | 13 ++++++++++- src/routes/checks.js | 24 +++++++++++++++----- src/routes/deposit-pdf.js | 4 ++++ src/routes/deposits.js | 27 ++++++++++++++++------ src/routes/pdf.js | 4 ++++ src/routes/qbo-import.js | 4 ++++ src/routes/users.js | 14 +++++------- 12 files changed, 137 insertions(+), 41 deletions(-) diff --git a/public/css/style.css b/public/css/style.css index be7df09..2a13a8c 100644 --- a/public/css/style.css +++ b/public/css/style.css @@ -421,6 +421,7 @@ td { transform: translate(-50%, -48%); width: 480px; max-width: calc(100vw - 2rem); + max-height: calc(100vh - 40px); background: var(--surface); border-radius: 6px; box-shadow: 0 8px 32px rgba(0,0,0,0.2); @@ -454,6 +455,9 @@ td { display: flex; flex-direction: column; gap: 12px; + flex: 1; + min-height: 0; + overflow-y: auto; } .modal-desc { @@ -598,6 +602,9 @@ input[type="file"] { border-radius: 3px; padding: 3px; } +#acct-settings-modal { + width: min(620px, calc(100vw - 2rem)); +} #acct-settings-modal .modal-body { max-height: calc(100vh - 160px); overflow-y: auto; diff --git a/public/js/app.js b/public/js/app.js index 82d288a..9b585ca 100644 --- a/public/js/app.js +++ b/public/js/app.js @@ -13,7 +13,8 @@ const state = { sortDir: 'desc', selected: new Set(), editingId: null, - user: null, // { id, username, role } + user: null, // { id, username, role } + accountRole: null, // 'editor' or 'viewer' for the current account }; // ── API helpers ────────────────────────────────────────────────────────────── @@ -128,7 +129,8 @@ async function logout() { function applyRoleUI() { const role = state.user ? state.user.role : 'viewer'; const isAdmin = role === 'admin'; - const isEditor = role === 'admin' || role === 'editor'; + // For editor-only elements, use per-account role when available + const isEditor = state.accountRole === 'editor' || (!state.accountRole && (role === 'admin' || role === 'editor')); document.getElementById('header-username').textContent = state.user ? state.user.username : ''; @@ -186,10 +188,11 @@ function renderUsersList() { ${users.map(u => { const isSelf = u.id === state.user.id; const accountsLabel = u.role === 'admin' - ? 'All accounts' - : (u.accounts.length ? u.accounts.map(aid => { - const a = state.accounts.find(x => x.id === aid); - return escHtml(a ? (a.company1 || `Account ${a.id}`) : `#${aid}`); + ? 'All accounts (editor)' + : (u.accounts.length ? u.accounts.map(ua => { + const a = state.accounts.find(x => x.id === ua.account_id); + const name = escHtml(a ? (a.company1 || `Account ${a.account_id}`) : `#${ua.account_id}`); + return `${name} ${ua.role}`; }).join(', ') : 'None'); return ` ${escHtml(u.username)}${isSelf ? ' (you)' : ''} @@ -212,12 +215,19 @@ function renderUfAccountCheckboxes() { const currentAccounts = usersState.editingId ? (usersState.users.find(u => u.id === usersState.editingId) || {}).accounts || [] : []; - container.innerHTML = state.accounts.map(a => - ``; + }).join(''); } function startUserEdit(userId) { @@ -257,7 +267,11 @@ async function saveUser() { const password = document.getElementById('uf-password').value; const role = document.getElementById('uf-role').value; const accounts = Array.from(document.querySelectorAll('input[name="uf-account"]:checked')) - .map(cb => parseInt(cb.value, 10)); + .map(cb => { + const accountId = parseInt(cb.value, 10); + const roleSelect = document.querySelector(`select[name="uf-account-role"][data-account-id="${accountId}"]`); + return { id: accountId, role: roleSelect ? roleSelect.value : 'viewer' }; + }); if (!username) { errEl.textContent = 'Username required.'; errEl.hidden = false; return; } if (!usersState.editingId && !password) { errEl.textContent = 'Password required.'; errEl.hidden = false; return; } @@ -340,6 +354,9 @@ async function loadAccounts() { localStorage.setItem('activeAccountId', state.activeAccountId); populateAccountSwitcher(); + const activeAcct = state.accounts.find(a => a.id === state.activeAccountId); + state.accountRole = activeAcct ? activeAcct.user_role : null; + applyRoleUI(); state.account = await apiFetch('GET', `/api/account/${state.activeAccountId}`); renderHeader(); await loadChecks(); @@ -359,6 +376,9 @@ async function switchAccount(accountId) { state.activeAccountId = accountId; localStorage.setItem('activeAccountId', accountId); state.selected.clear(); + const activeAcct = state.accounts.find(a => a.id === accountId); + state.accountRole = activeAcct ? activeAcct.user_role : null; + applyRoleUI(); state.account = await apiFetch('GET', `/api/account/${accountId}`); renderHeader(); await loadChecks(); @@ -430,7 +450,8 @@ function renderRow(c) { }) : '—'; - const isEditor = state.user && (state.user.role === 'admin' || state.user.role === 'editor'); + const isEditor = state.accountRole === 'editor' || + (!state.accountRole && state.user && (state.user.role === 'admin' || state.user.role === 'editor')); const checkbox = isEditor ? `` diff --git a/src/app.js b/src/app.js index f153055..15f036d 100644 --- a/src/app.js +++ b/src/app.js @@ -10,7 +10,7 @@ const multer = require('multer'); const session = require('express-session'); const db = require('./db/database'); -const { requireAuth, requireAdmin, requireEditor, canAccessAccount } = require('./middleware/auth'); +const { requireAuth, requireAdmin, canAccessAccount } = require('./middleware/auth'); const app = express(); const upload = multer({ dest: os.tmpdir() }); @@ -44,15 +44,15 @@ app.use('/api/users', require('./routes/users')); // ── Check routes ────────────────────────────────────────────────────────────── app.use('/api/checks', require('./routes/checks')); -// ── PDF (editor+) ───────────────────────────────────────────────────────────── -app.use('/api/pdf', requireEditor, require('./routes/pdf')); +// ── PDF (per-account editor check inside route) ─────────────────────────────── +app.use('/api/pdf', require('./routes/pdf')); // ── Deposits ────────────────────────────────────────────────────────────────── app.use('/api/deposits', require('./routes/deposits')); -app.use('/api/deposit-pdf', requireEditor, require('./routes/deposit-pdf')); +app.use('/api/deposit-pdf', require('./routes/deposit-pdf')); -// ── QBO import (editor+) ────────────────────────────────────────────────────── -app.use('/api/qbo-import', requireEditor, require('./routes/qbo-import')); +// ── QBO import (per-account editor check inside route) ──────────────────────── +app.use('/api/qbo-import', require('./routes/qbo-import')); // ── Accounts list — filtered by role ───────────────────────────────────────── app.get('/api/accounts', (req, res) => { @@ -61,9 +61,11 @@ app.get('/api/accounts', (req, res) => { accounts = db.prepare( 'SELECT id, company1, bank_name, current_check_no FROM account ORDER BY id ASC' ).all(); + // Admins have editor access to all accounts + accounts.forEach(a => { a.user_role = 'editor'; }); } else { accounts = db.prepare(` - SELECT a.id, a.company1, a.bank_name, a.current_check_no + SELECT a.id, a.company1, a.bank_name, a.current_check_no, ua.role AS user_role FROM account a JOIN user_accounts ua ON ua.account_id = a.id WHERE ua.user_id = ? diff --git a/src/db/database.js b/src/db/database.js index 8260079..e49be8e 100644 --- a/src/db/database.js +++ b/src/db/database.js @@ -99,6 +99,23 @@ if (!acctInfo.some(c => c.name === 'second_signature')) { db.exec('ALTER TABLE account ADD COLUMN second_signature INTEGER NOT NULL DEFAULT 0'); } +// Migration: add role column to user_accounts +const uaInfo = db.prepare('PRAGMA table_info(user_accounts)').all(); +if (!uaInfo.some(c => c.name === 'role')) { + db.exec(` + ALTER TABLE user_accounts RENAME TO user_accounts_old; + CREATE TABLE user_accounts ( + user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + account_id INTEGER NOT NULL REFERENCES account(id) ON DELETE CASCADE, + role TEXT NOT NULL DEFAULT 'viewer' CHECK(role IN ('editor','viewer')), + PRIMARY KEY (user_id, account_id) + ); + INSERT INTO user_accounts (user_id, account_id, role) + SELECT user_id, account_id, 'editor' FROM user_accounts_old; + DROP TABLE user_accounts_old; + `); +} + // Create account_id indexes unconditionally (safe after migrations have run) db.exec(` CREATE INDEX IF NOT EXISTS idx_checks_account ON checks(account_id); diff --git a/src/db/schema.sql b/src/db/schema.sql index 920810c..cb74e99 100644 --- a/src/db/schema.sql +++ b/src/db/schema.sql @@ -110,6 +110,7 @@ CREATE TABLE IF NOT EXISTS users ( CREATE TABLE IF NOT EXISTS user_accounts ( user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, account_id INTEGER NOT NULL REFERENCES account(id) ON DELETE CASCADE, + role TEXT NOT NULL DEFAULT 'viewer' CHECK(role IN ('editor','viewer')), PRIMARY KEY (user_id, account_id) ); diff --git a/src/middleware/auth.js b/src/middleware/auth.js index 0d5b122..545a16d 100644 --- a/src/middleware/auth.js +++ b/src/middleware/auth.js @@ -34,6 +34,17 @@ function canAccessAccount(session, accountId) { return !!row; } +// Returns true if the user has editor (write) access to the given account. +// Admins always return true. Non-admins need user_accounts.role = 'editor'. +function isEditorForAccount(session, accountId) { + if (!session || !session.userId) return false; + if (session.role === 'admin') return true; + const row = db.prepare( + "SELECT role FROM user_accounts WHERE user_id = ? AND account_id = ?" + ).get(session.userId, accountId); + return !!(row && row.role === 'editor'); +} + // Middleware factory — resolves accountId via a callback on req, then checks access function requireAccountAccess(getAccountId) { return (req, res, next) => { @@ -50,4 +61,4 @@ function requireAccountAccess(getAccountId) { }; } -module.exports = { requireAuth, requireAdmin, requireEditor, requireAccountAccess, canAccessAccount }; +module.exports = { requireAuth, requireAdmin, requireEditor, requireAccountAccess, canAccessAccount, isEditorForAccount }; diff --git a/src/routes/checks.js b/src/routes/checks.js index c6ce481..f5e79f9 100644 --- a/src/routes/checks.js +++ b/src/routes/checks.js @@ -3,7 +3,7 @@ const express = require('express'); const router = express.Router(); const db = require('../db/database'); -const { requireEditor, canAccessAccount } = require('../middleware/auth'); +const { canAccessAccount, isEditorForAccount } = require('../middleware/auth'); // Helper: resolve account_id from a check id (for edit/delete access checks) function checkAccountId(checkId) { @@ -47,13 +47,16 @@ router.get('/:id', (req, res) => { // TODO: Add payee address book -- store and recall payee name + address lines, autocomplete on new check form // POST /api/checks - create a new check (editor+) -router.post('/', requireEditor, (req, res) => { +router.post('/', (req, res) => { const { account_id, payee, amount, check_date, memo, note1, note2, payee_address1, payee_address2, payee_address3, payee_address4 } = req.body; if (!account_id || !payee || !amount || !check_date) { return res.status(400).json({ error: 'account_id, payee, amount, and check_date are required' }); } + if (!isEditorForAccount(req.session, parseInt(account_id, 10))) { + return res.status(403).json({ error: 'Write access required.' }); + } const account = db.prepare('SELECT current_check_no FROM account WHERE id = ?').get(account_id); if (!account) return res.status(400).json({ error: 'Account not found.' }); @@ -86,9 +89,12 @@ router.post('/', requireEditor, (req, res) => { }); // PUT /api/checks/:id - update a check (editor+) -router.put('/:id', requireEditor, (req, res) => { +router.put('/:id', (req, res) => { const check = db.prepare('SELECT * FROM checks WHERE id = ?').get(req.params.id); if (!check) return res.status(404).json({ error: 'Check not found' }); + if (!isEditorForAccount(req.session, check.account_id)) { + return res.status(403).json({ error: 'Write access required.' }); + } const { payee, amount, check_date, memo, note1, note2, payee_address1, payee_address2, payee_address3, payee_address4 } = req.body; @@ -116,19 +122,27 @@ router.put('/:id', requireEditor, (req, res) => { }); // DELETE /api/checks/:id (editor+) -router.delete('/:id', requireEditor, (req, res) => { +router.delete('/:id', (req, res) => { const check = db.prepare('SELECT * FROM checks WHERE id = ?').get(req.params.id); if (!check) return res.status(404).json({ error: 'Check not found' }); + if (!isEditorForAccount(req.session, check.account_id)) { + return res.status(403).json({ error: 'Write access required.' }); + } db.prepare('DELETE FROM checks WHERE id = ?').run(req.params.id); res.status(204).send(); }); // POST /api/checks/mark-printed (editor+) -router.post('/mark-printed', requireEditor, (req, res) => { +router.post('/mark-printed', (req, res) => { const { ids } = req.body; if (!Array.isArray(ids) || ids.length === 0) { return res.status(400).json({ error: 'ids array required' }); } + // Verify editor access via the first check's account + const first = db.prepare('SELECT account_id FROM checks WHERE id = ?').get(ids[0]); + if (!first || !isEditorForAccount(req.session, first.account_id)) { + return res.status(403).json({ error: 'Write access required.' }); + } const placeholders = ids.map(() => '?').join(','); db.prepare(`UPDATE checks SET printed = 1 WHERE id IN (${placeholders})`).run(...ids); res.json({ updated: ids.length }); diff --git a/src/routes/deposit-pdf.js b/src/routes/deposit-pdf.js index f9c60c5..92196a1 100644 --- a/src/routes/deposit-pdf.js +++ b/src/routes/deposit-pdf.js @@ -4,6 +4,7 @@ const express = require('express'); const router = express.Router(); const db = require('../db/database'); const { generateDepositPdf } = require('../services/depositPdfService'); +const { isEditorForAccount } = require('../middleware/auth'); // POST /api/deposit-pdf // Body: { depositId, type: 'slip' | 'report', mark_printed: true } @@ -17,6 +18,9 @@ router.post('/', async (req, res) => { const deposit = db.prepare('SELECT * FROM deposits WHERE id = ?').get(depositId); if (!deposit) return res.status(404).json({ error: 'Deposit not found.' }); + if (!isEditorForAccount(req.session, deposit.account_id)) { + return res.status(403).json({ error: 'Write access required.' }); + } const account = db.prepare('SELECT * FROM account WHERE id = ?').get(deposit.account_id); if (!account) return res.status(404).json({ error: 'Account not found.' }); diff --git a/src/routes/deposits.js b/src/routes/deposits.js index a233dff..f0f3766 100644 --- a/src/routes/deposits.js +++ b/src/routes/deposits.js @@ -3,7 +3,7 @@ const express = require('express'); const router = express.Router(); const db = require('../db/database'); -const { requireEditor, canAccessAccount } = require('../middleware/auth'); +const { canAccessAccount, isEditorForAccount } = require('../middleware/auth'); // Helper: fetch deposit with items function getDepositWithItems(id) { @@ -42,10 +42,13 @@ router.get('/:id', (req, res) => { }); // POST /api/deposits -router.post('/', requireEditor, (req, res) => { +router.post('/', (req, res) => { const { account_id, deposit_date, currency, coin, cash_back, items } = req.body; if (!account_id) return res.status(400).json({ error: 'account_id is required.' }); if (!deposit_date) return res.status(400).json({ error: 'deposit_date is required.' }); + if (!isEditorForAccount(req.session, parseInt(account_id, 10))) { + return res.status(403).json({ error: 'Write access required.' }); + } const insert = db.transaction(() => { const result = db.prepare(` @@ -86,9 +89,12 @@ router.post('/', requireEditor, (req, res) => { }); // PUT /api/deposits/:id -router.put('/:id', requireEditor, (req, res) => { - const existing = db.prepare('SELECT id FROM deposits WHERE id = ?').get(req.params.id); +router.put('/:id', (req, res) => { + const existing = db.prepare('SELECT id, account_id FROM deposits WHERE id = ?').get(req.params.id); if (!existing) return res.status(404).json({ error: 'Deposit not found.' }); + if (!isEditorForAccount(req.session, existing.account_id)) { + return res.status(403).json({ error: 'Write access required.' }); + } const { deposit_date, currency, coin, cash_back, items } = req.body; if (!deposit_date) return res.status(400).json({ error: 'deposit_date is required.' }); @@ -129,16 +135,23 @@ router.put('/:id', requireEditor, (req, res) => { }); // DELETE /api/deposits/:id -router.delete('/:id', requireEditor, (req, res) => { - const existing = db.prepare('SELECT id FROM deposits WHERE id = ?').get(req.params.id); +router.delete('/:id', (req, res) => { + const existing = db.prepare('SELECT id, account_id FROM deposits WHERE id = ?').get(req.params.id); if (!existing) return res.status(404).json({ error: 'Deposit not found.' }); + if (!isEditorForAccount(req.session, existing.account_id)) { + return res.status(403).json({ error: 'Write access required.' }); + } // deposit_items deleted via ON DELETE CASCADE db.prepare('DELETE FROM deposits WHERE id = ?').run(req.params.id); res.status(204).end(); }); // PATCH /api/deposits/:id/mark-printed -router.patch('/:id/mark-printed', requireEditor, (req, res) => { +router.patch('/:id/mark-printed', (req, res) => { + const existing = db.prepare('SELECT account_id FROM deposits WHERE id = ?').get(req.params.id); + if (!existing || !isEditorForAccount(req.session, existing.account_id)) { + return res.status(403).json({ error: 'Write access required.' }); + } db.prepare('UPDATE deposits SET printed = 1 WHERE id = ?').run(req.params.id); res.json({ ok: true }); }); diff --git a/src/routes/pdf.js b/src/routes/pdf.js index 99bd3d8..0166458 100644 --- a/src/routes/pdf.js +++ b/src/routes/pdf.js @@ -4,6 +4,7 @@ const express = require('express'); const router = express.Router(); const db = require('../db/database'); const { generateCheckPdf } = require('../services/pdfService'); +const { isEditorForAccount } = require('../middleware/auth'); /** * POST /api/pdf @@ -19,6 +20,9 @@ router.post('/', async (req, res) => { if (!Array.isArray(checkIds) || checkIds.length === 0) { return res.status(400).json({ error: 'checkIds must be a non-empty array' }); } + if (!isEditorForAccount(req.session, parseInt(account_id, 10))) { + return res.status(403).json({ error: 'Write access required.' }); + } // Fetch checks in the order provided let checks; diff --git a/src/routes/qbo-import.js b/src/routes/qbo-import.js index 2884829..7357680 100644 --- a/src/routes/qbo-import.js +++ b/src/routes/qbo-import.js @@ -7,6 +7,7 @@ const os = require('os'); const fs = require('fs'); const upload = multer({ dest: os.tmpdir() }); +const { isEditorForAccount } = require('../middleware/auth'); // ── CSV helpers ─────────────────────────────────────────────────────────────── @@ -291,6 +292,9 @@ router.post('/confirm', express.json(), (req, res) => { if (!type || !records || !account_id) { return res.status(400).json({ error: 'Missing required fields: type, records, account_id.' }); } + if (!isEditorForAccount(req.session, parseInt(account_id, 10))) { + return res.status(403).json({ error: 'Write access required.' }); + } if (type !== 'checks' && type !== 'deposits') { return res.status(400).json({ error: 'Invalid type.' }); } diff --git a/src/routes/users.js b/src/routes/users.js index a9f8385..e56e288 100644 --- a/src/routes/users.js +++ b/src/routes/users.js @@ -12,8 +12,7 @@ router.use(requireAuth, requireAdmin); function userWithAccounts(id) { const user = db.prepare('SELECT id, username, role, created_at FROM users WHERE id = ?').get(id); if (!user) return null; - user.accounts = db.prepare('SELECT account_id FROM user_accounts WHERE user_id = ?') - .all(id).map(r => r.account_id); + user.accounts = db.prepare('SELECT account_id, role FROM user_accounts WHERE user_id = ?').all(id); return user; } @@ -21,8 +20,7 @@ function userWithAccounts(id) { router.get('/', (req, res) => { const users = db.prepare('SELECT id, username, role, created_at FROM users ORDER BY id ASC').all(); users.forEach(u => { - u.accounts = db.prepare('SELECT account_id FROM user_accounts WHERE user_id = ?') - .all(u.id).map(r => r.account_id); + u.accounts = db.prepare('SELECT account_id, role FROM user_accounts WHERE user_id = ?').all(u.id); }); res.json(users); }); @@ -48,8 +46,8 @@ router.post('/', async (req, res) => { } if (role !== 'admin' && Array.isArray(accounts) && accounts.length > 0) { - const stmt = db.prepare('INSERT OR IGNORE INTO user_accounts (user_id, account_id) VALUES (?, ?)'); - accounts.forEach(aid => stmt.run(userId, aid)); + const stmt = db.prepare('INSERT OR IGNORE INTO user_accounts (user_id, account_id, role) VALUES (?, ?, ?)'); + accounts.forEach(a => stmt.run(userId, a.id, a.role === 'editor' ? 'editor' : 'viewer')); } res.status(201).json(userWithAccounts(userId)); @@ -92,8 +90,8 @@ router.put('/:id', async (req, res) => { db.prepare('DELETE FROM user_accounts WHERE user_id = ?').run(req.params.id); const effectiveRole = role || user.role; if (effectiveRole !== 'admin' && accounts.length > 0) { - const stmt = db.prepare('INSERT OR IGNORE INTO user_accounts (user_id, account_id) VALUES (?, ?)'); - accounts.forEach(aid => stmt.run(req.params.id, aid)); + const stmt = db.prepare('INSERT OR IGNORE INTO user_accounts (user_id, account_id, role) VALUES (?, ?, ?)'); + accounts.forEach(a => stmt.run(req.params.id, a.id, a.role === 'editor' ? 'editor' : 'viewer')); } }