feat: skip caching token in keychain mode for accounts cache

src/commands/accounts/add.ts

@@ -30,7 +30,14 @@ export default class Add extends Command {
     }
 
     const token = this.heroku.auth!
-
-    AccountsModule.add(name, email, token)
+    const config = AccountsModule.getStorageConfig()
+
+    if (config.credentialStore) {
+      // Keychain-mode: don't save token to cache file
+      AccountsModule.add(name, email)
+    } else {
+      // Netrc-mode: save token to cache file
+      AccountsModule.add(name, email, token)
+    }
   }
 }

src/lib/accounts/accounts.ts

@@ -16,7 +16,7 @@ export interface AccountEntry {
 }
 
 export interface IAccountsWrapper {
-  add(name: string, username: string, password: string): void
+  add(name: string, username: string, password?: string): void
   current(heroku: APIClient): Promise<null | string>
   currentNetrc(): Promise<null | string>
   getStorageConfig(): ReturnType<typeof getStorageConfig>
@@ -29,11 +29,14 @@ export interface IAccountsWrapper {
 export class AccountsWrapper implements IAccountsWrapper {
   private netrc: any
 
-  add(name: string, username: string, password: string): void {
+  add(name: string, username: string, password?: string): void {
     fs.mkdirSync(this.accountsDir(), {recursive: true})
 
-    // eslint-disable-next-line perfectionist/sort-objects
-    this.writeAccountFile(name, {username, password})
+    const content: Record<string, string> = password === undefined
+      ? {username}
+      : {username, password} // eslint-disable-line perfectionist/sort-objects
+
+    this.writeAccountFile(name, content)
   }
 
   async current(heroku: APIClient): Promise<null | string> {
@@ -92,12 +95,17 @@ export class AccountsWrapper implements IAccountsWrapper {
 
     if (account.name) {
       if (config.credentialStore) {
+        // Keychain mode: only update login state, skip netrc
         const email = this.getAliasEmail(account.name)
-        if (email) {
-          await this.writeLoginState(dataDir, email)
+        if (!email) {
+          throw new Error(`We can't find the alias file for ${account.name}.`)
         }
+
+        await this.writeLoginState(dataDir, email)
+        return
       }
 
+      // Netrc mode: update both login state and netrc files
       const netrcInstance = await this.initNetrc()
       let current
       try {

test/unit/commands/accounts/add.unit.test.ts

@@ -26,7 +26,7 @@ describe('accounts:add', function () {
   })
 
   describe('when the user is logged in', function () {
-    it('should call the accounts.add function with the account name, user email, and auth token', async function () {
+    it('should call the accounts.add function with the account name and user email', async function () {
       stubCredentialManager({
         getAuth: async () => ({account: 'testEmail', token: 'testHerokuAPIKey'}),
       })
@@ -38,6 +38,43 @@ describe('accounts:add', function () {
       expect(addStub.calledOnce).to.equal(true)
       expect(addStub.args[0][0]).to.equal('testAccountName')
       expect(addStub.args[0][1]).to.equal('testEmail')
+    })
+
+    it('should not pass token to add() when credentialStore is active (keychain-mode)', async function () {
+      stubCredentialManager({
+        getAuth: async () => ({account: 'testEmail', token: 'testHerokuAPIKey'}),
+      })
+
+      const getStorageConfigStub = stub(AccountsModule, 'getStorageConfig')
+      getStorageConfigStub.returns({credentialStore: 'keychain' as any, useNetrc: false})
+
+      api.get('/account')
+        .reply(200, {email: 'testEmail'})
+
+      await runCommand(Cmd, ['testAccountName'])
+
+      expect(addStub.calledOnce).to.equal(true)
+      expect(addStub.args[0][0]).to.equal('testAccountName')
+      expect(addStub.args[0][1]).to.equal('testEmail')
+      expect(addStub.args[0][2]).to.be.undefined
+    })
+
+    it('should pass token to add() when credentialStore is not active (netrc-mode)', async function () {
+      stubCredentialManager({
+        getAuth: async () => ({account: 'testEmail', token: 'testHerokuAPIKey'}),
+      })
+
+      const getStorageConfigStub = stub(AccountsModule, 'getStorageConfig')
+      getStorageConfigStub.returns({credentialStore: null, useNetrc: true})
+
+      api.get('/account')
+        .reply(200, {email: 'testEmail'})
+
+      await runCommand(Cmd, ['testAccountName'])
+
+      expect(addStub.calledOnce).to.equal(true)
+      expect(addStub.args[0][0]).to.equal('testAccountName')
+      expect(addStub.args[0][1]).to.equal('testEmail')
       expect(addStub.args[0][2]).to.equal('testHerokuAPIKey')
     })
 

test/unit/lib/accounts/accounts.unit.test.ts

@@ -119,6 +119,22 @@ describe('accounts', function () {
 
       expect(() => AccountsModule.add('test-user', 'username123', 'password123')).to.throw()
     })
+
+    it('should write only username when password is undefined (keychain-mode)', function () {
+      AccountsModule.add('test-user', 'username123')
+
+      expect(writeFileSyncStub.calledOnce).to.be.true
+      expect(writeFileSyncStub.firstCall.args[1]).to.equal('username: username123\n')
+      expect(writeFileSyncStub.firstCall.args[2]).to.equal('utf8')
+    })
+
+    it('should write username and password when password is provided (netrc-mode)', function () {
+      AccountsModule.add('test-user', 'username123', 'password123')
+
+      expect(writeFileSyncStub.calledOnce).to.be.true
+      expect(writeFileSyncStub.firstCall.args[1]).to.equal('username: username123\npassword: password123\n')
+      expect(writeFileSyncStub.firstCall.args[2]).to.equal('utf8')
+    })
   })
 
   describe('set()', function () {
@@ -145,7 +161,7 @@ describe('accounts', function () {
         setNetrc(null as unknown as typeof fakeNetrc)
       })
 
-      it('calls writeLoginState with email from alias file and writes to netrc', async function () {
+      it('calls writeLoginState with email from alias file but does not write to netrc', async function () {
         const account = {name: 'production', username: 'prod@example.com'}
         existsSyncStub.withArgs('/user/home/.config/heroku').returns(false)
         existsSyncStub.withArgs(match(/production$/)).returns(true)
@@ -157,9 +173,9 @@ describe('accounts', function () {
         expect(writeLoginStateStub.calledOnce).to.be.true
         expect(writeLoginStateStub.firstCall.args[0]).to.equal('/data/heroku')
         expect(writeLoginStateStub.firstCall.args[1]).to.equal('prod@example.com')
-        expect(fakeNetrc.machines['api.heroku.com']).to.deep.equal({login: 'prod@example.com', password: 'secret'})
-        expect(fakeNetrc.machines['git.heroku.com']).to.deep.equal({login: 'prod@example.com', password: 'secret'})
-        expect(fakeNetrc.save.calledOnce).to.be.true
+        expect(fakeNetrc.machines['api.heroku.com']).to.be.undefined
+        expect(fakeNetrc.machines['git.heroku.com']).to.be.undefined
+        expect(fakeNetrc.save.called).to.be.false
       })
 
       it('throws error when alias file does not exist', async function () {