Fix: Respect view permissions when viewer.initAnnotations() is called (…

…#379)

src/lib/annotations/Annotator.js

@@ -555,7 +555,7 @@ class Annotator extends EventEmitter {
 
         // Do not load any pre-existing annotations if the user does not have
         // the correct permissions
-        if (!this.permissions.canViewAllAnnotations || !this.permissions.canViewOwnAnnotations) {
+        if (!this.permissions.canViewAllAnnotations && !this.permissions.canViewOwnAnnotations) {
             return Promise.resolve(this.threads);
         }
 

src/lib/annotations/BoxAnnotations.js

@@ -2,6 +2,7 @@ import DocAnnotator from './doc/DocAnnotator';
 import ImageAnnotator from './image/ImageAnnotator';
 import DrawingModeController from './drawing/DrawingModeController';
 import { TYPES } from './annotationConstants';
+import { canLoadAnnotations } from './annotatorUtil';
 
 const ANNOTATORS = [
     {
@@ -87,15 +88,17 @@ class BoxAnnotations {
      * Chooses a annotator based on viewer.
      *
      * @param {Object} viewerName - Current preview viewer name
+     * @param {Object} permissions - File permissions
      * @param {Object} [viewerConfig] - Annotation configuration for a specific viewer
      * @param {Array} [disabledAnnotators] - List of disabled annotators
      * @return {Object|null} A copy of the annotator to use, if available
      */
-    determineAnnotator(viewerName, viewerConfig = {}, disabledAnnotators = []) {
-        const annotator = this.getAnnotatorsForViewer(viewerName, disabledAnnotators);
+    determineAnnotator(viewerName, permissions, viewerConfig = {}, disabledAnnotators = []) {
         let modifiedAnnotator = null;
 
-        if (!annotator || viewerConfig.enabled === false) {
+        const hasAnnotationPermissions = canLoadAnnotations(permissions);
+        const annotator = this.getAnnotatorsForViewer(viewerName, disabledAnnotators);
+        if (!hasAnnotationPermissions || !annotator || viewerConfig.enabled === false) {
             return modifiedAnnotator;
         }
 

src/lib/annotations/__tests__/Annotator-test.js

@@ -477,23 +477,29 @@ describe('lib/annotations/Annotator', () => {
                 stubs.serviceMock.expects('getThreadMap').never();
                 annotator.permissions = {
                     canViewAllAnnotations: false,
-                    canViewOwnAnnotations: true
+                    canViewOwnAnnotations: false
                 };
-                let result = annotator.fetchAnnotations();
+                const result = annotator.fetchAnnotations();
                 expect(result instanceof Promise).to.be.truthy;
+            });
 
+            it('should fetch existing annotations if the user can view all annotations', () => {
+                stubs.serviceMock.expects('getThreadMap').returns(Promise.resolve());
                 annotator.permissions = {
-                    canViewAllAnnotations: true,
-                    canViewOwnAnnotations: false
+                    canViewAllAnnotations: false,
+                    canViewOwnAnnotations: true
                 };
-                result = annotator.fetchAnnotations();
+                const result = annotator.fetchAnnotations();
                 expect(result instanceof Promise).to.be.truthy;
+            });
 
+            it('should fetch existing annotations if the user can view all annotations', () => {
+                stubs.serviceMock.expects('getThreadMap').returns(Promise.resolve());
                 annotator.permissions = {
-                    canViewAllAnnotations: false,
+                    canViewAllAnnotations: true,
                     canViewOwnAnnotations: false
                 };
-                result = annotator.fetchAnnotations();
+                const result = annotator.fetchAnnotations();
                 expect(result instanceof Promise).to.be.truthy;
             });
 

src/lib/annotations/__tests__/BoxAnnotations-test.js

@@ -1,6 +1,7 @@
 /* eslint-disable no-unused-expressions */
 import BoxAnnotations from '../BoxAnnotations';
 import { TYPES } from '../annotationConstants';
+import * as annotatorUtil from '../annotatorUtil';
 import DrawingModeController from '../drawing/DrawingModeController';
 
 let loader;
@@ -62,22 +63,28 @@ describe('lib/annotators/BoxAnnotations', () => {
     describe('determineAnnotator()', () => {
         beforeEach(() => {
             stubs.instantiateControllers = sandbox.stub(loader, 'instantiateControllers');
+            stubs.canLoad = sandbox.stub(annotatorUtil, 'canLoadAnnotations').returns(true);
+        });
+
+        it('should not return an annotator if the user has incorrect permissions/scopes', () => {
+            stubs.canLoad.returns(false);
+            expect(loader.determineAnnotator('Image', {})).to.be.null;
         });
 
         it('should choose the first annotator that matches the viewer', () => {
             const viewer = 'Document';
-            const annotator = loader.determineAnnotator(viewer);
+            const annotator = loader.determineAnnotator(viewer, {});
             expect(annotator.NAME).to.equal(viewer);
             expect(stubs.instantiateControllers).to.be.called;
         });
 
         it('should not choose a disabled annotator', () => {
-            const annotator = loader.determineAnnotator('Image', {}, ['Image']);
+            const annotator = loader.determineAnnotator('Image', {}, {}, ['Image']);
             expect(annotator).to.be.null;
         });
 
-        it('should not return a annotator if no matching annotator is found', () => {
-            const annotator = loader.determineAnnotator('Swf');
+        it('should not return an annotator if no matching annotator is found', () => {
+            const annotator = loader.determineAnnotator('Swf', {});
             expect(annotator).to.be.null;
         });
 
@@ -88,7 +95,7 @@ describe('lib/annotators/BoxAnnotations', () => {
                 VIEWER: ['Document']
             };
             loader.annotators = [docAnnotator];
-            const annotator = loader.determineAnnotator(viewer);
+            const annotator = loader.determineAnnotator(viewer, {});
             docAnnotator.NAME = 'another_name';
             expect(annotator.NAME).to.equal(viewer);
             expect(annotator.NAME).to.not.equal(docAnnotator.NAME);
@@ -98,7 +105,7 @@ describe('lib/annotators/BoxAnnotations', () => {
             const config = {
                 enabled: false
             };
-            const annotator = loader.determineAnnotator('Document', config);
+            const annotator = loader.determineAnnotator('Document', {}, config);
             expect(annotator).to.be.null;
         });
 
@@ -113,7 +120,7 @@ describe('lib/annotators/BoxAnnotations', () => {
                 TYPE: ['point', 'highlight']
             };
             loader.annotators = [docAnnotator];
-            const annotator = loader.determineAnnotator('Document', config);
+            const annotator = loader.determineAnnotator('Document', {}, config);
             expect(annotator.TYPE.includes('point')).to.be.false;
             expect(annotator.TYPE.includes('highlight')).to.be.true;
             expect(annotator).to.deep.equal({

src/lib/annotations/__tests__/annotatorUtil-test.js

@@ -26,7 +26,8 @@ import {
     replacePlaceholders,
     createLocation,
     round,
-    prevDefAndStopProp
+    prevDefAndStopProp,
+    canLoadAnnotations
 } from '../annotatorUtil';
 import {
     STATES,
@@ -38,6 +39,7 @@ import {
 const DIALOG_WIDTH = 81;
 
 const sandbox = sinon.sandbox.create();
+let stubs = {};
 
 describe('lib/annotations/annotatorUtil', () => {
     let childEl;
@@ -633,4 +635,33 @@ describe('lib/annotations/annotatorUtil', () => {
             expect(replacePlaceholders('{2} highlighted {2}', ['Bob', 'Suzy'])).to.equal('Suzy highlighted Suzy');
         });
     });
+
+    describe('canLoadAnnotations()', () => {
+        beforeEach(() => {
+            stubs.permissions = {
+                can_annotate: false,
+                can_view_annotations_all: false,
+                can_view_annotations_self: false
+            };
+        });
+
+        it('should return false if permissions do not exist', () => {
+            expect(canLoadAnnotations()).to.be.false;
+        });
+
+        it('should return true if user has at least can_annotate permissions', () => {
+            stubs.permissions.can_annotate = true;
+            expect(canLoadAnnotations(stubs.permissions)).to.be.true;
+        });
+
+        it('should return true if user has at least can_view_annotations_all permissions', () => {
+            stubs.permissions.can_view_annotations_all = true;
+            expect(canLoadAnnotations(stubs.permissions)).to.be.true;
+        });
+
+        it('should return true if user has at least can_view_annotations_self permissions', () => {
+            stubs.permissions.can_view_annotations_self = true;
+            expect(canLoadAnnotations(stubs.permissions)).to.be.true;
+        });
+    });
 });

src/lib/annotations/annotationConstants.js

@@ -78,6 +78,10 @@ export const SELECTOR_DIALOG_CLOSE = `.${CLASS_DIALOG_CLOSE}`;
 export const SELECTOR_HIGHLIGHT_BTNS = `.${CLASS_HIGHLIGHT_BTNS}`;
 export const SELECTOR_ADD_HIGHLIGHT_BTN = `.${CLASS_ADD_HIGHLIGHT_BTN}`;
 
+export const PERMISSION_ANNOTATE = 'can_annotate';
+export const PERMISSION_CAN_VIEW_ANNOTATIONS_ALL = 'can_view_annotations_all';
+export const PERMISSION_CAN_VIEW_ANNOTATIONS_SELF = 'can_view_annotations_self';
+
 export const DRAW_STATES = {
     idle: 'idle',
     drawing: 'drawing',

src/lib/annotations/annotatorUtil.js

@@ -1,5 +1,8 @@
 import 'whatwg-fetch';
 import {
+    PERMISSION_ANNOTATE,
+    PERMISSION_CAN_VIEW_ANNOTATIONS_ALL,
+    PERMISSION_CAN_VIEW_ANNOTATIONS_SELF,
     TYPES,
     SELECTOR_ANNOTATION_CARET,
     PENDING_STATES,
@@ -279,7 +282,10 @@ export function getAvatarHtml(avatarUrl, userId, userName) {
     let initials = '';
     if (userId !== '0') {
         // http://stackoverflow.com/questions/8133630/spliting-the-first-character-of-the-words
-        initials = userName.replace(/\W*(\w)\w*/g, '$1').toUpperCase().substring(0, 3);
+        initials = userName
+            .replace(/\W*(\w)\w*/g, '$1')
+            .toUpperCase()
+            .substring(0, 3);
     }
 
     const index = parseInt(userId, 10) || 0;
@@ -642,3 +648,23 @@ export function replacePlaceholders(string, placeholderValues) {
         /* eslint-enable no-plusplus */
     });
 }
+
+/**
+ * Determines whether the user has file permissions to annotate, view (either
+ * their own or everyone's) annotations which would allow annotations to at
+ * least be fetched for the current file
+ *
+ * @param {Object} permissions - File permissions
+ * @return {boolean} Whether or not the user has either view OR annotate permissions
+ */
+export function canLoadAnnotations(permissions) {
+    if (!permissions) {
+        return false;
+    }
+
+    const canAnnotate = permissions[PERMISSION_ANNOTATE];
+    const canViewAllAnnotations = permissions[PERMISSION_CAN_VIEW_ANNOTATIONS_ALL];
+    const canViewOwnAnnotations = permissions[PERMISSION_CAN_VIEW_ANNOTATIONS_SELF];
+
+    return !!canAnnotate || !!canViewAllAnnotations || !!canViewOwnAnnotations;
+}

src/lib/constants.js

@@ -61,7 +61,6 @@ export const SELECTOR_BOX_PREVIEW_LOGO_DEFAULT = `.${CLASS_BOX_PREVIEW_LOGO_DEFA
 export const SELECTOR_BOX_PREVIEW_PROGRESS_BAR = `.${CLASS_BOX_PREVIEW_PROGRESS_BAR}`;
 
 export const PERMISSION_DOWNLOAD = 'can_download';
-export const PERMISSION_ANNOTATE = 'can_annotate';
 export const PERMISSION_PREVIEW = 'can_preview';
 
 export const API_HOST = 'https://api.box.com';

src/lib/viewers/BaseViewer.js

@@ -12,10 +12,8 @@ import {
     prefetchAssets,
     createAssetUrlCreator
 } from '../util';
-import { checkPermission } from '../file';
 import Browser from '../Browser';
 import {
-    PERMISSION_ANNOTATE,
     CLASS_FULLSCREEN,
     CLASS_FULLSCREEN_UNSUPPORTED,
     CLASS_HIDDEN,
@@ -659,17 +657,15 @@ class BaseViewer extends EventEmitter {
         const viewerName = this.options.viewer.NAME;
         const annotationsConfig = this.getViewerAnnotationsConfig();
 
-        this.annotatorConf = boxAnnotations.determineAnnotator(viewerName, annotationsConfig);
+        const { file } = this.options;
+        this.annotatorConf = boxAnnotations.determineAnnotator(viewerName, file.permissions, annotationsConfig);
+
+        // No annotatorConf will be returned if the user does not have the correct permissions
         if (!this.annotatorConf) {
             return;
         }
 
-        const { file } = this.options;
-        this.canAnnotate = checkPermission(file, PERMISSION_ANNOTATE);
-
-        if (this.canAnnotate) {
-            this.initAnnotations();
-        }
+        this.initAnnotations();
     }
 
     /**

src/lib/viewers/__tests__/BaseViewer-test.js

@@ -744,7 +744,7 @@ describe('lib/viewers/BaseViewer', () => {
             sandbox.stub(base, 'getViewerOption').returns(annConfig);
             const config = base.getViewerAnnotationsConfig();
             expect(config).to.deep.equal(annConfig);
-            
+
         });
     });
 
@@ -759,7 +759,19 @@ describe('lib/viewers/BaseViewer', () => {
             };
             stubs.areAnnotationsEnabled = sandbox.stub(base, 'areAnnotationsEnabled').returns(true);
             sandbox.stub(base, 'initAnnotations');
-            stubs.checkPermission = sandbox.stub(file, 'checkPermission').returns(false);
+
+            base.options = {
+                viewer: {
+                    NAME: 'VIEWER'
+                },
+                file: {
+                    permissions: {
+                        can_annotate: true,
+                        can_view_annotations_all: true,
+                        can_view_annotations_self: true
+                    }
+                }
+            };
         });
 
         it('should load the appropriate annotator for the current viewer', () => {
@@ -768,7 +780,19 @@ describe('lib/viewers/BaseViewer', () => {
                     return stubs.annotatorConf;
                 }
             }
-            stubs.checkPermission.returns(true);
+
+            base.options = {
+                viewer: {
+                    NAME: 'VIEWER'
+                },
+                file: {
+                    permissions: {
+                        can_annotate: true,
+                        can_view_annotations_all: true,
+                        can_view_annotations_self: true
+                    }
+                }
+            };
 
             window.BoxAnnotations = BoxAnnotations;
             base.loadAnnotator();