mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-07-26 11:49:16 +00:00
fix: enforce the signed-in user's role on cross-node proxied requests (#1505)
Proxied requests authenticated to a remote node previously ran as admin regardless of the originating user's role, so a non-admin using the UI against a remote node could reach admin-only handlers there. The forwarding primary now asserts the user's role on a trusted header that the remote honors only for node_proxy/pilot_tunnel bearers (the same trust model as the license tier header), and the gateway overwrites the header on every proxied request so a client cannot smuggle it. An absent header keeps admin for direct instance-to-instance and background service calls; an unrecognized role fails closed to read-only.
This commit is contained in:
@@ -0,0 +1,217 @@
|
||||
/**
|
||||
* Tests for cross-node RBAC preservation: the forwarding primary asserts the
|
||||
* signed-in user's role on PROXY_ROLE_HEADER, and the remote's authMiddleware
|
||||
* honors it for node_proxy / pilot_tunnel bearers instead of granting every
|
||||
* proxied request blanket admin.
|
||||
*
|
||||
* The probe route is `GET /api/users` (admin-only, no Docker dependency). A
|
||||
* forwarded non-admin role must be denied there; an absent header (a direct
|
||||
* instance-to-instance / background service call) must keep admin.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import http from 'http';
|
||||
import bcrypt from 'bcrypt';
|
||||
import request from 'supertest';
|
||||
import jwt from 'jsonwebtoken';
|
||||
import {
|
||||
setupTestDb,
|
||||
cleanupTestDb,
|
||||
loginAsTestAdmin,
|
||||
TEST_JWT_SECRET,
|
||||
} from './helpers/setupTestDb';
|
||||
import { PROXY_ROLE_HEADER } from '../services/license-headers';
|
||||
|
||||
let tmpDir: string;
|
||||
let app: import('express').Express;
|
||||
let viewerBearer: string;
|
||||
|
||||
const VIEWER_USER = 'proxy-role-viewer';
|
||||
|
||||
beforeAll(async () => {
|
||||
tmpDir = await setupTestDb();
|
||||
({ app } = await import('../index'));
|
||||
const { DatabaseService } = await import('../services/DatabaseService');
|
||||
const db = DatabaseService.getInstance();
|
||||
const hash = await bcrypt.hash('password123', 1);
|
||||
db.addUser({ username: VIEWER_USER, password_hash: hash, role: 'viewer' });
|
||||
const viewer = db.getUserByUsername(VIEWER_USER)!;
|
||||
viewerBearer = jwt.sign(
|
||||
{ username: VIEWER_USER, role: 'viewer', tv: viewer.token_version },
|
||||
TEST_JWT_SECRET,
|
||||
{ expiresIn: '1m' },
|
||||
);
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
cleanupTestDb(tmpDir);
|
||||
});
|
||||
|
||||
const signToken = (payload: Record<string, unknown>, expiresIn: string | number = '1m') =>
|
||||
jwt.sign(payload, TEST_JWT_SECRET, { expiresIn: expiresIn as jwt.SignOptions['expiresIn'] });
|
||||
|
||||
const ADMIN_ONLY = '/api/users';
|
||||
// Docker-free, gated by requirePermission('stack:read'); every role carries it,
|
||||
// so a 200 here proves a forwarded non-admin retained its legitimate access.
|
||||
const READ_ONLY = '/api/stacks';
|
||||
|
||||
describe('authMiddleware - forwarded actor role (node_proxy)', () => {
|
||||
it('keeps admin when no role header is present (direct service call)', async () => {
|
||||
const token = signToken({ scope: 'node_proxy' });
|
||||
const res = await request(app).get(ADMIN_ONLY).set('Authorization', `Bearer ${token}`);
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
|
||||
it('keeps admin when the forwarded role is admin', async () => {
|
||||
const token = signToken({ scope: 'node_proxy' });
|
||||
const res = await request(app)
|
||||
.get(ADMIN_ONLY)
|
||||
.set('Authorization', `Bearer ${token}`)
|
||||
.set(PROXY_ROLE_HEADER, 'admin');
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
|
||||
it('denies an admin-only route when the forwarded role is viewer', async () => {
|
||||
const token = signToken({ scope: 'node_proxy' });
|
||||
const res = await request(app)
|
||||
.get(ADMIN_ONLY)
|
||||
.set('Authorization', `Bearer ${token}`)
|
||||
.set(PROXY_ROLE_HEADER, 'viewer');
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
it('denies an admin-only route when the forwarded role is deployer', async () => {
|
||||
const token = signToken({ scope: 'node_proxy' });
|
||||
const res = await request(app)
|
||||
.get(ADMIN_ONLY)
|
||||
.set('Authorization', `Bearer ${token}`)
|
||||
.set(PROXY_ROLE_HEADER, 'deployer');
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
it('fails closed to read-only for an unrecognized forwarded role', async () => {
|
||||
const token = signToken({ scope: 'node_proxy' });
|
||||
const res = await request(app)
|
||||
.get(ADMIN_ONLY)
|
||||
.set('Authorization', `Bearer ${token}`)
|
||||
.set(PROXY_ROLE_HEADER, 'superadmin');
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
it('applies the same trust to pilot_tunnel bearers', async () => {
|
||||
const token = signToken({ scope: 'pilot_tunnel' });
|
||||
const res = await request(app)
|
||||
.get(ADMIN_ONLY)
|
||||
.set('Authorization', `Bearer ${token}`)
|
||||
.set(PROXY_ROLE_HEADER, 'viewer');
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
it('still grants a forwarded viewer its legitimate read access', async () => {
|
||||
// The fail-closed branch must not over-restrict: a forwarded viewer keeps
|
||||
// stack:read, so this is a 200 rather than a blanket denial.
|
||||
const token = signToken({ scope: 'node_proxy' });
|
||||
const res = await request(app)
|
||||
.get(READ_ONLY)
|
||||
.set('Authorization', `Bearer ${token}`)
|
||||
.set(PROXY_ROLE_HEADER, 'viewer');
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
|
||||
it('treats an unrecognized forwarded role as read-only, not zero access', async () => {
|
||||
// "Fails closed to read-only" means viewer-equivalent: denied on admin
|
||||
// routes (asserted above) but still allowed on stack:read reads.
|
||||
const token = signToken({ scope: 'node_proxy' });
|
||||
const res = await request(app)
|
||||
.get(READ_ONLY)
|
||||
.set('Authorization', `Bearer ${token}`)
|
||||
.set(PROXY_ROLE_HEADER, 'superadmin');
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Security - actor-role header cannot be smuggled by a user session', () => {
|
||||
it('ignores the role header on a cookie session (uses the DB role)', async () => {
|
||||
const cookie = await loginAsTestAdmin(app);
|
||||
// A browser session that tries to downgrade itself (or, by the same path,
|
||||
// elevate) via the header must be unaffected: the header is honored only on
|
||||
// node_proxy / pilot_tunnel bearers, and the gateway overwrites it anyway.
|
||||
const res = await request(app)
|
||||
.get(ADMIN_ONLY)
|
||||
.set('Cookie', cookie)
|
||||
.set(PROXY_ROLE_HEADER, 'viewer');
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
|
||||
it('rejects an unauthenticated request even with a role header set', async () => {
|
||||
const res = await request(app).get(ADMIN_ONLY).set(PROXY_ROLE_HEADER, 'admin');
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
|
||||
it('does not let a non-admin session elevate by setting the role header', async () => {
|
||||
// The attack the header could enable: a viewer setting x-sencho-actor-role:
|
||||
// admin on their own session. The user-session branch never reads the
|
||||
// header (only node_proxy/pilot_tunnel does), so the DB role wins -> 403.
|
||||
const res = await request(app)
|
||||
.get(ADMIN_ONLY)
|
||||
.set('Authorization', `Bearer ${viewerBearer}`)
|
||||
.set(PROXY_ROLE_HEADER, 'admin');
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
});
|
||||
|
||||
// The gateway half of the fix: the forwarding primary must overwrite the actor
|
||||
// role header from the authenticated session before forwarding, so a client
|
||||
// cannot smuggle an elevated role through to the remote. Exercised end-to-end
|
||||
// by routing a real proxied request to a loopback capture server and asserting
|
||||
// the header it received.
|
||||
describe('remote proxy gateway - actor role header forwarding', () => {
|
||||
let captured: http.IncomingHttpHeaders | null = null;
|
||||
let captureServer: http.Server;
|
||||
let remoteNodeId: number;
|
||||
|
||||
beforeAll(async () => {
|
||||
captureServer = http.createServer((req, res) => {
|
||||
captured = req.headers;
|
||||
res.writeHead(200, { 'Content-Type': 'application/json' });
|
||||
res.end('[]');
|
||||
});
|
||||
await new Promise<void>((resolve) => captureServer.listen(0, '127.0.0.1', resolve));
|
||||
const port = (captureServer.address() as import('net').AddressInfo).port;
|
||||
|
||||
const { DatabaseService } = await import('../services/DatabaseService');
|
||||
remoteNodeId = DatabaseService.getInstance().addNode({
|
||||
name: 'capture-remote',
|
||||
type: 'remote',
|
||||
mode: 'proxy',
|
||||
compose_dir: '/tmp',
|
||||
is_default: false,
|
||||
api_url: `http://127.0.0.1:${port}`,
|
||||
api_token: 'capture-node-token',
|
||||
});
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await new Promise<void>((resolve) => captureServer.close(() => resolve()));
|
||||
});
|
||||
|
||||
it('overwrites a smuggled actor-role header with the session role', async () => {
|
||||
captured = null;
|
||||
const res = await request(app)
|
||||
.get(READ_ONLY)
|
||||
.set('Authorization', `Bearer ${viewerBearer}`)
|
||||
.set('x-node-id', String(remoteNodeId))
|
||||
.set(PROXY_ROLE_HEADER, 'admin'); // smuggled
|
||||
expect(res.status).toBe(200);
|
||||
expect(captured?.[PROXY_ROLE_HEADER]).toBe('viewer');
|
||||
});
|
||||
|
||||
it('forwards the session role when no header is supplied', async () => {
|
||||
captured = null;
|
||||
const res = await request(app)
|
||||
.get(READ_ONLY)
|
||||
.set('Authorization', `Bearer ${viewerBearer}`)
|
||||
.set('x-node-id', String(remoteNodeId));
|
||||
expect(res.status).toBe(200);
|
||||
expect(captured?.[PROXY_ROLE_HEADER]).toBe('viewer');
|
||||
});
|
||||
});
|
||||
@@ -3,11 +3,12 @@ import jwt from 'jsonwebtoken';
|
||||
import {
|
||||
DatabaseService,
|
||||
API_TOKEN_SCOPE_TO_ROLE,
|
||||
isUserRole,
|
||||
type UserRole,
|
||||
type ApiTokenScope,
|
||||
} from '../services/DatabaseService';
|
||||
import { getErrorMessage } from '../utils/errors';
|
||||
import { PROXY_TIER_HEADER } from '../services/license-headers';
|
||||
import { PROXY_TIER_HEADER, PROXY_ROLE_HEADER } from '../services/license-headers';
|
||||
import { isLicenseTier, normalizeTier } from '../services/license-normalize';
|
||||
import { isDebugEnabled } from '../utils/debug';
|
||||
import {
|
||||
@@ -105,7 +106,21 @@ export const authMiddleware: RequestHandler = async (req: Request, res: Response
|
||||
// after the primary itself re-signed/trusted them. Same tier-header trust
|
||||
// rules apply.
|
||||
if (decoded.scope === 'node_proxy' || decoded.scope === 'pilot_tunnel') {
|
||||
req.user = { username: 'node-proxy', role: 'admin', userId: 0 };
|
||||
// Preserve the originating user's RBAC across the proxy. The forwarding
|
||||
// primary asserts the signed-in user's role on PROXY_ROLE_HEADER, trusted
|
||||
// under the same rule as the tier header (only a valid node_proxy /
|
||||
// pilot_tunnel bearer reaches this branch). Honor that role so a non-admin
|
||||
// proxied here is bound to their own permissions instead of inheriting
|
||||
// blanket admin. An absent header is a direct instance-to-instance or
|
||||
// background service call (fleet orchestration, sync, monitor) that
|
||||
// legitimately runs as admin; a present-but-unrecognized role fails closed
|
||||
// to read-only rather than admin.
|
||||
const forwardedRole = req.headers[PROXY_ROLE_HEADER] as string | undefined;
|
||||
let role: UserRole = 'admin';
|
||||
if (forwardedRole !== undefined) {
|
||||
role = isUserRole(forwardedRole) ? forwardedRole : 'viewer';
|
||||
}
|
||||
req.user = { username: 'node-proxy', role, userId: 0 };
|
||||
|
||||
// Distributed License Enforcement: trust tier headers only from authenticated node proxy requests.
|
||||
// Browser sessions and API tokens cannot set these; only a valid node_proxy JWT (signed with
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import type { Request, Response, NextFunction, RequestHandler } from 'express';
|
||||
import { createProxyMiddleware } from 'http-proxy-middleware';
|
||||
import { NodeRegistry } from '../services/NodeRegistry';
|
||||
import { PROXY_TIER_HEADER } from '../services/license-headers';
|
||||
import { PROXY_TIER_HEADER, PROXY_ROLE_HEADER } from '../services/license-headers';
|
||||
import { LicenseService } from '../services/LicenseService';
|
||||
import { isProxyExemptPath } from '../helpers/proxyExemptPaths';
|
||||
import { getErrorMessage } from '../utils/errors';
|
||||
@@ -49,6 +49,15 @@ export function createRemoteProxyMiddleware(): RequestHandler {
|
||||
// state changes within one proxy call.
|
||||
const headers = LicenseService.getInstance().getProxyHeaders();
|
||||
proxyReq.setHeader(PROXY_TIER_HEADER, headers.tier);
|
||||
// Forward the signed-in user's role so the remote enforces their RBAC
|
||||
// rather than treating every proxied request as admin. Strip first so a
|
||||
// browser/API client cannot smuggle the header through the gateway, then
|
||||
// re-set from the authenticated session (authGate runs before this proxy,
|
||||
// so req.user is always resolved here).
|
||||
proxyReq.removeHeader(PROXY_ROLE_HEADER);
|
||||
if (req.user?.role) {
|
||||
proxyReq.setHeader(PROXY_ROLE_HEADER, req.user.role);
|
||||
}
|
||||
// Strip the ?nodeId= query param so the remote's nodeContextMiddleware
|
||||
// doesn't reject the request with 404 ("Node X not found") - the remote
|
||||
// has no record of the gateway's node IDs and should treat the request
|
||||
|
||||
@@ -304,7 +304,14 @@ export interface WebhookExecution {
|
||||
|
||||
export type AuthProvider = 'local' | 'ldap' | 'oidc_google' | 'oidc_github' | 'oidc_okta' | 'oidc_custom';
|
||||
|
||||
export type UserRole = 'admin' | 'viewer' | 'deployer' | 'node-admin' | 'auditor';
|
||||
// Source of truth for the role set. UserRole derives from it so the union and
|
||||
// the runtime list cannot drift (adding a role here updates both).
|
||||
export const USER_ROLES = ['admin', 'viewer', 'deployer', 'node-admin', 'auditor'] as const;
|
||||
export type UserRole = typeof USER_ROLES[number];
|
||||
/** Narrow an untrusted string (e.g. a proxied role header) to a known UserRole. */
|
||||
export function isUserRole(value: unknown): value is UserRole {
|
||||
return typeof value === 'string' && (USER_ROLES as readonly string[]).includes(value);
|
||||
}
|
||||
export type ResourceType = 'stack' | 'node';
|
||||
|
||||
export interface User {
|
||||
|
||||
@@ -6,3 +6,13 @@
|
||||
* authenticated as a node_proxy bearer.
|
||||
*/
|
||||
export const PROXY_TIER_HEADER = 'x-sencho-tier';
|
||||
|
||||
/**
|
||||
* Carries the signed-in user's role from the forwarding primary to the remote
|
||||
* node, so the remote enforces that user's RBAC instead of treating every
|
||||
* proxied request as admin. Trusted under the same rule as PROXY_TIER_HEADER:
|
||||
* only a request authenticated as a node_proxy/pilot_tunnel bearer may set it,
|
||||
* and the gateway overwrites it on every proxied request so a browser or API
|
||||
* client cannot smuggle a role through.
|
||||
*/
|
||||
export const PROXY_ROLE_HEADER = 'x-sencho-actor-role';
|
||||
|
||||
Reference in New Issue
Block a user