mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-10 10:49:35 +00:00
fix(api-tokens): harden rate limiting and surface list-load errors (#1292)
* fix(api-tokens): scope per-token rate limits to live tokens Forged or token-shaped Authorization headers no longer mint their own rate-limit budget. The key generator now grants a per-token budget only to a real, active token and falls back to per-IP keying for anything else, so a single source cannot evade the global limiter by rotating fake tokens. The validated token is memoized on the request, so authentication reuses it without a second database lookup. Token validation (format, checksum, lookup, revocation, expiry) is now a single shared helper used by the HTTP auth middleware, the WebSocket upgrade handler, and the rate-limit key generator, replacing two near-identical inline copies that could drift apart. The last-used timestamp write is throttled so a busy token no longer writes to the database on every request. * fix(api-tokens): surface token list-load failures with a retry A failed load of the API tokens list was swallowed: a server error rendered the empty "no tokens yet" state with no sign that anything went wrong. The list now shows an error card with a Retry action and raises a toast on any non-ok response or network error, matching the create and revoke flows. Adds a troubleshooting entry for the error. * test(api-tokens): seed tokens via the shared test helper The new hardening and WS-scope suites computed sha256 of a raw token directly, which CodeQL flags as js/insufficient-password-hash (a false positive: these are 256-bit CSPRNG opaque tokens, not passwords). Route token creation through the existing apiTokenTestHelper and read the stored token_hash back from the row, so the suites no longer hash anything themselves. Also removes the duplicated createToken helpers. * fix(api-tokens): key the rate limiter by the same credential auth uses The rate-limit key generator checked the session cookie before the Authorization bearer, while authMiddleware authenticates bearer-over-cookie (bearerToken || cookieToken). A request could send a Bearer API token plus a forged cookie and be keyed by the cookie's (forgeable, rotatable) username, sidestepping the per-token / per-IP keying the limiter applies to API tokens: a valid token would lose its own bucket, and a forged token-shaped bearer would no longer collapse to per-IP. Reorder the generator to mirror auth: process the bearer first (validate the API token and key per-token or fall back to per-IP; otherwise decode the JWT by username/sub), and consult the cookie only when there is no bearer. Regression tests cover a valid and a forged sen_sk_ bearer, each sent with a forged cookie.
This commit is contained in:
Anso
@@ -0,0 +1,220 @@
|
||||
/**
|
||||
* Tests for the API-token hardening pass:
|
||||
* - validateApiToken: shared format/checksum/lookup/revocation/expiry result.
|
||||
* - touchApiTokenLastUsed: throttled last-used write.
|
||||
* - rateLimitKeyGenerator: per-token budget only for live tokens; forged,
|
||||
* bad-checksum, revoked, or expired token-shaped bearers fall back to
|
||||
* per-IP keying so they cannot fragment the limiter (H-1).
|
||||
*
|
||||
* Token rows are seeded through the shared apiTokenTestHelper and their stored
|
||||
* hash is read back from the row, so this suite hashes nothing itself.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll, afterEach, vi } from 'vitest';
|
||||
import jwt from 'jsonwebtoken';
|
||||
import type { Request } from 'express';
|
||||
import type { ApiToken } from '../services/DatabaseService';
|
||||
import { setupTestDb, cleanupTestDb, TEST_JWT_SECRET } from './helpers/setupTestDb';
|
||||
import { COOKIE_NAME } from '../helpers/constants';
|
||||
import { createTestApiToken, unbackedApiToken } from './helpers/apiTokenTestHelper';
|
||||
import { validateApiToken, touchApiTokenLastUsed } from '../utils/apiTokenAuth';
|
||||
import { rateLimitKeyGenerator } from '../middleware/rateLimiters';
|
||||
|
||||
let tmpDir: string;
|
||||
let DatabaseService: typeof import('../services/DatabaseService').DatabaseService;
|
||||
|
||||
/** Seed an API token via the shared helper and return the raw value plus its stored row. */
|
||||
function createToken(
|
||||
scope: 'read-only' | 'deploy-only' | 'full-admin',
|
||||
opts: { expiresAt?: number | null; revoked?: boolean } = {},
|
||||
): { raw: string; row: ApiToken } {
|
||||
const db = DatabaseService.getInstance();
|
||||
const userId = db.getUserByUsername('testadmin')!.id;
|
||||
const name = `hardening-${scope}-${Date.now()}-${Math.random().toString(36).slice(2)}`;
|
||||
const raw = createTestApiToken({ db: DatabaseService, scope, userId, name, expiresAt: opts.expiresAt ?? null });
|
||||
const row = db.getActiveApiTokenByNameAndUser(name, userId)!;
|
||||
if (opts.revoked) db.revokeApiToken(row.id);
|
||||
return { raw, row };
|
||||
}
|
||||
|
||||
/** Minimal Express request carrying a Bearer token, for the key generator. */
|
||||
function bearerReq(token: string): Request {
|
||||
return { cookies: {}, headers: { authorization: `Bearer ${token}` }, ip: '203.0.113.7' } as unknown as Request;
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
tmpDir = await setupTestDb();
|
||||
({ DatabaseService } = await import('../services/DatabaseService'));
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
cleanupTestDb(tmpDir);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe('validateApiToken', () => {
|
||||
it('accepts a live token', () => {
|
||||
const { raw } = createToken('read-only');
|
||||
const result = validateApiToken(raw);
|
||||
expect(result.ok).toBe(true);
|
||||
if (result.ok) expect(result.token.scope).toBe('read-only');
|
||||
});
|
||||
|
||||
it('rejects a non-token string', () => {
|
||||
const result = validateApiToken('not-a-sencho-token');
|
||||
expect(result).toEqual({ ok: false, reason: 'not-api-token' });
|
||||
});
|
||||
|
||||
it('rejects a token-shaped string with a bad checksum', () => {
|
||||
const result = validateApiToken('sen_sk_' + 'A'.repeat(49));
|
||||
expect(result).toEqual({ ok: false, reason: 'checksum' });
|
||||
});
|
||||
|
||||
it('rejects a well-formed token that is not in the database', () => {
|
||||
const result = validateApiToken(unbackedApiToken());
|
||||
expect(result).toEqual({ ok: false, reason: 'not-found' });
|
||||
});
|
||||
|
||||
it('rejects a revoked token', () => {
|
||||
const { raw } = createToken('full-admin', { revoked: true });
|
||||
const result = validateApiToken(raw);
|
||||
expect(result).toEqual({ ok: false, reason: 'revoked' });
|
||||
});
|
||||
|
||||
it('rejects an expired token', () => {
|
||||
const { raw } = createToken('deploy-only', { expiresAt: Date.now() - 1000 });
|
||||
const result = validateApiToken(raw);
|
||||
expect(result).toEqual({ ok: false, reason: 'expired' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('touchApiTokenLastUsed', () => {
|
||||
it('writes when last_used_at is null', () => {
|
||||
const { row } = createToken('read-only');
|
||||
const spy = vi.spyOn(DatabaseService.getInstance(), 'updateApiTokenLastUsed');
|
||||
touchApiTokenLastUsed({ ...row, last_used_at: null });
|
||||
expect(spy).toHaveBeenCalledWith(row.id);
|
||||
});
|
||||
|
||||
it('writes when last_used_at is stale (older than the throttle window)', () => {
|
||||
const { row } = createToken('read-only');
|
||||
const spy = vi.spyOn(DatabaseService.getInstance(), 'updateApiTokenLastUsed');
|
||||
touchApiTokenLastUsed({ ...row, last_used_at: Date.now() - 70_000 });
|
||||
expect(spy).toHaveBeenCalledWith(row.id);
|
||||
});
|
||||
|
||||
it('skips the write when last_used_at is within the throttle window', () => {
|
||||
const { row } = createToken('read-only');
|
||||
const spy = vi.spyOn(DatabaseService.getInstance(), 'updateApiTokenLastUsed');
|
||||
touchApiTokenLastUsed({ ...row, last_used_at: Date.now() - 1000 });
|
||||
expect(spy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('rateLimitKeyGenerator (API token branch)', () => {
|
||||
it('keys a live token by its own hash and memoizes the row for auth reuse', () => {
|
||||
const { raw, row } = createToken('read-only');
|
||||
const spy = vi.spyOn(DatabaseService.getInstance(), 'getApiTokenByHash');
|
||||
const req = bearerReq(raw);
|
||||
expect(rateLimitKeyGenerator(req)).toBe(`user:sk:${row.token_hash.slice(0, 16)}`);
|
||||
expect(req._apiToken?.token_hash).toBe(row.token_hash);
|
||||
expect(spy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('reuses the memoized row on a second pass without another lookup', () => {
|
||||
const { raw, row } = createToken('read-only');
|
||||
const req = bearerReq(raw);
|
||||
rateLimitKeyGenerator(req);
|
||||
const spy = vi.spyOn(DatabaseService.getInstance(), 'getApiTokenByHash');
|
||||
expect(rateLimitKeyGenerator(req)).toBe(`user:sk:${row.token_hash.slice(0, 16)}`);
|
||||
expect(spy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('falls back to per-IP keying for a bad-checksum token without a DB lookup', () => {
|
||||
const spy = vi.spyOn(DatabaseService.getInstance(), 'getApiTokenByHash');
|
||||
const req = bearerReq('sen_sk_' + 'A'.repeat(49));
|
||||
const key = rateLimitKeyGenerator(req);
|
||||
expect(key).not.toMatch(/^user:sk:/);
|
||||
expect(key).toContain('203.0.113.7');
|
||||
expect(req._apiToken).toBeUndefined();
|
||||
expect(spy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('falls back to per-IP keying for a well-formed but unknown token', () => {
|
||||
const req = bearerReq(unbackedApiToken());
|
||||
const key = rateLimitKeyGenerator(req);
|
||||
expect(key).not.toMatch(/^user:sk:/);
|
||||
expect(key).toContain('203.0.113.7');
|
||||
expect(req._apiToken).toBeUndefined();
|
||||
});
|
||||
|
||||
it('falls back to per-IP keying for a revoked token', () => {
|
||||
const { raw } = createToken('full-admin', { revoked: true });
|
||||
const key = rateLimitKeyGenerator(bearerReq(raw));
|
||||
expect(key).not.toMatch(/^user:sk:/);
|
||||
expect(key).toContain('203.0.113.7');
|
||||
});
|
||||
|
||||
it('falls back to per-IP keying for an expired token', () => {
|
||||
const { raw } = createToken('deploy-only', { expiresAt: Date.now() - 1000 });
|
||||
const key = rateLimitKeyGenerator(bearerReq(raw));
|
||||
expect(key).not.toMatch(/^user:sk:/);
|
||||
expect(key).toContain('203.0.113.7');
|
||||
});
|
||||
|
||||
it('collapses distinct forged tokens from one IP into the same anonymous bucket (no fragmentation)', () => {
|
||||
// The H-1 property: a single source cannot mint a fresh per-token budget by
|
||||
// rotating forged token-shaped bearers. Two different well-formed tokens
|
||||
// that are not in the DB, from one IP, must share one key, and that key must
|
||||
// be the same per-IP bucket an unauthenticated request from that IP gets.
|
||||
const ip = '198.51.100.42';
|
||||
const forged = (token: string) =>
|
||||
({ cookies: {}, headers: { authorization: `Bearer ${token}` }, ip } as unknown as Request);
|
||||
const keyA = rateLimitKeyGenerator(forged(unbackedApiToken()));
|
||||
const keyB = rateLimitKeyGenerator(forged(unbackedApiToken()));
|
||||
const keyAnon = rateLimitKeyGenerator({ cookies: {}, headers: {}, ip } as unknown as Request);
|
||||
expect(keyA).not.toMatch(/^user:sk:/);
|
||||
expect(keyA).toBe(keyB);
|
||||
expect(keyA).toBe(keyAnon);
|
||||
});
|
||||
|
||||
it('keys a live API-token bearer by the token even when a cookie is present (bearer precedence)', () => {
|
||||
// authMiddleware prefers the Bearer token over the cookie, so the limiter
|
||||
// must key off the same credential; a forged cookie must not override the
|
||||
// token's bucket.
|
||||
const { raw, row } = createToken('read-only');
|
||||
const forgedCookie = jwt.sign({ username: 'someone-else' }, 'attacker-secret');
|
||||
const req = {
|
||||
cookies: { [COOKIE_NAME]: forgedCookie },
|
||||
headers: { authorization: `Bearer ${raw}` },
|
||||
ip: '203.0.113.7',
|
||||
} as unknown as Request;
|
||||
expect(rateLimitKeyGenerator(req)).toBe(`user:sk:${row.token_hash.slice(0, 16)}`);
|
||||
});
|
||||
|
||||
it('does not let a forged cookie rescue a forged API-token bearer from per-IP keying', () => {
|
||||
const forgedCookie = jwt.sign({ username: 'rotated-1' }, 'attacker-secret');
|
||||
const req = {
|
||||
cookies: { [COOKIE_NAME]: forgedCookie },
|
||||
headers: { authorization: `Bearer ${unbackedApiToken()}` },
|
||||
ip: '203.0.113.7',
|
||||
} as unknown as Request;
|
||||
const key = rateLimitKeyGenerator(req);
|
||||
expect(key).not.toMatch(/^user:/);
|
||||
expect(key).toContain('203.0.113.7');
|
||||
});
|
||||
|
||||
it('still keys a JWT session bearer by username (non-token branch intact)', () => {
|
||||
const token = jwt.sign({ username: 'ci-bot' }, TEST_JWT_SECRET);
|
||||
const req = { cookies: {}, headers: { authorization: `Bearer ${token}` }, ip: '203.0.113.7' } as unknown as Request;
|
||||
expect(rateLimitKeyGenerator(req)).toBe('user:ci-bot');
|
||||
});
|
||||
|
||||
it('still keys a session cookie by username (cookie branch intact)', () => {
|
||||
const token = jwt.sign({ username: 'cookie-user' }, TEST_JWT_SECRET);
|
||||
const req = { cookies: { [COOKIE_NAME]: token }, headers: {}, ip: '203.0.113.7' } as unknown as Request;
|
||||
expect(rateLimitKeyGenerator(req)).toBe('user:cookie-user');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,79 @@
|
||||
/**
|
||||
* Integration tests for API-token scope enforcement on the WebSocket upgrade.
|
||||
* Restricted scopes (read-only, deploy-only) may reach only stack logs and
|
||||
* notifications; every other WS path (host console, generic) is 403'd by the
|
||||
* scope gate before dispatch. Driven through a real listening server, no mocks.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
|
||||
import WebSocket from 'ws';
|
||||
import { setupTestDb, cleanupTestDb } from './helpers/setupTestDb';
|
||||
import { createTestApiToken } from './helpers/apiTokenTestHelper';
|
||||
|
||||
let tmpDir: string;
|
||||
let server: import('http').Server;
|
||||
let DatabaseService: typeof import('../services/DatabaseService').DatabaseService;
|
||||
|
||||
function createToken(scope: 'read-only' | 'deploy-only' | 'full-admin'): string {
|
||||
const db = DatabaseService.getInstance();
|
||||
return createTestApiToken({ db: DatabaseService, scope, userId: db.getUserByUsername('testadmin')!.id });
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
vi.restoreAllMocks();
|
||||
tmpDir = await setupTestDb();
|
||||
({ DatabaseService } = await import('../services/DatabaseService'));
|
||||
const mod = await import('../index');
|
||||
server = mod.server;
|
||||
await new Promise<void>((resolve) => server.listen(0, resolve));
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await new Promise<void>((resolve) => server.close(() => resolve()));
|
||||
cleanupTestDb(tmpDir);
|
||||
});
|
||||
|
||||
function wsUrl(path: string): string {
|
||||
const addr = server.address();
|
||||
if (!addr || typeof addr === 'string') throw new Error('Server not listening');
|
||||
return `ws://127.0.0.1:${addr.port}${path}`;
|
||||
}
|
||||
|
||||
/** Resolve to the rejected-upgrade HTTP status, or 200 if the socket opens. */
|
||||
function upgradeStatus(token: string, path: string): Promise<number> {
|
||||
return new Promise<number>((resolve) => {
|
||||
const ws = new WebSocket(wsUrl(path), { headers: { Authorization: `Bearer ${token}` } });
|
||||
ws.on('unexpected-response', (_req, res) => { ws.close(); resolve(res.statusCode ?? 0); });
|
||||
ws.on('open', () => { ws.close(); resolve(200); });
|
||||
ws.on('error', () => resolve(0));
|
||||
});
|
||||
}
|
||||
|
||||
describe('WebSocket API-token scope enforcement', () => {
|
||||
it('blocks a read-only token from the host console (403)', async () => {
|
||||
expect(await upgradeStatus(createToken('read-only'), '/api/system/host-console')).toBe(403);
|
||||
});
|
||||
|
||||
it('blocks a deploy-only token from the host console (403)', async () => {
|
||||
expect(await upgradeStatus(createToken('deploy-only'), '/api/system/host-console')).toBe(403);
|
||||
});
|
||||
|
||||
it('blocks a read-only token from a generic socket (403)', async () => {
|
||||
expect(await upgradeStatus(createToken('read-only'), '/ws')).toBe(403);
|
||||
});
|
||||
|
||||
it('does not scope-block a read-only token from notifications', async () => {
|
||||
expect(await upgradeStatus(createToken('read-only'), '/ws/notifications')).not.toBe(403);
|
||||
});
|
||||
|
||||
it('does not scope-block a deploy-only token from notifications', async () => {
|
||||
expect(await upgradeStatus(createToken('deploy-only'), '/ws/notifications')).not.toBe(403);
|
||||
});
|
||||
|
||||
it('does not scope-block a read-only token from stack logs', async () => {
|
||||
expect(await upgradeStatus(createToken('read-only'), '/api/stacks/test-stack/logs')).not.toBe(403);
|
||||
});
|
||||
|
||||
it('does not scope-block a full-admin token from a generic socket', async () => {
|
||||
expect(await upgradeStatus(createToken('full-admin'), '/ws')).not.toBe(403);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user