mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-02-02 20:43:36 +00:00
fix(auth): Enhance credential detection logic for OAuth
- Updated getClaudeAuthIndicators() to ensure that empty or token-less credential files do not prevent the detection of valid credentials in subsequent paths. - Improved error handling for settings file readability checks, providing clearer feedback on file access issues. - Added unit tests to validate the new behavior, ensuring that the system continues to check all credential paths even when some files are empty or invalid. This change improves the robustness of the credential detection process and enhances user experience by allowing for more flexible credential management.
This commit is contained in:
@@ -1065,11 +1065,20 @@ export async function getClaudeAuthIndicators(): Promise<ClaudeAuthIndicators> {
|
||||
};
|
||||
|
||||
// Check settings file
|
||||
// First check existence, then try to read to confirm it's actually readable
|
||||
try {
|
||||
if (await systemPathAccess(settingsPath)) {
|
||||
settingsFileCheck.exists = true;
|
||||
settingsFileCheck.readable = true;
|
||||
result.hasSettingsFile = true;
|
||||
// Try to actually read the file to confirm read permissions
|
||||
try {
|
||||
await systemPathReadFile(settingsPath);
|
||||
settingsFileCheck.readable = true;
|
||||
result.hasSettingsFile = true;
|
||||
} catch (readErr) {
|
||||
// File exists but cannot be read (permission denied, etc.)
|
||||
settingsFileCheck.readable = false;
|
||||
settingsFileCheck.error = `Cannot read: ${readErr instanceof Error ? readErr.message : String(readErr)}`;
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
settingsFileCheck.error = err instanceof Error ? err.message : String(err);
|
||||
@@ -1117,6 +1126,9 @@ export async function getClaudeAuthIndicators(): Promise<ClaudeAuthIndicators> {
|
||||
}
|
||||
|
||||
// Check credentials files
|
||||
// We iterate through all credential paths and only stop when we find a file
|
||||
// that contains actual credentials (OAuth tokens or API keys). An empty or
|
||||
// token-less file should not prevent checking subsequent credential paths.
|
||||
for (let i = 0; i < credentialPaths.length; i++) {
|
||||
const credPath = credentialPaths[i];
|
||||
const credCheck = credentialFileChecks[i];
|
||||
@@ -1126,18 +1138,27 @@ export async function getClaudeAuthIndicators(): Promise<ClaudeAuthIndicators> {
|
||||
credCheck.readable = true;
|
||||
try {
|
||||
const credentials = JSON.parse(content);
|
||||
result.hasCredentialsFile = true;
|
||||
// Support multiple credential formats:
|
||||
// 1. Claude Code CLI format: { claudeAiOauth: { accessToken, refreshToken } }
|
||||
// 2. Legacy format: { oauth_token } or { access_token }
|
||||
// 3. API key format: { api_key }
|
||||
const hasClaudeOauth = !!credentials.claudeAiOauth?.accessToken;
|
||||
const hasLegacyOauth = !!(credentials.oauth_token || credentials.access_token);
|
||||
result.credentials = {
|
||||
hasOAuthToken: hasClaudeOauth || hasLegacyOauth,
|
||||
hasApiKey: !!credentials.api_key,
|
||||
};
|
||||
break;
|
||||
const hasOAuthToken = hasClaudeOauth || hasLegacyOauth;
|
||||
const hasApiKey = !!credentials.api_key;
|
||||
|
||||
// Only consider this a valid credentials file if it actually contains tokens
|
||||
// An empty JSON file ({}) or file without tokens should not stop us from
|
||||
// checking subsequent credential paths
|
||||
if (hasOAuthToken || hasApiKey) {
|
||||
result.hasCredentialsFile = true;
|
||||
result.credentials = {
|
||||
hasOAuthToken,
|
||||
hasApiKey,
|
||||
};
|
||||
break; // Found valid credentials, stop searching
|
||||
}
|
||||
// File exists and is valid JSON but contains no tokens - continue checking other paths
|
||||
} catch (parseErr) {
|
||||
credCheck.error = `JSON parse error: ${parseErr instanceof Error ? parseErr.message : String(parseErr)}`;
|
||||
}
|
||||
|
||||
@@ -173,10 +173,14 @@ describe('OAuth Credential Detection', () => {
|
||||
const { getClaudeAuthIndicators } = await import('../src/system-paths');
|
||||
const indicators = await getClaudeAuthIndicators();
|
||||
|
||||
expect(indicators.hasCredentialsFile).toBe(true);
|
||||
expect(indicators.credentials).not.toBeNull();
|
||||
expect(indicators.credentials?.hasOAuthToken).toBe(false);
|
||||
expect(indicators.credentials?.hasApiKey).toBe(false);
|
||||
// Empty credentials file ({}) should NOT be treated as having credentials
|
||||
// because it contains no actual tokens. This allows the system to continue
|
||||
// checking subsequent credential paths that might have valid tokens.
|
||||
expect(indicators.hasCredentialsFile).toBe(false);
|
||||
expect(indicators.credentials).toBeNull();
|
||||
// But the file should still show as existing and readable in the checks
|
||||
expect(indicators.checks.credentialFiles[0].exists).toBe(true);
|
||||
expect(indicators.checks.credentialFiles[0].readable).toBe(true);
|
||||
});
|
||||
|
||||
it('should handle credentials file with null values', async () => {
|
||||
@@ -191,9 +195,10 @@ describe('OAuth Credential Detection', () => {
|
||||
const { getClaudeAuthIndicators } = await import('../src/system-paths');
|
||||
const indicators = await getClaudeAuthIndicators();
|
||||
|
||||
expect(indicators.hasCredentialsFile).toBe(true);
|
||||
expect(indicators.credentials?.hasOAuthToken).toBe(false);
|
||||
expect(indicators.credentials?.hasApiKey).toBe(false);
|
||||
// File with all null values should NOT be treated as having credentials
|
||||
// because null values are not valid tokens
|
||||
expect(indicators.hasCredentialsFile).toBe(false);
|
||||
expect(indicators.credentials).toBeNull();
|
||||
});
|
||||
|
||||
it('should handle credentials with empty string values', async () => {
|
||||
@@ -210,10 +215,10 @@ describe('OAuth Credential Detection', () => {
|
||||
const { getClaudeAuthIndicators } = await import('../src/system-paths');
|
||||
const indicators = await getClaudeAuthIndicators();
|
||||
|
||||
expect(indicators.hasCredentialsFile).toBe(true);
|
||||
// Empty strings should not be treated as valid credentials
|
||||
expect(indicators.credentials?.hasOAuthToken).toBe(false);
|
||||
expect(indicators.credentials?.hasApiKey).toBe(false);
|
||||
// Empty strings should NOT be treated as having credentials
|
||||
// This allows checking subsequent credential paths for valid tokens
|
||||
expect(indicators.hasCredentialsFile).toBe(false);
|
||||
expect(indicators.credentials).toBeNull();
|
||||
});
|
||||
|
||||
it('should detect settings file presence', async () => {
|
||||
@@ -337,6 +342,27 @@ describe('OAuth Credential Detection', () => {
|
||||
expect(indicators.credentials?.hasOAuthToken).toBe(true);
|
||||
expect(indicators.credentials?.hasApiKey).toBe(false);
|
||||
});
|
||||
|
||||
it('should check second credentials file if first file has no tokens', async () => {
|
||||
// Write empty/token-less content to .credentials.json (first path checked)
|
||||
// This tests the bug fix: previously, an empty JSON file would stop the search
|
||||
await fs.writeFile(path.join(mockClaudeDir, '.credentials.json'), JSON.stringify({}));
|
||||
|
||||
// Write actual credentials to credentials.json (second path)
|
||||
await fs.writeFile(
|
||||
path.join(mockClaudeDir, 'credentials.json'),
|
||||
JSON.stringify({
|
||||
api_key: 'sk-test-key-from-second-file',
|
||||
})
|
||||
);
|
||||
|
||||
const { getClaudeAuthIndicators } = await import('../src/system-paths');
|
||||
const indicators = await getClaudeAuthIndicators();
|
||||
|
||||
// Should find credentials in second file since first file has no tokens
|
||||
expect(indicators.hasCredentialsFile).toBe(true);
|
||||
expect(indicators.credentials?.hasApiKey).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getCodexAuthIndicators', () => {
|
||||
@@ -585,9 +611,9 @@ describe('OAuth Credential Detection', () => {
|
||||
const { getClaudeAuthIndicators } = await import('../src/system-paths');
|
||||
const indicators = await getClaudeAuthIndicators();
|
||||
|
||||
expect(indicators.hasCredentialsFile).toBe(true);
|
||||
expect(indicators.credentials?.hasOAuthToken).toBe(false);
|
||||
expect(indicators.credentials?.hasApiKey).toBe(false);
|
||||
// File with unexpected structure but no valid tokens should NOT be treated as having credentials
|
||||
expect(indicators.hasCredentialsFile).toBe(false);
|
||||
expect(indicators.credentials).toBeNull();
|
||||
});
|
||||
|
||||
it('should handle array instead of object in credentials', async () => {
|
||||
@@ -598,10 +624,9 @@ describe('OAuth Credential Detection', () => {
|
||||
const { getClaudeAuthIndicators } = await import('../src/system-paths');
|
||||
const indicators = await getClaudeAuthIndicators();
|
||||
|
||||
// Array is valid JSON but wrong structure - should handle gracefully
|
||||
expect(indicators.hasCredentialsFile).toBe(true);
|
||||
expect(indicators.credentials?.hasOAuthToken).toBe(false);
|
||||
expect(indicators.credentials?.hasApiKey).toBe(false);
|
||||
// Array is valid JSON but wrong structure - no valid tokens, so not treated as credentials file
|
||||
expect(indicators.hasCredentialsFile).toBe(false);
|
||||
expect(indicators.credentials).toBeNull();
|
||||
});
|
||||
|
||||
it('should handle numeric values in credential fields', async () => {
|
||||
|
||||
Reference in New Issue
Block a user