From e0e9c9c08cc3e9d90bd46cac7066ba041a04c552 Mon Sep 17 00:00:00 2001 From: evgeny Date: Wed, 30 Sep 2026 12:24:01 +0100 Subject: [PATCH 1/2] feat(mau): resolve one stable client identity per process Every client the CLI built used to mint a random `ably-cli-`, so a 30-command session acted as 30 different clients and left random names in message history and presence. The CLI now resolves its client ID once per process with the precedence --client-id > ABLY_CLIENT_ID > `client.id` in the config > an install default generated once and persisted in the config. Every client, every interactive command and every minted token share it. - Reject "" (which silently fell back to a random ID) and "*" with messages naming where the value came from; deprecate "none". - Under token auth the identity is the token's and is never overridden; a JWT without an `x-ably-clientId` claim is rejected up front. - Show the resolved client ID in the "Using:" banner. - bench suffixes its default identity per process so concurrent bench processes stay under the per-client-ID connection cap. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/Environment-Variables/General-Usage.md | 1 + docs/Project-Structure.md | 1 + src/base-command.ts | 145 +++++++++++--- src/commands/auth/issue-ably-token.ts | 5 +- src/commands/auth/issue-jwt-token.ts | 7 +- src/commands/bench/publisher.ts | 2 + src/commands/bench/subscriber.ts | 2 + src/data/env-vars.ts | 24 +++ src/services/client-identity.ts | 125 ++++++++++++ src/services/config-manager.ts | 33 ++++ test/helpers/mock-config-manager.ts | 15 ++ test/unit/base/base-command.test.ts | 2 + test/unit/base/client-identity.test.ts | 204 ++++++++++++++++++++ test/unit/commands/env.test.ts | 5 +- test/unit/data/env-vars.test.ts | 6 +- test/unit/setup.ts | 2 + 16 files changed, 546 insertions(+), 33 deletions(-) create mode 100644 src/services/client-identity.ts create mode 100644 test/unit/base/client-identity.test.ts diff --git a/docs/Environment-Variables/General-Usage.md b/docs/Environment-Variables/General-Usage.md index 1c37eff0e..58ffeca51 100644 --- a/docs/Environment-Variables/General-Usage.md +++ b/docs/Environment-Variables/General-Usage.md @@ -14,6 +14,7 @@ These environment variables are most commonly used during development as well as | `ABLY_TOKEN` | Authentication | Token/JWT for data plane commands | None | | `ABLY_ACCESS_TOKEN` | Authentication | Access token for Control API commands | None | | `ABLY_APP_ID` | App Selection | Default app for `--app` flag | None | +| `ABLY_CLIENT_ID` | Configuration | Client ID the CLI acts as | Generated once per install | | `ABLY_CLI_CONFIG_DIR` | Configuration | Custom config directory | `~/.ably` | | `ABLY_HISTORY_FILE` | Configuration | Custom history file location | `~/.ably/history` | | `ABLY_CLI_DEFAULT_DURATION` | Behavior | Auto-exit long-running commands (seconds) | None (forever) | diff --git a/docs/Project-Structure.md b/docs/Project-Structure.md index 07c5fa8c6..6b7dca448 100644 --- a/docs/Project-Structure.md +++ b/docs/Project-Structure.md @@ -63,6 +63,7 @@ This document outlines the directory structure of the Ably CLI project. │ │ ├── command_not_found/ # Fuzzy-match suggestions for unknown commands │ │ └── init/ # CLI initialization │ ├── services/ # Business logic +│ │ ├── client-identity.ts # Resolves the client ID the CLI acts as │ │ ├── config-manager.ts # CLI configuration (accounts, apps, API keys) │ │ ├── control-api.ts # Ably Control API HTTP client │ │ ├── history-manager.ts # Interactive mode command history persistence diff --git a/src/base-command.ts b/src/base-command.ts index b7c85aa8c..e3458646f 100644 --- a/src/base-command.ts +++ b/src/base-command.ts @@ -2,8 +2,8 @@ import { InteractiveBaseCommand } from "./interactive-base-command.js"; import type * as Ably from "@ably/pubsub-server"; import { createHttpClient, createRealtimeClient } from "@ably/pubsub-server"; import chalk from "chalk"; -import colorJson from "color-json"; import { randomUUID } from "node:crypto"; +import colorJson from "color-json"; import { ConfigManager, @@ -11,6 +11,13 @@ import { type DataPlaneConfig, } from "./services/config-manager.js"; import { ControlApi, controlHostScheme } from "./services/control-api.js"; +import { + CLIENT_ID_CLAIM, + CLIENT_ID_ENV_VAR, + InvalidClientIdError, + readTokenIdentity, + resolveClientIdentity, +} from "./services/client-identity.js"; import { extractAppIdFromApiKey, extractKeyNameFromApiKey, @@ -146,6 +153,13 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { private _cachedRestClient: Ably.PubSubHttpClient | null = null; private _cachedRealtimeClient: Ably.PubSubRealtimeClient | null = null; + /** + * Suffix the default client ID per process, for commands that run as many + * concurrent processes (bench), so they stay under the per-client-ID + * connection cap. An explicit --client-id is always used as given. + */ + protected suffixClientIdPerProcess = false; + // Core global flags available to all commands (verbose, json, pretty-json, web-cli-help) static globalFlags = { ...coreGlobalFlags }; @@ -727,6 +741,13 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { ); } } + + const clientId = this.bannerClientId(flags); + if (clientId) { + displayParts.push( + `${chalk.blue("Client=")}${chalk.blue.bold(clientId)}`, + ); + } } } @@ -740,6 +761,24 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { } } + /** + * The client ID shown in the banner. Invalid values are reported when the + * client is built, so the banner just omits them. + */ + private bannerClientId(flags: BaseFlags): string | undefined { + if (process.env.ABLY_TOKEN) { + const identity = readTokenIdentity(process.env.ABLY_TOKEN); + return identity.kind === "jwt" ? identity.clientId : undefined; + } + + try { + return resolveClientIdentity(flags["client-id"], this.configManager) + .clientId; + } catch { + return undefined; + } + } + /** * Resolve host, port and TLS for the data plane from the first source that * supplies them: `--url`, `ABLY_URL`, `ABLY_ENDPOINT`, then the account @@ -1157,15 +1196,7 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { // Handle authentication: ABLY_TOKEN env → flags["api-key"] (set by ensureAppAndKey) → ABLY_API_KEY env → config if (process.env.ABLY_TOKEN) { options.token = process.env.ABLY_TOKEN; - - // When using token auth, we don't set the clientId as it may conflict - // with any clientId embedded in the token - if (flags["client-id"] && !this.shouldSuppressOutput(flags)) { - this.logWarning( - "clientId is ignored when using token authentication as the clientId is embedded in the token.", - flags, - ); - } + this.applyTokenIdentity(process.env.ABLY_TOKEN, flags); } else if (flags["api-key"]) { this.applyApiKeyAuth(options, flags["api-key"], flags); } else if (process.env.ABLY_API_KEY) { @@ -1174,8 +1205,6 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { const apiKey = this.configManager.getApiKey(); if (apiKey) { options.key = apiKey; - - // Handle client ID for API key auth this.setClientId(options, flags); } } @@ -1549,18 +1578,90 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { this.setClientId(options, flags); } - private setClientId(options: Ably.ClientOptions, flags: BaseFlags): void { - if (flags["client-id"]) { - // Special case: "none" means explicitly no client ID - if (flags["client-id"].toLowerCase() === "none") { - // Don't set clientId at all - } else { - options.clientId = flags["client-id"]; + /** + * The client ID this command acts as under API-key auth. Fails the command + * on a value the CLI cannot act as, such as "" or "*". + */ + protected resolveClientId(flags: BaseFlags): string | undefined { + let identity; + try { + identity = resolveClientIdentity(flags["client-id"], this.configManager); + } catch (error) { + if (error instanceof InvalidClientIdError) { + this.fail(error.message, flags, "clientId"); } - } else { - // Generate a default client ID for the CLI - options.clientId = `ably-cli-${randomUUID().slice(0, 8)}`; + + throw error; } + + if (identity.optedOut) { + this.logWarning( + `Acting with no client ID ("none") is deprecated: apps that require identified clients reject it. Omit the value to use your default client ID, or set ${CLIENT_ID_ENV_VAR}.`, + flags, + ); + } + + const clientId = + identity.clientId && + this.suffixClientIdPerProcess && + identity.source !== "flag" + ? `${identity.clientId}-${randomUUID().slice(0, 8)}` + : identity.clientId; + + this.logCliEvent( + flags, + "clientId", + "resolved", + clientId + ? `Acting as client ID ${clientId} (from ${identity.source}).` + : "Acting with no client ID.", + { clientId, source: identity.source }, + ); + + return clientId; + } + + private setClientId(options: Ably.ClientOptions, flags: BaseFlags): void { + const clientId = this.resolveClientId(flags); + if (clientId !== undefined) { + options.clientId = clientId; + } + } + + /** + * Under token auth the identity is the token's, never overridden. A JWT + * must carry a clientId; a native Ably token is opaque, so its identity is + * only known once Ably accepts it. + */ + private applyTokenIdentity(token: string, flags: BaseFlags): void { + if (flags["client-id"] && !this.shouldSuppressOutput(flags)) { + this.logWarning( + "--client-id is ignored when using token authentication; the client ID comes from the token.", + flags, + ); + } + + const identity = readTokenIdentity(token); + if (identity.kind === "jwt" && identity.clientId === undefined) { + this.fail( + `The JWT in ABLY_TOKEN has no ${CLIENT_ID_CLAIM} claim. Tokens used with the CLI must carry a client ID; re-issue it with "ably auth issue-jwt-token --client-id ".`, + flags, + "auth", + ); + } + + this.logCliEvent( + flags, + "clientId", + "resolved", + identity.kind === "jwt" + ? `Acting as client ID ${identity.clientId} (from token).` + : "Acting as the client ID bound to the token.", + { + clientId: identity.kind === "jwt" ? identity.clientId : undefined, + source: "token", + }, + ); } /** diff --git a/src/commands/auth/issue-ably-token.ts b/src/commands/auth/issue-ably-token.ts index 97c1d6a44..867e1601d 100644 --- a/src/commands/auth/issue-ably-token.ts +++ b/src/commands/auth/issue-ably-token.ts @@ -1,6 +1,5 @@ import { Flags } from "@oclif/core"; import type * as Ably from "@ably/pubsub-server"; -import { randomUUID } from "node:crypto"; import { AblyBaseCommand } from "../../base-command.js"; import { productApiFlags } from "../../flags.js"; @@ -91,8 +90,8 @@ export default class IssueAblyTokenCommand extends AblyBaseCommand { tokenParams.clientId = flags["client-id"]; } } else { - // Generate a default client ID - tokenParams.clientId = `ably-cli-${randomUUID().slice(0, 8)}`; + // Default to the identity the CLI itself acts as + tokenParams.clientId = this.resolveClientId(flags); } // Create Ably REST client and request token diff --git a/src/commands/auth/issue-jwt-token.ts b/src/commands/auth/issue-jwt-token.ts index 334ffac01..5414f8b0c 100644 --- a/src/commands/auth/issue-jwt-token.ts +++ b/src/commands/auth/issue-jwt-token.ts @@ -114,10 +114,9 @@ export default class IssueJwtTokenCommand extends AblyBaseCommand { clientId = flags["client-id"]; } } else { - // Generate a default client ID - const defaultClientId = `ably-cli-${randomUUID().slice(0, 8)}`; - jwtPayload["x-ably-clientId"] = defaultClientId; - clientId = defaultClientId; + // Default to the identity the CLI itself acts as + clientId = this.resolveClientId(flags) ?? null; + if (clientId) jwtPayload["x-ably-clientId"] = clientId; } // Sign the JWT diff --git a/src/commands/bench/publisher.ts b/src/commands/bench/publisher.ts index 249511bc6..54d2a2721 100644 --- a/src/commands/bench/publisher.ts +++ b/src/commands/bench/publisher.ts @@ -54,6 +54,8 @@ interface BenchmarkPayload { } export default class BenchPublisher extends AblyBaseCommand { + protected override suffixClientIdPerProcess = true; + static override args = { channelName: Args.string({ description: "The channel name to publish to", diff --git a/src/commands/bench/subscriber.ts b/src/commands/bench/subscriber.ts index edd121a48..ebb385672 100644 --- a/src/commands/bench/subscriber.ts +++ b/src/commands/bench/subscriber.ts @@ -20,6 +20,8 @@ interface TestMetrics { } export default class BenchSubscriber extends AblyBaseCommand { + protected override suffixClientIdPerProcess = true; + static override args = { channelName: Args.string({ description: "The channel name to subscribe to", diff --git a/src/data/env-vars.ts b/src/data/env-vars.ts index 2a21537b6..1404e59e6 100644 --- a/src/data/env-vars.ts +++ b/src/data/env-vars.ts @@ -212,6 +212,29 @@ const ABLY_APP_ID = new EnvVarEntry( [], ); +const ABLY_CLIENT_ID = new EnvVarEntry( + "ABLY_CLIENT_ID", + "Configuration", + "Client ID the CLI acts as", + "Any client ID except an empty string or `*`", + "A stable ID generated once per install", + "`--client-id` CLI flag > **`ABLY_CLIENT_ID`** > `client.id` in config > generated default", + ["channels", "rooms", "spaces", "connections", "bench", "logs", "auth"], + "Set the client ID every data plane command acts as under API key auth, so a machine or CI job keeps one identity across runs.", + new Example([ + `export ABLY_CLIENT_ID="deploy-bot"`, + `ably channels publish my-channel hello`, + ]), + [ + new DetailSection("Token authentication", [ + { + kind: "paragraph", + text: "Ignored when `ABLY_TOKEN` is set — the client ID comes from the token.", + }, + ]), + ], +); + const ABLY_CLI_CONFIG_DIR = new EnvVarEntry( "ABLY_CLI_CONFIG_DIR", "Configuration", @@ -544,6 +567,7 @@ export const ENV_VARS_DATA: EnvVarsData = new EnvVarsData( ABLY_URL, ABLY_ENDPOINT, ABLY_APP_ID, + ABLY_CLIENT_ID, ABLY_CLI_CONFIG_DIR, ABLY_HISTORY_FILE, ABLY_CLI_DEFAULT_DURATION, diff --git a/src/services/client-identity.ts b/src/services/client-identity.ts new file mode 100644 index 000000000..2e306ffe2 --- /dev/null +++ b/src/services/client-identity.ts @@ -0,0 +1,125 @@ +import jwt from "jsonwebtoken"; + +import type { ConfigManager } from "./config-manager.js"; + +/** Environment variable that sets the CLI's client ID for a whole session. */ +export const CLIENT_ID_ENV_VAR = "ABLY_CLIENT_ID"; + +/** The JWT claim that carries the client ID an Ably JWT is bound to. */ +export const CLIENT_ID_CLAIM = "x-ably-clientId"; + +/** Deprecated sentinel that opts out of a client ID entirely. */ +const NO_CLIENT_ID = "none"; + +export type ClientIdSource = "flag" | "env" | "config" | "default"; + +export interface ClientIdentity { + /** The client ID to act as, or undefined when the user opted out with "none". */ + clientId: string | undefined; + source: ClientIdSource; + /** True when the deprecated "none" sentinel was used. */ + optedOut: boolean; +} + +const SOURCE_NAMES: Record = { + flag: "--client-id", + env: CLIENT_ID_ENV_VAR, + config: "the clientId config setting", + default: "the generated default client ID", +}; + +/** A user-supplied client ID the CLI cannot act as. */ +export class InvalidClientIdError extends Error {} + +/** + * Reject client IDs that would otherwise fail obscurely: an empty value used + * to fall back to a random identity silently, and "*" is rejected by the SDK + * with an error that never names the flag the user typed. + */ +function validateClientId(value: string, source: ClientIdSource): void { + const origin = SOURCE_NAMES[source]; + if (value.trim() === "") { + throw new InvalidClientIdError( + `${origin} must not be empty. Set a client ID, or omit ${origin} to use the default.`, + ); + } + + if (value === "*") { + throw new InvalidClientIdError( + `${origin} cannot be "*". The wildcard is only valid inside a token's capability; the CLI must act as one concrete client ID.`, + ); + } +} + +function toIdentity(value: string, source: ClientIdSource): ClientIdentity { + validateClientId(value, source); + const optedOut = value.toLowerCase() === NO_CLIENT_ID; + return { clientId: optedOut ? undefined : value, source, optedOut }; +} + +let processDefaultIdentity: ClientIdentity | undefined; + +/** + * The client ID this process acts as when no --client-id is given: + * ABLY_CLIENT_ID, then the config setting, then a generated install default + * persisted in the config. Resolved once per process so every client, every + * interactive command and every minted token share one identity. + */ +function resolveDefaultClientIdentity( + configManager: ConfigManager, +): ClientIdentity { + if (processDefaultIdentity) return processDefaultIdentity; + + const envClientId = process.env[CLIENT_ID_ENV_VAR]; + const configClientId = configManager.getClientId(); + + processDefaultIdentity = + envClientId === undefined + ? configClientId === undefined + ? toIdentity(configManager.getDefaultClientId(), "default") + : toIdentity(configClientId, "config") + : toIdentity(envClientId, "env"); + + return processDefaultIdentity; +} + +/** + * Resolve the client ID to act as: --client-id > ABLY_CLIENT_ID > config + * setting > generated install default. An explicit flag always wins, so two + * terminals can still act as different clients. + * + * @throws InvalidClientIdError when the winning value is empty or "*". + */ +export function resolveClientIdentity( + flagValue: string | undefined, + configManager: ConfigManager, +): ClientIdentity { + return flagValue === undefined + ? resolveDefaultClientIdentity(configManager) + : toIdentity(flagValue, "flag"); +} + +/** Forget the memoized process identity. For tests only. */ +export function resetClientIdentityCache(): void { + processDefaultIdentity = undefined; +} + +export type TokenIdentity = + | { kind: "jwt"; clientId: string | undefined } + | { kind: "opaque" }; + +/** + * The client ID a token is bound to. A JWT carries it in a readable claim; a + * native Ably token is opaque, so its identity is only known once Ably + * accepts it. + */ +export function readTokenIdentity(token: string): TokenIdentity { + const payload = jwt.decode(token, { json: true }); + if (!payload) return { kind: "opaque" }; + + const clientId: unknown = payload[CLIENT_ID_CLAIM]; + return { + kind: "jwt", + clientId: typeof clientId === "string" ? clientId : undefined, + }; +} diff --git a/src/services/config-manager.ts b/src/services/config-manager.ts index ec9cc14aa..322ce601d 100644 --- a/src/services/config-manager.ts +++ b/src/services/config-manager.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -65,8 +66,17 @@ export interface OAuthSession { refreshToken: string; } +/** The identity the CLI acts as on the data plane, shared by every account. */ +export interface ClientConfig { + /** Client ID set by the user; wins over the generated default. */ + id?: string; + /** Stable client ID generated on first use, so runs don't mint random IDs. */ + defaultId?: string; +} + export interface AblyConfig { accounts: Record; + client?: ClientConfig; current?: { account?: string; /** @deprecated Legacy field migrated to account.currentAppId on first load */ @@ -184,9 +194,17 @@ export interface ConfigManager { // Config file getConfigPath(): string; saveConfig(): void; + + // Data plane client identity + getClientId(): string | undefined; + getDefaultClientId(): string; reloadConfig(): void; } +export function generateDefaultClientId(): string { + return `ably-cli-${randomUUID().slice(0, 8)}`; +} + // Type declaration for test mocks available on globalThis declare global { var __TEST_MOCKS__: @@ -331,6 +349,21 @@ export class TomlConfigManager implements ConfigManager { return this.configPath; } + public getClientId(): string | undefined { + return this.config.client?.id; + } + + // Get the install's default client ID, generating and persisting it on first use + public getDefaultClientId(): string { + const existing = this.config.client?.defaultId; + if (existing) return existing; + + const defaultId = generateDefaultClientId(); + this.config.client = { ...this.config.client, defaultId }; + this.saveConfig(); + return defaultId; + } + // Get the current account configuration public getCurrentAccount(): AccountConfig | undefined { const currentAlias = this.getCurrentAccountAlias(); diff --git a/test/helpers/mock-config-manager.ts b/test/helpers/mock-config-manager.ts index e2d7565cb..0b0d13e0c 100644 --- a/test/helpers/mock-config-manager.ts +++ b/test/helpers/mock-config-manager.ts @@ -34,6 +34,9 @@ import type { // and breaks interception for commands that make HTTPS requests. const DEFAULT_OAUTH_HOST = "ably.com"; +/** The generated default client ID the mock reports when none is configured. */ +export const MOCK_DEFAULT_CLIENT_ID = "ably-cli-mockdflt"; + /** * Type for test configuration values. */ @@ -269,6 +272,18 @@ export class MockConfigManager implements ConfigManager { return "/mock/config/path"; } + public getClientId(): string | undefined { + return this.config.client?.id; + } + + public getDefaultClientId(): string { + this.config.client = { + ...this.config.client, + defaultId: this.config.client?.defaultId ?? MOCK_DEFAULT_CLIENT_ID, + }; + return this.config.client.defaultId!; + } + public getEndpoint(alias?: string): string | undefined { if (alias) { return this.config.accounts[alias]?.endpoint; diff --git a/test/unit/base/base-command.test.ts b/test/unit/base/base-command.test.ts index 29be6d15f..4fe2b3d0c 100644 --- a/test/unit/base/base-command.test.ts +++ b/test/unit/base/base-command.test.ts @@ -142,6 +142,8 @@ describe("AblyBaseCommand", function () { setCurrentApp: vi.fn(), storeAppInfo: vi.fn(), storeAppKey: vi.fn(), + getClientId: vi.fn(), + getDefaultClientId: vi.fn().mockReturnValue("ably-cli-default"), } as MockConfigManager; // Instead of stubbing loadConfig which is private, we'll stub methods that might access the file system diff --git a/test/unit/base/client-identity.test.ts b/test/unit/base/client-identity.test.ts new file mode 100644 index 000000000..42738f362 --- /dev/null +++ b/test/unit/base/client-identity.test.ts @@ -0,0 +1,204 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { Config } from "@oclif/core"; +import jwt from "jsonwebtoken"; + +import { AblyBaseCommand } from "../../../src/base-command.js"; +import { + CLIENT_ID_ENV_VAR, + readTokenIdentity, + resolveClientIdentity, +} from "../../../src/services/client-identity.js"; +import { BaseFlags } from "../../../src/types/cli.js"; +import { + MOCK_DEFAULT_CLIENT_ID, + getMockConfigManager, +} from "../../helpers/mock-config-manager.js"; + +class TestCommand extends AblyBaseCommand { + async run(): Promise {} + + public testGetClientOptions(flags: BaseFlags) { + return this.getClientOptions(flags); + } +} + +class SuffixedCommand extends TestCommand { + protected override suffixClientIdPerProcess = true; +} + +function newCommand(Command = TestCommand): TestCommand { + return new Command([], { runHook: vi.fn() } as unknown as Config); +} + +const API_KEY = "appId.keyId:keySecret"; + +function signJwt(claims: Record): string { + return jwt.sign(claims, "keySecret", { + algorithm: "HS256", + keyid: "appId.keyId", + }); +} + +describe("client identity", () => { + beforeEach(() => { + process.env.ABLY_API_KEY = API_KEY; + }); + + afterEach(() => { + delete process.env.ABLY_API_KEY; + delete process.env.ABLY_TOKEN; + delete process.env[CLIENT_ID_ENV_VAR]; + }); + + describe("resolveClientIdentity", () => { + it("uses the generated install default when nothing is set", () => { + expect(resolveClientIdentity(undefined, getMockConfigManager())).toEqual({ + clientId: MOCK_DEFAULT_CLIENT_ID, + source: "default", + optedOut: false, + }); + }); + + it("prefers the config setting over the generated default", () => { + const config = getMockConfigManager(); + config.getConfig().client = { id: "configured" }; + expect(resolveClientIdentity(undefined, config).clientId).toBe( + "configured", + ); + }); + + it("prefers ABLY_CLIENT_ID over the config setting", () => { + const config = getMockConfigManager(); + config.getConfig().client = { id: "configured" }; + process.env[CLIENT_ID_ENV_VAR] = "from-env"; + expect(resolveClientIdentity(undefined, config)).toMatchObject({ + clientId: "from-env", + source: "env", + }); + }); + + it("prefers an explicit flag over everything else", () => { + process.env[CLIENT_ID_ENV_VAR] = "from-env"; + expect( + resolveClientIdentity("from-flag", getMockConfigManager()), + ).toMatchObject({ clientId: "from-flag", source: "flag" }); + }); + + it("resolves the default once per process", () => { + const config = getMockConfigManager(); + const first = resolveClientIdentity(undefined, config); + process.env[CLIENT_ID_ENV_VAR] = "changed-later"; + expect(resolveClientIdentity(undefined, config)).toBe(first); + }); + + it("treats 'none' as an opt-out", () => { + expect( + resolveClientIdentity("none", getMockConfigManager()), + ).toMatchObject({ clientId: undefined, optedOut: true }); + }); + + it.each(["", " ", "*"])("rejects %j", (value) => { + expect(() => + resolveClientIdentity(value, getMockConfigManager()), + ).toThrow(/--client-id/); + }); + + it("names ABLY_CLIENT_ID when the env var is invalid", () => { + process.env[CLIENT_ID_ENV_VAR] = "*"; + expect(() => + resolveClientIdentity(undefined, getMockConfigManager()), + ).toThrow(/ABLY_CLIENT_ID/); + }); + }); + + describe("readTokenIdentity", () => { + it("reads the client ID claim from a JWT", () => { + expect( + readTokenIdentity(signJwt({ "x-ably-clientId": "alice" })), + ).toEqual({ kind: "jwt", clientId: "alice" }); + }); + + it("reports a JWT without a client ID", () => { + expect(readTokenIdentity(signJwt({}))).toEqual({ + kind: "jwt", + clientId: undefined, + }); + }); + + it("treats a native Ably token as opaque", () => { + expect(readTokenIdentity("xVLyHw.opaque")).toEqual({ kind: "opaque" }); + }); + }); + + describe("getClientOptions", () => { + it("acts as the stable default client ID", () => { + const first = newCommand().testGetClientOptions({}); + const second = newCommand().testGetClientOptions({}); + expect(first.clientId).toBe(MOCK_DEFAULT_CLIENT_ID); + expect(second.clientId).toBe(MOCK_DEFAULT_CLIENT_ID); + }); + + it("acts as ABLY_CLIENT_ID when set", () => { + process.env[CLIENT_ID_ENV_VAR] = "deploy-bot"; + expect(newCommand().testGetClientOptions({}).clientId).toBe("deploy-bot"); + }); + + it("acts as an explicit --client-id", () => { + expect( + newCommand().testGetClientOptions({ "client-id": "alice" }).clientId, + ).toBe("alice"); + }); + + it("omits the client ID and warns for the deprecated 'none'", () => { + const command = newCommand(); + const stderr = vi + .spyOn(command as unknown as { logToStderr: () => void }, "logToStderr") + .mockImplementation(() => {}); + + const options = command.testGetClientOptions({ "client-id": "none" }); + + expect(options.clientId).toBeUndefined(); + expect(stderr).toHaveBeenCalledWith( + expect.stringContaining("deprecated"), + ); + }); + + it.each(["", "*"])("fails for --client-id %j", (value) => { + expect(() => + newCommand().testGetClientOptions({ "client-id": value }), + ).toThrow(/--client-id/); + }); + + it("suffixes the default per process when the command asks", () => { + const options = newCommand(SuffixedCommand).testGetClientOptions({}); + expect(options.clientId).toMatch( + new RegExp(`^${MOCK_DEFAULT_CLIENT_ID}-[0-9a-f]{8}$`), + ); + }); + + it("never suffixes an explicit --client-id", () => { + expect( + newCommand(SuffixedCommand).testGetClientOptions({ + "client-id": "alice", + }).clientId, + ).toBe("alice"); + }); + + it("never sets a client ID over a token's", () => { + process.env.ABLY_TOKEN = signJwt({ "x-ably-clientId": "alice" }); + expect(newCommand().testGetClientOptions({}).clientId).toBeUndefined(); + }); + + it("fails for a JWT without a client ID", () => { + process.env.ABLY_TOKEN = signJwt({}); + expect(() => newCommand().testGetClientOptions({})).toThrow( + /x-ably-clientId/, + ); + }); + + it("accepts an opaque native token", () => { + process.env.ABLY_TOKEN = "xVLyHw.opaque"; + expect(newCommand().testGetClientOptions({}).token).toBe("xVLyHw.opaque"); + }); + }); +}); diff --git a/test/unit/commands/env.test.ts b/test/unit/commands/env.test.ts index 69d0e1bf3..b64b20a28 100644 --- a/test/unit/commands/env.test.ts +++ b/test/unit/commands/env.test.ts @@ -13,6 +13,7 @@ const CANONICAL_NAMES = [ "ABLY_ACCESS_TOKEN", "ABLY_ENDPOINT", "ABLY_APP_ID", + "ABLY_CLIENT_ID", "ABLY_CLI_CONFIG_DIR", "ABLY_HISTORY_FILE", "ABLY_CLI_DEFAULT_DURATION", @@ -81,7 +82,7 @@ describe("env command", () => { expect(result).toHaveProperty("type", "result"); expect(result).toHaveProperty("command", "env"); expect(result).toHaveProperty("success", true); - expect(result.envVars).toHaveLength(10); + expect(result.envVars).toHaveLength(11); expect(result.envVars[0]).toMatchObject({ name: "ABLY_API_KEY", category: "Authentication", @@ -89,7 +90,7 @@ describe("env command", () => { }); expect(result.envVars[3].name).toBe("ABLY_URL"); expect(result.envVars[4].name).toBe("ABLY_ENDPOINT"); - expect(result.envVars[9].name).toBe("ABLY_CLI_NON_INTERACTIVE"); + expect(result.envVars[10].name).toBe("ABLY_CLI_NON_INTERACTIVE"); expect(result.crossCutting).toBeDefined(); expect(result.crossCutting.authResolutionOrder.heading).toBe( "Authentication Resolution Order", diff --git a/test/unit/data/env-vars.test.ts b/test/unit/data/env-vars.test.ts index 42548a142..ac8a4f76d 100644 --- a/test/unit/data/env-vars.test.ts +++ b/test/unit/data/env-vars.test.ts @@ -17,6 +17,7 @@ const CANONICAL_NAMES = [ "ABLY_URL", "ABLY_ENDPOINT", "ABLY_APP_ID", + "ABLY_CLIENT_ID", "ABLY_CLI_CONFIG_DIR", "ABLY_HISTORY_FILE", "ABLY_CLI_DEFAULT_DURATION", @@ -28,8 +29,8 @@ describe("ENV_VARS_DATA", () => { expect(ENV_VARS_DATA).toBeInstanceOf(EnvVarsData); }); - it("lists exactly 10 variables in canonical order", () => { - expect(ENV_VARS_DATA.variables).toHaveLength(10); + it("lists exactly 11 variables in canonical order", () => { + expect(ENV_VARS_DATA.variables).toHaveLength(11); expect(ENV_VARS_DATA.variables.map((v) => v.name)).toEqual(CANONICAL_NAMES); }); @@ -73,6 +74,7 @@ describe("ENV_VARS_DATA", () => { expect(byName.ABLY_TOKEN.details).toHaveLength(4); expect(byName.ABLY_ACCESS_TOKEN.details).toHaveLength(1); expect(byName.ABLY_APP_ID.details).toHaveLength(0); + expect(byName.ABLY_CLIENT_ID.details).toHaveLength(1); expect(byName.ABLY_CLI_CONFIG_DIR.details).toHaveLength(0); expect(byName.ABLY_HISTORY_FILE.details).toHaveLength(1); expect(byName.ABLY_CLI_DEFAULT_DURATION.details).toHaveLength(0); diff --git a/test/unit/setup.ts b/test/unit/setup.ts index 5fd9415e6..3ae8287d8 100644 --- a/test/unit/setup.ts +++ b/test/unit/setup.ts @@ -9,6 +9,7 @@ */ import { beforeAll, beforeEach, vi } from "vitest"; +import { resetClientIdentityCache } from "../../src/services/client-identity.js"; import { initializeMockConfigManager, resetMockConfig, @@ -46,6 +47,7 @@ beforeEach(() => { // Reset each mock's internal state resetMockConfig(); + resetClientIdentityCache(); resetMockAblyRealtime(); resetMockAblyRest(); resetMockAblySpaces(); From f432c0a054d979fce8c238e2d8e3df4e904b3b2b Mon Sep 17 00:00:00 2001 From: evgeny Date: Mon, 12 Oct 2026 19:37:29 +0100 Subject: [PATCH 2/2] feat(mau): address review feedback on client identity - Resolve the acting identity once per command, bench suffix included, so the banner shows the ID the clients connect as - Route explicit --client-id on the token commands through the shared resolver: "none" now warns as deprecated, and "*" is allowed so wildcard tokens can still be minted. With an explicit --client-id the issuing client acts as the token's identity, so the default identity is never resolved - Under a wildcard ("*") JWT, act as --client-id, ABLY_CLIENT_ID or the default client ID, since the token lets its holder choose - Keep the generated default ID in memory if the config can't be written, instead of failing every command - Name the client.id config setting correctly in errors, say which source a deprecated "none" came from, and update the --client-id descriptions Tests cover the banner matching the suffixed ID, wildcard tokens when minting and connecting, source names in messages, and the default ID's first-write and read-only-config cases. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/base-command.ts | 130 ++++++++++++++---- src/commands/auth/issue-ably-token.ts | 16 +-- src/commands/auth/issue-jwt-token.ts | 20 +-- src/data/env-vars.ts | 4 +- src/flags.ts | 2 +- src/services/client-identity.ts | 46 +++++-- src/services/config-manager.ts | 8 +- test/unit/base/client-identity.test.ts | 67 +++++++++ .../commands/auth/issue-ably-token.test.ts | 47 +++++++ .../commands/auth/issue-jwt-token.test.ts | 15 ++ test/unit/services/config-manager.test.ts | 25 ++++ 11 files changed, 307 insertions(+), 73 deletions(-) diff --git a/src/base-command.ts b/src/base-command.ts index e3458646f..2026b684b 100644 --- a/src/base-command.ts +++ b/src/base-command.ts @@ -15,6 +15,8 @@ import { CLIENT_ID_CLAIM, CLIENT_ID_ENV_VAR, InvalidClientIdError, + type ClientIdentity, + describeClientIdSource, readTokenIdentity, resolveClientIdentity, } from "./services/client-identity.js"; @@ -160,6 +162,10 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { */ protected suffixClientIdPerProcess = false; + // The identity this command acts as, suffix included, so the banner and + // every client this command builds agree on it. + private actingIdentity: ClientIdentity | undefined; + // Core global flags available to all commands (verbose, json, pretty-json, web-cli-help) static globalFlags = { ...coreGlobalFlags }; @@ -768,17 +774,42 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { private bannerClientId(flags: BaseFlags): string | undefined { if (process.env.ABLY_TOKEN) { const identity = readTokenIdentity(process.env.ABLY_TOKEN); - return identity.kind === "jwt" ? identity.clientId : undefined; + if (identity.kind !== "jwt") return undefined; + if (identity.clientId !== "*") return identity.clientId; } try { - return resolveClientIdentity(flags["client-id"], this.configManager) - .clientId; + return this.resolveActingIdentity(flags).clientId; } catch { return undefined; } } + /** + * The identity this command acts as under API-key auth or a wildcard + * token, resolved once. + * + * @throws InvalidClientIdError when the winning value is empty or "*". + */ + private resolveActingIdentity(flags: BaseFlags): ClientIdentity { + if (this.actingIdentity) return this.actingIdentity; + + const identity = resolveClientIdentity( + flags["client-id"], + this.configManager, + ); + this.actingIdentity = + identity.clientId && + this.suffixClientIdPerProcess && + identity.source !== "flag" + ? { + ...identity, + clientId: `${identity.clientId}-${randomUUID().slice(0, 8)}`, + } + : identity; + return this.actingIdentity; + } + /** * Resolve host, port and TLS for the data plane from the first source that * supplies them: `--url`, `ABLY_URL`, `ABLY_ENDPOINT`, then the account @@ -1196,7 +1227,7 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { // Handle authentication: ABLY_TOKEN env → flags["api-key"] (set by ensureAppAndKey) → ABLY_API_KEY env → config if (process.env.ABLY_TOKEN) { options.token = process.env.ABLY_TOKEN; - this.applyTokenIdentity(process.env.ABLY_TOKEN, flags); + this.applyTokenIdentity(options, process.env.ABLY_TOKEN, flags); } else if (flags["api-key"]) { this.applyApiKeyAuth(options, flags["api-key"], flags); } else if (process.env.ABLY_API_KEY) { @@ -1583,9 +1614,61 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { * on a value the CLI cannot act as, such as "" or "*". */ protected resolveClientId(flags: BaseFlags): string | undefined { + const identity = this.validatedIdentity(flags, () => + this.resolveActingIdentity(flags), + ); + + this.logCliEvent( + flags, + "clientId", + "resolved", + identity.clientId + ? `Acting as client ID ${identity.clientId} (from ${identity.source}).` + : "Acting with no client ID.", + { clientId: identity.clientId, source: identity.source }, + ); + + return identity.clientId; + } + + /** + * The client ID to bind a minted token to: --client-id when given, which + * may be "*" to let the holder assume any identity, otherwise the identity + * the CLI itself acts as. + */ + protected resolveTokenClientId(flags: BaseFlags): string | undefined { + if (flags["client-id"] === undefined) return this.resolveClientId(flags); + + const identity = this.validatedIdentity(flags, () => + resolveClientIdentity(flags["client-id"], this.configManager, { + allowWildcard: true, + }), + ); + + // The client that issues the token acts as the token's identity, so the + // default identity is never resolved. It cannot act as "*", and only + // signs with the key, so it then acts with no client ID. The "none" + // warning has already been shown. + this.actingIdentity = { + ...identity, + clientId: identity.clientId === "*" ? undefined : identity.clientId, + optedOut: false, + }; + + return identity.clientId; + } + + /** + * Resolve an identity, failing the command on an invalid value and warning + * when the deprecated "none" opt-out was used. + */ + private validatedIdentity( + flags: BaseFlags, + resolve: () => ClientIdentity, + ): ClientIdentity { let identity; try { - identity = resolveClientIdentity(flags["client-id"], this.configManager); + identity = resolve(); } catch (error) { if (error instanceof InvalidClientIdError) { this.fail(error.message, flags, "clientId"); @@ -1596,29 +1679,12 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { if (identity.optedOut) { this.logWarning( - `Acting with no client ID ("none") is deprecated: apps that require identified clients reject it. Omit the value to use your default client ID, or set ${CLIENT_ID_ENV_VAR}.`, + `Acting with no client ID ("none" from ${describeClientIdSource(identity.source)}) is deprecated: apps that require identified clients reject it. Remove it to use your default client ID, or set ${CLIENT_ID_ENV_VAR}.`, flags, ); } - const clientId = - identity.clientId && - this.suffixClientIdPerProcess && - identity.source !== "flag" - ? `${identity.clientId}-${randomUUID().slice(0, 8)}` - : identity.clientId; - - this.logCliEvent( - flags, - "clientId", - "resolved", - clientId - ? `Acting as client ID ${clientId} (from ${identity.source}).` - : "Acting with no client ID.", - { clientId, source: identity.source }, - ); - - return clientId; + return identity; } private setClientId(options: Ably.ClientOptions, flags: BaseFlags): void { @@ -1631,9 +1697,20 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { /** * Under token auth the identity is the token's, never overridden. A JWT * must carry a clientId; a native Ably token is opaque, so its identity is - * only known once Ably accepts it. + * only known once Ably accepts it. A wildcard ("*") JWT lets its holder + * pick an identity, so the CLI acts as the one it would under API-key auth. */ - private applyTokenIdentity(token: string, flags: BaseFlags): void { + private applyTokenIdentity( + options: Ably.ClientOptions, + token: string, + flags: BaseFlags, + ): void { + const identity = readTokenIdentity(token); + if (identity.kind === "jwt" && identity.clientId === "*") { + this.setClientId(options, flags); + return; + } + if (flags["client-id"] && !this.shouldSuppressOutput(flags)) { this.logWarning( "--client-id is ignored when using token authentication; the client ID comes from the token.", @@ -1641,7 +1718,6 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { ); } - const identity = readTokenIdentity(token); if (identity.kind === "jwt" && identity.clientId === undefined) { this.fail( `The JWT in ABLY_TOKEN has no ${CLIENT_ID_CLAIM} claim. Tokens used with the CLI must carry a client ID; re-issue it with "ably auth issue-jwt-token --client-id ".`, diff --git a/src/commands/auth/issue-ably-token.ts b/src/commands/auth/issue-ably-token.ts index 867e1601d..01db88e6b 100644 --- a/src/commands/auth/issue-ably-token.ts +++ b/src/commands/auth/issue-ably-token.ts @@ -37,7 +37,7 @@ export default class IssueAblyTokenCommand extends AblyBaseCommand { }), "client-id": Flags.string({ description: - 'Client ID to associate with the token. Use "none" to explicitly issue a token with no client ID, otherwise a default will be generated.', + 'Client ID to bind the token to. Use "*" to let the token holder act as any client ID. Defaults to the client ID the CLI acts as.', }), "token-only": Flags.boolean({ default: false, @@ -81,18 +81,8 @@ export default class IssueAblyTokenCommand extends AblyBaseCommand { ttl: flags.ttl * 1000, // Convert to milliseconds for Ably SDK }; - // Handle client ID - use special "none" value to explicitly indicate no clientId - if (flags["client-id"]) { - if (flags["client-id"].toLowerCase() === "none") { - // No client ID - leave clientId undefined in the token params - } else { - // Use the provided client ID - tokenParams.clientId = flags["client-id"]; - } - } else { - // Default to the identity the CLI itself acts as - tokenParams.clientId = this.resolveClientId(flags); - } + const clientId = this.resolveTokenClientId(flags); + if (clientId !== undefined) tokenParams.clientId = clientId; // Create Ably REST client and request token const rest = await this.createAblyRestClient( diff --git a/src/commands/auth/issue-jwt-token.ts b/src/commands/auth/issue-jwt-token.ts index 5414f8b0c..8abb6b7dd 100644 --- a/src/commands/auth/issue-jwt-token.ts +++ b/src/commands/auth/issue-jwt-token.ts @@ -46,7 +46,7 @@ export default class IssueJwtTokenCommand extends AblyBaseCommand { }), "client-id": Flags.string({ description: - 'Client ID to associate with the token. Use "none" to explicitly issue a token with no client ID, otherwise a default will be generated.', + 'Client ID to bind the token to. Use "*" to let the token holder act as any client ID. Defaults to the client ID the CLI acts as.', }), "token-only": Flags.boolean({ default: false, @@ -102,22 +102,8 @@ export default class IssueJwtTokenCommand extends AblyBaseCommand { "x-ably-capability": capabilities, }; - // Handle client ID - use special "none" value to explicitly indicate no clientId - let clientId: null | string = null; - if (flags["client-id"]) { - if (flags["client-id"].toLowerCase() === "none") { - // No client ID - don't add it to the token - clientId = null; - } else { - // Use the provided client ID - jwtPayload["x-ably-clientId"] = flags["client-id"]; - clientId = flags["client-id"]; - } - } else { - // Default to the identity the CLI itself acts as - clientId = this.resolveClientId(flags) ?? null; - if (clientId) jwtPayload["x-ably-clientId"] = clientId; - } + const clientId = this.resolveTokenClientId(flags) ?? null; + if (clientId) jwtPayload["x-ably-clientId"] = clientId; // Sign the JWT const token = jwt.sign(jwtPayload, keySecret, { diff --git a/src/data/env-vars.ts b/src/data/env-vars.ts index 1404e59e6..d0f8772fb 100644 --- a/src/data/env-vars.ts +++ b/src/data/env-vars.ts @@ -158,7 +158,7 @@ const ABLY_TOKEN = new EnvVarEntry( new DetailSection("Client ID", [ { kind: "paragraph", - text: "`--client-id` is ignored when `ABLY_TOKEN` is set — the client ID is embedded in the token. A warning is logged if `--client-id` is passed.", + text: "`--client-id` is ignored when `ABLY_TOKEN` is set — the client ID is embedded in the token. A warning is logged if `--client-id` is passed. The exception is a JWT whose client ID is `*`, which lets its holder choose: the CLI then acts as `--client-id`, `ABLY_CLIENT_ID`, or its default client ID.", }, ]), new DetailSection("Token expiry", [ @@ -229,7 +229,7 @@ const ABLY_CLIENT_ID = new EnvVarEntry( new DetailSection("Token authentication", [ { kind: "paragraph", - text: "Ignored when `ABLY_TOKEN` is set — the client ID comes from the token.", + text: "Ignored when `ABLY_TOKEN` is set — the client ID comes from the token — unless the token is a JWT whose client ID is `*`. A wildcard token lets its holder choose, so the CLI acts as `ABLY_CLIENT_ID` (or `--client-id`).", }, ]), ], diff --git a/src/flags.ts b/src/flags.ts index aec47cbb4..a5803c36a 100644 --- a/src/flags.ts +++ b/src/flags.ts @@ -75,7 +75,7 @@ export const hiddenControlApiFlags = { export const clientIdFlag = { "client-id": Flags.string({ description: - 'Overrides any default client ID when using API authentication. Use "none" to explicitly set no client ID. Not applicable when using token authentication.', + 'Client ID to act as. Defaults to ABLY_CLIENT_ID, or a stable ID generated for this install. Ignored under token authentication unless the token\'s client ID is "*".', }), }; diff --git a/src/services/client-identity.ts b/src/services/client-identity.ts index 2e306ffe2..c94c001c6 100644 --- a/src/services/client-identity.ts +++ b/src/services/client-identity.ts @@ -8,6 +8,9 @@ export const CLIENT_ID_ENV_VAR = "ABLY_CLIENT_ID"; /** The JWT claim that carries the client ID an Ably JWT is bound to. */ export const CLIENT_ID_CLAIM = "x-ably-clientId"; +/** The JWT claim that wraps a native Ably token inside a JWT. */ +const WRAPPED_TOKEN_CLAIM = "x-ably-token"; + /** Deprecated sentinel that opts out of a client ID entirely. */ const NO_CLIENT_ID = "none"; @@ -24,19 +27,29 @@ export interface ClientIdentity { const SOURCE_NAMES: Record = { flag: "--client-id", env: CLIENT_ID_ENV_VAR, - config: "the clientId config setting", + config: "the client.id config setting", default: "the generated default client ID", }; +/** How a resolved client ID's source is named in messages to the user. */ +export function describeClientIdSource(source: ClientIdSource): string { + return SOURCE_NAMES[source]; +} + /** A user-supplied client ID the CLI cannot act as. */ export class InvalidClientIdError extends Error {} /** * Reject client IDs that would otherwise fail obscurely: an empty value used * to fall back to a random identity silently, and "*" is rejected by the SDK - * with an error that never names the flag the user typed. + * with an error that never names the flag the user typed. A minted token may + * still carry "*", which lets its holder assume any identity. */ -function validateClientId(value: string, source: ClientIdSource): void { +function validateClientId( + value: string, + source: ClientIdSource, + allowWildcard: boolean, +): void { const origin = SOURCE_NAMES[source]; if (value.trim() === "") { throw new InvalidClientIdError( @@ -44,15 +57,19 @@ function validateClientId(value: string, source: ClientIdSource): void { ); } - if (value === "*") { + if (value === "*" && !allowWildcard) { throw new InvalidClientIdError( - `${origin} cannot be "*". The wildcard is only valid inside a token's capability; the CLI must act as one concrete client ID.`, + `${origin} cannot be "*". The wildcard is only valid in a token's client ID; the CLI must act as one concrete client ID.`, ); } } -function toIdentity(value: string, source: ClientIdSource): ClientIdentity { - validateClientId(value, source); +function toIdentity( + value: string, + source: ClientIdSource, + allowWildcard = false, +): ClientIdentity { + validateClientId(value, source, allowWildcard); const optedOut = value.toLowerCase() === NO_CLIENT_ID; return { clientId: optedOut ? undefined : value, source, optedOut }; } @@ -88,15 +105,20 @@ function resolveDefaultClientIdentity( * setting > generated install default. An explicit flag always wins, so two * terminals can still act as different clients. * - * @throws InvalidClientIdError when the winning value is empty or "*". + * `allowWildcard` accepts "*" from the flag, for commands that mint a token + * rather than act as the client ID. + * + * @throws InvalidClientIdError when the winning value is empty or a + * disallowed "*". */ export function resolveClientIdentity( flagValue: string | undefined, configManager: ConfigManager, + { allowWildcard = false }: { allowWildcard?: boolean } = {}, ): ClientIdentity { return flagValue === undefined ? resolveDefaultClientIdentity(configManager) - : toIdentity(flagValue, "flag"); + : toIdentity(flagValue, "flag", allowWildcard); } /** Forget the memoized process identity. For tests only. */ @@ -110,12 +132,12 @@ export type TokenIdentity = /** * The client ID a token is bound to. A JWT carries it in a readable claim; a - * native Ably token is opaque, so its identity is only known once Ably - * accepts it. + * native Ably token, alone or wrapped in a JWT, is opaque, so its identity is + * only known once Ably accepts it. */ export function readTokenIdentity(token: string): TokenIdentity { const payload = jwt.decode(token, { json: true }); - if (!payload) return { kind: "opaque" }; + if (!payload || WRAPPED_TOKEN_CLAIM in payload) return { kind: "opaque" }; const clientId: unknown = payload[CLIENT_ID_CLAIM]; return { diff --git a/src/services/config-manager.ts b/src/services/config-manager.ts index 322ce601d..a397348ed 100644 --- a/src/services/config-manager.ts +++ b/src/services/config-manager.ts @@ -360,7 +360,13 @@ export class TomlConfigManager implements ConfigManager { const defaultId = generateDefaultClientId(); this.config.client = { ...this.config.client, defaultId }; - this.saveConfig(); + try { + this.saveConfig(); + } catch { + // A read-only config (e.g. a locked-down CI container) must not break + // every command: keep the ID in memory for this process instead. + } + return defaultId; } diff --git a/test/unit/base/client-identity.test.ts b/test/unit/base/client-identity.test.ts index 42738f362..22580b234 100644 --- a/test/unit/base/client-identity.test.ts +++ b/test/unit/base/client-identity.test.ts @@ -20,6 +20,10 @@ class TestCommand extends AblyBaseCommand { public testGetClientOptions(flags: BaseFlags) { return this.getClientOptions(flags); } + + public testDisplayAuthInfo(flags: BaseFlags) { + return this.displayAuthInfo(flags); + } } class SuffixedCommand extends TestCommand { @@ -103,6 +107,22 @@ describe("client identity", () => { ).toThrow(/--client-id/); }); + it("accepts '*' from the flag only when the wildcard is allowed", () => { + expect( + resolveClientIdentity("*", getMockConfigManager(), { + allowWildcard: true, + }), + ).toMatchObject({ clientId: "*", source: "flag" }); + }); + + it("names the client.id config setting when it is invalid", () => { + const config = getMockConfigManager(); + config.getConfig().client = { id: "*" }; + expect(() => resolveClientIdentity(undefined, config)).toThrow( + /client\.id config setting/, + ); + }); + it("names ABLY_CLIENT_ID when the env var is invalid", () => { process.env[CLIENT_ID_ENV_VAR] = "*"; expect(() => @@ -128,6 +148,12 @@ describe("client identity", () => { it("treats a native Ably token as opaque", () => { expect(readTokenIdentity("xVLyHw.opaque")).toEqual({ kind: "opaque" }); }); + + it("treats a JWT wrapping a native Ably token as opaque", () => { + expect( + readTokenIdentity(signJwt({ "x-ably-token": "xVLyHw.opaque" })), + ).toEqual({ kind: "opaque" }); + }); }); describe("getClientOptions", () => { @@ -163,6 +189,20 @@ describe("client identity", () => { ); }); + it("names where a deprecated 'none' came from", () => { + process.env[CLIENT_ID_ENV_VAR] = "none"; + const command = newCommand(); + const stderr = vi + .spyOn(command as unknown as { logToStderr: () => void }, "logToStderr") + .mockImplementation(() => {}); + + command.testGetClientOptions({}); + + expect(stderr).toHaveBeenCalledWith( + expect.stringContaining(`"none" from ${CLIENT_ID_ENV_VAR}`), + ); + }); + it.each(["", "*"])("fails for --client-id %j", (value) => { expect(() => newCommand().testGetClientOptions({ "client-id": value }), @@ -176,6 +216,19 @@ describe("client identity", () => { ); }); + it("shows the suffixed client ID it acts as in the banner", async () => { + const command = newCommand(SuffixedCommand); + const log = vi + .spyOn(command as unknown as { log: (msg: string) => void }, "log") + .mockImplementation(() => {}); + + const { clientId } = command.testGetClientOptions({}); + await command.testDisplayAuthInfo({}); + + expect(clientId).toMatch(/-[0-9a-f]{8}$/); + expect(log).toHaveBeenCalledWith(expect.stringContaining(clientId!)); + }); + it("never suffixes an explicit --client-id", () => { expect( newCommand(SuffixedCommand).testGetClientOptions({ @@ -189,6 +242,20 @@ describe("client identity", () => { expect(newCommand().testGetClientOptions({}).clientId).toBeUndefined(); }); + it("acts as --client-id under a wildcard JWT", () => { + process.env.ABLY_TOKEN = signJwt({ "x-ably-clientId": "*" }); + expect( + newCommand().testGetClientOptions({ "client-id": "alice" }).clientId, + ).toBe("alice"); + }); + + it("acts as the default client ID under a wildcard JWT", () => { + process.env.ABLY_TOKEN = signJwt({ "x-ably-clientId": "*" }); + expect(newCommand().testGetClientOptions({}).clientId).toBe( + MOCK_DEFAULT_CLIENT_ID, + ); + }); + it("fails for a JWT without a client ID", () => { process.env.ABLY_TOKEN = signJwt({}); expect(() => newCommand().testGetClientOptions({})).toThrow( diff --git a/test/unit/commands/auth/issue-ably-token.test.ts b/test/unit/commands/auth/issue-ably-token.test.ts index 4390dac61..3933bd417 100644 --- a/test/unit/commands/auth/issue-ably-token.test.ts +++ b/test/unit/commands/auth/issue-ably-token.test.ts @@ -148,6 +148,53 @@ describe("auth:issue-ably-token command", () => { expect(tokenParams.clientId).toBeUndefined(); }); + it("should issue a wildcard token for --client-id '*'", async () => { + const restMock = getMockAblyRest(); + restMock.auth.createTokenRequest.mockResolvedValue({}); + restMock.auth.requestToken.mockResolvedValue({ + token: "mock-ably-token-wildcard", + issued: Date.now(), + expires: Date.now() + 3600000, + capability: '{"*":["*"]}', + clientId: "*", + }); + + const { error } = await runCommand( + ["auth:issue-ably-token", "--client-id", "*"], + import.meta.url, + ); + + expect(error).toBeUndefined(); + const tokenParams = restMock.auth.createTokenRequest.mock.calls[0][0]; + expect(tokenParams.clientId).toBe("*"); + }); + + it("should not resolve the default identity for an explicit --client-id", async () => { + const restMock = getMockAblyRest(); + restMock.auth.createTokenRequest.mockResolvedValue({}); + restMock.auth.requestToken.mockResolvedValue({ + token: "mock-ably-token-explicit", + issued: Date.now(), + expires: Date.now() + 3600000, + capability: '{"*":["*"]}', + clientId: "alice", + }); + process.env.ABLY_CLIENT_ID = "*"; + + try { + const { error } = await runCommand( + ["auth:issue-ably-token", "--client-id", "alice"], + import.meta.url, + ); + + expect(error).toBeUndefined(); + const tokenParams = restMock.auth.createTokenRequest.mock.calls[0][0]; + expect(tokenParams.clientId).toBe("alice"); + } finally { + delete process.env.ABLY_CLIENT_ID; + } + }); + it("should output only token string with --token-only flag", async () => { const restMock = getMockAblyRest(); const mockTokenString = "mock-ably-token-only"; diff --git a/test/unit/commands/auth/issue-jwt-token.test.ts b/test/unit/commands/auth/issue-jwt-token.test.ts index 5790ca336..216d411ef 100644 --- a/test/unit/commands/auth/issue-jwt-token.test.ts +++ b/test/unit/commands/auth/issue-jwt-token.test.ts @@ -128,6 +128,21 @@ describe("auth:issue-jwt-token command", () => { expect(decoded["x-ably-clientId"]).toBeUndefined(); }); + it("should issue a wildcard token for --client-id '*'", async () => { + const apiKey = getMockConfigManager().getApiKey()!; + const mockKeySecret = apiKey.split(":")[1]; + const { stdout } = await runCommand( + ["auth:issue-jwt-token", "--client-id", "*", "--token-only"], + import.meta.url, + ); + + const decoded = jwt.verify(stdout.trim(), mockKeySecret, { + algorithms: ["HS256"], + }) as jwt.JwtPayload; + + expect(decoded["x-ably-clientId"]).toBe("*"); + }); + it("should output only token string with --token-only flag", async () => { const { stdout } = await runCommand( ["auth:issue-jwt-token", "--token-only"], diff --git a/test/unit/services/config-manager.test.ts b/test/unit/services/config-manager.test.ts index fdac1f3cd..809836213 100644 --- a/test/unit/services/config-manager.test.ts +++ b/test/unit/services/config-manager.test.ts @@ -501,4 +501,29 @@ accessToken = "testaccesstoken" }); }); }); + + describe("#getDefaultClientId", () => { + it("generates and persists a default on first use", () => { + const id = configManager.getDefaultClientId(); + + expect(id).toMatch(/^ably-cli-[0-9a-f]{8}$/); + expect(fs.writeFileSync).toHaveBeenCalledWith( + expect.any(String), + expect.stringContaining(`defaultId = "${id}"`), + expect.anything(), + ); + expect(configManager.getDefaultClientId()).toBe(id); + }); + + it("keeps the default in memory when the config cannot be written", () => { + vi.mocked(fs.writeFileSync).mockImplementation(() => { + throw new Error("EROFS: read-only file system"); + }); + + const id = configManager.getDefaultClientId(); + + expect(id).toMatch(/^ably-cli-[0-9a-f]{8}$/); + expect(configManager.getDefaultClientId()).toBe(id); + }); + }); });