styra-link: correct password flow (#81)

### What code changed, and why? Several issues revolving around the password entry flow when installing the Styra CLI from the plugin. DAS-770, DAS-771, DAS-772. 1. fix poor error reporting upon bad password. 2. allow password retries without re-downloading full binary: Add loop for retry but move the binary fetch outside the loop 3. cancel out of password entry closes progress bar.
StyraInc · Apr 22, 2024 · d517cb0 · d517cb0
1 parent 43fc7c2
commit d517cb0
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   With a bad download URL this caused a cryptic "Error spawn Unknown system error -8" message.
 - Fixed download URLs for auto-installing Styra CLI.
 - Optimized vsix package, making the plugin binary 80% smaller!
+- Cancelling password entry during CLI install now aborts the progress bar, too.
+- Bad password during CLI install now reports a short, clean message instead of a cryptic, verbose one.
+- Password entry for CLI install now allows retries without re-downloading the CLI binary.
 
 ## [2.0.0] - 2023-09-25
 

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -12,7 +12,7 @@
     "url": "https://github.com/StyraInc/vscode-styra/issues"
   },
   "publisher": "styra",
-  "version": "2.0.1-next.3",
+  "version": "2.0.1-next.4",
   "private": true,
   "main": "./out/main.js",
   "engines": {

diff --git a/src/lib/styra-install.ts b/src/lib/styra-install.ts
@@ -45,12 +45,12 @@ export class StyraInstall {
   }
 
   static async checkCliInstallation(): Promise<boolean> {
-    if (await StyraInstall.styraCmdExists()) {
+    if (await this.styraCmdExists()) {
       infoDebug('Styra CLI is installed');
       return true;
     }
     info('Styra CLI is not installed');
-    return await StyraInstall.promptForInstall('is not installed', 'installation');
+    return await this.promptForInstall('is not installed', 'installation');
   }
 
   private static async promptForInstall(description: string, operation: string): Promise<boolean> {
@@ -60,15 +60,29 @@ export class StyraInstall {
       `Styra CLI ${description}. Would you like to install it now?`, 'Install');
 
     if (selection === 'Install') {
+      const tempFile = path.join(os.homedir(), this.BinaryFile);
       info('Installing Styra CLI. This may take a few minutes...');
       try {
-        await this.installStyra();
-        teeInfo(`CLI ${operation} completed.`);
-        return true;
+        await this.downloadBinary(tempFile);
       } catch (err) {
-        teeError(`CLI ${operation} failed: ${err}`);
+        teeError(`CLI ${operation} failed: ${(err as Error).message}`);
         return false;
       }
+      // eslint-disable-next-line no-constant-condition
+      while (true) {
+        try {
+          await this.installOnPath(tempFile);
+          teeInfo(`CLI ${operation} completed.`);
+          return true;
+        } catch (err) {
+          if ((err as Error).message.includes('Sorry, try again')) {
+            info('invalid password; try again...');
+          } else {
+            teeError(`CLI ${operation} failed: ${(err as Error).message}`);
+            return false;
+          }
+        }
+      }
     } else {
       infoFromUserAction(`CLI ${operation} cancelled`);
       return false;
@@ -88,7 +102,7 @@ export class StyraInstall {
         const available = await DAS.runQuery('/v1/system/version') as VersionType;
         const installedVersion = await this.getInstalledCliVersion();
         if (compare(available.cliVersion, installedVersion) === 1) {
-          await StyraInstall.promptForInstall(
+          await this.promptForInstall(
             `has an update available (installed=${installedVersion}, available=${available.cliVersion})`, 'update');
         }
       } catch (err) {
@@ -102,7 +116,7 @@ export class StyraInstall {
       const versionInfo = await DAS.runQuery('/v1/system/version') as VersionType;
       infoDebug(`DAS release: ${versionInfo.release} `);
       infoDebug(`DAS edition: ${versionInfo.dasEdition} `);
-      const cliVersion = await StyraInstall.getInstalledCliVersion();
+      const cliVersion = await this.getInstalledCliVersion();
       infoDebug(`CLI version: ${cliVersion} `);
       if (cliVersion !== versionInfo.cliVersion) {
         infoDebug(`(Latest CLI version: ${versionInfo.cliVersion})`);
@@ -163,12 +177,11 @@ export class StyraInstall {
           : `${prefix}/linux/amd64/styra`;
   }
 
-  private static async installStyra(): Promise<void> {
+  private static async downloadBinary(tempFileLocation: string): Promise<void> {
 
-    const tempFileLocation = path.join(os.homedir(), this.BinaryFile);
     const url = this.getDownloadUrl();
 
-    return await IDE.withProgress({
+    await IDE.withProgress({
       location: IDE.ProgressLocation.Notification,
       title: 'Installing Styra CLI',
       cancellable: false
@@ -178,22 +191,26 @@ export class StyraInstall {
       info(`    Architecture: ${process.arch}`);
       info(`    Executable: ${this.ExeFile}`);
       fs.chmodSync(tempFileLocation, '755');
-      if (this.isWindows()) {
-        await moveFile(tempFileLocation, this.ExeFile);
-        await this.adjustWindowsPath(this.ExePath);
-      } else {
-        const state = await this.collectInputs();
-        // see https://stackoverflow.com/q/39785436/115690 for ideas on running sudo
-        const args = ['-c', `echo ${state.pwd} | sudo -S bash -c 'mv ${tempFileLocation} ${this.ExeFile}'`];
-        // vital to run in quiet mode so password does not display
-        await new CommandRunner().runShellCmd('sh', args, {progressTitle: '', quiet: true});
-      }
     });
   }
 
+  private static async installOnPath(tempFileLocation: string) {
+    if (this.isWindows()) {
+      await moveFile(tempFileLocation, this.ExeFile);
+      await this.adjustWindowsPath(this.ExePath);
+    } else {
+      const state = await this.collectInputs();
+      // see https://stackoverflow.com/q/39785436/115690 for ideas on running sudo
+      const args = ['-c', `echo ${state.pwd} | sudo -S bash -c 'mv ${tempFileLocation} ${this.ExeFile}'`];
+      // vital to run in quiet mode so password does not display
+      await new CommandRunner().runShellCmd('sh', args, {progressTitle: '', quiet: true});
+    }
+  }
+
   private static async getBinary(url: string, tempFileLocation: string): Promise<void> {
     // adapted from https://stackoverflow.com/a/69290915
     const response = await fetch(url);
+
     if (!response.ok) {
       throw new Error(
         response.status === 404 ? `Bad URL for downloading styra - ${url}`
@@ -261,7 +278,7 @@ export class StyraInstall {
       value: state.pwd ?? '',
       prompt: `Enter admin password to install into ${STD_LINUX_INSTALL_DIR}`,
       validate: validateNoop,
-      shouldResume
+      shouldResume // TODO: override and delete temp file
     });
   }
 }
diff --git a/src/test-jest/lib/styra-install.test.ts b/src/test-jest/lib/styra-install.test.ts
@@ -33,9 +33,13 @@ describe('StyraInstall', () => {
 
   const spy = new OutputPaneSpy();
 
+  function setPrivateMock(functionName: string, mock: jest.Mock) {
+    (StyraInstall as any)[functionName] = mock;
+  }
+
   beforeEach(() => {
-    // eslint-disable-next-line dot-notation
-    StyraInstall['installStyra'] = jest.fn().mockResolvedValue('');
+    setPrivateMock('downloadBinary', jest.fn().mockResolvedValue(''));
+    setPrivateMock('installOnPath', jest.fn().mockResolvedValue(''));
     StyraInstall.styraCmdExists = jest.fn().mockResolvedValue(false);
     IDE.getConfigValue = mockVSCodeSettings();
   });
@@ -91,13 +95,36 @@ describe('StyraInstall', () => {
       });
     });
 
-    test('returns false if installStyra throws an error', async () => {
+    test('returns true and succeeds if get bad pwd then a good pwd', async () => {
+      IDE.showInformationMessageModal = jest.fn().mockReturnValue('Install');
+      setPrivateMock('installOnPath', jest.fn()
+        .mockRejectedValueOnce({message: 'Sorry, try again. Bad password'})
+        .mockResolvedValueOnce(''));
+
+      expect(await StyraInstall.checkCliInstallation()).toBe(true);
+      expect(spy.content).toMatch(/invalid password/);
+      expect(spy.content).toMatch(/CLI installation completed/);
+    });
+
+    test('returns false and fails if get bad pwd, then some other error', async () => {
+      IDE.showInformationMessageModal = jest.fn().mockReturnValue('Install');
+      setPrivateMock('installOnPath', jest.fn()
+        .mockRejectedValueOnce({message: 'Sorry, try again. Bad password'})
+        .mockRejectedValue({message: 'some error'}));
+
+      expect(await StyraInstall.checkCliInstallation()).toBe(false);
+      expect(spy.content).toMatch(/invalid password/);
+      expect(spy.content).toMatch(/CLI installation failed/);
+      expect(spy.content).toMatch(/some error/);
+    });
+
+    test('returns false and fails if throws an error other than bad pwd', async () => {
       IDE.showInformationMessageModal = jest.fn().mockReturnValue('Install');
-      // eslint-disable-next-line dot-notation
-      StyraInstall['installStyra'] = jest.fn().mockRejectedValue('error');
+      setPrivateMock('downloadBinary', jest.fn().mockRejectedValue({message: 'some error'}));
 
       expect(await StyraInstall.checkCliInstallation()).toBe(false);
       expect(spy.content).toMatch(/CLI installation failed/);
+      expect(spy.content).toMatch(/some error/);
     });
   });
 
@@ -200,8 +227,7 @@ describe('StyraInstall', () => {
         DAS.runQuery = jest.fn().mockResolvedValue({cliVersion: available});
         CommandRunner.prototype.runShellCmd = jest.fn().mockResolvedValue(installed);
         const installMock = jest.fn();
-        // eslint-disable-next-line dot-notation
-        StyraInstall['promptForInstall'] = installMock;
+        setPrivateMock('promptForInstall', installMock);
 
         await StyraInstall.checkForUpdates();
 
@@ -221,8 +247,7 @@ describe('StyraInstall', () => {
         DAS.runQuery = jest.fn().mockImplementation(available);
         CommandRunner.prototype.runShellCmd = jest.fn().mockImplementation(installed);
         const installMock = jest.fn();
-        // eslint-disable-next-line dot-notation
-        StyraInstall['promptForInstall'] = installMock;
+        setPrivateMock('promptForInstall', installMock);
 
         await StyraInstall.checkForUpdates();