Merge branch '15159-disable-open-in-new-tab-when-cannot-serve-inline' into main....
authorStephen Smith <stephen@curii.com>
Thu, 26 Aug 2021 16:51:29 +0000 (12:51 -0400)
committerStephen Smith <stephen@curii.com>
Thu, 26 Aug 2021 16:51:29 +0000 (12:51 -0400)
Arvados-DCO-1.1-Signed-off-by: Stephen Smith <stephen@curii.com>

cypress/integration/collection.spec.js
src/common/config.ts
src/views-components/context-menu/actions/collection-file-viewer-action.test.tsx [new file with mode: 0644]
src/views-components/context-menu/actions/collection-file-viewer-action.tsx
src/views-components/context-menu/actions/helpers.ts
src/views-components/details-panel/collection-details.tsx
src/views-components/details-panel/details-data.tsx
src/views-components/details-panel/details-panel.tsx
src/views-components/details-panel/file-details.tsx
tools/arvados_config.yml

index c169de2f6f4101e41d2235b91bb29cb4d907201b..2cb6281a300bb124d43d0119d2a3d52e246bd545 100644 (file)
@@ -122,12 +122,22 @@ describe('Collection panel tests', function () {
             }).as('sharedGroup').then(function () {
                 // Creates the collection using the admin token so we can set up
                 // a bogus manifest text without block signatures.
-                cy.createCollection(adminUser.token, {
-                    name: 'Test collection',
-                    owner_uuid: this.sharedGroup.uuid,
-                    properties: { someKey: 'someValue' },
-                    manifest_text: `. 37b51d194a7513e45b56f6524f2d51f2+3 0:3:${fileName}\n./${subDirName} 37b51d194a7513e45b56f6524f2d51f2+3 0:3:${fileName}\n`
-                })
+                cy.doRequest('GET', '/arvados/v1/config', null, null)
+                    .its('body').should((clusterConfig) => {
+                      expect(clusterConfig.Collections, "clusterConfig").to.have.property("TrustAllContent", false);
+                      expect(clusterConfig.Services, "clusterConfig").to.have.property("WebDAV").have.property("ExternalURL");
+                      expect(clusterConfig.Services, "clusterConfig").to.have.property("WebDAVDownload").have.property("ExternalURL");
+                      const inlineUrl = clusterConfig.Services.WebDAV.ExternalURL !== ""
+                          ? clusterConfig.Services.WebDAV.ExternalURL
+                          : clusterConfig.Services.WebDAVDownload.ExternalURL;
+                      expect(inlineUrl).to.not.contain("*");
+                    })
+                    .createCollection(adminUser.token, {
+                      name: 'Test collection',
+                      owner_uuid: this.sharedGroup.uuid,
+                      properties: { someKey: 'someValue' },
+                      manifest_text: `. 37b51d194a7513e45b56f6524f2d51f2+3 0:3:${fileName}\n./${subDirName} 37b51d194a7513e45b56f6524f2d51f2+3 0:3:${fileName}\n`
+                    })
                     .as('testCollection').then(function () {
                         // Share the group with active user.
                         cy.createLink(adminUser.token, {
@@ -189,7 +199,7 @@ describe('Collection panel tests', function () {
                             .contains(fileName).rightclick({ force: true });
                         cy.get('[data-cy=context-menu]')
                             .should('contain', 'Download')
-                            .and('contain', 'Open in new tab')
+                            .and('not.contain', 'Open in new tab')
                             .and('contain', 'Copy to clipboard')
                             .and(`${isWritable ? '' : 'not.'}contain`, 'Rename')
                             .and(`${isWritable ? '' : 'not.'}contain`, 'Remove');
@@ -198,7 +208,7 @@ describe('Collection panel tests', function () {
                             .contains(subDirName).rightclick({ force: true });
                         cy.get('[data-cy=context-menu]')
                             .should('not.contain', 'Download')
-                            .and('contain', 'Open in new tab')
+                            .and('not.contain', 'Open in new tab')
                             .and('contain', 'Copy to clipboard')
                             .and(`${isWritable ? '' : 'not.'}contain`, 'Rename')
                             .and(`${isWritable ? '' : 'not.'}contain`, 'Remove');
index 28d4855b47ea7f51c5f702d026fb86edaeb258dd..56f7c4884b19542ee057b937b5ff420809477a9e 100644 (file)
@@ -89,7 +89,8 @@ export interface ClusterConfigJSON {
                 Value: string,
                 Protected?: boolean,
             }
-        }
+        },
+        TrustAllContent: boolean
     };
     Volumes: {
         [key: string]: {
@@ -271,6 +272,7 @@ export const mockClusterConfigJSON = (config: Partial<ClusterConfigJSON>): Clust
     },
     Collections: {
         ForwardSlashNameSubstitution: "",
+        TrustAllContent: false,
     },
     Volumes: {},
     ...config
diff --git a/src/views-components/context-menu/actions/collection-file-viewer-action.test.tsx b/src/views-components/context-menu/actions/collection-file-viewer-action.test.tsx
new file mode 100644 (file)
index 0000000..8b90f58
--- /dev/null
@@ -0,0 +1,117 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+import React from 'react';
+import { mount, configure } from 'enzyme';
+import Adapter from 'enzyme-adapter-react-16';
+import configureMockStore from 'redux-mock-store'
+import { Provider } from 'react-redux';
+import { CollectionFileViewerAction } from './collection-file-viewer-action';
+import { ContextMenuKind } from 'views-components/context-menu/context-menu';
+import { createTree, initTreeNode, setNode, getNodeValue } from "models/tree";
+import { getInlineFileUrl, sanitizeToken } from "./helpers";
+
+const middlewares = [];
+const mockStore = configureMockStore(middlewares);
+
+configure({ adapter: new Adapter() });
+
+describe('CollectionFileViewerAction', () => {
+    let defaultStore;
+    const fileUrl = "https://download.host:12345/c=abcde-4zz18-abcdefghijklmno/t=v2/token2/token3/cat.jpg";
+    const insecureKeepInlineUrl = "https://download.host:12345/";
+    const secureKeepInlineUrl = "https://*.collections.host:12345/";
+
+    beforeEach(() => {
+        let filesTree = createTree();
+        let data = {id: "000", value: {"url": fileUrl}};
+        filesTree = setNode(initTreeNode(data))(filesTree);
+
+        defaultStore = {
+            auth: {
+                config: {
+                    keepWebServiceUrl: "https://download.host:12345/",
+                    keepWebInlineServiceUrl: insecureKeepInlineUrl,
+                    clusterConfig: {
+                        Collections: {
+                          TrustAllContent: false
+                        }
+                    }
+                }
+            },
+            contextMenu: {
+                resource: {
+                    uuid: "000",
+                    menuKind: ContextMenuKind.COLLECTION_FILE_ITEM,
+                }
+            },
+            collectionPanel: {
+                item: {
+                    uuid: ""
+                }
+            },
+            collectionPanelFiles: filesTree
+        };
+    });
+
+    it('should hide open in new tab when unsafe', () => {
+        // given
+        const store = mockStore(defaultStore);
+
+        // when
+        const wrapper = mount(<Provider store={store}>
+            <CollectionFileViewerAction />
+        </Provider>);
+
+        // then
+        expect(wrapper).not.toBeUndefined();
+
+        // and
+        expect(wrapper.find("a")).toHaveLength(0);
+    });
+
+    it('should show open in new tab when TrustAllContent=true', () => {
+        // given
+        let initialState = defaultStore;
+        initialState.auth.config.clusterConfig.Collections.TrustAllContent = true;
+        const store = mockStore(initialState);
+
+        // when
+        const wrapper = mount(<Provider store={store}>
+            <CollectionFileViewerAction />
+        </Provider>);
+
+        // then
+        expect(wrapper).not.toBeUndefined();
+
+        // and
+        expect(wrapper.find("a").prop("href"))
+            .toEqual(sanitizeToken(getInlineFileUrl(fileUrl,
+                initialState.auth.config.keepWebServiceUrl,
+                initialState.auth.config.keepWebInlineServiceUrl))
+            );
+    });
+
+    it('should show open in new tab when inline url is secure', () => {
+        // given
+        let initialState = defaultStore;
+        initialState.auth.config.keepWebInlineServiceUrl = secureKeepInlineUrl;
+        const store = mockStore(initialState);
+
+        // when
+        const wrapper = mount(<Provider store={store}>
+            <CollectionFileViewerAction />
+        </Provider>);
+
+        // then
+        expect(wrapper).not.toBeUndefined();
+
+        // and
+        expect(wrapper.find("a").prop("href"))
+            .toEqual(sanitizeToken(getInlineFileUrl(fileUrl,
+                initialState.auth.config.keepWebServiceUrl,
+                initialState.auth.config.keepWebInlineServiceUrl))
+            );
+    });
+});
index 27a6501876d7185731847005cd1beb6fb888bfd3..f736f0bf2705bf8d3481eb974cfdb93a3a1cc23f 100644 (file)
@@ -7,7 +7,7 @@ import { RootState } from "../../../store/store";
 import { FileViewerAction } from 'views-components/context-menu/actions/file-viewer-action';
 import { getNodeValue } from "models/tree";
 import { ContextMenuKind } from 'views-components/context-menu/context-menu';
-import { getInlineFileUrl, sanitizeToken } from "./helpers";
+import { getInlineFileUrl, sanitizeToken, isInlineFileUrlSafe } from "./helpers";
 
 const mapStateToProps = (state: RootState) => {
     const { resource } = state.contextMenu;
@@ -18,7 +18,12 @@ const mapStateToProps = (state: RootState) => {
         ContextMenuKind.COLLECTION_DIRECTORY_ITEM,
         ContextMenuKind.READONLY_COLLECTION_DIRECTORY_ITEM ].indexOf(resource.menuKind as ContextMenuKind) > -1) {
         const file = getNodeValue(resource.uuid)(state.collectionPanelFiles);
-        if (file) {
+        const shouldShowInlineUrl = isInlineFileUrlSafe(
+                                file ? file.url : "",
+                                state.auth.config.keepWebServiceUrl,
+                                state.auth.config.keepWebInlineServiceUrl
+                              ) || state.auth.config.clusterConfig.Collections.TrustAllContent;
+        if (file && shouldShowInlineUrl) {
             const fileUrl = sanitizeToken(getInlineFileUrl(
                 file.url,
                 state.auth.config.keepWebServiceUrl,
index dfa8d04f1d83124b27283efa857403936329bd83..159b1c189737be75fd5c942792513c31b1f54204 100644 (file)
@@ -43,4 +43,11 @@ export const getInlineFileUrl = (url: string, keepWebSvcUrl: string, keepWebInli
         inlineUrl = inlineUrl.replace(`/c=${collMatch[1]}`, '');
     }
     return inlineUrl;
-};
\ No newline at end of file
+};
+
+export const isInlineFileUrlSafe = (url: string, keepWebSvcUrl: string, keepWebInlineSvcUrl: string): boolean => {
+  let inlineUrl = keepWebInlineSvcUrl !== ""
+      ? url.replace(keepWebSvcUrl, keepWebInlineSvcUrl)
+      : url;
+  return inlineUrl.indexOf('*.') > -1;
+}
index 3905427b2fbf3a5b5f9386fbb274f1f74ed270eb..dcd2ee4826fa74ca26cd9ee0f05755019a6d3a99 100644 (file)
@@ -42,8 +42,8 @@ export class CollectionDetails extends DetailsData<CollectionResource> {
         return ['Details', 'Versions'];
     }
 
-    getDetails(tabNumber: number) {
-        switch (tabNumber) {
+    getDetails({tabNr}) {
+        switch (tabNr) {
             case 0:
                 return this.getCollectionInfo();
             case 1:
index 0fae2ac48a9f9421c953f6538657438d6284b574..bcca325c01f2a97ce93044b335b23765c51b9282 100644 (file)
@@ -5,6 +5,11 @@
 import React from 'react';
 import { DetailsResource } from "models/details";
 
+interface GetDetailsParams {
+  tabNr?: number
+  showPreview?: boolean
+}
+
 export abstract class DetailsData<T extends DetailsResource = DetailsResource> {
     constructor(protected item: T) { }
 
@@ -17,5 +22,5 @@ export abstract class DetailsData<T extends DetailsResource = DetailsResource> {
     }
 
     abstract getIcon(className?: string): React.ReactElement<any>;
-    abstract getDetails(tabNr?: number): React.ReactElement<any>;
+    abstract getDetails({tabNr, showPreview}: GetDetailsParams): React.ReactElement<any>;
 }
index 38ac163efa7a416d7c8167b218221d0482a40485..058db81b89ce71e4a907d45d3590c14039e31305 100644 (file)
@@ -20,6 +20,8 @@ import { ProcessDetails } from "./process-details";
 import { EmptyDetails } from "./empty-details";
 import { DetailsData } from "./details-data";
 import { DetailsResource } from "models/details";
+import { Config } from 'common/config';
+import { isInlineFileUrlSafe } from "../context-menu/actions/helpers";
 import { getResource } from 'store/resources/resources';
 import { toggleDetailsPanel, SLIDE_TIMEOUT, openDetailsPanel } from 'store/details-panel/details-panel-action';
 import { FileDetails } from 'views-components/details-panel/file-details';
@@ -77,12 +79,13 @@ const getItem = (res: DetailsResource): DetailsData => {
     }
 };
 
-const mapStateToProps = ({ detailsPanel, resources, collectionPanelFiles }: RootState) => {
+const mapStateToProps = ({ auth, detailsPanel, resources, collectionPanelFiles }: RootState) => {
     const resource = getResource(detailsPanel.resourceUuid)(resources) as DetailsResource | undefined;
     const file = resource
         ? undefined
         : getNode(detailsPanel.resourceUuid)(collectionPanelFiles);
     return {
+        authConfig: auth.config,
         isOpened: detailsPanel.isOpened,
         tabNr: detailsPanel.tabNr,
         res: resource || (file && file.value) || EMPTY_RESOURCE,
@@ -101,6 +104,7 @@ const mapDispatchToProps = (dispatch: Dispatch) => ({
 export interface DetailsPanelDataProps {
     onCloseDrawer: () => void;
     setActiveTab: (tabNr: number) => void;
+    authConfig: Config;
     isOpened: boolean;
     tabNr: number;
     res: DetailsResource;
@@ -143,7 +147,17 @@ export const DetailsPanel = withStyles(styles)(
             }
 
             renderContent() {
-                const { classes, onCloseDrawer, res, tabNr } = this.props;
+                const { classes, onCloseDrawer, res, tabNr, authConfig } = this.props;
+
+                let shouldShowInlinePreview = false;
+                if (!('kind' in res)) {
+                    shouldShowInlinePreview = isInlineFileUrlSafe(
+                      res ? res.url : "",
+                      authConfig.keepWebServiceUrl,
+                      authConfig.keepWebInlineServiceUrl
+                    ) || authConfig.clusterConfig.Collections.TrustAllContent;
+                }
+
                 const item = getItem(res);
                 return <Grid
                     container
@@ -183,7 +197,7 @@ export const DetailsPanel = withStyles(styles)(
                         </Tabs>
                     </Grid>
                     <Grid item xs className={this.props.classes.tabContainer} >
-                        {item.getDetails(tabNr)}
+                        {item.getDetails({tabNr, showPreview: shouldShowInlinePreview})}
                     </Grid>
                 </Grid >;
             }
index 7c11eb8baa263a80b3890428c42995bb9390e5e8..7b128c2cbd97e7ee739cc2f99825e5716fdfb412 100644 (file)
@@ -18,13 +18,13 @@ export class FileDetails extends DetailsData<CollectionFile | CollectionDirector
         return <Icon className={className} />;
     }
 
-    getDetails() {
+    getDetails({showPreview}) {
         const { item } = this;
         return item.type === CollectionFileType.FILE
             ? <>
                 <DetailsAttribute label='Size' value={formatFileSize(item.size)} />
                 {
-                    isImage(item.url) && <>
+                    isImage(item.url) && showPreview && <>
                         <DetailsAttribute label='Preview' />
                         <FileThumbnail file={item} />
                     </>
index 3fcd61194b3c2635287ebc0fe07bb0754dddc773..369046e68c0e2ff04a458c992faa472bab88b821 100644 (file)
@@ -10,7 +10,7 @@ Clusters:
       CollectionVersioning: true
       PreserveVersionIfIdle: -1s
       BlobSigningKey: zfhgfenhffzltr9dixws36j1yhksjoll2grmku38mi7yxd66h5j4q9w4jzanezacp8s6q0ro3hxakfye02152hncy6zml2ed0uc
-      TrustAllContent: true
+      TrustAllContent: false
       ForwardSlashNameSubstitution: /
       ManagedProperties:
         original_owner_uuid: {Function: original_owner, Protected: true}