diff --git a/package-lock.json b/package-lock.json index 3c093bfc7a..3d5b096ed9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "version": "11.2.0", "license": "ISC", "dependencies": { - "@heroku-cli/command": "13.0.0-beta.1", + "@heroku-cli/command": "13.0.0-beta.3", "@heroku-cli/notifications": "^1.2.6", "@heroku-cli/schema": "^1.0.25", "@heroku/buildpack-registry": "^1.0.1", @@ -2142,9 +2142,9 @@ } }, "node_modules/@heroku-cli/command": { - "version": "13.0.0-beta.1", - "resolved": "https://registry.npmjs.org/@heroku-cli/command/-/command-13.0.0-beta.1.tgz", - "integrity": "sha512-mKJ7uXQivhLO9p8/iTg+nUh6Kj9elwzSt7jGM3Y71vkaCmrfRkErQfiuSUajNERTqQ/lK/fci/meZw433h03vw==", + "version": "13.0.0-beta.3", + "resolved": "https://registry.npmjs.org/@heroku-cli/command/-/command-13.0.0-beta.3.tgz", + "integrity": "sha512-vrUNs/7Yp9E2w7I8CUTSork9s++wIBQuVFc5s+a3yyzPf4pnFYmGpadHoCz+TXhwfqJy+MMTehCmMEiyPWM67Q==", "license": "ISC", "dependencies": { "@heroku/http-call": "^5.4.0", diff --git a/package.json b/package.json index 4b32097364..9b9fd07b2d 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "bin": "./bin/run.js", "bugs": "https://github.com/heroku/cli/issues", "dependencies": { - "@heroku-cli/command": "13.0.0-beta.1", + "@heroku-cli/command": "13.0.0-beta.3", "@heroku-cli/notifications": "^1.2.6", "@heroku-cli/schema": "^1.0.25", "@heroku/buildpack-registry": "^1.0.1", @@ -378,7 +378,7 @@ "license": "ISC", "main": "dist/index.js", "overrides": { - "@heroku-cli/command": "13.0.0-beta.1", + "@heroku-cli/command": "13.0.0-beta.3", "serialize-javascript": "^7.0.4" }, "repository": "heroku/cli", diff --git a/src/commands/accounts/add.ts b/src/commands/accounts/add.ts index 9bc7bfa19f..42a882b93e 100644 --- a/src/commands/accounts/add.ts +++ b/src/commands/accounts/add.ts @@ -19,7 +19,7 @@ export default class Add extends Command { const {name} = args const logInMessage = 'You must be logged in to run this command.' - if (AccountsModule.list().some(a => a.name === name)) { + if ((await AccountsModule.list()).some(account => account.name === name)) { ux.error(`${name} already exists`) } diff --git a/src/commands/accounts/current.ts b/src/commands/accounts/current.ts index 7b88043c6f..3b24113ea4 100644 --- a/src/commands/accounts/current.ts +++ b/src/commands/accounts/current.ts @@ -11,7 +11,7 @@ export default class Current extends Command { static promptFlagActive = false async run() { - const accountName = await AccountsModule.current() + const accountName = await AccountsModule.current(this.heroku) if (accountName) { hux.styledHeader(`Current account is ${color.user(accountName)}`) } else { diff --git a/src/commands/accounts/index.ts b/src/commands/accounts/index.ts index 9824f4132f..ae2558cc18 100644 --- a/src/commands/accounts/index.ts +++ b/src/commands/accounts/index.ts @@ -11,16 +11,17 @@ export default class AccountsIndex extends Command { static promptFlagActive = false async run() { - const accounts = accountsModule.list() + const accounts = await accountsModule.list() if (accounts.length === 0) { ux.error('You don\'t have any accounts in your cache.') } + const current = await accountsModule.current(this.heroku) for (const account of accounts) { - if (account.name === await accountsModule.current()) { - ux.stdout(`* ${account.name}`) + if (account.name === current || account.username === current) { + ux.stdout(`* ${account.name ?? account.username}`) } else { - ux.stdout(` ${account.name}`) + ux.stdout(` ${account.name ?? account.username}`) } } } diff --git a/src/commands/accounts/remove.ts b/src/commands/accounts/remove.ts index 6fd64289f2..7d4c3f9357 100644 --- a/src/commands/accounts/remove.ts +++ b/src/commands/accounts/remove.ts @@ -17,11 +17,11 @@ export default class Remove extends Command { const {args} = await this.parse(Remove) const {name} = args - if (!AccountsModule.list().some(a => a.name === name)) { + if (!(await AccountsModule.list()).some(account => account.name === name)) { ux.error(`${name} doesn't exist in your accounts cache.`) } - if (await AccountsModule.current() === name) { + if (await AccountsModule.current(this.heroku) === name) { ux.error(`${name} is the current account.`) } diff --git a/src/commands/accounts/set.ts b/src/commands/accounts/set.ts index a8398c476a..e2bb76f8a5 100644 --- a/src/commands/accounts/set.ts +++ b/src/commands/accounts/set.ts @@ -17,7 +17,7 @@ export default class Set extends Command { const {args} = await this.parse(Set) const {name} = args - if (!AccountsModule.list().some(a => a.name === name)) { + if (!(await AccountsModule.list()).some(account => account.name === name)) { ux.error(`${name} does not exist in your accounts cache.`) } diff --git a/src/commands/git/credentials.ts b/src/commands/git/credentials.ts index b844c5ce4b..c253a77d59 100644 --- a/src/commands/git/credentials.ts +++ b/src/commands/git/credentials.ts @@ -20,7 +20,6 @@ export class GitCredentials extends Command { return new Promise(resolve => { const rl = readline.createInterface({ input: process.stdin, - output: process.stdout, terminal: false, }) @@ -28,7 +27,7 @@ export class GitCredentials extends Command { rl.on('line', (line: string) => { if (!line.trim()) { - rl.close() + setImmediate(() => rl.close()) return } diff --git a/src/lib/accounts/accounts.ts b/src/lib/accounts/accounts.ts index ea2530a977..6dd51c5757 100644 --- a/src/lib/accounts/accounts.ts +++ b/src/lib/accounts/accounts.ts @@ -1,12 +1,18 @@ -import {parse, stringify} from 'yaml' +import {APIClient, listKeychainAccounts, getStorageConfig} from '@heroku-cli/command' +import * as Heroku from '@heroku-cli/schema' import fs from 'node:fs' import os from 'node:os' import path from 'node:path' -import * as Heroku from '@heroku-cli/schema' +import {parse, stringify} from 'yaml' + +export interface AccountEntry { + name?: string + username: string +} export interface IAccountsWrapper { - list(): Heroku.Account[] | [] - current(): Promise + list(): Promise + current(heroku: APIClient): Promise add(name: string, username: string, password: string): void remove(name: string): void set(name: string): Promise @@ -50,21 +56,47 @@ export class AccountsWrapper implements IAccountsWrapper { return account } - list(): Heroku.Account[] | [] { + async getKeychainAccounts(): Promise<(string | null | undefined)[]> { + return listKeychainAccounts() + } + + getStorageConfig() { + return getStorageConfig() + } + + async list(): Promise { + const config = this.getStorageConfig() + if (config.credentialStore) { + const accounts = await this.getKeychainAccounts() + return accounts + .filter((account): account is string => account !== null && account !== undefined) + .map(account => ({username: account})) + } + + return this.listNetrc() + } + + listNetrc(): AccountEntry[] { const basedir = path.join(this.configDir(), 'accounts') try { return fs.readdirSync(basedir) - .map(name => Object.assign(this.account(name), {name})) + .map(name => ({name, username: this.account(name).username ?? ''})) } catch { return [] } } - async current(): Promise { + async current(heroku: APIClient): Promise { + const config = getStorageConfig() + if (config.credentialStore) { + const authEntry = await heroku.getAuthEntry() + return authEntry?.account ?? null + } + const netrcInstance = await this.initNetrc() if (netrcInstance.machines['api.heroku.com']) { - const current = this.list().find(a => a.username === netrcInstance.machines['api.heroku.com'].login) - return current && current.name ? current.name : null + const current = this.listNetrc().find(a => a.username === netrcInstance.machines['api.heroku.com'].login) + return current?.name ?? null } return null diff --git a/test/unit/commands/accounts/add.unit.test.ts b/test/unit/commands/accounts/add.unit.test.ts index 1b8180c1d1..4716beded7 100644 --- a/test/unit/commands/accounts/add.unit.test.ts +++ b/test/unit/commands/accounts/add.unit.test.ts @@ -10,7 +10,7 @@ describe('accounts:add', function () { let addStub: sinon.SinonStub beforeEach(function () { - sinon.stub(AccountsModule, 'list').returns([]) + sinon.stub(AccountsModule, 'list').resolves([]) addStub = sinon.stub(AccountsModule, 'add') api = nock('https://api.heroku.com') }) diff --git a/test/unit/commands/accounts/current.unit.test.ts b/test/unit/commands/accounts/current.unit.test.ts index 74ae6f89ef..cda211f5d5 100644 --- a/test/unit/commands/accounts/current.unit.test.ts +++ b/test/unit/commands/accounts/current.unit.test.ts @@ -19,13 +19,13 @@ describe('accounts:current', function () { }) it('should print the name of the current account if an account is found', async function () { - currentStub.returns('test-account') + currentStub.resolves('test-account') await runCommand(Cmd, []) expect(stdout.output).to.contain('test-account') }) it('should print an error message if no account is found', async function () { - currentStub.returns(null) + currentStub.resolves(null) await runCommand(Cmd, []) .catch((error: Error) => { expect(ansis.strip(error.message)).to.equal('You haven\'t set an account. Run heroku accounts:add to add an account to your cache or heroku accounts:set to set the current account.') diff --git a/test/unit/commands/accounts/index.unit.test.ts b/test/unit/commands/accounts/index.unit.test.ts index a174d1bcf2..2112891874 100644 --- a/test/unit/commands/accounts/index.unit.test.ts +++ b/test/unit/commands/accounts/index.unit.test.ts @@ -20,13 +20,13 @@ describe('accounts', function () { it('should print a list of added accounts with the current account highlighted if accounts are found', async function () { currentStub.resolves('test-account') - listStub.returns([{name: 'test-account'}, {name: 'test-account-2'}]) + listStub.resolves([{name: 'test-account', username: 'user1'}, {name: 'test-account-2', username: 'user2'}]) await runCommand(Cmd, []) expect(stdout.output).to.equal('* test-account\n test-account-2\n') }) it('should print an error message if no accounts are found', async function () { - listStub.returns([]) + listStub.resolves([]) await runCommand(Cmd, []) .catch((error: Error) => { expect(error.message).to.contain('You don\'t have any accounts in your cache.') diff --git a/test/unit/commands/accounts/remove.unit.test.ts b/test/unit/commands/accounts/remove.unit.test.ts index 51498acefb..e03c2c89fd 100644 --- a/test/unit/commands/accounts/remove.unit.test.ts +++ b/test/unit/commands/accounts/remove.unit.test.ts @@ -20,15 +20,15 @@ describe('accounts:remove', function () { }) it('calls the remove function with the account name when the account exists and it is not the current account', async function () { - currentStub.returns('test-account') - listStub.returns([{name: 'test-account'}, {name: 'test-account-2'}]) + currentStub.resolves('test-account') + listStub.resolves([{name: 'test-account', username: 'user1'}, {name: 'test-account-2', username: 'user2'}]) await runCommand(Cmd, ['test-account-2']) expect(removeStub.calledWith('test-account-2')) }) it('should return an error if the selected account name is not included in the account list', async function () { - currentStub.returns('test-account') - listStub.returns([{name: 'test-account'}, {name: 'test-account-2'}]) + currentStub.resolves('test-account') + listStub.resolves([{name: 'test-account', username: 'user1'}, {name: 'test-account-2', username: 'user2'}]) await runCommand(Cmd, ['test-account-3']) .catch((error: Error) => { expect(error.message).to.contain('test-account-3 doesn\'t exist in your accounts cache.') @@ -36,8 +36,8 @@ describe('accounts:remove', function () { }) it('should return an error if the selected account name is the current account', async function () { - currentStub.returns('test-account') - listStub.returns([{name: 'test-account'}, {name: 'test-account-2'}]) + currentStub.resolves('test-account') + listStub.resolves([{name: 'test-account', username: 'user1'}, {name: 'test-account-2', username: 'user2'}]) await runCommand(Cmd, ['test-account']) .catch((error: Error) => { expect(error.message).to.contain('test-account is the current account.') diff --git a/test/unit/commands/accounts/set.unit.test.ts b/test/unit/commands/accounts/set.unit.test.ts index f343446c71..d149e8b2ac 100644 --- a/test/unit/commands/accounts/set.unit.test.ts +++ b/test/unit/commands/accounts/set.unit.test.ts @@ -18,13 +18,13 @@ describe('accounts:set', function () { }) it('calls the set function with the account name when the account exists', async function () { - listStub.returns([{name: 'test-account'}, {name: 'test-account-2'}]) + listStub.resolves([{name: 'test-account', username: 'user1'}, {name: 'test-account-2', username: 'user2'}]) await runCommand(Cmd, ['test-account-2']) expect(setStub.calledWith('test-account-2')) }) it('should return an error if the selected account name is not included in the account list', async function () { - listStub.returns([{name: 'test-account'}, {name: 'test-account-2'}]) + listStub.resolves([{name: 'test-account', username: 'user1'}, {name: 'test-account-2', username: 'user2'}]) await runCommand(Cmd, ['test-account-3']) .catch((error: Error) => { expect(error.message).to.contain('test-account-3 does not exist in your accounts cache.') diff --git a/test/unit/lib/accounts/accounts.unit.test.ts b/test/unit/lib/accounts/accounts.unit.test.ts index 5a24c1811f..42668dc264 100644 --- a/test/unit/lib/accounts/accounts.unit.test.ts +++ b/test/unit/lib/accounts/accounts.unit.test.ts @@ -11,61 +11,107 @@ describe('accounts', function () { let fsReadFileStub: sinon.SinonStub beforeEach(function () { + process.env.HEROKU_NETRC_WRITE = 'true' fsReaddirStub = sinon.stub(fs, 'readdirSync') fsReadFileStub = sinon.stub(fs, 'readFileSync') }) afterEach(function () { + delete process.env.HEROKU_NETRC_WRITE + delete process.env.HEROKU_NATIVE_STORE_WRITE sinon.restore() - // fs.unlinkSync(tmpNetrc) }) describe('list()', function () { - it('should return an empty array when directory is not accessible', function () { + it('should return an empty array when directory is not accessible', async function () { fsReaddirStub.throws(new Error('Directory not found')) - const result = AccountsModule.list() + const result = await AccountsModule.list() expect(result).to.be.an('array').that.is.empty }) - it('should return array of account objects when files exist', function () { + it('should return array of AccountEntry objects when files exist', async function () { fsReaddirStub.returns(['account1', 'account2']) fsReadFileStub.withArgs(sinon.match(/account1$/), 'utf8') - .returns('{"username": "user1", "password": "pass1"}') + .returns('username: user1\npassword: pass1\n') fsReadFileStub.withArgs(sinon.match(/account2$/), 'utf8') - .returns('{"username": "user2", "password": "pass2"}') + .returns('username: user2\npassword: pass2\n') - const result = AccountsModule.list() + const result = await AccountsModule.list() expect(result).to.be.an('array') expect(result).to.have.lengthOf(2) - expect(result[0]).to.deep.include({ - name: 'account1', - username: 'user1', - password: 'pass1', - }) - expect(result[1]).to.deep.include({ - name: 'account2', - username: 'user2', - password: 'pass2', - }) - }) - - it('should handle ruby-style symbol keys', function () { + expect(result[0]).to.deep.equal({name: 'account1', username: 'user1'}) + expect(result[1]).to.deep.equal({name: 'account2', username: 'user2'}) + }) + + it('should handle ruby-style symbol keys', async function () { fsReaddirStub.returns(['account1']) fsReadFileStub.withArgs(sinon.match(/account1$/), 'utf8') .returns('{":username": "user1", ":password": "pass1"}') - const result = AccountsModule.list() + const result = await AccountsModule.list() expect(result).to.be.an('array') expect(result).to.have.lengthOf(1) - expect(result[0]).to.deep.include({ - name: 'account1', - username: 'user1', - password: 'pass1', - }) - expect(result[0]).to.not.have.property(':username') - expect(result[0]).to.not.have.property(':password') + expect(result[0]).to.deep.equal({name: 'account1', username: 'user1'}) + }) + }) + + describe('list() with credentialStore', function () { + beforeEach(function () { + delete process.env.HEROKU_NETRC_WRITE + sinon.stub(AccountsModule, 'getStorageConfig').returns({credentialStore: 'keychain' as any, useNetrc: false}) + }) + + it('should return AccountEntry objects with only username when credentialStore is set', async function () { + sinon.stub(AccountsModule, 'getKeychainAccounts').resolves(['user1@example.com', 'user2@example.com']) + + const result = await AccountsModule.list() + + expect(result).to.be.an('array').that.has.lengthOf(2) + expect(result[0]).to.deep.equal({username: 'user1@example.com'}) + expect(result[1]).to.deep.equal({username: 'user2@example.com'}) + }) + + it('should return an empty array when the keychain has no accounts', async function () { + sinon.stub(AccountsModule, 'getKeychainAccounts').resolves([]) + + const result = await AccountsModule.list() + + expect(result).to.be.an('array').that.is.empty + }) + + it('should filter out null and undefined keychain entries', async function () { + sinon.stub(AccountsModule, 'getKeychainAccounts').resolves(['user1@example.com', null, undefined, 'user2@example.com']) + + const result = await AccountsModule.list() + + expect(result).to.have.lengthOf(2) + expect(result[0]).to.deep.equal({username: 'user1@example.com'}) + expect(result[1]).to.deep.equal({username: 'user2@example.com'}) + }) + }) + + describe('listNetrc()', function () { + it('should return an empty array when directory is not accessible', function () { + fsReaddirStub.throws(new Error('Directory not found')) + const result = AccountsModule.listNetrc() + expect(result).to.be.an('array').that.is.empty + }) + + it('should return array of AccountEntry objects with name and username', function () { + fsReaddirStub.returns(['account1', 'account2']) + fsReadFileStub.withArgs(sinon.match(/account1$/), 'utf8') + .returns('username: user1\npassword: pass1\n') + fsReadFileStub.withArgs(sinon.match(/account2$/), 'utf8') + .returns('username: user2\npassword: pass2\n') + + const result = AccountsModule.listNetrc() + + expect(result).to.be.an('array') + expect(result).to.have.lengthOf(2) + expect(result[0]).to.deep.equal({name: 'account1', username: 'user1'}) + expect(result[1]).to.deep.equal({name: 'account2', username: 'user2'}) }) }) @@ -75,7 +121,6 @@ describe('accounts', function () { let chmodSyncStub: sinon.SinonStub beforeEach(function () { - // Setup stubs before each test mkdirSyncStub = sinon.stub(fs, 'mkdirSync') writeFileSyncStub = sinon.stub(fs, 'writeFileSync') chmodSyncStub = sinon.stub(fs, 'chmodSync') @@ -89,11 +134,7 @@ describe('accounts', function () { }) it('should write credentials to file with correct format', function () { - const testName = 'test-user' - const testUsername = 'username123' - const testPassword = 'password123' - - AccountsModule.add(testName, testUsername, testPassword) + AccountsModule.add('test-user', 'username123', 'password123') expect(writeFileSyncStub.calledOnce).to.be.true expect(writeFileSyncStub.firstCall.args[1]).to.equal('username: username123\npassword: password123\n') @@ -101,9 +142,7 @@ describe('accounts', function () { }) it('should set correct file permissions', function () { - const testName = 'test-user' - - AccountsModule.add(testName, 'username123', 'password123') + AccountsModule.add('test-user', 'username123', 'password123') expect(chmodSyncStub.calledOnce).to.be.true expect(chmodSyncStub.firstCall.args[1]).to.equal(0o600) @@ -122,7 +161,6 @@ describe('accounts', function () { let existsSyncStub: sinon.SinonStub beforeEach(function () { - // Create a stub for fs.unlinkSync before each test unlinkStub = sinon.stub(fs, 'unlinkSync') osHomeStub = sinon.stub(os, 'homedir') existsSyncStub = sinon.stub(fs, 'existsSync') diff --git a/test/unit/lib/ps-exec/exec.unit.test.ts b/test/unit/lib/ps-exec/exec.unit.test.ts index 626e476b7a..d6969c9a87 100644 --- a/test/unit/lib/ps-exec/exec.unit.test.ts +++ b/test/unit/lib/ps-exec/exec.unit.test.ts @@ -426,7 +426,7 @@ describe('HerokuExec', function () { const {oclif} = error as Errors.ExitError expect(uxActionStartStub.calledWith('Initializing feature')).to.be.true expect(uxStdoutStub.called).to.be.true - expect(childExecSyncStub.calledOnce).to.be.true + expect(childExecSyncStub.calledWith(sinon.match(/heroku buildpacks:add/))).to.be.true expect(oclif.exit).to.equal(0) } })