diff --git a/src/middleware/rateLimit.js b/src/middleware/rateLimit.js index 1ec7ca4..b65f1a2 100644 --- a/src/middleware/rateLimit.js +++ b/src/middleware/rateLimit.js @@ -28,9 +28,20 @@ setInterval(() => { /** * Get rate limit key from request + * Parses Authorization header directly instead of relying on req.token + * to avoid middleware execution order issues (req.token is set by + * requireAuth, which may run after the rate limiter) */ function getKey(req, limitType) { - const identifier = req.token || req.ip || 'anonymous'; + const authHeader = req.headers.authorization; + let identifier; + + if (authHeader && authHeader.startsWith('Bearer ')) { + identifier = authHeader.substring(7); + } else { + identifier = req.ip || 'anonymous'; + } + return `rl:${limitType}:${identifier}`; } diff --git a/test/rate-limiter.test.js b/test/rate-limiter.test.js new file mode 100644 index 0000000..60dea7f --- /dev/null +++ b/test/rate-limiter.test.js @@ -0,0 +1,104 @@ +/** + * Rate limiter fix tests + * Verifies getKey() parses Authorization header directly + * instead of relying on req.token (which caused #5) + */ + +// Mirror the fixed getKey function +function getKey(req, limitType) { + const authHeader = req.headers.authorization; + let identifier; + + if (authHeader && authHeader.startsWith('Bearer ')) { + identifier = authHeader.substring(7); + } else { + identifier = req.ip || 'anonymous'; + } + + return `rl:${limitType}:${identifier}`; +} + +let passed = 0; +let failed = 0; + +function test(name, fn) { + try { + fn(); + console.log(` + ${name}`); + passed++; + } catch (error) { + console.log(` - ${name}`); + console.log(` Error: ${error.message}`); + failed++; + } +} + +function assertEqual(actual, expected, message) { + if (actual !== expected) { + throw new Error(message || `Expected "${expected}", got "${actual}"`); + } +} + +function assertNotEqual(actual, expected, message) { + if (actual === expected) { + throw new Error(message || `Expected values to differ, both were "${actual}"`); + } +} + +console.log('\nRate Limiter Fix Tests\n' + '='.repeat(50)); + +test('extracts token from Bearer header', () => { + const req = { + headers: { authorization: 'Bearer moltbook_test_token_123' }, + ip: '127.0.0.1' + }; + assertEqual(getKey(req, 'comments'), 'rl:comments:moltbook_test_token_123'); +}); + +test('falls back to IP when no Authorization header', () => { + const req = { headers: {}, ip: '192.168.1.1' }; + assertEqual(getKey(req, 'comments'), 'rl:comments:192.168.1.1'); +}); + +test('falls back to anonymous when nothing available', () => { + const req = { headers: {}, ip: null }; + assertEqual(getKey(req, 'comments'), 'rl:comments:anonymous'); +}); + +test('handles non-Bearer auth schemes by falling back to IP', () => { + const req = { headers: { authorization: 'Basic abc123' }, ip: '10.0.0.1' }; + assertEqual(getKey(req, 'requests'), 'rl:requests:10.0.0.1'); +}); + +test('ignores req.token entirely (root cause of #5)', () => { + const req = { + headers: { authorization: 'Bearer real_token' }, + ip: '127.0.0.1', + token: 'this_should_be_ignored' + }; + const key = getKey(req, 'comments'); + assertEqual(key, 'rl:comments:real_token'); + assertNotEqual(key, 'rl:comments:this_should_be_ignored', + 'REGRESSION: getKey is using req.token instead of Authorization header'); +}); + +test('works when req.token is undefined (the actual #5 scenario)', () => { + const req = { + headers: { authorization: 'Bearer moltbook_sk_valid_key' }, + ip: '172.17.0.1', + token: undefined + }; + assertEqual(getKey(req, 'comments'), 'rl:comments:moltbook_sk_valid_key'); +}); + +test('handles repeated requests without identity drift', () => { + const apiKey = 'moltbook_sk_8Xn6T1MLuY_test'; + for (let i = 0; i < 50; i++) { + const req = { headers: { authorization: `Bearer ${apiKey}` }, ip: '10.0.0.1' }; + assertEqual(getKey(req, 'comments'), `rl:comments:${apiKey}`); + } +}); + +console.log('\n' + '='.repeat(50)); +console.log(`Results: ${passed} passed, ${failed} failed`); +process.exit(failed > 0 ? 1 : 0);