[Workspace]Fix page crash caused by invalid workspace color (#7671)

* Add validation for workspace color Signed-off-by: Lin Wang <wonglam@amazon.com> * Changeset file for PR #7671 created/updated --------- Signed-off-by: Lin Wang <wonglam@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
opensearch-project · Aug 12, 2024 · 248c8ff · 248c8ff
1 parent 1bf63e3
commit 248c8ff
Show file tree

Hide file tree

Showing 10 changed files with 151 additions and 5 deletions.
diff --git a/changelogs/fragments/7671.yml b/changelogs/fragments/7671.yml
@@ -0,0 +1,2 @@
+fix:
+- [Workspace]Fix page crash caused by invalid workspace color ([#7671](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7671))
diff --git a/src/plugins/workspace/common/__tests__/utils.test.ts b/src/plugins/workspace/common/__tests__/utils.test.ts
@@ -0,0 +1,38 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { validateWorkspaceColor } from '../utils';
+
+describe('validateWorkspaceColor', () => {
+  it('should return true for a valid 6-digit hex color code', () => {
+    expect(validateWorkspaceColor('#ABCDEF')).toBe(true);
+    expect(validateWorkspaceColor('#123456')).toBe(true);
+  });
+
+  it('should return true for a valid 3-digit hex color code', () => {
+    expect(validateWorkspaceColor('#ABC')).toBe(true);
+    expect(validateWorkspaceColor('#DEF')).toBe(true);
+  });
+
+  it('should return false for an invalid color code', () => {
+    expect(validateWorkspaceColor('#GHI')).toBe(false);
+    expect(validateWorkspaceColor('#12345')).toBe(false);
+    expect(validateWorkspaceColor('#ABCDEFG')).toBe(false);
+    expect(validateWorkspaceColor('ABCDEF')).toBe(false);
+  });
+
+  it('should return false for an empty string', () => {
+    expect(validateWorkspaceColor('')).toBe(false);
+  });
+
+  it('should return false for undefined', () => {
+    expect(validateWorkspaceColor()).toBe(false);
+  });
+
+  it('should be case-insensitive', () => {
+    expect(validateWorkspaceColor('#abcdef')).toBe(true);
+    expect(validateWorkspaceColor('#ABC')).toBe(true);
+  });
+});
diff --git a/src/plugins/workspace/common/utils.ts b/src/plugins/workspace/common/utils.ts
@@ -0,0 +1,8 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+// Reference https://github.com/opensearch-project/oui/blob/main/src/services/color/is_valid_hex.ts
+export const validateWorkspaceColor = (color?: string) =>
+  !!color && /(^#[0-9A-F]{6}$)|(^#[0-9A-F]{3}$)/i.test(color);
diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts
@@ -55,6 +55,7 @@ export enum WorkspaceFormErrorCode {
   PermissionSettingOwnerMissing,
   InvalidDataSource,
   DuplicateDataSource,
+  InvalidColor,
 }
 
 export interface WorkspaceFormError {

diff --git a/src/plugins/workspace/public/components/workspace_form/utils.test.ts b/src/plugins/workspace/public/components/workspace_form/utils.test.ts
@@ -179,6 +179,12 @@ describe('validateWorkspaceForm', () => {
       message: 'Name is invalid. Enter a valid name.',
     });
   });
+  it('should return error if color is invalid', () => {
+    expect(validateWorkspaceForm({ color: 'QWERTY' }, false).color).toEqual({
+      code: WorkspaceFormErrorCode.InvalidColor,
+      message: 'Color is invalid. Enter a valid color.',
+    });
+  });
   it('should return error if use case is empty', () => {
     expect(validateWorkspaceForm({}, false).features).toEqual({
       code: WorkspaceFormErrorCode.UseCaseMissing,
@@ -393,6 +399,18 @@ describe('getNumberOfErrors', () => {
       })
     ).toEqual(1);
   });
+
+  it('should return consistent color errors count', () => {
+    expect(
+      getNumberOfErrors({
+        name: {
+          code: WorkspaceFormErrorCode.InvalidColor,
+          message: '',
+        },
+      })
+    ).toEqual(1);
+  });
+
   it('should return consistent permission settings errors count', () => {
     expect(
       getNumberOfErrors({
@@ -461,6 +479,32 @@ describe('getNumberOfChanges', () => {
       )
     ).toEqual(1);
   });
+  it('should return consistent color changes count', () => {
+    expect(
+      getNumberOfChanges(
+        {
+          name: 'foo',
+          color: '#000',
+        },
+        {
+          name: 'foo',
+          color: '#000',
+        }
+      )
+    ).toEqual(0);
+    expect(
+      getNumberOfChanges(
+        {
+          name: 'foo',
+          color: '#000',
+        },
+        {
+          name: 'foo',
+          color: '#001',
+        }
+      )
+    ).toEqual(1);
+  });
   it('should return consistent features changes count', () => {
     expect(
       getNumberOfChanges(

diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts
@@ -30,6 +30,7 @@ import {
   WorkspaceUserPermissionSetting,
 } from './types';
 import { DataSource } from '../../../common/types';
+import { validateWorkspaceColor } from '../../../common/utils';
 
 export const appendDefaultFeatureIds = (ids: string[]) => {
   // concat default checked ids and unique the result
@@ -62,6 +63,9 @@ export const getNumberOfErrors = (formErrors: WorkspaceFormErrors) => {
   if (formErrors.features) {
     numberOfErrors += 1;
   }
+  if (formErrors.color) {
+    numberOfErrors += 1;
+  }
   return numberOfErrors;
 };
 
@@ -308,7 +312,7 @@ export const validateWorkspaceForm = (
   isPermissionEnabled: boolean
 ) => {
   const formErrors: WorkspaceFormErrors = {};
-  const { name, permissionSettings, features, selectedDataSources } = formData;
+  const { name, permissionSettings, color, features, selectedDataSources } = formData;
   if (name && name.trim()) {
     if (!isValidFormTextInput(name)) {
       formErrors.name = {
@@ -334,6 +338,14 @@ export const validateWorkspaceForm = (
       }),
     };
   }
+  if (color && !validateWorkspaceColor(color)) {
+    formErrors.color = {
+      code: WorkspaceFormErrorCode.InvalidColor,
+      message: i18n.translate('workspace.form.features.empty', {
+        defaultMessage: 'Color is invalid. Enter a valid color.',
+      }),
+    };
+  }
   if (isPermissionEnabled) {
     formErrors.permissionSettings = validatePermissionSetting(permissionSettings);
   }
@@ -463,6 +475,9 @@ export const getNumberOfChanges = (
   if (newFormData.description !== initialFormData.description) {
     count++;
   }
+  if (newFormData.color !== initialFormData.color) {
+    count++;
+  }
   if (
     newFormData.features?.length !== initialFormData.features?.length ||
     newFormData.features?.some((item) => !initialFormData.features?.includes(item))

diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.test.tsx
@@ -48,6 +48,19 @@ describe('WorkspaceFormErrorCallout', () => {
     expect(renderResult.getByText('Name: Enter a valid name.')).toBeInTheDocument();
   });
 
+  it('should render color suggestion', () => {
+    const { renderResult } = setup({
+      errors: {
+        color: {
+          code: WorkspaceFormErrorCode.InvalidColor,
+          message: '',
+        },
+      },
+    });
+
+    expect(renderResult.getByText('Color: Enter a valid color.')).toBeInTheDocument();
+  });
+
   it('should render use case suggestion', () => {
     const { renderResult } = setup({
       errors: {

diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form_error_callout.tsx
@@ -42,6 +42,10 @@ const getSuggestionFromErrorCode = (error: WorkspaceFormError) => {
       return i18n.translate('workspace.form.errorCallout.permissionSettingOwnerMissing', {
         defaultMessage: 'Add a workspace owner.',
       });
+    case WorkspaceFormErrorCode.InvalidColor:
+      return i18n.translate('workspace.form.errorCallout.invalidColor', {
+        defaultMessage: 'Enter a valid color.',
+      });
     default:
       return error.message;
   }
@@ -106,6 +110,14 @@ export const WorkspaceFormErrorCallout = ({ errors }: WorkspaceFormErrorCalloutP
               message={getSuggestionFromErrorCode(errors.name)}
             />
           )}
+          {errors.color && (
+            <WorkspaceFormErrorCalloutItem
+              errorKey={i18n.translate('workspace.form.errorCallout.nameKey', {
+                defaultMessage: 'Color:',
+              })}
+              message={getSuggestionFromErrorCode(errors.color)}
+            />
+          )}
           {errors.features && (
             <WorkspaceFormErrorCalloutItem
               errorKey={i18n.translate('workspace.form.errorCallout.useCaseKey', {

diff --git a/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx b/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx
@@ -34,6 +34,7 @@ import { getFirstUseCaseOfFeatureConfigs } from '../../utils';
 import { recentWorkspaceManager } from '../../recent_workspace_manager';
 import { WorkspaceUseCase } from '../../types';
 import { navigateToWorkspaceDetail } from '../utils/workspace';
+import { validateWorkspaceColor } from '../../../common/utils';
 
 const defaultHeaderName = i18n.translate('workspace.menu.defaultHeaderName', {
   defaultMessage: 'Workspaces',
@@ -63,6 +64,9 @@ const manageWorkspacesButton = i18n.translate('workspace.menu.button.manageWorks
   defaultMessage: 'Manage workspaces',
 });
 
+const getValidWorkspaceColor = (color?: string) =>
+  validateWorkspaceColor(color) ? color : undefined;
+
 interface Props {
   coreStart: CoreStart;
   registeredUseCases$: BehaviorSubject<WorkspaceUseCase[]>;
@@ -111,7 +115,7 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => {
         size="s"
         type="space"
         name={currentWorkspace.name}
-        color={currentWorkspace.color}
+        color={getValidWorkspaceColor(currentWorkspace.color)}
         initialsLength={2}
       />
     </EuiButtonEmpty>
@@ -148,7 +152,7 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => {
               size="s"
               type="space"
               name={workspace.name}
-              color={workspace.color}
+              color={getValidWorkspaceColor(workspace.color)}
               initialsLength={2}
             />
           }
@@ -196,7 +200,7 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => {
                   size="m"
                   type="space"
                   name={currentWorkspaceName}
-                  color={currentWorkspace?.color}
+                  color={getValidWorkspaceColor(currentWorkspace?.color)}
                   initialsLength={2}
                 />
               </EuiFlexItem>

diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts
@@ -10,6 +10,7 @@ import { IWorkspaceClientImpl, WorkspaceAttributeWithPermission } from '../types
 import { SavedObjectsPermissionControlContract } from '../permission_control/client';
 import { registerDuplicateRoute } from './duplicate';
 import { transferCurrentUserInPermissions } from '../utils';
+import { validateWorkspaceColor } from '../../common/utils';
 
 export const WORKSPACES_API_BASE_URL = '/api/workspaces';
 
@@ -40,7 +41,15 @@ const settingsSchema = schema.object({
 const workspaceOptionalAttributesSchema = {
   description: schema.maybe(schema.string()),
   features: schema.maybe(schema.arrayOf(schema.string())),
-  color: schema.maybe(schema.string()),
+  color: schema.maybe(
+    schema.string({
+      validate: (color) => {
+        if (!validateWorkspaceColor(color)) {
+          return 'invalid workspace color format';
+        }
+      },
+    })
+  ),
   icon: schema.maybe(schema.string()),
   defaultVISTheme: schema.maybe(schema.string()),
   reserved: schema.maybe(schema.boolean()),