Skip to content

feat: per-user MCP auth + complete moderation audit trail (#445)#446

Merged
fatherlinux merged 2 commits into
masterfrom
feature/445-mcp-user-auth
May 29, 2026
Merged

feat: per-user MCP auth + complete moderation audit trail (#445)#446
fatherlinux merged 2 commits into
masterfrom
feature/445-mcp-user-auth

Conversation

@fatherlinux
Copy link
Copy Markdown
Member

Summary

  • Replace shared MCP_ADMIN_TOKEN with per-user tokens in the users table — MCP actions are now attributed to the real user
  • URL-embedded auth (/mcp/<token>) like Postiz — no Bearer header needed
  • MCP handler mounted on Express main port with body parser bypass
  • Settings UI: MCP tab with connection URL, reveal/hide, copy, and rotate key
  • Fix 10 audit trail gaps across createItem, admin create, media soft-delete, primary image inserts, and deny-list sweep
  • Auto-publish/reject paths set moderated_by = -1 (Auto-Publisher system user)
  • Only pending items have moderated_by = NULL

Closes #445

Test plan

  • ./run.sh build passes
  • Tests pass (3 pre-existing flaky UI failures)
  • MCP auth: URL token resolves to real user, tools work, rejection sets correct moderated_by
  • Invalid token returns 401
  • Exhaustive audit: every publish/reject path has moderated_by
  • Run migration 069 on production
  • Verify MCP tab in Settings UI on production
  • Configure production MCP to use new URL format

🤖 Generated with Claude Code

Replace shared MCP_ADMIN_TOKEN with per-user tokens stored in the
users table. MCP server authenticates via URL-embedded token
(e.g. /mcp/<token>) and resolves to the real user ID. All MCP
actions are now attributed to the authenticated user.

- Add mcp_token column to users table (migration 069)
- MCP auth: URL path token + Bearer header fallback + env var bootstrap
- Mount MCP handler on Express main port (body parser bypass)
- Settings UI: MCP tab with connection URL, reveal/copy/rotate
- Fix 10 audit trail gaps: createItem, admin create, media ops, deny-list
- Auto-publish/reject sets moderated_by to Auto-Publisher (-1)
- Only pending items have moderated_by=NULL

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces per-user Model Context Protocol (MCP) authentication tokens, allowing users to connect AI assistants to the application. It adds database migrations, backend routes, and middleware to support MCP token generation, regeneration, and request handling, while also updating the frontend with an MCP settings tab. Feedback on these changes highlights several critical issues: handleMcpRequest lacks error handling and robust buffer length checks for crypto.timingSafeEqual, which could crash the server; the /mcp/:token route is registered too late in server.js and will be intercepted by the wildcard handler; and the frontend McpSettings component needs better error handling for token regeneration and safe clipboard access in non-secure contexts.

Comment on lines +791 to +837
async function handleMcpRequest(req, res, pool, boss) {
const urlMatch = req.url.match(/^\/mcp\/([A-Za-z0-9_-]+)$/);
const urlToken = urlMatch ? urlMatch[1] : null;
const authHeader = req.headers.authorization;
const bearerToken = authHeader?.startsWith('Bearer ') ? authHeader.slice(7) : null;
const token = urlToken || bearerToken;

if (!token) {
res.writeHead(401, { 'Content-Type': 'application/json' });
res.end(JSON.stringify({ error: 'Unauthorized' }));
return;
}

let mcpUserId = null;

const userResult = await pool.query(
'SELECT id, is_admin FROM users WHERE mcp_token = $1', [token]
);
if (userResult.rows.length > 0) {
mcpUserId = userResult.rows[0].id;
} else {
const envToken = process.env.MCP_ADMIN_TOKEN;
if (envToken && token.length === envToken.length &&
crypto.timingSafeEqual(Buffer.from(token), Buffer.from(envToken))) {
const adminResult = await pool.query(
'SELECT id FROM users WHERE is_admin = TRUE ORDER BY id LIMIT 1'
);
mcpUserId = adminResult.rows.length > 0 ? adminResult.rows[0].id : null;
}
}

if (!mcpUserId) {
res.writeHead(401, { 'Content-Type': 'application/json' });
res.end(JSON.stringify({ error: 'Unauthorized' }));
return;
}

const server = new McpServer({ name: 'rotv-admin', version: '1.0.0' });
registerTools(server, pool, boss, mcpUserId);

const transport = new StreamableHTTPServerTransport({
sessionIdGenerator: undefined
});

await server.connect(transport);
await transport.handleRequest(req, res);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-critical critical

The handleMcpRequest function lacks error handling and can throw unhandled exceptions (e.g., from database queries, MCP server connection, or transport handling), which can crash the Node.js process or hang the connection. Additionally, crypto.timingSafeEqual throws a TypeError if the compared buffers have different byte lengths. Comparing string lengths (token.length === envToken.length) is insufficient if multi-byte characters are present.

Wrapping the function body in a try...catch block and comparing the byte lengths of the buffers before calling crypto.timingSafeEqual ensures robustness and security.

async function handleMcpRequest(req, res, pool, boss) {
  try {
    const urlMatch = req.url.match(/^\/mcp\/([A-Za-z0-9_-]+)$/);
    const urlToken = urlMatch ? urlMatch[1] : null;
    const authHeader = req.headers.authorization;
    const bearerToken = authHeader?.startsWith('Bearer ') ? authHeader.slice(7) : null;
    const token = urlToken || bearerToken;

    if (!token) {
      res.writeHead(401, { 'Content-Type': 'application/json' });
      res.end(JSON.stringify({ error: 'Unauthorized' }));
      return;
    }

    let mcpUserId = null;

    const userResult = await pool.query(
      'SELECT id, is_admin FROM users WHERE mcp_token = $1', [token]
    );
    if (userResult.rows.length > 0) {
      mcpUserId = userResult.rows[0].id;
    } else {
      const envToken = process.env.MCP_ADMIN_TOKEN;
      if (envToken) {
        const tokenBuf = Buffer.from(token);
        const envTokenBuf = Buffer.from(envToken);
        if (tokenBuf.length === envTokenBuf.length && crypto.timingSafeEqual(tokenBuf, envTokenBuf)) {
          const adminResult = await pool.query(
            'SELECT id FROM users WHERE is_admin = TRUE ORDER BY id LIMIT 1'
          );
          mcpUserId = adminResult.rows.length > 0 ? adminResult.rows[0].id : null;
        }
      }
    }

    if (!mcpUserId) {
      res.writeHead(401, { 'Content-Type': 'application/json' });
      res.end(JSON.stringify({ error: 'Unauthorized' }));
      return;
    }

    const server = new McpServer({ name: 'rotv-admin', version: '1.0.0' });
    registerTools(server, pool, boss, mcpUserId);

    const transport = new StreamableHTTPServerTransport({
      sessionIdGenerator: undefined
    });

    await server.connect(transport);
    await transport.handleRequest(req, res);
  } catch (err) {
    console.error('MCP request handling failed:', err);
    if (!res.headersSent) {
      res.writeHead(500, { 'Content-Type': 'application/json' });
      res.end(JSON.stringify({ error: 'Internal server error' }));
    }
  }
}

Comment thread backend/server.js Outdated
Comment on lines +2966 to +2967
startMcpServer(pool, app.get('boss'), parseInt(process.env.MCP_PORT || '3001'));
app.all('/mcp/:token', mcpMiddleware(pool, app.get('boss')));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Registering the /mcp/:token route handler inside the start() function places it after the wildcard app.get('*') handler (which is registered at the top level during module load). In Express, routes are matched in the order of registration, meaning any GET request to /mcp/:token will be intercepted by app.get('*') and serve index.html instead of reaching the MCP middleware.

To fix this, register the route handler at the top level (before app.get('*')) and dynamically resolve the boss instance from req.app.get('boss').

Suggested change
startMcpServer(pool, app.get('boss'), parseInt(process.env.MCP_PORT || '3001'));
app.all('/mcp/:token', mcpMiddleware(pool, app.get('boss')));
startMcpServer(pool, app.get('boss'), parseInt(process.env.MCP_PORT || '3001'));

Comment on lines +18 to +22
const handleCopy = () => {
navigator.clipboard.writeText(mcpUrl);
setCopied(true);
setTimeout(() => setCopied(false), 2000);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In non-secure contexts (e.g., local development over HTTP in some browsers), navigator.clipboard is undefined. Directly calling navigator.clipboard.writeText will throw a TypeError and crash the component. Additionally, writeText returns a Promise that should have its rejection handled to avoid unhandled promise rejections.

  const handleCopy = () => {
    if (navigator.clipboard) {
      navigator.clipboard.writeText(mcpUrl)
        .then(() => {
          setCopied(true);
          setTimeout(() => setCopied(false), 2000);
        })
        .catch(err => {
          console.error('Failed to copy token:', err);
        });
    } else {
      console.warn('Clipboard API not available');
    }
  };

Comment on lines +24 to +39
const handleRegenerate = async () => {
if (!confirm('Rotate your MCP key? Your current URL will stop working immediately.')) return;
setRegenerating(true);
try {
const res = await fetch('/api/user/settings/mcp-token/regenerate', {
method: 'POST', credentials: 'include'
});
if (res.ok) {
const data = await res.json();
setToken(data.token);
setRevealed(false);
}
} finally {
setRegenerating(false);
}
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The handleRegenerate function performs an asynchronous fetch request but does not catch potential network errors or handle non-ok HTTP responses. This can lead to unhandled promise rejections and a poor user experience if the request fails.

  const handleRegenerate = async () => {
    if (!confirm('Rotate your MCP key? Your current URL will stop working immediately.')) return;
    setRegenerating(true);
    try {
      const res = await fetch('/api/user/settings/mcp-token/regenerate', {
        method: 'POST', credentials: 'include'
      });
      if (res.ok) {
        const data = await res.json();
        setToken(data.token);
        setRevealed(false);
      } else {
        alert('Failed to rotate key. Please try again.');
      }
    } catch (err) {
      console.error('Error rotating key:', err);
      alert('A network error occurred. Please try again.');
    } finally {
      setRegenerating(false);
    }
  };

… user (PR #446 review)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@fatherlinux fatherlinux merged commit 9d5e3d0 into master May 29, 2026
2 of 3 checks passed
@fatherlinux fatherlinux deleted the feature/445-mcp-user-auth branch May 29, 2026 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Per-user MCP auth + complete moderation audit trail

1 participant