feat: a key per search provider, and the sign-in code email looks like our mail
Two things, both about not making someone redo work. One key per provider. There was 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 keys are not interchangeable, so a wrong pairing fails as an authentication error that reads like a dead provider. Each now has its own slot. The old shared key is still read as a fallback: whatever was configured before this is the right key for whichever provider was selected at the time. The sign-in code email went out raw, while every other message this app sends goes through emailWrapper — so the one mail a person receives while locked out was the one that looked least like it came from us. It now uses the same wrapper, and the body is built around the thing the reader actually needs: the code, alone, large, monospaced so a 0 cannot be read as an O, in a box of its own. It also names the address it signs into. A code arriving at a shared mailbox, or to someone with two accounts, is otherwise a number with no indication of what it opens — and that line is the one thing that lets a person notice a sign-in they did not start. The address is escaped; it is the only part of that mail that did not come from us. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
This commit is contained in:
parent
302736af70
commit
0287fd091b
5 changed files with 136 additions and 14 deletions
|
|
@ -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));
|
||||
|
||||
|
|
|
|||
|
|
@ -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 (?, ?, ?)',
|
||||
|
|
|
|||
|
|
@ -98,12 +98,52 @@ async function sweep(db) {
|
|||
} catch (e) { /* housekeeping never fails a request */ }
|
||||
}
|
||||
|
||||
function emailBody(code, appName) {
|
||||
return '<p>Your sign-in code for ' + (appName || 'PedAI') + ' is:</p>' +
|
||||
'<p style="font-size:28px;font-weight:700;letter-spacing:4px;margin:16px 0;">' + code + '</p>' +
|
||||
'<p>It expires in ' + TTL_MINUTES + ' minutes and can be used once.</p>' +
|
||||
'<p style="color:#6b7280;font-size:13px;">If you did not ask to sign in, ignore this message ' +
|
||||
'and nothing will happen.</p>';
|
||||
// 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, '"').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 '' +
|
||||
'<p style="margin:0 0 8px;font-size:20px;font-weight:600;color:#111827;">Sign in to ' + name + '</p>' +
|
||||
(email
|
||||
? '<p style="margin:0 0 20px;color:#4b5563;font-size:14px;line-height:1.6;">' +
|
||||
'Enter this code to sign in as <strong style="color:#111827;">' + escapeHtml(email) + '</strong>.</p>'
|
||||
: '<p style="margin:0 0 20px;color:#4b5563;font-size:14px;line-height:1.6;">Enter this code to sign in.</p>') +
|
||||
|
||||
// A table, not a div: Outlook ignores padding and border-radius on a div,
|
||||
// and this box is the whole point of the message.
|
||||
'<table role="presentation" width="100%" cellpadding="0" cellspacing="0" border="0" style="width:100%;margin:0 0 20px;">' +
|
||||
'<tr><td align="center" style="background:#f9fafb;border:1px solid #d1d5db;border-radius:8px;padding:18px 28px;">' +
|
||||
'<span style="font-family:\'SFMono-Regular\',Consolas,\'Liberation Mono\',Menlo,monospace;' +
|
||||
'font-size:30px;font-weight:700;letter-spacing:8px;color:#111827;">' + escapeHtml(code) + '</span>' +
|
||||
'</td></tr></table>' +
|
||||
|
||||
'<p style="margin:0;color:#6b7280;font-size:13px;line-height:1.6;">' +
|
||||
'It expires in ' + TTL_MINUTES + ' minutes and can be used once.</p>';
|
||||
}
|
||||
|
||||
module.exports = { issue, consume, sweep, generate, normalise, emailBody,
|
||||
|
|
|
|||
|
|
@ -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 };
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue