From 3e10b6faa7d5a7d194e18bee60bb33b0118d085b Mon Sep 17 00:00:00 2001 From: Daniel Date: Sun, 13 Sep 2026 17:35:35 +0200 Subject: [PATCH] feat: sign out ends the PedsHub session too (RP-initiated logout); the assistant sends the stored excerpt only Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU --- docs/authentication.md | 13 +++++++++++ docs/retrieval-tuning.md | 4 ++-- public/components/admin.html | 4 ---- public/js/admin/clinicalAssistant.js | 2 -- public/js/auth.js | 21 ++++++++++++++--- src/routes/auth.js | 24 +++++++++++++++++++- src/routes/clinicalAssistant.js | 19 ++++------------ src/utils/clinicalMcpClient.js | 2 -- test/admin-clinical-assistant-wiring.test.js | 6 ++--- test/backend-hardening.test.js | 20 ++++++++++++---- test/clinical-release-integration.test.js | 2 +- test/frontend-prompt-env.test.js | 6 +---- 12 files changed, 82 insertions(+), 41 deletions(-) diff --git a/docs/authentication.md b/docs/authentication.md index 0e39b0ba..b006f21a 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -28,6 +28,19 @@ sign-in page (`?sso=none`, no message). The attempt happens once per browser session, never after an explicit sign-out and never inside the mobile shell, and the URL fragment (a share link, a tab) is kept across the round trip. +## Signing out signs you out of PedsHub + +Sign out ends this app's session and then the PedsHub (Authentik) session: +the server answers the sign-out with the provider's end-session address +(`end_session_endpoint` from discovery, with `post_logout_redirect_uri` back +to this app and the client id), and the browser goes there and returns. So +"sign out" means signed out — on a shared ward computer the next person is not +one click from the account — and because the two PedsHub apps share the one +provider session, signing out of either signs you out of both. The app's +landing page must be in the provider's redirect list for the return to work; +without it Authentik shows its own "you've logged out" page, which still ends +the session. The mobile shell only signs out locally. + ## Lockdown: the admin panel as view-only `ADMIN_LOCKDOWN=true` in the environment (never a setting, so no admin can diff --git a/docs/retrieval-tuning.md b/docs/retrieval-tuning.md index 4a3a0615..e75c0e3e 100644 --- a/docs/retrieval-tuning.md +++ b/docs/retrieval-tuning.md @@ -53,7 +53,7 @@ clamped on read so a bad value cannot break a search. | Feature | Keys | Default | Clamp | |---|---|---|---| -| Clinical Assistant | `clinical_assistant.search_limit`, `clinical_assistant.context_chars` | 8, 0 | 3–20, 0–4000 (0 = excerpt only) | +| Clinical Assistant | `clinical_assistant.search_limit` | 8 | 3–20 | | My Resources | `learning.search_limit`, `learning.context_chars` | 30, 2500 | 3–60, 300–8000 | The `learning.*` names are historical: they were the Learning Hub's, and My @@ -61,7 +61,7 @@ Resources inherited the retrieval code when that was removed. Renaming the keys would orphan whatever an administrator has already set, so they keep the old names. -`search_limit` is how many excerpts to request; `context_chars` is how much extra text around each excerpt to fetch from the source at query time — 0, the default, sends the stored excerpt as it is, which already carries the page, its tables and figure captions; any other value makes the search service download and re-extract the source document per hit (about a second each, cached for 15 minutes). It is how much text +`search_limit` is how many excerpts to request. (There used to be a `context_chars` setting that made the search service download and re-extract the source document per hit to widen the excerpt; it is gone — the stored excerpt already carries the page, its tables and figure captions.) `learning.context_chars` is how much text to pull around each one. See [my-resources.md](my-resources.md) for the rest of that feature — its diff --git a/public/components/admin.html b/public/components/admin.html index b1ee3358..1d49b0cc 100644 --- a/public/components/admin.html +++ b/public/components/admin.html @@ -407,10 +407,6 @@ -
- - -
Library index diff --git a/public/js/admin/clinicalAssistant.js b/public/js/admin/clinicalAssistant.js index 524cdbbf..fc305f3e 100644 --- a/public/js/admin/clinicalAssistant.js +++ b/public/js/admin/clinicalAssistant.js @@ -226,7 +226,6 @@ export function initClinicalAssistantAdmin(adminEscapeHtml) { setValue('assistant-indexer-url', cfg['clinical_assistant.indexer_url'] || ''); setValue('assistant-indexer-token', ''); // never echoed back; blank means keep loadLibraryIndexStatus(); - setValue('assistant-context-chars', cfg['clinical_assistant.context_chars'] == null || cfg['clinical_assistant.context_chars'] === '' ? '0' : cfg['clinical_assistant.context_chars']); setValue('assistant-translate-provider', 'libretranslate'); // the only provider the server accepts var sourcesBox = document.getElementById('assistant-show-sources'); if (sourcesBox) { @@ -436,7 +435,6 @@ export function initClinicalAssistantAdmin(adminEscapeHtml) { Promise.all([ putAssistantConfig('clinical_assistant.conversation_chars', getValue('assistant-conversation-budget')), putAssistantConfig('clinical_assistant.search_limit', getValue('assistant-search-limit') || '8'), - putAssistantConfig('clinical_assistant.context_chars', getValue('assistant-context-chars') || '0'), putAssistantConfig('clinical_assistant.translate_provider', getValue('assistant-translate-provider') || 'libretranslate'), putAssistantConfig('clinical_assistant.show_sources', (document.getElementById('assistant-show-sources') || {}).checked === false ? 'false' : 'true') diff --git a/public/js/auth.js b/public/js/auth.js index bd63737c..3c755a0e 100644 --- a/public/js/auth.js +++ b/public/js/auth.js @@ -528,13 +528,28 @@ document.addEventListener('DOMContentLoaded', function() { if (boundary.blocked()) return; var headers = getAuthHeaders(); boundary.end(); // Synchronously hide and stop activity before changing credentials/cookies. - var logout = boundary.logoutRequest(headers).catch(function() {}); + // Signing out means signed out at PedsHub too, not just here: the server + // answers with the provider's end-session address and the browser goes + // there, then comes back to this page signed out. The signed-out latch is + // already set, so the page will not silently sign in again on return. + var endSessionUrl = null; + var logout = boundary.logoutRequest(headers) + .then(function(r) { return r && r.json ? r.json() : null; }) + .then(function(data) { if (data && data.endSessionUrl) endSessionUrl = String(data.endSessionUrl); }) + .catch(function() {}); var clearing = clearSession(true); var forgetting = window.PedBio ? window.PedBio.forget() : Promise.resolve(); + var leaving = false; // The signed-out latch survives both failed logout and a canceled reload. - Promise.all([logout, clearing, forgetting]).finally(function() { boundary.reload(); }); + Promise.all([logout, clearing, forgetting]).finally(function() { + if (endSessionUrl && !isNativeApp() && /^https:\/\//.test(endSessionUrl)) { + leaving = true; + try { window.location.replace(endSessionUrl); return; } catch (e) { leaving = false; } + } + boundary.reload(); + }); // A hung network/native bridge must not leave the old UI usable either. - setTimeout(function() { boundary.reload(); }, 3000); + setTimeout(function() { if (!leaving) boundary.reload(); }, 3000); } function clearSession(explicit) { diff --git a/src/routes/auth.js b/src/routes/auth.js index e605ca91..57e773b9 100644 --- a/src/routes/auth.js +++ b/src/routes/auth.js @@ -617,9 +617,31 @@ router.post('/logout', async function(req, res) { } } catch (e) { /* best effort */ } clearAuthCookie(res); - res.json({ success: true }); + // Signing out means signed out, not "this app forgot you": the provider's + // session ends too, or the next person at a ward computer is one click from + // the account, and the other PedsHub app would still be signed in. The page + // sends the browser to the provider's end-session URL and comes back here. + var endSessionUrl = null; + try { endSessionUrl = await providerEndSessionUrl(); } catch (e) { endSessionUrl = null; } + res.json({ success: true, endSessionUrl: endSessionUrl }); }); +async function providerEndSessionUrl() { + if (await db.getSetting('oidc.enabled') !== 'true') return null; + var issuer = await db.getSetting('oidc.issuer'); + var clientId = await db.getSetting('oidc.client_id'); + if (!issuer || !clientId) return null; + var oidc = require('openid-client'); + var config = await oidc.discovery(new URL(issuer), clientId); + var meta = config.serverMetadata(); + if (!meta.end_session_endpoint) return null; + var appUrl = (process.env.APP_URL || 'http://localhost:3000').replace(/\/$/, '') + '/'; + var url = new URL(meta.end_session_endpoint); + url.searchParams.set('post_logout_redirect_uri', appUrl); + url.searchParams.set('client_id', clientId); + return url.href; +} + // Returns true if the user has a real password hash and can change it. // SSO-auto-created users have a random hex blob in `password` — changing // it is meaningless because they never authenticate locally. diff --git a/src/routes/clinicalAssistant.js b/src/routes/clinicalAssistant.js index 88a33e52..70a7ffdd 100644 --- a/src/routes/clinicalAssistant.js +++ b/src/routes/clinicalAssistant.js @@ -91,8 +91,7 @@ router.get('/clinical-assistant/status', async function(req, res) { var chatModel = choices.chatConfigured; var imageModel = choices.imageConfigured; var searchLimit = clampInt(await getSetting('clinical_assistant.search_limit', '8'), 3, 20, 8); - var contextChars = clampInt(await getSetting('clinical_assistant.context_chars', '0'), 0, 4000, 0); - var translateProvider = String(await getSetting('clinical_assistant.translate_provider', '') || 'libretranslate').toLowerCase(); + var translateProvider = String(await getSetting('clinical_assistant.translate_provider', '') || 'libretranslate').toLowerCase(); if (!clinicalTranslation.TRANSLATE_PROVIDERS.includes(translateProvider)) translateProvider = 'libretranslate'; var budget = conversationBudget(process.env); var mcpHealth = await getMcpHealth(); @@ -103,7 +102,6 @@ router.get('/clinical-assistant/status', async function(req, res) { allowedChatModels: choices.allowedChatModels, allowedImageModels: choices.allowedImageModels, searchLimit: searchLimit, - contextChars: contextChars, conversationChars: budget.limit, conversationUnit: budget.unit, conversationEnv: budget.env, @@ -574,13 +572,7 @@ async function prepareAssistantChat(body) { // The model that gets shown an attachment when the chat model cannot be. var visionModel = String(await getSetting('clinical_assistant.vision_model', '') || ''); var searchLimit = clampInt(await getSetting('clinical_assistant.search_limit', '8'), 3, 20, 8); - var contextChars = clampInt(await getSetting('clinical_assistant.context_chars', '0'), 0, 4000, 0); var behavior = await getSetting('clinical_assistant.system_behavior', DEFAULT_BEHAVIOR) || DEFAULT_BEHAVIOR; - // Context expansion made the search service download each hit's whole PDF - // and extract it again at query time to widen the excerpt — a second per - // hit. The stored excerpt already carries the page, its tables and figure - // captions, so the default is off: 0 characters means "the excerpt only". - var includeContext = body.includeContext !== false && contextChars > 0; var showSources = await showSourcesEnabled(); // Phase timings, numbers only, so "is it slow?" can be answered from the @@ -593,11 +585,10 @@ async function prepareAssistantChat(body) { return message; }); timing.rewriteMs = Date.now() - phase; phase = Date.now(); - var searchResponse = await semanticSearch(searchQuery, { - limit: searchLimit, - includeContext: includeContext, - contextChars: contextChars - }); + // The stored excerpt is what the model sees: it already carries the page, + // its tables and figure captions. The search service no longer re-reads + // the source document at query time. + var searchResponse = await semanticSearch(searchQuery, { limit: searchLimit }); timing.searchMs = Date.now() - phase; // Retrieval is text-only. The multimodal path called nc_multimodal_search // against a second hardcoded collection with an embedding service that was diff --git a/src/utils/clinicalMcpClient.js b/src/utils/clinicalMcpClient.js index 565610af..41c6e0bc 100644 --- a/src/utils/clinicalMcpClient.js +++ b/src/utils/clinicalMcpClient.js @@ -89,8 +89,6 @@ async function semanticSearch(query, opts) { doc_types: ['file'], score_threshold: 0, fusion: 'rrf', - include_context: opts.includeContext, - context_chars: opts.contextChars }); } diff --git a/test/admin-clinical-assistant-wiring.test.js b/test/admin-clinical-assistant-wiring.test.js index ebb2010a..7c8a9086 100644 --- a/test/admin-clinical-assistant-wiring.test.js +++ b/test/admin-clinical-assistant-wiring.test.js @@ -80,15 +80,15 @@ test('native admin initializer preserves lazy navigation, assistant actions and const save = document.getElementById('btn-save-assistant-config'); save.click(); await tick(); // Each card saves exactly what it shows. Save & Close on the Clinical - // Assistant card writes the five retrieval/citation/translation/budget keys; + // Assistant card writes the four retrieval/citation/translation/budget keys; // the chat model and the two allowed lists belong to the Availability card // and are written by its own Save below. The signed-out preview is a feature // flag saved by the Feature Flags card, not by either. - assert.equal(writes().length, 5); + assert.equal(writes().length, 4); assert.equal(writes().filter(c => /preview/.test(c.url)).length, 0, 'this button no longer writes the preview flag'); assert.deepEqual(writes().map(c => c.url.split('/').pop()).sort(), [ - 'clinical_assistant.context_chars', 'clinical_assistant.conversation_chars', 'clinical_assistant.search_limit', 'clinical_assistant.show_sources', 'clinical_assistant.translate_provider' + 'clinical_assistant.conversation_chars', 'clinical_assistant.search_limit', 'clinical_assistant.show_sources', 'clinical_assistant.translate_provider' ]); assert.ok(toasts.some(([message, kind]) => message === 'Assistant settings saved' && kind === 'success')); assert.equal(save.closest('details').open, false, 'Save & Close folds the card once saved'); diff --git a/test/backend-hardening.test.js b/test/backend-hardening.test.js index 22b858e3..ae7c2db3 100644 --- a/test/backend-hardening.test.js +++ b/test/backend-hardening.test.js @@ -297,9 +297,21 @@ test('silent SSO: prompt=none on request, a refusal is not an error, the page tr }); -test('context expansion is off unless an admin asks for it: 0 characters means the stored excerpt only', () => { +test('the assistant sends the stored excerpt only: no context expansion request, no setting, no field', () => { const src = read('src/routes/clinicalAssistant.js'); - assert.equal((src.match(/getSetting\('clinical_assistant\.context_chars', '0'\), 0, 4000, 0\)/g) || []).length, 2); - assert.match(src, /includeContext = body\.includeContext !== false && contextChars > 0;/); - assert.match(read('public/components/admin.html'), /id="assistant-context-chars" type="number" min="0"/); + assert.doesNotMatch(src, /includeContext|contextChars|context_chars/); + assert.doesNotMatch(read('src/utils/clinicalMcpClient.js'), /include_context|context_chars/); + assert.doesNotMatch(read('public/components/admin.html'), /assistant-context-chars/); + assert.doesNotMatch(read('public/js/admin/clinicalAssistant.js'), /context_chars/); +}); + + +test('sign-out ends the provider session: the server hands back the end-session address and the page goes there', () => { + const auth = read('src/routes/auth.js'); + assert.match(auth, /res\.json\(\{ success: true, endSessionUrl: endSessionUrl \}\)/); + assert.match(auth, /url\.searchParams\.set\('post_logout_redirect_uri', appUrl\)/); + const js = read('public/js/auth.js'); + assert.match(js, /window\.location\.replace\(endSessionUrl\)/); + assert.match(js, /if \(!leaving\) boundary\.reload\(\)/); + assert.match(read('docs/authentication.md'), /## Signing out signs you out of PedsHub/); }); diff --git a/test/clinical-release-integration.test.js b/test/clinical-release-integration.test.js index abbd0a1f..6d62b421 100644 --- a/test/clinical-release-integration.test.js +++ b/test/clinical-release-integration.test.js @@ -89,7 +89,7 @@ test('native admin and assistant modules retain budget, table/source identity an // Five: Save & Close writes the Clinical Assistant card's own settings. The // chat model and allowed lists are saved by the Availability card, and the // signed-out preview by the Feature Flags card. - assert.equal(calls.filter(call => call.options.method === 'PUT').length, 5, 'one native admin initializer; prompts are not generic setting saves'); + assert.equal(calls.filter(call => call.options.method === 'PUT').length, 4, 'one native admin initializer; prompts are not generic setting saves'); assert.equal(calls.some(call => call.url.endsWith('/config/clinical_assistant.conversation_chars')), true, 'the conversation budget is an admin-settable override'); document.dispatchEvent(new window.CustomEvent('tabChanged', { detail: { tab: 'assistant' } })); await tick(); await tick(); await tick(); diff --git a/test/frontend-prompt-env.test.js b/test/frontend-prompt-env.test.js index 2fd1a38a..5b35a9fe 100644 --- a/test/frontend-prompt-env.test.js +++ b/test/frontend-prompt-env.test.js @@ -268,7 +268,6 @@ const assistantConfig = () => ({ success: true, config: [ { key: 'clinical_assistant.chat_model', value: 'saved-chat' }, { key: 'clinical_assistant.image_model', value: 'saved-image' }, { key: 'clinical_assistant.search_limit', value: '17' }, - { key: 'clinical_assistant.context_chars', value: '2300' } ], conversationBudget: { limit: 240000, unit: 'characters', measure: 'UTF-16 code units', env: 'CLINICAL_ASSISTANT_CONVERSATION_CHARS', source: 'environment' } }); const adminVisit = ui => ui.document.dispatchEvent(new ui.window.CustomEvent('tabChanged', { detail: { tab: 'admin' } })); const setting = (ui, name) => ui.document.getElementById('assistant-' + name); @@ -303,14 +302,12 @@ test('assistant config GET503 plus Save makes zero PUTs; failed retry preserves assert.equal(ui.document.getElementById('workflow-image-settings').children.length, 0, 'image settings load only after config success'); assert.equal(ui.calls.some(c => c.url.endsWith('/image-models/discover')), false); setting(ui, 'search-limit').value = '21'; - setting(ui, 'context-chars').value = '3100'; setting(ui, 'chat-model').appendChild(new ui.window.Option('Draft chat', 'draft-chat')); const retry = ui.document.getElementById('btn-retry-assistant-config'); assert.equal(retry.hidden, false); assert.equal(retry.type, 'button'); retry.click(); await tick(); assert.equal(setting(ui, 'search-limit').value, '21'); - assert.equal(setting(ui, 'context-chars').value, '3100'); assert.equal(setting(ui, 'chat-model').value, 'draft-chat'); await forceAssistantSave(ui); assert.equal(writes(ui).length, 0); assert.equal(ui.calls.filter(c => c.url === '/api/admin/config').length, 2); @@ -318,7 +315,6 @@ test('assistant config GET503 plus Save makes zero PUTs; failed retry preserves adminVisit(ui); await tick(); assert.equal(setting(ui, 'chat-model').value, 'saved-chat'); assert.equal(setting(ui, 'search-limit').value, '17'); - assert.equal(setting(ui, 'context-chars').value, '2300'); // Empty means "no saved override" — see admin-clinical-assistant-wiring. // The environment value is the placeholder so Save cannot promote it silently. assert.equal(setting(ui, 'conversation-budget').value, '', 'no saved override, so the field stays empty'); @@ -339,7 +335,7 @@ test('assistant config GET503 plus Save makes zero PUTs; failed retry preserves // Saving an untouched form must NOT turn the environment value into a // stored override — empty is the "use the environment" signal. ['clinical_assistant.conversation_chars', ''], - ['clinical_assistant.search_limit', '19'], ['clinical_assistant.context_chars', '2300'], + ['clinical_assistant.search_limit', '19'], ['clinical_assistant.translate_provider', 'libretranslate'], ['clinical_assistant.show_sources', 'true'] ]);