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

@@ -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;

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'
-        ? '<em style="color:var(--text-muted)">All accounts</em>'
+        ? '<em style="color:var(--text-muted)">All accounts (editor)</em>'
-        : (u.accounts.length ? u.accounts.map(aid => {
+        : (u.accounts.length ? u.accounts.map(ua => {
-            const a = state.accounts.find(x => x.id === aid);
+            const a = state.accounts.find(x => x.id === ua.account_id);
-            return escHtml(a ? (a.company1 || `Account ${a.id}`) : `#${aid}`);
+            const name = escHtml(a ? (a.company1 || `Account ${a.account_id}`) : `#${ua.account_id}`);
+            return `${name} <span style="font-size:10px;color:${ua.role === 'editor' ? '#16a34a' : '#6b7280'};font-weight:600;text-transform:uppercase">${ua.role}</span>`;
           }).join(', ') : '<em style="color:var(--text-muted)">None</em>');
       return `<tr>
         <td><strong>${escHtml(u.username)}</strong>${isSelf ? ' <em style="color:var(--text-muted)">(you)</em>' : ''}</td>
@@ -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 =>
+  container.innerHTML = state.accounts.map(a => {
-    `<label class="account-checkbox-label">
+    const assignment = currentAccounts.find(x => x.account_id === a.id);
-      <input type="checkbox" name="uf-account" value="${a.id}"${currentAccounts.includes(a.id) ? ' checked' : ''}>
+    const checked = !!assignment;
+    const acctRole = assignment ? assignment.role : 'viewer';
+    return `<label class="account-checkbox-label">
+      <input type="checkbox" name="uf-account" value="${a.id}"${checked ? ' checked' : ''}>
       ${escHtml(a.company1 || a.bank_name || `Account ${a.id}`)}
-    </label>`
+      <select name="uf-account-role" data-account-id="${a.id}" style="margin-left:6px;font-size:12px">
-  ).join('');
+        <option value="editor"${acctRole === 'editor' ? ' selected' : ''}>Editor</option>
+        <option value="viewer"${acctRole === 'viewer' ? ' selected' : ''}>Viewer</option>
+      </select>
+    </label>`;
+  }).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
     ? `<td class="col-select"><input type="checkbox" data-id="${c.id}"${selected ? ' checked' : ''}></td>`

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+) ─────────────────────────────────────────────────────────────
+// ── PDF (per-account editor check inside route) ───────────────────────────────
-app.use('/api/pdf', requireEditor, require('./routes/pdf'));
+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+) ──────────────────────────────────────────────────────
+// ── QBO import (per-account editor check inside route) ────────────────────────
-app.use('/api/qbo-import', requireEditor, require('./routes/qbo-import'));
+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 = ?

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);

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)
 );
 

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 };

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 });

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.' });

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) => {
+router.put('/:id', (req, res) => {
-  const existing = db.prepare('SELECT id FROM deposits WHERE id = ?').get(req.params.id);
+  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) => {
+router.delete('/:id', (req, res) => {
-  const existing = db.prepare('SELECT id FROM deposits WHERE id = ?').get(req.params.id);
+  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 });
 });

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;

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.' });
   }

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 = ?')
+  user.accounts = db.prepare('SELECT account_id, role FROM user_accounts WHERE user_id = ?').all(id);
-    .all(id).map(r => r.account_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 = ?')
+    u.accounts = db.prepare('SELECT account_id, role FROM user_accounts WHERE user_id = ?').all(u.id);
-      .all(u.id).map(r => r.account_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 (?, ?)');
+    const stmt = db.prepare('INSERT OR IGNORE INTO user_accounts (user_id, account_id, role) VALUES (?, ?, ?)');
-    accounts.forEach(aid => stmt.run(userId, aid));
+    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 (?, ?)');
+      const stmt = db.prepare('INSERT OR IGNORE INTO user_accounts (user_id, account_id, role) VALUES (?, ?, ?)');
-      accounts.forEach(aid => stmt.run(req.params.id, aid));
+      accounts.forEach(a => stmt.run(req.params.id, a.id, a.role === 'editor' ? 'editor' : 'viewer'));
     }
   }