Closes #130. Multi-site, not multi-tenancy: a site is a named collection of devices whose clients, events, topology and dashboards are viewed separately. Sites separate what you see, not who you are — the admin/manage/read-only roles are unchanged and every user sees every site. Per-site permissions would have multiplied the work for a need nobody stated. Two measurements decided the design. Every device-derived table hangs off devices(id) — 25 of 50 tables, 35 foreign keys — so a single site_id column on devices scopes clients, events, topology, backups and the rest transitively. No other table needed one. The frontend has 162 useQuery call sites across 57 files. Threading a site through each would be a large change that leaves a permanent trap: every future query another chance to forget. The site is injected in the existing axios interceptor instead, beside the auth token, so a new query is scoped by default. Only 23 collection routes needed filtering; the 131 per-device routes are already scoped by their device. Existing installs are untouched. Migration creates one "Default Site", adopts every device into it, and a single-site install never meets the concept. An absent X-Site-Id header means unscoped, so existing API tokens and scripts behave exactly as before. Polling stays fleet-wide. Sites are a view over data already gathered, not a change to how it is gathered — a device does not stop being monitored because you are looking elsewhere. Rogue-AP detection scopes the scans but keeps the "these are ours" BSSID set fleet-wide, or your own AP in another site reads as an evil twin. Bulk actions do obey the selector: an operation that silently spanned two customers is the blast radius the wave guards exist to contain. Four rounds of defects came out of testing the UI rather than the API, and the first is the one worth recording. Switching sites called queryClient.clear(), which empties the cache and reviews as correct. It isn't: clear() destroys each query but a *mounted* observer keeps its own reference and last result, so every rendered page went on showing the previous site's data. The QueryClient is now keyed on the site, which remounts the tree against an empty cache. Three tests pin the old behaviour down so the shortcut cannot return. The others: an unscoped fallback path behind the Connected Clients chart; Config Health, TX retries and the activity feed reading fleet-wide; and site_id added to the device list query but none of the other four that build a device payload, so the Site field could not read its own value and appeared to reject every change while the writes were landing. That last one is now structural — DEVICE_BASE_COLUMNS is shared by all five, with a test that fails when a device payload is hand-rolled, and which found two further instances while being written. Frontend tests were never run by CI despite vitest being configured and six suites existing; they now run in both the workflow and ci-preflight. Verified against a live four-device fleet: the migration left every figure identical, an empty site returns zero from every read endpoint, moving a device carries its history with it, and deleting a site with devices is refused. Minor bump rather than patch: sites change the shape of the product rather than adding to it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
149 lines
5.0 KiB
TypeScript
149 lines
5.0 KiB
TypeScript
import {
|
|
resolveSiteId,
|
|
siteScopeDevices,
|
|
siteScopeByDevice,
|
|
siteScopeByNullableDevice,
|
|
andSite,
|
|
clientSeriesTag,
|
|
} from '../siteScope';
|
|
|
|
describe('resolveSiteId', () => {
|
|
it('accepts positive integers', () => {
|
|
expect(resolveSiteId('3')).toBe(3);
|
|
expect(resolveSiteId(7)).toBe(7);
|
|
expect(resolveSiteId(' 12 ')).toBe(12);
|
|
});
|
|
|
|
// Unscoped is the compatibility path: anything that isn't a real site id must
|
|
// behave exactly as the API did before sites existed, never as an error.
|
|
it('treats absent, empty and "all" as unscoped', () => {
|
|
expect(resolveSiteId(undefined)).toBeNull();
|
|
expect(resolveSiteId(null)).toBeNull();
|
|
expect(resolveSiteId('')).toBeNull();
|
|
expect(resolveSiteId(' ')).toBeNull();
|
|
expect(resolveSiteId('all')).toBeNull();
|
|
expect(resolveSiteId('ALL')).toBeNull();
|
|
});
|
|
|
|
it('rejects zero and negatives', () => {
|
|
expect(resolveSiteId('0')).toBeNull();
|
|
expect(resolveSiteId('-4')).toBeNull();
|
|
});
|
|
|
|
// The id is inlined into SQL, so this is the boundary that makes that safe.
|
|
// Every one of these must come back null rather than reaching a query.
|
|
it('rejects anything that is not a plain integer', () => {
|
|
const hostile = [
|
|
'1; DROP TABLE devices',
|
|
'1 OR 1=1',
|
|
"1'",
|
|
'1.5',
|
|
'1e3',
|
|
'abc',
|
|
'0x1',
|
|
'١٢',
|
|
{},
|
|
[],
|
|
];
|
|
for (const raw of hostile) expect(resolveSiteId(raw)).toBeNull();
|
|
});
|
|
|
|
// Surrounding whitespace is stripped, not rejected -- '\n3' is the number 3
|
|
// with padding, which is what a header or query string may legitimately carry.
|
|
it('trims surrounding whitespace rather than rejecting it', () => {
|
|
expect(resolveSiteId('\n3')).toBe(3);
|
|
expect(resolveSiteId('\t9 ')).toBe(9);
|
|
});
|
|
|
|
it('never returns a value whose string form contains non-digits', () => {
|
|
for (const raw of ['5', ' 5 ', 5]) {
|
|
const id = resolveSiteId(raw);
|
|
expect(id).not.toBeNull();
|
|
expect(String(id)).toMatch(/^\d+$/);
|
|
}
|
|
});
|
|
});
|
|
|
|
describe('siteScopeDevices', () => {
|
|
it('returns null when unscoped so callers can skip the filter', () => {
|
|
expect(siteScopeDevices(null)).toBeNull();
|
|
expect(siteScopeDevices(null, 'd')).toBeNull();
|
|
});
|
|
|
|
it('filters on site_id, qualified when an alias is given', () => {
|
|
expect(siteScopeDevices(3)).toBe('site_id = 3');
|
|
expect(siteScopeDevices(3, 'd')).toBe('d.site_id = 3');
|
|
});
|
|
});
|
|
|
|
describe('siteScopeByDevice', () => {
|
|
it('returns null when unscoped', () => {
|
|
expect(siteScopeByDevice(null)).toBeNull();
|
|
});
|
|
|
|
it('scopes any device_id column through the devices table', () => {
|
|
expect(siteScopeByDevice(4)).toBe('device_id IN (SELECT id FROM devices WHERE site_id = 4)');
|
|
expect(siteScopeByDevice(4, 'c.device_id'))
|
|
.toBe('c.device_id IN (SELECT id FROM devices WHERE site_id = 4)');
|
|
});
|
|
});
|
|
|
|
describe('siteScopeByNullableDevice', () => {
|
|
it('returns null when unscoped, so fleet-wide rows still appear', () => {
|
|
expect(siteScopeByNullableDevice(null, 'e.device_id')).toBeNull();
|
|
});
|
|
|
|
// A row with no device belongs to no site. Showing it in every site would
|
|
// misrepresent it as that site's own, so it is hidden while one is selected.
|
|
it('excludes rows with no device when a site is selected', () => {
|
|
const sql = siteScopeByNullableDevice(2, 'e.device_id');
|
|
expect(sql).toContain('e.device_id IS NOT NULL');
|
|
expect(sql).toContain('site_id = 2');
|
|
});
|
|
});
|
|
|
|
describe('andSite', () => {
|
|
it('returns the base untouched when unscoped', () => {
|
|
expect(andSite('WHERE a = 1', null)).toBe('WHERE a = 1');
|
|
expect(andSite('', null)).toBe('');
|
|
});
|
|
|
|
it('starts a WHERE clause when the base is empty', () => {
|
|
expect(andSite('', 'site_id = 1')).toBe('WHERE site_id = 1');
|
|
expect(andSite(' ', 'site_id = 1')).toBe('WHERE site_id = 1');
|
|
});
|
|
|
|
it('appends with AND when the base already has a clause', () => {
|
|
expect(andSite('WHERE a = 1', 'site_id = 1')).toBe('WHERE a = 1 AND site_id = 1');
|
|
});
|
|
});
|
|
|
|
describe('clientSeriesTag', () => {
|
|
it('uses the global series when unscoped', () => {
|
|
expect(clientSeriesTag(null, 0, 4)).toBe('_global');
|
|
});
|
|
|
|
// The case that matters for every existing single-site install: the site
|
|
// holds the whole fleet, so the global series is exactly right and keeps the
|
|
// chart's history instead of restarting it at the upgrade.
|
|
it('uses the global series when the site holds every device', () => {
|
|
expect(clientSeriesTag(1, 4, 4)).toBe('_global');
|
|
});
|
|
|
|
it('uses the per-site series once the fleet is actually split', () => {
|
|
expect(clientSeriesTag(1, 3, 4)).toBe('_site_1');
|
|
expect(clientSeriesTag(2, 1, 4)).toBe('_site_2');
|
|
});
|
|
|
|
// An empty site must read a series nothing writes, so the chart is empty
|
|
// rather than showing the fleet's.
|
|
it('uses the per-site series for an empty site', () => {
|
|
expect(clientSeriesTag(3, 0, 4)).toBe('_site_3');
|
|
});
|
|
|
|
// No devices at all: nothing to be equal to, so don't claim the global total.
|
|
it('does not treat an empty install as a full site', () => {
|
|
expect(clientSeriesTag(1, 0, 0)).toBe('_site_1');
|
|
});
|
|
});
|