DC-085 link-first invite — Discord-style share it however you want

Flip POST /api/v1/auth/admin/invites default to no email; always return the link. Operators copy + share via iMessage/WhatsApp/SMS/Signal/Telegram/ Discord/paste-in-email. Email becomes an opt-in checkbox (was the default). Add shareText field with pre-formatted message for one-tap paste. Stop logging raw invite URLs to error.log when SMTP is unconfigured (was just a dev fallback — link is now in the response). Frontend flips the checkbox default to unchecked and renders shareText + native share sheet button (navigator.share) alongside the raw copy-link button. 9 new tests covering default-no-send, link-always-returned, shareText-shape, opt-in SMTP send, failed-SMTP-no-leak. Full suite: 2474/2474 + 9 new = 2483.
2026-08-22 06:14:47 -07:00
parent 84edb035e3
commit eab2b00b13
4 changed files with 355 additions and 41 deletions
@@ -0,0 +1,240 @@
+/**
+ * Tests for DC-085: link-first invite (Discord-style "share it however you want").
+ *
+ *   - default sendEmail omission = no email sent, link returned, no token in logs
+ *   - sendEmail:true triggers SMTP send when configured
+ *   - sendEmail:true + SMTP unconfigured = deliveredVia:'failed', no token leaked
+ *   - shareText field present and well-formed in every response
+ *   - acceptUrl always present (regardless of sendEmail)
+ *   - role + ttl validation unchanged from DC-048
+ *
+ * Strategy: drive the route handler directly with mock req/res, mount the admin
+ * router against an isolated userStore + inviteStore + email-sender stub.
+ */
+
+'use strict';
+
+const fs = require('fs');
+const path = require('path');
+const os = require('os');
+const express = require('express');
+
+function _tmpDir() {
+  return fs.mkdtempSync(path.join(os.tmpdir(), 'dashcaddy-admin-invites-test-'));
+}
+function _cleanup(dir) {
+  try { fs.rmSync(dir, { recursive: true, force: true }); } catch {}
+}
+
+// Stub email-sender so we can assert "was it called?" without an SMTP server.
+// NOTE: the variable name MUST start with `mock` so Jest's hoisted `jest.mock()`
+// call is allowed to reference it (Babel guard against out-of-scope access).
+const mockEmailSender = {
+  isConfigured: jest.fn(() => false),
+  sendEmail: jest.fn(async () => undefined),
+};
+jest.mock('../src/auth/providers/email-sender', () => mockEmailSender);
+
+describe('DC-085: link-first admin invites', () => {
+  let dir, app, request;
+  let logCalls; // captured { level, msg, meta } from our fake log
+
+  beforeEach(async () => {
+    jest.clearAllMocks();
+    dir = _tmpDir();
+    logCalls = [];
+
+    // Set up email auth enable flag so userStore mounts.
+    process.env.NODE_ENV = 'test';
+
+    const { createUserStore } = require('../src/security/user-store');
+    const userStore = createUserStore({ dataDir: dir });
+
+    // Bootstrap the admin so we have a session-attributable user.
+    await userStore.login({ email: 'admin@sami-host.me' });
+
+    // Build a tiny Express app with the admin router mounted, but skip the
+    // global auth gate (we inject req.user directly).
+    const adminRouter = require('../routes/auth/admin')({
+      asyncHandler: (fn) => (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next),
+      errorResponse: (_res, code, msg) => ({ status: code, msg }),
+      log: {
+        info: (topic, msg, meta) => logCalls.push({ level: 'info', topic, msg, meta }),
+        warn: (topic, msg, meta) => logCalls.push({ level: 'warn', topic, msg, meta }),
+        error: (topic, msg, meta) => logCalls.push({ level: 'error', topic, msg, meta }),
+      },
+      session: { isSessionValid: () => true, create: () => {}, setCookie: () => {} },
+      dataDir: dir,
+    });
+
+    app = express();
+    app.use(express.json());
+    // Inject req.user = admin so /admin/* passes the role gate.
+    app.use((req, _res, next) => {
+      req.user = { id: 'admin-id', email: 'admin@sami-host.me', role: 'admin' };
+      req.app.locals = req.app.locals || {};
+      req.app.locals.siteConfig = {}; // no publicBaseUrl — route uses req.headers
+      req.app.locals.emailConfig = null; // SMTP not configured by default
+      next();
+    });
+    app.use('/api/v1/auth', adminRouter);
+    // Error handler — last in chain.
+    app.use((err, _req, res, _next) => {
+      const code = (err && err.statusCode) || 500;
+      res.status(code).json({
+        success: false,
+        error: err && err.message,
+        code: err && err.code,
+      });
+    });
+
+    request = require('supertest');
+  });
+
+  afterEach(() => _cleanup(dir));
+
+  test('default sendEmail (omitted) returns link and does NOT send email', async () => {
+    const res = await request(app)
+      .post('/api/v1/auth/admin/invites')
+      .send({ email: 'friend@example.com', role: 'operator' });
+
+    expect(res.status).toBe(200);
+    expect(res.body.success).toBe(true);
+    expect(mockEmailSender.sendEmail).not.toHaveBeenCalled();
+    expect(res.body.acceptUrl).toMatch(/\/api\/v1\/auth\/invites\/[^/]+\/accept$/);
+    expect(res.body.deliveredVia).toBe('manual');
+  });
+
+  test('default sendEmail does NOT log raw token to server log', async () => {
+    const res = await request(app)
+      .post('/api/v1/auth/admin/invites')
+      .send({ email: 'friend@example.com', role: 'operator' });
+
+    const acceptUrl = res.body.acceptUrl;
+    // Extract the token from the URL and verify it does NOT appear in any log call.
+    const token = acceptUrl.match(/invites\/([^/]+)\/accept/)[1];
+    const tokenLeaked = logCalls.some(c =>
+      typeof c.msg === 'string' && c.msg.includes(token)
+    );
+    expect(tokenLeaked).toBe(false);
+
+    // Also assert no log entry mentions the URL verbatim (the old
+    // `[DC-048-DEV-INVITE-LINK] url=...` spam).
+    const oldSpam = logCalls.find(c =>
+      typeof c.msg === 'string' && c.msg.includes('[DC-048-DEV-INVITE-LINK]')
+    );
+    expect(oldSpam).toBeUndefined();
+  });
+
+  test('shareText is present and well-formed in every response', async () => {
+    const res = await request(app)
+      .post('/api/v1/auth/admin/invites')
+      .send({ email: 'friend@example.com', role: 'operator', ttlHours: 24 });
+
+    expect(res.body.shareText).toBeDefined();
+    expect(res.body.shareText).toContain('Join my DashCaddy');
+    expect(res.body.shareText).toContain('operator');
+    expect(res.body.shareText).toContain(res.body.acceptUrl);
+    expect(res.body.shareText).toContain('expires in 24h');
+  });
+
+  test('acceptUrl is always returned regardless of sendEmail', async () => {
+    const r1 = await request(app)
+      .post('/api/v1/auth/admin/invites')
+      .send({ email: 'a@x.com', sendEmail: false });
+    const r2 = await request(app)
+      .post('/api/v1/auth/admin/invites')
+      .send({ email: 'b@x.com' });
+    expect(r1.body.acceptUrl).toBeTruthy();
+    expect(r2.body.acceptUrl).toBeTruthy();
+  });
+
+  test('sendEmail: true triggers SMTP send when configured', async () => {
+    // Build a SECOND app instance where emailConfig is a real-looking object,
+    // so isConfigured() returns true. The first app uses emailConfig=null.
+    mockEmailSender.isConfigured.mockReturnValueOnce(true);
+    mockEmailSender.sendEmail.mockResolvedValueOnce(undefined);
+    const app2 = express();
+    app2.use(express.json());
+    app2.use((req, _res, next) => {
+      req.user = { id: 'admin-id', email: 'admin@sami-host.me', role: 'admin' };
+      req.app.locals = req.app.locals || {};
+      req.app.locals.siteConfig = {};
+      req.app.locals.emailConfig = { host: 'smtp.test', from: 'noreply@test' };
+      next();
+    });
+    const { createUserStore } = require('../src/security/user-store');
+    const userStore2 = createUserStore({ dataDir: dir });
+    await userStore2.login({ email: 'admin@sami-host.me' });
+    const router2 = require('../routes/auth/admin')({
+      asyncHandler: (fn) => (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next),
+      errorResponse: (_res, code, msg) => ({ status: code, msg }),
+      log: { info() {}, warn: (t, m, meta) => logCalls.push({ level: 'warn', topic: t, msg: m, meta }), error() {} },
+      session: { isSessionValid: () => true, create: () => {}, setCookie: () => {} },
+      dataDir: dir,
+    });
+    app2.use('/api/v1/auth', router2);
+
+    const res = await request(app2)
+      .post('/api/v1/auth/admin/invites')
+      .send({ email: 'friend@example.com', role: 'viewer', sendEmail: true });
+
+    expect(res.status).toBe(200);
+    expect(mockEmailSender.sendEmail).toHaveBeenCalledTimes(1);
+    const [_cfg, to, subject, text, html] = mockEmailSender.sendEmail.mock.calls[0];
+    expect(to).toBe('friend@example.com');
+    expect(subject).toMatch(/invited/i);
+    expect(text).toContain(res.body.acceptUrl);
+    expect(html).toContain(res.body.acceptUrl);
+    expect(res.body.deliveredVia).toBe('email');
+  });
+
+  test('sendEmail: true + SMTP unconfigured returns deliveredVia:failed and does NOT leak token', async () => {
+    mockEmailSender.isConfigured.mockReturnValueOnce(false);
+
+    const res = await request(app)
+      .post('/api/v1/auth/admin/invites')
+      .send({ email: 'friend@example.com', role: 'operator', sendEmail: true });
+
+    expect(res.status).toBe(200);
+    expect(mockEmailSender.sendEmail).not.toHaveBeenCalled();
+    expect(res.body.deliveredVia).toBe('failed');
+    // acceptUrl + shareText still present so the operator can share manually.
+    expect(res.body.acceptUrl).toBeTruthy();
+    expect(res.body.shareText).toBeTruthy();
+    // Token does NOT appear in any log call.
+    const token = res.body.acceptUrl.match(/invites\/([^/]+)\/accept/)[1];
+    const tokenLeaked = logCalls.some(c =>
+      typeof c.msg === 'string' && c.msg.includes(token)
+    );
+    expect(tokenLeaked).toBe(false);
+  });
+
+  test('invalid role silently defaults to operator (DC-048 behavior preserved)', async () => {
+    // DC-048: the route's `(role && VALID_ROLES.has(role)) ? role : 'operator'`
+    // silently substitutes default rather than throwing. This test pins that
+    // behavior so a future "strict role validation" change is a deliberate
+    // decision, not a silent regression.
+    const res = await request(app)
+      .post('/api/v1/auth/admin/invites')
+      .send({ email: 'a@x.com', role: 'superuser' });
+    expect(res.status).toBe(200);
+    expect(res.body.role).toBe('operator');
+    expect(mockEmailSender.sendEmail).not.toHaveBeenCalled();
+  });
+
+  test('email validation: missing email still rejected', async () => {
+    const res = await request(app)
+      .post('/api/v1/auth/admin/invites')
+      .send({ role: 'operator' });
+    expect(res.status).toBe(400);
+    expect(mockEmailSender.sendEmail).not.toHaveBeenCalled();
+  });
+
+  test('ttlHours: 1 still produces shareText with correct expiry wording', async () => {
+    const res = await request(app)
+      .post('/api/v1/auth/admin/invites')
+      .send({ email: 'a@x.com', ttlHours: 1 });
+    expect(res.body.shareText).toContain('expires in 1h');
+  });
+});
@@ -32,23 +32,6 @@ const emailSender = require('../../src/auth/providers/email-sender');
 const { ValidationError, NotFoundError, ForbiddenError, PaymentRequiredError } = require('../../src/utilities/errors');
 const { ok, successMessage } = require('../../src/utils/responses');

-/**
- * Build the URL an invitee should click. Mirrors EmailMagicLinkProvider's
- * _resolvePublicUrl logic — kept duplicated (not extracted) because the two
- * callers have slightly different link paths and the duplication is smaller
- * than the abstraction would be.
- */
-function _buildInviteUrl(req, siteConfig, token) {
-  if (siteConfig && siteConfig.publicBaseUrl) {
-    return siteConfig.publicBaseUrl.replace(/\/+$/, '') +
-      '/api/v1/auth/invites/' + encodeURIComponent(token) + '/accept';
-  }
-  const proto = (req.headers && req.headers['x-forwarded-proto']) || (req.protocol || 'https');
-  const host = (req.headers && (req.headers['x-forwarded-host'] || req.headers.host))
-    || (siteConfig && siteConfig.dashboardHost) || 'localhost:3001';
-  return `${proto}://${host}/api/v1/auth/invites/${encodeURIComponent(token)}/accept`;
-}
-
 function _requireAdmin(req, _res, next) {
  if (!req.user || req.user.role !== 'admin') {
    return next(new ForbiddenError('Admin role required'));
@@ -240,11 +223,21 @@ module.exports = function({ asyncHandler, errorResponse, log, session, dataDir }
    });
    if (!issued.ok) throw new ValidationError(issued.reason, 'email');

-    let deliveredVia = 'none';
-    const maskedEmail = email.replace(/(^.).+(@.*$)/, '$1***$2');
-    if (sendEmail !== false) {
-      // Best-effort send. If SMTP isn't configured, log to error.log (dev path).
-      const acceptUrl = _buildInviteUrl(req, /* siteConfig */ req.app.locals && req.app.locals.siteConfig, issued.token);
+    // Build the accept URL once — used both for the response and for email delivery.
+    const baseUrl = (req.app.locals && req.app.locals.siteConfig && req.app.locals.siteConfig.publicBaseUrl
+      ? req.app.locals.siteConfig.publicBaseUrl.replace(/\/+$/, '')
+      : ((req.headers['x-forwarded-proto'] || req.protocol || 'https') + '://' +
+         (req.headers['x-forwarded-host'] || req.headers.host || 'localhost:3001')));
+    const acceptUrl = baseUrl + '/api/v1/auth/invites/' + encodeURIComponent(issued.token) + '/accept';
+
+    // DC-085: link-first delivery. Default = no email, just hand the link back.
+    // Operators opt INTO email by sending { sendEmail: true } (or the admin UI
+    // checks the "Send email" checkbox). When SMTP is unconfigured AND the
+    // operator did opt in, we surface the failure as `deliveredVia: 'failed'`
+    // but NEVER leak the raw token into the server log — the link is already
+    // in the response, so the operator has a UI-side fallback.
+    let deliveredVia = 'manual';
+    if (sendEmail === true) {
      const ttlHoursOut = Math.round(issued.ttlMs / (60 * 60 * 1000));
      const text = _buildEmailText({ acceptUrl, ttlHours: ttlHoursOut, role: issued.role });
      const html = _buildEmailHtml({ acceptUrl, ttlHours: ttlHoursOut, role: issued.role });
@@ -254,33 +247,37 @@ module.exports = function({ asyncHandler, errorResponse, log, session, dataDir }
          await emailSender.sendEmail(smtpConfig, issued.email, 'You\'re invited to DashCaddy', text, html);
          deliveredVia = 'email';
        } else {
-          // Dev fallback — log the raw link so operators can grab it.
-          log.warn && log.warn('auth-invite-dev',
-            '[DC-048-DEV-INVITE-LINK] email=' + issued.email +
-            ' role=' + issued.role + ' url=' + acceptUrl);
-          deliveredVia = 'dev-console';
+          // Operator asked for email but SMTP isn't configured. Surface the
+          // failure cleanly; the link is still in the response so the
+          // operator can share it manually. Do NOT log the raw URL — it
+          // would duplicate what's already in the response and pollute the
+          // server log on every unconfigured-install invite.
+          log.warn && log.warn('auth-invite-send',
+            'invite send skipped: SMTP not configured (operator opted in)',
+            { inviteId: issued.id, email: issued.email });
+          deliveredVia = 'failed';
        }
      } catch (sendErr) {
        log.warn && log.warn('auth-invite-send',
-          'invite send failed: ' + (sendErr.message || String(sendErr)));
+          'invite send failed: ' + (sendErr.message || String(sendErr)),
+          { inviteId: issued.id });
        deliveredVia = 'failed';
      }
-    } else {
-      deliveredVia = 'manual';
    }

+    const maskedEmail = email.replace(/(^.).+(@.*$)/, '$1***$2');
+    const ttlHoursOut = Math.round(issued.ttlMs / (60 * 60 * 1000));
+    const shareText =
+      'Join my DashCaddy as ' + issued.role + ' — ' + acceptUrl +
+      ' — expires in ' + ttlHoursOut + 'h.';
+
    return ok(res, {
      id: issued.id,
      email: issued.email,
      role: issued.role,
      expiresAt: issued.expiresAt,
-      // The raw token is returned ONCE so the admin UI can show/copy the
-      // link. It is also embedded in the email when sendEmail !== false.
-      acceptUrl: (req.app.locals && req.app.locals.siteConfig && req.app.locals.siteConfig.publicBaseUrl
-        ? req.app.locals.siteConfig.publicBaseUrl.replace(/\/+$/, '')
-        : ((req.headers['x-forwarded-proto'] || req.protocol || 'https') + '://' +
-           (req.headers['x-forwarded-host'] || req.headers.host || 'localhost:3001'))) +
-        '/api/v1/auth/invites/' + encodeURIComponent(issued.token) + '/accept',
+      acceptUrl,
+      shareText,
      deliveredVia,
      maskedEmail,
    });