mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(plugin): clear brokered session state on logout
sessionStorage outlives a logout inside the same tab, so the keys stay readable after the next user signs in. They are namespaced by user id, which stops the new user's plugins from reading them, but the data is still sitting in the tab — the same reason logout already drops the appearance snapshot, the SW caches and the user-scoped offline DB. Purge them there too. Three things around it, from reading the handlers next to the new ones: - The inactive-plugin purge ran before the store was updated, so a sessionStorage failure cost us the plugin list we had just fetched. Commit first, purge after. - The session handlers answered without checking requestId, unlike confirm and geolocation — an answer the frame can never match is worth skipping. - The mock host threw plain Errors while the real bridge rejects with .code set, and checked the key before the trip scope where the host does the reverse, so a plugin's error handling could pass its tests and still break in the app.
This commit is contained in:
@@ -416,6 +416,9 @@ export default function PluginFrame({ pluginId, tripId = null, placeId = null, d
|
|||||||
break
|
break
|
||||||
}
|
}
|
||||||
case 'trek:session:clear': {
|
case 'trek:session:clear': {
|
||||||
|
// Same guard the other request/response handlers apply: without an id
|
||||||
|
// the frame could never match our answer to its call.
|
||||||
|
if (typeof msg.requestId !== 'string' || !msg.requestId) break
|
||||||
const scope = msg.scope === 'trip' ? 'trip' : 'plugin'
|
const scope = msg.scope === 'trip' ? 'trip' : 'plugin'
|
||||||
|
|
||||||
// tripId required if we are on a trip-scoped key.
|
// tripId required if we are on a trip-scoped key.
|
||||||
@@ -437,6 +440,7 @@ export default function PluginFrame({ pluginId, tripId = null, placeId = null, d
|
|||||||
case 'trek:session:get':
|
case 'trek:session:get':
|
||||||
case 'trek:session:set':
|
case 'trek:session:set':
|
||||||
case 'trek:session:remove': {
|
case 'trek:session:remove': {
|
||||||
|
if (typeof msg.requestId !== 'string' || !msg.requestId) break
|
||||||
const scope = msg.scope === 'trip' ? 'trip' : 'plugin'
|
const scope = msg.scope === 'trip' ? 'trip' : 'plugin'
|
||||||
if (scope === 'trip' && tripId == null) {
|
if (scope === 'trip' && tripId == null) {
|
||||||
post({ type: 'trek:error', requestId: msg.requestId, code: 'NO_TRIP_CONTEXT', message: 'trip session storage requires a trip context' })
|
post({ type: 'trek:error', requestId: msg.requestId, code: 'NO_TRIP_CONTEXT', message: 'trip session storage requires a trip context' })
|
||||||
@@ -552,7 +556,7 @@ export default function PluginFrame({ pluginId, tripId = null, placeId = null, d
|
|||||||
// The frame is going away (or re-bridging) — never leave a live GPS watch behind.
|
// The frame is going away (or re-bridging) — never leave a live GPS watch behind.
|
||||||
if (geoWatchRef.current != null) { navigator.geolocation.clearWatch(geoWatchRef.current); geoWatchRef.current = null }
|
if (geoWatchRef.current != null) { navigator.geolocation.clearWatch(geoWatchRef.current); geoWatchRef.current = null }
|
||||||
}
|
}
|
||||||
}, [pluginId, tripId, fill, navigate, postFrame, buildContext])
|
}, [pluginId, tripId, fill, navigate, postFrame, buildContext, userId])
|
||||||
|
|
||||||
// Hosts swap pluginId in place (tab bar, /plugins/:id route) — the iframe below
|
// Hosts swap pluginId in place (tab bar, /plugins/:id route) — the iframe below
|
||||||
// is keyed so the document is a fresh first load, and the per-plugin bridge
|
// is keyed so the document is a fresh first load, and the per-plugin bridge
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import { setAuthed } from '../sync/authGate'
|
|||||||
import { unregisterSyncTriggers } from '../sync/syncTriggers'
|
import { unregisterSyncTriggers } from '../sync/syncTriggers'
|
||||||
import { useSystemNoticeStore } from './systemNoticeStore.js'
|
import { useSystemNoticeStore } from './systemNoticeStore.js'
|
||||||
import { clearAppearanceSnapshot } from '../theme/applyAppearance'
|
import { clearAppearanceSnapshot } from '../theme/applyAppearance'
|
||||||
|
import { clearAllPluginSessions } from './pluginStore'
|
||||||
|
|
||||||
interface AuthResponse {
|
interface AuthResponse {
|
||||||
user: User
|
user: User
|
||||||
@@ -195,6 +196,9 @@ export const useAuthStore = create<AuthState>()(
|
|||||||
// Drop the per-device appearance snapshot so the next user on a shared
|
// Drop the per-device appearance snapshot so the next user on a shared
|
||||||
// browser doesn't get a pre-paint flash of this user's theme.
|
// browser doesn't get a pre-paint flash of this user's theme.
|
||||||
clearAppearanceSnapshot()
|
clearAppearanceSnapshot()
|
||||||
|
// Same reason for the brokered plugin session state: it is keyed by user id,
|
||||||
|
// but sessionStorage outlives a logout within the tab.
|
||||||
|
clearAllPluginSessions()
|
||||||
// 4. Tell server to clear the httpOnly cookie (best-effort).
|
// 4. Tell server to clear the httpOnly cookie (best-effort).
|
||||||
await fetch('/api/auth/logout', { method: 'POST', credentials: 'include' }).catch(() => {})
|
await fetch('/api/auth/logout', { method: 'POST', credentials: 'include' }).catch(() => {})
|
||||||
// 5. Clear service worker caches containing sensitive data.
|
// 5. Clear service worker caches containing sensitive data.
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
// FE-STORE-PLUGIN-001 to 004
|
// FE-STORE-PLUGIN-001 to 006
|
||||||
import { http, HttpResponse } from 'msw';
|
import { http, HttpResponse } from 'msw';
|
||||||
import { server } from '../../tests/helpers/msw/server';
|
import { server } from '../../tests/helpers/msw/server';
|
||||||
import { usePluginStore } from './pluginStore';
|
import { usePluginStore, clearAllPluginSessions } from './pluginStore';
|
||||||
|
|
||||||
const initial = usePluginStore.getState();
|
const initial = usePluginStore.getState();
|
||||||
|
|
||||||
@@ -72,4 +72,31 @@ describe('pluginStore', () => {
|
|||||||
expect(sessionStorage.getItem('trek:plugin-session:7:disabled:trip:42:view')).toBeNull();
|
expect(sessionStorage.getItem('trek:plugin-session:7:disabled:trip:42:view')).toBeNull();
|
||||||
expect(sessionStorage.getItem('trek_session')).toBe('app-session');
|
expect(sessionStorage.getItem('trek_session')).toBe('app-session');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('FE-STORE-PLUGIN-005: clearAllPluginSessions drops every user and scope, but nothing else', () => {
|
||||||
|
sessionStorage.setItem('trek:plugin-session:7:active:plugin:filters', '["flight"]');
|
||||||
|
sessionStorage.setItem('trek:plugin-session:9:active:trip:42:view', '"table"');
|
||||||
|
sessionStorage.setItem('trek_session', 'app-session');
|
||||||
|
|
||||||
|
clearAllPluginSessions();
|
||||||
|
|
||||||
|
expect(sessionStorage.getItem('trek:plugin-session:7:active:plugin:filters')).toBeNull();
|
||||||
|
expect(sessionStorage.getItem('trek:plugin-session:9:active:trip:42:view')).toBeNull();
|
||||||
|
expect(sessionStorage.getItem('trek_session')).toBe('app-session');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('FE-STORE-PLUGIN-006: a sessionStorage failure during the purge still keeps the loaded plugins', async () => {
|
||||||
|
const key = vi.spyOn(Storage.prototype, 'key').mockImplementation(() => { throw new Error('storage disabled'); });
|
||||||
|
server.use(
|
||||||
|
http.get('/api/plugins', () =>
|
||||||
|
HttpResponse.json({ plugins: [{ id: 'active', name: 'Active', type: 'widget', icon: null }] }),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
await usePluginStore.getState().loadPlugins();
|
||||||
|
|
||||||
|
expect(usePluginStore.getState().plugins.map((p) => p.id)).toEqual(['active']);
|
||||||
|
expect(usePluginStore.getState().loaded).toBe(true);
|
||||||
|
key.mockRestore();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -23,6 +23,20 @@ function clearInactivePluginSessions(activePluginIds: Set<string>) {
|
|||||||
keysToRemove.forEach((storageKey) => sessionStorage.removeItem(storageKey))
|
keysToRemove.forEach((storageKey) => sessionStorage.removeItem(storageKey))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Drops every plugin's session state, whatever user or trip it belonged to.
|
||||||
|
* Logout calls this so the next user on a shared browser starts clean — the
|
||||||
|
* same reason the appearance snapshot and the user-scoped offline DB go.
|
||||||
|
*/
|
||||||
|
export function clearAllPluginSessions() {
|
||||||
|
const keysToRemove: string[] = []
|
||||||
|
for (let i = 0; i < sessionStorage.length; i += 1) {
|
||||||
|
const storageKey = sessionStorage.key(i)
|
||||||
|
if (storageKey?.startsWith(PLUGIN_SESSION_NAMESPACE)) keysToRemove.push(storageKey)
|
||||||
|
}
|
||||||
|
keysToRemove.forEach((storageKey) => sessionStorage.removeItem(storageKey))
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Active plugins the client renders (#plugins, M3). Page plugins become nav
|
* Active plugins the client renders (#plugins, M3). Page plugins become nav
|
||||||
* entries + a full-page iframe route; widget plugins mount on the dashboard.
|
* entries + a full-page iframe route; widget plugins mount on the dashboard.
|
||||||
@@ -70,8 +84,12 @@ export const usePluginStore = create<PluginState>((set, get) => ({
|
|||||||
try {
|
try {
|
||||||
const data = await pluginsApi.active()
|
const data = await pluginsApi.active()
|
||||||
const plugins = (data.plugins as ActivePlugin[]) || []
|
const plugins = (data.plugins as ActivePlugin[]) || []
|
||||||
clearInactivePluginSessions(new Set(plugins.map((plugin) => plugin.id)))
|
|
||||||
set({ plugins, loaded: true })
|
set({ plugins, loaded: true })
|
||||||
|
// After the state is committed: a sessionStorage failure (Safari private
|
||||||
|
// mode, quota) must not cost us the plugin list we just fetched.
|
||||||
|
try {
|
||||||
|
clearInactivePluginSessions(new Set(plugins.map((plugin) => plugin.id)))
|
||||||
|
} catch { /* leaving stale plugin state behind beats losing the nav entries */ }
|
||||||
} catch {
|
} catch {
|
||||||
set({ loaded: true })
|
set({ loaded: true })
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -242,17 +242,28 @@ export function createMockHost(opts: MockHostOptions = {}): MockHost {
|
|||||||
const emitted: MockHost['emitted'] = [];
|
const emitted: MockHost['emitted'] = [];
|
||||||
const notifications: MockHost['notifications'] = [];
|
const notifications: MockHost['notifications'] = [];
|
||||||
const sessionValues = new Map<string, string>();
|
const sessionValues = new Map<string, string>();
|
||||||
|
// The real bridge rejects with a plain Error carrying the code on `.code`
|
||||||
|
// (ui/kit.ts). Keep the `CODE: message` text the rest of this mock uses, but
|
||||||
|
// attach `.code` too, so a plugin that branches on it behaves the same here.
|
||||||
|
const sessionError = (code: string, message: string) => {
|
||||||
|
const err = new Error(`${code}: ${message}`) as Error & { code: string };
|
||||||
|
err.code = code;
|
||||||
|
return err;
|
||||||
|
};
|
||||||
const sessionPrefix = (scope: 'plugin' | 'trip') => {
|
const sessionPrefix = (scope: 'plugin' | 'trip') => {
|
||||||
if (scope === 'trip' && opts.sessionTripId === undefined) {
|
if (scope === 'trip' && opts.sessionTripId === undefined) {
|
||||||
throw new Error('NO_TRIP_CONTEXT: trip session storage requires a trip context');
|
throw sessionError('NO_TRIP_CONTEXT', 'trip session storage requires a trip context');
|
||||||
}
|
}
|
||||||
return scope === 'trip' ? `trip:${opts.sessionTripId}:` : 'plugin:';
|
return scope === 'trip' ? `trip:${opts.sessionTripId}:` : 'plugin:';
|
||||||
};
|
};
|
||||||
const sessionKey = (key: string, scope: 'plugin' | 'trip') => {
|
const sessionKey = (key: string, scope: 'plugin' | 'trip') => {
|
||||||
|
// Scope before key, in the host's order — otherwise the same bad call
|
||||||
|
// reports a different code here than it does in the app.
|
||||||
|
const prefix = sessionPrefix(scope);
|
||||||
if (!key || key.length > PLUGIN_SESSION_MAX_KEY_LENGTH) {
|
if (!key || key.length > PLUGIN_SESSION_MAX_KEY_LENGTH) {
|
||||||
throw new Error(`SESSION_INVALID_KEY: session key must be 1-${PLUGIN_SESSION_MAX_KEY_LENGTH} characters`);
|
throw sessionError('SESSION_INVALID_KEY', `session key must be 1-${PLUGIN_SESSION_MAX_KEY_LENGTH} characters`);
|
||||||
}
|
}
|
||||||
return `${sessionPrefix(scope)}${key}`;
|
return `${prefix}${key}`;
|
||||||
};
|
};
|
||||||
const session: PluginSessionStorage = {
|
const session: PluginSessionStorage = {
|
||||||
async get(key, options) {
|
async get(key, options) {
|
||||||
@@ -262,15 +273,15 @@ export function createMockHost(opts: MockHostOptions = {}): MockHost {
|
|||||||
async set(key, value, options) {
|
async set(key, value, options) {
|
||||||
const fullKey = sessionKey(key, options?.scope === 'trip' ? 'trip' : 'plugin');
|
const fullKey = sessionKey(key, options?.scope === 'trip' ? 'trip' : 'plugin');
|
||||||
const serialized = JSON.stringify(value);
|
const serialized = JSON.stringify(value);
|
||||||
if (serialized === undefined) throw new Error('SESSION_INVALID_VALUE: session value must be JSON-serialisable');
|
if (serialized === undefined) throw sessionError('SESSION_INVALID_VALUE', 'session value must be JSON-serialisable');
|
||||||
const valueBytes = Buffer.byteLength(serialized, 'utf8');
|
const valueBytes = Buffer.byteLength(serialized, 'utf8');
|
||||||
if (valueBytes > PLUGIN_SESSION_MAX_VALUE_BYTES) {
|
if (valueBytes > PLUGIN_SESSION_MAX_VALUE_BYTES) {
|
||||||
throw new Error(`SESSION_VALUE_TOO_LARGE: session value exceeds ${PLUGIN_SESSION_MAX_VALUE_BYTES} bytes`);
|
throw sessionError('SESSION_VALUE_TOO_LARGE', `session value exceeds ${PLUGIN_SESSION_MAX_VALUE_BYTES} bytes`);
|
||||||
}
|
}
|
||||||
const prefix = sessionPrefix(options?.scope === 'trip' ? 'trip' : 'plugin');
|
const prefix = sessionPrefix(options?.scope === 'trip' ? 'trip' : 'plugin');
|
||||||
const scopedKeyCount = [...sessionValues.keys()].filter((storedKey) => storedKey.startsWith(prefix)).length;
|
const scopedKeyCount = [...sessionValues.keys()].filter((storedKey) => storedKey.startsWith(prefix)).length;
|
||||||
if (!sessionValues.has(fullKey) && scopedKeyCount >= PLUGIN_SESSION_MAX_KEYS) {
|
if (!sessionValues.has(fullKey) && scopedKeyCount >= PLUGIN_SESSION_MAX_KEYS) {
|
||||||
throw new Error(`SESSION_KEY_LIMIT: plugin session storage allows at most ${PLUGIN_SESSION_MAX_KEYS} keys`);
|
throw sessionError('SESSION_KEY_LIMIT', `plugin session storage allows at most ${PLUGIN_SESSION_MAX_KEYS} keys`);
|
||||||
}
|
}
|
||||||
sessionValues.set(fullKey, serialized);
|
sessionValues.set(fullKey, serialized);
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -501,14 +501,25 @@ window.parent.postMessage({ type: 'trek:invoke', requestId: '1', sub: '/status',
|
|||||||
| `trek:confirm` | `{ requestId, title?, message?, confirmLabel?, cancelLabel?, danger? }` | host-rendered ConfirmDialog; answered as `trek:confirm:result` |
|
| `trek:confirm` | `{ requestId, title?, message?, confirmLabel?, cancelLabel?, danger? }` | host-rendered ConfirmDialog; answered as `trek:confirm:result` |
|
||||||
| `trek:openExternal` | `{ url }` | open an `http(s)` URL in a new `noopener` tab; anything else is dropped |
|
| `trek:openExternal` | `{ url }` | open an `http(s)` URL in a new `noopener` tab; anything else is dropped |
|
||||||
| `trek:geolocation` | `{ requestId, action?: 'get'\|'watch'\|'clear' }` | host-brokered browser position (needs `geolocation:read`); answered as `trek:geolocation:result`, watch updates stream as `trek:geolocation:update` |
|
| `trek:geolocation` | `{ requestId, action?: 'get'\|'watch'\|'clear' }` | host-brokered browser position (needs `geolocation:read`); answered as `trek:geolocation:result`, watch updates stream as `trek:geolocation:update` |
|
||||||
|
| `trek:session:get` | `{ requestId, key, scope?: 'plugin'\|'trip' }` | read tab-scoped state; answered as `trek:response` with `data` (`undefined` when unset) |
|
||||||
|
| `trek:session:set` | `{ requestId, key, value, scope? }` | store a JSON-serialisable value; answered as `trek:response` |
|
||||||
|
| `trek:session:remove` | `{ requestId, key, scope? }` | drop one key; answered as `trek:response` |
|
||||||
|
| `trek:session:clear` | `{ requestId, scope? }` | drop every key in that scope; answered as `trek:response` |
|
||||||
|
|
||||||
|
The session messages are rejected as `trek:error` with `code` set to `NO_TRIP_CONTEXT`
|
||||||
|
(trip scope outside a trip), `SESSION_INVALID_KEY` (empty or over 64 characters),
|
||||||
|
`SESSION_INVALID_VALUE` (not JSON-serialisable), `SESSION_VALUE_TOO_LARGE` (over 1 KiB
|
||||||
|
serialised), `SESSION_KEY_LIMIT` (over 32 keys in that scope) or `SESSION_STORAGE_ERROR`
|
||||||
|
(the browser refused the write). The storage key itself is host-owned and includes the
|
||||||
|
signed-in user and your plugin id, so scopes never collide.
|
||||||
|
|
||||||
**Messages TREK sends you:**
|
**Messages TREK sends you:**
|
||||||
|
|
||||||
| Message | Payload |
|
| Message | Payload |
|
||||||
|---|---|
|
|---|---|
|
||||||
| `trek:context` | `{ tripId, placeId, dayId, reservationId, userId, theme, locale, dir, hostOrigin, user, formats, tokens, appearance }` (see below) — re-sent whenever the theme, appearance, **locale or formats** change |
|
| `trek:context` | `{ tripId, placeId, dayId, reservationId, userId, theme, locale, dir, hostOrigin, user, formats, tokens, appearance }` (see below) — re-sent whenever the theme, appearance, **locale or formats** change |
|
||||||
| `trek:response` | `{ requestId, data }` — a successful `trek:invoke` |
|
| `trek:response` | `{ requestId, data }` — a successful `trek:invoke` or `trek:session:*` |
|
||||||
| `trek:error` | `{ requestId, code, message }` — a failed `trek:invoke` (`code` is the HTTP status or `"error"`) |
|
| `trek:error` | `{ requestId, code, message }` — a failed request; for `trek:invoke` `code` is the HTTP status or `"error"`, for `trek:session:*` one of the `SESSION_*` / `NO_TRIP_CONTEXT` codes above |
|
||||||
| `trek:confirm:result` | `{ requestId, confirmed }` — the user's answer to your `trek:confirm` |
|
| `trek:confirm:result` | `{ requestId, confirmed }` — the user's answer to your `trek:confirm` |
|
||||||
| `trek:event` | `{ event, tripId }` — a core event fired on the trip in view; **names only, never payloads** — refetch what you need via `trek:invoke` |
|
| `trek:event` | `{ event, tripId }` — a core event fired on the trip in view; **names only, never payloads** — refetch what you need via `trek:invoke` |
|
||||||
| `trek:geolocation:result` | `{ requestId, position? \| watching? \| cleared? \| error? }` — the answer to a `trek:geolocation` request (`error` ∈ `forbidden`/`denied`/`timeout`/`unavailable`/`unsupported`) |
|
| `trek:geolocation:result` | `{ requestId, position? \| watching? \| cleared? \| error? }` — the answer to a `trek:geolocation` request (`error` ∈ `forbidden`/`denied`/`timeout`/`unavailable`/`unsupported`) |
|
||||||
|
|||||||
Reference in New Issue
Block a user