mirror of
https://github.com/tiennm99/ccs.git
synced 2026-09-02 18:18:43 +00:00
fix(cliproxy): use nickname as accountId for kiro/ghcp providers
Kiro/GHCP OAuth tokens have empty email field, causing all accounts to use accountId='default' and overwrite each other. This fix: - Add PROVIDERS_WITHOUT_EMAIL constant for kiro/ghcp identification - Require nickname for kiro/ghcp during registration (CLI + web UI) - Use nickname as accountId instead of email for these providers - Enforce nickname uniqueness to prevent collisions - Update discoverExistingAccounts() to generate unique IDs (kiro-1, etc.) Closes #258, #267
This commit is contained in:
@@ -14,6 +14,12 @@ import { CLIProxyProvider } from './types';
|
|||||||
import { getCliproxyDir, getAuthDir } from './config-generator';
|
import { getCliproxyDir, getAuthDir } from './config-generator';
|
||||||
import { PROVIDER_TYPE_VALUES } from './auth/auth-types';
|
import { PROVIDER_TYPE_VALUES } from './auth/auth-types';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Providers that typically have empty email in OAuth token files.
|
||||||
|
* For these providers, nickname is used as accountId instead of email.
|
||||||
|
*/
|
||||||
|
export const PROVIDERS_WITHOUT_EMAIL: CLIProxyProvider[] = ['kiro', 'ghcp'];
|
||||||
|
|
||||||
/** Account information */
|
/** Account information */
|
||||||
export interface AccountInfo {
|
export interface AccountInfo {
|
||||||
/** Account identifier (email or custom name) */
|
/** Account identifier (email or custom name) */
|
||||||
@@ -257,6 +263,14 @@ export function findAccountByQuery(provider: CLIProxyProvider, query: string): A
|
|||||||
/**
|
/**
|
||||||
* Register a new account
|
* Register a new account
|
||||||
* Called after successful OAuth to record the account
|
* Called after successful OAuth to record the account
|
||||||
|
*
|
||||||
|
* For providers without email (kiro, ghcp):
|
||||||
|
* - nickname is REQUIRED and used as accountId
|
||||||
|
* - Uniqueness is enforced to prevent overwriting
|
||||||
|
*
|
||||||
|
* For providers with email:
|
||||||
|
* - email is used as accountId
|
||||||
|
* - nickname is auto-generated from email if not provided
|
||||||
*/
|
*/
|
||||||
export function registerAccount(
|
export function registerAccount(
|
||||||
provider: CLIProxyProvider,
|
provider: CLIProxyProvider,
|
||||||
@@ -279,12 +293,44 @@ export function registerAccount(
|
|||||||
throw new Error('Failed to initialize provider accounts');
|
throw new Error('Failed to initialize provider accounts');
|
||||||
}
|
}
|
||||||
|
|
||||||
// Determine account ID - use email if available, otherwise extract from filename
|
// Determine account ID based on provider type
|
||||||
const accountId = extractAccountIdFromTokenFile(tokenFile, email);
|
let accountId: string;
|
||||||
const isFirstAccount = Object.keys(providerAccounts.accounts).length === 0;
|
let accountNickname: string;
|
||||||
|
|
||||||
// Generate nickname if not provided
|
if (PROVIDERS_WITHOUT_EMAIL.includes(provider)) {
|
||||||
const accountNickname = nickname || generateNickname(email);
|
// For kiro/ghcp: nickname is REQUIRED and used as accountId
|
||||||
|
if (!nickname || nickname === 'default') {
|
||||||
|
throw new Error(
|
||||||
|
`Nickname is required when adding ${provider} accounts. ` +
|
||||||
|
`Use --nickname <name> or provide a nickname in the UI.`
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Validate nickname format
|
||||||
|
const validationError = validateNickname(nickname);
|
||||||
|
if (validationError) {
|
||||||
|
throw new Error(validationError);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check uniqueness
|
||||||
|
for (const [existingId, _account] of Object.entries(providerAccounts.accounts)) {
|
||||||
|
if (existingId.toLowerCase() === nickname.toLowerCase()) {
|
||||||
|
throw new Error(
|
||||||
|
`An account with nickname "${nickname}" already exists for ${provider}. ` +
|
||||||
|
`Choose a different nickname.`
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
accountId = nickname;
|
||||||
|
accountNickname = nickname;
|
||||||
|
} else {
|
||||||
|
// For other providers: use email as accountId, fallback to filename extraction
|
||||||
|
accountId = extractAccountIdFromTokenFile(tokenFile, email);
|
||||||
|
accountNickname = nickname || generateNickname(email);
|
||||||
|
}
|
||||||
|
|
||||||
|
const isFirstAccount = Object.keys(providerAccounts.accounts).length === 0;
|
||||||
|
|
||||||
// Create or update account
|
// Create or update account
|
||||||
providerAccounts.accounts[accountId] = {
|
providerAccounts.accounts[accountId] = {
|
||||||
@@ -428,6 +474,10 @@ export function getAccountTokenPath(provider: CLIProxyProvider, accountId?: stri
|
|||||||
/**
|
/**
|
||||||
* Auto-discover accounts from existing token files
|
* Auto-discover accounts from existing token files
|
||||||
* Called during migration or first run to populate accounts registry
|
* Called during migration or first run to populate accounts registry
|
||||||
|
*
|
||||||
|
* For kiro/ghcp providers without email, generates unique accountId from:
|
||||||
|
* 1. OAuth provider + profile ID from filename (e.g., github-ABC123)
|
||||||
|
* 2. Fallback: provider + index (e.g., kiro-1, kiro-2)
|
||||||
*/
|
*/
|
||||||
export function discoverExistingAccounts(): void {
|
export function discoverExistingAccounts(): void {
|
||||||
const authDir = getAuthDir();
|
const authDir = getAuthDir();
|
||||||
@@ -478,13 +528,10 @@ export function discoverExistingAccounts(): void {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Use unified ID extraction: email or filename-based unique ID
|
|
||||||
const accountId = extractAccountIdFromTokenFile(file, email);
|
|
||||||
|
|
||||||
// Initialize provider section if needed
|
// Initialize provider section if needed
|
||||||
if (!registry.providers[provider]) {
|
if (!registry.providers[provider]) {
|
||||||
registry.providers[provider] = {
|
registry.providers[provider] = {
|
||||||
default: accountId,
|
default: 'default',
|
||||||
accounts: {},
|
accounts: {},
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -492,11 +539,45 @@ export function discoverExistingAccounts(): void {
|
|||||||
const providerAccounts = registry.providers[provider];
|
const providerAccounts = registry.providers[provider];
|
||||||
if (!providerAccounts) continue;
|
if (!providerAccounts) continue;
|
||||||
|
|
||||||
|
// Skip if token file already registered (under any accountId)
|
||||||
|
const existingTokenFiles = Object.values(providerAccounts.accounts).map((a) => a.tokenFile);
|
||||||
|
if (existingTokenFiles.includes(file)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Determine accountId based on provider type
|
||||||
|
let accountId: string;
|
||||||
|
|
||||||
|
if (PROVIDERS_WITHOUT_EMAIL.includes(provider) && !email) {
|
||||||
|
// For kiro/ghcp without email: extract from filename or generate unique
|
||||||
|
// Pattern: kiro-github-ABC123.json -> github-ABC123
|
||||||
|
const filenameId = extractAccountIdFromTokenFile(file, undefined);
|
||||||
|
|
||||||
|
if (filenameId !== 'default') {
|
||||||
|
accountId = filenameId;
|
||||||
|
} else {
|
||||||
|
// Generate unique ID: provider + incrementing index
|
||||||
|
let index = 1;
|
||||||
|
while (providerAccounts.accounts[`${provider}-${index}`]) {
|
||||||
|
index++;
|
||||||
|
}
|
||||||
|
accountId = `${provider}-${index}`;
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
// For providers with email: use email or filename extraction
|
||||||
|
accountId = extractAccountIdFromTokenFile(file, email);
|
||||||
|
}
|
||||||
|
|
||||||
// Skip if account already registered
|
// Skip if account already registered
|
||||||
if (providerAccounts.accounts[accountId]) {
|
if (providerAccounts.accounts[accountId]) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Set as default if first account
|
||||||
|
if (Object.keys(providerAccounts.accounts).length === 0) {
|
||||||
|
providerAccounts.default = accountId;
|
||||||
|
}
|
||||||
|
|
||||||
// Get file stats for creation time
|
// Get file stats for creation time
|
||||||
const stats = fs.statSync(filePath);
|
const stats = fs.statSync(filePath);
|
||||||
|
|
||||||
|
|||||||
@@ -20,6 +20,8 @@ import {
|
|||||||
getProviderAccounts,
|
getProviderAccounts,
|
||||||
getDefaultAccount,
|
getDefaultAccount,
|
||||||
touchAccount,
|
touchAccount,
|
||||||
|
PROVIDERS_WITHOUT_EMAIL,
|
||||||
|
validateNickname,
|
||||||
} from '../account-manager';
|
} from '../account-manager';
|
||||||
import {
|
import {
|
||||||
enhancedPreflightOAuthCheck,
|
enhancedPreflightOAuthCheck,
|
||||||
@@ -49,6 +51,66 @@ async function promptAddAccount(): Promise<boolean> {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Prompt user for account nickname (required for kiro/ghcp)
|
||||||
|
* Returns null if user cancels
|
||||||
|
*/
|
||||||
|
async function promptNickname(
|
||||||
|
provider: CLIProxyProvider,
|
||||||
|
existingAccounts: AccountInfo[]
|
||||||
|
): Promise<string | null> {
|
||||||
|
const readline = await import('readline');
|
||||||
|
const rl = readline.createInterface({
|
||||||
|
input: process.stdin,
|
||||||
|
output: process.stdout,
|
||||||
|
});
|
||||||
|
|
||||||
|
const existingNicknames = existingAccounts.map(
|
||||||
|
(a) => a.nickname?.toLowerCase() || a.id.toLowerCase()
|
||||||
|
);
|
||||||
|
|
||||||
|
console.log('');
|
||||||
|
console.log(info(`${provider} accounts require a unique nickname to distinguish them.`));
|
||||||
|
if (existingNicknames.length > 0) {
|
||||||
|
console.log(` Existing: ${existingNicknames.join(', ')}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
return new Promise<string | null>((resolve) => {
|
||||||
|
// Handle Ctrl+C gracefully
|
||||||
|
rl.on('close', () => resolve(null));
|
||||||
|
|
||||||
|
const askForNickname = () => {
|
||||||
|
rl.question('[?] Enter a nickname for this account: ', (answer) => {
|
||||||
|
const nickname = answer.trim();
|
||||||
|
|
||||||
|
if (!nickname) {
|
||||||
|
console.log(fail('Nickname cannot be empty'));
|
||||||
|
askForNickname();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const validationError = validateNickname(nickname);
|
||||||
|
if (validationError) {
|
||||||
|
console.log(fail(validationError));
|
||||||
|
askForNickname();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (existingNicknames.includes(nickname.toLowerCase())) {
|
||||||
|
console.log(fail(`Nickname "${nickname}" is already in use. Choose a different one.`));
|
||||||
|
askForNickname();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
rl.close();
|
||||||
|
resolve(nickname);
|
||||||
|
});
|
||||||
|
};
|
||||||
|
|
||||||
|
askForNickname();
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Run pre-flight OAuth checks
|
* Run pre-flight OAuth checks
|
||||||
*/
|
*/
|
||||||
@@ -127,7 +189,21 @@ export async function triggerOAuth(
|
|||||||
options: OAuthOptions = {}
|
options: OAuthOptions = {}
|
||||||
): Promise<AccountInfo | null> {
|
): Promise<AccountInfo | null> {
|
||||||
const oauthConfig = getOAuthConfig(provider);
|
const oauthConfig = getOAuthConfig(provider);
|
||||||
const { verbose = false, add = false, nickname, fromUI = false, noIncognito = true } = options;
|
const { verbose = false, add = false, fromUI = false, noIncognito = true } = options;
|
||||||
|
let { nickname } = options;
|
||||||
|
|
||||||
|
// Check for existing accounts
|
||||||
|
const existingAccounts = getProviderAccounts(provider);
|
||||||
|
|
||||||
|
// For kiro/ghcp: require nickname if not provided (CLI only, not fromUI)
|
||||||
|
if (PROVIDERS_WITHOUT_EMAIL.includes(provider) && !nickname && !fromUI) {
|
||||||
|
const promptedNickname = await promptNickname(provider, existingAccounts);
|
||||||
|
if (!promptedNickname) {
|
||||||
|
console.log(info('Cancelled'));
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
nickname = promptedNickname;
|
||||||
|
}
|
||||||
|
|
||||||
// Handle --import flag: skip OAuth and import from Kiro IDE directly
|
// Handle --import flag: skip OAuth and import from Kiro IDE directly
|
||||||
if (options.import && provider === 'kiro') {
|
if (options.import && provider === 'kiro') {
|
||||||
@@ -144,8 +220,6 @@ export async function triggerOAuth(
|
|||||||
const headless = options.headless ?? isHeadlessEnvironment();
|
const headless = options.headless ?? isHeadlessEnvironment();
|
||||||
const isDeviceCodeFlow = callbackPort === null;
|
const isDeviceCodeFlow = callbackPort === null;
|
||||||
|
|
||||||
// Check for existing accounts
|
|
||||||
const existingAccounts = getProviderAccounts(provider);
|
|
||||||
if (existingAccounts.length > 0 && !add) {
|
if (existingAccounts.length > 0 && !add) {
|
||||||
console.log('');
|
console.log('');
|
||||||
console.log(
|
console.log(
|
||||||
|
|||||||
@@ -24,6 +24,8 @@ import {
|
|||||||
setDefaultAccount as setDefaultAccountFn,
|
setDefaultAccount as setDefaultAccountFn,
|
||||||
removeAccount as removeAccountFn,
|
removeAccount as removeAccountFn,
|
||||||
touchAccount,
|
touchAccount,
|
||||||
|
PROVIDERS_WITHOUT_EMAIL,
|
||||||
|
validateNickname,
|
||||||
} from '../../cliproxy/account-manager';
|
} from '../../cliproxy/account-manager';
|
||||||
import { getProxyTarget } from '../../cliproxy/proxy-target-resolver';
|
import { getProxyTarget } from '../../cliproxy/proxy-target-resolver';
|
||||||
import { fetchRemoteAuthStatus } from '../../cliproxy/remote-auth-fetcher';
|
import { fetchRemoteAuthStatus } from '../../cliproxy/remote-auth-fetcher';
|
||||||
@@ -283,6 +285,39 @@ router.post('/:provider/start', async (req: Request, res: Response): Promise<voi
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// For kiro/ghcp: nickname is required
|
||||||
|
if (PROVIDERS_WITHOUT_EMAIL.includes(provider as CLIProxyProvider)) {
|
||||||
|
if (!nickname) {
|
||||||
|
res.status(400).json({
|
||||||
|
error: `Nickname is required for ${provider} accounts. Please provide a unique nickname.`,
|
||||||
|
code: 'NICKNAME_REQUIRED',
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const validationError = validateNickname(nickname);
|
||||||
|
if (validationError) {
|
||||||
|
res.status(400).json({
|
||||||
|
error: validationError,
|
||||||
|
code: 'INVALID_NICKNAME',
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check uniqueness
|
||||||
|
const existingAccounts = getProviderAccounts(provider as CLIProxyProvider);
|
||||||
|
const existingNicknames = existingAccounts.map(
|
||||||
|
(a) => a.nickname?.toLowerCase() || a.id.toLowerCase()
|
||||||
|
);
|
||||||
|
if (existingNicknames.includes(nickname.toLowerCase())) {
|
||||||
|
res.status(400).json({
|
||||||
|
error: `Nickname "${nickname}" is already in use. Choose a different one.`,
|
||||||
|
code: 'NICKNAME_EXISTS',
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Check Kiro no-incognito setting from config (or request body)
|
// Check Kiro no-incognito setting from config (or request body)
|
||||||
// Default to true (use normal browser) for reliability - incognito often fails
|
// Default to true (use normal browser) for reliability - incognito often fails
|
||||||
let noIncognito = true;
|
let noIncognito = true;
|
||||||
|
|||||||
@@ -164,7 +164,7 @@ describe('Account Manager - discoverExistingAccounts', () => {
|
|||||||
assert.strictEqual(accountIds[0], 'actual@email.com', 'Should use data.email when available');
|
assert.strictEqual(accountIds[0], 'actual@email.com', 'Should use data.email when available');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('falls back to "default" when no email in file or filename', () => {
|
it('generates unique ID for kiro/ghcp when no email in file or filename', () => {
|
||||||
createAuthFile('kiro-nomail.json', {
|
createAuthFile('kiro-nomail.json', {
|
||||||
type: 'kiro',
|
type: 'kiro',
|
||||||
email: '',
|
email: '',
|
||||||
@@ -174,7 +174,8 @@ describe('Account Manager - discoverExistingAccounts', () => {
|
|||||||
const accounts = getAccountsFile();
|
const accounts = getAccountsFile();
|
||||||
|
|
||||||
const accountIds = Object.keys(accounts.providers.kiro.accounts);
|
const accountIds = Object.keys(accounts.providers.kiro.accounts);
|
||||||
assert.strictEqual(accountIds[0], 'default', 'Should fall back to "default"');
|
// For kiro/ghcp without email, generates unique ID like "kiro-1"
|
||||||
|
assert.strictEqual(accountIds[0], 'kiro-1', 'Should generate unique ID for kiro without email');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('handles dots in email local part', () => {
|
it('handles dots in email local part', () => {
|
||||||
@@ -274,6 +275,72 @@ describe('Account Manager - discoverExistingAccounts', () => {
|
|||||||
'First account should be default'
|
'First account should be default'
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('generates sequential IDs for multiple kiro files without email', () => {
|
||||||
|
// Create multiple kiro files without email in filename
|
||||||
|
// Files like "kiro-nomail.json" don't match oauth pattern (need 2+ hyphens)
|
||||||
|
createAuthFile('kiro-account1.json', {
|
||||||
|
type: 'kiro',
|
||||||
|
email: '',
|
||||||
|
});
|
||||||
|
createAuthFile('kiro-account2.json', {
|
||||||
|
type: 'kiro',
|
||||||
|
email: '',
|
||||||
|
});
|
||||||
|
|
||||||
|
accountManager.discoverExistingAccounts();
|
||||||
|
const accounts = getAccountsFile();
|
||||||
|
|
||||||
|
const accountIds = Object.keys(accounts.providers.kiro.accounts);
|
||||||
|
assert.strictEqual(accountIds.length, 2, 'Should have 2 accounts');
|
||||||
|
// Both should have kiro-N format since filenames don't match oauth pattern (1 hyphen only)
|
||||||
|
assert(accountIds.includes('kiro-1'), 'Should have kiro-1');
|
||||||
|
assert(accountIds.includes('kiro-2'), 'Should have kiro-2');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('skips to next ID when collision exists', () => {
|
||||||
|
// Pre-create accounts.json with kiro-1 already registered
|
||||||
|
const accountsPath = path.join(testDir, '.ccs', 'cliproxy', 'accounts.json');
|
||||||
|
const existingRegistry = {
|
||||||
|
version: 1,
|
||||||
|
providers: {
|
||||||
|
kiro: {
|
||||||
|
default: 'kiro-1',
|
||||||
|
accounts: {
|
||||||
|
'kiro-1': {
|
||||||
|
tokenFile: 'kiro-existing.json',
|
||||||
|
createdAt: new Date().toISOString(),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
fs.writeFileSync(accountsPath, JSON.stringify(existingRegistry));
|
||||||
|
|
||||||
|
// Create the existing token file (matches registry)
|
||||||
|
createAuthFile('kiro-existing.json', {
|
||||||
|
type: 'kiro',
|
||||||
|
email: '',
|
||||||
|
});
|
||||||
|
|
||||||
|
// Create new file that will need auto-generated ID (single hyphen = no oauth pattern match)
|
||||||
|
createAuthFile('kiro-newaccount.json', {
|
||||||
|
type: 'kiro',
|
||||||
|
email: '',
|
||||||
|
});
|
||||||
|
|
||||||
|
// Reload module to pick up pre-existing accounts.json
|
||||||
|
delete require.cache[require.resolve('../../../dist/cliproxy/account-manager')];
|
||||||
|
accountManager = require('../../../dist/cliproxy/account-manager');
|
||||||
|
accountManager.discoverExistingAccounts();
|
||||||
|
|
||||||
|
const accounts = getAccountsFile();
|
||||||
|
const accountIds = Object.keys(accounts.providers.kiro.accounts);
|
||||||
|
|
||||||
|
assert.strictEqual(accountIds.length, 2, 'Should have 2 accounts');
|
||||||
|
assert(accountIds.includes('kiro-1'), 'Should keep existing kiro-1');
|
||||||
|
assert(accountIds.includes('kiro-2'), 'New account should be kiro-2 (skipped kiro-1)');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
// =========================================================================
|
// =========================================================================
|
||||||
|
|||||||
Reference in New Issue
Block a user