diff --git a/public/js/settings.js b/public/js/settings.js index 13818f2c..e9972856 100644 --- a/public/js/settings.js +++ b/public/js/settings.js @@ -1455,6 +1455,7 @@ if (enabledEl) enabledEl.classList.toggle('on', !!cfg.enabled); if (chainEl) chainEl.classList.toggle('on', !!cfg.manage_dns_chain); if (intervalEl) intervalEl.value = cfg.sync_interval_sec || 30; + var countEl = document.getElementById('pihole-top-clients-count'); if (countEl) countEl.value = cfg.top_clients_count || 1000; phInstances = (cfg.instances || []).slice(); renderInstances(); if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('pihole'); @@ -1713,10 +1714,12 @@ var enabledEl = document.getElementById('pihole-enabled'); var chainEl = document.getElementById('pihole-manage-chain'); var intervalEl = document.getElementById('pihole-sync-interval'); + var countEl = document.getElementById('pihole-top-clients-count'); var payload = { enabled: enabledEl ? enabledEl.classList.contains('on') : false, manage_dns_chain: chainEl ? chainEl.classList.contains('on') : false, sync_interval_sec: intervalEl ? (parseInt(intervalEl.value, 10) || 30) : 30, + top_clients_count: countEl ? (parseInt(countEl.value, 10) || 1000) : 1000, instances: phInstances.map(function (inst) { var out = { id: inst.id, @@ -1740,15 +1743,17 @@ } var phIntervalEl = document.getElementById('pihole-sync-interval'); + var phCountEl = document.getElementById('pihole-top-clients-count'); SettingsAutosave.bind({ cluster: 'pihole', - fields: [enabledToggle, chainToggle, phIntervalEl].filter(Boolean), + fields: [enabledToggle, chainToggle, phIntervalEl, phCountEl].filter(Boolean), statusEl: document.getElementById('pihole-status'), valuesById: function () { return { 'pihole-enabled': enabledToggle ? enabledToggle.classList.contains('on') : false, 'pihole-manage-chain': chainToggle ? chainToggle.classList.contains('on') : false, 'pihole-sync-interval': phIntervalEl ? phIntervalEl.value : '30', + 'pihole-top-clients-count': phCountEl ? phCountEl.value : '1000', }; }, save: function () { return savePihole(false); }, diff --git a/src/i18n/de.json b/src/i18n/de.json index 19ac4723..5e649a8f 100644 --- a/src/i18n/de.json +++ b/src/i18n/de.json @@ -1917,6 +1917,8 @@ "pihole.cfg.manage_chain": "DNS-Kette verwalten", "pihole.cfg.manage_chain_hint": "GateControl setzt Pi-hole als DNS-Upstream in WireGuard. Änderungen erfordern einen WireGuard-Neustart.", "pihole.cfg.sync_interval": "Synchronisationsintervall (Sekunden)", + "pihole.cfg.top_clients_count": "Abzurufende Top-Clients", + "pihole.cfg.top_clients_count_hint": "Wie viele Top-Clients Pi-hole pro Sync liefert — höher = vollständigere pro-Gerät/pro-Besitzer-Statistik. Default 1000.", "pihole.cfg.instances_title": "Pi-hole-Instanzen", "pihole.cfg.no_instances": "Keine Instanzen konfiguriert", "pihole.cfg.add_instance": "Instanz hinzufügen", diff --git a/src/i18n/en.json b/src/i18n/en.json index 80deee77..85874dbf 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -1917,6 +1917,8 @@ "pihole.cfg.manage_chain": "Manage DNS chain", "pihole.cfg.manage_chain_hint": "GateControl will set Pi-hole as the DNS upstream in WireGuard. Changes require a WireGuard restart.", "pihole.cfg.sync_interval": "Sync interval (seconds)", + "pihole.cfg.top_clients_count": "Top clients to fetch", + "pihole.cfg.top_clients_count_hint": "How many top clients Pi-hole returns per sync — higher = more complete per-device/per-owner stats. Default 1000.", "pihole.cfg.instances_title": "Pi-hole Instances", "pihole.cfg.no_instances": "No instances configured", "pihole.cfg.add_instance": "Add instance", diff --git a/src/routes/api/pihole.js b/src/routes/api/pihole.js index d24d389c..eba77b9f 100644 --- a/src/routes/api/pihole.js +++ b/src/routes/api/pihole.js @@ -43,7 +43,9 @@ router.get('/top-domains', (req, res) => { router.get('/top-clients', (req, res) => { const cache = pihole.getCache(); - res.json({ ok: true, data: scopeFilter(req, cache.topClients) }); + // Display cap: the monitoring card is a top-10 widget; cache.topClients may now hold + // up to top_clients_count for portal.js per-device/per-owner attribution. + res.json({ ok: true, data: scopeFilter(req, cache.topClients || []).slice(0, 10) }); }); router.get('/query-types', (req, res) => { diff --git a/src/routes/api/settings/pihole.js b/src/routes/api/settings/pihole.js index 774bd0c5..2d0e9a88 100644 --- a/src/routes/api/settings/pihole.js +++ b/src/routes/api/settings/pihole.js @@ -44,6 +44,7 @@ router.put('/pihole', requireFeature('pihole_integration'), (req, res) => { enabled: !!body.enabled, sync_interval_sec: Number(body.sync_interval_sec) || 30, manage_dns_chain: body.manage_dns_chain !== false, + top_clients_count: Math.max(1, Math.min(5000, parseInt(body.top_clients_count, 10) || 1000)), instances, }); diff --git a/src/routes/api/users.js b/src/routes/api/users.js index 2d8ecbe6..a4588ec2 100644 --- a/src/routes/api/users.js +++ b/src/routes/api/users.js @@ -155,6 +155,10 @@ router.delete('/:id', (req, res) => { return res.status(400).json({ ok: false, error: req.t('error.users.self_delete') }); } users.remove(id); + // Invalidate the deleted user's sessions (route-level, like the password-change flow). + if (req.sessionStore && typeof req.sessionStore.destroyByUserId === 'function') { + req.sessionStore.destroyByUserId(id); + } res.json({ ok: true }); } catch (err) { logger.error({ error: err.message }, 'Failed to delete user'); diff --git a/src/services/piholeClient.js b/src/services/piholeClient.js index 0e32cc93..dee276ab 100644 --- a/src/services/piholeClient.js +++ b/src/services/piholeClient.js @@ -146,8 +146,12 @@ function createClient(instance) { return (r.domains || []).map(d => ({ domain: d.domain, count: d.count })); } - async function getTopClients(blocked = false) { - const r = await request(`/api/stats/top_clients${blocked ? '?blocked=true' : ''}`); + async function getTopClients(blocked = false, count) { + const params = []; + if (blocked) params.push('blocked=true'); + if (Number.isInteger(count) && count > 0) params.push('count=' + count); + const qs = params.length ? '?' + params.join('&') : ''; + const r = await request(`/api/stats/top_clients${qs}`); return (r.clients || []).map(c => ({ ip: c.ip, count: c.count })); } diff --git a/src/services/piholeConfig.js b/src/services/piholeConfig.js index e22d0f2a..4f9f4857 100644 --- a/src/services/piholeConfig.js +++ b/src/services/piholeConfig.js @@ -9,12 +9,13 @@ const DEFAULT = { enabled: false, sync_interval_sec: 30, manage_dns_chain: true, + top_clients_count: 1000, instances: [], }; /** * Load pihole config from settings, decrypting each instance's app_password. - * @returns {{ enabled: boolean, sync_interval_sec: number, manage_dns_chain: boolean, instances: Array }} + * @returns {{ enabled: boolean, sync_interval_sec: number, manage_dns_chain: boolean, top_clients_count: number, instances: Array }} */ function load() { const raw = settings.get(KEY); @@ -31,7 +32,7 @@ function load() { /** * Save pihole config to settings, encrypting each instance's app_password. - * @param {{ enabled: boolean, sync_interval_sec: number, manage_dns_chain: boolean, instances: Array }} config + * @param {{ enabled: boolean, sync_interval_sec: number, manage_dns_chain: boolean, top_clients_count: number, instances: Array }} config */ function save(config) { const instances = (config.instances || []).map((inst) => ({ diff --git a/src/services/piholeSync.js b/src/services/piholeSync.js index 389c21e8..a0ca596c 100644 --- a/src/services/piholeSync.js +++ b/src/services/piholeSync.js @@ -71,17 +71,17 @@ function createSync(deps) { /** * Pull all data from a single client in parallel. */ - async function pull(client) { + async function pull(client, count) { const [summary, history, topDomains, topClients, topClientsBlocked, queryTypes, blocking] = await Promise.all([ client.getSummary(), client.getHistory(), client.getTopDomains(true), - client.getTopClients(), + client.getTopClients(false, count), // NOTE: getTopClients(true) requires Pi-hole v6 FTL. On a v5 instance this call // throws; the .catch(() => []) degrades silently to an empty blocked list instead // of rejecting pull() entirely (which would mark the instance connected:false). - client.getTopClients(true).catch(() => []), + client.getTopClients(true, count).catch(() => []), client.getQueryTypes(), client.getBlocking(), ]); @@ -148,7 +148,8 @@ function createSync(deps) { if (!activeIds.has(id)) clientCache.delete(id); } const clients = config.instances.map(getOrCreateClient); - const results = await Promise.allSettled(clients.map(pull)); + const count = config.top_clients_count || 1000; + const results = await Promise.allSettled(clients.map(c => pull(c, count))); // Build per-instance metadata and collect fulfilled data const instances = []; @@ -178,9 +179,9 @@ function createSync(deps) { const peerIps = peers.map(p => p.ip); const summary = mergeSummary(ok.map(r => r.summary)); - const topClientsRaw = mergeTopList(ok.map(r => r.topClients), 'ip', 10); + const topClientsRaw = mergeTopList(ok.map(r => r.topClients), 'ip', count); const topClients = mapClientsToPeers(topClientsRaw, peersByIp); - const topClientsBlockedRaw = mergeTopList(ok.map(r => r.topClientsBlocked ?? []), 'ip', 10); + const topClientsBlockedRaw = mergeTopList(ok.map(r => r.topClientsBlocked ?? []), 'ip', count); const topClientsBlocked = mapClientsToPeers(topClientsBlockedRaw, peersByIp); const blocking = mergeBlocking(perInstanceBlocking.filter(Boolean)); const history = mergeHistory(ok.map(r => r.history), config.sync_interval_sec || 60); diff --git a/templates/aurora/pages/settings.njk b/templates/aurora/pages/settings.njk index d897fc87..85b2c108 100644 --- a/templates/aurora/pages/settings.njk +++ b/templates/aurora/pages/settings.njk @@ -890,10 +890,15 @@
{{ t('pihole.cfg.manage_chain_hint') }}
-
+
+
+ + +
{{ t('pihole.cfg.top_clients_count_hint') }}
+
diff --git a/templates/default/pages/settings.njk b/templates/default/pages/settings.njk index caa95d27..e77fb0d1 100644 --- a/templates/default/pages/settings.njk +++ b/templates/default/pages/settings.njk @@ -1056,10 +1056,15 @@
{{ t('pihole.cfg.manage_chain_hint') }}
-
+
+
+ + +
{{ t('pihole.cfg.top_clients_count_hint') }}
+
diff --git a/templates/pro/pages/settings.njk b/templates/pro/pages/settings.njk index d34b85bf..f14f8dad 100644 --- a/templates/pro/pages/settings.njk +++ b/templates/pro/pages/settings.njk @@ -942,10 +942,15 @@
{{ t('pihole.cfg.manage_chain_hint') }}
-
+
+
+ + +
{{ t('pihole.cfg.top_clients_count_hint') }}
+
diff --git a/tests/pihole_topclients_count.test.js b/tests/pihole_topclients_count.test.js new file mode 100644 index 00000000..35170314 --- /dev/null +++ b/tests/pihole_topclients_count.test.js @@ -0,0 +1,38 @@ +'use strict'; +const crypto = require('crypto'); +process.env.GC_ENCRYPTION_KEY = process.env.GC_ENCRYPTION_KEY || crypto.randomBytes(32).toString('hex'); +const { test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const http = require('node:http'); +const { createClient } = require('../src/services/piholeClient'); + +let server, lastTopClientsUrl; +function start(handler){ return new Promise(r=>{ server=http.createServer(handler); server.listen(0,()=>r()); }); } +function baseUrl(){ return `http://127.0.0.1:${server.address().port}`; } +beforeEach(()=>{ lastTopClientsUrl=null; }); +afterEach(()=> new Promise(r=>server.close(r))); + +async function handler(req,res){ + if (req.url==='/api/auth'){ res.end(JSON.stringify({ session:{ sid:'S', csrf:'c', validity:300, valid:true } })); return; } + if (req.url.startsWith('/api/stats/top_clients')){ lastTopClientsUrl=req.url; res.end(JSON.stringify({ clients:[{ip:'10.0.0.1',count:1}] })); return; } + res.statusCode=404; res.end('{}'); +} + +test('getTopClients(false, 500) appends count=500', async () => { + await start(handler); + const c = createClient({ id:'p1', url: baseUrl(), app_password:'pw' }); + await c.getTopClients(false, 500); + assert.equal(lastTopClientsUrl, '/api/stats/top_clients?count=500'); +}); +test('getTopClients(true, 500) → blocked=true&count=500', async () => { + await start(handler); + const c = createClient({ id:'p1', url: baseUrl(), app_password:'pw' }); + await c.getTopClients(true, 500); + assert.equal(lastTopClientsUrl, '/api/stats/top_clients?blocked=true&count=500'); +}); +test('getTopClients() without count is unchanged (back-compat)', async () => { + await start(handler); + const c = createClient({ id:'p1', url: baseUrl(), app_password:'pw' }); + await c.getTopClients(); + assert.equal(lastTopClientsUrl, '/api/stats/top_clients'); +}); diff --git a/tests/pihole_topclients_count_setting.test.js b/tests/pihole_topclients_count_setting.test.js new file mode 100644 index 00000000..0e89fff2 --- /dev/null +++ b/tests/pihole_topclients_count_setting.test.js @@ -0,0 +1,22 @@ +'use strict'; +const crypto = require('crypto'); +process.env.GC_ENCRYPTION_KEY = process.env.GC_ENCRYPTION_KEY || crypto.randomBytes(32).toString('hex'); +const { test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { setup, teardown, getAgent, getCsrf } = require('./helpers/setup'); +let piholeConfig, license; +beforeEach(async () => { await setup(); piholeConfig = require('../src/services/piholeConfig'); license = require('../src/services/license'); license._overrideForTest({ pihole_integration: true }); }); +afterEach(teardown); + +test('DEFAULT.top_clients_count is 1000 (load reflects it when unset)', () => { + assert.equal(piholeConfig.load().top_clients_count, 1000); +}); +test('PUT persists top_clients_count (clamped)', async () => { + const agent = getAgent(); const csrf = getCsrf(); + await agent.put('/api/v1/settings/pihole').set('X-CSRF-Token', csrf).send({ enabled:true, sync_interval_sec:30, top_clients_count: 250, instances: [] }).expect(200); + assert.equal(piholeConfig.load().top_clients_count, 250); + await agent.put('/api/v1/settings/pihole').set('X-CSRF-Token', csrf).send({ enabled:true, top_clients_count: -5, instances: [] }).expect(200); + assert.equal(piholeConfig.load().top_clients_count, 1); + await agent.put('/api/v1/settings/pihole').set('X-CSRF-Token', csrf).send({ enabled:true, top_clients_count: 99999, instances: [] }).expect(200); + assert.equal(piholeConfig.load().top_clients_count, 5000); +}); diff --git a/tests/pihole_topclients_count_sync.test.js b/tests/pihole_topclients_count_sync.test.js new file mode 100644 index 00000000..d463eb9d --- /dev/null +++ b/tests/pihole_topclients_count_sync.test.js @@ -0,0 +1,43 @@ +'use strict'; +const crypto = require('crypto'); +process.env.GC_ENCRYPTION_KEY = process.env.GC_ENCRYPTION_KEY || crypto.randomBytes(32).toString('hex'); +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const { createSync } = require('../src/services/piholeSync'); + +// 15 clients (> default 10). fakeClient records the count it was called with. +function makeClients(n){ return Array.from({length:n}, (_,i)=>({ ip:'10.8.0.'+(i+1), count: n-i })); } +function fakeClient(rec){ + return { + id:'p1', + getSummary: async () => ({ queries:{total:100,blocked:10}, gravity:{domains_being_blocked:5}, clients:{active:15} }), + getHistory: async () => [], + getTopDomains: async () => [{domain:'a.com',count:5}], + getTopClients: async (blocked=false, count) => { rec.push({blocked, count}); return makeClients(15); }, + getQueryTypes: async () => ({}), + getBlocking: async () => ({ blocking:true }), + }; +} + +test('syncOnce passes top_clients_count to getTopClients and caches all >10 clients', async () => { + const rec = []; + const sync = createSync({ + loadConfig: () => ({ enabled:true, sync_interval_sec:30, top_clients_count:1000, manage_dns_chain:false, instances:[{id:'p1'}] }), + clientFactory: () => fakeClient(rec), + peersProvider: () => [{ id:7, name:'Dev7', ip:'10.8.0.7' }], + eventBus: { publish(){} }, + dnsChain: { apply(){}, revert(){} }, + }); + const cache = await sync.syncOnce(); + // count threaded into BOTH calls (allowed + blocked) + assert.ok(rec.some(r => r.count === 1000 && r.blocked === false), 'allowed call got count 1000'); + assert.ok(rec.some(r => r.count === 1000 && r.blocked === true), 'blocked call got count 1000'); + // cache holds all 15 (not capped at 10) + assert.equal(cache.topClients.length, 15); + // the BLOCKED list cap must be raised too (separate change in Step 3c — easy to miss) + assert.equal(cache.topClientsBlocked.length, 15, 'topClientsBlocked must also respect count'); + // peer enrichment still works + assert.equal(cache.topClients.find(c => c.ip === '10.8.0.7').peerId, 7); + // topDomains cap unchanged (1 here, but the cap stays 10 — assert it is not raised by checking <=10) + assert.ok(cache.topDomains.length <= 10); +}); diff --git a/tests/pihole_topclients_count_ui.test.js b/tests/pihole_topclients_count_ui.test.js new file mode 100644 index 00000000..7902a976 --- /dev/null +++ b/tests/pihole_topclients_count_ui.test.js @@ -0,0 +1,16 @@ +'use strict'; +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); const path = require('node:path'); +test('top_clients_count field in all 3 theme settings pages', () => { + for (const theme of ['aurora','default','pro']) { + const html = fs.readFileSync(path.join(__dirname,'..','templates',theme,'pages','settings.njk'),'utf8'); + assert.ok(html.includes('pihole-top-clients-count'), `${theme}: field id`); + assert.ok(html.includes('pihole.cfg.top_clients_count'), `${theme}: i18n key`); + } +}); +test('settings.js wires top_clients_count (populate + save + valuesById)', () => { + const js = fs.readFileSync(path.join(__dirname,'..','public','js','settings.js'),'utf8'); + assert.ok(/pihole-top-clients-count/.test(js)); + assert.ok(/top_clients_count/.test(js)); +}); diff --git a/tests/pihole_topclients_display_cap.test.js b/tests/pihole_topclients_display_cap.test.js new file mode 100644 index 00000000..8073cc44 --- /dev/null +++ b/tests/pihole_topclients_display_cap.test.js @@ -0,0 +1,17 @@ +'use strict'; +const crypto = require('crypto'); +process.env.GC_ENCRYPTION_KEY = process.env.GC_ENCRYPTION_KEY || crypto.randomBytes(32).toString('hex'); +const { test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { setup, teardown, getAgent } = require('./helpers/setup'); +let pihole, license; +beforeEach(async () => { await setup(); pihole = require('../src/services/pihole'); license = require('../src/services/license'); license._overrideForTest({ pihole_integration: true }); }); +afterEach(teardown); + +test('GET /pihole/top-clients caps display at 10 even when cache holds more', async () => { + pihole.getCache = () => ({ instances:[{id:'p1',connected:true}], attribution:'per_peer', lastSyncAt:1, + topClients: Array.from({length:15}, (_,i)=>({ ip:'10.8.0.'+(i+1), count:15-i, peerId:null, peerName:null })) }); + const r = await getAgent().get('/api/v1/pihole/top-clients').expect(200); + assert.ok(Array.isArray(r.body.data)); + assert.ok(r.body.data.length <= 10, 'display must be capped at 10, got ' + r.body.data.length); +}); diff --git a/tests/user_delete_session_invalidation.test.js b/tests/user_delete_session_invalidation.test.js new file mode 100644 index 00000000..5d578c4a --- /dev/null +++ b/tests/user_delete_session_invalidation.test.js @@ -0,0 +1,22 @@ +'use strict'; +const crypto = require('crypto'); +process.env.GC_ENCRYPTION_KEY = process.env.GC_ENCRYPTION_KEY || crypto.randomBytes(32).toString('hex'); +const { test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { setup, teardown, getAgent, getCsrf } = require('./helpers/setup'); +let getDb; +beforeEach(async () => { await setup(); getDb = require('../src/db/connection').getDb; }); +afterEach(teardown); +function seedUser(n){ return getDb().prepare("INSERT INTO users (username,password_hash,role) VALUES (?,?,'admin')").run(n,'x').lastInsertRowid; } +function seedSession(sid, userId){ getDb().prepare("INSERT INTO sessions (sid,data,expires_at) VALUES (?,?,?)").run(sid, JSON.stringify({ userId, cookie:{} }), Date.now()+86400000); } +function sessionCount(userId){ return getDb().prepare("SELECT COUNT(*) n FROM sessions WHERE json_extract(data,'$.userId')=?").get(userId).n; } + +test('deleting a user destroys their sessions; other users\' sessions survive', async () => { + const victim = seedUser('victim'); const other = seedUser('other'); + seedSession('victim-sid-1', victim); seedSession('victim-sid-2', victim); seedSession('other-sid', other); + assert.equal(sessionCount(victim), 2); + const agent = getAgent(); const csrf = getCsrf(); + await agent.delete('/api/v1/users/' + victim).set('X-CSRF-Token', csrf).expect(200); + assert.equal(sessionCount(victim), 0, 'victim sessions must be gone'); + assert.equal(sessionCount(other), 1, 'other user session must survive'); +});