Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add permission control service for saved objects and workspace saved objects client wrapper #230

Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
caa8035
feat: add basic workspace saved objects client wrapper
wanglam Sep 15, 2023
ab7c1ca
feat: add unit test (#2)
SuZhou-Joe Oct 8, 2023
35a7605
feat: update client wrapper
raintygao Oct 16, 2023
73b1de1
feat: init permission control in workspace plugin
wanglam Oct 17, 2023
1e3c1b8
Support disable permission check on workspace (#228)
Hailong-am Oct 16, 2023
48fea51
feat: add ACLSearchParams consumer in repository (#3)
SuZhou-Joe Oct 17, 2023
c84cb39
fix: ACLSearchParams missing in search dsl
wanglam Oct 18, 2023
cb97c2e
test: add integration test for workspace saved objects client wrapper
wanglam Oct 18, 2023
e3cf913
style: add empty line under license
wanglam Oct 18, 2023
cb2116e
test: enable workspace permission control for integration tests
wanglam Oct 18, 2023
76a0bfc
feat: add workspace into includeHiddenTypes (#249)
SuZhou-Joe Feb 27, 2024
78a3fed
fix workspace client wrapper integration tests
wanglam Feb 28, 2024
036d032
add permissions fields to workspace CRUD APIs
wanglam Feb 28, 2024
7804d6b
Move WorkspacePermissionMode inside workspace plugin
wanglam Mar 1, 2024
719e5c2
Address pr comments
wanglam Mar 1, 2024
e1b0c55
Remove ACLSearchParams in public SavedObjectsFindOptions
wanglam Mar 1, 2024
04f8602
Remove lodash and Add default permissionModes
wanglam Mar 1, 2024
195ed9d
feat: address concerns on ensureRawRequest (#4)
SuZhou-Joe Mar 4, 2024
6924115
Update annotations and error
wanglam Mar 4, 2024
b3f10df
Add unit tests for worksapce saved objects client wrapper
wanglam Mar 4, 2024
9ed67c7
Remove getPrincipalsOfObjects in permission
wanglam Mar 5, 2024
6386883
Fix permissionEnabled flag missed in workspace plugin setup test
wanglam Mar 5, 2024
adae252
Change back to Not Authorized error
wanglam Mar 5, 2024
953e6c2
Fix unit tests for query_params and plugin setup
wanglam Mar 5, 2024
70655ce
Fix unittests in workspace server utils
wanglam Mar 5, 2024
43b0eb3
feat: add workspacesSearchOperators to decouple ACLSearchParams
SuZhou-Joe Mar 5, 2024
f16f1cc
feat: update test cases
SuZhou-Joe Mar 5, 2024
60b3b02
feat: optimize test cases
SuZhou-Joe Mar 5, 2024
2abdc57
feat: optimize comment
SuZhou-Joe Mar 6, 2024
7fe7530
feat: omit defaultSearchOperator in public savedobjetcs client
SuZhou-Joe Mar 6, 2024
b36c380
feat: omit workspacesSearchOperator field
SuZhou-Joe Mar 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion config/opensearch_dashboards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -277,4 +277,7 @@
# vis_augmenter.pluginAugmentationEnabled: true

# Set the value to true to enable workspace feature
# workspace.enabled: false
# workspace.enabled: false
# Set the value to false to disable permission check on workspace
# Permission check depends on OpenSearch Dashboards has authentication enabled, set it to false if no authentication is configured
# workspace.permission.enabled: true
1 change: 1 addition & 0 deletions src/core/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export {
StringValidationRegex,
StringValidationRegexString,
WorkspaceObject,
WorkspaceAttribute,
} from '../types';

export {
Expand Down
21 changes: 9 additions & 12 deletions src/core/public/saved_objects/saved_objects_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import { HttpFetchOptions, HttpSetup } from '../http';

type SavedObjectsFindOptions = Omit<
SavedObjectFindOptionsServer,
'sortOrder' | 'rootSearchFields' | 'typeToNamespacesMap'
'sortOrder' | 'rootSearchFields' | 'typeToNamespacesMap' | 'ACLSearchParams'
>;

type PromiseType<T extends Promise<any>> = T extends Promise<infer U> ? U : never;
Expand Down Expand Up @@ -392,17 +392,14 @@ export class SavedObjectsClient {
finalWorkspaces = Array.from(new Set([currentWorkspaceId]));
}

const renamedQuery = renameKeys<Omit<SavedObjectsFindOptions, 'ACLSearchParams'>, any>(
renameMap,
{
...options,
...(finalWorkspaces
? {
workspaces: finalWorkspaces,
}
: {}),
}
);
const renamedQuery = renameKeys<SavedObjectsFindOptions, any>(renameMap, {
...options,
...(finalWorkspaces
? {
workspaces: finalWorkspaces,
}
: {}),
});
const query = pick.apply(null, [renamedQuery, ...Object.values<string>(renameMap)]) as Partial<
Record<string, any>
>;
Expand Down
5 changes: 5 additions & 0 deletions src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,11 @@ export {
importSavedObjectsFromStream,
resolveSavedObjectsImportErrors,
SavedObjectsDeleteByWorkspaceOptions,
ACL,
Principals,
TransformedPermission,
PrincipalType,
Permissions,
} from './saved_objects';

export {
Expand Down
8 changes: 8 additions & 0 deletions src/core/server/saved_objects/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,11 @@ 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';
2 changes: 2 additions & 0 deletions src/core/server/saved_objects/service/lib/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,7 @@ export class SavedObjectsRepository {
filter,
preference,
workspaces,
ACLSearchParams,
} = options;

if (!type && !typeToNamespacesMap) {
Expand Down Expand Up @@ -897,6 +898,7 @@ export class SavedObjectsRepository {
hasReference,
kueryNode,
workspaces,
ACLSearchParams,
}),
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,79 @@ describe('#getQueryParams', () => {
});
});
});

describe('when using ACLSearchParams search', () => {
it('no ACLSearchParams provided', () => {
const result: Result = getQueryParams({
registry,
ACLSearchParams: {},
});
expect(result.query.bool.filter[1]).toEqual(undefined);
});

it('workspaces provided in ACLSearchParams', () => {
const result: Result = getQueryParams({
registry,
ACLSearchParams: {
workspaces: ['foo'],
},
});
expect(result.query.bool.filter[1]).toEqual({
bool: { should: [{ terms: { workspaces: ['foo'] } }] },
});
});

it('principals and permissionModes provided in ACLSearchParams', () => {
const result: Result = getQueryParams({
registry,
ACLSearchParams: {
principals: {
users: ['user-foo'],
groups: ['group-foo'],
},
permissionModes: ['read'],
},
});
expect(result.query.bool.filter[1]).toEqual({
bool: {
should: [
{
bool: {
filter: [
{
bool: {
should: [
{
terms: {
'permissions.read.users': ['user-foo'],
},
},
{
term: {
'permissions.read.users': '*',
},
},
{
terms: {
'permissions.read.groups': ['group-foo'],
},
},
{
term: {
'permissions.read.groups': '*',
},
},
],
},
},
],
},
},
],
},
});
});
});
});

describe('namespaces property', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ type KueryNode = any;

import { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry';
import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils';
import { SavedObjectsFindOptions } from '../../../types';
import { ACL } from '../../../permission_control/acl';

/**
* Gets the types based on the type. Uses mappings to support
Expand Down Expand Up @@ -166,6 +168,7 @@ interface QueryParams {
hasReference?: HasReferenceQueryParams;
kueryNode?: KueryNode;
workspaces?: string[];
ACLSearchParams?: SavedObjectsFindOptions['ACLSearchParams'];
}

export function getClauseForReference(reference: HasReferenceQueryParams) {
Expand Down Expand Up @@ -223,6 +226,7 @@ export function getQueryParams({
hasReference,
kueryNode,
workspaces,
ACLSearchParams,
}: QueryParams) {
const types = getTypes(
registry,
Expand Down Expand Up @@ -279,7 +283,59 @@ export function getQueryParams({
}
}

return { query: { bool } };
const result = { query: { bool } };

if (ACLSearchParams) {
const shouldClause: any = [];
if (ACLSearchParams.permissionModes?.length && ACLSearchParams.principals) {
const permissionDSL = ACL.generateGetPermittedSavedObjectsQueryDSL(
ACLSearchParams.permissionModes,
ACLSearchParams.principals
);
shouldClause.push(permissionDSL.query);
}

if (ACLSearchParams.workspaces?.length) {
shouldClause.push({
terms: {
workspaces: ACLSearchParams.workspaces,
},
});
}

if (shouldClause.length) {
bool.filter.push({
bool: {
should: [
/**
* Return those objects without workspaces field and permissions field to keep find find API backward compatible
*/
{
bool: {
must_not: [
{
exists: {
field: 'workspaces',
},
},
{
exists: {
field: 'permissions',
},
},
],
},
},
...shouldClause,
],
},
});
}

return result;
}

return result;
}

// we only want to add match_phrase_prefix clauses
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { IndexMapping } from '../../../mappings';
import { getQueryParams } from './query_params';
import { getSortingParams } from './sorting_params';
import { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry';
import { SavedObjectsFindOptions } from '../../../types';

type KueryNode = any;

Expand All @@ -53,6 +54,7 @@ interface GetSearchDslOptions {
};
kueryNode?: KueryNode;
workspaces?: string[];
ACLSearchParams?: SavedObjectsFindOptions['ACLSearchParams'];
}

export function getSearchDsl(
Expand All @@ -73,6 +75,7 @@ export function getSearchDsl(
hasReference,
kueryNode,
workspaces,
ACLSearchParams,
} = options;

if (!type) {
Expand All @@ -96,6 +99,7 @@ export function getSearchDsl(
hasReference,
kueryNode,
workspaces,
ACLSearchParams,
}),
...getSortingParams(mappings, type, sortField, sortOrder),
};
Expand Down
9 changes: 9 additions & 0 deletions src/core/server/saved_objects/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export {
} from './import/types';

import { SavedObject } from '../../types';
import { Principals } from './permission_control/acl';

type KueryNode = any;

Expand Down Expand Up @@ -112,6 +113,14 @@ export interface SavedObjectsFindOptions {
preference?: string;
/** If specified, will only retrieve objects that are in the workspaces */
workspaces?: string[];
/**
* The params here will be combined with bool clause and is used for filtering with ACL structure.
*/
ACLSearchParams?: {
workspaces?: string[];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot really remember the difference of ACLSearchParams.workspaces vs SavedObjectsFindOptions.workspaces, could you help me to recall? :D

Copy link
Collaborator

@wanglam wanglam Mar 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the code, the SavedObjectsFindOptions.workspaces support pass * to match workspaces. The ACLSearchParams.workspaces doesn't support that. @SuZhou-Joe Could we remove the ACLSearchParams.workspaces? It's a little bit confused.

The SavedObjectsFindOptions.workspaces support * to match all workspaces. It's using 'and' operator when generate search dsl. Here is the code.
The ACLSearchParams.workspaces doesn't support * to match all workspaces. It's using 'or' operator with permission modes and principals, which means used to find all permitted saved objects.
They are the differences in the repository part.
In the WorkspaceSavedObjectClientWrapper, the ACLSearchParams.workspaces will be set to all permitted workspaces, if no workspaces passed. It will always return saved objects inner permitted saved objects.

principals?: Principals;
permissionModes?: string[];
};
}

/**
Expand Down
7 changes: 7 additions & 0 deletions src/plugins/workspace/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,10 @@ export const WORKSPACE_FATAL_ERROR_APP_ID = 'workspace_fatal_error';
export const WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID = 'workspace';
export const WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID =
'workspace_conflict_control';

export enum WorkspacePermissionMode {
Read = 'read',
Write = 'write',
LibraryRead = 'library_read',
LibraryWrite = 'library_write',
}
5 changes: 4 additions & 1 deletion src/plugins/workspace/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import { schema, TypeOf } from '@osd/config-schema';

export const configSchema = schema.object({
enabled: schema.boolean({ defaultValue: false }),
permission: schema.object({
enabled: schema.boolean({ defaultValue: true }),
}),
});

export type ConfigSchema = TypeOf<typeof configSchema>;
export type WorkspacePluginConfigType = TypeOf<typeof configSchema>;
Loading
Loading