feat: handle unprotected session expiration (#747)

* feat: hide note contents if the protection expires when the protected note is open and wasn't edited for a while * feat: handle session expiration for opened protected note for both plain advanced editors * fix: if after canceling session expiry modal only one unprotected note stays selected, show its contents in the editor * refactor: handle session expiration for opened protected note (move the logic to web client) * feat: handle the case of selecting "Don't remember" option in session expiry dialog * test (WIP): add unit tests for protecting opened note after the session has expired * test: add remaining unit tests * refactor: move the opened note protection logic to "editor_view" * refactor: reviewer comments - don't rely on user signed-in/out status to require authentication for protected note - remove unnecessary async/awaits - better wording on ui * refactor: reviewer's comments: - use snjs method to check if "Don't remember" option is selected in authentication modal - move the constant to snjs - fix eslint error * refactor: avoid `any` type for `appEvent` payload * test: add unit tests * chore: update function name * refactor: use simpler protection session event types * refactor: protected access terminology * refactor: start counting idle timer after every edit (instead of counting by interval in spite of edits) * test: unit tests * style: don't give extra brightness to the "View Note"/"Authenticate" button on hover/focus * chore: bump snjs version Co-authored-by: Mo Bitar <me@bitar.io>
2021-12-14 19:14:01 +04:00
parent f494b106b9
commit 8db549f6f6
26 changed files with 418 additions and 755 deletions
--- a/app/assets/javascripts/views/abstract/pure_view_ctrl.ts
+++ b/app/assets/javascripts/views/abstract/pure_view_ctrl.ts
@@ -129,8 +129,8 @@ export class PureViewCtrl<P = CtrlProps, S = CtrlState> {
    if (this.application!.isLaunched()) {
      this.onAppLaunch();
    }
-    this.unsubApp = this.application!.addEventObserver(async (eventName) => {
-      this.onAppEvent(eventName);
+    this.unsubApp = this.application!.addEventObserver(async (eventName, data: any) => {
+      this.onAppEvent(eventName, data);
      if (eventName === ApplicationEvent.Started) {
        await this.onAppStart();
      } else if (eventName === ApplicationEvent.Launched) {
@@ -147,7 +147,7 @@ export class PureViewCtrl<P = CtrlProps, S = CtrlState> {
    });
  }

-  onAppEvent(eventName: ApplicationEvent) {
+  onAppEvent(eventName: ApplicationEvent, data?: any) {
    /** Optional override */
  }

--- a/app/assets/javascripts/views/application/application_view.ts
+++ b/app/assets/javascripts/views/application/application_view.ts
@@ -19,9 +19,9 @@ class ApplicationViewCtrl extends PureViewCtrl<unknown, {
  needsUnlock?: boolean,
  appClass: string,
 }> {
-  public platformString: string
-  private notesCollapsed = false
-  private tagsCollapsed = false
+  public platformString: string;
+  private notesCollapsed = false;
+  private tagsCollapsed = false;
  /**
   * To prevent stale state reads (setState is async),
   * challenges is a mutable array
@@ -108,14 +108,17 @@ class ApplicationViewCtrl extends PureViewCtrl<unknown, {
  /** @override */
  async onAppEvent(eventName: ApplicationEvent) {
    super.onAppEvent(eventName);
-    if (eventName === ApplicationEvent.LocalDatabaseReadError) {
-      alertDialog({
-        text: 'Unable to load local database. Please restart the app and try again.'
-      });
-    } else if (eventName === ApplicationEvent.LocalDatabaseWriteError) {
-      alertDialog({
-        text: 'Unable to write to local database. Please restart the app and try again.'
-      });
+    switch (eventName) {
+      case ApplicationEvent.LocalDatabaseReadError:
+        alertDialog({
+          text: 'Unable to load local database. Please restart the app and try again.'
+        });
+        break;
+      case ApplicationEvent.LocalDatabaseWriteError:
+        alertDialog({
+          text: 'Unable to write to local database. Please restart the app and try again.'
+        });
+        break;
    }
  }

--- a/app/assets/javascripts/views/challenge_modal/challenge_modal.tsx
+++ b/app/assets/javascripts/views/challenge_modal/challenge_modal.tsx
@@ -352,8 +352,8 @@ function ChallengePrompts({
      {/** ProtectionSessionDuration can't just be an input field */}
      {prompt.validation === ChallengeValidation.ProtectionSessionDuration ? (
        <div key={prompt.id} className="sk-panel-row">
-          <div className="sk-horizontal-group">
-            <div className="sk-p sk-bold">Remember For</div>
+          <div className="sk-horizontal-group mt-3">
+            <div className="sk-p sk-bold">Allow protected access for</div>
            {ProtectionSessionDurations.map((option) => (
              <a
                className={
@@ -374,10 +374,13 @@ function ChallengePrompts({
        </div>
      ) : (
        <div key={prompt.id} className="sk-panel-row">
-          <form className="w-full" onSubmit={(event) => {
-            event.preventDefault();
-            ctrl.submit();
-          }}>
+          <form
+            className="w-full"
+            onSubmit={(event) => {
+              event.preventDefault();
+              ctrl.submit();
+            }}
+          >
            <input
              className="sk-input contrast"
              value={ctrl.state.values[prompt.id]!.value as string | number}
--- a/app/assets/javascripts/views/constants.ts
+++ b/app/assets/javascripts/views/constants.ts
@@ -1,4 +1,4 @@
 export const PANEL_NAME_NOTES = 'notes';
-export const PANEL_NAME_TAGS  = 'tags';
-// eslint-disable-next-line no-useless-escape
-export const EMAIL_REGEX = /^([a-zA-Z0-9!#$%&'*+\/=?^_`{|}~-]+(?:\.[a-zA-Z0-9!#$%&'*+\/=?^_`{|}~-]+)*@(?:[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?\.)+[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?)$/;
+export const PANEL_NAME_TAGS = 'tags';
+export const EMAIL_REGEX =
+  /^([a-zA-Z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-zA-Z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?\.)+[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?)$/;
--- a/app/assets/javascripts/views/editor/editor-view.pug
+++ b/app/assets/javascripts/views/editor/editor-view.pug
@@ -2,6 +2,7 @@
  protected-note-panel.h-full.flex.justify-center.items-center(
    ng-if='self.state.showProtectedWarning'
    app-state='self.appState'
+    require-authentication-for-protected-note='self.requireAuthenticationForProtectedNote'
    on-view-note='self.dismissProtectedWarning()'
  )
  .flex-grow.flex.flex-col(
--- a/app/assets/javascripts/views/editor/editor_view.test.ts
+++ b/app/assets/javascripts/views/editor/editor_view.test.ts
@@ -0,0 +1,196 @@
+/**
+ * @jest-environment jsdom
+ */
+
+import { EditorViewCtrl } from '@Views/editor/editor_view';
+import {
+  ApplicationEvent,
+  ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction,
+} from '@standardnotes/snjs/';
+
+describe('editor-view', () => {
+  let ctrl: EditorViewCtrl;
+  let setShowProtectedWarningSpy: jest.SpyInstance;
+
+  beforeEach(() => {
+    const $timeout = {} as jest.Mocked<ng.ITimeoutService>;
+    ctrl = new EditorViewCtrl($timeout);
+
+    setShowProtectedWarningSpy = jest.spyOn(ctrl, 'setShowProtectedWarning');
+
+    Object.defineProperties(ctrl, {
+      application: {
+        value: {
+          getAppState: () => {
+            return {
+              notes: {
+                setShowProtectedWarning: jest.fn(),
+              },
+            };
+          },
+          hasProtectionSources: () => true,
+          authorizeNoteAccess: jest.fn(),
+        },
+      },
+      removeComponentsObserver: {
+        value: jest.fn(),
+        writable: true,
+      },
+      removeTrashKeyObserver: {
+        value: jest.fn(),
+        writable: true,
+      },
+      unregisterComponent: {
+        value: jest.fn(),
+        writable: true,
+      },
+      editor: {
+        value: {
+          clearNoteChangeListener: jest.fn(),
+        },
+      },
+    });
+  });
+  beforeEach(() => {
+    jest.useFakeTimers();
+  });
+
+  afterEach(() => {
+    jest.useRealTimers();
+  });
+
+  afterEach(() => {
+    ctrl.deinit();
+  });
+
+  describe('note is protected', () => {
+    beforeEach(() => {
+      Object.defineProperty(ctrl, 'note', {
+        value: {
+          protected: true,
+        },
+      });
+    });
+
+    it("should hide the note if at the time of the session expiration the note wasn't edited for longer than the allowed idle time", async () => {
+      jest
+        .spyOn(ctrl, 'getSecondsElapsedSinceLastEdit')
+        .mockImplementation(
+          () =>
+            ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction +
+            5
+        );
+
+      await ctrl.onAppEvent(ApplicationEvent.UnprotectedSessionExpired);
+
+      expect(setShowProtectedWarningSpy).toHaveBeenCalledWith(true);
+    });
+
+    it('should postpone the note hiding by correct time if the time passed after its last modification is less than the allowed idle time', async () => {
+      const secondsElapsedSinceLastEdit =
+        ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction -
+        3;
+
+      Object.defineProperty(ctrl.note, 'userModifiedDate', {
+        value: new Date(Date.now() - secondsElapsedSinceLastEdit * 1000),
+        configurable: true,
+      });
+
+      await ctrl.onAppEvent(ApplicationEvent.UnprotectedSessionExpired);
+
+      const secondsAfterWhichTheNoteShouldHide =
+        ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction -
+        secondsElapsedSinceLastEdit;
+      jest.advanceTimersByTime((secondsAfterWhichTheNoteShouldHide - 1) * 1000);
+      expect(setShowProtectedWarningSpy).not.toHaveBeenCalled();
+
+      jest.advanceTimersByTime(1 * 1000);
+      expect(setShowProtectedWarningSpy).toHaveBeenCalledWith(true);
+    });
+
+    it('should postpone the note hiding by correct time if the user continued editing it even after the protection session has expired', async () => {
+      const secondsElapsedSinceLastModification = 3;
+      Object.defineProperty(ctrl.note, 'userModifiedDate', {
+        value: new Date(
+          Date.now() - secondsElapsedSinceLastModification * 1000
+        ),
+        configurable: true,
+      });
+
+      await ctrl.onAppEvent(ApplicationEvent.UnprotectedSessionExpired);
+
+      let secondsAfterWhichTheNoteShouldHide =
+        ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction -
+        secondsElapsedSinceLastModification;
+      jest.advanceTimersByTime((secondsAfterWhichTheNoteShouldHide - 1) * 1000);
+
+      // A new modification has just happened
+      Object.defineProperty(ctrl.note, 'userModifiedDate', {
+        value: new Date(),
+        configurable: true,
+      });
+
+      secondsAfterWhichTheNoteShouldHide =
+        ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction;
+      jest.advanceTimersByTime((secondsAfterWhichTheNoteShouldHide - 1) * 1000);
+      expect(setShowProtectedWarningSpy).not.toHaveBeenCalled();
+
+      jest.advanceTimersByTime(1 * 1000);
+      expect(setShowProtectedWarningSpy).toHaveBeenCalledWith(true);
+    });
+  });
+
+  describe('note is unprotected', () => {
+    it('should not call any hiding logic', async () => {
+      Object.defineProperty(ctrl, 'note', {
+        value: {
+          protected: false,
+        },
+      });
+      const hideProtectedNoteIfInactiveSpy = jest.spyOn(
+        ctrl,
+        'hideProtectedNoteIfInactive'
+      );
+
+      await ctrl.onAppEvent(ApplicationEvent.UnprotectedSessionExpired);
+
+      expect(hideProtectedNoteIfInactiveSpy).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('dismissProtectedWarning', () => {
+    describe('the note has protection sources', () => {
+      it('should reveal note contents if the authorization has been passed', async () => {
+        jest
+          .spyOn(ctrl.application, 'authorizeNoteAccess')
+          .mockImplementation(async () => Promise.resolve(true));
+
+        await ctrl.dismissProtectedWarning();
+
+        expect(setShowProtectedWarningSpy).toHaveBeenCalledWith(false);
+      });
+
+      it('should not reveal note contents if the authorization has not been passed', async () => {
+        jest
+          .spyOn(ctrl.application, 'authorizeNoteAccess')
+          .mockImplementation(async () => Promise.resolve(false));
+
+        await ctrl.dismissProtectedWarning();
+
+        expect(setShowProtectedWarningSpy).not.toHaveBeenCalled();
+      });
+    });
+
+    describe('the note does not have protection sources', () => {
+      it('should reveal note contents', async () => {
+        jest
+          .spyOn(ctrl.application, 'hasProtectionSources')
+          .mockImplementation(() => false);
+
+        await ctrl.dismissProtectedWarning();
+
+        expect(setShowProtectedWarningSpy).toHaveBeenCalledWith(false);
+      });
+    });
+  });
+});
--- a/app/assets/javascripts/views/editor/editor_view.ts
+++ b/app/assets/javascripts/views/editor/editor_view.ts
@@ -16,6 +16,7 @@ import {
  PrefKey,
  ComponentMutator,
  PayloadSource,
+  ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction,
 } from '@standardnotes/snjs';
 import { isDesktopApplication } from '@/utils';
 import { KeyboardModifier, KeyboardKey } from '@/services/ioService';
@@ -89,7 +90,7 @@ function sortAlphabetically(array: SNComponent[]): SNComponent[] {
  );
 }

-class EditorViewCtrl extends PureViewCtrl<unknown, EditorState> {
+export class EditorViewCtrl extends PureViewCtrl<unknown, EditorState> {
  /** Passed through template */
  readonly application!: WebApplication;
  readonly editor!: Editor;
@@ -108,6 +109,8 @@ class EditorViewCtrl extends PureViewCtrl<unknown, EditorState> {
  private removeTabObserver?: any;

  private removeComponentsObserver!: () => void;
+  private protectionTimeoutId: ReturnType<typeof setTimeout> | null = null;
+  public requireAuthenticationForProtectedNote = false;

  /* @ngInject */
  constructor($timeout: ng.ITimeoutService) {
@@ -124,14 +127,15 @@ class EditorViewCtrl extends PureViewCtrl<unknown, EditorState> {
    this.setScrollPosition = this.setScrollPosition.bind(this);
    this.resetScrollPosition = this.resetScrollPosition.bind(this);
    this.onEditorLoad = () => {
-      this.application!.getDesktopService().redoSearch();
+      this.application.getDesktopService().redoSearch();
    };
  }

  deinit() {
+    this.clearNoteProtectionInactivityTimer();
    this.editor.clearNoteChangeListener();
    this.removeComponentsObserver();
-    (this.removeComponentsObserver as any) = undefined;
+    (this.removeComponentsObserver as unknown) = undefined;
    this.removeTrashKeyObserver();
    this.removeTrashKeyObserver = undefined;
    this.removeTabObserver && this.removeTabObserver();
@@ -143,8 +147,8 @@ class EditorViewCtrl extends PureViewCtrl<unknown, EditorState> {
    this.unregisterComponent = undefined;
    this.saveTimeout = undefined;
    this.statusTimeout = undefined;
-    (this.onPanelResizeFinish as any) = undefined;
-    (this.editorMenuOnSelect as any) = undefined;
+    (this.onPanelResizeFinish as unknown) = undefined;
+    (this.editorMenuOnSelect as unknown) = undefined;
    super.deinit();
  }

@@ -229,7 +233,7 @@ class EditorViewCtrl extends PureViewCtrl<unknown, EditorState> {
  }

  /** @override */
-  onAppEvent(eventName: ApplicationEvent) {
+  async onAppEvent(eventName: ApplicationEvent) {
    switch (eventName) {
      case ApplicationEvent.PreferencesChanged:
        this.reloadPreferences();
@@ -262,14 +266,64 @@ class EditorViewCtrl extends PureViewCtrl<unknown, EditorState> {
          desc: 'Changes not saved',
        });
        break;
+      case ApplicationEvent.UnprotectedSessionBegan: {
+        this.setShowProtectedWarning(false);
+        break;
+      }
+      case ApplicationEvent.UnprotectedSessionExpired: {
+        if (this.note.protected) {
+          this.hideProtectedNoteIfInactive();
+        }
+        break;
+      }
+    }
+  }
+
+  getSecondsElapsedSinceLastEdit(): number {
+    return (Date.now() - this.note.userModifiedDate.getTime()) / 1000;
+  }
+
+  hideProtectedNoteIfInactive(): void {
+    const secondsElapsedSinceLastEdit = this.getSecondsElapsedSinceLastEdit();
+    if (
+      secondsElapsedSinceLastEdit >=
+      ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction
+    ) {
+      this.setShowProtectedWarning(true);
+    } else {
+      const secondsUntilTheNextCheck =
+        ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction -
+        secondsElapsedSinceLastEdit;
+      this.startNoteProtectionInactivityTimer(secondsUntilTheNextCheck);
+    }
+  }
+
+  startNoteProtectionInactivityTimer(timerDurationInSeconds: number): void {
+    this.clearNoteProtectionInactivityTimer();
+    this.protectionTimeoutId = setTimeout(() => {
+      this.hideProtectedNoteIfInactive();
+    }, timerDurationInSeconds * 1000);
+  }
+
+  clearNoteProtectionInactivityTimer(): void {
+    if (this.protectionTimeoutId) {
+      clearTimeout(this.protectionTimeoutId);
    }
  }

  async handleEditorNoteChange() {
+    this.clearNoteProtectionInactivityTimer();
    this.cancelPendingSetStatus();
    const note = this.editor.note;
+
    const showProtectedWarning =
-      note.protected && !this.application.hasProtectionSources();
+      note.protected &&
+      (!this.application.hasProtectionSources() ||
+        this.application.getProtectionSessionExpiryDate().getTime() <
+          Date.now());
+    this.requireAuthenticationForProtectedNote =
+      note.protected && this.application.hasProtectionSources();
+
    this.setShowProtectedWarning(showProtectedWarning);
    await this.setState({
      showActionsMenu: false,
@@ -288,6 +342,13 @@ class EditorViewCtrl extends PureViewCtrl<unknown, EditorState> {
  }

  async dismissProtectedWarning() {
+    let showNoteContents = true;
+    if (this.application.hasProtectionSources()) {
+      showNoteContents = await this.application.authorizeNoteAccess(this.note);
+    }
+    if (!showNoteContents) {
+      return;
+    }
    this.setShowProtectedWarning(false);
    this.focusTitle();
  }