diff --git a/CHANGELOG.md b/CHANGELOG.md index a2c5c0a7b165..c3bcd0c78c45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,6 +75,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [BUG] Remove duplicate sample data as id 90943e30-9a47-11e8-b64d-95841ca0b247 ([5668](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5668)) - [BUG][Multiple Datasource] Fix datasource testing connection unexpectedly passed with wrong endpoint [#5663](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5663) - [Table Visualization] Fix filter action buttons for split table aggregations ([#5619](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5619)) +- [BUG][Discover] Allow saved sort from search embeddable to load in Dashboard ([#5934](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5934)) ### 🚞 Infrastructure diff --git a/package.json b/package.json index cb24289e686f..8bdd92b6371c 100644 --- a/package.json +++ b/package.json @@ -329,6 +329,7 @@ "@types/react-router-dom": "^5.3.2", "@types/react-virtualized": "^9.18.7", "@types/recompose": "^0.30.6", + "@types/redux-mock-store": "^1.0.6", "@types/selenium-webdriver": "^4.0.9", "@types/semver": "^7.5.0", "@types/sinon": "^7.0.13", @@ -446,6 +447,7 @@ "react-test-renderer": "^16.12.0", "reactcss": "1.2.3", "redux": "^4.0.5", + "redux-mock-store": "^1.5.4", "regenerate": "^1.4.0", "reselect": "^4.0.0", "resize-observer-polyfill": "^1.5.1", diff --git a/src/core/public/saved_objects/saved_objects_client.ts b/src/core/public/saved_objects/saved_objects_client.ts index ce3ebc710ed1..d40b91188cfc 100644 --- a/src/core/public/saved_objects/saved_objects_client.ts +++ b/src/core/public/saved_objects/saved_objects_client.ts @@ -38,6 +38,7 @@ import { SavedObjectsClientContract as SavedObjectsApi, SavedObjectsFindOptions as SavedObjectFindOptionsServer, SavedObjectsMigrationVersion, + SavedObjectsBaseOptions, } from '../../server'; import { SimpleSavedObject } from './simple_saved_object'; @@ -65,7 +66,7 @@ export interface SavedObjectsCreateOptions { /** {@inheritDoc SavedObjectsMigrationVersion} */ migrationVersion?: SavedObjectsMigrationVersion; references?: SavedObjectReference[]; - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } /** diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 84ee65dcb199..f497bed22755 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -321,12 +321,11 @@ export { exportSavedObjectsToStream, importSavedObjectsFromStream, resolveSavedObjectsImportErrors, - SavedObjectsDeleteByWorkspaceOptions, ACL, Principals, - TransformedPermission, PrincipalType, Permissions, + SavedObjectsDeleteByWorkspaceOptions, } from './saved_objects'; export { diff --git a/src/core/server/mocks.ts b/src/core/server/mocks.ts index 687d408e40a6..dce39d03da7f 100644 --- a/src/core/server/mocks.ts +++ b/src/core/server/mocks.ts @@ -89,6 +89,9 @@ export function pluginInitializerContextConfigMock(config: T) { path: { data: '/tmp' }, savedObjects: { maxImportPayloadBytes: new ByteSizeValue(26214400), + permission: { + enabled: true, + }, }, }; diff --git a/src/core/server/plugins/plugin_context.test.ts b/src/core/server/plugins/plugin_context.test.ts index 7a8ba042825b..57aa372514de 100644 --- a/src/core/server/plugins/plugin_context.test.ts +++ b/src/core/server/plugins/plugin_context.test.ts @@ -108,7 +108,12 @@ describe('createPluginInitializerContext', () => { pingTimeout: duration(30, 's'), }, path: { data: fromRoot('data') }, - savedObjects: { maxImportPayloadBytes: new ByteSizeValue(26214400) }, + savedObjects: { + maxImportPayloadBytes: new ByteSizeValue(26214400), + permission: { + enabled: false, + }, + }, }); }); diff --git a/src/core/server/plugins/types.ts b/src/core/server/plugins/types.ts index 59b9881279c3..c225a24aa386 100644 --- a/src/core/server/plugins/types.ts +++ b/src/core/server/plugins/types.ts @@ -295,7 +295,7 @@ export const SharedGlobalConfigKeys = { ] as const, opensearch: ['shardTimeout', 'requestTimeout', 'pingTimeout'] as const, path: ['data'] as const, - savedObjects: ['maxImportPayloadBytes'] as const, + savedObjects: ['maxImportPayloadBytes', 'permission'] as const, }; /** diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts index 6c99db6c24af..44595b0c33ff 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts @@ -30,7 +30,7 @@ import Boom from '@hapi/boom'; import { createListStream } from '../../utils/streams'; -import { SavedObjectsClientContract, SavedObject } from '../types'; +import { SavedObjectsClientContract, SavedObject, SavedObjectsBaseOptions } from '../types'; import { fetchNestedDependencies } from './inject_nested_depdendencies'; import { sortObjects } from './sort_objects'; @@ -61,7 +61,7 @@ export interface SavedObjectsExportOptions { /** optional namespace to override the namespace used by the savedObjectsClient. */ namespace?: string; /** optional workspaces to override the workspaces used by the savedObjectsClient. */ - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } /** diff --git a/src/core/server/saved_objects/import/check_conflicts.ts b/src/core/server/saved_objects/import/check_conflicts.ts index d173af45f761..7afab618b1ff 100644 --- a/src/core/server/saved_objects/import/check_conflicts.ts +++ b/src/core/server/saved_objects/import/check_conflicts.ts @@ -35,6 +35,7 @@ import { SavedObjectsImportError, SavedObjectError, SavedObjectsImportRetry, + SavedObjectsBaseOptions, } from '../types'; interface CheckConflictsParams { @@ -44,7 +45,7 @@ interface CheckConflictsParams { ignoreRegularConflicts?: boolean; retries?: SavedObjectsImportRetry[]; createNewCopies?: boolean; - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } const isUnresolvableConflict = (error: SavedObjectError) => diff --git a/src/core/server/saved_objects/import/create_saved_objects.ts b/src/core/server/saved_objects/import/create_saved_objects.ts index f98e2e816b75..35d0ddd349fa 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -28,7 +28,12 @@ * under the License. */ -import { SavedObject, SavedObjectsClientContract, SavedObjectsImportError } from '../types'; +import { + SavedObject, + SavedObjectsBaseOptions, + SavedObjectsClientContract, + SavedObjectsImportError, +} from '../types'; import { extractErrors } from './extract_errors'; import { CreatedObject } from './types'; @@ -41,7 +46,7 @@ interface CreateSavedObjectsParams { overwrite?: boolean; dataSourceId?: string; dataSourceTitle?: string; - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } interface CreateSavedObjectsResult { createdObjects: Array>; diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index 689b8222b510..a243e08f83e0 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -29,7 +29,7 @@ */ import { Readable } from 'stream'; -import { SavedObjectsClientContract, SavedObject } from '../types'; +import { SavedObjectsClientContract, SavedObject, SavedObjectsBaseOptions } from '../types'; import { ISavedObjectTypeRegistry } from '..'; /** @@ -190,7 +190,7 @@ export interface SavedObjectsImportOptions { dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } /** @@ -215,7 +215,7 @@ export interface SavedObjectsResolveImportErrorsOptions { dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } export type CreatedObject = SavedObject & { destinationId?: string }; diff --git a/src/core/server/saved_objects/index.ts b/src/core/server/saved_objects/index.ts index 11809c5b88c9..dccf63d4dcf4 100644 --- a/src/core/server/saved_objects/index.ts +++ b/src/core/server/saved_objects/index.ts @@ -85,10 +85,4 @@ export { export { savedObjectsConfig, savedObjectsMigrationConfig } from './saved_objects_config'; export { SavedObjectTypeRegistry, ISavedObjectTypeRegistry } from './saved_objects_type_registry'; -export { - Permissions, - ACL, - Principals, - TransformedPermission, - PrincipalType, -} from './permission_control/acl'; +export { Permissions, ACL, Principals, PrincipalType } from './permission_control/acl'; diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index d26589882273..087ff2e458d9 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -167,7 +167,7 @@ describe('SavedObjectsRepository', () => { }); const getMockGetResponse = ( - { type, id, references, namespace: objectNamespace, originId, workspaces, permissions }, + { type, id, references, namespace: objectNamespace, originId, permissions, workspaces }, namespace ) => { const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace; @@ -181,9 +181,9 @@ describe('SavedObjectsRepository', () => { _source: { ...(registry.isSingleNamespace(type) && { namespace: namespaceId }), ...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }), - workspaces, ...(originId && { originId }), ...(permissions && { permissions }), + ...(workspaces && { workspaces }), type, [type]: { title: 'Testing' }, references, @@ -3169,7 +3169,7 @@ describe('SavedObjectsRepository', () => { const namespace = 'foo-namespace'; const originId = 'some-origin-id'; - const getSuccess = async (type, id, options, includeOriginId, permissions) => { + const getSuccess = async (type, id, options, includeOriginId, permissions, workspaces) => { const response = getMockGetResponse( { type, @@ -3178,6 +3178,7 @@ describe('SavedObjectsRepository', () => { // operation will return it in the result. This flag is just used for test purposes to modify the mock cluster call response. ...(includeOriginId && { originId }), ...(permissions && { permissions }), + ...(workspaces && { workspaces }), }, options?.namespace ); @@ -3343,6 +3344,14 @@ describe('SavedObjectsRepository', () => { permissions: permissions, }); }); + + it(`includes workspaces property if present`, async () => { + const workspaces = ['workspace-1']; + const result = await getSuccess(type, id, { namespace }, undefined, undefined, workspaces); + expect(result).toMatchObject({ + workspaces: workspaces, + }); + }); }); }); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index c3de94bf84b9..34ff9b1e0d8f 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -1044,7 +1044,7 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); } - const { originId, updated_at: updatedAt, workspaces, permissions } = body._source; + const { originId, updated_at: updatedAt, permissions, workspaces } = body._source; let namespaces: string[] = []; if (!this._registry.isNamespaceAgnostic(type)) { @@ -1059,8 +1059,8 @@ export class SavedObjectsRepository { namespaces, ...(originId && { originId }), ...(updatedAt && { updated_at: updatedAt }), - ...(workspaces && { workspaces }), ...(permissions && { permissions }), + ...(workspaces && { workspaces }), version: encodeHitVersion(body), attributes: body._source[type], references: body._source.references || [], diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 22093b90d6b3..369cbfd53bf4 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -132,7 +132,7 @@ export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; /** Specify the workspaces for this operation */ - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObject['workspaces'] | null; } /** diff --git a/src/core/server/utils/workspace.ts b/src/core/server/utils/workspace.ts index 831755c414a5..2003e615d501 100644 --- a/src/core/server/utils/workspace.ts +++ b/src/core/server/utils/workspace.ts @@ -22,10 +22,6 @@ export const updateWorkspaceState = ( ) => { const rawRequest = ensureRawRequest(request); - if (!rawRequest.app) { - rawRequest.app = {}; - } - rawRequest.app = { ...rawRequest.app, ...payload, diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index e91a5012098e..65ff595fa200 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -126,3 +126,5 @@ export interface SavedObjectError { statusCode: number; metadata?: Record; } + +export type SavedObjectPermissions = Permissions; diff --git a/src/core/types/workspace.ts b/src/core/types/workspace.ts index ffad76fb48a2..7cdc3f92382b 100644 --- a/src/core/types/workspace.ts +++ b/src/core/types/workspace.ts @@ -3,6 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { Permissions } from '../server/saved_objects'; + export interface WorkspaceAttribute { id: string; name: string; @@ -14,6 +16,10 @@ export interface WorkspaceAttribute { defaultVISTheme?: string; } -export interface WorkspaceObject extends WorkspaceAttribute { +export interface WorkspaceAttributeWithPermission extends WorkspaceAttribute { + permissions?: Permissions; +} + +export interface WorkspaceObject extends WorkspaceAttributeWithPermission { readonly?: boolean; } diff --git a/src/plugins/data/public/data_sources/datasource_selector/datasource_selectable.tsx b/src/plugins/data/public/data_sources/datasource_selector/datasource_selectable.tsx index 1c6876815a0e..682c9c5f5a24 100644 --- a/src/plugins/data/public/data_sources/datasource_selector/datasource_selectable.tsx +++ b/src/plugins/data/public/data_sources/datasource_selector/datasource_selectable.tsx @@ -13,10 +13,17 @@ import { DataSourceGroup, DataSourceSelectableProps } from './types'; type DataSourceTypeKey = 'DEFAULT_INDEX_PATTERNS' | 's3glue' | 'spark'; // Mapping between datasource type and its display name. +// Temporary solution, will be removed along with refactoring of data source APIs const DATASOURCE_TYPE_DISPLAY_NAME_MAP: Record = { - DEFAULT_INDEX_PATTERNS: 'Index patterns', - s3glue: 'Amazon S3', - spark: 'Spark', + DEFAULT_INDEX_PATTERNS: i18n.translate('dataExplorer.dataSourceSelector.indexPatternGroupTitle', { + defaultMessage: 'Index patterns', + }), + s3glue: i18n.translate('dataExplorer.dataSourceSelector.amazonS3GroupTitle', { + defaultMessage: 'Amazon S3', + }), + spark: i18n.translate('dataExplorer.dataSourceSelector.sparkGroupTitle', { + defaultMessage: 'Spark', + }), }; type DataSetType = ISourceDataSet['data_sets'][number]; @@ -66,7 +73,19 @@ const getSourceList = (allDataSets: ISourceDataSet[]) => { const finalList = [] as DataSourceGroup[]; allDataSets.forEach((curDataSet) => { const typeKey = curDataSet.ds.getType() as DataSourceTypeKey; - const groupName = DATASOURCE_TYPE_DISPLAY_NAME_MAP[typeKey] || 'Default Group'; + let groupName = + DATASOURCE_TYPE_DISPLAY_NAME_MAP[typeKey] || + i18n.translate('dataExplorer.dataSourceSelector.defaultGroupTitle', { + defaultMessage: 'Default Group', + }); + + // add '- Opens in Log Explorer' to hint user that selecting these types of data sources + // will lead to redirection to log explorer + if (typeKey !== 'DEFAULT_INDEX_PATTERNS') { + groupName = `${groupName}${i18n.translate('dataExplorer.dataSourceSelector.redirectionHint', { + defaultMessage: ' - Opens in Log Explorer', + })}`; + } const existingGroup = finalList.find((item) => item.label === groupName); const mappedOptions = curDataSet.data_sets.map((dataSet) => diff --git a/src/plugins/data_explorer/public/components/sidebar/index.test.tsx b/src/plugins/data_explorer/public/components/sidebar/index.test.tsx new file mode 100644 index 000000000000..eccb0ffa909e --- /dev/null +++ b/src/plugins/data_explorer/public/components/sidebar/index.test.tsx @@ -0,0 +1,110 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { render, fireEvent, waitFor } from '@testing-library/react'; +import { Sidebar } from './index'; // Adjust the import path as necessary +import { Provider } from 'react-redux'; +import configureMockStore from 'redux-mock-store'; +import { MockS3DataSource } from '../../../../discover/public/__mock__/index.test.mock'; +import { createMemoryHistory } from 'history'; +import { Router } from 'react-router-dom'; + +const mockStore = configureMockStore(); +const initialState = { + metadata: { indexPattern: 'some-index-pattern-id' }, +}; +const store = mockStore(initialState); + +jest.mock('../../../../opensearch_dashboards_react/public', () => { + return { + toMountPoint: jest.fn().mockImplementation((component) => () => component), + useOpenSearchDashboards: jest.fn().mockReturnValue({ + services: { + data: { + indexPatterns: {}, + dataSources: { + dataSourceService: { + dataSources$: { + subscribe: jest.fn((callback) => { + callback({ + 's3-prod-mock': new MockS3DataSource({ + name: 's3-prod-mock', + type: 's3glue', + metadata: {}, + }), + }); + return { unsubscribe: jest.fn() }; + }), + }, + }, + }, + }, + notifications: { + toasts: { + addError: jest.fn(), + }, + }, + application: { + navigateToUrl: jest.fn(), + }, + overlays: { + openConfirm: jest.fn(), + }, + }, + }), + withOpenSearchDashboards: () => (Component: React.ComponentClass) => (props: any) => ( + + ), + }; +}); + +jest.mock('../../../../data_explorer/public', () => ({ + useTypedSelector: jest.fn(), + useTypedDispatch: jest.fn(), +})); + +describe('Sidebar Component', () => { + it('renders without crashing', () => { + const { container, getByTestId } = render( + + + + ); + expect(container).toBeInTheDocument(); + expect(getByTestId('dataExplorerDSSelect')).toBeInTheDocument(); + }); + + it('shows title extensions on the non-index pattern data source', () => { + const { getByText, getByTestId } = render( + + + + ); + + fireEvent.click(getByTestId('comboBoxToggleListButton')); + waitFor(() => { + expect(getByText('Open in Log Explorer')).toBeInTheDocument(); + }); + }); + + it('redirects to log explorer when clicking open-in-log-explorer button', () => { + const history = createMemoryHistory(); + const { getByText, getByTestId } = render( + + + + + + ); + + fireEvent.click(getByTestId('comboBoxToggleListButton')); + waitFor(() => { + expect(getByText('s3-prod-mock')).toBeInTheDocument(); + fireEvent.click(getByText('s3-prod-mock')); + expect(history.location.pathname).toContain('observability-logs#/explorer'); + }); + }); +}); diff --git a/src/plugins/data_explorer/public/components/sidebar/index.tsx b/src/plugins/data_explorer/public/components/sidebar/index.tsx index ee2dd63a6d57..ff07a59ab4b7 100644 --- a/src/plugins/data_explorer/public/components/sidebar/index.tsx +++ b/src/plugins/data_explorer/public/components/sidebar/index.tsx @@ -59,23 +59,31 @@ export const Sidebar: FC = ({ children }) => { } }, [indexPatternId, activeDataSources, dataSourceOptionList]); + const redirectToLogExplorer = useCallback( + (dsName: string, dsType: string) => { + return application.navigateToUrl( + `../observability-logs#/explorer?datasourceName=${dsName}&datasourceType=${dsType}` + ); + }, + [application] + ); + const handleSourceSelection = useCallback( (selectedDataSources: DataSourceOption[]) => { if (selectedDataSources.length === 0) { setSelectedSources(selectedDataSources); return; } - // Temporary redirection solution for 2.11, where clicking non-index-pattern datasource - // will redirect user to Observability event explorer + // Temporary redirection solution for 2.11, where clicking non-index-pattern data sources + // will prompt users with modal explaining they are being redirected to Observability log explorer if (selectedDataSources[0]?.ds?.getType() !== 'DEFAULT_INDEX_PATTERNS') { - return application.navigateToUrl( - `../observability-logs#/explorer?datasourceName=${selectedDataSources[0].label}&datasourceType=${selectedDataSources[0].type}` - ); + redirectToLogExplorer(selectedDataSources[0].label, selectedDataSources[0].type); + return; } setSelectedSources(selectedDataSources); dispatch(setIndexPattern(selectedDataSources[0].value)); }, - [application, dispatch] + [dispatch, redirectToLogExplorer, setSelectedSources] ); const handleGetDataSetError = useCallback( diff --git a/src/plugins/discover/public/__mock__/index.test.mock.ts b/src/plugins/discover/public/__mock__/index.test.mock.ts new file mode 100644 index 000000000000..6b09d1d84253 --- /dev/null +++ b/src/plugins/discover/public/__mock__/index.test.mock.ts @@ -0,0 +1,28 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +export class MockS3DataSource { + protected name: string; + protected type: string; + protected metadata: any; + + constructor({ name, type, metadata }: { name: string; type: string; metadata: any }) { + this.name = name; + this.type = type; + this.metadata = metadata; + } + + async getDataSet(dataSetParams?: any) { + return [this.name]; + } + + getName() { + return this.name; + } + + getType() { + return this.type; + } +} diff --git a/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx b/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx index a0c1851e7716..d4b8de6ad211 100644 --- a/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx +++ b/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx @@ -46,6 +46,7 @@ export interface DataGridTableProps { isContextView?: boolean; isLoading?: boolean; showPagination?: boolean; + scrollToTop?: () => void; } export const DataGridTable = ({ @@ -67,6 +68,7 @@ export const DataGridTable = ({ isContextView = false, isLoading = false, showPagination, + scrollToTop, }: DataGridTableProps) => { const services = getServices(); const [inspectedHit, setInspectedHit] = useState(); @@ -179,6 +181,7 @@ export const DataGridTable = ({ isShortDots={isShortDots} hideTimeColumn={hideTimeColumn} defaultSortOrder={defaultSortOrder} + scrollToTop={scrollToTop} /> ), [ @@ -197,6 +200,7 @@ export const DataGridTable = ({ defaultSortOrder, hideTimeColumn, isShortDots, + scrollToTop, ] ); diff --git a/src/plugins/discover/public/application/components/default_discover_table/_table_cell.scss b/src/plugins/discover/public/application/components/default_discover_table/_table_cell.scss index c960e87a9477..e87c3e62ae1f 100644 --- a/src/plugins/discover/public/application/components/default_discover_table/_table_cell.scss +++ b/src/plugins/discover/public/application/components/default_discover_table/_table_cell.scss @@ -85,8 +85,8 @@ .osdDocTableCell__source { .truncate-by-height { - transform: translateY(-1.5px); - margin-bottom: -1.5px; + margin-top: -1.5px; + margin-bottom: -3.5px; } dd, diff --git a/src/plugins/discover/public/application/components/default_discover_table/default_discover_table.tsx b/src/plugins/discover/public/application/components/default_discover_table/default_discover_table.tsx index fe8092ed8c9c..21fc1f3670da 100644 --- a/src/plugins/discover/public/application/components/default_discover_table/default_discover_table.tsx +++ b/src/plugins/discover/public/application/components/default_discover_table/default_discover_table.tsx @@ -5,7 +5,7 @@ import './_doc_table.scss'; -import React, { useEffect, useRef, useState } from 'react'; +import React, { useEffect, useRef, useState, useCallback } from 'react'; import { EuiButtonEmpty, EuiCallOut, EuiProgress } from '@elastic/eui'; import { FormattedMessage } from '@osd/i18n/react'; import { TableHeader } from './table_header'; @@ -33,6 +33,7 @@ export interface DefaultDiscoverTableProps { hideTimeColumn: boolean; defaultSortOrder: SortDirection; showPagination?: boolean; + scrollToTop?: () => void; } export const LegacyDiscoverTable = ({ @@ -52,6 +53,7 @@ export const LegacyDiscoverTable = ({ hideTimeColumn, defaultSortOrder, showPagination, + scrollToTop, }: DefaultDiscoverTableProps) => { const displayedColumns = getLegacyDisplayedColumns( columns, @@ -68,33 +70,34 @@ export const LegacyDiscoverTable = ({ endRow: rows.length < pageSize ? rows.length : pageSize, }); const observerRef = useRef(null); - const sentinelRef = useRef(null); - - const loadMoreRows = () => { - setRenderedRowCount((prevRowCount) => prevRowCount + 50); // Load 50 more rows - }; + const [sentinelEle, setSentinelEle] = useState(); + // Need a callback ref since the element isnt set on the first render. + const sentinelRef = useCallback((node: HTMLDivElement | null) => { + if (node !== null) { + setSentinelEle(node); + } + }, []); useEffect(() => { - const sentinel = sentinelRef.current; - observerRef.current = new IntersectionObserver( - (entries) => { - if (entries[0].isIntersecting) { - loadMoreRows(); - } - }, - { threshold: 1.0 } - ); + if (sentinelEle) { + observerRef.current = new IntersectionObserver( + (entries) => { + if (entries[0].isIntersecting) { + setRenderedRowCount((prevRowCount) => prevRowCount + 50); // Load 50 more rows + } + }, + { threshold: 1.0 } + ); - if (sentinelRef.current) { - observerRef.current.observe(sentinelRef.current); + observerRef.current.observe(sentinelEle); } return () => { - if (observerRef.current && sentinel) { - observerRef.current.unobserve(sentinel); + if (observerRef.current && sentinelEle) { + observerRef.current.unobserve(sentinelEle); } }; - }, []); + }, [sentinelEle]); const [activePage, setActivePage] = useState(0); const pageCount = Math.ceil(rows.length / pageSize); @@ -173,7 +176,7 @@ export const LegacyDiscoverTable = ({ values={{ sampleSize }} /> - window.scrollTo(0, 0)}> + diff --git a/src/plugins/discover/public/application/components/top_nav/get_top_nav_links.tsx b/src/plugins/discover/public/application/components/top_nav/get_top_nav_links.tsx index 1b20c444e860..6760416ab8c9 100644 --- a/src/plugins/discover/public/application/components/top_nav/get_top_nav_links.tsx +++ b/src/plugins/discover/public/application/components/top_nav/get_top_nav_links.tsx @@ -5,7 +5,6 @@ import { i18n } from '@osd/i18n'; import React from 'react'; -import { EuiText } from '@elastic/eui'; import { DiscoverViewServices } from '../../../build_services'; import { SavedSearch } from '../../../saved_searches'; import { Adapters } from '../../../../../inspector/public'; @@ -25,7 +24,6 @@ import { DOC_HIDE_TIME_COLUMN_SETTING, SORT_DEFAULT_ORDER_SETTING } from '../../ import { getSortForSearchSource } from '../../view_components/utils/get_sort_for_search_source'; import { getRootBreadcrumbs } from '../../helpers/breadcrumbs'; import { syncQueryStateWithUrl } from '../../../../../data/public'; -import { getNewDiscoverSetting, setNewDiscoverSetting } from '../utils/local_storage'; import { OpenSearchPanel } from './open_search_panel'; export const getTopNavLinks = ( @@ -44,7 +42,6 @@ export const getTopNavLinks = ( store, data: { query }, osdUrlStateStorage, - storage, } = services; const newSearch = { @@ -234,61 +231,7 @@ export const getTopNavLinks = ( }, }; - const newDiscoverButtonLabel = i18n.translate('discover.localMenu.discoverButton.label.new', { - defaultMessage: 'Try new Discover', - }); - const oldDiscoverButtonLabel = i18n.translate('discover.localMenu.discoverButton.label.old', { - defaultMessage: 'Use legacy Discover', - }); - const isNewDiscover = getNewDiscoverSetting(storage); - const newTable: TopNavMenuData = { - id: 'table-datagrid', - label: isNewDiscover ? oldDiscoverButtonLabel : newDiscoverButtonLabel, - description: i18n.translate('discover.localMenu.newTableDescription', { - defaultMessage: 'New Discover toggle Experience', - }), - testId: 'datagridTableButton', - run: async () => { - // Read the current state from localStorage - const newDiscoverEnabled = getNewDiscoverSetting(storage); - if (newDiscoverEnabled) { - const confirmed = await services.overlays.openConfirm( - toMountPoint( - -

- Help drive future improvements by{' '} - - providing feedback - {' '} - about your experience. -

-
- ), - { - title: i18n.translate('discover.localMenu.newTableConfirmModalTitle', { - defaultMessage: 'Share your thoughts on the latest Discover features', - }), - cancelButtonText: 'Cancel', - confirmButtonText: 'Turn off new features', - defaultFocusedButton: 'confirm', - } - ); - - if (confirmed) { - setNewDiscoverSetting(false, storage); - window.location.reload(); - } - } else { - // Save the new setting to localStorage - setNewDiscoverSetting(true, storage); - window.location.reload(); - } - }, - iconType: isNewDiscover ? 'editorUndo' : 'cheer', - }; - return [ - newTable, newSearch, ...(capabilities.discover?.save ? [saveSearch] : []), openSearch, diff --git a/src/plugins/discover/public/application/components/utils/local_storage.ts b/src/plugins/discover/public/application/components/utils/local_storage.ts index 5e812de8e97d..68bba0aafc99 100644 --- a/src/plugins/discover/public/application/components/utils/local_storage.ts +++ b/src/plugins/discover/public/application/components/utils/local_storage.ts @@ -9,9 +9,9 @@ export const NEW_DISCOVER_KEY = 'discover:newExpereince'; export const getNewDiscoverSetting = (storage: Storage): boolean => { const storedValue = storage.get(NEW_DISCOVER_KEY); - return storedValue !== null ? JSON.parse(storedValue) : false; + return storedValue !== null ? storedValue : false; }; export const setNewDiscoverSetting = (value: boolean, storage: Storage) => { - storage.set(NEW_DISCOVER_KEY, JSON.stringify(value)); + storage.set(NEW_DISCOVER_KEY, value); }; diff --git a/src/plugins/discover/public/application/view_components/canvas/discover_canvas.scss b/src/plugins/discover/public/application/view_components/canvas/discover_canvas.scss index 36408bd88366..2c2c8dfe8ebb 100644 --- a/src/plugins/discover/public/application/view_components/canvas/discover_canvas.scss +++ b/src/plugins/discover/public/application/view_components/canvas/discover_canvas.scss @@ -9,6 +9,13 @@ &_results { margin-left: $euiSizeM; + position: relative; + } + + &_options { + position: absolute; + top: 0; + right: 0; } } diff --git a/src/plugins/discover/public/application/view_components/canvas/discover_table.tsx b/src/plugins/discover/public/application/view_components/canvas/discover_table.tsx index 17f9f26e8b54..ccf82e4ccba0 100644 --- a/src/plugins/discover/public/application/view_components/canvas/discover_table.tsx +++ b/src/plugins/discover/public/application/view_components/canvas/discover_table.tsx @@ -12,7 +12,6 @@ import { addColumn, moveColumn, removeColumn, - reorderColumn, setColumns, setSort, useDispatch, @@ -27,9 +26,10 @@ import { popularizeField } from '../../helpers/popularize_field'; interface Props { rows?: OpenSearchSearchHit[]; + scrollToTop?: () => void; } -export const DiscoverTable = ({ rows }: Props) => { +export const DiscoverTable = ({ rows, scrollToTop }: Props) => { const { services } = useOpenSearchDashboards(); const { uiSettings, @@ -115,6 +115,7 @@ export const DiscoverTable = ({ rows }: Props) => { displayTimeColumn={displayTimeColumn} title={savedSearch?.id ? savedSearch.title : ''} description={savedSearch?.id ? savedSearch.description : ''} + scrollToTop={scrollToTop} /> ); }; diff --git a/src/plugins/discover/public/application/view_components/canvas/index.tsx b/src/plugins/discover/public/application/view_components/canvas/index.tsx index e3efe878aa83..ab34878750a7 100644 --- a/src/plugins/discover/public/application/view_components/canvas/index.tsx +++ b/src/plugins/discover/public/application/view_components/canvas/index.tsx @@ -4,7 +4,7 @@ */ import React, { useEffect, useState, useRef, useCallback } from 'react'; -import { EuiPanel } from '@elastic/eui'; +import { EuiButtonIcon, EuiContextMenu, EuiPanel, EuiPopover, EuiSwitch } from '@elastic/eui'; import { TopNav } from './top_nav'; import { ViewProps } from '../../../../../data_explorer/public'; import { DiscoverTable } from './discover_table'; @@ -21,12 +21,14 @@ import { filterColumns } from '../utils/filter_columns'; import { DEFAULT_COLUMNS_SETTING, MODIFY_COLUMNS_ON_SWITCH } from '../../../../common'; import { OpenSearchSearchHit } from '../../../application/doc_views/doc_views_types'; import './discover_canvas.scss'; +import { getNewDiscoverSetting, setNewDiscoverSetting } from '../../components/utils/local_storage'; // eslint-disable-next-line import/no-default-export export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewProps) { + const panelRef = useRef(null); const { data$, refetch$, indexPattern } = useDiscoverContext(); const { - services: { uiSettings }, + services: { uiSettings, storage }, } = useOpenSearchDashboards(); const { columns } = useSelector((state) => state.discover); const filteredColumns = filterColumns( @@ -89,9 +91,59 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro }, [dispatch, filteredColumns, indexPattern]); const timeField = indexPattern?.timeFieldName ? indexPattern.timeFieldName : undefined; + const scrollToTop = () => { + if (panelRef.current) { + panelRef.current.scrollTop = 0; + } + }; + + const [isOptionsOpen, setOptionsOpen] = useState(false); + const [useLegacy, setUseLegacy] = useState(!getNewDiscoverSetting(storage)); + const DiscoverOptions = () => ( + setOptionsOpen(!isOptionsOpen)} + /> + } + closePopover={() => setOptionsOpen(false)} + isOpen={isOptionsOpen} + panelPaddingSize="none" + className="dscCanvas_options" + > + + { + const checked = e.target.checked; + setUseLegacy(checked); + setNewDiscoverSetting(!checked, storage); + window.location.reload(); + }} + /> + + ), + }, + ]} + /> + + ); return ( - + + )} diff --git a/src/plugins/discover/public/embeddable/search_embeddable.tsx b/src/plugins/discover/public/embeddable/search_embeddable.tsx index 79080cf8657f..e2c1b1271397 100644 --- a/src/plugins/discover/public/embeddable/search_embeddable.tsx +++ b/src/plugins/discover/public/embeddable/search_embeddable.tsx @@ -47,7 +47,6 @@ import { } from '../../../data/public'; import { Container, Embeddable } from '../../../embeddable/public'; import { ISearchEmbeddable, SearchInput, SearchOutput } from './types'; -import { getDefaultSort } from '../application/view_components/utils/get_default_sort'; import { getSortForSearchSource } from '../application/view_components/utils/get_sort_for_search_source'; import { getRequestInspectorStats, @@ -216,12 +215,6 @@ export class SearchEmbeddable return; } - const sort = getDefaultSort( - indexPattern, - this.services.uiSettings.get(SORT_DEFAULT_ORDER_SETTING, 'desc') - ); - this.savedSearch.sort = sort; - const searchProps: SearchProps = { columns: this.savedSearch.columns, sort: [], diff --git a/src/plugins/workspace/public/application.tsx b/src/plugins/workspace/public/application.tsx index a0af9e695c31..87fcbc555264 100644 --- a/src/plugins/workspace/public/application.tsx +++ b/src/plugins/workspace/public/application.tsx @@ -12,6 +12,7 @@ import { WorkspaceFatalError } from './components/workspace_fatal_error'; import { WorkspaceCreatorApp } from './components/workspace_creator_app'; import { WorkspaceUpdaterApp } from './components/workspace_updater_app'; import { Services } from './types'; +import { WorkspaceOverviewApp } from './components/workspace_overview_app'; export const renderCreatorApp = ({ element }: AppMountParameters, services: Services) => { ReactDOM.render( @@ -66,3 +67,16 @@ export const renderListApp = ({ element }: AppMountParameters, services: Service ReactDOM.unmountComponentAtNode(element); }; }; + +export const renderOverviewApp = ({ element }: AppMountParameters, services: Services) => { + ReactDOM.render( + + + , + element + ); + + return () => { + ReactDOM.unmountComponentAtNode(element); + }; +}; diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx index f10fd39cfe9d..b67870b55294 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx @@ -152,7 +152,7 @@ describe('WorkspaceCreator', () => { color: '#000000', description: 'test workspace description', }), - expect.any(Array) + undefined ); await waitFor(() => { expect(notificationToastsAddSuccess).toHaveBeenCalled(); @@ -174,7 +174,7 @@ describe('WorkspaceCreator', () => { name: 'test workspace name', features: expect.arrayContaining(['app1', 'app2', 'app3']), }), - expect.any(Array) + undefined ); await waitFor(() => { expect(notificationToastsAddSuccess).toHaveBeenCalled(); @@ -201,7 +201,14 @@ describe('WorkspaceCreator', () => { expect.objectContaining({ name: 'test workspace name', }), - expect.arrayContaining([expect.objectContaining({ type: 'user', userId: 'test user id' })]) + { + read: { + users: ['test user id'], + }, + library_read: { + users: ['test user id'], + }, + } ); await waitFor(() => { expect(notificationToastsAddSuccess).toHaveBeenCalled(); diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx index 2b3511f18b8b..4b3e6e57c486 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx @@ -11,6 +11,7 @@ import { WorkspaceForm, WorkspaceFormSubmitData, WorkspaceOperationType } from ' import { WORKSPACE_OVERVIEW_APP_ID } from '../../../common/constants'; import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils'; import { WorkspaceClient } from '../../workspace_client'; +import { convertPermissionSettingsToPermissions } from '../workspace_form/utils'; export const WorkspaceCreator = () => { const { @@ -22,8 +23,11 @@ export const WorkspaceCreator = () => { async (data: WorkspaceFormSubmitData) => { let result; try { - const { permissions, ...attributes } = data; - result = await workspaceClient.create(attributes, permissions); + const { permissionSettings, ...attributes } = data; + result = await workspaceClient.create( + attributes, + convertPermissionSettingsToPermissions(permissionSettings) + ); } catch (error) { notifications?.toasts.addDanger({ title: i18n.translate('workspace.create.failed', { diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts index 15af85965943..5f81ba3e6daa 100644 --- a/src/plugins/workspace/public/components/workspace_form/types.ts +++ b/src/plugins/workspace/public/components/workspace_form/types.ts @@ -5,7 +5,7 @@ import type { WorkspacePermissionItemType, WorkspaceOperationType } from './constants'; import type { WorkspacePermissionMode } from '../../../common/constants'; -import type { App, ApplicationStart } from '../../../../../core/public'; +import type { ApplicationStart } from '../../../../../core/public'; export type WorkspacePermissionSetting = | { type: WorkspacePermissionItemType.User; userId: string; modes: WorkspacePermissionMode[] } @@ -18,7 +18,7 @@ export interface WorkspaceFormSubmitData { color?: string; icon?: string; defaultVISTheme?: string; - permissions: WorkspacePermissionSetting[]; + permissionSettings?: WorkspacePermissionSetting[]; } export interface WorkspaceFormData extends WorkspaceFormSubmitData { diff --git a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts index 7158693aedff..00bee7b3ab37 100644 --- a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts +++ b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts @@ -46,8 +46,8 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works const [permissionSettings, setPermissionSettings] = useState< Array> >( - defaultValues?.permissions && defaultValues.permissions.length > 0 - ? defaultValues.permissions + defaultValues?.permissionSettings && defaultValues.permissionSettings.length > 0 + ? defaultValues.permissionSettings : [] ); @@ -58,7 +58,7 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works description, features: selectedFeatureIds, color, - permissions: permissionSettings, + permissionSettings, }); const getFormDataRef = useRef(getFormData); getFormDataRef.current = getFormData; @@ -96,12 +96,15 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works }), }; } - const permissionErrors: string[] = new Array(formData.permissions.length); - for (let i = 0; i < formData.permissions.length; i++) { - const permission = formData.permissions[i]; + const permissionErrors: string[] = new Array(formData.permissionSettings.length); + for (let i = 0; i < formData.permissionSettings.length; i++) { + const permission = formData.permissionSettings[i]; if (isValidWorkspacePermissionSetting(permission)) { if ( - isUserOrGroupPermissionSettingDuplicated(formData.permissions.slice(0, i), permission) + isUserOrGroupPermissionSettingDuplicated( + formData.permissionSettings.slice(0, i), + permission + ) ) { permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.group', { defaultMessage: 'Duplicate permission setting', @@ -162,8 +165,11 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works formData.features = defaultValues?.features ?? []; } - const permissions = formData.permissions.filter(isValidWorkspacePermissionSetting); - onSubmit?.({ ...formData, name: formData.name!, permissions }); + onSubmit?.({ + ...formData, + name: formData.name!, + permissionSettings: formData.permissionSettings.filter(isValidWorkspacePermissionSetting), + }); }, [defaultFeatures, onSubmit, defaultValues?.features] ); diff --git a/src/plugins/workspace/public/components/workspace_form/utils.test.ts b/src/plugins/workspace/public/components/workspace_form/utils.test.ts new file mode 100644 index 000000000000..a12222cbd22b --- /dev/null +++ b/src/plugins/workspace/public/components/workspace_form/utils.test.ts @@ -0,0 +1,70 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { WorkspacePermissionMode } from '../../../common/constants'; +import { WorkspacePermissionItemType } from './constants'; +import { + convertPermissionSettingsToPermissions, + convertPermissionsToPermissionSettings, +} from './utils'; + +describe('convertPermissionSettingsToPermissions', () => { + it('should return undefined if permission items not provided', () => { + expect(convertPermissionSettingsToPermissions(undefined)).toBeUndefined(); + expect(convertPermissionSettingsToPermissions([])).toBeUndefined(); + }); + + it('should return consistent permission settings', () => { + expect( + convertPermissionSettingsToPermissions([ + { + type: WorkspacePermissionItemType.User, + userId: 'foo', + modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + }, + { + type: WorkspacePermissionItemType.Group, + group: 'bar', + modes: [WorkspacePermissionMode.LibraryWrite], + }, + ]) + ).toEqual({ + library_read: { users: ['foo'] }, + library_write: { groups: ['bar'] }, + read: { users: ['foo'] }, + }); + }); +}); + +describe('convertPermissionsToPermissionSettings', () => { + it('should return consistent permission settings', () => { + expect( + convertPermissionsToPermissionSettings({ + library_read: { users: ['foo'] }, + library_write: { groups: ['bar'] }, + read: { users: ['foo'] }, + write: { groups: ['bar'] }, + }) + ).toEqual([ + { + type: WorkspacePermissionItemType.User, + userId: 'foo', + modes: [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.Read], + }, + { + type: WorkspacePermissionItemType.Group, + group: 'bar', + modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], + }, + ]); + }); + it('should only convert workspace supported permissions', () => { + expect( + convertPermissionsToPermissionSettings({ + another_read: { users: ['foo'] }, + }) + ).toEqual([]); + }); +}); diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index 133a3bc563de..bb68f69c10e2 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -4,6 +4,7 @@ */ import { WorkspacePermissionMode, DEFAULT_CHECKED_FEATURES_IDS } from '../../../common/constants'; +import type { SavedObjectPermissions } from '../../../../../core/types'; import { WorkspaceFeature, @@ -95,3 +96,75 @@ export const getPermissionModeId = (modes: WorkspacePermissionMode[]) => { } return PermissionModeId.Read; }; + +export const convertPermissionSettingsToPermissions = ( + permissionItems: WorkspacePermissionSetting[] | undefined +) => { + if (!permissionItems || permissionItems.length === 0) { + return undefined; + } + return permissionItems.reduce((previous, current) => { + current.modes.forEach((mode) => { + if (!previous[mode]) { + previous[mode] = {}; + } + switch (current.type) { + case 'user': + previous[mode].users = [...(previous[mode].users || []), current.userId]; + break; + case 'group': + previous[mode].groups = [...(previous[mode].groups || []), current.group]; + break; + } + }); + return previous; + }, {}); +}; + +const isWorkspacePermissionMode = (test: string): test is WorkspacePermissionMode => + test === WorkspacePermissionMode.LibraryRead || + test === WorkspacePermissionMode.LibraryWrite || + test === WorkspacePermissionMode.Read || + test === WorkspacePermissionMode.Write; + +export const convertPermissionsToPermissionSettings = (permissions: SavedObjectPermissions) => { + const userPermissionSettings: WorkspacePermissionSetting[] = []; + const groupPermissionSettings: WorkspacePermissionSetting[] = []; + const settingType2Modes: { [key: string]: WorkspacePermissionMode[] } = {}; + + Object.keys(permissions).forEach((mode) => { + if (!isWorkspacePermissionMode(mode)) { + return; + } + permissions[mode].users?.forEach((userId) => { + const settingTypeKey = `userId-${userId}`; + const modes = settingType2Modes[settingTypeKey] ?? []; + + modes.push(mode); + if (modes.length === 1) { + userPermissionSettings.push({ + type: WorkspacePermissionItemType.User, + userId, + modes, + }); + settingType2Modes[settingTypeKey] = modes; + } + }); + permissions[mode].groups?.forEach((group) => { + const settingTypeKey = `group-${group}`; + const modes = settingType2Modes[settingTypeKey] ?? []; + + modes.push(mode); + if (modes.length === 1) { + groupPermissionSettings.push({ + type: WorkspacePermissionItemType.Group, + group, + modes, + }); + settingType2Modes[settingTypeKey] = modes; + } + }); + }); + + return [...userPermissionSettings, ...groupPermissionSettings]; +}; diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_feature_selector.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_feature_selector.tsx index 61181a7a749e..e172b851f1f3 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_feature_selector.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_feature_selector.tsx @@ -16,14 +16,11 @@ import { import { i18n } from '@osd/i18n'; import { groupBy } from 'lodash'; -import { - AppNavLinkStatus, - DEFAULT_APP_CATEGORIES, - PublicAppInfo, -} from '../../../../../core/public'; +import { DEFAULT_APP_CATEGORIES, PublicAppInfo } from '../../../../../core/public'; import { WorkspaceFeature, WorkspaceFeatureGroup } from './types'; import { isDefaultCheckedFeatureId, isWorkspaceFeatureGroup } from './utils'; +import { getAllExcludingManagementApps } from '../../utils'; const libraryCategoryLabel = i18n.translate('core.ui.libraryNavList.label', { defaultMessage: 'Library', @@ -58,17 +55,10 @@ export const WorkspaceFeatureSelector = ({ Array >((previousValue, currentKey) => { const apps = category2Applications[currentKey]; - const features = apps - .filter( - ({ navLinkStatus, chromeless, category }) => - navLinkStatus !== AppNavLinkStatus.hidden && - !chromeless && - category?.id !== DEFAULT_APP_CATEGORIES.management.id - ) - .map(({ id, title }) => ({ - id, - name: title, - })); + const features = getAllExcludingManagementApps(apps).map(({ id, title }) => ({ + id, + name: title, + })); if (features.length === 0) { return previousValue; } diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx index ec4f2bfed3e0..b340a71588c9 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx @@ -170,7 +170,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { diff --git a/src/plugins/workspace/public/components/workspace_list/index.tsx b/src/plugins/workspace/public/components/workspace_list/index.tsx index bc92a01f8f58..f529524d797e 100644 --- a/src/plugins/workspace/public/components/workspace_list/index.tsx +++ b/src/plugins/workspace/public/components/workspace_list/index.tsx @@ -17,7 +17,7 @@ import { import useObservable from 'react-use/lib/useObservable'; import { of } from 'rxjs'; import { i18n } from '@osd/i18n'; -import { debounce } from '../../../../../core/public'; +import { debounce, WorkspaceObject } from '../../../../../core/public'; import { WorkspaceAttribute } from '../../../../../core/public'; import { useOpenSearchDashboards } from '../../../../../plugins/opensearch_dashboards_react/public'; import { switchWorkspace, updateWorkspace } from '../utils/workspace'; @@ -26,12 +26,16 @@ import { WORKSPACE_CREATE_APP_ID } from '../../../common/constants'; import { cleanWorkspaceId } from '../../../../../core/public'; import { DeleteWorkspaceModal } from '../delete_workspace_modal'; +import { useApplications } from '././../../hooks'; +import { getSelectedFeatureQuantities } from '../../utils'; const WORKSPACE_LIST_PAGE_DESCRIPTIOIN = i18n.translate('workspace.list.description', { defaultMessage: 'Workspace allow you to save and organize library items, such as index patterns, visualizations, dashboards, saved searches, and share them with other OpenSearch Dashboards users. You can control which features are visible in each workspace, and which users and groups have read and write access to the library items in the workspace.', }); +const emptyWorkspaceList: WorkspaceObject[] = []; + export const WorkspaceList = () => { const { services: { workspaces, application, http }, @@ -39,7 +43,10 @@ export const WorkspaceList = () => { const initialSortField = 'name'; const initialSortDirection = 'asc'; - const workspaceList = useObservable(workspaces?.workspaceList$ ?? of([]), []); + const workspaceList = useObservable( + workspaces?.workspaceList$ ?? of(emptyWorkspaceList), + emptyWorkspaceList + ); const [queryInput, setQueryInput] = useState(''); const [pagination, setPagination] = useState({ pageIndex: 0, @@ -47,6 +54,7 @@ export const WorkspaceList = () => { pageSizeOptions: [5, 10, 20], }); const [deletedWorkspace, setDeletedWorkspace] = useState(null); + const applications = useApplications(application); const handleSwitchWorkspace = useCallback( (id: string) => { @@ -106,6 +114,10 @@ export const WorkspaceList = () => { name: 'Features', isExpander: true, hasActions: true, + render: (features: string[]) => { + const { total, selected } = getSelectedFeatureQuantities(features, applications); + return `${selected}/${total}`; + }, }, { name: 'Actions', diff --git a/src/plugins/workspace/public/components/workspace_overview/workspace_overview.tsx b/src/plugins/workspace/public/components/workspace_overview/workspace_overview.tsx new file mode 100644 index 000000000000..1a33f3ef233d --- /dev/null +++ b/src/plugins/workspace/public/components/workspace_overview/workspace_overview.tsx @@ -0,0 +1,31 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { EuiPanel, EuiSpacer, EuiTitle } from '@elastic/eui'; +import { useObservable } from 'react-use'; +import { of } from 'rxjs'; + +import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; + +export const WorkspaceOverview = () => { + const { + services: { workspaces }, + } = useOpenSearchDashboards(); + + const currentWorkspace = useObservable(workspaces ? workspaces.currentWorkspace$ : of(null)); + + return ( + <> + + +

Workspace

+
+ + {JSON.stringify(currentWorkspace)} +
+ + ); +}; diff --git a/src/plugins/workspace/public/components/workspace_overview_app.tsx b/src/plugins/workspace/public/components/workspace_overview_app.tsx new file mode 100644 index 000000000000..034dc12e1c62 --- /dev/null +++ b/src/plugins/workspace/public/components/workspace_overview_app.tsx @@ -0,0 +1,35 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React, { useEffect } from 'react'; +import { I18nProvider } from '@osd/i18n/react'; +import { i18n } from '@osd/i18n'; +import { useOpenSearchDashboards } from '../../../opensearch_dashboards_react/public'; +import { WorkspaceOverview } from './workspace_overview/workspace_overview'; + +export const WorkspaceOverviewApp = () => { + const { + services: { chrome }, + } = useOpenSearchDashboards(); + + /** + * set breadcrumbs to chrome + */ + useEffect(() => { + chrome?.setBreadcrumbs([ + { + text: i18n.translate('workspace.workspaceOverviewTitle', { + defaultMessage: 'Workspace overview', + }), + }, + ]); + }, [chrome]); + + return ( + + + + ); +}; diff --git a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx index 3c113a71e948..ff07076d99d1 100644 --- a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx +++ b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx @@ -169,7 +169,14 @@ describe('WorkspaceUpdater', () => { description: 'test workspace description', features: expect.arrayContaining(['app1', 'app2', 'app3']), }), - expect.arrayContaining([expect.objectContaining({ type: 'user', userId: 'test user id' })]) + { + read: { + users: ['test user id'], + }, + library_read: { + users: ['test user id'], + }, + } ); await waitFor(() => { expect(notificationToastsAddSuccess).toHaveBeenCalled(); diff --git a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx index dcc750f18be8..1f67f2063d9b 100644 --- a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx +++ b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx @@ -6,27 +6,30 @@ import React, { useCallback, useEffect, useState } from 'react'; import { EuiPage, EuiPageBody, EuiPageHeader, EuiPageContent } from '@elastic/eui'; import { i18n } from '@osd/i18n'; -import { WorkspaceAttribute } from 'opensearch-dashboards/public'; import { useObservable } from 'react-use'; import { of } from 'rxjs'; import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; import { WorkspaceForm, WorkspaceFormSubmitData, WorkspaceOperationType } from '../workspace_form'; import { WORKSPACE_OVERVIEW_APP_ID } from '../../../common/constants'; import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils'; +import { WorkspaceAttributeWithPermission } from '../../../../../core/types'; import { WorkspaceClient } from '../../workspace_client'; -import { WorkspaceFormData, WorkspacePermissionSetting } from '../workspace_form/types'; - -interface WorkspaceWithPermission extends WorkspaceAttribute { - permissions?: WorkspacePermissionSetting[]; -} +import { + convertPermissionSettingsToPermissions, + convertPermissionsToPermissionSettings, +} from '../workspace_form/utils'; function getFormDataFromWorkspace( - currentWorkspace: WorkspaceAttribute | null | undefined -): WorkspaceFormData { - const currentWorkspaceWithPermission = (currentWorkspace || {}) as WorkspaceWithPermission; + currentWorkspace: WorkspaceAttributeWithPermission | null | undefined +) { + if (!currentWorkspace) { + return null; + } return { - ...currentWorkspaceWithPermission, - permissions: currentWorkspaceWithPermission.permissions || [], + ...currentWorkspace, + permissionSettings: currentWorkspace.permissions + ? convertPermissionsToPermissionSettings(currentWorkspace.permissions) + : currentWorkspace.permissions, }; } @@ -38,7 +41,7 @@ export const WorkspaceUpdater = () => { const isPermissionEnabled = application?.capabilities.workspaces.permissionEnabled; const currentWorkspace = useObservable(workspaces ? workspaces.currentWorkspace$ : of(null)); - const [currentWorkspaceFormData, setCurrentWorkspaceFormData] = useState( + const [currentWorkspaceFormData, setCurrentWorkspaceFormData] = useState( getFormDataFromWorkspace(currentWorkspace) ); @@ -59,8 +62,12 @@ export const WorkspaceUpdater = () => { } try { - const { permissions, ...attributes } = data; - result = await workspaceClient.update(currentWorkspace.id, attributes, permissions); + const { permissionSettings, ...attributes } = data; + result = await workspaceClient.update( + currentWorkspace.id, + attributes, + convertPermissionSettingsToPermissions(permissionSettings) + ); } catch (error) { notifications?.toasts.addDanger({ title: i18n.translate('workspace.update.failed', { @@ -100,7 +107,7 @@ export const WorkspaceUpdater = () => { [notifications?.toasts, currentWorkspace, http, application, workspaceClient] ); - if (!currentWorkspaceFormData.name) { + if (!currentWorkspaceFormData) { return null; } diff --git a/src/plugins/workspace/public/hooks.ts b/src/plugins/workspace/public/hooks.ts index e84ee46507ef..4309dab2e5b0 100644 --- a/src/plugins/workspace/public/hooks.ts +++ b/src/plugins/workspace/public/hooks.ts @@ -3,15 +3,18 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { ApplicationStart, PublicAppInfo } from 'opensearch-dashboards/public'; import { useObservable } from 'react-use'; import { useMemo } from 'react'; +import { of } from 'rxjs'; +import { ApplicationStart, PublicAppInfo } from '../../../core/public'; -export function useApplications(application: ApplicationStart) { - const applications = useObservable(application.applications$); +const emptyMap = new Map(); + +export function useApplications(application?: ApplicationStart) { + const applications = useObservable(application?.applications$ ?? of(emptyMap), emptyMap); return useMemo(() => { const apps: PublicAppInfo[] = []; - applications?.forEach((app) => { + applications.forEach((app) => { apps.push(app); }); return apps; diff --git a/src/plugins/workspace/public/plugin.test.ts b/src/plugins/workspace/public/plugin.test.ts index 428d7bbc7d76..fc16bc207345 100644 --- a/src/plugins/workspace/public/plugin.test.ts +++ b/src/plugins/workspace/public/plugin.test.ts @@ -23,7 +23,7 @@ describe('Workspace plugin', () => { await workspacePlugin.setup(setupMock, { savedObjectsManagement: savedObjectManagementSetupMock, }); - expect(setupMock.application.register).toBeCalledTimes(4); + expect(setupMock.application.register).toBeCalledTimes(5); expect(WorkspaceClientMock).toBeCalledTimes(1); expect(workspaceClientMock.enterWorkspace).toBeCalledTimes(0); expect(savedObjectManagementSetupMock.columns.register).toBeCalledTimes(1); @@ -61,7 +61,7 @@ describe('Workspace plugin', () => { await workspacePlugin.setup(setupMock, { savedObjectsManagement: savedObjectsManagementPluginMock.createSetupContract(), }); - expect(setupMock.application.register).toBeCalledTimes(4); + expect(setupMock.application.register).toBeCalledTimes(5); expect(WorkspaceClientMock).toBeCalledTimes(1); expect(workspaceClientMock.enterWorkspace).toBeCalledWith('workspaceId'); expect(setupMock.getStartServices).toBeCalledTimes(1); diff --git a/src/plugins/workspace/public/plugin.ts b/src/plugins/workspace/public/plugin.ts index a4910f50d86a..1950d199074b 100644 --- a/src/plugins/workspace/public/plugin.ts +++ b/src/plugins/workspace/public/plugin.ts @@ -190,6 +190,19 @@ export class WorkspacePlugin implements Plugin<{}, {}> { }, }); + // overview + core.application.register({ + id: WORKSPACE_OVERVIEW_APP_ID, + title: i18n.translate('workspace.settings.workspaceOverview', { + defaultMessage: 'Workspace Overview', + }), + navLinkStatus: AppNavLinkStatus.hidden, + async mount(params: AppMountParameters) { + const { renderOverviewApp } = await import('./application'); + return mountWorkspaceApp(params, renderOverviewApp); + }, + }); + // workspace fatal error core.application.register({ id: WORKSPACE_FATAL_ERROR_APP_ID, diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index 510a775cd745..5ce89d9fffc6 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -3,7 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { featureMatchesConfig } from './utils'; +import { featureMatchesConfig, getSelectedFeatureQuantities } from './utils'; +import { PublicAppInfo } from '../../../core/public'; describe('workspace utils: featureMatchesConfig', () => { it('feature configured with `*` should match any features', () => { @@ -91,3 +92,48 @@ describe('workspace utils: featureMatchesConfig', () => { ); }); }); + +describe('workspace utils: getSelectedFeatureQuantities', () => { + const defaultApplications = [ + { + appRoute: '/app/dashboards', + id: 'dashboards', + title: 'Dashboards', + category: { + id: 'opensearchDashboards', + label: 'OpenSearch Dashboards', + euiIconType: 'inputOutput', + order: 1000, + }, + status: 0, + navLinkStatus: 1, + }, + { + appRoute: '/app/dev_tools', + id: 'dev_tools', + title: 'Dev Tools', + category: { + id: 'management', + label: 'Management', + order: 5000, + euiIconType: 'managementApp', + }, + status: 0, + navLinkStatus: 1, + }, + ] as PublicAppInfo[]; + it('should support * rules and exclude management category', () => { + const { total, selected } = getSelectedFeatureQuantities(['*'], defaultApplications); + expect(total).toBe(1); + expect(selected).toBe(1); + }); + + it('should get quantity normally and exclude management category', () => { + const { total, selected } = getSelectedFeatureQuantities( + ['dev_tools', '!@management'], + defaultApplications + ); + expect(total).toBe(1); + expect(selected).toBe(0); + }); +}); diff --git a/src/plugins/workspace/public/utils.ts b/src/plugins/workspace/public/utils.ts index 444b3aadadf3..2c0ad62d7775 100644 --- a/src/plugins/workspace/public/utils.ts +++ b/src/plugins/workspace/public/utils.ts @@ -3,7 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { AppCategory } from '../../../core/public'; +import { + AppCategory, + PublicAppInfo, + AppNavLinkStatus, + DEFAULT_APP_CATEGORIES, +} from '../../../core/public'; /** * Checks if a given feature matches the provided feature configuration. @@ -55,3 +60,26 @@ export const featureMatchesConfig = (featureConfigs: string[]) => ({ return matched; }; + +// Get all apps excluding management category +export const getAllExcludingManagementApps = (applications: PublicAppInfo[]): PublicAppInfo[] => { + return applications.filter( + ({ navLinkStatus, chromeless, category }) => + navLinkStatus !== AppNavLinkStatus.hidden && + !chromeless && + category?.id !== DEFAULT_APP_CATEGORIES.management.id + ); +}; + +export const getSelectedFeatureQuantities = ( + featuresConfig: string[], + applications: PublicAppInfo[] +) => { + const visibleApplications = getAllExcludingManagementApps(applications); + const featureFilter = featureMatchesConfig(featuresConfig); + const selectedApplications = visibleApplications.filter((app) => featureFilter(app)); + return { + total: visibleApplications.length, + selected: selectedApplications.length, + }; +}; diff --git a/src/plugins/workspace/public/workspace_client.ts b/src/plugins/workspace/public/workspace_client.ts index 31a08b6ae9c2..76bbb618b506 100644 --- a/src/plugins/workspace/public/workspace_client.ts +++ b/src/plugins/workspace/public/workspace_client.ts @@ -11,7 +11,7 @@ import { WorkspaceAttribute, WorkspacesSetup, } from '../../../core/public'; -import { WorkspacePermissionMode } from '../common/constants'; +import { SavedObjectPermissions, WorkspaceAttributeWithPermission } from '../../../core/types'; const WORKSPACES_API_BASE_URL = '/api/workspaces'; @@ -31,15 +31,6 @@ type IResponse = error?: string; }; -type WorkspacePermissionItem = { - modes: Array< - | WorkspacePermissionMode.LibraryRead - | WorkspacePermissionMode.LibraryWrite - | WorkspacePermissionMode.Read - | WorkspacePermissionMode.Write - >; -} & ({ type: 'user'; userId: string } | { type: 'group'; group: string }); - interface WorkspaceFindOptions { page?: number; perPage?: number; @@ -195,7 +186,7 @@ export class WorkspaceClient { */ public async create( attributes: Omit, - permissions?: WorkspacePermissionItem[] + permissions?: SavedObjectPermissions ): Promise>> { const path = this.getPath(); @@ -246,7 +237,7 @@ export class WorkspaceClient { options?: WorkspaceFindOptions ): Promise< IResponse<{ - workspaces: WorkspaceAttribute[]; + workspaces: WorkspaceAttributeWithPermission[]; total: number; per_page: number; page: number; @@ -263,9 +254,9 @@ export class WorkspaceClient { * Fetches a single workspace by a workspace id * * @param {string} id - * @returns {Promise>} The metadata of the workspace for the given id. + * @returns {Promise>} The metadata of the workspace for the given id. */ - public get(id: string): Promise> { + public get(id: string): Promise> { const path = this.getPath(id); return this.safeFetch(path, { method: 'GET', @@ -277,13 +268,13 @@ export class WorkspaceClient { * * @param {string} id * @param {object} attributes - * @param {WorkspacePermissionItem[]} permissions + * @param {WorkspacePermissionItem[]} permissionItems * @returns {Promise>} result for this operation */ public async update( id: string, attributes: Partial, - permissions?: WorkspacePermissionItem[] + permissions?: SavedObjectPermissions ): Promise> { const path = this.getPath(id); const body = { diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index 0c6e55101b7f..832c43c66399 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -5,8 +5,7 @@ import { WorkspaceAttribute } from 'src/core/types'; import * as osdTestServer from '../../../../core/test_helpers/osd_server'; -import { WORKSPACE_TYPE } from '../../../../core/server'; -import { WorkspacePermissionItem } from '../types'; +import { WORKSPACE_TYPE, Permissions } from '../../../../core/server'; const omitId = (object: T): Omit => { const { id, ...others } = object; @@ -19,7 +18,7 @@ const testWorkspace: WorkspaceAttribute = { description: 'test_workspace_description', }; -describe('workspace service', () => { +describe('workspace service api integration test', () => { let root: ReturnType; let opensearchServer: osdTestServer.TestOpenSearchUtils; let osd: osdTestServer.TestOpenSearchDashboardsUtils; @@ -36,7 +35,7 @@ describe('workspace service', () => { }, savedObjects: { permission: { - enabled: true, + enabled: false, }, }, migrations: { skip: false }, @@ -89,39 +88,6 @@ describe('workspace service', () => { expect(result.body.success).toEqual(true); expect(typeof result.body.result.id).toBe('string'); }); - it('create with permissions', async () => { - await osdTestServer.request - .post(root, `/api/workspaces`) - .send({ - attributes: omitId(testWorkspace), - permissions: [{ type: 'invalid-type', userId: 'foo', modes: ['read'] }], - }) - .expect(400); - - const result: any = await osdTestServer.request - .post(root, `/api/workspaces`) - .send({ - attributes: omitId(testWorkspace), - permissions: [{ type: 'user', userId: 'foo', modes: ['read'] }], - }) - .expect(200); - - expect(result.body.success).toEqual(true); - expect(typeof result.body.result.id).toBe('string'); - expect( - ( - await osd.coreStart.savedObjects - .createInternalRepository([WORKSPACE_TYPE]) - .get<{ permissions: WorkspacePermissionItem[] }>(WORKSPACE_TYPE, result.body.result.id) - ).attributes.permissions - ).toEqual([ - { - modes: ['read'], - type: 'user', - userId: 'foo', - }, - ]); - }); it('get', async () => { const result = await osdTestServer.request .post(root, `/api/workspaces`) @@ -162,39 +128,6 @@ describe('workspace service', () => { expect(getResult.body.success).toEqual(true); expect(getResult.body.result.name).toEqual('updated'); }); - it('update with permissions', async () => { - const result: any = await osdTestServer.request - .post(root, `/api/workspaces`) - .send({ - attributes: omitId(testWorkspace), - permissions: [{ type: 'user', userId: 'foo', modes: ['read'] }], - }) - .expect(200); - - await osdTestServer.request - .put(root, `/api/workspaces/${result.body.result.id}`) - .send({ - attributes: { - ...omitId(testWorkspace), - }, - permissions: [{ type: 'user', userId: 'foo', modes: ['write'] }], - }) - .expect(200); - - expect( - ( - await osd.coreStart.savedObjects - .createInternalRepository([WORKSPACE_TYPE]) - .get<{ permissions: WorkspacePermissionItem[] }>(WORKSPACE_TYPE, result.body.result.id) - ).attributes.permissions - ).toEqual([ - { - modes: ['write'], - type: 'user', - userId: 'foo', - }, - ]); - }); it('delete', async () => { const result: any = await osdTestServer.request .post(root, `/api/workspaces`) @@ -339,3 +272,107 @@ describe('workspace service', () => { }); }); }); + +describe('workspace service api integration test when savedObjects.permission.enabled equal true', () => { + let root: ReturnType; + let opensearchServer: osdTestServer.TestOpenSearchUtils; + let osd: osdTestServer.TestOpenSearchDashboardsUtils; + beforeAll(async () => { + const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ + adjustTimeout: (t: number) => jest.setTimeout(t), + settings: { + osd: { + workspace: { + enabled: true, + }, + savedObjects: { + permission: { + enabled: true, + }, + }, + migrations: { skip: false }, + }, + }, + }); + opensearchServer = await startOpenSearch(); + osd = await startOpenSearchDashboards(); + root = osd.root; + }); + afterAll(async () => { + await root.shutdown(); + await opensearchServer.stop(); + }); + describe('Workspace CRUD APIs', () => { + afterEach(async () => { + const listResult = await osdTestServer.request + .post(root, `/api/workspaces/_list`) + .send({ + page: 1, + }) + .expect(200); + const savedObjectsRepository = osd.coreStart.savedObjects.createInternalRepository([ + WORKSPACE_TYPE, + ]); + await Promise.all( + listResult.body.result.workspaces.map((item: WorkspaceAttribute) => + // this will delete reserved workspace + savedObjectsRepository.delete(WORKSPACE_TYPE, item.id) + ) + ); + }); + it('create', async () => { + await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omitId(testWorkspace), + permissions: { invalid_type: { users: ['foo'] } }, + }) + .expect(400); + + const result: any = await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omitId(testWorkspace), + permissions: { read: { users: ['foo'] } }, + }) + .expect(200); + + expect(result.body.success).toEqual(true); + expect(typeof result.body.result.id).toBe('string'); + expect( + ( + await osd.coreStart.savedObjects + .createInternalRepository([WORKSPACE_TYPE]) + .get<{ permissions: Permissions }>(WORKSPACE_TYPE, result.body.result.id) + ).permissions + ).toEqual({ read: { users: ['foo'] } }); + }); + it('update', async () => { + const result: any = await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omitId(testWorkspace), + }) + .expect(200); + + const updateResult = await osdTestServer.request + .put(root, `/api/workspaces/${result.body.result.id}`) + .send({ + attributes: { + ...omitId(testWorkspace), + }, + permissions: { write: { users: ['foo'] } }, + }) + .expect(200); + expect(updateResult.body.result).toBe(true); + + expect( + ( + await osd.coreStart.savedObjects + .createInternalRepository([WORKSPACE_TYPE]) + .get<{ permissions: Permissions }>(WORKSPACE_TYPE, result.body.result.id) + ).permissions + ).toEqual({ write: { users: ['foo'] } }); + }); + }); +}); diff --git a/src/plugins/workspace/server/permission_control/client.test.ts b/src/plugins/workspace/server/permission_control/client.test.ts index e05e299c153b..4d041cc7df56 100644 --- a/src/plugins/workspace/server/permission_control/client.test.ts +++ b/src/plugins/workspace/server/permission_control/client.test.ts @@ -109,6 +109,47 @@ describe('PermissionControl', () => { expect(batchValidateResult.result).toEqual(true); }); + it('should return false and log not permitted saved objects', async () => { + const logger = loggerMock.create(); + const permissionControlClient = new SavedObjectsPermissionControl(logger); + const getScopedClient = jest.fn(); + const clientMock = savedObjectsClientMock.create(); + getScopedClient.mockImplementation((request) => { + return clientMock; + }); + permissionControlClient.setup(getScopedClient, mockAuth); + + clientMock.bulkGet.mockResolvedValue({ + saved_objects: [ + { + id: 'foo', + type: 'dashboard', + references: [], + attributes: {}, + }, + { + id: 'bar', + type: 'dashboard', + references: [], + attributes: {}, + permissions: { + read: { + users: ['foo'], + }, + }, + }, + ], + }); + const batchValidateResult = await permissionControlClient.batchValidate( + httpServerMock.createOpenSearchDashboardsRequest(), + [], + ['read'] + ); + expect(batchValidateResult.success).toEqual(true); + expect(batchValidateResult.result).toEqual(false); + expect(logger.debug).toHaveBeenCalledTimes(1); + }); + describe('getPrincipalsFromRequest', () => { const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create()); const getScopedClient = jest.fn(); @@ -120,4 +161,40 @@ describe('PermissionControl', () => { expect(result.users).toEqual(['bar']); }); }); + + describe('validateSavedObjectsACL', () => { + it("should return true if saved objects don't have permissions property", () => { + const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create()); + expect( + permissionControlClient.validateSavedObjectsACL([{ type: 'workspace', id: 'foo' }], {}, []) + ).toBe(true); + }); + it('should return true if principals permitted to saved objects', () => { + const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create()); + expect( + permissionControlClient.validateSavedObjectsACL( + [{ type: 'workspace', id: 'foo', permissions: { write: { users: ['bar'] } } }], + { users: ['bar'] }, + ['write'] + ) + ).toBe(true); + }); + it('should return false and log saved objects if not permitted', () => { + const logger = loggerMock.create(); + const permissionControlClient = new SavedObjectsPermissionControl(logger); + expect( + permissionControlClient.validateSavedObjectsACL( + [{ type: 'workspace', id: 'foo', permissions: { write: { users: ['bar'] } } }], + { users: ['foo'] }, + ['write'] + ) + ).toBe(false); + expect(logger.debug).toHaveBeenCalledTimes(1); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringMatching( + /Authorization failed, principals:.*has no.*permissions on the requested saved object:.*foo/ + ) + ); + }); + }); }); diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts index 03a8fb0aaf3d..ea404e42974f 100644 --- a/src/plugins/workspace/server/permission_control/client.ts +++ b/src/plugins/workspace/server/permission_control/client.ts @@ -6,7 +6,6 @@ import { i18n } from '@osd/i18n'; import { ACL, - TransformedPermission, SavedObjectsBulkGetObject, SavedObjectsServiceStart, Logger, @@ -14,7 +13,6 @@ import { Principals, SavedObject, WORKSPACE_TYPE, - Permissions, HttpAuth, } from '../../../../core/server'; import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../../common/constants'; @@ -31,6 +29,13 @@ export class SavedObjectsPermissionControl { private readonly logger: Logger; private _getScopedClient?: SavedObjectsServiceStart['getScopedClient']; private auth?: HttpAuth; + /** + * Returns a saved objects client that is able to: + * 1. Read objects whose type is `workspace` because workspace is a hidden type and the permission control client will need to get the metadata of a specific workspace to do the permission check. + * 2. Bypass saved objects permission control wrapper because the permission control client is a dependency of the wrapper to provide the ACL validation capability. It will run into infinite loop if not bypass. + * @param request + * @returns SavedObjectsContract + */ private getScopedClient(request: OpenSearchDashboardsRequest) { return this._getScopedClient?.(request, { excludedWrappers: [WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID], @@ -83,6 +88,15 @@ export class SavedObjectsPermissionControl { return getPrincipalsFromRequest(request, this.auth); } + /** + * Validates the permissions for a collection of saved objects based on their Access Control Lists (ACL). + * This method checks whether the provided principals have the specified permission modes for each saved object. + * If any saved object lacks the required permissions, the function logs details of unauthorized access. + * + * @remarks + * If a saved object doesn't have an ACL (e.g., config objects), it is considered as having the required permissions. + * The function logs detailed information when unauthorized access is detected, including the list of denied saved objects. + */ public validateSavedObjectsACL( savedObjects: Array, 'id' | 'type' | 'workspaces' | 'permissions'>>, principals: Principals, @@ -101,7 +115,12 @@ export class SavedObjectsPermissionControl { const aclInstance = new ACL(savedObject.permissions); const hasPermission = aclInstance.hasPermission(permissionModes, principals); if (!hasPermission) { - notPermittedSavedObjects.push(savedObject); + notPermittedSavedObjects.push({ + id: savedObject.id, + type: savedObject.type, + workspaces: savedObject.workspaces, + permissions: savedObject.permissions, + }); } return hasPermission; }); @@ -145,38 +164,11 @@ export class SavedObjectsPermissionControl { } const principals = this.getPrincipalsFromRequest(request); - const deniedObjects: Array< - Pick & { - workspaces?: SavedObject['workspaces']; - permissions?: Permissions; - } - > = []; - const hasPermissionToAllObjects = savedObjectsGet.every((item) => { - // for object that doesn't contain ACL like config, return true - if (!item.permissions) { - return true; - } - const aclInstance = new ACL(item.permissions); - const hasPermission = aclInstance.hasPermission(permissionModes, principals); - if (!hasPermission) { - deniedObjects.push({ - id: item.id, - type: item.type, - workspaces: item.workspaces, - permissions: item.permissions, - }); - } - return hasPermission; - }); - if (!hasPermissionToAllObjects) { - this.logger.debug( - `Authorization failed, principals: ${JSON.stringify( - principals - )} has no [${permissionModes}] permissions on the requested saved object: ${JSON.stringify( - deniedObjects - )}` - ); - } + const hasPermissionToAllObjects = this.validateSavedObjectsACL( + savedObjectsGet, + principals, + permissionModes + ); return { success: true, result: hasPermissionToAllObjects, diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index 300b5e982a01..0ad72b51b6dc 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -14,9 +14,6 @@ describe('Workspace server plugin', () => { const setupMock = coreMock.createSetup(); const initializerContextConfigMock = coreMock.createPluginInitializerContext({ enabled: true, - permission: { - enabled: true, - }, }); setupMock.capabilities.registerProvider.mockImplementationOnce((fn) => (value = fn())); const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); @@ -56,7 +53,7 @@ describe('Workspace server plugin', () => { expect(toolKitMock.rewriteUrl).toBeCalledWith('http://localhost/app'); expect(toolKitMock.next).toBeCalledTimes(0); expect(getWorkspaceState(requestWithWorkspaceInUrl)).toEqual({ - id: 'foo', + requestWorkspaceId: 'foo', }); const requestWithoutWorkspaceInUrl = httpServerMock.createOpenSearchDashboardsRequest({ diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index e7029f8387f6..bd0b32ce62a0 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -11,13 +11,14 @@ import { Plugin, Logger, CoreStart, + SharedGlobalConfig, } from '../../../core/server'; import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID, WORKSPACE_ID_CONSUMER_WRAPPER_ID, } from '../common/constants'; -import { IWorkspaceClientImpl } from './types'; +import { IWorkspaceClientImpl, WorkspacePluginSetup, WorkspacePluginStart } from './types'; import { WorkspaceClient } from './workspace_client'; import { registerRoutes } from './routes'; import { WorkspaceSavedObjectsClientWrapper } from './saved_objects'; @@ -31,15 +32,14 @@ import { SavedObjectsPermissionControl, SavedObjectsPermissionControlContract, } from './permission_control/client'; -import { WorkspacePluginConfigType } from '../config'; import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; -export class WorkspacePlugin implements Plugin<{}, {}> { +export class WorkspacePlugin implements Plugin { private readonly logger: Logger; private client?: IWorkspaceClientImpl; private workspaceConflictControl?: WorkspaceConflictSavedObjectsClientWrapper; private permissionControl?: SavedObjectsPermissionControlContract; - private readonly config$: Observable; + private readonly globalConfig$: Observable; private workspaceSavedObjectsClientWrapper?: WorkspaceSavedObjectsClientWrapper; private proxyWorkspaceTrafficToRealHandler(setupDeps: CoreSetup) { @@ -66,14 +66,13 @@ export class WorkspacePlugin implements Plugin<{}, {}> { constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get('plugins', 'workspace'); - this.config$ = initializerContext.config.create(); + this.globalConfig$ = initializerContext.config.legacy.globalConfig$; } public async setup(core: CoreSetup) { this.logger.debug('Setting up Workspaces service'); - const config: WorkspacePluginConfigType = await this.config$.pipe(first()).toPromise(); - const isPermissionControlEnabled = - config.permission.enabled === undefined ? true : config.permission.enabled; + const globalConfig = await this.globalConfig$.pipe(first()).toPromise(); + const isPermissionControlEnabled = globalConfig.savedObjects.permission.enabled === true; this.client = new WorkspaceClient(core, this.logger); @@ -114,6 +113,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { logger: this.logger, client: this.client as IWorkspaceClientImpl, permissionControlClient: this.permissionControl, + isPermissionControlEnabled, }); core.capabilities.registerProvider(() => ({ diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index b02bff76c132..701eb8888130 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -4,9 +4,10 @@ */ import { schema } from '@osd/config-schema'; -import { CoreSetup, Logger } from '../../../../core/server'; +import { CoreSetup, Logger, PrincipalType, ACL } from '../../../../core/server'; +import { WorkspaceAttributeWithPermission } from '../../../../core/types'; import { WorkspacePermissionMode } from '../../common/constants'; -import { IWorkspaceClientImpl, WorkspacePermissionItem } from '../types'; +import { IWorkspaceClientImpl } from '../types'; import { SavedObjectsPermissionControlContract } from '../permission_control/client'; const WORKSPACES_API_BASE_URL = '/api/workspaces'; @@ -18,19 +19,16 @@ const workspacePermissionMode = schema.oneOf([ schema.literal(WorkspacePermissionMode.LibraryWrite), ]); -const workspacePermission = schema.oneOf([ - schema.object({ - type: schema.literal('user'), - userId: schema.string(), - modes: schema.arrayOf(workspacePermissionMode), - }), - schema.object({ - type: schema.literal('group'), - group: schema.string(), - modes: schema.arrayOf(workspacePermissionMode), - }), +const principalType = schema.oneOf([ + schema.literal(PrincipalType.Users), + schema.literal(PrincipalType.Groups), ]); +const workspacePermissions = schema.recordOf( + workspacePermissionMode, + schema.recordOf(principalType, schema.arrayOf(schema.string()), {}) +); + const workspaceAttributesSchema = schema.object({ description: schema.maybe(schema.string()), name: schema.string(), @@ -46,11 +44,13 @@ export function registerRoutes({ logger, http, permissionControlClient, + isPermissionControlEnabled, }: { client: IWorkspaceClientImpl; logger: Logger; http: CoreSetup['http']; permissionControlClient?: SavedObjectsPermissionControlContract; + isPermissionControlEnabled: boolean; }) { const router = http.createRouter(); router.post( @@ -119,29 +119,30 @@ export function registerRoutes({ validate: { body: schema.object({ attributes: workspaceAttributesSchema, - permissions: schema.maybe( - schema.oneOf([workspacePermission, schema.arrayOf(workspacePermission)]) - ), + permissions: schema.maybe(workspacePermissions), }), }, }, router.handleLegacyErrors(async (context, req, res) => { - const { attributes, permissions: permissionsInRequest } = req.body; - const authInfo = permissionControlClient?.getPrincipalsFromRequest(req); - let permissions: WorkspacePermissionItem[] = []; - if (permissionsInRequest) { - permissions = Array.isArray(permissionsInRequest) - ? permissionsInRequest - : [permissionsInRequest]; - } + const { attributes, permissions } = req.body; + const principals = permissionControlClient?.getPrincipalsFromRequest(req); + const createPayload: Omit = attributes; - // Assign workspace owner to current user - if (!!authInfo?.users?.length) { - permissions.push({ - type: 'user', - userId: authInfo.users[0], - modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], - }); + if (isPermissionControlEnabled) { + createPayload.permissions = permissions; + // Assign workspace owner to current user + if (!!principals?.users?.length) { + const acl = new ACL(permissions); + const currentUserId = principals.users[0]; + [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite].forEach( + (permissionMode) => { + if (!acl.hasPermission([permissionMode], { users: [currentUserId] })) { + acl.addPermission([permissionMode], { users: [currentUserId] }); + } + } + ); + createPayload.permissions = acl.getPermissions(); + } } const result = await client.create( @@ -150,10 +151,7 @@ export function registerRoutes({ request: req, logger, }, - { - ...attributes, - ...(permissions.length ? { permissions } : {}), - } + createPayload ); return res.ok({ body: result }); }) @@ -167,19 +165,13 @@ export function registerRoutes({ }), body: schema.object({ attributes: workspaceAttributesSchema, - permissions: schema.maybe( - schema.oneOf([workspacePermission, schema.arrayOf(workspacePermission)]) - ), + permissions: schema.maybe(workspacePermissions), }), }, }, router.handleLegacyErrors(async (context, req, res) => { const { id } = req.params; const { attributes, permissions } = req.body; - let finalPermissions: WorkspacePermissionItem[] = []; - if (permissions) { - finalPermissions = Array.isArray(permissions) ? permissions : [permissions]; - } const result = await client.update( { @@ -190,7 +182,7 @@ export function registerRoutes({ id, { ...attributes, - ...(finalPermissions.length ? { permissions: finalPermissions } : {}), + ...(isPermissionControlEnabled ? { permissions } : {}), } ); return res.ok({ body: result }); diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts index 2a7fb0e440b5..b6ea26456f0e 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts @@ -527,4 +527,70 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(SavedObjectsErrorHelpers.isNotFoundError(error)).toBe(true); }); }); + + describe('deleteByWorkspace', () => { + it('should throw forbidden error when workspace not permitted', async () => { + let error; + try { + await notPermittedSavedObjectedClient.deleteByWorkspace('workspace-1'); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + }); + + it('should be able to delete all data in permitted workspace', async () => { + const deleteWorkspaceId = 'workspace-to-delete'; + await repositoryKit.create( + internalSavedObjectsRepository, + 'workspace', + {}, + { + id: deleteWorkspaceId, + permissions: { + library_read: { users: ['foo'] }, + library_write: { users: ['foo'] }, + }, + } + ); + const dashboardIds = [ + 'inner-delete-workspace-dashboard-1', + 'inner-delete-workspace-dashboard-2', + ]; + await Promise.all( + dashboardIds.map((dashboardId) => + repositoryKit.create( + internalSavedObjectsRepository, + 'dashboard', + {}, + { + id: dashboardId, + workspaces: [deleteWorkspaceId], + } + ) + ) + ); + + expect( + ( + await permittedSavedObjectedClient.find({ + type: 'dashboard', + workspaces: [deleteWorkspaceId], + }) + ).total + ).toBe(2); + + await permittedSavedObjectedClient.deleteByWorkspace(deleteWorkspaceId, { refresh: true }); + + expect( + ( + await permittedSavedObjectedClient.find({ + type: 'dashboard', + workspaces: [deleteWorkspaceId], + }) + ).total + ).toBe(0); + }); + }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts index 6b40f6e60fa0..07d1e6aff40c 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts @@ -26,6 +26,15 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { }, permissions: {}, }, + { + type: 'dashboard', + id: 'dashboard-with-empty-workspace-property', + workspaces: [], + attributes: { + bar: 'baz', + }, + permissions: {}, + }, { type: 'workspace', id: 'workspace-1', attributes: { name: 'Workspace - 1' } }, { type: 'workspace', @@ -40,6 +49,9 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { type: 'config', }; } + if (id === 'unknown-error-dashboard') { + throw new Error('Unknown error'); + } return ( savedObjectsStore.find((item) => item.type === type && item.id === id) || SavedObjectsErrorHelpers.createGenericNotFoundError() @@ -82,14 +94,16 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { getPrincipalsFromRequest: jest.fn().mockImplementation(() => ({ users: ['user-1'] })), }; const wrapper = new WorkspaceSavedObjectsClientWrapper(permissionControlMock); - wrapper.setScopedClient(() => ({ + const scopedClientMock = { find: jest.fn().mockImplementation(async () => ({ saved_objects: [{ id: 'workspace-1', type: 'workspace' }], })), - })); + }; + wrapper.setScopedClient(() => scopedClientMock); return { wrapper: wrapper.wrapperFactory(wrapperOptions), clientMock, + scopedClientMock, permissionControlMock, requestMock, }; @@ -122,6 +136,16 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ); expect(errorCatched?.message).toEqual('Invalid saved objects permission'); }); + it("should throw permission error if deleting saved object's workspace property is empty", async () => { + const { wrapper } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.delete('dashboard', 'dashboard-with-empty-workspace-property'); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid saved objects permission'); + }); it('should call client.delete with arguments if permitted', async () => { const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); const deleteArgs = ['dashboard', 'foo', { force: true }] as const; @@ -157,6 +181,18 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ); expect(errorCatched?.message).toEqual('Invalid saved objects permission'); }); + it("should throw permission error if updating saved object's workspace property is empty", async () => { + const { wrapper } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.update('dashboard', 'dashboard-with-empty-workspace-property', { + bar: 'foo', + }); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid saved objects permission'); + }); it('should call client.update with arguments if permitted', async () => { const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); const updateArgs = [ @@ -260,6 +296,26 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ); expect(errorCatched?.message).toEqual('Invalid workspace permission'); }); + it('should throw error if unable to get object when override', async () => { + const { + wrapper, + permissionControlMock, + requestMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + permissionControlMock.validate.mockResolvedValueOnce({ success: true, result: false }); + let errorCatched; + try { + await wrapper.bulkCreate( + [{ type: 'dashboard', id: 'unknown-error-dashboard', attributes: { bar: 'baz' } }], + { + overwrite: true, + } + ); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toBe('Unknown error'); + }); it('should call client.bulkCreate with arguments if some objects not found', async () => { const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); const objectsToBulkCreate = [ @@ -268,11 +324,9 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ]; await wrapper.bulkCreate(objectsToBulkCreate, { overwrite: true, - workspaces: ['workspace-1'], }); expect(clientMock.bulkCreate).toHaveBeenCalledWith(objectsToBulkCreate, { overwrite: true, - workspaces: ['workspace-1'], }); }); it('should call client.bulkCreate with arguments if permitted', async () => { @@ -549,6 +603,24 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ACLSearchParams: {}, }); }); + it('should find permitted workspaces with filtered permission modes', async () => { + const { wrapper, scopedClientMock } = generateWorkspaceSavedObjectsClientWrapper(); + await wrapper.find({ + type: 'dashboard', + ACLSearchParams: { + permissionModes: ['read', 'library_read'], + }, + }); + expect(scopedClientMock.find).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'workspace', + ACLSearchParams: { + permissionModes: ['library_read'], + principals: { users: ['user-1'] }, + }, + }) + ); + }); it('should call client.find with arguments if not workspace type and no options.workspace', async () => { const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); await wrapper.find({ @@ -556,8 +628,6 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { }); expect(clientMock.find).toHaveBeenCalledWith({ type: 'dashboard', - workspaces: ['workspace-1'], - workspacesSearchOperator: 'OR', ACLSearchParams: { permissionModes: ['read', 'write'], principals: { users: ['user-1'] }, diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index d5497bc6475c..4d5d03641b5f 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -22,10 +22,10 @@ import { SavedObjectsBulkUpdateResponse, SavedObjectsBulkUpdateOptions, WORKSPACE_TYPE, - SavedObjectsDeleteByWorkspaceOptions, SavedObjectsErrorHelpers, SavedObjectsServiceStart, SavedObjectsClientContract, + SavedObjectsDeleteByWorkspaceOptions, } from '../../../../core/server'; import { SavedObjectsPermissionControlContract } from '../permission_control/client'; import { @@ -68,22 +68,13 @@ const getDefaultValuesForEmpty = (values: T[] | undefined, defaultValues: T[] export class WorkspaceSavedObjectsClientWrapper { private getScopedClient?: SavedObjectsServiceStart['getScopedClient']; - private formatWorkspacePermissionModeToStringArray( - permission: WorkspacePermissionMode | WorkspacePermissionMode[] - ): string[] { - if (Array.isArray(permission)) { - return permission; - } - - return [permission]; - } private async validateObjectsPermissions( objects: Array>, request: OpenSearchDashboardsRequest, - permissionMode: WorkspacePermissionMode | WorkspacePermissionMode[] + permissionModes: WorkspacePermissionMode[] ) { - // PermissionMode here is an array which is merged by workspace type required permission and other saved object required permission. + // PermissionModes here is an array which is merged by workspace type required permission and other saved object required permission. // So we only need to do one permission check no matter its type. for (const { id, type } of objects) { const validateResult = await this.permissionControl.validate( @@ -92,7 +83,7 @@ export class WorkspaceSavedObjectsClientWrapper { type, id, }, - this.formatWorkspacePermissionModeToStringArray(permissionMode) + permissionModes ); if (!validateResult?.result) { return false; @@ -105,20 +96,20 @@ export class WorkspaceSavedObjectsClientWrapper { private validateMultiWorkspacesPermissions = async ( workspacesIds: string[], request: OpenSearchDashboardsRequest, - permissionMode: WorkspacePermissionMode | WorkspacePermissionMode[] + permissionModes: WorkspacePermissionMode[] ) => { // for attributes and options passed in this function, the num of workspaces may be 0.This case should not be passed permission check. if (workspacesIds.length === 0) { return false; } const workspaces = workspacesIds.map((id) => ({ id, type: WORKSPACE_TYPE })); - return await this.validateObjectsPermissions(workspaces, request, permissionMode); + return await this.validateObjectsPermissions(workspaces, request, permissionModes); }; private validateAtLeastOnePermittedWorkspaces = async ( workspaces: string[] | undefined, request: OpenSearchDashboardsRequest, - permissionMode: WorkspacePermissionMode | WorkspacePermissionMode[] + permissionModes: WorkspacePermissionMode[] ) => { // for attributes and options passed in this function, the num of workspaces attribute may be 0.This case should not be passed permission check. if (!workspaces || workspaces.length === 0) { @@ -131,7 +122,7 @@ export class WorkspaceSavedObjectsClientWrapper { type: WORKSPACE_TYPE, id: workspaceId, }, - this.formatWorkspacePermissionModeToStringArray(permissionMode) + permissionModes ); if (validateResult?.result) { return true; @@ -499,12 +490,9 @@ export class WorkspaceSavedObjectsClientWrapper { options.workspaces = permittedWorkspaces; } else { /** - * Select all the docs that - * 1. ACL matches read / write / user passed permission OR - * 2. workspaces matches library_read or library_write OR + * If no workspaces present, find all the docs that + * ACL matches read / write / user passed permission */ - options.workspaces = permittedWorkspaceIds; - options.workspacesSearchOperator = 'OR'; options.ACLSearchParams.permissionModes = getDefaultValuesForEmpty( options.ACLSearchParams.permissionModes, [WorkspacePermissionMode.Read, WorkspacePermissionMode.Write] diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index ba1ff8a9cd47..b506bb493a4c 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -12,7 +12,7 @@ import { WorkspaceAttribute, SavedObjectsServiceStart, } from '../../../core/server'; -import { WorkspacePermissionMode } from '../common/constants'; +import { WorkspaceAttributeWithPermission } from '../../../core/types'; export interface WorkspaceFindOptions { page?: number; @@ -53,7 +53,7 @@ export interface IWorkspaceClientImpl { */ create( requestDetail: IRequestDetail, - payload: Omit + payload: Omit ): Promise>; /** * List workspaces @@ -68,7 +68,7 @@ export interface IWorkspaceClientImpl { ): Promise< IResponse< { - workspaces: WorkspaceAttribute[]; + workspaces: WorkspaceAttributeWithPermission[]; } & Pick > >; @@ -76,10 +76,13 @@ export interface IWorkspaceClientImpl { * Get the detail of a given workspace id * @param requestDetail {@link IRequestDetail} * @param id workspace id - * @returns a Promise with the detail of {@link WorkspaceAttribute} + * @returns a Promise with the detail of {@link WorkspaceAttributeWithPermission} * @public */ - get(requestDetail: IRequestDetail, id: string): Promise>; + get( + requestDetail: IRequestDetail, + id: string + ): Promise>; /** * Update the detail of a given workspace * @param requestDetail {@link IRequestDetail} @@ -91,7 +94,7 @@ export interface IWorkspaceClientImpl { update( requestDetail: IRequestDetail, id: string, - payload: Omit + payload: Omit ): Promise>; /** * Delete a given workspace @@ -124,11 +127,10 @@ export interface AuthInfo { user_name?: string; } -export type WorkspacePermissionItem = { - modes: Array< - | WorkspacePermissionMode.LibraryRead - | WorkspacePermissionMode.LibraryWrite - | WorkspacePermissionMode.Read - | WorkspacePermissionMode.Write - >; -} & ({ type: 'user'; userId: string } | { type: 'group'; group: string }); +export interface WorkspacePluginSetup { + client: IWorkspaceClientImpl; +} + +export interface WorkspacePluginStart { + client: IWorkspaceClientImpl; +} diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index a8b006c99a87..79df5f5d4558 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -4,7 +4,7 @@ */ import { i18n } from '@osd/i18n'; -import type { +import { SavedObject, SavedObjectsClientContract, CoreSetup, @@ -17,6 +17,7 @@ import { WORKSPACE_TYPE, Logger, } from '../../../core/server'; +import { WorkspaceAttributeWithPermission } from '../../../core/types'; import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; @@ -73,10 +74,11 @@ export class WorkspaceClient implements IWorkspaceClientImpl { } private getFlattenedResultWithSavedObject( savedObject: SavedObject - ): WorkspaceAttribute { + ): WorkspaceAttributeWithPermission { return { ...savedObject.attributes, id: savedObject.id, + permissions: savedObject.permissions, }; } private formatError(error: Error | any): string { @@ -122,10 +124,10 @@ export class WorkspaceClient implements IWorkspaceClientImpl { } public async create( requestDetail: IRequestDetail, - payload: Omit + payload: Omit ): ReturnType { try { - const attributes = payload; + const { permissions, ...attributes } = payload; const id = generateRandomId(WORKSPACE_ID_SIZE); const client = this.getSavedObjectClientsFromRequestDetail(requestDetail); const existingWorkspaceRes = await this.getScopedClientWithoutPermission(requestDetail)?.find( @@ -143,6 +145,7 @@ export class WorkspaceClient implements IWorkspaceClientImpl { attributes, { id, + permissions, } ); return { @@ -222,7 +225,7 @@ export class WorkspaceClient implements IWorkspaceClientImpl { public async get( requestDetail: IRequestDetail, id: string - ): Promise> { + ): ReturnType { try { const result = await this.getSavedObjectClientsFromRequestDetail(requestDetail).get< WorkspaceAttribute @@ -241,9 +244,9 @@ export class WorkspaceClient implements IWorkspaceClientImpl { public async update( requestDetail: IRequestDetail, id: string, - payload: Omit + payload: Omit ): Promise> { - const attributes = payload; + const { permissions, ...attributes } = payload; try { const client = this.getSavedObjectClientsFromRequestDetail(requestDetail); const workspaceInDB: SavedObject = await client.get(WORKSPACE_TYPE, id); @@ -263,9 +266,9 @@ export class WorkspaceClient implements IWorkspaceClientImpl { throw new Error(DUPLICATE_WORKSPACE_NAME_ERROR); } } - await client.create>(WORKSPACE_TYPE, attributes, { id, + permissions, overwrite: true, version: workspaceInDB.version, }); diff --git a/test/functional/apps/dashboard/dashboard_filter_bar.js b/test/functional/apps/dashboard/dashboard_filter_bar.js index dde86c697e3c..17e29fa5cf6f 100644 --- a/test/functional/apps/dashboard/dashboard_filter_bar.js +++ b/test/functional/apps/dashboard/dashboard_filter_bar.js @@ -193,6 +193,7 @@ export default function ({ getService, getPageObjects }) { before(async () => { await filterBar.ensureFieldEditorModalIsClosed(); await PageObjects.common.navigateToApp('discover'); + await PageObjects.timePicker.setDefaultDataRange(); await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.common.navigateToApp('dashboard'); await PageObjects.dashboard.gotoDashboardLandingPage(); diff --git a/test/functional/apps/dashboard/dashboard_filtering.js b/test/functional/apps/dashboard/dashboard_filtering.js index 1040b87f6168..6be4b4837da0 100644 --- a/test/functional/apps/dashboard/dashboard_filtering.js +++ b/test/functional/apps/dashboard/dashboard_filtering.js @@ -80,6 +80,7 @@ export default function ({ getService, getPageObjects }) { describe('adding a filter that excludes all data', () => { before(async () => { await PageObjects.common.navigateToApp('discover'); + await PageObjects.timePicker.setDefaultDataRange(); await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.common.navigateToApp('dashboard'); await PageObjects.dashboard.gotoDashboardLandingPage(); diff --git a/test/functional/apps/dashboard/dashboard_state.js b/test/functional/apps/dashboard/dashboard_state.js index 11196a1b69b9..78f0cdb16184 100644 --- a/test/functional/apps/dashboard/dashboard_state.js +++ b/test/functional/apps/dashboard/dashboard_state.js @@ -57,6 +57,7 @@ export default function ({ getService, getPageObjects }) { describe('dashboard state', function describeIndexTests() { before(async function () { await PageObjects.common.navigateToApp('discover'); + await PageObjects.timePicker.setHistoricalDataRange(); await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.common.navigateToApp('dashboard'); await PageObjects.dashboard.initTests(); diff --git a/test/functional/apps/management/_scripted_fields.js b/test/functional/apps/management/_scripted_fields.js index 8a4659630ee1..f3d89138eb77 100644 --- a/test/functional/apps/management/_scripted_fields.js +++ b/test/functional/apps/management/_scripted_fields.js @@ -161,9 +161,9 @@ export default function ({ getService, getPageObjects }) { const fromTime = 'Sep 17, 2015 @ 06:31:44.000'; const toTime = 'Sep 18, 2015 @ 18:31:44.000'; await PageObjects.common.navigateToApp('discover'); - await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.selectIndexPattern('logstash-*'); await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime); + await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.clickFieldListItem(scriptedPainlessFieldName); await retry.try(async function () { @@ -280,9 +280,9 @@ export default function ({ getService, getPageObjects }) { const fromTime = 'Sep 17, 2015 @ 06:31:44.000'; const toTime = 'Sep 18, 2015 @ 18:31:44.000'; await PageObjects.common.navigateToApp('discover'); - await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.selectIndexPattern('logstash-*'); await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime); + await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.clickFieldListItem(scriptedPainlessFieldName2); await retry.try(async function () { @@ -377,9 +377,9 @@ export default function ({ getService, getPageObjects }) { const fromTime = 'Sep 17, 2015 @ 06:31:44.000'; const toTime = 'Sep 18, 2015 @ 18:31:44.000'; await PageObjects.common.navigateToApp('discover'); - await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.selectIndexPattern('logstash-*'); await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime); + await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.clickFieldListItem(scriptedPainlessFieldName2); await retry.try(async function () { @@ -477,9 +477,9 @@ export default function ({ getService, getPageObjects }) { const fromTime = 'Sep 17, 2015 @ 19:22:00.000'; const toTime = 'Sep 18, 2015 @ 07:00:00.000'; await PageObjects.common.navigateToApp('discover'); - await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.selectIndexPattern('logstash-*'); await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime); + await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.clickFieldListItem(scriptedPainlessFieldName2); await retry.try(async function () { diff --git a/test/functional/page_objects/discover_page.ts b/test/functional/page_objects/discover_page.ts index cf0b03022ab9..8f03ae20ddf7 100644 --- a/test/functional/page_objects/discover_page.ts +++ b/test/functional/page_objects/discover_page.ts @@ -519,12 +519,15 @@ export function DiscoverPageProvider({ getService, getPageObjects }: FtrProvider public async switchDiscoverTable(tableType: string) { await retry.try(async () => { - const switchButton = await testSubjects.find('datagridTableButton'); - const buttonText = await switchButton.getVisibleText(); + const optionsButton = await testSubjects.find('discoverOptionsButton'); + await optionsButton.click(); - if (tableType === 'new' && buttonText.includes('Try new Discover')) { + const switchButton = await testSubjects.find('discoverOptionsLegacySwitch'); + const isLegacyChecked = (await switchButton.getAttribute('aria-checked')) === 'true'; + + if (tableType === 'new' && isLegacyChecked) { await switchButton.click(); - } else if (tableType === 'legacy' && buttonText.includes('Use legacy Discover')) { + } else if (tableType === 'legacy' && !isLegacyChecked) { await switchButton.click(); } }); diff --git a/test/plugin_functional/test_suites/doc_views/doc_views.ts b/test/plugin_functional/test_suites/doc_views/doc_views.ts index b745d6e8a417..d215017132ae 100644 --- a/test/plugin_functional/test_suites/doc_views/doc_views.ts +++ b/test/plugin_functional/test_suites/doc_views/doc_views.ts @@ -39,10 +39,10 @@ export default function ({ getService, getPageObjects }: PluginFunctionalProvide describe('custom doc views with datagrid table', function () { before(async () => { await PageObjects.common.navigateToApp('discover'); - await PageObjects.discover.switchDiscoverTable('new'); // TODO: change back to setDefaultRange() once we resolve // https://github.com/opensearch-project/OpenSearch-Dashboards/issues/5241 await PageObjects.timePicker.setDefaultRangeForDiscover(); + await PageObjects.discover.switchDiscoverTable('new'); }); it('should show custom doc views', async () => { diff --git a/yarn.lock b/yarn.lock index e1ac33225f8d..a6bd5836ef6c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3904,6 +3904,13 @@ dependencies: "@types/react" "*" +"@types/redux-mock-store@^1.0.6": + version "1.0.6" + resolved "https://registry.yarnpkg.com/@types/redux-mock-store/-/redux-mock-store-1.0.6.tgz#0a03b2655028b7cf62670d41ac1de5ca1b1f5958" + integrity sha512-eg5RDfhJTXuoJjOMyXiJbaDb1B8tfTaJixscmu+jOusj6adGC0Krntz09Tf4gJgXeCqCrM5bBMd+B7ez0izcAQ== + dependencies: + redux "^4.0.5" + "@types/refractor@^3.0.0": version "3.0.2" resolved "https://registry.yarnpkg.com/@types/refractor/-/refractor-3.0.2.tgz#2d42128d59f78f84d2c799ffc5ab5cadbcba2d82" @@ -15452,6 +15459,13 @@ redent@^3.0.0: indent-string "^4.0.0" strip-indent "^3.0.0" +redux-mock-store@^1.5.4: + version "1.5.4" + resolved "https://registry.yarnpkg.com/redux-mock-store/-/redux-mock-store-1.5.4.tgz#90d02495fd918ddbaa96b83aef626287c9ab5872" + integrity sha512-xmcA0O/tjCLXhh9Fuiq6pMrJCwFRaouA8436zcikdIpYWWCjU76CRk+i2bHx8EeiSiMGnB85/lZdU3wIJVXHTA== + dependencies: + lodash.isplainobject "^4.0.6" + redux-thunk@^2.3.0, redux-thunk@^2.4.1: version "2.4.1" resolved "https://registry.yarnpkg.com/redux-thunk/-/redux-thunk-2.4.1.tgz#0dd8042cf47868f4b29699941de03c9301a75714"