[Account Switching] Misc Bug Fixes and Refactors (#1223)

* [bug] Pull serverUrl directly from stateService for the account switcher

Create a small extended Account model for handling the switchers server url, and pull environment urls from disk where they actually live

* [refactor] Add a message handler for switching accounts

* This allows for logic reuse between manually switching accounts and automatically switching accounts on login
* This commit also adds a loading spinner to app root while syncing after a switch

* [bug] Remove vertical scrollbar

* An old styling fix to add extra height and padding seems to be now creating an unecassary scroll bar. It is likely that since making more use of flexbox for our containers that this issue has been resolved without the manually added extra hight & padding

* [refactor] Turn down activity monitoring

Saving last activity is a disk call, and we currently do this a lot more than is necassary. For example:
* We track mousedown & click, which is redundant
* We track every mouse movement regardless of if an action is taken. This seems inappropriate for use in locking behavior.

* [bug] Address potential race condition when locking

Sometimes when swapping between an unlocked account and a locked account a race condition occurs that swaps the user but doesn't redirect to the lock screen
This commit just adds some awaits and restructures lock order of operations to be more in line with other message handlers

* [refactor] Change click event to mousedown event for the account switcher

This is simply a little snappier, and ensures we stay ahead of change detection and don't get stuck not properly interpreting the action

* [chore] Update jslib

* [chore] Linter fixes

* [chore] Linter fixes

* [chore] Update jslib

* [chore] Update jslib

jslib

@@ -1 +1 @@
-Subproject commit def3c47a1e27a48466830e4e688efa40aae94aba
+Subproject commit 172392ff3b3d8e68795549deef5433c0d1329c9c

src/app/app.component.ts

@@ -56,14 +56,21 @@ const SyncInterval = 6 * 60 * 60 * 1000; // 6 hours
 @Component({
   selector: "app-root",
   styles: [],
-  template: ` <ng-template #settings></ng-template>
+  template: `
+    <ng-template #settings></ng-template>
     <ng-template #premium></ng-template>
     <ng-template #passwordHistory></ng-template>
     <ng-template #appFolderAddEdit></ng-template>
     <ng-template #exportVault></ng-template>
     <ng-template #appPasswordGenerator></ng-template>
     <app-header></app-header>
-    <div id="container"><router-outlet></router-outlet></div>`,
+    <div id="container">
+      <div class="loading" *ngIf="loading">
+        <i class="fa fa-spinner fa-spin fa-3x" aria-hidden="true"></i>
+      </div>
+      <router-outlet *ngIf="!loading"></router-outlet>
+    </div>
+  `,
 })
 export class AppComponent implements OnInit {
   @ViewChild("settings", { read: ViewContainerRef, static: true }) settingsRef: ViewContainerRef;
@@ -77,6 +84,8 @@ export class AppComponent implements OnInit {
   @ViewChild("appPasswordGenerator", { read: ViewContainerRef, static: true })
   passwordGeneratorModalRef: ViewContainerRef;
 
+  loading = false;
+
   private lastActivity: number = null;
   private modal: ModalRef = null;
   private idleTimer: number = null;
@@ -118,10 +127,8 @@ export class AppComponent implements OnInit {
         await this.updateAppMenu();
       }, 1000);
 
-      window.onmousemove = () => this.recordActivity();
-      window.onmousedown = () => this.recordActivity();
       window.ontouchstart = () => this.recordActivity();
-      window.onclick = () => this.recordActivity();
+      window.onmousedown = () => this.recordActivity();
       window.onscroll = () => this.recordActivity();
       window.onkeypress = () => this.recordActivity();
     });
@@ -164,16 +171,16 @@ export class AppComponent implements OnInit {
             if (this.modal != null) {
               this.modal.close();
             }
-            this.updateAppMenu();
             if (
               message.userId == null ||
               message.userId === (await this.stateService.getUserId())
             ) {
-              this.router.navigate(["lock"]);
+              await this.router.navigate(["lock"]);
             }
             this.notificationsService.updateConnection();
-            await this.systemService.clearPendingClipboard();
+            await this.updateAppMenu();
             this.systemService.startProcessReload();
+            await this.systemService.clearPendingClipboard();
             break;
           case "reloadProcess":
             window.location.reload(true);
@@ -310,6 +317,21 @@ export class AppComponent implements OnInit {
             await this.keyConnectorService.setConvertAccountRequired(true);
             this.router.navigate(["/remove-password"]);
             break;
+          case "switchAccount":
+            if (message.userId != null) {
+              await this.stateService.setActiveUser(message.userId);
+            }
+            const locked = await this.vaultTimeoutService.isLocked(message.userId);
+            if (locked) {
+              this.messagingService.send("locked", { userId: message.userId });
+            } else {
+              this.messagingService.send("unlocked");
+              this.loading = true;
+              await this.syncService.fullSync(true);
+              this.loading = false;
+              this.router.navigate(["vault"]);
+            }
+            break;
         }
       });
     });
@@ -447,14 +469,7 @@ export class AppComponent implements OnInit {
     if (this.stateService.activeAccount.getValue() == null) {
       this.router.navigate(["login"]);
     } else {
-      const locked = await this.vaultTimeoutService.isLocked();
+      this.messagingService.send("switchAccount");
-      if (locked) {
-        this.messagingService.send("locked");
-      } else {
-        this.messagingService.send("unlocked");
-        this.messagingService.send("syncVault");
-        this.router.navigate(["vault"]);
-      }
     }
 
     await this.updateAppMenu();

src/app/layout/account-switcher.component.html

@@ -34,11 +34,10 @@
         class="account"
         [ngClass]="{ active: a.value.profile.authenticationStatus == 'active' }"
         href="#"
-        appStopClick
+        (mousedown)="switch(a.key)"
-        (click)="switch(a.key)"
       >
         <span class="email">{{ a.value.profile.email }}</span>
-        <span class="server">{{ a.value.settings.environmentUrls.server }}</span>
+        <span class="server">{{ a.value.serverUrl }}</span>
         <span class="status">{{ a.value.profile.authenticationStatus }}</span>
       </a>
     </div>

src/app/layout/account-switcher.component.ts

@@ -10,6 +10,21 @@ import { AuthenticationStatus } from "jslib-common/enums/authenticationStatus";
 
 import { Account } from "jslib-common/models/domain/account";
 
+export class SwitcherAccount extends Account {
+  get serverUrl() {
+    return this.removeWebProtocolFromString(
+      this.settings?.environmentUrls?.base ??
+        this.settings?.environmentUrls.api ??
+        "https://bitwarden.com"
+    );
+  }
+
+  private removeWebProtocolFromString(urlString: string) {
+    const regex = /http(s)?(:)?(\/\/)?|(\/\/)?(www\.)?/g;
+    return urlString.replace(regex, "");
+  }
+}
+
 @Component({
   selector: "app-account-switcher",
   templateUrl: "account-switcher.component.html",
@@ -36,8 +51,9 @@ import { Account } from "jslib-common/models/domain/account";
 })
 export class AccountSwitcherComponent implements OnInit {
   isOpen: boolean = false;
-  accounts: { [userId: string]: Account } = {};
+  accounts: { [userId: string]: SwitcherAccount } = {};
   activeAccountEmail: string;
+  serverUrl: string;
 
   get showSwitcher() {
     return this.accounts != null && Object.keys(this.accounts).length > 0;
@@ -46,8 +62,7 @@ export class AccountSwitcherComponent implements OnInit {
   constructor(
     private stateService: StateService,
     private vaultTimeoutService: VaultTimeoutService,
-    private messagingService: MessagingService,
+    private messagingService: MessagingService
-    private router: Router
   ) {}
 
   async ngOnInit(): Promise<void> {
@@ -64,7 +79,7 @@ export class AccountSwitcherComponent implements OnInit {
         }
       }
 
-      this.accounts = accounts;
+      this.accounts = await this.createSwitcherAccounts(accounts);
       this.activeAccountEmail = await this.stateService.getEmail();
     });
   }
@@ -74,14 +89,24 @@ export class AccountSwitcherComponent implements OnInit {
   }
 
   async switch(userId: string) {
-    await this.stateService.setActiveUser(userId);
+    this.messagingService.send("switchAccount", { userId: userId });
-    const locked = await this.vaultTimeoutService.isLocked(userId);
+    this.toggle();
-    if (locked) {
-      this.messagingService.send("locked", { userId: userId });
-    } else {
-      this.messagingService.send("unlocked");
-      this.messagingService.send("syncVault");
-      this.router.navigate(["vault"]);
   }
+
+  private async createSwitcherAccounts(baseAccounts: {
+    [userId: string]: Account;
+  }): Promise<{ [userId: string]: SwitcherAccount }> {
+    const switcherAccounts: { [userId: string]: SwitcherAccount } = {};
+    for (const userId in baseAccounts) {
+      if (userId == null) {
+        continue;
+      }
+      // environmentUrls are stored on disk and must be retrieved seperatly from the in memory state offered from subscribing to accounts
+      baseAccounts[userId].settings.environmentUrls = await this.stateService.getEnvironmentUrls({
+        userId: userId,
+      });
+      switcherAccounts[userId] = new SwitcherAccount(baseAccounts[userId]);
+    }
+    return switcherAccounts;
   }
 }

src/scss/misc.scss

@@ -316,7 +316,8 @@ form,
   }
 }
 
-app-root > #loading {
+app-root > #loading,
+.loading {
   display: flex;
   text-align: center;
   justify-content: center;

src/scss/pages.scss

@@ -11,13 +11,11 @@
   height: 100%;
 
   @media (min-height: 500px) {
-    height: calc(100% + 50px);
+    height: calc(100%);
-    padding-bottom: 50px;
   }
 
   @media (min-height: 800px) {
-    height: calc(100% + 300px);
+    height: calc(100%);
-    padding-bottom: 300px;
   }
 
   img {