Skip to content

Commit

Permalink
[Workspace]Add permission control logic for workspace (opensearch-pro…
Browse files Browse the repository at this point in the history
…ject#6052) (opensearch-project#6531)

* Add permission control for workspace



* Add changelog for permission control in workspace



* Fix integration tests and remove no need type



* Update permission enabled for workspace CRUD integration tests



* Change back to config schema



* feat: do not append workspaces field when no workspaces present (#6)

* feat: do not append workspaces field when no workspaces present



* feat: do not append workspaces field when no workspaces present



---------



* fix: authInfo destructure (#7)

* fix: authInfo destructure



* fix: unit test error



---------



* Fix permissions assign in attributes



* Remove deleteByWorkspace since not exists



* refactor: remove formatWorkspacePermissionModeToStringArray



* Remove current not used code



* Add missing unit tests for permission control



* Update workspaces API test describe



* Fix workspace CRUD API integration tests failed



* Address PR comments



* Store permissions when savedObjects.permissions.enabled



* Add permission control for deleteByWorkspace



* Update src/plugins/workspace/server/permission_control/client.ts



* Update src/plugins/workspace/server/permission_control/client.ts



* Refactor permissions field in workspace create and update API



* Fix workspace CRUD API integration tests



---------

Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Co-authored-by: SuZhou-Joe <suzhou@amazon.com>
Co-authored-by: ZilongX <99905560+ZilongX@users.noreply.github.com>
  • Loading branch information
3 people committed Apr 19, 2024
1 parent 88c5dfa commit 6d881e0
Show file tree
Hide file tree
Showing 28 changed files with 2,813 additions and 37 deletions.
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'
| '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 @@ -322,6 +322,10 @@ export {
exportSavedObjectsToStream,
importSavedObjectsFromStream,
resolveSavedObjectsImportErrors,
ACL,
Principals,
PrincipalType,
Permissions,
updateDataSourceNameInVegaSpec,
extractVegaSpecFromSavedObject,
} from './saved_objects';
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
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,8 @@ interface QueryParams {
hasReference?: HasReferenceQueryParams;
kueryNode?: KueryNode;
workspaces?: string[];
workspacesSearchOperator?: 'AND' | 'OR';
ACLSearchParams?: SavedObjectsFindOptions['ACLSearchParams'];
}

export function getClauseForReference(reference: HasReferenceQueryParams) {
Expand Down Expand Up @@ -223,6 +227,8 @@ export function getQueryParams({
hasReference,
kueryNode,
workspaces,
workspacesSearchOperator = 'AND',
ACLSearchParams,
}: QueryParams) {
const types = getTypes(
registry,
Expand All @@ -247,17 +253,6 @@ export function getQueryParams({
],
};

if (workspaces) {
bool.filter.push({
bool: {
should: workspaces.map((workspace) => {
return getClauseForWorkspace(workspace);
}),
minimum_should_match: 1,
},
});
}

if (search) {
const useMatchPhrasePrefix = shouldUseMatchPhrasePrefix(search);
const simpleQueryStringClause = getSimpleQueryStringClause({
Expand All @@ -279,6 +274,69 @@ export function getQueryParams({
}
}

const ACLSearchParamsShouldClause: any = [];

if (ACLSearchParams) {
if (ACLSearchParams.permissionModes?.length && ACLSearchParams.principals) {
const permissionDSL = ACL.generateGetPermittedSavedObjectsQueryDSL(
ACLSearchParams.permissionModes,
ACLSearchParams.principals
);
ACLSearchParamsShouldClause.push(permissionDSL.query);
}
}

if (workspaces?.length) {
if (workspacesSearchOperator === 'OR') {
ACLSearchParamsShouldClause.push({
bool: {
should: workspaces.map((workspace) => {
return getClauseForWorkspace(workspace);
}),
minimum_should_match: 1,
},
});
} else {
bool.filter.push({
bool: {
should: workspaces.map((workspace) => {
return getClauseForWorkspace(workspace);
}),
minimum_should_match: 1,
},
});
}
}

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

return { query: { bool } };
}

Expand Down
Loading

0 comments on commit 6d881e0

Please sign in to comment.