Fix: Show password to user on node connect form (#1533)

* Add password visibility to node-connect form * Fix specfile to adjust for clipboard service * Update CHANGELOG.md
Azure · Aug 3, 2018 · fb66849 · fb66849
1 parent 8c62c05
commit fb66849
Show file tree

Hide file tree

Showing 8 changed files with 145 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 ### Bug fixes:
 * Task outputs is broken when not using autostorage account [\#1522](https://github.com/Azure/BatchExplorer/issues/1522)
 * Cannot connect to Windows Cloud Service node [\#1529](https://github.com/Azure/BatchExplorer/issues/1529)
+* Users should be able to see password used to connect to remote node [\#1532](https://github.com/Azure/BatchExplorer/issues/1532)
 
 ### Other
 * Task properties pool and node should be links [\#1523](https://github.com/Azure/BatchExplorer/issues/1523)
@@ -35,7 +36,6 @@
 * Task timeline doesn't cancel requests when leaving component [\#1472](https://github.com/Azure/BatchExplorer/issues/1472)
 * Pool from Windows managed image displays as Linux [\#1436](https://github.com/Azure/BatchExplorer/issues/1436)
 
-
 ### Accessibility:
 * Server error component is not keyboard accessible [\#1426](https://github.com/Azure/BatchExplorer/issues/1426)
 * Images tags are missing alt attributes [\#1482](https://github.com/Azure/BatchExplorer/issues/1482)

diff --git a/app/components/node/connect/node-connect.component.spec.ts b/app/components/node/connect/node-connect.component.spec.ts
@@ -20,7 +20,6 @@ import {
     SettingsService,
 } from "app/services";
 import { PoolUtils, SecureUtils } from "app/utils";
-import { clipboard } from "electron";
 import * as Fixtures from "test/fixture";
 
 @Component({
@@ -44,6 +43,7 @@ describe("NodeConnectComponent", () => {
     let electronShellSpy;
     let secureUtilsSpy;
     let nodeConnectServiceSpy;
+    let clipboardServiceSpy;
 
     beforeEach(() => {
         nodeUserServiceSpy = {
@@ -98,6 +98,10 @@ describe("NodeConnectComponent", () => {
             }),
         };
 
+        clipboardServiceSpy = {
+            writeText: jasmine.createSpy("writeText"),
+        };
+
         TestBed.configureTestingModule({
             declarations: [
                 NodeConnectComponent, ButtonComponent,
@@ -114,6 +118,7 @@ describe("NodeConnectComponent", () => {
                 { provide: ElectronShell, useValue: electronShellSpy },
                 { provide: SecureUtils, useValue: secureUtilsSpy },
                 { provide: NodeConnectService, useValue: nodeConnectServiceSpy },
+                { provide: ClipboardService, useValue: clipboardServiceSpy },
             ],
             schemas: [NO_ERRORS_SCHEMA],
         });
@@ -220,7 +225,7 @@ describe("NodeConnectComponent", () => {
                     expect(openItemArgs.length).toBe(1);
                     expect(openItemArgs[0]).toContain("path/to/file");
 
-                    expect(clipboard.readText()).toEqual(updateUserArgs[2].password);
+                    expect(clipboardServiceSpy.writeText).toHaveBeenCalledWith(updateUserArgs[2].password);
 
                     done();
                 });

diff --git a/app/components/node/connect/node-connect.component.ts b/app/components/node/connect/node-connect.component.ts
@@ -1,7 +1,6 @@
 import { ChangeDetectionStrategy, ChangeDetectorRef, Component, Input, OnInit } from "@angular/core";
 import { ServerError, autobind } from "@batch-flask/core";
-import { ElectronShell } from "@batch-flask/ui";
-import { clipboard } from "electron";
+import { ClipboardService, ElectronShell } from "@batch-flask/ui";
 import * as moment from "moment";
 import * as path from "path";
 
@@ -32,6 +31,7 @@ export class NodeConnectComponent implements OnInit {
     public loading: boolean = false;
     public credentials: AddNodeUserAttributes;
     public publicKeyFile: string;
+    public passwordCopied: boolean = false;
 
     // NOTE: using linux does not necessarily mean using SSH! (user can still use password)
     public linux = false;
@@ -75,16 +75,17 @@ export class NodeConnectComponent implements OnInit {
         private shell: ElectronShell,
         private changeDetector: ChangeDetectorRef,
         private fs: FileSystemService,
+        private clipboardService: ClipboardService,
     ) { }
 
     public ngOnInit() {
         this.credentials = {
             name: this.settingsService.settings["node-connect.default-username"],
-            password: "",
             expiryTime: null,
             isAdmin: true,
             sshPublicKey: "",
         };
+        this.generatePassword();
         this.publicKeyFile = path.join(this.fs.commonFolders.home, ".ssh", "id_rsa.pub");
 
         this.linux = PoolUtils.isLinux(this.pool);
@@ -105,20 +106,24 @@ export class NodeConnectComponent implements OnInit {
         }
     }
 
+    @autobind()
+    public generatePassword(): void {
+        this.credentials.password = SecureUtils.generateWindowsPassword();
+    }
+
     @autobind()
     public autoConnect(): Observable<any> {
         this.loading = true;
 
+        if (!this.credentials.password) {
+            this.generatePassword();
+        }
+
         const credentials = {...this.credentials};
         if (!credentials.expiryTime) {
             credentials.expiryTime = moment().add(moment.duration({days: 1})).toDate();
         }
 
-        // generate a password if the user didn't provide one
-        if (!credentials.password) {
-            credentials.password = SecureUtils.generateWindowsPassword();
-        }
-
         if (this.linux) {
             // we are going to use ssh keys, so we don't need a password
             if (this.usingSSHKeys) {
@@ -132,7 +137,9 @@ export class NodeConnectComponent implements OnInit {
                 next: (pid) => {
                     // if using password, save it to clipboard
                     if (!this.usingSSHKeys) {
-                        clipboard.writeText(credentials.password);
+                        this.clipboardService.writeText(credentials.password);
+                        this.passwordCopied = true;
+                        this.changeDetector.markForCheck();
                     }
                     this.loading = false;
                     this.error = null;
@@ -257,7 +264,9 @@ export class NodeConnectComponent implements OnInit {
                 this.error = null;
 
                 // save password to clipboard
-                clipboard.writeText(credentials.password);
+                this.clipboardService.writeText(credentials.password);
+                this.passwordCopied = true;
+                this.changeDetector.markForCheck();
 
                 // create and launch the rdp program
                 return this.nodeConnectService.saveRdpFile(this.connectionSettings, this.credentials, this.node.id);

diff --git a/app/components/node/connect/node-connect.html b/app/components/node/connect/node-connect.html
@@ -15,6 +15,8 @@ <h1>Connect to node</h1>
                     [publicKeyFile]="publicKeyFile"
                     [(usingSSHKeys)]="usingSSHKeys"
                     [(credentials)]="credentials"
+                    [(passwordCopied)]="passwordCopied"
+                    [generatePassword]="generatePassword"
                 ></bl-node-property-display>
             </div>
             <div class="credentials-source">

diff --git a/app/components/node/connect/property-display/node-property-display.component.ts b/app/components/node/connect/property-display/node-property-display.component.ts
@@ -4,6 +4,7 @@ import {
     Component,
     EventEmitter,
     Input,
+    OnChanges,
     OnInit,
     Output,
 } from "@angular/core";
@@ -14,29 +15,40 @@ import { AddNodeUserAttributes, NodeConnectService, SettingsService } from "app/
 
 import "./node-property-display.scss";
 
-const AUTH_STRATEGIES = {
-    Keys: "Keys",
-    Password: "Password",
-};
+enum AuthStrategies {
+    Keys = "Keys",
+    Password = "Password",
+}
+
+enum CopyText {
+    BeforeCopy = "Password will be copied to clipboard on Connect",
+    AfterCopy = "Password copied to clipboard!",
+}
 
 @Component({
     selector: "bl-node-property-display",
     templateUrl: "node-property-display.html",
     changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class NodePropertyDisplayComponent implements OnInit {
+export class NodePropertyDisplayComponent implements OnInit, OnChanges {
     // inherited input properties
     @Input() public connectionSettings: NodeConnectionSettings;
     @Input() public node: Node;
     @Input() public credentials: AddNodeUserAttributes;
     @Input() public publicKeyFile: string;
     @Input() public usingSSHKeys: boolean;
+    @Input() public passwordCopied: boolean;
+    @Input() public generatePassword: () => void;
     @Output() public credentialsChange = new EventEmitter<AddNodeUserAttributes>();
     @Output() public usingSSHKeysChange = new EventEmitter<boolean>();
+    @Output() public passwordCopiedChange = new EventEmitter<boolean>();
 
     public isLinux: boolean;
     public otherStrategy: string;
     public hasPublicKey: boolean;
+    public passwordVisible: boolean;
+    public passwordCopyText: string;
+    public regeneratingPassword: boolean;
 
     public PORT_NOT_SPECIFIED: string = "(Not Specified)";
 
@@ -48,8 +60,12 @@ export class NodePropertyDisplayComponent implements OnInit {
     ) {}
 
     public ngOnInit() {
+        this.passwordVisible = false;
+        this.passwordCopyText = CopyText.BeforeCopy;
+        this.regeneratingPassword = false;
+
         this.isLinux = this.connectionSettings.type === ConnectionType.SSH;
-        this.otherStrategy = this.usingSSHKeys ? AUTH_STRATEGIES.Password : AUTH_STRATEGIES.Keys;
+        this.otherStrategy = this.usingSSHKeys ? AuthStrategies.Password : AuthStrategies.Keys;
 
         this.nodeConnectService.getPublicKey(this.publicKeyFile).subscribe({
             next: (key) => {
@@ -63,6 +79,12 @@ export class NodePropertyDisplayComponent implements OnInit {
         });
     }
 
+    public ngOnChanges(changes) {
+        if (changes.passwordCopied) {
+            this.passwordCopyText = changes.passwordCopied.currentValue ? CopyText.AfterCopy : CopyText.BeforeCopy;
+        }
+    }
+
     public get sshCommand() {
         if (!this.connectionSettings) {
             return "N/A";
@@ -82,10 +104,29 @@ export class NodePropertyDisplayComponent implements OnInit {
 
     @autobind()
     public setPassword(event) {
+        // set the passwordCopied binding to be false, to reconvert the passwordCopyText
+        this.passwordCopied = false;
+        this.passwordCopiedChange.emit(this.passwordCopied);
+
+        // set the password itself in the credentials binding
         this.credentials.password = event.target.value;
         this.credentialsChange.emit(this.credentials);
     }
 
+    @autobind()
+    public regeneratePassword() {
+        if (this.regeneratingPassword) { return; }
+
+        setTimeout(() => {
+            this.generatePassword();
+            this.regeneratingPassword = false;
+            this.changeDetector.markForCheck();
+        }, 500);
+
+        this.regeneratingPassword = true;
+        this.changeDetector.markForCheck();
+    }
+
     @autobind()
     public downloadRdp() {
         const obs = this.nodeConnectService.saveRdpFile(this.connectionSettings, this.credentials, this.node.id);
@@ -101,11 +142,16 @@ export class NodePropertyDisplayComponent implements OnInit {
     public switchAuthStrategy() {
         this.usingSSHKeys = !this.usingSSHKeys;
         this.usingSSHKeysChange.emit(this.usingSSHKeys);
-        if (this.otherStrategy === AUTH_STRATEGIES.Keys) {
-            this.otherStrategy = AUTH_STRATEGIES.Password;
+        if (this.otherStrategy === AuthStrategies.Keys) {
+            this.otherStrategy = AuthStrategies.Password;
          } else {
-            this.otherStrategy = AUTH_STRATEGIES.Keys;
+            this.otherStrategy = AuthStrategies.Keys;
          }
         this.changeDetector.markForCheck();
     }
+
+    @autobind()
+    public togglePasswordVisible() {
+        this.passwordVisible = !this.passwordVisible;
+    }
 }
diff --git a/app/components/node/connect/property-display/node-property-display.html b/app/components/node/connect/property-display/node-property-display.html
@@ -24,9 +24,22 @@ <h5 class="label-small">SSH Key File</h5>
         <div class="display-field">{{publicKeyFile}}</div>
     </div>
     <div class="custom-property" *ngIf="!usingSSHKeys">
-        <h5 class="label-small">Password</h5>
-        <input blInput (input)="setPassword($event)" [value]="credentials.password" id="password-input" type="password" placeholder="(Optional)" />
-        <span class="clipboard-info">Password will be copied to clipboard on Connect</span>
+        <div class="label-small">
+            <span>Password</span>
+            <div class="regenerate-password">
+                <bl-clickable (click)="regeneratePassword()">Regenerate Password</bl-clickable>
+                <i *ngIf="regeneratingPassword" class="fa fa-pulse fa-spinner"></i>
+            </div>
+        </div>
+        <div class="password-input-container" *ngIf="passwordVisible">
+            <input blInput (input)="setPassword($event)" [value]="credentials.password" id="password-input" placeholder="(Autogenerated)" />
+            <i class="set-visible fa fa-eye" (click)="togglePasswordVisible()"></i>
+        </div>
+        <div class="password-input-container" *ngIf="!passwordVisible">
+            <input blInput (input)="setPassword($event)" [value]="credentials.password" id="password-input" type="password" placeholder="(Autogenerated)" />
+            <i class="set-visible fa fa-eye-slash" (click)="togglePasswordVisible()"></i>
+        </div>
+        <span class="clipboard-info" [class.flash]="passwordCopied">{{passwordCopyText}}</span>
     </div>
     <bl-clickable *ngIf="isLinux && hasPublicKey" class="switch-strategy" (click)="switchAuthStrategy()">Use {{otherStrategy}}</bl-clickable>
 </bl-property-group>
diff --git a/app/components/node/connect/property-display/node-property-display.scss b/app/components/node/connect/property-display/node-property-display.scss
@@ -1,5 +1,14 @@
 @import "app/styles/variables";
 
+@keyframes flash {
+    0%, 50%, 100% {
+    }
+
+    25%, 75% {
+        background: map-get($warn, 300);
+    }
+}
+
 bl-node-property-display {
 
     bl-property-group {
@@ -18,13 +27,39 @@ bl-node-property-display {
                 font-size: 13px;
                 flex-basis: 200px;
                 font-weight: bold;
-                line-height: 20px;
+                line-height: 25px;
                 margin-right: 5px;
+
+                .regenerate-password {
+                    font-weight: normal;
+                    position: absolute;
+                    top: 20px;
+                    left: 0;
+
+                    i {
+                        margin-left: 5px;
+                    }
+                }
             }
 
             > input {
                 flex: 1;
-                max-width: 349px;
+                max-width: 350px;
+            }
+
+            > .password-input-container {
+                input {
+                    flex: 1;
+                    width: 350px;
+                }
+
+                .set-visible {
+                    position: absolute;
+                    line-height: 25px;
+                    right: 38px;
+                    font-size: 12pt;
+                    cursor: pointer;
+                }
             }
 
             > .display-field {
@@ -57,6 +92,10 @@ bl-node-property-display {
                 left: 210px;
                 color: $secondary-text;
             }
+
+            .flash {
+                animation: flash 1s 1;
+            }
         }
     }