mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 06:16:37 +00:00
fix(cursor): address second-round review feedback for auth module
- Add comprehensive unit tests for cursor-auth.test.ts
- validateToken: valid/invalid tokens, short tokens, UUID formats, empty strings
- extractUserInfo: JWT parsing, email handling, non-JWT tokens, malformed base64
- saveCredentials/loadCredentials: round-trip, invalid JSON/types, missing fields
- checkAuthStatus: authenticated/not authenticated, expired tokens, JWT exp, invalid dates
- deleteCredentials: delete existing/non-existent files, multiple deletes
- All tests use CCS_HOME env var for isolation, real file I/O, no mocks
- Fix dead try-catch around new Date() in checkAuthStatus()
- Replace try-catch with isNaN check (new Date('garbage') returns Invalid Date, not throw)
- Properly handle Invalid Date by checking isNaN(getTime())
- Fix email populated with sub claim in extractUserInfo()
- Change email: decoded.email || decoded.sub to email: decoded.email || undefined
- Prevent non-email values (UUIDs) from populating email field
- Add type guards for JSON.parse result in extractUserInfo()
- Cast to Record<string, unknown> and validate types
- Use typeof checks for email, userId, exp fields
This commit is contained in:
@@ -164,11 +164,16 @@ export function extractUserInfo(
|
||||
}
|
||||
const decoded = JSON.parse(
|
||||
Buffer.from(payload.replace(/-/g, '+').replace(/_/g, '/'), 'base64').toString()
|
||||
);
|
||||
) as Record<string, unknown>;
|
||||
return {
|
||||
email: decoded.email || decoded.sub,
|
||||
userId: decoded.sub || decoded.user_id,
|
||||
exp: decoded.exp,
|
||||
email: typeof decoded.email === 'string' ? decoded.email : undefined,
|
||||
userId:
|
||||
typeof decoded.sub === 'string'
|
||||
? decoded.sub
|
||||
: typeof decoded.user_id === 'string'
|
||||
? decoded.user_id
|
||||
: undefined,
|
||||
exp: typeof decoded.exp === 'number' ? decoded.exp : undefined,
|
||||
};
|
||||
}
|
||||
} catch {
|
||||
@@ -274,13 +279,11 @@ export function checkAuthStatus(): CursorAuthStatus {
|
||||
} else {
|
||||
// Fallback to importedAt heuristic
|
||||
const TOKEN_EXPIRY_HOURS = 24;
|
||||
try {
|
||||
const importedDate = new Date(credentials.importedAt);
|
||||
const importedDate = new Date(credentials.importedAt);
|
||||
if (!isNaN(importedDate.getTime())) {
|
||||
const now = new Date();
|
||||
tokenAge = Math.floor((now.getTime() - importedDate.getTime()) / (1000 * 60 * 60));
|
||||
expired = tokenAge >= TOKEN_EXPIRY_HOURS;
|
||||
} catch {
|
||||
// Invalid date format
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,388 @@
|
||||
/**
|
||||
* Unit tests for Cursor authentication module
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
import type { CursorCredentials } from '../../../src/cursor/types';
|
||||
import {
|
||||
validateToken,
|
||||
extractUserInfo,
|
||||
saveCredentials,
|
||||
loadCredentials,
|
||||
checkAuthStatus,
|
||||
deleteCredentials,
|
||||
} from '../../../src/cursor/cursor-auth';
|
||||
|
||||
// Test isolation
|
||||
let originalCcsHome: string | undefined;
|
||||
let tempDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
// Save original CCS_HOME
|
||||
originalCcsHome = process.env.CCS_HOME;
|
||||
|
||||
// Create temp directory for test isolation
|
||||
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-cursor-test-'));
|
||||
process.env.CCS_HOME = tempDir;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Restore original CCS_HOME
|
||||
if (originalCcsHome !== undefined) {
|
||||
process.env.CCS_HOME = originalCcsHome;
|
||||
} else {
|
||||
delete process.env.CCS_HOME;
|
||||
}
|
||||
|
||||
// Clean up temp directory
|
||||
if (fs.existsSync(tempDir)) {
|
||||
fs.rmSync(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
describe('validateToken', () => {
|
||||
it('should accept valid token and machineId', () => {
|
||||
const token = 'a'.repeat(50); // 50 chars minimum
|
||||
const machineId = 'a'.repeat(32); // 32 hex chars
|
||||
expect(validateToken(token, machineId)).toBe(true);
|
||||
});
|
||||
|
||||
it('should reject 49-char token (too short)', () => {
|
||||
const token = 'a'.repeat(49); // Just under minimum
|
||||
const machineId = 'a'.repeat(32);
|
||||
expect(validateToken(token, machineId)).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject 31-char hex UUID (too short)', () => {
|
||||
const token = 'a'.repeat(50);
|
||||
const machineId = 'a'.repeat(31); // Just under 32
|
||||
expect(validateToken(token, machineId)).toBe(false);
|
||||
});
|
||||
|
||||
it('should accept UUID with hyphens (strips them)', () => {
|
||||
const token = 'a'.repeat(50);
|
||||
const machineId = '12345678-1234-1234-1234-123456789abc'; // 36 chars with hyphens
|
||||
expect(validateToken(token, machineId)).toBe(true);
|
||||
});
|
||||
|
||||
it('should reject empty token', () => {
|
||||
const machineId = 'a'.repeat(32);
|
||||
expect(validateToken('', machineId)).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject empty machineId', () => {
|
||||
const token = 'a'.repeat(50);
|
||||
expect(validateToken(token, '')).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject non-hex characters in machineId', () => {
|
||||
const token = 'a'.repeat(50);
|
||||
const machineId = 'g'.repeat(32); // 'g' is not valid hex
|
||||
expect(validateToken(token, machineId)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('extractUserInfo', () => {
|
||||
it('should extract email and sub from valid JWT', () => {
|
||||
// JWT: {"email":"user@example.com","sub":"12345","exp":1234567890}
|
||||
const payload = Buffer.from(
|
||||
JSON.stringify({ email: 'user@example.com', sub: '12345', exp: 1234567890 })
|
||||
).toString('base64');
|
||||
const token = `header.${payload}.signature`;
|
||||
|
||||
const result = extractUserInfo(token);
|
||||
expect(result).toEqual({
|
||||
email: 'user@example.com',
|
||||
userId: '12345',
|
||||
exp: 1234567890,
|
||||
});
|
||||
});
|
||||
|
||||
it('should return undefined email when only sub claim exists', () => {
|
||||
// JWT: {"sub":"uuid-12345","exp":1234567890}
|
||||
const payload = Buffer.from(
|
||||
JSON.stringify({ sub: 'uuid-12345', exp: 1234567890 })
|
||||
).toString('base64');
|
||||
const token = `header.${payload}.signature`;
|
||||
|
||||
const result = extractUserInfo(token);
|
||||
expect(result).toEqual({
|
||||
email: undefined,
|
||||
userId: 'uuid-12345',
|
||||
exp: 1234567890,
|
||||
});
|
||||
});
|
||||
|
||||
it('should return null for non-JWT token', () => {
|
||||
const token = 'a'.repeat(50); // Plain token
|
||||
const result = extractUserInfo(token);
|
||||
expect(result).toBe(null);
|
||||
});
|
||||
|
||||
it('should return null for malformed base64', () => {
|
||||
const token = 'header.!!!invalid-base64!!!.signature';
|
||||
const result = extractUserInfo(token);
|
||||
expect(result).toBe(null);
|
||||
});
|
||||
|
||||
it('should handle JWT with user_id instead of sub', () => {
|
||||
// JWT: {"email":"user@example.com","user_id":"67890"}
|
||||
const payload = Buffer.from(
|
||||
JSON.stringify({ email: 'user@example.com', user_id: '67890' })
|
||||
).toString('base64');
|
||||
const token = `header.${payload}.signature`;
|
||||
|
||||
const result = extractUserInfo(token);
|
||||
expect(result).toEqual({
|
||||
email: 'user@example.com',
|
||||
userId: '67890',
|
||||
exp: undefined,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('saveCredentials and loadCredentials', () => {
|
||||
it('should save and load credentials successfully', () => {
|
||||
const credentials: CursorCredentials = {
|
||||
accessToken: 'a'.repeat(50),
|
||||
machineId: 'b'.repeat(32),
|
||||
authMethod: 'auto-detect',
|
||||
importedAt: new Date().toISOString(),
|
||||
};
|
||||
|
||||
saveCredentials(credentials);
|
||||
const loaded = loadCredentials();
|
||||
|
||||
expect(loaded).toEqual(credentials);
|
||||
});
|
||||
|
||||
it('should return null when no credentials file exists', () => {
|
||||
const loaded = loadCredentials();
|
||||
expect(loaded).toBe(null);
|
||||
});
|
||||
|
||||
it('should create directory with restrictive permissions', () => {
|
||||
const credentials: CursorCredentials = {
|
||||
accessToken: 'a'.repeat(50),
|
||||
machineId: 'b'.repeat(32),
|
||||
authMethod: 'manual',
|
||||
importedAt: new Date().toISOString(),
|
||||
};
|
||||
|
||||
saveCredentials(credentials);
|
||||
|
||||
// CCS_HOME is set to tempDir, but getCcsDir() appends '.ccs' to it
|
||||
const credDir = path.join(tempDir, '.ccs', 'cursor');
|
||||
expect(fs.existsSync(credDir)).toBe(true);
|
||||
|
||||
// Check directory permissions (skip on Windows)
|
||||
if (process.platform !== 'win32') {
|
||||
const stats = fs.statSync(credDir);
|
||||
const mode = stats.mode & 0o777;
|
||||
expect(mode).toBe(0o700);
|
||||
}
|
||||
});
|
||||
|
||||
it('should return null for invalid JSON in credentials file', () => {
|
||||
const credPath = path.join(tempDir, 'cursor', 'credentials.json');
|
||||
fs.mkdirSync(path.dirname(credPath), { recursive: true });
|
||||
fs.writeFileSync(credPath, 'invalid json{{{');
|
||||
|
||||
const loaded = loadCredentials();
|
||||
expect(loaded).toBe(null);
|
||||
});
|
||||
|
||||
it('should return null for credentials missing required fields', () => {
|
||||
const credPath = path.join(tempDir, 'cursor', 'credentials.json');
|
||||
fs.mkdirSync(path.dirname(credPath), { recursive: true });
|
||||
fs.writeFileSync(
|
||||
credPath,
|
||||
JSON.stringify({
|
||||
accessToken: 'token',
|
||||
// Missing machineId, authMethod, importedAt
|
||||
})
|
||||
);
|
||||
|
||||
const loaded = loadCredentials();
|
||||
expect(loaded).toBe(null);
|
||||
});
|
||||
|
||||
it('should return null for credentials with wrong types', () => {
|
||||
const credPath = path.join(tempDir, 'cursor', 'credentials.json');
|
||||
fs.mkdirSync(path.dirname(credPath), { recursive: true });
|
||||
fs.writeFileSync(
|
||||
credPath,
|
||||
JSON.stringify({
|
||||
accessToken: 123, // Wrong type (number instead of string)
|
||||
machineId: 'abc',
|
||||
authMethod: 'auto-detect',
|
||||
importedAt: new Date().toISOString(),
|
||||
})
|
||||
);
|
||||
|
||||
const loaded = loadCredentials();
|
||||
expect(loaded).toBe(null);
|
||||
});
|
||||
|
||||
it('should return null for invalid authMethod', () => {
|
||||
const credPath = path.join(tempDir, 'cursor', 'credentials.json');
|
||||
fs.mkdirSync(path.dirname(credPath), { recursive: true });
|
||||
fs.writeFileSync(
|
||||
credPath,
|
||||
JSON.stringify({
|
||||
accessToken: 'token',
|
||||
machineId: 'abc',
|
||||
authMethod: 'invalid-method', // Invalid authMethod
|
||||
importedAt: new Date().toISOString(),
|
||||
})
|
||||
);
|
||||
|
||||
const loaded = loadCredentials();
|
||||
expect(loaded).toBe(null);
|
||||
});
|
||||
});
|
||||
|
||||
describe('checkAuthStatus', () => {
|
||||
it('should return not authenticated when no credentials exist', () => {
|
||||
const status = checkAuthStatus();
|
||||
expect(status.authenticated).toBe(false);
|
||||
expect(status.credentials).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should return authenticated for valid credentials', () => {
|
||||
const credentials: CursorCredentials = {
|
||||
accessToken: 'a'.repeat(50),
|
||||
machineId: 'b'.repeat(32),
|
||||
authMethod: 'auto-detect',
|
||||
importedAt: new Date().toISOString(),
|
||||
};
|
||||
|
||||
saveCredentials(credentials);
|
||||
const status = checkAuthStatus();
|
||||
|
||||
expect(status.authenticated).toBe(true);
|
||||
expect(status.credentials).toEqual(credentials);
|
||||
expect(status.expired).toBe(false);
|
||||
expect(status.tokenAge).toBeDefined();
|
||||
expect(status.tokenAge).toBeLessThan(1); // Just imported
|
||||
});
|
||||
|
||||
it('should detect expired credentials (importedAt > 24h ago)', () => {
|
||||
// Create credentials from 25 hours ago
|
||||
const past = new Date();
|
||||
past.setHours(past.getHours() - 25);
|
||||
|
||||
const credentials: CursorCredentials = {
|
||||
accessToken: 'a'.repeat(50),
|
||||
machineId: 'b'.repeat(32),
|
||||
authMethod: 'manual',
|
||||
importedAt: past.toISOString(),
|
||||
};
|
||||
|
||||
saveCredentials(credentials);
|
||||
const status = checkAuthStatus();
|
||||
|
||||
expect(status.authenticated).toBe(true);
|
||||
expect(status.expired).toBe(true);
|
||||
expect(status.tokenAge).toBeGreaterThanOrEqual(24);
|
||||
});
|
||||
|
||||
it('should return not authenticated for invalid token format', () => {
|
||||
const credentials: CursorCredentials = {
|
||||
accessToken: 'short', // Invalid (too short)
|
||||
machineId: 'b'.repeat(32),
|
||||
authMethod: 'manual',
|
||||
importedAt: new Date().toISOString(),
|
||||
};
|
||||
|
||||
saveCredentials(credentials);
|
||||
const status = checkAuthStatus();
|
||||
|
||||
expect(status.authenticated).toBe(false);
|
||||
});
|
||||
|
||||
it('should use JWT exp claim when available', () => {
|
||||
// Create JWT token that expired 1 hour ago
|
||||
const expiredTime = Math.floor(Date.now() / 1000) - 3600;
|
||||
const payload = Buffer.from(
|
||||
JSON.stringify({ email: 'test@example.com', sub: '123', exp: expiredTime })
|
||||
).toString('base64');
|
||||
const jwtToken = `header.${payload}.signature`;
|
||||
|
||||
const credentials: CursorCredentials = {
|
||||
accessToken: jwtToken,
|
||||
machineId: 'b'.repeat(32),
|
||||
authMethod: 'auto-detect',
|
||||
importedAt: new Date().toISOString(), // Recent import
|
||||
};
|
||||
|
||||
saveCredentials(credentials);
|
||||
const status = checkAuthStatus();
|
||||
|
||||
expect(status.authenticated).toBe(true);
|
||||
expect(status.expired).toBe(true); // Should detect expiry from JWT exp
|
||||
});
|
||||
|
||||
it('should handle invalid importedAt date gracefully', () => {
|
||||
// Create credentials with valid format but garbage date value
|
||||
const credentials: CursorCredentials = {
|
||||
accessToken: 'a'.repeat(50),
|
||||
machineId: 'b'.repeat(32),
|
||||
authMethod: 'manual',
|
||||
importedAt: 'invalid-date-garbage-2026-99-99T99:99:99Z',
|
||||
};
|
||||
|
||||
saveCredentials(credentials);
|
||||
const status = checkAuthStatus();
|
||||
|
||||
// Should still authenticate if token format is valid
|
||||
expect(status.authenticated).toBe(true);
|
||||
// tokenAge should be undefined due to invalid date (NaN from getTime())
|
||||
expect(status.tokenAge).toBeUndefined();
|
||||
// expired should be false (defaults to false when date parsing fails)
|
||||
expect(status.expired).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('deleteCredentials', () => {
|
||||
it('should delete existing credentials file and return true', () => {
|
||||
const credentials: CursorCredentials = {
|
||||
accessToken: 'a'.repeat(50),
|
||||
machineId: 'b'.repeat(32),
|
||||
authMethod: 'auto-detect',
|
||||
importedAt: new Date().toISOString(),
|
||||
};
|
||||
|
||||
saveCredentials(credentials);
|
||||
expect(loadCredentials()).not.toBe(null);
|
||||
|
||||
const result = deleteCredentials();
|
||||
expect(result).toBe(true);
|
||||
expect(loadCredentials()).toBe(null);
|
||||
});
|
||||
|
||||
it('should return false when credentials file does not exist', () => {
|
||||
const result = deleteCredentials();
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it('should handle multiple delete calls gracefully', () => {
|
||||
const credentials: CursorCredentials = {
|
||||
accessToken: 'a'.repeat(50),
|
||||
machineId: 'b'.repeat(32),
|
||||
authMethod: 'manual',
|
||||
importedAt: new Date().toISOString(),
|
||||
};
|
||||
|
||||
saveCredentials(credentials);
|
||||
|
||||
// First delete should succeed
|
||||
expect(deleteCredentials()).toBe(true);
|
||||
|
||||
// Second delete should return false (already deleted)
|
||||
expect(deleteCredentials()).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user