diff --git a/src/context/TenantContext.jsx b/src/context/TenantContext.jsx index 2f22d63f..a2e62273 100644 --- a/src/context/TenantContext.jsx +++ b/src/context/TenantContext.jsx @@ -82,6 +82,25 @@ function nameFor(id) { return found ? found.name : DEFAULT_TENANT_NAME; } +/** + * Whatever is stored under the tenant key, unvalidated, or null. + * + * Separate from `readStoredTenant()` because the API-driven paths match the + * stored id against the tenant list the server returned, which is not the same + * set as `KNOWN_TENANTS` — validating against the static list there would + * discard a legitimate server-side workspace id. + * + * @returns {string|null} + */ +function readRawStoredTenant() { + try { + return localStorage.getItem(STORAGE_KEY); + } catch { + // Private browsing, or site data blocked. No stored choice is readable. + return null; + } +} + /** * The tenant the visitor previously chose, or the default. * @@ -90,12 +109,8 @@ function nameFor(id) { * @returns {string} */ function readStoredTenant() { - try { - const stored = localStorage.getItem(STORAGE_KEY); - return isKnownTenant(stored) ? stored : DEFAULT_TENANT_ID; - } catch { - return DEFAULT_TENANT_ID; - } + const stored = readRawStoredTenant(); + return isKnownTenant(stored) ? stored : DEFAULT_TENANT_ID; } /** @@ -141,9 +156,12 @@ export function TenantProvider({ children }) { const data = await fetchUserTenants(); setTenants(data); - // If we have API tenants and no current tenant is set, use the first one + // If we have API tenants and no current tenant is set, use the first one. + // Read through readStoredTenant() rather than touching localStorage + // directly: a browser with site data blocked throws on the bare call, and + // this one runs inside an async callback where nothing catches it (#843). if (data.length > 0 && !currentTenant) { - const savedTenantId = localStorage.getItem(STORAGE_KEY); + const savedTenantId = readRawStoredTenant(); const savedTenant = savedTenantId ? data.find(t => t.id === savedTenantId) || data[0] : data[0]; @@ -227,7 +245,7 @@ export function TenantProvider({ children }) { }, [tenantId, currentTenant]); useEffect(() => { - const activeId = localStorage.getItem(STORAGE_KEY); + const activeId = readRawStoredTenant(); fetchTenants().then(() => { if (activeId && tenants.length > 0) { const saved = tenants.find((t) => t.id === activeId); diff --git a/src/services/historicalDataService.tenantScoping.test.js b/src/services/historicalDataService.tenantScoping.test.js new file mode 100644 index 00000000..1299b971 --- /dev/null +++ b/src/services/historicalDataService.tenantScoping.test.js @@ -0,0 +1,87 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; + +/** + * Cover for #1049. + * + * `historicalDataService` calls `getTenantScopedDbName()` and + * `getTenantScopedStoreName()` at module scope: + * + * const SCOPED_DB_NAME = getTenantScopedDbName(DB_NAME); + * + * so when `./tenantService` resolved to a module without those exports, the + * import threw `TypeError: getTenantScopedDbName is not a function` before any + * of the module's own code ran. Every other test in this directory failed at + * collection, which reads as "the suite is broken" rather than "this one import + * is wrong" — hence a test whose subject is the import itself. + */ +describe('historicalDataService — tenant-scoped storage (#1049)', () => { + beforeEach(() => { + vi.resetModules(); + localStorage.clear(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('imports without throwing', async () => { + await expect(import('./historicalDataService')).resolves.toBeDefined(); + }); + + it('exports the functions its consumers import', async () => { + const module = await import('./historicalDataService'); + + for (const name of [ + 'openDB', + 'getCachedData', + 'setCachedData', + 'pruneCache', + 'fetchHistoricalData', + 'formatHistoricalCSV', + 'getDelimiterForLocale', + 'HISTORY_CACHE_TTL', + ]) { + expect(module[name], `expected historicalDataService to export ${name}`).toBeDefined(); + } + }); + + it('opens a database name scoped to the default tenant', async () => { + const opened = []; + vi.stubGlobal('indexedDB', { + open: (name) => { + opened.push(name); + const request = { onsuccess: null, onerror: null, onupgradeneeded: null, result: null, error: new Error('stub') }; + setTimeout(() => request.onerror?.({ target: request }), 0); + return request; + }, + }); + + const { openDB } = await import('./historicalDataService'); + await expect(openDB()).rejects.toBeTruthy(); + + expect(opened).toEqual(['PollutionHubDB__default']); + vi.unstubAllGlobals(); + }); + + it('opens a database name scoped to the selected tenant', async () => { + localStorage.setItem('pch_tenant_id', 'mumbai-municipal'); + + const opened = []; + vi.stubGlobal('indexedDB', { + open: (name) => { + opened.push(name); + const request = { onsuccess: null, onerror: null, onupgradeneeded: null, result: null, error: new Error('stub') }; + setTimeout(() => request.onerror?.({ target: request }), 0); + return request; + }, + }); + + const { openDB } = await import('./historicalDataService'); + await expect(openDB()).rejects.toBeTruthy(); + + // The whole point of #759: two organisations on one browser profile must not + // share a cache. A shared database name is the failure that scoping prevents. + expect(opened).toEqual(['PollutionHubDB__mumbai-municipal']); + vi.unstubAllGlobals(); + }); +}); diff --git a/src/services/tenantService.client.test.js b/src/services/tenantService.client.test.js new file mode 100644 index 00000000..1451c65b --- /dev/null +++ b/src/services/tenantService.client.test.js @@ -0,0 +1,163 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { + fetchUserTenants, + updateTenantSettings, + inviteTenantMember, + removeTenantMember, + getTenantScopedDbName, +} from './tenantService'; + +/** + * Cover for #1049. + * + * The point of these is less the individual assertions than the import at the + * top: `./tenantService` has to resolve to a module that exports both halves. + * While `tenantService.js` shadowed `tenantService.ts` exactly one of the two + * groups was reachable at a time, and which one depended on the resolver. + */ + +/** @param {any} body @param {{ok?: boolean, status?: number}} [init] */ +function jsonResponse(body, { ok = true, status = 200 } = {}) { + return { + ok, + status, + json: async () => body, + }; +} + +describe('tenantService — one module, both halves (#1049)', () => { + beforeEach(() => { + localStorage.clear(); + vi.stubGlobal('fetch', vi.fn()); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it('exposes the scoping helpers and the REST client from the same specifier', async () => { + expect(typeof getTenantScopedDbName).toBe('function'); + expect(typeof fetchUserTenants).toBe('function'); + expect(typeof updateTenantSettings).toBe('function'); + expect(typeof inviteTenantMember).toBe('function'); + expect(typeof removeTenantMember).toBe('function'); + }); + + describe('fetchUserTenants', () => { + it('returns the parsed workspace list', async () => { + fetch.mockResolvedValue(jsonResponse([{ id: 'a', name: 'A' }])); + + await expect(fetchUserTenants()).resolves.toEqual([{ id: 'a', name: 'A' }]); + expect(fetch).toHaveBeenCalledWith( + expect.stringContaining('/tenants'), + expect.objectContaining({ method: 'GET' }) + ); + }); + + it('omits the Authorization header entirely when no token is stored', async () => { + fetch.mockResolvedValue(jsonResponse([])); + + await fetchUserTenants(); + + const { headers } = fetch.mock.calls[0][1]; + // Not `Bearer null`: an absent token has to read as an anonymous request, + // not as a request presenting the string "null" as a credential. + expect(headers).not.toHaveProperty('Authorization'); + }); + + it('sends the stored token when there is one', async () => { + localStorage.setItem('token', 'abc123'); + fetch.mockResolvedValue(jsonResponse([])); + + await fetchUserTenants(); + + expect(fetch.mock.calls[0][1].headers.Authorization).toBe('Bearer abc123'); + }); + + it('does not throw when localStorage is unreadable', async () => { + vi.spyOn(Storage.prototype, 'getItem').mockImplementation(() => { + throw new DOMException('The operation is insecure.', 'SecurityError'); + }); + fetch.mockResolvedValue(jsonResponse([])); + + await expect(fetchUserTenants()).resolves.toEqual([]); + }); + + it('surfaces the server message on a failed response', async () => { + fetch.mockResolvedValue( + jsonResponse({ message: 'Workspace access revoked' }, { ok: false, status: 403 }) + ); + + await expect(fetchUserTenants()).rejects.toThrow('Workspace access revoked'); + }); + + it('falls back to the status when the error body is not JSON', async () => { + fetch.mockResolvedValue({ + ok: false, + status: 502, + json: async () => { + throw new SyntaxError('Unexpected token < in JSON'); + }, + }); + + await expect(fetchUserTenants()).rejects.toThrow('HTTP 502'); + }); + }); + + describe('updateTenantSettings', () => { + it('PATCHes the settings under a `settings` key', async () => { + fetch.mockResolvedValue(jsonResponse({ id: 't1', settings: { theme: 'dark' } })); + + await updateTenantSettings('t1', { theme: 'dark' }); + + const [url, init] = fetch.mock.calls[0]; + expect(url).toContain('/tenants/t1/settings'); + expect(init.method).toBe('PATCH'); + expect(JSON.parse(init.body)).toEqual({ settings: { theme: 'dark' } }); + }); + + it('encodes the tenant id into the path', async () => { + fetch.mockResolvedValue(jsonResponse({})); + + await updateTenantSettings('a/b', {}); + + expect(fetch.mock.calls[0][0]).toContain('/tenants/a%2Fb/settings'); + }); + }); + + describe('inviteTenantMember', () => { + it('POSTs the email and role', async () => { + fetch.mockResolvedValue(jsonResponse({ id: 'm1' })); + + await inviteTenantMember('t1', 'someone@example.com', 'MEMBER'); + + const [, init] = fetch.mock.calls[0]; + expect(init.method).toBe('POST'); + expect(JSON.parse(init.body)).toEqual({ + email: 'someone@example.com', + role: 'MEMBER', + }); + }); + }); + + describe('removeTenantMember', () => { + it('DELETEs the membership and resolves with nothing', async () => { + fetch.mockResolvedValue({ ok: true, status: 204, json: async () => undefined }); + + await expect(removeTenantMember('t1', 'm1')).resolves.toBeUndefined(); + expect(fetch.mock.calls[0][0]).toContain('/tenants/t1/members/m1'); + expect(fetch.mock.calls[0][1].method).toBe('DELETE'); + }); + + it('rejects with the server message when the removal is refused', async () => { + fetch.mockResolvedValue( + jsonResponse({ message: 'Cannot remove the last admin' }, { ok: false, status: 409 }) + ); + + await expect(removeTenantMember('t1', 'm1')).rejects.toThrow( + 'Cannot remove the last admin' + ); + }); + }); +}); diff --git a/src/services/tenantService.js b/src/services/tenantService.js deleted file mode 100644 index 23d5a502..00000000 --- a/src/services/tenantService.js +++ /dev/null @@ -1,174 +0,0 @@ -/** - * @fileoverview Service layer for tenant workspace CRUD operations, - * member management, and tenant-scoped local data. - */ - -const API_BASE = import.meta.env.VITE_API_BASE_URL || "/api"; -const TENANT_STORAGE_KEY = "pch_tenant_id"; - -/** - * Returns the current tenant ID from localStorage, or "default". - * - * @returns {string} Current tenant ID. - */ -export const getCurrentTenantId = () => { - try { - return localStorage.getItem(TENANT_STORAGE_KEY) || "default"; - } catch { - return "default"; - } -}; - -/** - * Returns a scoped IndexedDB database name for the current tenant. - * - * @param {string} baseName - Base database name. - * @returns {string} Tenant-scoped database name. - */ -export const getTenantScopedDbName = (baseName) => { - const tenantId = getCurrentTenantId(); - return `${baseName}__${tenantId}`; -}; - -/** - * Returns a scoped object store name for the current tenant. - * - * @param {string} baseName - Base object store name. - * @returns {string} Tenant-scoped object store name. - */ -export const getTenantScopedStoreName = (baseName) => { - const tenantId = getCurrentTenantId(); - return `${baseName}__${tenantId}`; -}; - -/** - * Returns a scoped cache key for the current tenant. - * - * @param {string} key - Base cache key. - * @returns {string} Tenant-scoped cache key. - */ -export const getTenantScopedKey = (key) => { - const tenantId = getCurrentTenantId(); - return `${tenantId}:${key}`; -}; - -/** - * Appends tenant_id as a query parameter to an API URL. - * - * @param {string} url - API URL. - * @returns {string} Tenant-scoped API URL. - */ -export const scopeApiUrl = (url) => { - const tenantId = getCurrentTenantId(); - const separator = url.includes("?") ? "&" : "?"; - - return `${url}${separator}tenant_id=${encodeURIComponent(tenantId)}`; -}; - -/** - * Fetches all tenants associated with the current authenticated user. - * - * @returns {Promise} List of tenant objects. - */ -export const fetchUserTenants = async () => { - const response = await fetch(`${API_BASE}/tenants`, { - method: "GET", - headers: { - "Content-Type": "application/json", - Authorization: `Bearer ${localStorage.getItem("token")}`, - }, - }); - - if (!response.ok) { - throw new Error(`HTTP error! status: ${response.status}`); - } - - return response.json(); -}; - -/** - * Updates the settings of a specific tenant. - * - * @param {string} tenantId - The ID of the tenant to update. - * @param {Object} settings - The new settings object. - * @returns {Promise} The updated tenant object. - */ -export const updateTenantSettings = async (tenantId, settings) => { - const response = await fetch( - `${API_BASE}/tenants/${tenantId}/settings`, - { - method: "PATCH", - headers: { - "Content-Type": "application/json", - Authorization: `Bearer ${localStorage.getItem("token")}`, - }, - body: JSON.stringify({ settings }), - }, - ); - - if (!response.ok) { - throw new Error(`HTTP error! status: ${response.status}`); - } - - return response.json(); -}; - -/** - * Invites a new member to a specific tenant workspace. - * - * @param {string} tenantId - The ID of the tenant. - * @param {string} email - The email address of the user to invite. - * @param {string} role - The role to assign ('ADMIN', 'MANAGER', 'MEMBER'). - * @returns {Promise} The created tenant member record. - */ -export const inviteTenantMember = async (tenantId, email, role) => { - const response = await fetch( - `${API_BASE}/tenants/${tenantId}/members`, - { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: `Bearer ${localStorage.getItem("token")}`, - }, - body: JSON.stringify({ email, role }), - }, - ); - - if (!response.ok) { - let errorMessage = `HTTP error! status: ${response.status}`; - - try { - const errorData = await response.json(); - errorMessage = errorData.message || errorMessage; - } catch { - // Keep the default HTTP error when the response is not valid JSON. - } - - throw new Error(errorMessage); - } - - return response.json(); -}; - -/** - * Removes a member from a tenant workspace. - * - * @param {string} tenantId - The ID of the tenant. - * @param {string} memberId - The ID of the membership record to remove. - * @returns {Promise} - */ -export const removeTenantMember = async (tenantId, memberId) => { - const response = await fetch( - `${API_BASE}/tenants/${tenantId}/members/${memberId}`, - { - method: "DELETE", - headers: { - Authorization: `Bearer ${localStorage.getItem("token")}`, - }, - }, - ); - - if (!response.ok) { - throw new Error(`HTTP error! status: ${response.status}`); - } -}; diff --git a/src/services/tenantService.ts b/src/services/tenantService.ts new file mode 100644 index 00000000..5da83bf7 --- /dev/null +++ b/src/services/tenantService.ts @@ -0,0 +1,185 @@ +// src/services/tenantService.ts +// +// The single module answering to `./tenantService`. +// +// This file previously held only the scoping helpers (#759). The workspace +// management work (#1031) added a second `tenantService.js` beside it holding +// the REST client, and the two collided: both satisfy the extensionless +// specifier `../services/tenantService`, Vite resolves `.js` before `.ts`, and +// `historicalDataService` — which imports `getTenantScopedDbName` at module +// scope — started throwing `TypeError: getTenantScopedDbName is not a function` +// before it had executed a single statement of its own. +// +// The two halves are not unrelated: both answer "which organisation is this +// request for", one for local storage and one for the API. They belong in one +// module, which is also what keeps `npm run check:shadowing` (a blocking step +// in CI, added for #990) green. + +import type { Tenant, TenantMember, TenantSettings } from '../types/tenant'; + +const TENANT_STORAGE_KEY = 'pch_tenant_id'; +const AUTH_TOKEN_KEY = 'token'; + +const API_BASE = import.meta.env.VITE_API_BASE_URL || '/api'; + +// ─── Local scoping (#759) ──────────────────────────────────────────────────── + +/** + * Reads a key from localStorage, or null when storage is unusable. + * + * Every read here goes through this. A Firefox private window and a browser + * configured to block site data both throw `SecurityError` on plain property + * access, so an unguarded `localStorage.getItem` is not a missing value — it is + * an exception thrown out of whichever render or module evaluation reached it. + */ +function readStorage(key: string): string | null { + try { + return localStorage.getItem(key); + } catch { + return null; + } +} + +/** + * The current tenant ID from localStorage, or "default". + */ +export function getCurrentTenantId(): string { + return readStorage(TENANT_STORAGE_KEY) || 'default'; +} + +/** + * A scoped IndexedDB database name for the current tenant. + * Each tenant gets its own isolated IndexedDB database. + */ +export function getTenantScopedDbName(baseName: string): string { + return `${baseName}__${getCurrentTenantId()}`; +} + +/** + * A scoped object store name for the current tenant. + */ +export function getTenantScopedStoreName(baseName: string): string { + return `${baseName}__${getCurrentTenantId()}`; +} + +/** + * A scoped cache key for the current tenant. + * Use this to prefix localStorage / sessionStorage keys. + */ +export function getTenantScopedKey(key: string): string { + return `${getCurrentTenantId()}:${key}`; +} + +/** + * Appends `tenant_id` as a query parameter to an API URL. + */ +export function scopeApiUrl(url: string): string { + const separator = url.includes('?') ? '&' : '?'; + return `${url}${separator}tenant_id=${encodeURIComponent(getCurrentTenantId())}`; +} + +// ─── Workspace REST client (#1031) ─────────────────────────────────────────── + +/** + * Request headers carrying the stored bearer token. + * + * The token is only attached when there is one. Sending `Authorization: Bearer + * null` — which is what template-interpolating a missing token produces — asks + * the server to reject a request that an anonymous call might have been allowed + * to make, and turns "not signed in" into an opaque 401. + */ +function authHeaders(extra: Record = {}): Record { + const token = readStorage(AUTH_TOKEN_KEY); + return token ? { ...extra, Authorization: `Bearer ${token}` } : { ...extra }; +} + +const JSON_HEADERS = { 'Content-Type': 'application/json' }; + +/** + * The server's error message for a failed response, falling back to the status. + * + * `response.json()` on an error is not guaranteed to be JSON — a proxy 502 is + * usually HTML — so the parse is guarded and the status is used when it fails. + */ +async function errorFor(response: Response, fallback: string): Promise { + try { + const body = await response.json(); + if (body && typeof body.message === 'string' && body.message) { + return new Error(body.message); + } + } catch { + // Not JSON. The status line below is the best available description. + } + return new Error(`${fallback} (HTTP ${response.status})`); +} + +/** + * All tenants associated with the current authenticated user. + */ +export async function fetchUserTenants(): Promise { + const response = await fetch(`${API_BASE}/tenants`, { + method: 'GET', + headers: authHeaders(JSON_HEADERS), + }); + if (!response.ok) { + throw await errorFor(response, 'Failed to load workspaces'); + } + return response.json(); +} + +/** + * Updates the settings of a specific tenant. + */ +export async function updateTenantSettings( + tenantId: string, + settings: Partial +): Promise { + const response = await fetch(`${API_BASE}/tenants/${encodeURIComponent(tenantId)}/settings`, { + method: 'PATCH', + headers: authHeaders(JSON_HEADERS), + body: JSON.stringify({ settings }), + }); + if (!response.ok) { + throw await errorFor(response, 'Failed to update workspace settings'); + } + return response.json(); +} + +/** + * Invites a new member to a tenant workspace. + */ +export async function inviteTenantMember( + tenantId: string, + email: string, + role: TenantMember['role'] +): Promise { + const response = await fetch(`${API_BASE}/tenants/${encodeURIComponent(tenantId)}/members`, { + method: 'POST', + headers: authHeaders(JSON_HEADERS), + body: JSON.stringify({ email, role }), + }); + if (!response.ok) { + throw await errorFor(response, 'Failed to invite member'); + } + return response.json(); +} + +/** + * Removes a member from a tenant workspace. + * + * The path segments are encoded. They are ids that arrive from the API, but a + * `/` or `?` in one would otherwise re-point the request at a different route + * rather than 404 — an id is a value, not a path fragment. + */ +export async function removeTenantMember(tenantId: string, memberId: string): Promise { + const response = await fetch( + `${API_BASE}/tenants/${encodeURIComponent(tenantId)}/members/${encodeURIComponent(memberId)}`, + { + method: 'DELETE', + headers: authHeaders(), + } + ); + if (!response.ok) { + throw await errorFor(response, 'Failed to remove member'); + } +}