mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-17 04:17:11 +00:00
fix(cursor): address third-round review feedback for auth module
This commit is contained in:
+26
-26
@@ -165,16 +165,20 @@ export function extractUserInfo(
|
||||
const decoded = JSON.parse(
|
||||
Buffer.from(payload.replace(/-/g, '+').replace(/_/g, '/'), 'base64').toString()
|
||||
) as Record<string, unknown>;
|
||||
return {
|
||||
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,
|
||||
};
|
||||
|
||||
const email = typeof decoded.email === 'string' ? decoded.email : undefined;
|
||||
const userId =
|
||||
typeof decoded.sub === 'string'
|
||||
? decoded.sub
|
||||
: typeof decoded.user_id === 'string'
|
||||
? decoded.user_id
|
||||
: undefined;
|
||||
const exp = typeof decoded.exp === 'number' ? decoded.exp : undefined;
|
||||
|
||||
// If all claims are undefined, treat as if JWT parsing failed
|
||||
if (!email && !userId && exp === undefined) return null;
|
||||
|
||||
return { email, userId, exp };
|
||||
}
|
||||
} catch {
|
||||
// Token is not a JWT, that's okay
|
||||
@@ -272,17 +276,19 @@ export function checkAuthStatus(): CursorAuthStatus {
|
||||
const userInfo = extractUserInfo(credentials.accessToken);
|
||||
|
||||
if (userInfo?.exp) {
|
||||
// Use JWT exp claim (Unix timestamp in seconds)
|
||||
// Use JWT exp claim for expiry detection
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
expired = now >= userInfo.exp;
|
||||
tokenAge = Math.floor((now - (userInfo.exp - 24 * 60 * 60)) / (60 * 60)); // Assume 24h token
|
||||
} else {
|
||||
// Fallback to importedAt heuristic
|
||||
const TOKEN_EXPIRY_HOURS = 24;
|
||||
const importedDate = new Date(credentials.importedAt);
|
||||
if (!isNaN(importedDate.getTime())) {
|
||||
const now = new Date();
|
||||
tokenAge = Math.floor((now.getTime() - importedDate.getTime()) / (1000 * 60 * 60));
|
||||
}
|
||||
|
||||
// Always use importedAt for tokenAge (more reliable than reverse-engineering JWT lifetime)
|
||||
const TOKEN_EXPIRY_HOURS = 24;
|
||||
const importedDate = new Date(credentials.importedAt);
|
||||
if (!isNaN(importedDate.getTime())) {
|
||||
const now = new Date();
|
||||
tokenAge = Math.floor((now.getTime() - importedDate.getTime()) / (1000 * 60 * 60));
|
||||
// Only set expired from importedAt if JWT exp was not available
|
||||
if (userInfo?.exp === undefined) {
|
||||
expired = tokenAge >= TOKEN_EXPIRY_HOURS;
|
||||
}
|
||||
}
|
||||
@@ -299,14 +305,8 @@ export function checkAuthStatus(): CursorAuthStatus {
|
||||
* Delete credentials file
|
||||
*/
|
||||
export function deleteCredentials(): boolean {
|
||||
const credPath = getCredentialsPath();
|
||||
|
||||
if (!fs.existsSync(credPath)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
try {
|
||||
fs.unlinkSync(credPath);
|
||||
fs.unlinkSync(getCredentialsPath());
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
|
||||
@@ -14,6 +14,7 @@ import {
|
||||
loadCredentials,
|
||||
checkAuthStatus,
|
||||
deleteCredentials,
|
||||
autoDetectTokens,
|
||||
} from '../../../src/cursor/cursor-auth';
|
||||
|
||||
// Test isolation
|
||||
@@ -103,9 +104,9 @@ describe('extractUserInfo', () => {
|
||||
|
||||
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 payload = Buffer.from(JSON.stringify({ sub: 'uuid-12345', exp: 1234567890 })).toString(
|
||||
'base64'
|
||||
);
|
||||
const token = `header.${payload}.signature`;
|
||||
|
||||
const result = extractUserInfo(token);
|
||||
@@ -142,6 +143,15 @@ describe('extractUserInfo', () => {
|
||||
exp: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it('should return null for JWT with no meaningful claims', () => {
|
||||
// JWT: {"iat":1234567890} (only issued-at, no email/sub/exp)
|
||||
const payload = Buffer.from(JSON.stringify({ iat: 1234567890 })).toString('base64');
|
||||
const token = `header.${payload}.signature`;
|
||||
|
||||
const result = extractUserInfo(token);
|
||||
expect(result).toBe(null);
|
||||
});
|
||||
});
|
||||
|
||||
describe('saveCredentials and loadCredentials', () => {
|
||||
@@ -187,8 +197,10 @@ describe('saveCredentials and loadCredentials', () => {
|
||||
});
|
||||
|
||||
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 });
|
||||
// CCS_HOME is set to tempDir, getCcsDir() returns path.join(tempDir, '.ccs')
|
||||
const credDir = path.join(tempDir, '.ccs', 'cursor');
|
||||
const credPath = path.join(credDir, 'credentials.json');
|
||||
fs.mkdirSync(credDir, { recursive: true });
|
||||
fs.writeFileSync(credPath, 'invalid json{{{');
|
||||
|
||||
const loaded = loadCredentials();
|
||||
@@ -196,8 +208,9 @@ describe('saveCredentials and loadCredentials', () => {
|
||||
});
|
||||
|
||||
it('should return null for credentials missing required fields', () => {
|
||||
const credPath = path.join(tempDir, 'cursor', 'credentials.json');
|
||||
fs.mkdirSync(path.dirname(credPath), { recursive: true });
|
||||
const credDir = path.join(tempDir, '.ccs', 'cursor');
|
||||
const credPath = path.join(credDir, 'credentials.json');
|
||||
fs.mkdirSync(credDir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
credPath,
|
||||
JSON.stringify({
|
||||
@@ -211,8 +224,9 @@ describe('saveCredentials and loadCredentials', () => {
|
||||
});
|
||||
|
||||
it('should return null for credentials with wrong types', () => {
|
||||
const credPath = path.join(tempDir, 'cursor', 'credentials.json');
|
||||
fs.mkdirSync(path.dirname(credPath), { recursive: true });
|
||||
const credDir = path.join(tempDir, '.ccs', 'cursor');
|
||||
const credPath = path.join(credDir, 'credentials.json');
|
||||
fs.mkdirSync(credDir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
credPath,
|
||||
JSON.stringify({
|
||||
@@ -228,8 +242,9 @@ describe('saveCredentials and loadCredentials', () => {
|
||||
});
|
||||
|
||||
it('should return null for invalid authMethod', () => {
|
||||
const credPath = path.join(tempDir, 'cursor', 'credentials.json');
|
||||
fs.mkdirSync(path.dirname(credPath), { recursive: true });
|
||||
const credDir = path.join(tempDir, '.ccs', 'cursor');
|
||||
const credPath = path.join(credDir, 'credentials.json');
|
||||
fs.mkdirSync(credDir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
credPath,
|
||||
JSON.stringify({
|
||||
@@ -386,3 +401,48 @@ describe('deleteCredentials', () => {
|
||||
expect(deleteCredentials()).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('autoDetectTokens', () => {
|
||||
it('should return not found for Windows platform', () => {
|
||||
// Save original platform
|
||||
const originalPlatform = process.platform;
|
||||
|
||||
// Mock Windows platform
|
||||
Object.defineProperty(process, 'platform', {
|
||||
value: 'win32',
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
const result = autoDetectTokens();
|
||||
|
||||
expect(result.found).toBe(false);
|
||||
expect(result.error).toContain('not supported on Windows');
|
||||
|
||||
// Restore original platform
|
||||
Object.defineProperty(process, 'platform', {
|
||||
value: originalPlatform,
|
||||
configurable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('should return not found when database file does not exist', () => {
|
||||
// Skip on Windows (already covered by previous test)
|
||||
if (process.platform === 'win32') {
|
||||
return;
|
||||
}
|
||||
|
||||
const result = autoDetectTokens();
|
||||
|
||||
// Should fail because Cursor database doesn't exist in test environment
|
||||
expect(result.found).toBe(false);
|
||||
expect(result.error).toBeDefined();
|
||||
});
|
||||
|
||||
it('should have found property in return type', () => {
|
||||
const result = autoDetectTokens();
|
||||
|
||||
// Verify return type structure
|
||||
expect(result).toHaveProperty('found');
|
||||
expect(typeof result.found).toBe('boolean');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user