diff --git a/src/routes/adminConfig.js b/src/routes/adminConfig.js index 18e599b9..890e27db 100644 --- a/src/routes/adminConfig.js +++ b/src/routes/adminConfig.js @@ -1011,6 +1011,19 @@ router.get('/websearch', adminMiddleware, async function (req, res) { for (var i = 0; i < WEBSEARCH_KEYS.length; i++) { out[WEBSEARCH_KEYS[i]] = await db.getSetting(WEBSEARCH_KEYS[i], '') || ''; } + // The key belongs to the selected provider. The old shared slot is the + // fallback, so a setup made before per-provider keys still shows its key. + var webSearchLib = require('../utils/webSearch'); + var selected = out['websearch.provider'] || 'tavily'; + out['websearch.api_key'] = + (await db.getSetting(webSearchLib.keySetting(selected), '') || '') || out['websearch.api_key']; + // Which providers already have one, so the screen can say so without ever + // sending a key back. + out.configuredProviders = []; + for (var p = 0; p < webSearchLib.PROVIDERS.length; p++) { + var name = webSearchLib.PROVIDERS[p]; + if (await db.getSetting(webSearchLib.keySetting(name), '')) out.configuredProviders.push(name); + } // Never send the key back. Enough tail to recognise which one is set. ['websearch.api_key', 'pubmed.api_key'].forEach(function (k) { if (out[k]) out[k] = '••••••••' + out[k].slice(-4); @@ -1024,7 +1037,8 @@ router.get('/websearch', adminMiddleware, async function (req, res) { router.put('/websearch', adminMiddleware, async function (req, res) { try { - var providers = require('../utils/webSearch').PROVIDERS; + var webSearchLib = require('../utils/webSearch'); + var providers = webSearchLib.PROVIDERS; var provider = String(req.body.provider || 'tavily'); if (providers.indexOf(provider) === -1) return res.status(400).json({ error: 'Unknown provider' }); @@ -1041,7 +1055,11 @@ router.put('/websearch', adminMiddleware, async function (req, res) { // silently wipe a key that was already working. The mask can never be saved // back as a key. var key = String(req.body.apiKey || '').trim(); - if (key && key.indexOf('•') === -1) await db.setSetting('websearch.api_key', key.slice(0, 400)); + if (key && key.indexOf('•') === -1) { + // Against this provider, not a shared slot: switching provider must not + // mean re-pasting, and the keys are not interchangeable. + await db.setSetting(webSearchLib.keySetting(provider), key.slice(0, 400)); + } var pmKey = String(req.body.pubmedApiKey || '').trim(); if (pmKey && pmKey.indexOf('•') === -1) await db.setSetting('pubmed.api_key', pmKey.slice(0, 400)); diff --git a/src/routes/auth.js b/src/routes/auth.js index 2e49948b..9323d44d 100644 --- a/src/routes/auth.js +++ b/src/routes/auth.js @@ -342,7 +342,12 @@ router.post('/login-code/request', requireLocalAuth, async (req, res) => { loginCodes.sweep(db); // Fire and forget: whether the mail went is not something the response may // reveal, and the password is still there if it did not. - sendEmail(user.email, 'Your sign-in code', loginCodes.emailBody(code, 'PedAI')) + // Through the same wrapper as the rest of the mail, so a sign-in code looks + // like it came from the same place as the verification and reset emails. + // It said 'PedAI' while every other message says SITE_NAME. + var codeSiteName = process.env.SITE_NAME || 'Pediatric AI Scribe'; + sendEmail(user.email, 'Your sign-in code for ' + codeSiteName, + emailWrapper(loginCodes.emailBody(code, codeSiteName, user.email))) .catch(function (err) { console.warn('[Auth] login-code send failed:', err.message); }); await db.run('INSERT INTO audit_log (user_id, action, ip_address) VALUES (?, ?, ?)', diff --git a/src/utils/loginCodes.js b/src/utils/loginCodes.js index e7b598d2..56dde832 100644 --- a/src/utils/loginCodes.js +++ b/src/utils/loginCodes.js @@ -98,12 +98,52 @@ async function sweep(db) { } catch (e) { /* housekeeping never fails a request */ } } -function emailBody(code, appName) { - return '
Your sign-in code for ' + (appName || 'PedAI') + ' is:
' + - '' + code + '
' + - 'It expires in ' + TTL_MINUTES + ' minutes and can be used once.
' + - 'If you did not ask to sign in, ignore this message ' + - 'and nothing will happen.
'; +// The address is the one thing here that did not come from us, and it is +// written into HTML, so it is escaped. The code cannot carry markup — it is six +// digits from a generator — but escaping only the interesting half is how the +// other half becomes interesting later. +function escapeHtml(value) { + return String(value == null ? '' : value) + .replace(/&/g, '&').replace(//g, '>') + .replace(/"/g, '"').replace(/'/g, '''); +} + +/** + * The body of the sign-in code email. + * + * Written for someone holding a phone in one hand: the code is the only thing + * on the screen that is large, it sits in a box of its own so it is obvious + * what to copy, and it is monospaced so a 0 cannot be mistaken for an O while + * being retyped. Everything else is quiet. + * + * It names the address the code signs into. A code that arrives at a shared + * mailbox, or to someone with two accounts, is otherwise a number with no + * indication of what it opens — and it is the one detail that lets a person + * notice a sign-in they did not start. + * + * Goes inside emailWrapper with the other mail, which supplies the wordmark, + * the rules and the footer — including the line about ignoring it, which is why + * there is not a second one here. + */ +function emailBody(code, appName, email) { + var name = escapeHtml(appName || 'PedAI'); + return '' + + 'Sign in to ' + name + '
' + + (email + ? '' + + 'Enter this code to sign in as ' + escapeHtml(email) + '.
' + : 'Enter this code to sign in.
') + + + // A table, not a div: Outlook ignores padding and border-radius on a div, + // and this box is the whole point of the message. + '| ' + + '' + escapeHtml(code) + '' + + ' |
' + + 'It expires in ' + TTL_MINUTES + ' minutes and can be used once.
'; } module.exports = { issue, consume, sweep, generate, normalise, emailBody, diff --git a/src/utils/webSearch.js b/src/utils/webSearch.js index 233cfb5a..cd7b4311 100644 --- a/src/utils/webSearch.js +++ b/src/utils/webSearch.js @@ -22,11 +22,26 @@ var PROVIDERS = ['tavily', 'serper', 'brave', 'exa', 'searxng']; var MAX_RESULTS = 8; var TIMEOUT_MS = 15000; +// One key per provider. There used to be a single websearch.api_key shared by +// all of them, so trying a different provider meant pasting a new key over the +// working one and pasting the old one back to return — and the two are not +// interchangeable, so a wrong pairing fails as an authentication error that +// looks like a dead provider. +// +// The shared key is still read as a fallback: it is whatever was configured +// before this, and for the provider that was selected at the time it is the +// right key. The first save under a provider writes its own. +function keySetting(provider) { + return 'websearch.api_key.' + provider; +} + async function settings() { + var provider = String(await db.getSetting('websearch.provider', 'tavily') || 'tavily'); + var own = String(await db.getSetting(keySetting(provider), '') || ''); return { enabled: String(await db.getSetting('websearch.enabled', 'false')) === 'true', - provider: String(await db.getSetting('websearch.provider', 'tavily') || 'tavily'), - apiKey: String(await db.getSetting('websearch.api_key', '') || ''), + provider: provider, + apiKey: own || String(await db.getSetting('websearch.api_key', '') || ''), // SearXNG has no key; it needs somewhere to reach instead. baseUrl: String(await db.getSetting('websearch.base_url', '') || '') }; @@ -177,4 +192,4 @@ function formatForPrompt(results) { }).join('\n\n'); } -module.exports = { search, isAvailable, settings, formatForPrompt, PROVIDERS, MAX_RESULTS }; +module.exports = { search, isAvailable, settings, formatForPrompt, keySetting, PROVIDERS, MAX_RESULTS }; diff --git a/test/web-search.test.js b/test/web-search.test.js index 2578426f..31d73582 100644 --- a/test/web-search.test.js +++ b/test/web-search.test.js @@ -168,8 +168,10 @@ test('the key is masked on read and preserved when left blank', () => { // Both keys, one rule: never send a key back, enough tail to recognise it. assert.match(admin, /\['websearch\.api_key', 'pubmed\.api_key'\]\.forEach/); assert.match(admin, /if \(out\[k\]\) out\[k\] = '••••••••' \+ out\[k\]\.slice\(-4\);/); - // Changing the provider must not silently wipe a working key. - assert.match(admin, /if \(key && key\.indexOf\('•'\) === -1\) await db\.setSetting\('websearch\.api_key'/); + // Changing the provider must not silently wipe a working key — now literally: + // each provider has its own slot, so switching does not overwrite anything. + assert.match(admin, /if \(key && key\.indexOf\('•'\) === -1\) \{/); + assert.match(admin, /db\.setSetting\(webSearchLib\.keySetting\(provider\), key\.slice\(0, 400\)\)/); assert.match(admin, /if \(pmKey && pmKey\.indexOf\('•'\) === -1\) await db\.setSetting\('pubmed\.api_key'/); assert.match(admin, /if \(providers\.indexOf\(provider\) === -1\)/, 'and the provider is validated'); }); @@ -246,3 +248,45 @@ test('the admin can choose it, and the server accepts what the admin can choose' assert.ok(lib.PROVIDERS.includes(provider), provider + ' is offered in the admin but rejected by the server'); } }); + +// ---- one key per provider -------------------------------------------------- +// There used to be a single websearch.api_key shared by all of them, so trying +// a different provider meant pasting a new key over the working one and pasting +// the old one back to return. The keys are not interchangeable, so a wrong +// pairing fails as an authentication error that looks like a dead provider. + +test('each provider keeps its own key', () => { + const lib = load({}, async () => ({ ok: true, json: async () => ({}) })); + assert.equal(lib.keySetting('exa'), 'websearch.api_key.exa'); + assert.equal(lib.keySetting('tavily'), 'websearch.api_key.tavily'); + assert.notEqual(lib.keySetting('exa'), lib.keySetting('tavily')); +}); + +test('the selected provider gets its own key, not another provider\'s', async () => { + let seen = null; + const lib = load({ + 'websearch.enabled': 'true', 'websearch.provider': 'exa', + 'websearch.api_key.exa': 'exa-key', 'websearch.api_key.tavily': 'tavily-key' + }, async (url, options) => { seen = options; return { ok: true, status: 200, json: async () => ({ results: [] }) }; }); + await lib.search('x'); + assert.equal(seen.headers['x-api-key'], 'exa-key'); + assert.equal((await lib.settings()).apiKey, 'exa-key'); +}); + +test('a key saved before per-provider slots still works', async () => { + // Whatever was configured before this change is the right key for whichever + // provider was selected at the time, so the old shared slot is the fallback. + const lib = load({ + 'websearch.enabled': 'true', 'websearch.provider': 'tavily', 'websearch.api_key': 'legacy' + }, async () => ({ ok: true, status: 200, json: async () => ({ results: [] }) })); + assert.equal((await lib.settings()).apiKey, 'legacy'); + assert.equal(await lib.isAvailable(), true); +}); + +test('a provider-specific key wins over the shared one', async () => { + const lib = load({ + 'websearch.enabled': 'true', 'websearch.provider': 'exa', + 'websearch.api_key': 'legacy', 'websearch.api_key.exa': 'the-exa-one' + }, async () => ({ ok: true, status: 200, json: async () => ({ results: [] }) })); + assert.equal((await lib.settings()).apiKey, 'the-exa-one'); +});