fix(web): refactor logic/fix scopes W-20329175

messages/web.login.md

@@ -56,6 +56,10 @@ Username of the already-authenticated user to link to the connected app or exter
 
 Authentication (OAuth) scopes to request. Use the scope's short name; specify multiple scopes using just one flag instance and separated by spaces: --scopes "sfap_api chatbot_api".
 
+# flags.scopes.invalidFormat
+
+The --scopes flag must be a space-separated list (example: "api web").
+
 # linkedClientApp
 
 Successfully linked "%s" client app to %s.

package.json

@@ -8,7 +8,7 @@
     "@inquirer/checkbox": "^2.5.0",
     "@inquirer/select": "^2.5.0",
     "@oclif/core": "^4",
-    "@salesforce/core": "8.23.4",
+    "@salesforce/core": "^8.23.5",
     "@salesforce/kit": "^3.2.4",
     "@salesforce/plugin-info": "^3.4.96",
     "@salesforce/sf-plugins-core": "^12.2.6",

src/commands/org/login/web.ts

@@ -24,6 +24,12 @@ Messages.importMessagesDirectoryFromMetaUrl(import.meta.url);
 const messages = Messages.loadMessages('@salesforce/plugin-auth', 'web.login');
 const commonMessages = Messages.loadMessages('@salesforce/plugin-auth', 'messages');
 
+export type ExecuteLoginFlowParams = {
+  oauthConfig: OAuth2Config;
+  browser?: string;
+  scopes?: string;
+} & ({ clientApp: { name: string; username: string } } | { clientApp?: undefined });
+
 export default class LoginWeb extends SfCommand<AuthFields> {
   public static readonly summary = messages.getMessage('summary');
   public static readonly description = messages.getMessage('description');
@@ -89,6 +95,12 @@ export default class LoginWeb extends SfCommand<AuthFields> {
     }),
     scopes: Flags.string({
       summary: messages.getMessage('flags.scopes.summary'),
+      parse: async (input: string) => {
+        if (input.includes(',')) {
+          throw new SfError(messages.getMessage('flags.scopes.invalidFormat'));
+        }
+        return Promise.resolve(input);
+      },
     }),
   };
 
@@ -118,7 +130,12 @@ export default class LoginWeb extends SfCommand<AuthFields> {
         ...{ clientSecret: await this.secretPrompt({ message: commonMessages.getMessage('clientSecretStdin') }) },
       };
 
-      await this.executeLoginFlow(oauthConfig, flags.browser, flags['client-app'], flags.username, flags.scopes);
+      await this.executeLoginFlow({
+        oauthConfig,
+        browser: flags.browser,
+        clientApp: { name: flags['client-app'], username: flags.username },
+        scopes: flags.scopes,
+      });
 
       this.logSuccess(messages.getMessage('linkedClientApp', [flags['client-app'], flags.username]));
       return userAuthInfo.getFields(true);
@@ -133,7 +150,12 @@ export default class LoginWeb extends SfCommand<AuthFields> {
     };
 
     try {
-      const authInfo = await this.executeLoginFlow(oauthConfig, flags.browser, flags.scopes);
+      const authInfo = await this.executeLoginFlow({
+        oauthConfig,
+        browser: flags.browser,
+        clientApp: undefined,
+        scopes: flags.scopes,
+      });
       await authInfo.handleAliasAndDefaultSettings({
         alias: flags.alias,
         setDefault: flags['set-default'],
@@ -156,13 +178,12 @@ export default class LoginWeb extends SfCommand<AuthFields> {
 
   // leave it because it's stubbed in the test
   // eslint-disable-next-line class-methods-use-this
-  private async executeLoginFlow(
-    oauthConfig: OAuth2Config,
-    browser?: string,
-    app?: string,
-    username?: string,
-    scopes?: string
-  ): Promise<AuthInfo> {
+  private async executeLoginFlow({
+    oauthConfig,
+    browser,
+    clientApp,
+    scopes,
+  }: ExecuteLoginFlowParams): Promise<AuthInfo> {
     // The server handles 2 possible auth scenarios:
     // a. 1st time auth, creates auth file.
     // b. Add CA/ECA to existing auth.
@@ -171,8 +192,8 @@ export default class LoginWeb extends SfCommand<AuthFields> {
         ...oauthConfig,
         scope: scopes,
       },
-      clientApp: app,
-      username,
+      clientApp: clientApp?.name,
+      username: clientApp?.username,
     });
     await oauthServer.start();
     const browserApp = browser && browser in apps ? (browser as AppName) : undefined;

test/commands/org/login/login.web.test.ts

@@ -23,7 +23,7 @@ import { StubbedType, stubInterface, stubMethod } from '@salesforce/ts-sinon';
 import { assert, expect } from 'chai';
 import { Env } from '@salesforce/kit';
 import { SfCommand, Ux } from '@salesforce/sf-plugins-core';
-import LoginWeb from '../../../../src/commands/org/login/web.js';
+import LoginWeb, { ExecuteLoginFlowParams } from '../../../../src/commands/org/login/web.js';
 
 describe('org:login:web', () => {
   const $$ = new TestContext();
@@ -195,8 +195,8 @@ describe('org:login:web', () => {
 
     // Verify that executeLoginFlow was called with the scopes parameter
     expect(executeLoginFlowStub.callCount).to.equal(1);
-    const callArgs = executeLoginFlowStub.getCall(0).args;
-    expect(callArgs[2]).to.equal('api web refresh_token'); // scopes should be the 3rd argument
+    const callArgs = executeLoginFlowStub.getCall(0).args[0] as ExecuteLoginFlowParams;
+    expect(callArgs.scopes).to.equal('api web refresh_token'); // scopes should be present in the object
   });
 
   it('should pass undefined scopes to executeLoginFlow when scopes flag is not provided', async () => {
@@ -223,8 +223,8 @@ describe('org:login:web', () => {
 
     // Verify that executeLoginFlow was called without scopes (undefined)
     expect(executeLoginFlowStub.callCount).to.equal(1);
-    const callArgs = executeLoginFlowStub.getCall(0).args;
-    expect(callArgs[2]).to.be.undefined; // scopes should be undefined when not provided
+    const callArgs = executeLoginFlowStub.getCall(0).args[0] as ExecuteLoginFlowParams;
+    expect(callArgs.scopes).to.be.undefined;
   });
 
   it('should pass scopes flag to executeLoginFlow when linking client-app with scopes', async () => {
@@ -242,8 +242,11 @@ describe('org:login:web', () => {
     $$.SANDBOX.stub(AuthInfo, 'create').resolves(authInfoStub);
     $$.SANDBOX.stub(AuthInfo, 'listAllAuthorizations').resolves([]);
 
-    // @ts-ignore
-    const login = new LoginWeb(['--client-app', 'MyApp', '--username', 'test@example.com', '--scopes', 'api web'], config);
+    const login = new LoginWeb(
+      ['--client-app', 'MyApp', '--username', 'test@example.com', '--scopes', 'api web'],
+      // @ts-ignore
+      config
+    );
     // @ts-ignore because protected member
     login.ux = uxStub;
     // @ts-ignore because protected member
@@ -258,10 +261,10 @@ describe('org:login:web', () => {
 
     // Verify that executeLoginFlow was called with the correct parameters
     expect(executeLoginFlowStub.callCount).to.equal(1);
-    const callArgs = executeLoginFlowStub.getCall(0).args;
-    expect(callArgs[2]).to.equal('MyApp'); // client-app should be the 3rd argument
-    expect(callArgs[3]).to.equal('test@example.com'); // username should be the 4th argument
-    expect(callArgs[4]).to.equal('api web'); // scopes should be the 5th argument
+    const callArgs = executeLoginFlowStub.getCall(0).args[0] as ExecuteLoginFlowParams;
+    expect(callArgs.clientApp?.name).to.equal('MyApp');
+    expect(callArgs.clientApp?.username).to.equal('test@example.com');
+    expect(callArgs.scopes).to.equal('api web');
   });
 
   it('should pass undefined scopes to executeLoginFlow when linking client-app without scopes', async () => {
@@ -294,9 +297,9 @@ describe('org:login:web', () => {
 
     // Verify that executeLoginFlow was called with the correct parameters
     expect(executeLoginFlowStub.callCount).to.equal(1);
-    const callArgs = executeLoginFlowStub.getCall(0).args;
-    expect(callArgs[2]).to.equal('MyApp'); // client-app should be the 3rd argument
-    expect(callArgs[3]).to.equal('test@example.com'); // username should be the 4th argument
-    expect(callArgs[4]).to.be.undefined; // scopes should be undefined when not provided
+    const callArgs = executeLoginFlowStub.getCall(0).args[0] as ExecuteLoginFlowParams;
+    expect(callArgs.clientApp?.name).to.equal('MyApp');
+    expect(callArgs.clientApp?.username).to.equal('test@example.com');
+    expect(callArgs.scopes).to.be.undefined;
   });
 });

yarn.lock

@@ -583,22 +583,14 @@
     "@smithy/types" "^4.9.0"
     tslib "^2.6.2"
 
-"@aws-sdk/types@3.936.0":
+"@aws-sdk/types@3.936.0", "@aws-sdk/types@^3.222.0":
   version "3.936.0"
   resolved "https://registry.yarnpkg.com/@aws-sdk/types/-/types-3.936.0.tgz#ecd3a4bec1a1bd4df834ab21fe52a76e332dc27a"
   integrity sha512-uz0/VlMd2pP5MepdrHizd+T+OKfyK4r3OA9JI+L/lPKg0YFQosdJNCKisr6o70E3dh8iMpFYxF1UN/4uZsyARg==
   dependencies:
     "@smithy/types" "^4.9.0"
     tslib "^2.6.2"
 
-"@aws-sdk/types@^3.222.0":
-  version "3.910.0"
-  resolved "https://registry.yarnpkg.com/@aws-sdk/types/-/types-3.910.0.tgz#1707a9f5d36d828789173fcd5622a5684cf67b2f"
-  integrity sha512-o67gL3vjf4nhfmuSUNNkit0d62QJEwwHLxucwVJkR/rw9mfUtAWsgBs8Tp16cdUbMgsyQtCQilL8RAJDoGtadQ==
-  dependencies:
-    "@smithy/types" "^4.7.1"
-    tslib "^2.6.2"
-
 "@aws-sdk/util-arn-parser@3.893.0":
   version "3.893.0"
   resolved "https://registry.yarnpkg.com/@aws-sdk/util-arn-parser/-/util-arn-parser-3.893.0.tgz#fcc9b792744b9da597662891c2422dda83881d8d"
@@ -1144,16 +1136,11 @@
     chardet "^2.1.1"
     iconv-lite "^0.7.0"
 
-"@inquirer/figures@^1.0.15":
+"@inquirer/figures@^1.0.15", "@inquirer/figures@^1.0.5":
   version "1.0.15"
   resolved "https://registry.yarnpkg.com/@inquirer/figures/-/figures-1.0.15.tgz#dbb49ed80df11df74268023b496ac5d9acd22b3a"
   integrity sha512-t2IEY+unGHOzAaVM5Xx6DEWKeXlDDcNPeDyUpsRc6CUhBfU3VQOEl+Vssh7VNp1dR8MdUJBWhuObjXCsVpjN5g==
 
-"@inquirer/figures@^1.0.5":
-  version "1.0.13"
-  resolved "https://registry.yarnpkg.com/@inquirer/figures/-/figures-1.0.13.tgz#ad0afd62baab1c23175115a9b62f511b6a751e45"
-  integrity sha512-lGPVU3yO9ZNqA7vTYz26jny41lE7yoQansmqdMLBEfqaGsmdg7V3W9mK9Pvb5IL4EVZ9GnSDGMO/cJXud5dMaw==
-
 "@inquirer/input@^2.2.4", "@inquirer/input@^2.3.0":
   version "2.3.0"
   resolved "https://registry.yarnpkg.com/@inquirer/input/-/input-2.3.0.tgz#9b99022f53780fecc842908f3f319b52a5a16865"
@@ -1333,10 +1320,10 @@
     "@jridgewell/resolve-uri" "^3.1.0"
     "@jridgewell/sourcemap-codec" "^1.4.14"
 
-"@jsforce/jsforce-node@^3.10.8":
-  version "3.10.8"
-  resolved "https://registry.yarnpkg.com/@jsforce/jsforce-node/-/jsforce-node-3.10.8.tgz#f13903a0885fa3501a513512984cf9a717aebb9a"
-  integrity sha512-XGD/ivZz+htN5SgctFyEZ+JNG6C8FXzaEwvPbRSdsIy/hpWlexY38XtTpdT5xX3KnYSnOE4zA1M/oIbTm7RD/Q==
+"@jsforce/jsforce-node@^3.10.10", "@jsforce/jsforce-node@^3.10.8":
+  version "3.10.10"
+  resolved "https://registry.yarnpkg.com/@jsforce/jsforce-node/-/jsforce-node-3.10.10.tgz#2d7bb77d1d739712733a30de7e9c941d2127bc4a"
+  integrity sha512-/zUOX9kapwk8lyjmTYgXlBF+GbqcEpb0zrkDfX9i94xu5cvzERZxRHqSSaS/IImoDmvoSbatFSVfB7Y4lmANOw==
   dependencies:
     "@sindresorhus/is" "^4"
     base64url "^3.0.1"
@@ -1538,12 +1525,12 @@
     strip-ansi "6.0.1"
     ts-retry-promise "^0.8.1"
 
-"@salesforce/core@8.23.4", "@salesforce/core@^8.18.7", "@salesforce/core@^8.23.1", "@salesforce/core@^8.23.3", "@salesforce/core@^8.23.4", "@salesforce/core@^8.5.1", "@salesforce/core@^8.8.0":
-  version "8.23.4"
-  resolved "https://registry.yarnpkg.com/@salesforce/core/-/core-8.23.4.tgz#f1fa18eace08f685e72975a09d96e7f6958ca3b4"
-  integrity sha512-+JZMFD76P7X8fLSrHJRi9+ygjTehqZqJRXxmNq51miqIHY1Xlb0qH/yr9u5QEGsFIOZ8H8oStl/Zj+ZbrFs0vw==
+"@salesforce/core@^8.18.7", "@salesforce/core@^8.23.1", "@salesforce/core@^8.23.3", "@salesforce/core@^8.23.4", "@salesforce/core@^8.23.5", "@salesforce/core@^8.5.1", "@salesforce/core@^8.8.0":
+  version "8.23.5"
+  resolved "https://registry.yarnpkg.com/@salesforce/core/-/core-8.23.5.tgz#fc4a18a814ff3c6c0c3a138a0369441595ef74a2"
+  integrity sha512-EOA4JzvYk5KZ7YJ6mnBrHCrR0e1x7ellb6e8s4/+kIGGYU8mMnD0PYIUnc2jjYKXujY1hPgdQh39NLBQiQmt8g==
   dependencies:
-    "@jsforce/jsforce-node" "^3.10.8"
+    "@jsforce/jsforce-node" "^3.10.10"
     "@salesforce/kit" "^3.2.4"
     "@salesforce/schemas" "^1.10.3"
     "@salesforce/ts-types" "^2.0.12"
@@ -2130,14 +2117,7 @@
     "@smithy/util-stream" "^4.5.6"
     tslib "^2.6.2"
 
-"@smithy/types@^4.7.1":
-  version "4.8.0"
-  resolved "https://registry.yarnpkg.com/@smithy/types/-/types-4.8.0.tgz#e6f65e712478910b74747081e6046e68159f767d"
-  integrity sha512-QpELEHLO8SsQVtqP+MkEgCYTFW0pleGozfs3cZ183ZBj9z3VC1CX1/wtFMK64p+5bhtZo41SeLK1rBRtd25nHQ==
-  dependencies:
-    tslib "^2.6.2"
-
-"@smithy/types@^4.9.0":
+"@smithy/types@^4.7.1", "@smithy/types@^4.9.0":
   version "4.9.0"
   resolved "https://registry.yarnpkg.com/@smithy/types/-/types-4.9.0.tgz#c6636ddfa142e1ddcb6e4cf5f3e1a628d420486f"
   integrity sha512-MvUbdnXDTwykR8cB1WZvNNwqoWVaTRA0RLlLmf/cIFNMM2cKWz01X4Ly6SMC4Kks30r8tT3Cty0jmeWfiuyHTA==
@@ -8280,12 +8260,7 @@ yocto-queue@^0.1.0:
   resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-0.1.0.tgz#0294eb3dee05028d31ee1a5fa2c556a6aaf10a1b"
   integrity sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==
 
-yoctocolors-cjs@^2.1.2:
-  version "2.1.2"
-  resolved "https://registry.yarnpkg.com/yoctocolors-cjs/-/yoctocolors-cjs-2.1.2.tgz#f4b905a840a37506813a7acaa28febe97767a242"
-  integrity sha512-cYVsTjKl8b+FrnidjibDWskAv7UKOfcwaVZdp/it9n1s9fU3IkgDbhdIRKCW4JDsAlECJY0ytoVPT3sK6kideA==
-
-yoctocolors-cjs@^2.1.3:
+yoctocolors-cjs@^2.1.2, yoctocolors-cjs@^2.1.3:
   version "2.1.3"
   resolved "https://registry.yarnpkg.com/yoctocolors-cjs/-/yoctocolors-cjs-2.1.3.tgz#7e4964ea8ec422b7a40ac917d3a344cfd2304baa"
   integrity sha512-U/PBtDf35ff0D8X8D0jfdzHYEPFxAI7jJlxZXwCSez5M3190m+QobIfh+sWDWSHMCWWJN2AWamkegn6vr6YBTw==