diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ba2098..6165555 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,19 @@ +# v1.30.1 + +### Fixed + +- **Test Email always failed with `Email.send is not a function`.** `models/email.js` exports `{Mail}`; the handler required the module and called `.send` on it directly. Every other caller destructures it. The button could never have worked. +- **Test SMS failed with `Unexpected token '<', " 0) { const inst = instances[0]; const manifest = registry.getManifest(inst.pluginType); diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index e10fbe3..bd5e400 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -1,12 +1,12 @@ { "name": "t42-sso-manager", - "version": "1.30.0", + "version": "1.30.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "t42-sso-manager", - "version": "1.30.0", + "version": "1.30.1", "license": "MIT", "dependencies": { "@fortawesome/fontawesome-free": "^7.3.0", diff --git a/nodejs/package.json b/nodejs/package.json index 1938bec..32668d1 100755 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -1,6 +1,6 @@ { "name": "t42-sso-manager", - "version": "1.30.0", + "version": "1.30.1", "description": "A very simple LDAP management and SSO system", "author": [ { diff --git a/nodejs/routes/api_conf.js b/nodejs/routes/api_conf.js index 03deea7..525f5d9 100644 --- a/nodejs/routes/api_conf.js +++ b/nodejs/routes/api_conf.js @@ -136,15 +136,25 @@ router.post('/test-email', async (req, res, next) => { return res.status(400).json({ error: 'Recipient email address is required' }); } - // Use the email model to send the test message - const Email = require('../models/email'); + // Send through the SAME sender every other feature uses (password reset, + // invites, OTP-by-email, notifications). A "test" that reimplements + // delivery proves nothing about whether real mail works. + // + // models/email.js exports `{Mail}`; requiring the module and calling + // `.send` on it directly -- as this did -- always threw + // "Email.send is not a function", so the button could never succeed. + const { Mail } = require('../models/email'); const testSubject = subject || 'SSO Manager Test Email'; const testBody = body || `

This is a test email from SSO Manager.

If you received this, your SMTP configuration is working correctly.

Sent at: ${new Date().toISOString()}

`; - await Email.send(to, testSubject, testBody); + await Mail.send(to, testSubject, testBody); res.json({ success: true, message: `Test email sent to ${to}` }); } catch(err) { - next(err); + // A failed test is almost always a misconfiguration (wrong host, refused + // connection, bad credentials) -- the operator's to fix, and something the + // UI should be able to show them. Surfacing it as a 400 with the reason + // beats an opaque 500 carrying a raw stack-trace name. + return res.status(400).json({ error: err.message || 'Failed to send test email' }); } }); @@ -156,38 +166,37 @@ router.post('/test-sms', async (req, res, next) => { return res.status(400).json({ error: 'Recipient phone number is required' }); } + // Send through models/sms.js -- the same path every real SMS takes. It + // prefers a configured messaging plugin and falls back to VoIP.ms, and it + // normalizes the destination to E.164 digits. + // + // This used to POST to `https://api.voip.ms/v1.0/sms/send` with Basic auth. + // No such endpoint exists: VoIP.ms's REST API is a GET against + // `https://voip.ms/api/v1/rest.php` with `api_username`/`api_password` and + // `method=sendSMS`. The fabricated URL returned an HTML page, so + // `response.json()` threw `Unexpected token '<', " []); const voipmsConf = conf.voipms || {}; - if (!voipmsConf.username || !voipmsConf.password || !voipmsConf.did) { - return res.status(400).json({ error: 'VoIP.ms credentials not configured. Please configure username, DID, and password in the SMS tab.' }); + if (!messagingPlugins.length && (!voipmsConf.username || !voipmsConf.password || !voipmsConf.did)) { + return res.status(400).json({ error: 'No messaging plugin is loaded and VoIP.ms credentials are not configured. Set username, DID and password in the SMS tab, or load a messaging plugin.' }); } - const testMessage = message || `SSO Manager Test SMS: This is a test message from ${conf.name}. If you received this, your VoIP.ms configuration is working correctly.`; + const testMessage = message || `SSO Manager Test SMS: This is a test message from ${conf.name}. If you received this, your SMS configuration is working correctly.`; - // VoIP.ms SMS API endpoint - const voipmsApiUrl = 'https://api.voip.ms/v1.0'; - const authHeader = Buffer.from(`${voipmsConf.username}:${voipmsConf.password}`).toString('base64'); - - const response = await fetch(`${voipmsApiUrl}/sms/send`, { - method: 'POST', - headers: { - 'Authorization': `Basic ${authHeader}`, - 'Content-Type': 'application/x-www-form-urlencoded' - }, - body: new URLSearchParams({ - did: voipmsConf.did, - to: to, - message: testMessage - }) - }); - - const result = await response.json(); - if (result.status === 'success') { - res.json({ success: true, message: `Test SMS sent to ${to}` }); - } else { - res.status(400).json({ error: `VoIP.ms API error: ${result.message || 'Unknown error'}` }); - } + await SMS.send(to, testMessage); + res.json({ success: true, message: `Test SMS sent to ${to}` }); } catch(err) { - next(err); + // The sender rejects with a useful reason (`VoIP.ms error: `, or a + // plugin's own error). Surface it as a 400 the UI can display rather than + // an opaque 500 -- a misconfiguration is the operator's to fix, not a bug. + return res.status(400).json({ error: err.message || 'Failed to send test SMS' }); } }); diff --git a/nodejs/tests/orm_method_guard.test.js b/nodejs/tests/orm_method_guard.test.js new file mode 100644 index 0000000..da76c8d --- /dev/null +++ b/nodejs/tests/orm_method_guard.test.js @@ -0,0 +1,118 @@ +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +// @simpleworkjs/orm models expose `list`/`get`/`count`/`create` -- there is no +// `find`, `findOne`, `findAll` or `where`. Calling one is not a syntax error and +// nothing catches it until the line actually runs, so it can sit in a rarely +// exercised path indefinitely. +// +// It did: `models/sms.js` called `PluginInstance.find({...})`, which threw +// "is not a function" on EVERY SMS send -- the test button, OTP-by-SMS and +// notifications alike -- before it could even reach the VoIP.ms fallback. SMS +// delivery had simply never worked. +const ORM_MODELS = [ + 'Resource', 'ResourceEdge', 'ResourceGroup', 'AccessRequest', 'Webhook', + 'PluginInstance', 'SharedSecret', 'SharedSecretGrant', 'VaultAppToken', + 'Agent', 'AgentJoinKey', +]; +const MISSING_STATICS = ['find', 'findOne', 'findAll', 'findAndCountAll', 'where']; + +const ROOT = path.join(__dirname, '..'); +const SCAN_DIRS = ['models', 'routes', 'services', 'utils', 'plugins', 'controller', 'middleware']; + +function walk(dir, out = []) { + let entries; + try { entries = fs.readdirSync(dir, { withFileTypes: true }); } catch (e) { return out; } + for (const entry of entries) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name === 'node_modules') continue; + walk(full, out); + } else if (entry.name.endsWith('.js')) { + out.push(full); + } + } + return out; +} + +// Strip comments so a line *describing* the bug (like the one in models/sms.js) +// isn't reported as the bug. +function stripComments(src) { + return src + .replace(/\/\*[\s\S]*?\*\//g, '') + .replace(/(^|[^:])\/\/.*$/gm, '$1'); +} + +test('no source file calls an ORM static that does not exist', () => { + const pattern = new RegExp( + `\\b(${ORM_MODELS.join('|')})\\s*\\.\\s*(${MISSING_STATICS.join('|')})\\s*\\(`, + 'g' + ); + + const offenders = []; + for (const dir of SCAN_DIRS) { + for (const file of walk(path.join(ROOT, dir))) { + const src = stripComments(fs.readFileSync(file, 'utf8')); + src.split('\n').forEach((line, i) => { + const m = line.match(pattern); + if (m) offenders.push(`${path.relative(ROOT, file)}:${i + 1} — ${m.join(', ')}`); + }); + } + } + + expect(offenders).toEqual([]); +}); + +// models/email.js exports `{Mail}`, not a bare sender. Requiring the module and +// calling `.send` on it -- as routes/api_conf.js's test-email did -- always +// threw "Email.send is not a function", so the Test Email button could never +// have worked. +test('the email module exports Mail.send and callers destructure it', () => { + const mod = require('../models/email'); + expect(typeof mod.Mail).toBe('object'); + expect(typeof mod.Mail.send).toBe('function'); + // The bare module has no send() -- this is exactly the mistake to catch. + expect(mod.send).toBeUndefined(); + + const offenders = []; + for (const dir of SCAN_DIRS) { + for (const file of walk(path.join(ROOT, dir))) { + const src = stripComments(fs.readFileSync(file, 'utf8')); + // `X = require('...email')` followed by `X.send(` where X was not + // destructured. + const assigned = [...src.matchAll(/(?:const|let|var)\s+(\w+)\s*=\s*require\([^)]*models\/email[^)]*\)/g)] + .map(m => m[1]); + for (const name of assigned) { + if (new RegExp(`\\b${name}\\s*\\.\\s*send\\s*\\(`).test(src)) { + offenders.push(`${path.relative(ROOT, file)} — ${name}.send(), but the module exports {Mail}`); + } + } + } + } + expect(offenders).toEqual([]); +}); + +// The VoIP.ms REST API is a GET against voip.ms/api/v1/rest.php with +// api_username/api_password and method=sendSMS. `api.voip.ms/v1.0/sms/send` +// (which test-sms used to POST to with Basic auth) does not exist -- it +// returned an HTML page, so response.json() threw +// `Unexpected token '<', " { + const offenders = []; + for (const dir of SCAN_DIRS) { + for (const file of walk(path.join(ROOT, dir))) { + // Comments stripped: the note in routes/api_conf.js explaining this + // very bug names the bad host, and describing a mistake is not + // making it. + const src = stripComments(fs.readFileSync(file, 'utf8')); + src.split('\n').forEach((line, i) => { + if (line.includes('api.voip.ms')) { + offenders.push(`${path.relative(ROOT, file)}:${i + 1}`); + } + }); + } + } + expect(offenders).toEqual([]); +}); diff --git a/nodejs/views/directory.ejs b/nodejs/views/directory.ejs index 158030b..7b0d1b9 100644 --- a/nodejs/views/directory.ejs +++ b/nodejs/views/directory.ejs @@ -1640,6 +1640,21 @@ // public_key must reach the host: without it the agent refuses every // high-risk command. It was never emitted before, which is why signed // commands only ever "worked" while verification was being skipped. + // Join-key command. Only a key we just minted can appear here -- the list + // endpoint deliberately never returns key values. + const joinUrl = ($('#agent-quick-url').val() || window.location.origin).replace(/\/+$/, ''); + const selectedKeyId = $('#agent-join-key-select').val(); + let joinCmd; + if (mintedJoinKey) { + joinCmd = `curl -fsSL ${joinUrl}/resources/theta-agent/install.sh | sh -s -- --url "${joinUrl}" --join-key "${mintedJoinKey}"`; + } else if (selectedKeyId) { + const k = agentJoinKeys.find(x => x.id === selectedKeyId); + joinCmd = `curl -fsSL ${joinUrl}/resources/theta-agent/install.sh | sh -s -- --url "${joinUrl}" --join-key "${k ? k.keyPrefix : ''}…"\n\n# Paste the full value of this key -- it was only shown when created.\n# If you no longer have it, create a new key above.`; + } else { + joinCmd = '# Create a join key above, or select one you already have the value for.'; + } + $('#agent-join-command').text(joinCmd); + const pubKey = (pendingEnrollment && pendingEnrollment.publicKey) || ''; const quickCmd = `curl -fsSL ${quickUrl}/resources/theta-agent/install.sh | sh -s -- --url "${quickUrl}" --token "${quickToken}"` + (pubKey ? ` --public-key "${pubKey}"` : ''); @@ -1719,14 +1734,67 @@ + + +
+ +
+
+
+ Install with a join key +
+
+

+ Run this on any host and it enrolls itself. The SSO issues that host its own + token and public key on first connect, and the agent writes both into its + agent.yml — nothing to copy back and forth. One key works for as + many hosts as you like; each still gets its own revocable identity. +

+
+
+ + +
A key's value is shown only when it is created — mint a new one if you don't have it saved.
+
+ +
+ + + +

+              
+ +
+
+
+
+ + +
+
1. Enroll this host

- The SSO issues the agent's token and records it. Tokens it did not issue are rejected, - so enroll the host first — the install command below is built from the result. + Use this when you want the agent bound to a specific Directory host from the start. + The SSO issues the token here and you copy it onto the machine yourself.

@@ -1870,6 +1938,8 @@
+
+
`; app.modal.open({ @@ -1878,6 +1948,8 @@ size: 'lg' }); + loadAgentJoinKeys(); + // Only hosts can carry an agent -- the API rejects anything else, so don't // offer it here. const $sel = $('#agent-enroll-resource').empty(); @@ -1898,6 +1970,55 @@ updateAgentCommands(); } + // Join keys the operator can reuse. Values are never returned by the list + // endpoint -- only a prefix -- so the dropdown identifies a key without being + // able to rebuild an install command from it. Minting is the only way to see + // a key's value, and only once. + var agentJoinKeys = []; + var mintedJoinKey = null; // in-memory, for the command shown right now + + function loadAgentJoinKeys() { + app.api.get('agent/join-keys', function(err, res) { + agentJoinKeys = (res && res.joinKeys ? res.joinKeys : []).filter(k => !k.revoked); + const $sel = $('#agent-join-key-select').empty(); + if (!agentJoinKeys.length) { + $sel.append(''); + } else { + $sel.append(''); + agentJoinKeys.forEach(k => { + const used = k.use_count ? `${k.use_count} host${k.use_count === 1 ? '' : 's'}` : 'unused'; + $sel.append($('