Skip to content

Commit 8e26bca

Browse files
wanglamSuZhou-Joe
wanglam
and
SuZhou-Joe
committed
[Workspace]Add permission control logic for workspace (opensearch-project#6052)
* Add permission control for workspace Signed-off-by: Lin Wang <[email protected]> * Add changelog for permission control in workspace Signed-off-by: Lin Wang <[email protected]> * Fix integration tests and remove no need type Signed-off-by: Lin Wang <[email protected]> * Update permission enabled for workspace CRUD integration tests Signed-off-by: Lin Wang <[email protected]> * Change back to config schema Signed-off-by: Lin Wang <[email protected]> * feat: do not append workspaces field when no workspaces present (#6) * feat: do not append workspaces field when no workspaces present Signed-off-by: SuZhou-Joe <[email protected]> * feat: do not append workspaces field when no workspaces present Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> * fix: authInfo destructure (#7) * fix: authInfo destructure Signed-off-by: SuZhou-Joe <[email protected]> * fix: unit test error Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> * Fix permissions assign in attributes Signed-off-by: Lin Wang <[email protected]> * Remove deleteByWorkspace since not exists Signed-off-by: Lin Wang <[email protected]> * refactor: remove formatWorkspacePermissionModeToStringArray Signed-off-by: Lin Wang <[email protected]> * Remove current not used code Signed-off-by: Lin Wang <[email protected]> * Add missing unit tests for permission control Signed-off-by: Lin Wang <[email protected]> * Update workspaces API test describe Signed-off-by: Lin Wang <[email protected]> * Fix workspace CRUD API integration tests failed Signed-off-by: Lin Wang <[email protected]> * Address PR comments Signed-off-by: Lin Wang <[email protected]> * Store permissions when savedObjects.permissions.enabled Signed-off-by: Lin Wang <[email protected]> * Add permission control for deleteByWorkspace Signed-off-by: Lin Wang <[email protected]> * Update src/plugins/workspace/server/permission_control/client.ts Signed-off-by: SuZhou-Joe <[email protected]> * Update src/plugins/workspace/server/permission_control/client.ts Signed-off-by: SuZhou-Joe <[email protected]> * Refactor permissions field in workspace create and update API Signed-off-by: Lin Wang <[email protected]> * Fix workspace CRUD API integration tests Signed-off-by: Lin Wang <[email protected]> --------- Signed-off-by: Lin Wang <[email protected]> Signed-off-by: SuZhou-Joe <[email protected]> Co-authored-by: SuZhou-Joe <[email protected]>
1 parent ded4759 commit 8e26bca

28 files changed

+2813
-37
lines changed

Diff for: ‎src/core/public/saved_objects/saved_objects_client.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ import { HttpFetchOptions, HttpSetup } from '../http';
4545

4646
type SavedObjectsFindOptions = Omit<
4747
SavedObjectFindOptionsServer,
48-
'sortOrder' | 'rootSearchFields' | 'typeToNamespacesMap'
48+
| 'sortOrder'
49+
| 'rootSearchFields'
50+
| 'typeToNamespacesMap'
51+
| 'ACLSearchParams'
52+
| 'workspacesSearchOperator'
4953
>;
5054

5155
type PromiseType<T extends Promise<any>> = T extends Promise<infer U> ? U : never;

Diff for: ‎src/core/server/index.ts

+4
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,10 @@ export {
322322
exportSavedObjectsToStream,
323323
importSavedObjectsFromStream,
324324
resolveSavedObjectsImportErrors,
325+
ACL,
326+
Principals,
327+
PrincipalType,
328+
Permissions,
325329
updateDataSourceNameInVegaSpec,
326330
extractVegaSpecFromSavedObject,
327331
} from './saved_objects';

Diff for: ‎src/core/server/mocks.ts

+3
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,9 @@ export function pluginInitializerContextConfigMock<T>(config: T) {
8989
path: { data: '/tmp' },
9090
savedObjects: {
9191
maxImportPayloadBytes: new ByteSizeValue(26214400),
92+
permission: {
93+
enabled: true,
94+
},
9295
},
9396
};
9497

Diff for: ‎src/core/server/plugins/plugin_context.test.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,12 @@ describe('createPluginInitializerContext', () => {
108108
pingTimeout: duration(30, 's'),
109109
},
110110
path: { data: fromRoot('data') },
111-
savedObjects: { maxImportPayloadBytes: new ByteSizeValue(26214400) },
111+
savedObjects: {
112+
maxImportPayloadBytes: new ByteSizeValue(26214400),
113+
permission: {
114+
enabled: false,
115+
},
116+
},
112117
});
113118
});
114119

Diff for: ‎src/core/server/plugins/types.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ export const SharedGlobalConfigKeys = {
295295
] as const,
296296
opensearch: ['shardTimeout', 'requestTimeout', 'pingTimeout'] as const,
297297
path: ['data'] as const,
298-
savedObjects: ['maxImportPayloadBytes'] as const,
298+
savedObjects: ['maxImportPayloadBytes', 'permission'] as const,
299299
};
300300

301301
/**

Diff for: ‎src/core/server/saved_objects/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,5 @@ export {
8484

8585
export { savedObjectsConfig, savedObjectsMigrationConfig } from './saved_objects_config';
8686
export { SavedObjectTypeRegistry, ISavedObjectTypeRegistry } from './saved_objects_type_registry';
87+
88+
export { Permissions, ACL, Principals, PrincipalType } from './permission_control/acl';

Diff for: ‎src/core/server/saved_objects/service/lib/repository.test.js

+12-2
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ describe('SavedObjectsRepository', () => {
168168
});
169169

170170
const getMockGetResponse = (
171-
{ type, id, references, namespace: objectNamespace, originId, permissions },
171+
{ type, id, references, namespace: objectNamespace, originId, permissions, workspaces },
172172
namespace
173173
) => {
174174
const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace;
@@ -184,6 +184,7 @@ describe('SavedObjectsRepository', () => {
184184
...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }),
185185
...(originId && { originId }),
186186
...(permissions && { permissions }),
187+
...(workspaces && { workspaces }),
187188
type,
188189
[type]: { title: 'Testing' },
189190
references,
@@ -3156,7 +3157,7 @@ describe('SavedObjectsRepository', () => {
31563157
const namespace = 'foo-namespace';
31573158
const originId = 'some-origin-id';
31583159

3159-
const getSuccess = async (type, id, options, includeOriginId, permissions) => {
3160+
const getSuccess = async (type, id, options, includeOriginId, permissions, workspaces) => {
31603161
const response = getMockGetResponse(
31613162
{
31623163
type,
@@ -3165,6 +3166,7 @@ describe('SavedObjectsRepository', () => {
31653166
// operation will return it in the result. This flag is just used for test purposes to modify the mock cluster call response.
31663167
...(includeOriginId && { originId }),
31673168
...(permissions && { permissions }),
3169+
...(workspaces && { workspaces }),
31683170
},
31693171
options?.namespace
31703172
);
@@ -3330,6 +3332,14 @@ describe('SavedObjectsRepository', () => {
33303332
permissions: permissions,
33313333
});
33323334
});
3335+
3336+
it(`includes workspaces property if present`, async () => {
3337+
const workspaces = ['workspace-1'];
3338+
const result = await getSuccess(type, id, { namespace }, undefined, undefined, workspaces);
3339+
expect(result).toMatchObject({
3340+
workspaces: workspaces,
3341+
});
3342+
});
33333343
});
33343344
});
33353345

Diff for: ‎src/core/server/saved_objects/service/lib/repository.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -799,6 +799,8 @@ export class SavedObjectsRepository {
799799
filter,
800800
preference,
801801
workspaces,
802+
workspacesSearchOperator,
803+
ACLSearchParams,
802804
} = options;
803805

804806
if (!type && !typeToNamespacesMap) {
@@ -873,6 +875,8 @@ export class SavedObjectsRepository {
873875
hasReference,
874876
kueryNode,
875877
workspaces,
878+
workspacesSearchOperator,
879+
ACLSearchParams,
876880
}),
877881
},
878882
};
@@ -1040,7 +1044,7 @@ export class SavedObjectsRepository {
10401044
throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
10411045
}
10421046

1043-
const { originId, updated_at: updatedAt, permissions } = body._source;
1047+
const { originId, updated_at: updatedAt, permissions, workspaces } = body._source;
10441048

10451049
let namespaces: string[] = [];
10461050
if (!this._registry.isNamespaceAgnostic(type)) {
@@ -1056,6 +1060,7 @@ export class SavedObjectsRepository {
10561060
...(originId && { originId }),
10571061
...(updatedAt && { updated_at: updatedAt }),
10581062
...(permissions && { permissions }),
1063+
...(workspaces && { workspaces }),
10591064
version: encodeHitVersion(body),
10601065
attributes: body._source[type],
10611066
references: body._source.references || [],

Diff for: ‎src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts

+125
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,131 @@ describe('#getQueryParams', () => {
646646
});
647647
});
648648
});
649+
650+
describe('when using ACLSearchParams search', () => {
651+
it('no ACLSearchParams provided', () => {
652+
const result: Result = getQueryParams({
653+
registry,
654+
ACLSearchParams: {},
655+
});
656+
expect(result.query.bool.filter[1]).toEqual(undefined);
657+
});
658+
659+
it('workspacesSearchOperator prvided as "OR"', () => {
660+
const result: Result = getQueryParams({
661+
registry,
662+
workspaces: ['foo'],
663+
workspacesSearchOperator: 'OR',
664+
});
665+
expect(result.query.bool.filter[1]).toEqual({
666+
bool: {
667+
should: [
668+
{
669+
bool: {
670+
must_not: [
671+
{
672+
exists: {
673+
field: 'workspaces',
674+
},
675+
},
676+
{
677+
exists: {
678+
field: 'permissions',
679+
},
680+
},
681+
],
682+
},
683+
},
684+
{
685+
bool: {
686+
minimum_should_match: 1,
687+
should: [
688+
{
689+
bool: {
690+
must: [
691+
{
692+
term: {
693+
workspaces: 'foo',
694+
},
695+
},
696+
],
697+
},
698+
},
699+
],
700+
},
701+
},
702+
],
703+
},
704+
});
705+
});
706+
707+
it('principals and permissionModes provided in ACLSearchParams', () => {
708+
const result: Result = getQueryParams({
709+
registry,
710+
ACLSearchParams: {
711+
principals: {
712+
users: ['user-foo'],
713+
groups: ['group-foo'],
714+
},
715+
permissionModes: ['read'],
716+
},
717+
});
718+
expect(result.query.bool.filter[1]).toEqual({
719+
bool: {
720+
should: [
721+
{
722+
bool: {
723+
must_not: [
724+
{
725+
exists: {
726+
field: 'workspaces',
727+
},
728+
},
729+
{
730+
exists: {
731+
field: 'permissions',
732+
},
733+
},
734+
],
735+
},
736+
},
737+
{
738+
bool: {
739+
filter: [
740+
{
741+
bool: {
742+
should: [
743+
{
744+
terms: {
745+
'permissions.read.users': ['user-foo'],
746+
},
747+
},
748+
{
749+
term: {
750+
'permissions.read.users': '*',
751+
},
752+
},
753+
{
754+
terms: {
755+
'permissions.read.groups': ['group-foo'],
756+
},
757+
},
758+
{
759+
term: {
760+
'permissions.read.groups': '*',
761+
},
762+
},
763+
],
764+
},
765+
},
766+
],
767+
},
768+
},
769+
],
770+
},
771+
});
772+
});
773+
});
649774
});
650775

651776
describe('namespaces property', () => {

Diff for: ‎src/core/server/saved_objects/service/lib/search_dsl/query_params.ts

+69-11
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ type KueryNode = any;
3434

3535
import { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry';
3636
import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils';
37+
import { SavedObjectsFindOptions } from '../../../types';
38+
import { ACL } from '../../../permission_control/acl';
3739

3840
/**
3941
* Gets the types based on the type. Uses mappings to support
@@ -166,6 +168,8 @@ interface QueryParams {
166168
hasReference?: HasReferenceQueryParams;
167169
kueryNode?: KueryNode;
168170
workspaces?: string[];
171+
workspacesSearchOperator?: 'AND' | 'OR';
172+
ACLSearchParams?: SavedObjectsFindOptions['ACLSearchParams'];
169173
}
170174

171175
export function getClauseForReference(reference: HasReferenceQueryParams) {
@@ -223,6 +227,8 @@ export function getQueryParams({
223227
hasReference,
224228
kueryNode,
225229
workspaces,
230+
workspacesSearchOperator = 'AND',
231+
ACLSearchParams,
226232
}: QueryParams) {
227233
const types = getTypes(
228234
registry,
@@ -247,17 +253,6 @@ export function getQueryParams({
247253
],
248254
};
249255

250-
if (workspaces) {
251-
bool.filter.push({
252-
bool: {
253-
should: workspaces.map((workspace) => {
254-
return getClauseForWorkspace(workspace);
255-
}),
256-
minimum_should_match: 1,
257-
},
258-
});
259-
}
260-
261256
if (search) {
262257
const useMatchPhrasePrefix = shouldUseMatchPhrasePrefix(search);
263258
const simpleQueryStringClause = getSimpleQueryStringClause({
@@ -279,6 +274,69 @@ export function getQueryParams({
279274
}
280275
}
281276

277+
const ACLSearchParamsShouldClause: any = [];
278+
279+
if (ACLSearchParams) {
280+
if (ACLSearchParams.permissionModes?.length && ACLSearchParams.principals) {
281+
const permissionDSL = ACL.generateGetPermittedSavedObjectsQueryDSL(
282+
ACLSearchParams.permissionModes,
283+
ACLSearchParams.principals
284+
);
285+
ACLSearchParamsShouldClause.push(permissionDSL.query);
286+
}
287+
}
288+
289+
if (workspaces?.length) {
290+
if (workspacesSearchOperator === 'OR') {
291+
ACLSearchParamsShouldClause.push({
292+
bool: {
293+
should: workspaces.map((workspace) => {
294+
return getClauseForWorkspace(workspace);
295+
}),
296+
minimum_should_match: 1,
297+
},
298+
});
299+
} else {
300+
bool.filter.push({
301+
bool: {
302+
should: workspaces.map((workspace) => {
303+
return getClauseForWorkspace(workspace);
304+
}),
305+
minimum_should_match: 1,
306+
},
307+
});
308+
}
309+
}
310+
311+
if (ACLSearchParamsShouldClause.length) {
312+
bool.filter.push({
313+
bool: {
314+
should: [
315+
/**
316+
* Return those objects without workspaces field and permissions field to keep find API backward compatible
317+
*/
318+
{
319+
bool: {
320+
must_not: [
321+
{
322+
exists: {
323+
field: 'workspaces',
324+
},
325+
},
326+
{
327+
exists: {
328+
field: 'permissions',
329+
},
330+
},
331+
],
332+
},
333+
},
334+
...ACLSearchParamsShouldClause,
335+
],
336+
},
337+
});
338+
}
339+
282340
return { query: { bool } };
283341
}
284342

0 commit comments

Comments
 (0)