From f32b917a9f41d877eb86da7f831dec1c49e27fc4 Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Wed, 12 Jan 2022 09:23:00 -0500 Subject: [PATCH] [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 | 2 +- src/app/app.component.ts | 47 ++++++++++++------ .../layout/account-switcher.component.html | 5 +- src/app/layout/account-switcher.component.ts | 49 ++++++++++++++----- src/scss/misc.scss | 3 +- src/scss/pages.scss | 6 +-- 6 files changed, 75 insertions(+), 37 deletions(-) diff --git a/jslib b/jslib index def3c47a1e..172392ff3b 160000 --- a/jslib +++ b/jslib @@ -1 +1 @@ -Subproject commit def3c47a1e27a48466830e4e688efa40aae94aba +Subproject commit 172392ff3b3d8e68795549deef5433c0d1329c9c diff --git a/src/app/app.component.ts b/src/app/app.component.ts index 1e7282a4f5..bedab39c47 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -56,14 +56,21 @@ const SyncInterval = 6 * 60 * 60 * 1000; // 6 hours @Component({ selector: "app-root", styles: [], - template: ` + template: ` + -
`, +
+
+ +
+ +
+ `, }) 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(); - if (locked) { - this.messagingService.send("locked"); - } else { - this.messagingService.send("unlocked"); - this.messagingService.send("syncVault"); - this.router.navigate(["vault"]); - } + this.messagingService.send("switchAccount"); } await this.updateAppMenu(); diff --git a/src/app/layout/account-switcher.component.html b/src/app/layout/account-switcher.component.html index 62c8922c18..b6b2839b5d 100644 --- a/src/app/layout/account-switcher.component.html +++ b/src/app/layout/account-switcher.component.html @@ -34,11 +34,10 @@ class="account" [ngClass]="{ active: a.value.profile.authenticationStatus == 'active' }" href="#" - appStopClick - (click)="switch(a.key)" + (mousedown)="switch(a.key)" > {{ a.value.profile.email }} - {{ a.value.settings.environmentUrls.server }} + {{ a.value.serverUrl }} {{ a.value.profile.authenticationStatus }} diff --git a/src/app/layout/account-switcher.component.ts b/src/app/layout/account-switcher.component.ts index b8c38cd6b0..7526c7e844 100644 --- a/src/app/layout/account-switcher.component.ts +++ b/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 router: Router + private messagingService: MessagingService ) {} async ngOnInit(): Promise { @@ -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); - const locked = await this.vaultTimeoutService.isLocked(userId); - if (locked) { - this.messagingService.send("locked", { userId: userId }); - } else { - this.messagingService.send("unlocked"); - this.messagingService.send("syncVault"); - this.router.navigate(["vault"]); + this.messagingService.send("switchAccount", { userId: userId }); + this.toggle(); + } + + 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; } } diff --git a/src/scss/misc.scss b/src/scss/misc.scss index 3a7023340c..aa38b0f86d 100644 --- a/src/scss/misc.scss +++ b/src/scss/misc.scss @@ -316,7 +316,8 @@ form, } } -app-root > #loading { +app-root > #loading, +.loading { display: flex; text-align: center; justify-content: center; diff --git a/src/scss/pages.scss b/src/scss/pages.scss index a36641acb1..844b61cac0 100644 --- a/src/scss/pages.scss +++ b/src/scss/pages.scss @@ -11,13 +11,11 @@ height: 100%; @media (min-height: 500px) { - height: calc(100% + 50px); - padding-bottom: 50px; + height: calc(100%); } @media (min-height: 800px) { - height: calc(100% + 300px); - padding-bottom: 300px; + height: calc(100%); } img {