Fix account login OAuth metadata and control-host fallback

cursoragent · cursoragent · commit 8f8f070526dc · 2026-03-04T11:06:26.000Z
diff --git a/src/commands/accounts/login.ts b/src/commands/accounts/login.ts
@@ -98,21 +98,23 @@ export default class AccountsLogin extends ControlBaseCommand {
         controlHost: flags["control-host"],
       });
 
-      const [{ user }, accounts] = await Promise.all([
+      const [{ account, user }, accounts] = await Promise.all([
         controlApi.getMe(),
         controlApi.getAccounts(),
       ]);
 
       let selectedAccountInfo: { id: string; name: string };
 
-      if (accounts.length === 1) {
+      if (accounts.length === 0) {
+        selectedAccountInfo = { id: account.id, name: account.name };
+      } else if (accounts.length === 1) {
         selectedAccountInfo = accounts[0];
       } else if (accounts.length > 1 && !this.shouldOutputJson(flags)) {
         const picked =
           await this.interactiveHelper.selectAccountFromApi(accounts);
         selectedAccountInfo = picked ?? accounts[0];
       } else {
-        // Multiple accounts in JSON mode or empty (backward compat: use first)
+        // Multiple accounts in JSON mode (backward compat: use first)
         selectedAccountInfo = accounts[0];
       }
 
@@ -126,10 +128,14 @@ export default class AccountsLogin extends ControlBaseCommand {
 
       // Store based on auth method
       if (oauthTokens) {
-        this.configManager.storeOAuthTokens(alias, oauthTokens, {
-          accountId: selectedAccountInfo.id,
-          accountName: selectedAccountInfo.name,
-        });
+        this.configManager.storeOAuthTokens(
+          alias,
+          { ...oauthTokens, userEmail: oauthTokens.userEmail ?? user.email },
+          {
+            accountId: selectedAccountInfo.id,
+            accountName: selectedAccountInfo.name,
+          },
+        );
       } else {
         this.configManager.storeAccount(accessToken, alias, {
           accountId: selectedAccountInfo.id,
diff --git a/src/commands/accounts/logout.ts b/src/commands/accounts/logout.ts
@@ -58,9 +58,10 @@ export default class AccountsLogout extends ControlBaseCommand {
     }
 
     const accounts = this.configManager.listAccounts();
-    const accountExists = accounts.some(
+    const targetAccount = accounts.find(
       (account) => account.alias === targetAlias,
-    );
+    )?.account;
+    const accountExists = Boolean(targetAccount);
 
     if (!accountExists) {
       const error = `Account with alias "${targetAlias}" not found. Use "ably accounts list" to see available accounts.`;
@@ -94,7 +95,9 @@ export default class AccountsLogout extends ControlBaseCommand {
       const oauthTokens = this.configManager.getOAuthTokens(targetAlias);
       if (oauthTokens) {
         const oauthClient = new OAuthClient({
-          controlHost: flags["control-host"],
+          controlHost:
+            (flags["control-host"] as string | undefined) ||
+            targetAccount?.controlHost,
         });
         // Best-effort revocation -- don't block on failure
         await Promise.all([
diff --git a/src/control-base-command.ts b/src/control-base-command.ts
@@ -19,21 +19,23 @@ export abstract class ControlBaseCommand extends AblyBaseCommand {
   protected createControlApi(flags: BaseFlags): ControlApi {
     let accessToken = flags["access-token"] || process.env.ABLY_ACCESS_TOKEN;
     let tokenRefreshMiddleware: TokenRefreshMiddleware | undefined;
+    let currentAccount = this.configManager.getCurrentAccount();
 
     if (!accessToken) {
-      const account = this.configManager.getCurrentAccount();
-      if (!account) {
+      if (!currentAccount) {
         this.error(
           `No access token provided. Please specify --access-token or configure an account with "ably accounts login".`,
         );
       }
 
-      accessToken = account.accessToken;
+      accessToken = currentAccount.accessToken;
 
       // Set up token refresh middleware for OAuth accounts
       if (this.configManager.getAuthMethod() === "oauth") {
         const oauthClient = new OAuthClient({
-          controlHost: flags["control-host"],
+          controlHost:
+            (flags["control-host"] as string | undefined) ||
+            currentAccount?.controlHost,
         });
         tokenRefreshMiddleware = new TokenRefreshMiddleware(
           this.configManager,
diff --git a/test/unit/base/control-base-command.test.ts b/test/unit/base/control-base-command.test.ts
@@ -0,0 +1,103 @@
+import { beforeEach, afterEach, describe, expect, it, vi } from "vitest";
+import fs from "node:fs";
+import nock from "nock";
+import { Config } from "@oclif/core";
+
+import { ControlBaseCommand } from "../../../src/control-base-command.js";
+import {
+  ConfigManager,
+  TomlConfigManager,
+} from "../../../src/services/config-manager.js";
+import { BaseFlags } from "../../../src/types/cli.js";
+
+class TestControlCommand extends ControlBaseCommand {
+  public testCreateControlApi(flags: BaseFlags) {
+    return this.createControlApi(flags);
+  }
+
+  public set testConfigManager(value: ConfigManager) {
+    this.configManager = value;
+  }
+
+  async run(): Promise<void> {
+    // No-op
+  }
+}
+
+describe("ControlBaseCommand", () => {
+  let command: TestControlCommand;
+  let originalEnv: NodeJS.ProcessEnv;
+
+  beforeEach(() => {
+    originalEnv = { ...process.env };
+    delete process.env.ABLY_ACCESS_TOKEN;
+
+    vi.spyOn(fs, "existsSync").mockReturnValue(true);
+    vi.spyOn(fs, "readFileSync").mockReturnValue("");
+    vi.spyOn(
+      TomlConfigManager.prototype as unknown as {
+        ensureConfigDirExists: () => void;
+      },
+      "ensureConfigDirExists",
+    ).mockImplementation(() => {});
+    vi.spyOn(
+      TomlConfigManager.prototype as unknown as {
+        saveConfig: () => void;
+      },
+      "saveConfig",
+    ).mockImplementation(() => {});
+
+    command = new TestControlCommand([], {} as Config);
+  });
+
+  afterEach(() => {
+    process.env = originalEnv;
+    nock.cleanAll();
+    vi.restoreAllMocks();
+  });
+
+  it("uses stored control host for OAuth token refresh when flag is not provided", async () => {
+    const customControlHost = "custom.ably.net";
+    const configManagerStub = {
+      getAccessToken: vi.fn().mockReturnValue("expired_access_token"),
+      getAuthMethod: vi.fn().mockReturnValue("oauth"),
+      getCurrentAccount: vi.fn().mockReturnValue({
+        accessToken: "expired_access_token",
+        controlHost: customControlHost,
+      }),
+      getCurrentAccountAlias: vi.fn().mockReturnValue("default"),
+      getOAuthTokens: vi.fn().mockReturnValue({
+        accessToken: "expired_access_token",
+        expiresAt: Date.now() - 1000,
+        refreshToken: "refresh_token",
+      }),
+      isAccessTokenExpired: vi.fn().mockReturnValue(true),
+      storeOAuthTokens: vi.fn(),
+    } as unknown as ConfigManager;
+    command.testConfigManager = configManagerStub;
+
+    const refreshScope = nock(`https://${customControlHost}`)
+      .post("/oauth/token")
+      .reply(200, {
+        access_token: "refreshed_access_token",
+        expires_in: 3600,
+        refresh_token: "refreshed_refresh_token",
+        token_type: "Bearer",
+      });
+
+    nock("https://control.ably.net")
+      .matchHeader("authorization", "Bearer refreshed_access_token")
+      .get("/v1/me")
+      .reply(200, {
+        account: { id: "acc-123", name: "Test Account" },
+        user: { email: "test@example.com" },
+      });
+
+    const controlApi = command.testCreateControlApi({});
+    const me = await controlApi.getMe();
+
+    expect(me.account.id).toBe("acc-123");
+    expect(refreshScope.isDone()).toBe(true);
+    expect(configManagerStub.storeOAuthTokens).toHaveBeenCalled();
+  });
+});
diff --git a/test/unit/commands/accounts/login.test.ts b/test/unit/commands/accounts/login.test.ts
@@ -128,6 +128,48 @@ describe("accounts:login command", () => {
       expect(config.accounts[customAlias].userEmail).toBe("test@example.com");
     });
 
+    it("should fall back to /me account when /me/accounts is empty", async () => {
+      const fallbackAccountId = "fallback-account-id";
+      const fallbackAccountName = "Fallback Account";
+
+      // Mock the /me endpoint twice - once for initial login, once for listApps
+      nock("https://control.ably.net")
+        .get("/v1/me")
+        .twice()
+        .reply(200, {
+          account: { id: fallbackAccountId, name: fallbackAccountName },
+          user: { email: "fallback@example.com" },
+        });
+
+      // Mock an empty /me/accounts response
+      nock("https://control.ably.net").get("/v1/me/accounts").reply(200, []);
+
+      // Mock the apps list endpoint
+      nock("https://control.ably.net")
+        .get(`/v1/accounts/${fallbackAccountId}/apps`)
+        .reply(200, []);
+
+      const { stdout } = await runCommand(
+        ["accounts:login", mockAccessToken, "--json"],
+        import.meta.url,
+      );
+
+      const result = JSON.parse(stdout);
+      expect(result).toHaveProperty("success", true);
+      expect(result.account).toHaveProperty("id", fallbackAccountId);
+      expect(result.account).toHaveProperty("name", fallbackAccountName);
+
+      const mock = getMockConfigManager();
+      const config = mock.getConfig();
+      expect(config.accounts["fallback-account"]).toBeDefined();
+      expect(config.accounts["fallback-account"].accountId).toBe(
+        fallbackAccountId,
+      );
+      expect(config.accounts["fallback-account"].accountName).toBe(
+        fallbackAccountName,
+      );
+    });
+
     it("should include app info when single app is auto-selected", async () => {
       const mockAppId = "app-123";
       const mockAppName = "My Only App";
@@ -401,5 +443,53 @@ describe("accounts:login command", () => {
         config.accounts["test-account"].accessTokenExpiresAt,
       ).toBeUndefined();
     });
+
+    it("should store user email from /me for OAuth login when token response omits user_email", async () => {
+      const oauthAccountId = "oauth-account-id";
+      const oauthEmail = "oauth-user@example.com";
+
+      nock("https://ably.com").post("/oauth/authorize_device").reply(200, {
+        device_code: "device-code-123",
+        expires_in: 300,
+        interval: 0.01,
+        user_code: "ABC123",
+        verification_uri: "https://ably.com/verify",
+        verification_uri_complete: "https://ably.com/verify?user_code=ABC123",
+      });
+
+      nock("https://ably.com").post("/oauth/token").reply(200, {
+        access_token: "oauth_access_token",
+        expires_in: 3600,
+        refresh_token: "oauth_refresh_token",
+        token_type: "Bearer",
+      });
+
+      // Mock the /me endpoint twice - once for initial login, once for listApps
+      nock("https://control.ably.net")
+        .get("/v1/me")
+        .twice()
+        .reply(200, {
+          account: { id: oauthAccountId, name: "OAuth Account" },
+          user: { email: oauthEmail },
+        });
+
+      nock("https://control.ably.net")
+        .get("/v1/me/accounts")
+        .reply(200, [{ id: oauthAccountId, name: "OAuth Account" }]);
+
+      nock("https://control.ably.net")
+        .get(`/v1/accounts/${oauthAccountId}/apps`)
+        .reply(200, []);
+
+      await runCommand(
+        ["accounts:login", "--no-browser", "--json"],
+        import.meta.url,
+      );
+
+      const mock = getMockConfigManager();
+      const config = mock.getConfig();
+      expect(config.accounts["oauth-account"].authMethod).toBe("oauth");
+      expect(config.accounts["oauth-account"].userEmail).toBe(oauthEmail);
+    });
   });
 });
diff --git a/test/unit/commands/accounts/logout.test.ts b/test/unit/commands/accounts/logout.test.ts
@@ -270,6 +270,40 @@ describe("accounts:logout command", () => {
       expect(config.accounts["testaccount"]).toBeUndefined();
     });
 
+    it("should use stored control host for revocation when flag is not provided", async () => {
+      const customControlHost = "custom.ably.net";
+      const mock = getMockConfigManager();
+      mock.setConfig({
+        current: { account: "testaccount" },
+        accounts: {
+          testaccount: {
+            accessToken: "oauth_access_token",
+            accessTokenExpiresAt: Date.now() + 3600000,
+            accountId: "acc-123",
+            accountName: "Test Account",
+            authMethod: "oauth",
+            controlHost: customControlHost,
+            refreshToken: "oauth_refresh_token",
+            userEmail: "test@example.com",
+          },
+        },
+      });
+
+      const revokeScope = nock(`https://${customControlHost}`)
+        .post("/oauth/revoke")
+        .twice()
+        .reply(200);
+
+      const { stdout } = await runCommand(
+        ["accounts:logout", "--force", "--json"],
+        import.meta.url,
+      );
+
+      const result = JSON.parse(stdout);
+      expect(result).toHaveProperty("success", true);
+      expect(revokeScope.isDone()).toBe(true);
+    });
+
     it("should not call revocation endpoint for non-OAuth account logout", async () => {
       const mock = getMockConfigManager();
       mock.setConfig({
