From fd4c106f2bcafd51b95e55d713142de24f862cee Mon Sep 17 00:00:00 2001 From: umair Date: Fri, 13 Mar 2026 02:08:51 +0000 Subject: [PATCH 1/3] Fix auth keys commands --app flag to resolve app names to IDs --- README.md | 8 +-- src/commands/auth/keys/create.ts | 10 +--- src/commands/auth/keys/current.ts | 14 ++---- src/commands/auth/keys/get.ts | 13 +++-- src/commands/auth/keys/list.ts | 19 +++---- src/commands/auth/keys/revoke.ts | 18 ++++--- src/commands/auth/keys/switch.ts | 17 ++++--- src/commands/auth/keys/update.ts | 36 +++++++------ test/helpers/control-api-test-helpers.ts | 18 +++++++ test/unit/commands/auth/keys/create.test.ts | 29 ++++++++++- test/unit/commands/auth/keys/current.test.ts | 11 +++- test/unit/commands/auth/keys/get.test.ts | 5 ++ test/unit/commands/auth/keys/list.test.ts | 53 +++++++++++++++++++- test/unit/commands/auth/keys/revoke.test.ts | 2 + test/unit/commands/auth/keys/switch.test.ts | 13 ++++- test/unit/commands/auth/keys/update.test.ts | 2 + 16 files changed, 193 insertions(+), 75 deletions(-) diff --git a/README.md b/README.md index ec5b71f7..e35b9df5 100644 --- a/README.md +++ b/README.md @@ -1006,7 +1006,7 @@ USAGE FLAGS -v, --verbose Output verbose logs - --app= The app ID (defaults to current app) + --app= The app ID or name (defaults to current app) --json Output in JSON format --pretty-json Output in colorized JSON format @@ -1067,7 +1067,7 @@ USAGE FLAGS -v, --verbose Output verbose logs - --app= The app ID (defaults to current app) + --app= The app ID or name (defaults to current app) --json Output in JSON format --pretty-json Output in colorized JSON format @@ -1099,7 +1099,7 @@ ARGUMENTS FLAGS -v, --verbose Output verbose logs - --app= The app ID (defaults to current app) + --app= The app ID or name (defaults to current app) --force Skip confirmation prompt --json Output in JSON format --pretty-json Output in colorized JSON format @@ -1167,7 +1167,7 @@ ARGUMENTS FLAGS -v, --verbose Output verbose logs - --app= The app ID (defaults to current app) + --app= The app ID or name (defaults to current app) --capabilities= New capabilities for the key (comma-separated list) --json Output in JSON format --name= New name for the key diff --git a/src/commands/auth/keys/create.ts b/src/commands/auth/keys/create.ts index 54a9f275..34577f61 100644 --- a/src/commands/auth/keys/create.ts +++ b/src/commands/auth/keys/create.ts @@ -43,15 +43,7 @@ export default class KeysCreateCommand extends ControlBaseCommand { async run(): Promise { const { flags } = await this.parse(KeysCreateCommand); - const appId = flags.app || this.configManager.getCurrentAppId(); - - if (!appId) { - this.fail( - 'No app specified. Please provide --app flag or switch to an app with "ably apps switch".', - flags, - "keyCreate", - ); - } + const appId = await this.requireAppId(flags); let capabilities; try { diff --git a/src/commands/auth/keys/current.ts b/src/commands/auth/keys/current.ts index 1999ca59..7c5f5e27 100644 --- a/src/commands/auth/keys/current.ts +++ b/src/commands/auth/keys/current.ts @@ -17,7 +17,7 @@ export default class KeysCurrentCommand extends ControlBaseCommand { static flags = { ...ControlBaseCommand.globalFlags, app: Flags.string({ - description: "The app ID (defaults to current app)", + description: "The app ID or name (defaults to current app)", env: "ABLY_APP_ID", }), }; @@ -30,16 +30,8 @@ export default class KeysCurrentCommand extends ControlBaseCommand { return this.handleWebCliMode(flags); } - // Get app ID from flag or current config - const appId = flags.app || this.configManager.getCurrentAppId(); - - if (!appId) { - this.fail( - 'No app specified. Please provide --app flag or switch to an app with "ably apps switch".', - flags, - "KeyCurrent", - ); - } + // Get app ID from flag or current config (resolves app names to IDs) + const appId = await this.requireAppId(flags); // Get the current key for this app const apiKey = this.configManager.getApiKey(appId); diff --git a/src/commands/auth/keys/get.ts b/src/commands/auth/keys/get.ts index c5494f67..c230eddd 100644 --- a/src/commands/auth/keys/get.ts +++ b/src/commands/auth/keys/get.ts @@ -39,12 +39,14 @@ export default class KeysGetCommand extends ControlBaseCommand { async run(): Promise { const { args, flags } = await this.parse(KeysGetCommand); - // Display authentication information - await this.showAuthInfoIfNeeded(flags); - - let appId = flags.app || this.configManager.getCurrentAppId(); + let appId: string | undefined; const keyIdentifier = args.keyNameOrValue; + // If flags.app is set, resolve it (could be a name or ID) + if (flags.app) { + appId = await this.resolveAppIdFromNameOrId(flags.app, flags); + } + // If keyNameOrValue is in APP_ID.KEY_ID format (one period, no colon), extract appId. // Only attempt this when no appId is already known (from --app flag or current app), // to avoid misinterpreting labels containing periods (e.g. "v1.0") as APP_ID.KEY_ID. @@ -61,6 +63,9 @@ export default class KeysGetCommand extends ControlBaseCommand { appId = await this.requireAppId(flags); } + // Display authentication information (after app resolution so name→ID is correct) + await this.showAuthInfoIfNeeded(flags); + try { if (!this.shouldOutputJson(flags)) { this.log(formatProgress("Fetching key details")); diff --git a/src/commands/auth/keys/list.ts b/src/commands/auth/keys/list.ts index 5acf45ae..5cb3fcbc 100644 --- a/src/commands/auth/keys/list.ts +++ b/src/commands/auth/keys/list.ts @@ -17,7 +17,7 @@ export default class KeysListCommand extends ControlBaseCommand { static flags = { ...ControlBaseCommand.globalFlags, app: Flags.string({ - description: "The app ID (defaults to current app)", + description: "The app ID or name (defaults to current app)", env: "ABLY_APP_ID", }), }; @@ -25,19 +25,12 @@ export default class KeysListCommand extends ControlBaseCommand { async run(): Promise { const { flags } = await this.parse(KeysListCommand); - // Display authentication information - await this.showAuthInfoIfNeeded(flags); - - // Get app ID from flag or current config - const appId = flags.app || this.configManager.getCurrentAppId(); + // Get app ID from flag or current config (resolves app names to IDs) + // Must resolve before showAuthInfoIfNeeded so --app names display correctly + const appId = await this.requireAppId(flags); - if (!appId) { - this.fail( - 'No app specified. Please provide --app flag or switch to an app with "ably apps switch".', - flags, - "keyList", - ); - } + // Display authentication information (after app resolution so name→ID is correct) + await this.showAuthInfoIfNeeded(flags); try { const controlApi = this.createControlApi(flags); diff --git a/src/commands/auth/keys/revoke.ts b/src/commands/auth/keys/revoke.ts index fc477bba..88ed35e1 100644 --- a/src/commands/auth/keys/revoke.ts +++ b/src/commands/auth/keys/revoke.ts @@ -26,7 +26,7 @@ export default class KeysRevokeCommand extends ControlBaseCommand { static flags = { ...ControlBaseCommand.globalFlags, app: Flags.string({ - description: "The app ID (defaults to current app)", + description: "The app ID or name (defaults to current app)", env: "ABLY_APP_ID", }), force: Flags.boolean({ @@ -38,7 +38,7 @@ export default class KeysRevokeCommand extends ControlBaseCommand { async run(): Promise { const { args, flags } = await this.parse(KeysRevokeCommand); - let appId = flags.app || this.configManager.getCurrentAppId(); + let appId: string | undefined; let keyId = args.keyName; const parsed = parseKeyIdentifier(args.keyName); @@ -46,11 +46,15 @@ export default class KeysRevokeCommand extends ControlBaseCommand { keyId = parsed.keyId; if (!appId) { - this.fail( - 'No app specified. Please provide --app flag, include APP_ID in the key name, or switch to an app with "ably apps switch".', - flags, - "keyRevoke", - ); + const resolved = await this.resolveAppId(flags); + if (!resolved) { + this.fail( + 'No app specified. Use --app flag, provide APP_ID.KEY_ID as the argument, or select an app with "ably apps switch"', + flags, + "keyRevoke", + ); + } + appId = resolved; } try { diff --git a/src/commands/auth/keys/switch.ts b/src/commands/auth/keys/switch.ts index b5747883..e15d66bf 100644 --- a/src/commands/auth/keys/switch.ts +++ b/src/commands/auth/keys/switch.ts @@ -33,8 +33,7 @@ export default class KeysSwitchCommand extends ControlBaseCommand { async run(): Promise { const { args, flags } = await this.parse(KeysSwitchCommand); - // Get app ID from flag or current config - let appId = flags.app || this.configManager.getCurrentAppId(); + let appId: string | undefined; let keyId: string | undefined = args.keyNameOrValue; if (args.keyNameOrValue) { @@ -44,11 +43,15 @@ export default class KeysSwitchCommand extends ControlBaseCommand { } if (!appId) { - this.fail( - 'No app specified. Please provide --app flag, include APP_ID in the key name, or switch to an app with "ably apps switch".', - flags, - "keySwitch", - ); + const resolved = await this.resolveAppId(flags); + if (!resolved) { + this.fail( + 'No app specified. Use --app flag, provide APP_ID.KEY_ID as the argument, or select an app with "ably apps switch"', + flags, + "keySwitch", + ); + } + appId = resolved; } try { diff --git a/src/commands/auth/keys/update.ts b/src/commands/auth/keys/update.ts index 62c3c687..8f1658bf 100644 --- a/src/commands/auth/keys/update.ts +++ b/src/commands/auth/keys/update.ts @@ -24,7 +24,7 @@ export default class KeysUpdateCommand extends ControlBaseCommand { static flags = { ...ControlBaseCommand.globalFlags, app: Flags.string({ - description: "The app ID (defaults to current app)", + description: "The app ID or name (defaults to current app)", env: "ABLY_APP_ID", }), capabilities: Flags.string({ @@ -40,7 +40,16 @@ export default class KeysUpdateCommand extends ControlBaseCommand { async run(): Promise { const { args, flags } = await this.parse(KeysUpdateCommand); - let appId = flags.app || this.configManager.getCurrentAppId(); + // Check if any update flags were provided before doing any API calls + if (!flags.name && !flags.capabilities) { + this.fail( + "No updates specified. Please provide at least one property to update (--name or --capabilities).", + flags, + "keyUpdate", + ); + } + + let appId: string | undefined; let keyId = args.keyName; const parsed = parseKeyIdentifier(args.keyName); @@ -48,20 +57,15 @@ export default class KeysUpdateCommand extends ControlBaseCommand { keyId = parsed.keyId; if (!appId) { - this.fail( - 'No app specified. Please provide --app flag, include APP_ID in the key name, or switch to an app with "ably apps switch".', - flags, - "keyUpdate", - ); - } - - // Check if any update flags were provided - if (!flags.name && !flags.capabilities) { - this.fail( - "No updates specified. Please provide at least one property to update (--name or --capabilities).", - flags, - "keyUpdate", - ); + const resolved = await this.resolveAppId(flags); + if (!resolved) { + this.fail( + 'No app specified. Use --app flag, provide APP_ID.KEY_ID as the argument, or select an app with "ably apps switch"', + flags, + "keyUpdate", + ); + } + appId = resolved; } try { diff --git a/test/helpers/control-api-test-helpers.ts b/test/helpers/control-api-test-helpers.ts index f906d8de..bcaef713 100644 --- a/test/helpers/control-api-test-helpers.ts +++ b/test/helpers/control-api-test-helpers.ts @@ -25,6 +25,24 @@ export function getControlApiContext() { }; } +/** + * Mock the app resolution flow used by `requireAppId` / `resolveAppIdFromNameOrId`. + * Call this **before** other nock mocks in tests that pass `--app`. + * Mocks `GET /v1/me` and `GET /v1/accounts/{accountId}/apps`. + */ +export function mockAppResolution(appId: string): void { + const { accountId } = getControlApiContext(); + nockControl() + .get("/v1/me") + .reply(200, { + account: { id: accountId, name: "Test Account" }, + user: { email: "test@example.com" }, + }); + nockControl() + .get(`/v1/accounts/${accountId}/apps`) + .reply(200, [{ id: appId, name: "Test App", accountId }]); +} + /** Clean up all nock interceptors. Call in afterEach. */ export function controlApiCleanup(): void { nock.cleanAll(); diff --git a/test/unit/commands/auth/keys/create.test.ts b/test/unit/commands/auth/keys/create.test.ts index 109647aa..e0cd071d 100644 --- a/test/unit/commands/auth/keys/create.test.ts +++ b/test/unit/commands/auth/keys/create.test.ts @@ -5,6 +5,8 @@ import { nockControl, controlApiCleanup, CONTROL_HOST, + mockAppResolution, + getControlApiContext, } from "../../../../helpers/control-api-test-helpers.js"; import { getMockConfigManager } from "../../../../helpers/mock-config-manager.js"; import { @@ -32,6 +34,7 @@ describe("auth:keys:create command", () => { describe("functionality", () => { it("should create a key successfully", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); // Mock the key creation endpoint nockControl() .post(`/v1/apps/${appId}/keys`, { @@ -62,6 +65,7 @@ describe("auth:keys:create command", () => { it("should create a key with custom capabilities", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); // Mock the key creation endpoint with custom capabilities nockControl() .post(`/v1/apps/${appId}/keys`, { @@ -107,6 +111,7 @@ describe("auth:keys:create command", () => { it("should output JSON format when --json flag is used", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); const mockKey = { id: mockKeyId, appId, @@ -146,6 +151,7 @@ describe("auth:keys:create command", () => { it("should use ABLY_ACCESS_TOKEN environment variable when provided", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); const customToken = "custom_access_token"; process.env.ABLY_ACCESS_TOKEN = customToken; @@ -185,6 +191,7 @@ describe("auth:keys:create command", () => { describe("argument validation", () => { it("should require name parameter", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); const { error } = await runCommand( ["auth:keys:create", "--app", appId], import.meta.url, @@ -195,17 +202,28 @@ describe("auth:keys:create command", () => { }); it("should require app parameter when no current app is set", async () => { + const { accountId } = getControlApiContext(); + // Mock the app resolution flow (requireAppId → promptForApp → listApps) + nockControl() + .get("/v1/me") + .reply(200, { + account: { id: accountId, name: "Test Account" }, + user: { email: "test@example.com" }, + }); + nockControl().get(`/v1/accounts/${accountId}/apps`).reply(200, []); + const { error } = await runCommand( ["auth:keys:create", "--name", `"${mockKeyName}"`], import.meta.url, ); expect(error).toBeDefined(); - expect(error?.message).toMatch(/No app specified/); + expect(error?.message).toMatch(/No apps found/); expect(error?.oclif?.exit).toBeGreaterThan(0); }); it("should handle invalid capabilities JSON", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); // Mock the key creation endpoint with invalid capabilities nockControl().post(`/v1/apps/${appId}/keys`).reply(400, { error: "Invalid capabilities format", @@ -232,6 +250,7 @@ describe("auth:keys:create command", () => { describe("error handling", () => { it("should handle 401 authentication error", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); // Mock authentication failure nockControl() .post(`/v1/apps/${appId}/keys`) @@ -248,6 +267,7 @@ describe("auth:keys:create command", () => { it("should handle 403 forbidden error", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); // Mock forbidden response nockControl() .post(`/v1/apps/${appId}/keys`) @@ -264,6 +284,7 @@ describe("auth:keys:create command", () => { it("should handle 404 not found error", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); // Mock not found response (app doesn't exist) nockControl() .post(`/v1/apps/${appId}/keys`) @@ -280,6 +301,7 @@ describe("auth:keys:create command", () => { it("should handle 500 server error", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); // Mock server error nockControl() .post(`/v1/apps/${appId}/keys`) @@ -296,6 +318,7 @@ describe("auth:keys:create command", () => { it("should handle network errors", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); // Mock network error nockControl() .post(`/v1/apps/${appId}/keys`) @@ -312,6 +335,7 @@ describe("auth:keys:create command", () => { it("should handle validation errors from API", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); // Mock validation error nockControl().post(`/v1/apps/${appId}/keys`).reply(400, { error: "Validation failed", @@ -329,6 +353,7 @@ describe("auth:keys:create command", () => { it("should handle rate limit errors", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); // Mock rate limit error nockControl() .post(`/v1/apps/${appId}/keys`) @@ -347,6 +372,7 @@ describe("auth:keys:create command", () => { describe("capability configurations", () => { it("should create a publish-only key", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); // Mock the key creation endpoint with publish-only capabilities nockControl() .post(`/v1/apps/${appId}/keys`, { @@ -384,6 +410,7 @@ describe("auth:keys:create command", () => { it("should create a key with mixed capabilities", async () => { const appId = getMockConfigManager().getRegisteredAppId(); + mockAppResolution(appId); // Mock the key creation endpoint with subscribe-only capabilities nockControl() .post(`/v1/apps/${appId}/keys`, { diff --git a/test/unit/commands/auth/keys/current.test.ts b/test/unit/commands/auth/keys/current.test.ts index ff81eb0d..e4dd831e 100644 --- a/test/unit/commands/auth/keys/current.test.ts +++ b/test/unit/commands/auth/keys/current.test.ts @@ -1,6 +1,10 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, afterEach } from "vitest"; import { runCommand } from "@oclif/test"; import { getMockConfigManager } from "../../../../helpers/mock-config-manager.js"; +import { + mockAppResolution, + controlApiCleanup, +} from "../../../../helpers/control-api-test-helpers.js"; import { standardHelpTests, standardArgValidationTests, @@ -8,6 +12,10 @@ import { } from "../../../../helpers/standard-tests.js"; describe("auth:keys:current command", () => { + afterEach(() => { + controlApiCleanup(); + }); + describe("functionality", () => { it("should display the current API key", async () => { const mockConfig = getMockConfigManager(); @@ -69,6 +77,7 @@ describe("auth:keys:current command", () => { it("should accept --app flag to specify a different app", async () => { const mockConfig = getMockConfigManager(); const appId = mockConfig.getCurrentAppId()!; + mockAppResolution(appId); const keyId = mockConfig.getKeyId()!; const { stdout } = await runCommand( ["auth:keys:current", "--app", appId], diff --git a/test/unit/commands/auth/keys/get.test.ts b/test/unit/commands/auth/keys/get.test.ts index 841a9fa9..e4d89d91 100644 --- a/test/unit/commands/auth/keys/get.test.ts +++ b/test/unit/commands/auth/keys/get.test.ts @@ -3,6 +3,7 @@ import { runCommand } from "@oclif/test"; import { nockControl, controlApiCleanup, + mockAppResolution, } from "../../../../helpers/control-api-test-helpers.js"; import { getMockConfigManager } from "../../../../helpers/mock-config-manager.js"; import { @@ -51,6 +52,7 @@ describe("auth:keys:get command", () => { it("should get key details with --app flag", async () => { const appId = getMockConfigManager().getCurrentAppId()!; + mockAppResolution(appId); mockKeysList(appId, [buildMockKey(appId, mockKeyId)]); const { stdout } = await runCommand( @@ -64,6 +66,7 @@ describe("auth:keys:get command", () => { it("should get key details by label name", async () => { const appId = getMockConfigManager().getCurrentAppId()!; + mockAppResolution(appId); mockKeysList(appId, [ buildMockKey(appId, mockKeyId, { name: "Root" }), buildMockKey(appId, "otherkey", { name: "Secondary" }), @@ -80,6 +83,7 @@ describe("auth:keys:get command", () => { it("should get key details by label containing a period (e.g. v1.0)", async () => { const appId = getMockConfigManager().getCurrentAppId()!; + mockAppResolution(appId); mockKeysList(appId, [ buildMockKey(appId, mockKeyId, { name: "v1.0" }), buildMockKey(appId, "otherkey", { name: "Secondary" }), @@ -96,6 +100,7 @@ describe("auth:keys:get command", () => { it("should get key details by key ID only", async () => { const appId = getMockConfigManager().getCurrentAppId()!; + mockAppResolution(appId); mockKeysList(appId, [ buildMockKey(appId, mockKeyId), buildMockKey(appId, "otherkey", { name: "Secondary" }), diff --git a/test/unit/commands/auth/keys/list.test.ts b/test/unit/commands/auth/keys/list.test.ts index 07f3c88d..ba7c8544 100644 --- a/test/unit/commands/auth/keys/list.test.ts +++ b/test/unit/commands/auth/keys/list.test.ts @@ -3,6 +3,8 @@ import { runCommand } from "@oclif/test"; import { nockControl, controlApiCleanup, + mockAppResolution, + getControlApiContext, } from "../../../../helpers/control-api-test-helpers.js"; import { getMockConfigManager } from "../../../../helpers/mock-config-manager.js"; import { @@ -57,6 +59,7 @@ describe("auth:keys:list command", () => { it("should list keys with --app flag", async () => { const appId = getMockConfigManager().getCurrentAppId()!; + mockAppResolution(appId); nockControl() .get(`/v1/apps/${appId}/keys`) .reply(200, [ @@ -80,6 +83,44 @@ describe("auth:keys:list command", () => { expect(stdout).toContain("Key Label: Test Key"); }); + it("should resolve app name to ID when --app is a name", async () => { + const appId = getMockConfigManager().getCurrentAppId()!; + const { accountId } = getControlApiContext(); + const meReply = { + account: { id: accountId, name: "Test Account" }, + user: { email: "test@example.com" }, + }; + const appsReply = [{ id: appId, name: "MyApp", accountId }]; + + // displayAuthInfo calls getApp → listApps (consumes /me + /apps) + nockControl().get("/v1/me").reply(200, meReply); + nockControl().get(`/v1/accounts/${accountId}/apps`).reply(200, appsReply); + // requireAppId → resolveAppIdFromNameOrId also calls listApps + nockControl().get("/v1/me").reply(200, meReply); + nockControl().get(`/v1/accounts/${accountId}/apps`).reply(200, appsReply); + nockControl() + .get(`/v1/apps/${appId}/keys`) + .reply(200, [ + { + id: "key1", + appId, + name: "Key One", + key: `${appId}.key1:secret1`, + capability: { "*": ["publish", "subscribe"] }, + created: Date.now(), + modified: Date.now(), + }, + ]); + + const { stdout } = await runCommand( + ["auth:keys:list", "--app", "MyApp"], + import.meta.url, + ); + + expect(stdout).toContain(`Key Name: ${appId}.key1`); + expect(stdout).toContain("Key Label: Key One"); + }); + it("should show message when no keys found", async () => { const appId = getMockConfigManager().getCurrentAppId()!; nockControl().get(`/v1/apps/${appId}/keys`).reply(200, []); @@ -129,12 +170,22 @@ describe("auth:keys:list command", () => { describe("error handling", () => { it("should error when no app is selected", async () => { const mock = getMockConfigManager(); + const { accountId } = getControlApiContext(); mock.setCurrentAppIdForAccount(undefined); + // Mock the app resolution flow (requireAppId → promptForApp → listApps) + nockControl() + .get("/v1/me") + .reply(200, { + account: { id: accountId, name: "Test Account" }, + user: { email: "test@example.com" }, + }); + nockControl().get(`/v1/accounts/${accountId}/apps`).reply(200, []); + const { error } = await runCommand(["auth:keys:list"], import.meta.url); expect(error).toBeDefined(); - expect(error?.message).toMatch(/No app specified/); + expect(error?.message).toMatch(/No apps found/); }); standardControlApiErrorTests({ diff --git a/test/unit/commands/auth/keys/revoke.test.ts b/test/unit/commands/auth/keys/revoke.test.ts index d41553fb..b62dcec1 100644 --- a/test/unit/commands/auth/keys/revoke.test.ts +++ b/test/unit/commands/auth/keys/revoke.test.ts @@ -3,6 +3,7 @@ import { runCommand } from "@oclif/test"; import { nockControl, controlApiCleanup, + mockAppResolution, } from "../../../../helpers/control-api-test-helpers.js"; import { getMockConfigManager } from "../../../../helpers/mock-config-manager.js"; import { @@ -49,6 +50,7 @@ describe("auth:keys:revoke command", () => { it("should revoke key with --app flag", async () => { const appId = getMockConfigManager().getCurrentAppId()!; + mockAppResolution(appId); mockKeysList(appId, [ buildMockKey(appId, mockKeyId, { capability: { "*": ["publish"] }, diff --git a/test/unit/commands/auth/keys/switch.test.ts b/test/unit/commands/auth/keys/switch.test.ts index 84b395df..5bcd45f7 100644 --- a/test/unit/commands/auth/keys/switch.test.ts +++ b/test/unit/commands/auth/keys/switch.test.ts @@ -3,6 +3,7 @@ import { runCommand } from "@oclif/test"; import { nockControl, controlApiCleanup, + getControlApiContext, } from "../../../../helpers/control-api-test-helpers.js"; import { getMockConfigManager } from "../../../../helpers/mock-config-manager.js"; import { @@ -107,15 +108,25 @@ describe("auth:keys:switch command", () => { it("should handle no app specified when config has no current app", async () => { const mockConfig = getMockConfigManager(); + const { accountId } = getControlApiContext(); mockConfig.setCurrentAppIdForAccount(undefined); + // Mock the app resolution flow (requireAppId → promptForApp → listApps) + nockControl() + .get("/v1/me") + .reply(200, { + account: { id: accountId, name: "Test Account" }, + user: { email: "test@example.com" }, + }); + nockControl().get(`/v1/accounts/${accountId}/apps`).reply(200, []); + const { error } = await runCommand( ["auth:keys:switch", "just-a-key-id"], import.meta.url, ); expect(error).toBeDefined(); - expect(error?.message).toMatch(/No app specified/i); + expect(error?.message).toMatch(/No apps found/i); }); it("should handle 401 authentication error", async () => { diff --git a/test/unit/commands/auth/keys/update.test.ts b/test/unit/commands/auth/keys/update.test.ts index ae4afd3c..7f15ca58 100644 --- a/test/unit/commands/auth/keys/update.test.ts +++ b/test/unit/commands/auth/keys/update.test.ts @@ -3,6 +3,7 @@ import { runCommand } from "@oclif/test"; import { nockControl, controlApiCleanup, + mockAppResolution, } from "../../../../helpers/control-api-test-helpers.js"; import { getMockConfigManager } from "../../../../helpers/mock-config-manager.js"; import { @@ -88,6 +89,7 @@ describe("auth:keys:update command", () => { it("should update key with --app flag", async () => { const appId = getMockConfigManager().getCurrentAppId()!; + mockAppResolution(appId); mockKeysList(appId, [ buildMockKey(appId, mockKeyId, { name: "OldName", From 2a49d85d45f66c6650440f3fa40ada228b1cf7c3 Mon Sep 17 00:00:00 2001 From: umair Date: Fri, 13 Mar 2026 16:05:21 +0000 Subject: [PATCH 2/3] Address PR comments --- src/commands/auth/keys/current.ts | 4 ++-- src/commands/auth/keys/revoke.ts | 14 +------------- src/commands/auth/keys/switch.ts | 16 +++------------- src/commands/auth/keys/update.ts | 14 +------------- 4 files changed, 7 insertions(+), 41 deletions(-) diff --git a/src/commands/auth/keys/current.ts b/src/commands/auth/keys/current.ts index 7c5f5e27..d1c05ff0 100644 --- a/src/commands/auth/keys/current.ts +++ b/src/commands/auth/keys/current.ts @@ -40,7 +40,7 @@ export default class KeysCurrentCommand extends ControlBaseCommand { this.fail( `No API key configured for app ${appId}. Use "ably auth keys switch" to select a key.`, flags, - "KeyCurrent", + "keyCurrent", ); } @@ -97,7 +97,7 @@ export default class KeysCurrentCommand extends ControlBaseCommand { this.fail( "ABLY_API_KEY environment variable is not set", flags, - "KeyCurrent", + "keyCurrent", ); } diff --git a/src/commands/auth/keys/revoke.ts b/src/commands/auth/keys/revoke.ts index 88ed35e1..0317f7de 100644 --- a/src/commands/auth/keys/revoke.ts +++ b/src/commands/auth/keys/revoke.ts @@ -38,24 +38,12 @@ export default class KeysRevokeCommand extends ControlBaseCommand { async run(): Promise { const { args, flags } = await this.parse(KeysRevokeCommand); - let appId: string | undefined; let keyId = args.keyName; const parsed = parseKeyIdentifier(args.keyName); - if (parsed.appId) appId = parsed.appId; keyId = parsed.keyId; - if (!appId) { - const resolved = await this.resolveAppId(flags); - if (!resolved) { - this.fail( - 'No app specified. Use --app flag, provide APP_ID.KEY_ID as the argument, or select an app with "ably apps switch"', - flags, - "keyRevoke", - ); - } - appId = resolved; - } + const appId = parsed.appId ?? (await this.requireAppId(flags)); try { const controlApi = this.createControlApi(flags); diff --git a/src/commands/auth/keys/switch.ts b/src/commands/auth/keys/switch.ts index e15d66bf..3327de63 100644 --- a/src/commands/auth/keys/switch.ts +++ b/src/commands/auth/keys/switch.ts @@ -33,26 +33,16 @@ export default class KeysSwitchCommand extends ControlBaseCommand { async run(): Promise { const { args, flags } = await this.parse(KeysSwitchCommand); - let appId: string | undefined; let keyId: string | undefined = args.keyNameOrValue; + let extractedAppId: string | undefined; if (args.keyNameOrValue) { const parsed = parseKeyIdentifier(args.keyNameOrValue); - if (parsed.appId) appId = parsed.appId; + if (parsed.appId) extractedAppId = parsed.appId; keyId = parsed.keyId; } - if (!appId) { - const resolved = await this.resolveAppId(flags); - if (!resolved) { - this.fail( - 'No app specified. Use --app flag, provide APP_ID.KEY_ID as the argument, or select an app with "ably apps switch"', - flags, - "keySwitch", - ); - } - appId = resolved; - } + const appId = extractedAppId ?? (await this.requireAppId(flags)); try { const controlApi = this.createControlApi(flags); diff --git a/src/commands/auth/keys/update.ts b/src/commands/auth/keys/update.ts index 8f1658bf..adfba580 100644 --- a/src/commands/auth/keys/update.ts +++ b/src/commands/auth/keys/update.ts @@ -49,24 +49,12 @@ export default class KeysUpdateCommand extends ControlBaseCommand { ); } - let appId: string | undefined; let keyId = args.keyName; const parsed = parseKeyIdentifier(args.keyName); - if (parsed.appId) appId = parsed.appId; keyId = parsed.keyId; - if (!appId) { - const resolved = await this.resolveAppId(flags); - if (!resolved) { - this.fail( - 'No app specified. Use --app flag, provide APP_ID.KEY_ID as the argument, or select an app with "ably apps switch"', - flags, - "keyUpdate", - ); - } - appId = resolved; - } + const appId = parsed.appId ?? (await this.requireAppId(flags)); try { const controlApi = this.createControlApi(flags); From 2c961174ae0c08557f8151c56e03171b77b6b59b Mon Sep 17 00:00:00 2001 From: umair Date: Tue, 17 Mar 2026 17:50:58 +0000 Subject: [PATCH 3/3] improve formatting (helpers and nested json) --- src/commands/auth/keys/list.ts | 14 +++++-- src/commands/auth/keys/revoke.ts | 22 ++++++---- src/commands/auth/keys/switch.ts | 21 ++++++---- src/commands/auth/keys/update.ts | 19 ++++----- test/unit/commands/auth/keys/current.test.ts | 3 ++ test/unit/commands/auth/keys/switch.test.ts | 7 ++-- test/unit/commands/auth/keys/update.test.ts | 42 +++++++++++++++++++- 7 files changed, 97 insertions(+), 31 deletions(-) diff --git a/src/commands/auth/keys/list.ts b/src/commands/auth/keys/list.ts index 5cb3fcbc..32cf781f 100644 --- a/src/commands/auth/keys/list.ts +++ b/src/commands/auth/keys/list.ts @@ -3,6 +3,11 @@ import chalk from "chalk"; import { ControlBaseCommand } from "../../../control-base-command.js"; import { formatCapabilities } from "../../../utils/key-display.js"; +import { + formatHeading, + formatLabel, + formatResource, +} from "../../../utils/output.js"; export default class KeysListCommand extends ControlBaseCommand { static description = "List all keys in the app"; @@ -74,14 +79,17 @@ export default class KeysListCommand extends ControlBaseCommand { const keyName = `${key.appId}.${key.id}`; const isCurrent = keyName === currentKeyName; const prefix = isCurrent ? chalk.green("▶ ") : " "; - const titleStyle = isCurrent ? chalk.green.bold : chalk.bold; this.log( prefix + - titleStyle(`Key Name: ${keyName}`) + + formatHeading( + `${formatLabel("Key Name")} ${formatResource(keyName)}`, + ) + (isCurrent ? chalk.green(" (current)") : ""), ); - this.log(` Key Label: ${key.name || "Unnamed key"}`); + this.log( + ` ${formatLabel("Key Label")} ${key.name || "Unnamed key"}`, + ); for (const line of formatCapabilities( key.capability as Record, diff --git a/src/commands/auth/keys/revoke.ts b/src/commands/auth/keys/revoke.ts index 0317f7de..eca0a615 100644 --- a/src/commands/auth/keys/revoke.ts +++ b/src/commands/auth/keys/revoke.ts @@ -3,7 +3,11 @@ import { Args, Flags } from "@oclif/core"; import { ControlBaseCommand } from "../../../control-base-command.js"; import { formatCapabilities } from "../../../utils/key-display.js"; import { parseKeyIdentifier } from "../../../utils/key-parsing.js"; -import { formatResource } from "../../../utils/output.js"; +import { + formatLabel, + formatResource, + formatSuccess, +} from "../../../utils/output.js"; export default class KeysRevokeCommand extends ControlBaseCommand { static args = { @@ -54,9 +58,9 @@ export default class KeysRevokeCommand extends ControlBaseCommand { if (!this.shouldOutputJson(flags)) { this.log(`Key to revoke:`); - this.log(`Key Name: ${keyName}`); - this.log(`Key Label: ${key.name || "Unnamed key"}`); - this.log(`Full key: ${key.key}`); + this.log(`${formatLabel("Key Name")} ${formatResource(keyName)}`); + this.log(`${formatLabel("Key Label")} ${key.name || "Unnamed key"}`); + this.log(`${formatLabel("Full key")} ${key.key}`); for (const line of formatCapabilities( key.capability as Record, @@ -92,13 +96,17 @@ export default class KeysRevokeCommand extends ControlBaseCommand { if (this.shouldOutputJson(flags)) { this.logJsonResult( { - keyName, - message: "Key has been revoked", + key: { + keyName, + message: "Key has been revoked", + }, }, flags, ); } else { - this.log(`Key ${formatResource(keyName)} has been revoked.`); + this.log( + formatSuccess(`Key ${formatResource(keyName)} has been revoked.`), + ); } // Check if the revoked key is the current key for this app diff --git a/src/commands/auth/keys/switch.ts b/src/commands/auth/keys/switch.ts index 3327de63..b2e0f166 100644 --- a/src/commands/auth/keys/switch.ts +++ b/src/commands/auth/keys/switch.ts @@ -3,6 +3,7 @@ import { Args, Flags } from "@oclif/core"; import { ControlBaseCommand } from "../../../control-base-command.js"; import { ControlApi } from "../../../services/control-api.js"; import { parseKeyIdentifier } from "../../../utils/key-parsing.js"; +import { formatResource, formatSuccess } from "../../../utils/output.js"; export default class KeysSwitchCommand extends ControlBaseCommand { static args = { @@ -96,15 +97,17 @@ export default class KeysSwitchCommand extends ControlBaseCommand { if (this.shouldOutputJson(flags)) { this.logJsonResult( { - appId, - keyName, - keyLabel: selectedKey.name || "Unnamed key", + key: { + appId, + keyName, + keyLabel: selectedKey.name || "Unnamed key", + }, }, flags, ); } else { this.log( - `Switched to key: ${selectedKey.name || "Unnamed key"} (${keyName})`, + formatSuccess(`Switched to key ${formatResource(keyName)}.`), ); } } else { @@ -154,14 +157,16 @@ export default class KeysSwitchCommand extends ControlBaseCommand { if (this.shouldOutputJson(flags)) { this.logJsonResult( { - appId, - keyName, - keyLabel: key.name || "Unnamed key", + key: { + appId, + keyName, + keyLabel: key.name || "Unnamed key", + }, }, flags, ); } else { - this.log(`Switched to key: ${key.name || "Unnamed key"} (${keyName})`); + this.log(formatSuccess(`Switched to key ${formatResource(keyName)}.`)); } } catch { this.fail( diff --git a/src/commands/auth/keys/update.ts b/src/commands/auth/keys/update.ts index adfba580..f195a37e 100644 --- a/src/commands/auth/keys/update.ts +++ b/src/commands/auth/keys/update.ts @@ -3,6 +3,7 @@ import { Args, Flags } from "@oclif/core"; import { ControlBaseCommand } from "../../../control-base-command.js"; import { formatCapabilityInline } from "../../../utils/key-display.js"; import { parseKeyIdentifier } from "../../../utils/key-parsing.js"; +import { formatLabel, formatResource } from "../../../utils/output.js"; export default class KeysUpdateCommand extends ControlBaseCommand { static args = { @@ -96,36 +97,36 @@ export default class KeysUpdateCommand extends ControlBaseCommand { const keyName = `${updatedKey.appId}.${updatedKey.id}`; if (this.shouldOutputJson(flags)) { - const result: Record = { keyName }; + const keyData: Record = { keyName }; if (flags.name) { - result.name = { + keyData.name = { before: originalKey.name || "Unnamed key", after: updatedKey.name || "Unnamed key", }; } if (flags.capabilities) { - result.capabilities = { + keyData.capabilities = { before: originalKey.capability, after: updatedKey.capability, }; } - this.logJsonResult(result, flags); + this.logJsonResult({ key: keyData }, flags); } else { - this.log(`Key Name: ${keyName}`); + this.log(`${formatLabel("Key Name")} ${formatResource(keyName)}`); if (flags.name) { this.log( - `Key Label: "${originalKey.name || "Unnamed key"}" → "${updatedKey.name || "Unnamed key"}"`, + `${formatLabel("Key Label")} "${originalKey.name || "Unnamed key"}" → "${updatedKey.name || "Unnamed key"}"`, ); } if (flags.capabilities) { - this.log(`Capabilities:`); + this.log(`${formatLabel("Capabilities")}`); this.log( - ` Before: ${formatCapabilityInline(originalKey.capability as Record)}`, + ` ${formatLabel("Before")} ${formatCapabilityInline(originalKey.capability as Record)}`, ); this.log( - ` After: ${formatCapabilityInline(updatedKey.capability as Record)}`, + ` ${formatLabel("After")} ${formatCapabilityInline(updatedKey.capability as Record)}`, ); } } diff --git a/test/unit/commands/auth/keys/current.test.ts b/test/unit/commands/auth/keys/current.test.ts index e4dd831e..8504091e 100644 --- a/test/unit/commands/auth/keys/current.test.ts +++ b/test/unit/commands/auth/keys/current.test.ts @@ -51,6 +51,9 @@ describe("auth:keys:current command", () => { ); const result = JSON.parse(stdout); + expect(result).toHaveProperty("type", "result"); + expect(result).toHaveProperty("command", "auth:keys:current"); + expect(result).toHaveProperty("success", true); expect(result).toHaveProperty("app"); expect(result).toHaveProperty("key"); expect(result.key).toHaveProperty("value"); diff --git a/test/unit/commands/auth/keys/switch.test.ts b/test/unit/commands/auth/keys/switch.test.ts index 5bcd45f7..87a14702 100644 --- a/test/unit/commands/auth/keys/switch.test.ts +++ b/test/unit/commands/auth/keys/switch.test.ts @@ -53,7 +53,7 @@ describe("auth:keys:switch command", () => { ); expect(stdout).toContain("Switched to key"); - expect(stdout).toContain(mockKeyName); + expect(stdout).toContain(`${appId}.${mockKeyId}`); }); it("should output JSON when --json flag is used", async () => { @@ -83,8 +83,9 @@ describe("auth:keys:switch command", () => { expect(result).toHaveProperty("type", "result"); expect(result).toHaveProperty("command", "auth:keys:switch"); expect(result).toHaveProperty("success", true); - expect(result).toHaveProperty("appId", appId); - expect(result).toHaveProperty("keyLabel", mockKeyName); + expect(result).toHaveProperty("key"); + expect(result.key).toHaveProperty("appId", appId); + expect(result.key).toHaveProperty("keyLabel", mockKeyName); }); }); diff --git a/test/unit/commands/auth/keys/update.test.ts b/test/unit/commands/auth/keys/update.test.ts index 7f15ca58..cdd8a18f 100644 --- a/test/unit/commands/auth/keys/update.test.ts +++ b/test/unit/commands/auth/keys/update.test.ts @@ -84,7 +84,47 @@ describe("auth:keys:update command", () => { ); expect(stdout).toContain(`Key Name: ${appId}.${mockKeyId}`); - expect(stdout).toContain("After: * → subscribe"); + expect(stdout).toContain("After:"); + expect(stdout).toContain("* → subscribe"); + }); + + it("should output JSON with nested key data when --json flag is used", async () => { + const appId = getMockConfigManager().getCurrentAppId()!; + mockKeysList(appId, [ + buildMockKey(appId, mockKeyId, { name: "OldName" }), + ]); + + nockControl() + .patch(`/v1/apps/${appId}/keys/${mockKeyId}`) + .reply(200, { + id: mockKeyId, + appId, + name: "NewName", + key: `${appId}.${mockKeyId}:secret`, + capability: { "*": ["publish", "subscribe"] }, + created: Date.now(), + modified: Date.now(), + }); + + const { stdout } = await runCommand( + [ + "auth:keys:update", + `${appId}.${mockKeyId}`, + "--name=NewName", + "--json", + ], + import.meta.url, + ); + + const result = JSON.parse(stdout); + expect(result).toHaveProperty("type", "result"); + expect(result).toHaveProperty("command", "auth:keys:update"); + expect(result).toHaveProperty("success", true); + expect(result).toHaveProperty("key"); + expect(result.key).toHaveProperty("keyName", `${appId}.${mockKeyId}`); + expect(result.key).toHaveProperty("name"); + expect(result.key.name.before).toBe("OldName"); + expect(result.key.name.after).toBe("NewName"); }); it("should update key with --app flag", async () => {