Skip to content

Commit

Permalink
Patch/acl (opensearch-project#231)
Browse files Browse the repository at this point in the history 
* consume permissions in repository

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

* feat: consume permissions in serializer

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

* Add unit tests for consuming permissions in repository

Signed-off-by: gaobinlong <gbinlong@amazon.com>

* feat: update

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

* fix: unit test

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

---------

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Signed-off-by: gaobinlong <gbinlong@amazon.com>
Co-authored-by: gaobinlong <gbinlong@amazon.com>
  • Loading branch information
@SuZhou-Joe @gaobinlong
SuZhou-Joe and gaobinlong authored Oct 16, 2023
1 parent 2c7aadf commit 120d3b7
Show file tree
Hide file tree
Showing 6 changed files with 192 additions and 23 deletions.
5 changes: 4 additions & 1 deletion src/core/server/saved_objects/serialization/serializer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export class SavedObjectsSerializer {
*/
public rawToSavedObject(doc: SavedObjectsRawDoc): SavedObjectSanitizedDoc {
const { _id, _source, _seq_no, _primary_term } = doc;
const { type, namespace, namespaces, originId, workspaces } = _source;
const { type, namespace, namespaces, originId, workspaces, permissions } = _source;

const version =
_seq_no != null || _primary_term != null
Expand All @@ -86,6 +86,7 @@ export class SavedObjectsSerializer {
...(namespace && this.registry.isSingleNamespace(type) && { namespace }),
...(namespaces && this.registry.isMultiNamespace(type) && { namespaces }),
...(originId && { originId }),
...(permissions && { permissions }),
attributes: _source[type],
references: _source.references || [],
...(_source.migrationVersion && { migrationVersion: _source.migrationVersion }),
Expand Down Expand Up @@ -114,6 +115,7 @@ export class SavedObjectsSerializer {
version,
references,
workspaces,
permissions,
} = savedObj;
const source = {
[type]: attributes,
Expand All @@ -125,6 +127,7 @@ export class SavedObjectsSerializer {
...(migrationVersion && { migrationVersion }),
...(updated_at && { updated_at }),
...(workspaces && { workspaces }),
...(permissions && { permissions }),
};

return {
Expand Down
2 changes: 2 additions & 0 deletions src/core/server/saved_objects/serialization/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
* under the License.
*/

import { Permissions } from '../permission_control/acl';
import { SavedObjectsMigrationVersion, SavedObjectReference } from '../types';

/**
Expand Down Expand Up @@ -71,6 +72,7 @@ interface SavedObjectDoc<T = unknown> {
updated_at?: string;
originId?: string;
workspaces?: string[];
permissions?: Permissions;
}

interface Referencable {
Expand Down
179 changes: 163 additions & 16 deletions src/core/server/saved_objects/service/lib/repository.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ describe('SavedObjectsRepository', () => {
});

const getMockGetResponse = (
{ type, id, references, namespace: objectNamespace, originId, workspaces },
{ type, id, references, namespace: objectNamespace, originId, workspaces, permissions },
namespace
) => {
const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace;
Expand All @@ -189,6 +189,7 @@ describe('SavedObjectsRepository', () => {
...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }),
workspaces,
...(originId && { originId }),
...(permissions && { permissions }),
type,
[type]: { title: 'Testing' },
references,
Expand Down Expand Up @@ -451,24 +452,35 @@ describe('SavedObjectsRepository', () => {
};
const namespace = 'foo-namespace';
const workspace = 'foo-workspace';
const permissions = {
read: {
users: ['user1'],
},
write: {
groups: ['groups1'],
},
};

const getMockBulkCreateResponse = (objects, namespace) => {
return {
items: objects.map(({ type, id, originId, attributes, references, migrationVersion }) => ({
create: {
_id: `${namespace ? `${namespace}:` : ''}${type}:${id}`,
_source: {
[type]: attributes,
type,
namespace,
...(originId && { originId }),
references,
...mockTimestampFields,
migrationVersion: migrationVersion || { [type]: '1.1.1' },
items: objects.map(
({ type, id, originId, attributes, references, migrationVersion, permissions }) => ({
create: {
_id: `${namespace ? `${namespace}:` : ''}${type}:${id}`,
_source: {
[type]: attributes,
type,
namespace,
...(originId && { originId }),
...(permissions && { permissions }),
references,
...mockTimestampFields,
migrationVersion: migrationVersion || { [type]: '1.1.1' },
},
...mockVersionProps,
},
...mockVersionProps,
},
})),
})
),
};
};

Expand Down Expand Up @@ -750,6 +762,18 @@ describe('SavedObjectsRepository', () => {
expect.anything()
);
});

it(`accepts permissions property when providing permissions info`, async () => {
const objects = [obj1, obj2].map((obj) => ({ ...obj, permissions: permissions }));
await bulkCreateSuccess(objects);
const expected = expect.objectContaining({ permissions });
const body = [expect.any(Object), expected, expect.any(Object), expected];
expect(client.bulk).toHaveBeenCalledWith(
expect.objectContaining({ body }),
expect.anything()
);
client.bulk.mockClear();
});
});

describe('errors', () => {
Expand Down Expand Up @@ -1088,6 +1112,17 @@ describe('SavedObjectsRepository', () => {
);
expect(result.saved_objects[1].id).toEqual(obj2.id);
});

it(`includes permissions property if present`, async () => {
const objects = [obj1, obj2].map((obj) => ({ ...obj, permissions: permissions }));
const result = await bulkCreateSuccess(objects);
expect(result).toEqual({
saved_objects: [
expect.objectContaining({ permissions }),
expect.objectContaining({ permissions }),
],
});
});
});
});

Expand Down Expand Up @@ -1307,6 +1342,22 @@ describe('SavedObjectsRepository', () => {
],
});
});

it(`includes permissions property if present`, async () => {
const permissions = {
read: {
users: ['user1'],
},
write: {
groups: ['groups1'],
},
};
const obj = { id: 'three', type: MULTI_NAMESPACE_TYPE, permissions: permissions };
const result = await bulkGetSuccess([obj]);
expect(result).toEqual({
saved_objects: [expect.objectContaining({ permissions: permissions })],
});
});
});
});

Expand All @@ -1324,6 +1375,14 @@ describe('SavedObjectsRepository', () => {
const references = [{ name: 'ref_0', type: 'test', id: '1' }];
const originId = 'some-origin-id';
const namespace = 'foo-namespace';
const permissions = {
read: {
users: ['user1'],
},
write: {
groups: ['groups1'],
},
};

const getMockBulkUpdateResponse = (objects, options, includeOriginId) => ({
items: objects.map(({ type, id }) => ({
Expand Down Expand Up @@ -1584,6 +1643,20 @@ describe('SavedObjectsRepository', () => {
await bulkUpdateSuccess([{ ..._obj2, namespace }]);
expectClientCallArgsAction([_obj2], { method: 'update', getId, overrides }, 2);
});

it(`accepts permissions property when providing permissions info`, async () => {
const objects = [obj1, obj2].map((obj) => ({ ...obj, permissions: permissions }));
await bulkUpdateSuccess(objects);
const doc = {
doc: expect.objectContaining({ permissions }),
};
const body = [expect.any(Object), doc, expect.any(Object), doc];
expect(client.bulk).toHaveBeenCalledWith(
expect.objectContaining({ body }),
expect.anything()
);
client.bulk.mockClear();
});
});

describe('errors', () => {
Expand Down Expand Up @@ -1776,6 +1849,14 @@ describe('SavedObjectsRepository', () => {
],
});
});

it(`includes permissions property if present`, async () => {
const obj = { type: MULTI_NAMESPACE_TYPE, id: 'three', permissions: permissions };
const result = await bulkUpdateSuccess([obj1, obj], {}, true);
expect(result).toEqual({
saved_objects: [expect.anything(), expect.objectContaining({ permissions })],
});
});
});
});

Expand Down Expand Up @@ -1965,6 +2046,14 @@ describe('SavedObjectsRepository', () => {
id: '123',
},
];
const permissions = {
read: {
users: ['user1'],
},
write: {
groups: ['groups1'],
},
};

const createSuccess = async (type, attributes, options) => {
const result = await savedObjectsRepository.create(type, attributes, options);
Expand Down Expand Up @@ -2193,6 +2282,16 @@ describe('SavedObjectsRepository', () => {
expect.anything()
);
});

it(`accepts permissions property`, async () => {
await createSuccess(type, attributes, { id, permissions });
expect(client.create).toHaveBeenCalledWith(
expect.objectContaining({
body: expect.objectContaining({ permissions }),
}),
expect.anything()
);
});
});

describe('errors', () => {
Expand Down Expand Up @@ -2288,6 +2387,11 @@ describe('SavedObjectsRepository', () => {
expect(serializer.savedObjectToRaw).toHaveBeenLastCalledWith(migratedDoc);
});

it(`adds permissions to body when providing permissions info`, async () => {
await createSuccess(type, attributes, { id, permissions });
expectMigrationArgs({ permissions });
});

it(`adds namespace to body when providing namespace for single-namespace type`, async () => {
await createSuccess(type, attributes, { id, namespace });
expectMigrationArgs({ namespace });
Expand Down Expand Up @@ -2334,11 +2438,13 @@ describe('SavedObjectsRepository', () => {
namespace,
references,
originId,
permissions,
});
expect(result).toEqual({
type,
id,
originId,
permissions,
...mockTimestampFields,
version: mockVersion,
attributes,
Expand Down Expand Up @@ -3208,14 +3314,15 @@ describe('SavedObjectsRepository', () => {
const namespace = 'foo-namespace';
const originId = 'some-origin-id';

const getSuccess = async (type, id, options, includeOriginId) => {
const getSuccess = async (type, id, options, includeOriginId, permissions) => {
const response = getMockGetResponse(
{
type,
id,
// "includeOriginId" is not an option for the operation; however, if the existing saved object contains an originId attribute, the
// 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 }),
},
options?.namespace
);
Expand Down Expand Up @@ -3366,6 +3473,21 @@ describe('SavedObjectsRepository', () => {
const result = await getSuccess(type, id, {}, true);
expect(result).toMatchObject({ originId });
});

it(`includes permissions property if present`, async () => {
const permissions = {
read: {
users: ['user1'],
},
write: {
groups: ['groups1'],
},
};
const result = await getSuccess(type, id, { namespace }, undefined, permissions);
expect(result).toMatchObject({
permissions: permissions,
});
});
});
});

Expand Down Expand Up @@ -3967,6 +4089,14 @@ describe('SavedObjectsRepository', () => {
},
];
const originId = 'some-origin-id';
const permissions = {
read: {
users: ['user1'],
},
write: {
groups: ['groups1'],
},
};

const updateSuccess = async (type, id, attributes, options, includeOriginId) => {
if (registry.isMultiNamespace(type)) {
Expand Down Expand Up @@ -4143,6 +4273,18 @@ describe('SavedObjectsRepository', () => {
expect.anything()
);
});

it(`accepts permissions when providing permissions info`, async () => {
await updateSuccess(type, id, attributes, { permissions });
const expected = expect.objectContaining({ permissions });
const body = {
doc: expected,
};
expect(client.update).toHaveBeenCalledWith(
expect.objectContaining({ body }),
expect.anything()
);
});
});

describe('errors', () => {
Expand Down Expand Up @@ -4237,6 +4379,11 @@ describe('SavedObjectsRepository', () => {
const result = await updateSuccess(type, id, attributes, {}, true);
expect(result).toMatchObject({ originId });
});

it(`includes permissions property if present`, async () => {
const result = await updateSuccess(type, id, attributes, { permissions });
expect(result).toMatchObject({ permissions });
});
});
});
});
Loading

0 comments on commit 120d3b7

Please sign in to comment.