[PS-74] Fix user authentication state checks (#721)

* Create authService.authStatus, refactor isLocked checks

* Rename authStatus -> getAuthStatus

angular/src/services/auth-guard.service.ts

@@ -1,30 +1,29 @@
 import { Injectable } from "@angular/core";
 import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot } from "@angular/router";
 
+import { AuthService } from "jslib-common/abstractions/auth.service";
 import { KeyConnectorService } from "jslib-common/abstractions/keyConnector.service";
 import { MessagingService } from "jslib-common/abstractions/messaging.service";
-import { StateService } from "jslib-common/abstractions/state.service";
-import { VaultTimeoutService } from "jslib-common/abstractions/vaultTimeout.service";
+import { AuthenticationStatus } from "jslib-common/enums/authenticationStatus";
 
 @Injectable()
 export class AuthGuardService implements CanActivate {
   constructor(
-    private vaultTimeoutService: VaultTimeoutService,
+    private authService: AuthService,
     private router: Router,
     private messagingService: MessagingService,
-    private keyConnectorService: KeyConnectorService,
-    private stateService: StateService
+    private keyConnectorService: KeyConnectorService
   ) {}
 
   async canActivate(route: ActivatedRouteSnapshot, routerState: RouterStateSnapshot) {
-    const isAuthed = await this.stateService.getIsAuthenticated();
-    if (!isAuthed) {
+    const authStatus = await this.authService.getAuthStatus();
+
+    if (authStatus === AuthenticationStatus.LoggedOut) {
       this.messagingService.send("authBlocked", { url: routerState.url });
       return false;
     }
 
-    const locked = await this.vaultTimeoutService.isLocked();
-    if (locked) {
+    if (authStatus === AuthenticationStatus.Locked) {
       if (routerState != null) {
         this.messagingService.send("lockedUrl", { url: routerState.url });
       }

angular/src/services/jslib-services.module.ts

@@ -303,6 +303,7 @@ export const SYSTEM_LANGUAGE = new InjectionToken<string>("SYSTEM_LANGUAGE");
         PolicyServiceAbstraction,
         KeyConnectorServiceAbstraction,
         StateServiceAbstraction,
+        AuthServiceAbstraction,
         LOCKED_CALLBACK,
         LOGOUT_CALLBACK,
       ],
@@ -346,11 +347,11 @@ export const SYSTEM_LANGUAGE = new InjectionToken<string>("SYSTEM_LANGUAGE");
         SyncServiceAbstraction,
         AppIdServiceAbstraction,
         ApiServiceAbstraction,
-        VaultTimeoutServiceAbstraction,
         EnvironmentServiceAbstraction,
         LOGOUT_CALLBACK,
         LogService,
         StateServiceAbstraction,
+        AuthServiceAbstraction,
       ],
     },
     {

angular/src/services/lock-guard.service.ts

@@ -1,27 +1,24 @@
 import { Injectable } from "@angular/core";
 import { CanActivate, Router } from "@angular/router";
 
-import { StateService } from "jslib-common/abstractions/state.service";
-import { VaultTimeoutService } from "jslib-common/abstractions/vaultTimeout.service";
+import { AuthService } from "jslib-common/abstractions/auth.service";
+import { AuthenticationStatus } from "jslib-common/enums/authenticationStatus";
 
 @Injectable()
 export class LockGuardService implements CanActivate {
   protected homepage = "vault";
   protected loginpage = "login";
-  constructor(
-    private vaultTimeoutService: VaultTimeoutService,
-    private router: Router,
-    private stateService: StateService
-  ) {}
+  constructor(private authService: AuthService, private router: Router) {}
 
   async canActivate() {
-    if (await this.vaultTimeoutService.isLocked()) {
+    const authStatus = await this.authService.getAuthStatus();
+
+    if (authStatus === AuthenticationStatus.Locked) {
       return true;
     }
 
-    const redirectUrl = (await this.stateService.getIsAuthenticated())
-      ? [this.homepage]
-      : [this.loginpage];
+    const redirectUrl =
+      authStatus === AuthenticationStatus.LoggedOut ? [this.loginpage] : [this.homepage];
 
     this.router.navigate(redirectUrl);
     return false;

angular/src/services/unauth-guard.service.ts

@@ -1,27 +1,25 @@
 import { Injectable } from "@angular/core";
 import { CanActivate, Router } from "@angular/router";
 
-import { StateService } from "jslib-common/abstractions/state.service";
-import { VaultTimeoutService } from "jslib-common/abstractions/vaultTimeout.service";
+import { AuthService } from "jslib-common/abstractions/auth.service";
+import { AuthenticationStatus } from "jslib-common/enums/authenticationStatus";
 
 @Injectable()
 export class UnauthGuardService implements CanActivate {
   protected homepage = "vault";
-  constructor(
-    private vaultTimeoutService: VaultTimeoutService,
-    private router: Router,
-    private stateService: StateService
-  ) {}
+  constructor(private authService: AuthService, private router: Router) {}
 
   async canActivate() {
-    const isAuthed = await this.stateService.getIsAuthenticated();
-    if (isAuthed) {
-      const locked = await this.vaultTimeoutService.isLocked();
-      if (locked) {
-        return this.router.createUrlTree(["lock"]);
-      }
-      return this.router.createUrlTree([this.homepage]);
+    const authStatus = await this.authService.getAuthStatus();
+
+    if (authStatus === AuthenticationStatus.LoggedOut) {
+      return true;
     }
-    return true;
+
+    if (authStatus === AuthenticationStatus.Locked) {
+      return this.router.createUrlTree(["lock"]);
+    }
+
+    return this.router.createUrlTree([this.homepage]);
   }
 }

common/src/abstractions/auth.service.ts

@@ -1,3 +1,4 @@
+import { AuthenticationStatus } from "../enums/authenticationStatus";
 import { AuthResult } from "../models/domain/authResult";
 import {
   ApiLogInCredentials,
@@ -22,4 +23,5 @@ export abstract class AuthService {
   authingWithApiKey: () => boolean;
   authingWithSso: () => boolean;
   authingWithPassword: () => boolean;
+  getAuthStatus: (userId?: string) => Promise<AuthenticationStatus>;
 }

common/src/abstractions/vaultTimeout.service.ts

@@ -1,5 +1,4 @@
 export abstract class VaultTimeoutService {
-  isLocked: (userId?: string) => Promise<boolean>;
   checkVaultTimeout: () => Promise<void>;
   lock: (allowSoftLock?: boolean, userId?: string) => Promise<void>;
   logOut: (userId?: string) => Promise<void>;

common/src/enums/authenticationStatus.ts

@@ -1,6 +1,5 @@
 export enum AuthenticationStatus {
-  Locked = "locked",
-  Unlocked = "unlocked",
-  LoggedOut = "loggedOut",
-  Active = "active",
+  LoggedOut = 0,
+  Locked = 1,
+  Unlocked = 2,
 }

common/src/services/auth.service.ts

@@ -11,8 +11,10 @@ import { PlatformUtilsService } from "../abstractions/platformUtils.service";
 import { StateService } from "../abstractions/state.service";
 import { TokenService } from "../abstractions/token.service";
 import { TwoFactorService } from "../abstractions/twoFactor.service";
+import { AuthenticationStatus } from "../enums/authenticationStatus";
 import { AuthenticationType } from "../enums/authenticationType";
 import { KdfType } from "../enums/kdfType";
+import { KeySuffixOptions } from "../enums/keySuffixOptions";
 import { ApiLogInStrategy } from "../misc/logInStrategies/apiLogin.strategy";
 import { PasswordLogInStrategy } from "../misc/logInStrategies/passwordLogin.strategy";
 import { SsoLogInStrategy } from "../misc/logInStrategies/ssoLogin.strategy";
@@ -157,6 +159,31 @@ export class AuthService implements AuthServiceAbstraction {
     return this.logInStrategy instanceof PasswordLogInStrategy;
   }
 
+  async getAuthStatus(userId?: string): Promise<AuthenticationStatus> {
+    const isAuthenticated = await this.stateService.getIsAuthenticated({ userId: userId });
+    if (!isAuthenticated) {
+      return AuthenticationStatus.LoggedOut;
+    }
+
+    // Keys aren't stored for a device that is locked or logged out
+    // Make sure we're logged in before checking this, otherwise we could mix up those states
+    const neverLock =
+      (await this.cryptoService.hasKeyStored(KeySuffixOptions.Auto, userId)) &&
+      !(await this.stateService.getEverBeenUnlocked({ userId: userId }));
+    if (neverLock) {
+      // TODO: This also _sets_ the key so when we check memory in the next line it finds a key.
+      // We should refactor here.
+      await this.cryptoService.getKey(KeySuffixOptions.Auto, userId);
+    }
+
+    const hasKeyInMemory = await this.cryptoService.hasKeyInMemory(userId);
+    if (!hasKeyInMemory) {
+      return AuthenticationStatus.Locked;
+    }
+
+    return AuthenticationStatus.Unlocked;
+  }
+
   async makePreloginKey(masterPassword: string, email: string): Promise<SymmetricCryptoKey> {
     email = email.trim().toLowerCase();
     let kdf: KdfType = null;

common/src/services/notifications.service.ts

@@ -3,12 +3,13 @@ import * as signalRMsgPack from "@microsoft/signalr-protocol-msgpack";
 
 import { ApiService } from "../abstractions/api.service";
 import { AppIdService } from "../abstractions/appId.service";
+import { AuthService } from "../abstractions/auth.service";
 import { EnvironmentService } from "../abstractions/environment.service";
 import { LogService } from "../abstractions/log.service";
 import { NotificationsService as NotificationsServiceAbstraction } from "../abstractions/notifications.service";
 import { StateService } from "../abstractions/state.service";
 import { SyncService } from "../abstractions/sync.service";
-import { VaultTimeoutService } from "../abstractions/vaultTimeout.service";
+import { AuthenticationStatus } from "../enums/authenticationStatus";
 import { NotificationType } from "../enums/notificationType";
 import {
   NotificationResponse,
@@ -29,11 +30,11 @@ export class NotificationsService implements NotificationsServiceAbstraction {
     private syncService: SyncService,
     private appIdService: AppIdService,
     private apiService: ApiService,
-    private vaultTimeoutService: VaultTimeoutService,
     private environmentService: EnvironmentService,
     private logoutCallback: (expired: boolean) => Promise<void>,
     private logService: LogService,
-    private stateService: StateService
+    private stateService: StateService,
+    private authService: AuthService
   ) {
     this.environmentService.urls.subscribe(() => {
       if (!this.inited) {
@@ -216,11 +217,8 @@ export class NotificationsService implements NotificationsServiceAbstraction {
   }
 
   private async isAuthedAndUnlocked() {
-    if (await this.stateService.getIsAuthenticated()) {
-      const locked = await this.vaultTimeoutService.isLocked();
-      return !locked;
-    }
-    return false;
+    const authStatus = await this.authService.getAuthStatus();
+    return authStatus >= AuthenticationStatus.Unlocked;
   }
 
   private random(min: number, max: number) {

common/src/services/vaultTimeout.service.ts

@@ -1,3 +1,4 @@
+import { AuthService } from "../abstractions/auth.service";
 import { CipherService } from "../abstractions/cipher.service";
 import { CollectionService } from "../abstractions/collection.service";
 import { CryptoService } from "../abstractions/crypto.service";
@@ -10,7 +11,7 @@ import { SearchService } from "../abstractions/search.service";
 import { StateService } from "../abstractions/state.service";
 import { TokenService } from "../abstractions/token.service";
 import { VaultTimeoutService as VaultTimeoutServiceAbstraction } from "../abstractions/vaultTimeout.service";
-import { KeySuffixOptions } from "../enums/keySuffixOptions";
+import { AuthenticationStatus } from "../enums/authenticationStatus";
 import { PolicyType } from "../enums/policyType";
 
 export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
@@ -28,6 +29,7 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
     private policyService: PolicyService,
     private keyConnectorService: KeyConnectorService,
     private stateService: StateService,
+    private authService: AuthService,
     private lockedCallback: (userId?: string) => Promise<void> = null,
     private loggedOutCallback: (expired: boolean, userId?: string) => Promise<void> = null
   ) {}
@@ -48,20 +50,6 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
     setInterval(() => this.checkVaultTimeout(), 10 * 1000); // check every 10 seconds
   }
 
-  // Keys aren't stored for a device that is locked or logged out.
-  async isLocked(userId?: string): Promise<boolean> {
-    const neverLock =
-      (await this.cryptoService.hasKeyStored(KeySuffixOptions.Auto, userId)) &&
-      !(await this.stateService.getEverBeenUnlocked({ userId: userId }));
-    if (neverLock) {
-      // TODO: This also _sets_ the key so when we check memory in the next line it finds a key.
-      // We should refactor here.
-      await this.cryptoService.getKey(KeySuffixOptions.Auto, userId);
-    }
-
-    return !(await this.cryptoService.hasKeyInMemory(userId));
-  }
-
   async checkVaultTimeout(): Promise<void> {
     if (await this.platformUtilsService.isViewOpen()) {
       return;
@@ -187,16 +175,12 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
     await this.stateService.setProtectedPin(null, { userId: userId });
   }
 
-  private async isLoggedOut(userId?: string): Promise<boolean> {
-    return !(await this.stateService.getIsAuthenticated({ userId: userId }));
-  }
-
   private async shouldLock(userId: string): Promise<boolean> {
-    if (await this.isLoggedOut(userId)) {
-      return false;
-    }
-
-    if (await this.isLocked(userId)) {
+    const authStatus = await this.authService.getAuthStatus(userId);
+    if (
+      authStatus === AuthenticationStatus.Locked ||
+      authStatus === AuthenticationStatus.LoggedOut
+    ) {
       return false;
     }