[ENG-3882] Prevent special chars and show error messages (#1569)

futa-ikeda · web-flow · commit e93ea4546bb4 · 2022-07-12T11:48:45.000-04:00
-   Ticket: [ENG-3882]
-   Feature flag: n/a

## Purpose
- Disallow special characters in new folders and file/folder rename modals

## Summary of Changes
- Add logic to check against forbidden special chars for file rename and new folder modal
- Add error messages to those modals
diff --git a/app/models/file.ts b/app/models/file.ts
@@ -22,6 +22,9 @@ export interface FileLinks extends BaseFileLinks {
     render?: Link;
 }
 
+// Character that need to be excluded: ()<>~!@$&*:;,'"\|/?
+export const forbiddenFileNameCharacters = /[()<>~!@$&*:;,"\\|/?]/;
+
 export default class FileModel extends BaseFileItem {
     @attr() links!: FileLinks;
     @attr('fixstring') name!: string;
diff --git a/lib/osf-components/addon/components/file-browser/add-new/create-folder-modal/component.ts b/lib/osf-components/addon/components/file-browser/add-new/create-folder-modal/component.ts
@@ -1,7 +1,9 @@
+import { inject as service } from '@ember/service';
+import Intl from 'ember-intl/services/intl';
 import Component from '@glimmer/component';
 import { tracked } from '@glimmer/tracking';
-import { taskFor } from 'ember-concurrency-ts';
 
+import { forbiddenFileNameCharacters } from 'ember-osf-web/models/file';
 import StorageManager from 'osf-components/components/storage-provider-manager/storage-manager/component';
 
 interface Args {
@@ -10,9 +12,31 @@ interface Args {
 }
 
 export default class CreateFolderModal extends Component<Args> {
+    @service intl!: Intl;
     @tracked newFolderName = '';
 
-    get shouldDisable() {
-        return taskFor(this.args.manager.createNewFolder).isRunning || !this.newFolderName;
+    get isInvalid() {
+        const trimmedName = this.newFolderName.trim();
+        return !trimmedName || this.containsForbiddenChars || this.endsWithDot;
+    }
+
+    get containsForbiddenChars() {
+        return this.newFolderName && forbiddenFileNameCharacters.test(this.newFolderName);
+    }
+
+    get endsWithDot() {
+        return this.newFolderName && this.newFolderName.endsWith('.');
+    }
+
+    get errorText() {
+        let errorTextKey = 'osf-components.file-browser.create_folder.error_';
+        if (this.containsForbiddenChars) {
+            errorTextKey += 'forbidden_chars';
+        } else if (this.endsWithDot) {
+            errorTextKey += 'ends_with_dot';
+        } else {
+            errorTextKey += 'message';
+        }
+        return this.intl.t(errorTextKey);
     }
 }
diff --git a/lib/osf-components/addon/components/file-browser/add-new/create-folder-modal/styles.scss b/lib/osf-components/addon/components/file-browser/add-new/create-folder-modal/styles.scss
@@ -0,0 +1,10 @@
+.RenameInput {
+    min-width: 50vw;
+    width: 100%;
+}
+
+.ErrorText {
+    color: $color-text-gray;
+    font-style: italic;
+    font-weight: 400;
+}
diff --git a/lib/osf-components/addon/components/file-browser/add-new/create-folder-modal/template.hbs b/lib/osf-components/addon/components/file-browser/add-new/create-folder-modal/template.hbs
@@ -3,10 +3,10 @@
     @onClose={{queue (action (mut @isOpen) false) (action (mut this.newFolderName) '')}}
     as |dialog|
 >
-    <dialog.heading>
+    <dialog.heading data-test-create-folder-heading>
         {{t 'osf-components.file-browser.create_folder.title'}}
     </dialog.heading>
-    <dialog.main>
+    <dialog.main data-test-create-folder-main>
         <label>
             <p>{{t 'osf-components.file-browser.create_folder.new_folder_name'}}</p>
             <Input
@@ -16,13 +16,19 @@
                     (action (mut this.newFolderName) '')
                 }}
                 @value={{this.newFolderName}}
+                local-class='RenameInput'
             />
+            {{#if this.isInvalid}}
+                <p local-class='ErrorText' data-test-new-folder-error>{{this.errorText}}</p>
+            {{/if}}
         </label>
     </dialog.main>
     <dialog.footer>
         <Button
+            data-test-create-folder-button
+            data-analytics-name='Create folder'
             @type='create'
-            disabled={{this.shouldDisable}}
+            disabled={{or @manager.createNewFolder.isRunning this.isInvalid}}
             {{on 'click'
                 (queue
                     (perform @manager.createNewFolder this.newFolderName)
diff --git a/lib/osf-components/addon/components/file-browser/add-new/template.hbs b/lib/osf-components/addon/components/file-browser/add-new/template.hbs
@@ -6,6 +6,7 @@
         as |dropdown|
     >
         <dropdown.trigger
+            data-analytics-name='Add file or folder omnibutton'
             data-test-add-new-trigger
             aria-label={{t 'osf-components.file-browser.add_button_aria'}}
             local-class='TriggerButton {{if dropdown.isOpen 'CloseButton'}}'>
@@ -18,12 +19,16 @@
             {{will-destroy (fn @setClickableElementId '')}}
         >
             <Button
+                data-analytics-name='Open create folder modal'
+                data-test-create-folder
                 @layout='fake-link'
                 {{on 'click' (fn (mut this.createFolderModalOpen) true)}}
             >
                 {{t 'osf-components.file-browser.create_folder.title'}}
             </Button>
             <Button
+                data-analytics-name='Upload file'
+                data-test-upload-file
                 @layout='fake-link'
                 id={{uploadButtonId}}
             >
diff --git a/lib/osf-components/addon/components/file-browser/file-rename-modal/component.ts b/lib/osf-components/addon/components/file-browser/file-rename-modal/component.ts
@@ -6,6 +6,8 @@ import Toast from 'ember-toastr/services/toast';
 import { restartableTask } from 'ember-concurrency';
 import { waitFor } from '@ember/test-waiters';
 import Intl from 'ember-intl/services/intl';
+
+import { forbiddenFileNameCharacters } from 'ember-osf-web/models/file';
 import File from 'ember-osf-web/packages/files/file';
 import StorageManager from 'osf-components/components/storage-provider-manager/storage-manager/component';
 
@@ -28,12 +30,34 @@ export default class FileRenameModal extends Component<Args> {
     }
 
     get isValid() {
-        if(this.newFileName) {
+        if(this.newFileName && !this.containsForbiddenChars && !this.endsWithDot) {
             return (this.newFileName.trim() !== this.originalFileName && this.newFileName.trim() !== '');
         }
         return false;
     }
 
+    get containsForbiddenChars() {
+        return this.newFileName && forbiddenFileNameCharacters.test(this.newFileName);
+    }
+
+    get endsWithDot() {
+        return this.newFileName && this.newFileName.endsWith('.');
+    }
+
+    get errorText() {
+        let errorTextKey = 'osf-components.file-browser.file_rename_modal.error_';
+        if (!this.newFileName) {
+            errorTextKey += 'empty_name';
+        } else if (this.endsWithDot) {
+            errorTextKey += 'ends_with_dot';
+        } else if (this.containsForbiddenChars) {
+            errorTextKey += 'forbidden_chars';
+        } else {
+            errorTextKey += 'message';
+        }
+        return this.intl.t(errorTextKey);
+    }
+
     @action
     resetFileNameValue() {
         this.newFileName = this.originalFileName;
diff --git a/lib/osf-components/addon/components/file-browser/file-rename-modal/styles.scss b/lib/osf-components/addon/components/file-browser/file-rename-modal/styles.scss
@@ -20,4 +20,7 @@
     }
 }
 
-
+.ErrorText {
+    color: $color-text-gray;
+    font-style: italic;
+}
diff --git a/lib/osf-components/addon/components/file-browser/file-rename-modal/template.hbs b/lib/osf-components/addon/components/file-browser/file-rename-modal/template.hbs
@@ -9,7 +9,7 @@
         {{t 'osf-components.file-browser.file_rename_modal.heading'}}
     </dialog.heading>
     <dialog.main>
-        <div local-class='RenameInput'>
+        <div data-test-rename-main local-class='RenameInput'>
             <Input
                 data-test-user-input
                 aria-label={{t 'osf-components.file-browser.file_rename_modal.input_aria'}}
@@ -20,6 +20,11 @@
                     dialog.close
                 }}
             />
+            {{#unless this.isValid}}
+                <p local-class='ErrorText'>
+                    {{this.errorText}}
+                </p>
+            {{/unless}}
         </div>
     </dialog.main>
     <dialog.footer>
diff --git a/mirage/serializers/file-provider.ts b/mirage/serializers/file-provider.ts
@@ -62,6 +62,7 @@ export default class FileSerializer extends ApplicationSerializer<MirageFileProv
         return {
             ...super.buildNormalLinks(model),
             upload: `${apiUrl}/v2/${pathName}/${model.targetId.id}/files/${model.name}/upload`,
+            new_folder: `${apiUrl}/v2/${pathName}/${model.targetId.id}/files/${model.name}/upload/?kind=folder`,
         };
     }
 }
diff --git a/mirage/serializers/file.ts b/mirage/serializers/file.ts
@@ -68,6 +68,7 @@ export default class FileSerializer extends ApplicationSerializer<MirageFile> {
         const { id } = model;
         return {
             ...super.buildNormalLinks(model),
+            new_folder: model.kind === 'folder' ? `${apiUrl}/wb/files/${id}/upload/?kind=folder` : undefined,
             upload: `${apiUrl}/wb/files/${id}/upload/`,
             download: `${apiUrl}/wb/files/${id}/download/`,
             move: `${apiUrl}/wb/files/${id}/move/`,
diff --git a/mirage/views/file.ts b/mirage/views/file.ts
@@ -1,6 +1,7 @@
 import { HandlerContext, ModelInstance, Response, Schema } from 'ember-cli-mirage';
 import { MirageNode } from 'ember-osf-web/mirage/factories/node';
 import DraftNode from 'ember-osf-web/models/draft-node';
+import { FileItemKinds } from 'ember-osf-web/models/base-file-item';
 import faker from 'faker';
 
 import { guid } from '../factories/utils';
@@ -9,7 +10,7 @@ import { filter, process } from './utils';
 export function uploadToFolder(this: HandlerContext, schema: Schema) {
     const uploadAttrs = this.request.requestBody;
     const { id: folderId } = this.request.params;
-    const { name } = this.request.queryParams;
+    const { name, kind } = this.request.queryParams;
     const folder = schema.files.find(folderId);
 
     const randomNum = faker.random.number();
@@ -23,13 +24,17 @@ export function uploadToFolder(this: HandlerContext, schema: Schema) {
     const uploadedFile = schema.files.create({
         guid: id,
         id,
-        size: uploadAttrs.size,
-        dateModified: uploadAttrs.lastModified,
-        lastTouched: uploadAttrs.lastModifiedDate,
+        path: id,
         target: folder.target,
         name,
+        kind: kind as FileItemKinds,
         provider: 'osfstorage',
     });
+    if (kind === FileItemKinds.File) {
+        uploadedFile.dateModified = uploadAttrs.lastModified;
+        uploadedFile.lastTouched = uploadAttrs.lastModifiedDate;
+        uploadedFile.size = uploadAttrs.size;
+    }
 
     folder.files.models.pushObject(uploadedFile);
     folder.save();
@@ -40,7 +45,7 @@ export function uploadToFolder(this: HandlerContext, schema: Schema) {
 export function uploadToRoot(this: HandlerContext, schema: Schema) {
     const uploadAttrs = this.request.requestBody;
     const { parentID, fileProviderId } = this.request.params;
-    const { name } = this.request.queryParams;
+    const { name, kind } = this.request.queryParams;
     let node;
     if (this.request.url.includes('draft_nodes')) {
         node = schema.draftNodes.find(parentID);
@@ -66,13 +71,17 @@ export function uploadToRoot(this: HandlerContext, schema: Schema) {
     const uploadedFile = schema.files.create({
         guid: id,
         id,
-        size: uploadAttrs.size,
-        dateModified: uploadAttrs.lastModified,
-        lastTouched: uploadAttrs.lastModifiedDate,
+        path: id,
         target: node,
         name,
+        kind: kind as FileItemKinds,
         provider: 'osfstorage',
     });
+    if (kind === FileItemKinds.File) {
+        uploadedFile.dateModified = uploadAttrs.lastModified;
+        uploadedFile.lastTouched = uploadAttrs.lastModifiedDate;
+        uploadedFile.size = uploadAttrs.size;
+    }
 
     rootFolder.files.models.pushObject(uploadedFile);
     rootFolder.save();
diff --git a/tests/integration/components/file-browser/component-test.ts b/tests/integration/components/file-browser/component-test.ts
@@ -1,4 +1,4 @@
-import { render } from '@ember/test-helpers';
+import { fillIn, render } from '@ember/test-helpers';
 import { hbs } from 'ember-cli-htmlbars';
 import { ModelInstance } from 'ember-cli-mirage';
 import { TestContext, t } from 'ember-intl/test-support';
@@ -191,4 +191,58 @@ module('Integration | Component | file-browser', hooks => {
                 'Registration help guide',
             );
         });
+
+    test('it creates new folder',
+        async function(this: FileBrowserTestContext, assert) {
+            const node = await this.store.findRecord('node', this.mirageNode.id);
+            const storageProviders = await node.files;
+            this.osfStorageProvider = storageProviders.toArray()[0];
+            await render(hbs`
+                <StorageProviderManager::StorageManager @provider={{this.osfStorageProvider}} as |manager|>
+                    <FileBrowser @manager={{manager}} @enableUpload={{true}} @selectable={{true}} />
+                </StorageProviderManager::StorageManager>
+            `);
+            await click('[data-test-add-new-trigger]');
+            assert.dom('[data-test-upload-file]').exists('Upload file option shown');
+            assert.dom('[data-test-create-folder]').exists('Create folder option shown');
+            await click('[data-test-create-folder]');
+            assert.dom('[data-test-create-folder-heading]').containsText(
+                t('osf-components.file-browser.create_folder.title'), 'Create folder modal shown',
+            );
+            assert.dom('[data-test-create-folder-main] input').exists('Name input shown');
+            assert.dom('[data-test-new-folder-error]').containsText(
+                t('osf-components.file-browser.create_folder.error_message'), 'Message shown to enter folder name',
+            );
+            assert.dom('[data-test-create-folder-button]').isDisabled('Create folder button is disabled');
+
+            await fillIn('[data-test-create-folder-main] input', '     ');
+            assert.dom('[data-test-new-folder-error]').containsText(
+                t('osf-components.file-browser.create_folder.error_message'), 'Folder cannot be empty',
+            );
+            assert.dom('[data-test-create-folder-button]').isDisabled('Create folder button is still still disabled');
+
+            await fillIn('[data-test-create-folder-main] input', 'new fo/der?');
+            assert.dom('[data-test-new-folder-error]').containsText(
+                t('osf-components.file-browser.create_folder.error_forbidden_chars'),
+                'Folder cannot have special chars',
+            );
+            assert.dom('[data-test-create-folder-button]').isDisabled('Create folder button is still x3 disabled');
+
+            await fillIn('[data-test-create-folder-main] input', 'new folder.');
+            assert.dom('[data-test-new-folder-error]').containsText(
+                t('osf-components.file-browser.create_folder.error_ends_with_dot'), 'Folder name cannot end with a dot',
+            );
+            assert.dom('[data-test-create-folder-button]').isDisabled('Create folder button is still x4 disabled');
+
+            await fillIn('[data-test-create-folder-main] input', 'Shiny New Folder');
+            assert.dom('[data-test-new-folder-error]').doesNotExist('No error message shown');
+            assert.dom('[data-test-create-folder-button]').isEnabled('Create folder button is enabled');
+
+            await click('[data-test-create-folder-button]');
+
+            assert.dom('[data-test-create-folder-heading]').doesNotExist('Create folder modal autocloses');
+            const newFolderAria = t('osf-components.file-browser.view_folder', {folderName: 'Shiny New Folder'});
+            assert.dom(`[data-test-file-list-link][aria-label="${newFolderAria}"]`)
+                .exists('Shiny new folder exists');
+        });
 });
diff --git a/tests/integration/components/file-browser/file-rename-modal/component-test.ts b/tests/integration/components/file-browser/file-rename-modal/component-test.ts
@@ -64,8 +64,26 @@ module('Integration | Component | file-browser :: file-rename-modal', hooks => {
             assert.dom('[data-test-disabled-rename]').hasText(stripHtmlTags(
                 t('osf-components.file-browser.file_rename_modal.save'),
             ));
+            assert.dom('[data-test-rename-main]').containsText(
+                t('osf-components.file-browser.file_rename_modal.error_message'),
+            );
 
             await fillIn('[data-test-user-input]', 'What is the great globe itself but a Loose-Fish?');
+            assert.dom('[data-test-rename-main]').containsText(
+                t('osf-components.file-browser.file_rename_modal.error_forbidden_chars'),
+            );
+
+            await fillIn('[data-test-user-input]', '      ');
+            assert.dom('[data-test-rename-main]').containsText(
+                t('osf-components.file-browser.file_rename_modal.error_message'),
+            );
+
+            await fillIn('[data-test-user-input]', 'This will error.');
+            assert.dom('[data-test-rename-main]').containsText(
+                t('osf-components.file-browser.file_rename_modal.error_ends_with_dot'),
+            );
+
+            await fillIn('[data-test-user-input]', 'Save this file');
             assert.dom('[data-test-disabled-rename]').isEnabled('Save button is enabled');
         });
 });
diff --git a/translations/en-us.yml b/translations/en-us.yml
@@ -1906,6 +1906,9 @@ osf-components:
             error_title: 'Failed to create folder'
             error_409: 'A folder with that name already exists'
             error_generic: 'Something went wrong. Please try again.'
+            error_message: 'Please enter a folder name.'
+            error_ends_with_dot: 'File name cannot end with period'
+            error_forbidden_chars: 'Please remove special characters from the folder name.'
         add_button_aria: 'Add files or folders here'
         upload_file: 'Upload file'
         uploading_file: 'Uploading {fileCount, plural, one {# file} other {# files}}'
@@ -1968,6 +1971,8 @@ osf-components:
             clear_aria: 'Clear input field'
             success_message: 'File successfully renamed.'
             error_message: 'Please rename the file.'
+            error_ends_with_dot: 'File name cannot end with a period.'
+            error_forbidden_chars: 'Please remove special characters from the file name.'
             retry_message: 'Rename failed.'
 
     subjects:

Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,9 @@ export interface FileLinks extends BaseFileLinks {`
`22`	`22`	`render?: Link;`
`23`	`23`	`}`
`24`	`24`
	`25`	`+// Character that need to be excluded: ()<>~!@$&*:;,'"\\|/?`
	`26`	`+export const forbiddenFileNameCharacters = /[()<>~!@$&*:;,"\\\|/?]/;`
	`27`	`+`
`25`	`28`	`export default class FileModel extends BaseFileItem {`
`26`	`29`	`@attr() links!: FileLinks;`
`27`	`30`	`@attr('fixstring') name!: string;`
Original file line number	Diff line number	Diff line change
`@@ -6,6 +6,7 @@`
`6`	`6`	`as \|dropdown\|`
`7`	`7`	`>`
`8`	`8`	`<dropdown.trigger`
	`9`	`+ data-analytics-name='Add file or folder omnibutton'`
`9`	`10`	`data-test-add-new-trigger`
`10`	`11`	`aria-label={{t 'osf-components.file-browser.add_button_aria'}}`
`11`	`12`	`local-class='TriggerButton {{if dropdown.isOpen 'CloseButton'}}'>`
`@@ -18,12 +19,16 @@`
`18`	`19`	`{{will-destroy (fn @setClickableElementId '')}}`
`19`	`20`	`>`
`20`	`21`	`<Button`
	`22`	`+ data-analytics-name='Open create folder modal'`
	`23`	`+ data-test-create-folder`
`21`	`24`	`@layout='fake-link'`
`22`	`25`	`{{on 'click' (fn (mut this.createFolderModalOpen) true)}}`
`23`	`26`	`>`
`24`	`27`	`{{t 'osf-components.file-browser.create_folder.title'}}`
`25`	`28`	`</Button>`
`26`	`29`	`<Button`
	`30`	`+ data-analytics-name='Upload file'`
	`31`	`+ data-test-upload-file`
`27`	`32`	`@layout='fake-link'`
`28`	`33`	`id={{uploadButtonId}}`
`29`	`34`	`>`
Original file line number	Diff line number	Diff line change
`@@ -20,4 +20,7 @@`
`20`	`20`	`}`
`21`	`21`	`}`
`22`	`22`
`23`		`-`
	`23`	`+.ErrorText {`
	`24`	`+ color: $color-text-gray;`
	`25`	`+ font-style: italic;`
	`26`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -62,6 +62,7 @@ export default class FileSerializer extends ApplicationSerializer<MirageFileProv`
`62`	`62`	`return {`
`63`	`63`	`...super.buildNormalLinks(model),`
`64`	`64`	upload: `${apiUrl}/v2/${pathName}/${model.targetId.id}/files/${model.name}/upload`,
	`65`	+ new_folder: `${apiUrl}/v2/${pathName}/${model.targetId.id}/files/${model.name}/upload/?kind=folder`,
`65`	`66`	`};`
`66`	`67`	`}`
`67`	`68`	`}`