From 693ac693a426d50c44fa5fd549bf5a8ba68dcc57 Mon Sep 17 00:00:00 2001 From: Gmer4Lfe Date: Sat, 15 Aug 2026 11:30:36 -0400 Subject: [PATCH] Make access-control domains an editable list, and stop saves eating the rule labels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The block is rebuilt from the parsed model on every save and the parser discarded comments, so one press of Save deleted the five ## lines that are the only thing in the file saying what each rule is for — which inline editing was about to make far easier to press. --- Plugin/unraid/api/auth.php | 4 +- Plugin/unraid/include/auth.php | 75 +++++++++++--- Plugin/unraid/pages/auth.php | 181 +++++++++++++++++++++++++++++---- 3 files changed, 227 insertions(+), 33 deletions(-) diff --git a/Plugin/unraid/api/auth.php b/Plugin/unraid/api/auth.php index 3aa040c..5abe0e5 100644 --- a/Plugin/unraid/api/auth.php +++ b/Plugin/unraid/api/auth.php @@ -131,7 +131,9 @@ $result = match ($action) { 'lldap_add_to_group' => vv_lldap_add_to_group($_POST['uid'] ?? '', (int)($_POST['gid'] ?? 0)), 'lldap_remove_from_group' => vv_lldap_remove_from_group($_POST['uid'] ?? '', (int)($_POST['gid'] ?? 0)), // Authelia - 'authelia_save' => vv_authelia_write_rules(json_decode($_POST['rules'] ?? '[]', true) ?: [], $_POST['default_policy'] ?? 'deny'), + 'authelia_save' => vv_authelia_write_rules(json_decode($_POST['rules'] ?? '[]', true) ?: [], + $_POST['default_policy'] ?? 'deny', + $_POST['default_note'] ?? ''), default => ['ok' => false, 'error' => 'Unknown action: ' . $action], }; diff --git a/Plugin/unraid/include/auth.php b/Plugin/unraid/include/auth.php index 84517ed..a9f8ef0 100644 --- a/Plugin/unraid/include/auth.php +++ b/Plugin/unraid/include/auth.php @@ -390,10 +390,16 @@ function vv_authelia_read_rules(): array { $content = file_get_contents($file); if ($content === false) return ['ok' => false, 'error' => 'Cannot read config file']; - // Extract default_policy (strip inline comments) + // Extract default_policy, and keep any trailing comment rather than dropping it. The line in + // this config reads "default_policy: bypass #deny" — a note about what it used to be, or is + // meant to become, on the single most consequential setting in the file. The block is + // re-emitted on save, so anything not carried here is deleted by the next save. $defaultPolicy = 'deny'; - if (preg_match('/^[ \t]+default_policy:[ \t]+([a-z_]+)/m', $content, $m)) + $defaultNote = ''; + if (preg_match('/^[ \t]+default_policy:[ \t]+([a-z_]+)[ \t]*(#[^\n]*)?/m', $content, $m)) { $defaultPolicy = $m[1]; + $defaultNote = trim($m[2] ?? ''); + } // Extract the indented block under access_control: if (!preg_match('/^access_control:[ \t]*\n((?:[ \t][^\n]*\n?)*)/m', $content, $m)) @@ -403,18 +409,46 @@ function vv_authelia_read_rules(): array { // Extract the indented block under rules: (3+ space indent = rule list items) if (!preg_match('/^ rules:[ \t]*\n((?:[ \t]{3,}[^\n]*\n?)*)/m', $acBlock, $m)) - return ['ok' => true, 'default_policy' => $defaultPolicy, 'rules' => []]; + return ['ok' => true, 'default_policy' => $defaultPolicy, 'rules' => [], + 'default_note' => $defaultNote]; - // Split into individual rule chunks at " - " (indent-4 rule starts) - $chunks = preg_split('/(?=^ - )/m', $m[1]); - $rules = []; - foreach ($chunks as $chunk) { - if (!preg_match('/^ - /', $chunk)) continue; - $rule = vv_authelia_parse_rule_chunk($chunk); - if (!empty($rule)) $rules[] = $rule; + // Walked rather than preg_split, so the comment lines above each rule can be attached to it. + // + // This block is rebuilt from the parsed model on every save, so anything the parser drops is + // deleted the next time anyone touches this page — and the parser dropped every comment. The + // five rules here are labelled ## Media_Users_users, ## Admin Only, ## super_users, + // ## power_users and ## Home_users, which is the only thing in the file that says what a rule + // is *for*: the rule itself is thirteen hostnames and a group id. Saving once erased all five. + // + // A lookahead split cannot do this, because the comment above rule N lands at the end of rule + // N-1's chunk (or before the first chunk entirely), so it would be attributed to the wrong + // rule or lost with the preamble. + $lines = explode("\n", $m[1]); + $starts = []; + foreach ($lines as $i => $l) if (preg_match('/^ - /', $l)) $starts[] = $i; + + $rules = []; + foreach ($starts as $n => $s) { + // Contiguous comment lines immediately above this rule, in file order. A blank line or + // any content ends the run — a comment separated from the rule by a blank belongs to the + // block, not to the rule. + $label = []; + for ($j = $s - 1; $j >= 0; $j--) { + if (!preg_match('/^\s*#/', $lines[$j])) break; + array_unshift($label, trim($lines[$j])); + } + $end = $starts[$n + 1] ?? count($lines); + $chunk = implode("\n", array_slice($lines, $s, $end - $s)); + $rule = vv_authelia_parse_rule_chunk($chunk); + if (empty($rule)) continue; + // Underscore-prefixed so it cannot collide with an Authelia field name, and so the writer + // can tell presentation from configuration when it decides what to emit as YAML. + if ($label) $rule['_label'] = $label; + $rules[] = $rule; } - return ['ok' => true, 'default_policy' => $defaultPolicy, 'rules' => $rules]; + return ['ok' => true, 'default_policy' => $defaultPolicy, 'rules' => $rules, + 'default_note' => $defaultNote]; } function vv_authelia_parse_rule_chunk(string $chunk): array { @@ -488,7 +522,7 @@ function vv_authelia_parse_list_item(string $val): string { return vv_authelia_unquote($val); } -function vv_authelia_write_rules(array $rules, string $defaultPolicy): array { +function vv_authelia_write_rules(array $rules, string $defaultPolicy, string $defaultNote = ''): array { $conf = vv_auth_conf(); $file = $conf['authelia_config']; if (!file_exists($file)) return ['ok' => false, 'error' => 'Config not found: ' . $file]; @@ -498,17 +532,32 @@ function vv_authelia_write_rules(array $rules, string $defaultPolicy): array { // Build the new access_control block $block = "access_control:\n"; - $block .= " default_policy: $defaultPolicy\n"; + $block .= " default_policy: $defaultPolicy" . ($defaultNote !== '' ? ' ' . $defaultNote : '') . "\n"; $block .= " rules:\n"; // Preferred field output order $fieldOrder = ['domain', 'policy', 'subject', 'networks', 'resources']; foreach ($rules as $rule) { + // The labels the operator wrote above this rule, put back before it. Emitted here rather + // than inside the field loop because they are not a field — they carry no indent-4 dash + // and must land above the rule, not inside it. + foreach ((array) ($rule['_label'] ?? []) as $lbl) { + $lbl = trim((string) $lbl); + if ($lbl === '') continue; + // Forced back into comment form. This string reaches here from the browser, and a + // label that lost its # would be spliced into the config as YAML. + if ($lbl[0] !== '#') $lbl = '# ' . $lbl; + // One line only — a newline here would end the comment and start config. + $block .= ' ' . str_replace(["\r", "\n"], ' ', $lbl) . "\n"; + } $keys = array_merge( array_filter($fieldOrder, fn($k) => array_key_exists($k, $rule)), array_diff(array_keys($rule), $fieldOrder) ); + // Presentation, already emitted above. Left in the key list it would be written out as a + // YAML field named _label, which Authelia would reject on load. + $keys = array_filter($keys, fn($k) => $k !== '_label'); $first = true; foreach ($keys as $key) { if (!array_key_exists($key, $rule)) continue; diff --git a/Plugin/unraid/pages/auth.php b/Plugin/unraid/pages/auth.php index 6cca551..2397f03 100644 --- a/Plugin/unraid/pages/auth.php +++ b/Plugin/unraid/pages/auth.php @@ -134,10 +134,26 @@ require_once dirname(__DIR__) . '/include/ai_chat.php'; .vv-au-rule-meta { font-size:9px;color:#3a3a3a;text-transform:uppercase;letter-spacing:.06em;margin-bottom:5px; display:flex;justify-content:space-between;gap:8px; } .vv-au-rule-sfx { color:#4a4a4a;text-transform:none;letter-spacing:0;font-family:monospace; } -.vv-au-doms { display:flex;flex-wrap:wrap;gap:3px; } -.vv-au-dom { font-size:10px;padding:1px 5px;border-radius:2px;background:#0f1419;border:1px solid #1e2a33; - color:#7c9fb8;font-family:monospace;white-space:nowrap; } -.vv-au-dom.wild { background:#1a1000;border-color:#3a2800;color:#c9a227; } +.vv-au-rule-lbl { color:#5a6a4a;text-transform:none;letter-spacing:0;font-style:italic; + overflow:hidden;text-overflow:ellipsis;white-space:nowrap;min-width:0; } +/* One row per domain, each an input. Chips read well and cannot be edited; this is a list you + work in, so the row is the field rather than a label that opens a dialog somewhere else. */ +.vv-au-domlist { display:flex;flex-direction:column;gap:2px; } +.vv-au-domrow { display:flex;align-items:center;gap:4px; } +.vv-au-dominput { flex:1;min-width:0;background:#0f1419;border:1px solid #1e2a33;border-radius:2px; + color:#7c9fb8;font-family:monospace;font-size:10px;padding:2px 5px;outline:none; } +.vv-au-dominput:focus { border-color:#2d5a8a;background:#111820;color:#a8cde0; } +.vv-au-dominput.wild { color:#c9a227;border-color:#3a2800; } +/* An empty row would be written out as a blank domain, so it is marked while it is still on + screen rather than silently dropped at save time. */ +.vv-au-dominput.blank { border-color:#3a1a1a;background:#1a0d0d; } +.vv-au-domadd { font-size:10px;color:#4a7a4a;background:none;border:1px dashed #23331f;border-radius:2px; + padding:2px 6px;cursor:pointer;margin-top:4px;width:100%;text-align:center; } +.vv-au-domadd:hover { color:#7ac77a;border-color:#2d5a2d;background:#0d1a0d; } +/* The unsaved marker. Inline editing makes it very easy to change three cards and walk away, and + nothing here reaches Authelia until Save & Restart is pressed. */ +.vv-au-btn.dirty { border-color:#3a2800;background:#1f1200;color:#ffb74d; } +.vv-au-dirty-note{ font-size:10px;color:#ffb74d;margin-right:8px; } .vv-au-rule-x { margin-top:7px;padding-top:6px;border-top:1px solid #1c1c1c;font-size:10px;color:#555; display:flex;flex-direction:column;gap:2px; } .vv-au-rule-x b { color:#3a3a3a;font-weight:normal;text-transform:uppercase;font-size:9px;letter-spacing:.06em; } @@ -262,7 +278,8 @@ $isOwner = vv_is_owner(); default policy editable on HOST1 - + +
@@ -344,6 +361,9 @@ const IS_OWNER = ; let _proxies = [], _certs = []; let _users = [], _groups = []; let _rules = [], _defaultPolicy = 'deny'; +// The trailing comment on the default_policy line, carried so a save puts it back. The block is +// rebuilt from this model, so anything not held here is deleted by the next save. +let _defaultNote = ''; let _activeTab = 'proxies'; // ── Helpers ─────────────────────────────────────────────────────────────────── @@ -894,6 +914,10 @@ function _loadAcl() { if (!r.ok) { loading.innerHTML = ''+_esc(r.error)+''; loading.style.display='block'; return; } _rules = r.rules || []; _defaultPolicy = r.default_policy || 'deny'; + _defaultNote = r.default_note || ''; + // Freshly loaded is by definition not modified — this also resets the marker after a reload + // that followed an abandoned edit. + _aclMarkDirty(false); const sel = document.getElementById('vv-au-ac-defpol'); if (sel) sel.value = _defaultPolicy; @@ -917,9 +941,16 @@ function _renderAcl() { if (emptyEl) emptyEl.style.display = _rules.length ? 'none' : 'block'; body.innerHTML = _rules.map((rule, i) => { - const doms = _normDomain(rule.domain); + // Normalised in place so a domain has a stable index to edit against. Authelia allows the + // scalar form and this config uses it, but "rule 1's third domain" has to mean something for + // an inline editor to address it at all. + if (!Array.isArray(rule.domain)) rule.domain = _normDomain(rule.domain); + const doms = rule.domain; const subjs = _normSubject(rule.subject); - const sfx = _commonSuffix(doms); + // The comment the operator wrote above this rule in configuration.yml. It is the only thing in + // the file that says what a rule is for — the rule itself is a group id and thirteen hostnames + // — so it belongs on the card rather than only in the file it came from. + const label = _normList(rule._label).map(s => String(s).replace(/^#+\s*/, '')).join(' · '); // No subject means the rule applies to everyone who reaches that domain, which is the single // most consequential thing a rule can say and read as an empty cell in the table it replaced. @@ -927,12 +958,18 @@ function _renderAcl() { ? subjs.map(s => `${_esc(_subjLabel(s))}`).join('') : 'anyone'; - const domHtml = doms.map(d => { - const full = String(d); - const short = (sfx && full.endsWith(sfx)) ? full.slice(0, -sfx.length) : full; - // A wildcard covers everything under it, so it is worth spotting among its neighbours. - const wild = full.includes('*') ? ' wild' : ''; - return `${_esc(short)}`; + // Full domain in every field, not the shortened form the chips used. A shortened value in an + // editable box is a value that has to be reassembled before it means anything, and this is the + // page where a wrong domain hands a service to the wrong group. + const domHtml = doms.map((d, j) => { + const full = String(d); + const cls = (full.includes('*') ? ' wild' : '') + (full.trim() === '' ? ' blank' : ''); + return `
+ + +
`; }).join(''); // Both were invisible in the table — there was no column for them — so a rule narrowed to one @@ -962,9 +999,10 @@ function _renderAcl() {
${doms.length} domain${doms.length !== 1 ? 's' : ''} - ${sfx ? `${_esc(sfx)}` : ''} + ${label ? `${_esc(label)}` : ''}
-
${domHtml}
+
${domHtml}
+ ${extra}
`; @@ -1053,27 +1091,127 @@ function _ruleModal(idx) { else if (subjects.length) newRule.subject = subjects.length === 1 ? subjects[0] : subjects; if (networks.length) newRule.networks = networks; if (resources.length) newRule.resources = resources; + // The comment lines above this rule in configuration.yml — ## Admin Only and the rest. The + // dialog does not show them and rebuilds the rule from scratch, so without this, editing a + // rule's policy would delete the only line in the file that says what the rule is for. + if (rule && rule._label) newRule._label = rule._label; if (idx !== null) _rules[idx] = newRule; else _rules.push(newRule); _closeModal(); + _aclMarkDirty(true); _renderAcl(); }; } +// ── Unsaved state ──────────────────────────────────────────────────────────── +// Nothing on this tab reaches Authelia until Save & Restart is pressed — every edit mutates the +// in-memory rules and re-renders. That was tolerable when the only ways to change anything were a +// modal and a delete confirmation; with the domain rows editable in place it is far too easy to +// change three cards, switch tabs and lose the lot with no indication anything was pending. +let _aclDirty = false; +function _aclMarkDirty(on) { + _aclDirty = on; + const btn = document.getElementById('vv-au-ac-save'); + if (btn) btn.classList.toggle('dirty', on); + const note = document.getElementById('vv-au-ac-dirty'); + if (note) note.textContent = on ? 'unsaved changes' : ''; +} + +// Writes straight into the model and deliberately does not re-render: the element being typed in +// is inside the markup a render would replace, which would drop focus on the first keystroke. +document.getElementById('vv-au-panel-acl').addEventListener('input', e => { + const inp = e.target.closest('.vv-au-dominput'); + if (!inp) return; + const r = parseInt(inp.dataset.rule), d = parseInt(inp.dataset.dom); + if (!_rules[r] || !Array.isArray(_rules[r].domain)) return; + _rules[r].domain[d] = inp.value; + inp.classList.toggle('blank', inp.value.trim() === ''); + inp.classList.toggle('wild', inp.value.includes('*')); + _aclMarkDirty(true); +}); + +// Enter adds a row underneath and moves to it, so a list of fourteen can be typed straight +// through instead of returning to the add button between each one. +document.getElementById('vv-au-panel-acl').addEventListener('keydown', e => { + const inp = e.target.closest('.vv-au-dominput'); + if (!inp || e.key !== 'Enter') return; + e.preventDefault(); + const r = parseInt(inp.dataset.rule), d = parseInt(inp.dataset.dom); + if (!_rules[r]) return; + _rules[r].domain.splice(d + 1, 0, ''); + _aclMarkDirty(true); + _renderAcl(); + _focusDomain(r, d + 1); +}); + +function _focusDomain(r, d) { + const el = document.querySelector(`.vv-au-dominput[data-rule="${r}"][data-dom="${d}"]`); + if (el) { el.focus(); el.select(); } +} + // ACL event delegation document.getElementById('vv-au-panel-acl').addEventListener('click', async e => { if (e.target.id === 'vv-au-rule-add') { _ruleModal(null); return; } + const domAdd = e.target.closest('[data-dom-add]'); + if (domAdd) { + const r = parseInt(domAdd.dataset.domAdd); + if (!_rules[r]) return; + _rules[r].domain.push(''); + _aclMarkDirty(true); + _renderAcl(); + _focusDomain(r, _rules[r].domain.length - 1); + return; + } + + const domDel = e.target.closest('[data-dom-del]'); + if (domDel) { + const [r, d] = domDel.dataset.domDel.split(':').map(Number); + if (!_rules[r]) return; + // A rule with no domain matches nothing and Authelia will not load it. Refused here rather + // than allowed and caught at save, so the answer arrives while the rule is still on screen. + if (_rules[r].domain.length <= 1) { + vvAlert('A rule needs at least one domain. Delete the whole rule instead, with the ✕ in its header.'); + return; + } + _rules[r].domain.splice(d, 1); + _aclMarkDirty(true); + _renderAcl(); + return; + } + if (e.target.id === 'vv-au-ac-save') { const dp = document.getElementById('vv-au-ac-defpol').value; const btn = document.getElementById('vv-au-ac-save'); + + // Trimmed and emptied out here rather than on every keystroke, so a row can legitimately be + // blank while it is being typed into. A rule left with no domain at all is refused instead of + // written, because Authelia will not load the file and the failure would land at container + // restart — with everything behind it already down. + const payload = _rules.map(rule => { + const out = Object.assign({}, rule); + const doms = _normList(rule.domain).map(d => String(d).trim()).filter(Boolean); + out.domain = doms.length === 1 ? doms[0] : doms; + return out; + }); + const empty = payload.findIndex(r => !_normList(r.domain).length); + if (empty !== -1) { + vvAlert('Rule ' + (empty + 1) + ' has no domains left. Give it one, or delete the rule.'); + return; + } + btn.disabled = true; btn.textContent = 'Saving…'; - _post({ action:'authelia_save', rules: JSON.stringify(_rules), default_policy: dp }, r => { + _post({ action:'authelia_save', rules: JSON.stringify(payload), default_policy: dp, + default_note: _defaultNote }, r => { btn.disabled = false; btn.textContent = 'Save & Restart Authelia'; if (!r.ok) { vvAlert('Save failed: ' + (r.error||'unknown error')); return; } - // brief visual confirmation + // The model on screen is the model on disk now — including the trimming just applied, which + // is why the rules are replaced rather than left as typed. + _rules = payload; + _aclMarkDirty(false); + _renderAcl(); btn.textContent = 'Saved ✓'; setTimeout(() => { btn.textContent = 'Save & Restart Authelia'; }, 2000); }); @@ -1088,6 +1226,7 @@ document.getElementById('vv-au-panel-acl').addEventListener('click', async e => const i = parseInt(delBtn.dataset.ruleDel); if (!await vvConfirm('Delete this rule?')) return; _rules.splice(i, 1); + _aclMarkDirty(true); _renderAcl(); return; } @@ -1095,17 +1234,21 @@ document.getElementById('vv-au-panel-acl').addEventListener('click', async e => const upBtn = e.target.closest('[data-rule-up]'); if (upBtn) { const i = parseInt(upBtn.dataset.ruleUp); - if (i > 0) { [_rules[i-1], _rules[i]] = [_rules[i], _rules[i-1]]; _renderAcl(); } + if (i > 0) { [_rules[i-1], _rules[i]] = [_rules[i], _rules[i-1]]; _aclMarkDirty(true); _renderAcl(); } return; } const dnBtn = e.target.closest('[data-rule-dn]'); if (dnBtn) { const i = parseInt(dnBtn.dataset.ruleDn); - if (i < _rules.length-1) { [_rules[i], _rules[i+1]] = [_rules[i+1], _rules[i]]; _renderAcl(); } + if (i < _rules.length-1) { [_rules[i], _rules[i+1]] = [_rules[i+1], _rules[i]]; _aclMarkDirty(true); _renderAcl(); } } }); +// The default policy is the one control here that is not a rule, and it is the most consequential +// one on the tab — it decides what happens to every hostname no rule names. +document.getElementById('vv-au-ac-defpol').addEventListener('change', () => _aclMarkDirty(true)); + // ── Close modal on overlay click ────────────────────────────────────────────── document.getElementById('vv-au-overlay').addEventListener('click', e => { if (e.target === document.getElementById('vv-au-overlay')) _closeModal();