From f178f1a972bb8730bc814b185e0604e77d621697 Mon Sep 17 00:00:00 2001 From: William Mantly Date: Thu, 6 Aug 2026 11:51:37 -0400 Subject: [PATCH] fix: test email/SMS senders, all SMS delivery, join-key install UI (v1.30.1) Test Email always failed with "Email.send is not a function": models/email.js exports {Mail}, and the handler required the module and called .send on it directly. Every other caller destructures it. Test SMS failed with "Unexpected token '<'": it POSTed to https://api.voip.ms/v1.0/sms/send with Basic auth, an endpoint that does not exist. VoIP.ms's REST API is a GET against voip.ms/api/v1/rest.php with api_username/api_password and method=sendSMS, so the fabricated URL returned HTML and response.json() threw. Worse, ALL SMS delivery was broken. models/sms.js called PluginInstance.find({...}) but the ORM has no find -- the query method is list({where}) -- so it threw on every send, before it could even fall back to the direct VoIP.ms path. OTP-by-SMS and notifications were dead. Both test endpoints now send through the same senders every real message uses. A test that reimplements delivery proves nothing about whether real delivery works, which is how two broken paths went unnoticed. Failures report as 400 with the underlying reason rather than an opaque 500. Adds a guard suite that fails the build on any call to a non-existent ORM static, on requiring models/email without destructuring {Mail}, and on any reference to the bogus api.voip.ms host. Also: the Install Agent modal now leads with the join-key flow. v1.30.0 shipped join keys in the API and documented the modal as the place to get one, but the modal still only did the pre-register flow. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 16 ++++ nodejs/models/sms.js | 7 +- nodejs/package-lock.json | 4 +- nodejs/package.json | 2 +- nodejs/routes/api_conf.js | 71 ++++++++------- nodejs/tests/orm_method_guard.test.js | 118 ++++++++++++++++++++++++ nodejs/views/directory.ejs | 125 +++++++++++++++++++++++++- 7 files changed, 306 insertions(+), 37 deletions(-) create mode 100644 nodejs/tests/orm_method_guard.test.js 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($('