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

[Workspace]Add permission control logic for workspace #6052

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
f00c8f7
Add permission control for workspace
wanglam Mar 6, 2024
9bdfbf7
Add changelog for permission control in workspace
wanglam Mar 6, 2024
bf9c5af
Fix integration tests and remove no need type
wanglam Mar 6, 2024
fbfe74a
Update permission enabled for workspace CRUD integration tests
wanglam Mar 6, 2024
e7279f9
Change back to config schema
wanglam Mar 6, 2024
7ba2d5e
feat: do not append workspaces field when no workspaces present (#6)
SuZhou-Joe Mar 7, 2024
b72b670
fix: authInfo destructure (#7)
SuZhou-Joe Mar 7, 2024
166e2c1
Fix permissions assign in attributes
wanglam Mar 7, 2024
284e10d
Remove deleteByWorkspace since not exists
wanglam Mar 7, 2024
f99ca19
Merge remote-tracking branch 'origin/main' into feat-add-permission-c…
wanglam Mar 7, 2024
dd120e3
refactor: remove formatWorkspacePermissionModeToStringArray
wanglam Mar 8, 2024
b46caf5
Remove current not used code
wanglam Mar 8, 2024
e6c83e7
Add missing unit tests for permission control
wanglam Mar 8, 2024
68969c3
Merge remote-tracking branch 'origin/main' into feat-add-permission-c…
wanglam Mar 10, 2024
2f37567
Update workspaces API test describe
wanglam Mar 10, 2024
87df794
Merge remote-tracking branch 'origin/main' into feat-add-permission-c…
wanglam Mar 11, 2024
131b0e5
Fix workspace CRUD API integration tests failed
wanglam Mar 11, 2024
3a1f63f
Address PR comments
wanglam Mar 12, 2024
6e7a463
Store permissions when savedObjects.permissions.enabled
wanglam Mar 12, 2024
78da4f5
Merge remote-tracking branch 'origin/main' into feat-add-permission-c…
wanglam Mar 12, 2024
5d7d64f
Add permission control for deleteByWorkspace
wanglam Mar 13, 2024
898ce8f
Merge remote-tracking branch 'origin/main' into feat-add-permission-c…
wanglam Mar 13, 2024
128bf18
Merge remote-tracking branch 'origin/main' into feat-add-permission-c…
wanglam Mar 14, 2024
ff66b7a
Update src/plugins/workspace/server/permission_control/client.ts
SuZhou-Joe Mar 15, 2024
81d6e28
Update src/plugins/workspace/server/permission_control/client.ts
SuZhou-Joe Mar 19, 2024
88d449a
Merge remote-tracking branch 'origin/main' into feat-add-permission-c…
wanglam Mar 19, 2024
26421b2
Merge remote-tracking branch 'origin/main' into feat-add-permission-c…
wanglam Mar 21, 2024
76523f5
Refactor permissions field in workspace create and update API
wanglam Mar 21, 2024
5854d93
Fix workspace CRUD API integration tests
wanglam Mar 21, 2024
479dad4
Merge branch 'main' into feat-add-permission-control-for-workspace
SuZhou-Joe Mar 26, 2024
94dafe5
Merge branch 'main' into feat-add-permission-control-for-workspace
SuZhou-Joe Mar 27, 2024
ee41fb5
Merge remote-tracking branch 'origin/main' into feat-add-permission-c…
wanglam Apr 2, 2024
e50b60c
Merge remote-tracking branch 'origin/main' into feat-add-permission-c…
wanglam Apr 3, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Workspace] Add workspace list page ([#6182](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6182))
- [Workspace] Add workspaces column to saved objects page ([#6225](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6225))
- [Multiple Datasource] Add multi data source support to sample vega visualizations ([#6218](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6218))
- [Workspace] Add permission control logic ([#6052](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6052))

### 🐛 Bug Fixes

Expand Down
6 changes: 5 additions & 1 deletion src/core/public/saved_objects/saved_objects_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ import { HttpFetchOptions, HttpSetup } from '../http';

type SavedObjectsFindOptions = Omit<
SavedObjectFindOptionsServer,
'sortOrder' | 'rootSearchFields' | 'typeToNamespacesMap'
| 'sortOrder'
| 'rootSearchFields'
| 'typeToNamespacesMap'
| 'ACLSearchParams'
Copy link
Member

Choose a reason for hiding this comment

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

There are two newly added: ACLSearchParams and workspacesSearchOperator. Can we make sure that if no feature flag is enabled, it doesn't break the current functionality?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I've tried in my local cluster. The ACLSearchParams only be used in the new added WorkspaceSavedObjectClientWrapper, it won't change any existing logic if these options not provided. If feature flag is disabled, the saved objects without workspaces and permissions still can be find and return.
The saved objects with workspaces or permissions property will using ACLSearch logic to find all saved objects. Here is the details:

  1. ACLSearchParams provided
    It will return all saved objects that match ACLSearchParams.principals and ACLSearchParams.permissionModes.

  2. ACLSearchParams provided and workspacesSearchOperator === "OR" and options.workspaces provided
    It will combine all saved objects that match ACLSearchParams and saved objects in the options.workspaces.

| 'workspacesSearchOperator'
>;

type PromiseType<T extends Promise<any>> = T extends Promise<infer U> ? U : never;
Expand Down
4 changes: 4 additions & 0 deletions src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,10 @@ export {
exportSavedObjectsToStream,
importSavedObjectsFromStream,
resolveSavedObjectsImportErrors,
ACL,
Principals,
PrincipalType,
Permissions,
SavedObjectsDeleteByWorkspaceOptions,
updateDataSourceNameInVegaSpec,
extractVegaSpecFromSavedObject,
Expand Down
3 changes: 3 additions & 0 deletions src/core/server/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ export function pluginInitializerContextConfigMock<T>(config: T) {
path: { data: '/tmp' },
savedObjects: {
maxImportPayloadBytes: new ByteSizeValue(26214400),
permission: {
enabled: true,
},
},
};

Expand Down
7 changes: 6 additions & 1 deletion src/core/server/plugins/plugin_context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
});
});

Expand Down
2 changes: 1 addition & 1 deletion src/core/server/plugins/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

/**
Expand Down
2 changes: 2 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,5 @@ export {

export { savedObjectsConfig, savedObjectsMigrationConfig } from './saved_objects_config';
export { SavedObjectTypeRegistry, ISavedObjectTypeRegistry } from './saved_objects_type_registry';

export { Permissions, ACL, Principals, PrincipalType } from './permission_control/acl';
14 changes: 12 additions & 2 deletions src/core/server/saved_objects/service/lib/repository.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ describe('SavedObjectsRepository', () => {
});

const getMockGetResponse = (
{ type, id, references, namespace: objectNamespace, originId, permissions },
{ type, id, references, namespace: objectNamespace, originId, permissions, workspaces },
namespace
) => {
const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace;
Expand All @@ -184,6 +184,7 @@ describe('SavedObjectsRepository', () => {
...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }),
...(originId && { originId }),
...(permissions && { permissions }),
...(workspaces && { workspaces }),
type,
[type]: { title: 'Testing' },
references,
Expand Down Expand Up @@ -3156,7 +3157,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,
Expand All @@ -3165,6 +3166,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
);
Expand Down Expand Up @@ -3330,6 +3332,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,
});
});
});
});

Expand Down
7 changes: 6 additions & 1 deletion src/core/server/saved_objects/service/lib/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,8 @@ export class SavedObjectsRepository {
filter,
preference,
workspaces,
workspacesSearchOperator,
ACLSearchParams,
} = options;

if (!type && !typeToNamespacesMap) {
Expand Down Expand Up @@ -873,6 +875,8 @@ export class SavedObjectsRepository {
hasReference,
kueryNode,
workspaces,
workspacesSearchOperator,
ACLSearchParams,
}),
},
};
Expand Down Expand Up @@ -1040,7 +1044,7 @@ export class SavedObjectsRepository {
throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
}

const { originId, updated_at: updatedAt, permissions } = body._source;
const { originId, updated_at: updatedAt, permissions, workspaces } = body._source;

let namespaces: string[] = [];
if (!this._registry.isNamespaceAgnostic(type)) {
Expand All @@ -1056,6 +1060,7 @@ export class SavedObjectsRepository {
...(originId && { originId }),
...(updatedAt && { updated_at: updatedAt }),
...(permissions && { permissions }),
...(workspaces && { workspaces }),
version: encodeHitVersion(body),
attributes: body._source[type],
references: body._source.references || [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,131 @@ 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('workspacesSearchOperator prvided as "OR"', () => {
const result: Result = getQueryParams({
registry,
workspaces: ['foo'],
workspacesSearchOperator: 'OR',
});
expect(result.query.bool.filter[1]).toEqual({
bool: {
should: [
{
bool: {
must_not: [
{
exists: {
field: 'workspaces',
},
},
{
exists: {
field: 'permissions',
},
},
],
},
},
{
bool: {
minimum_should_match: 1,
should: [
{
bool: {
must: [
{
term: {
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: {
must_not: [
{
exists: {
field: 'workspaces',
},
},
{
exists: {
field: 'permissions',
},
},
],
},
},
{
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
Loading
Loading