Skip to content

Commit 58fa3de

Browse files
jeramysoucyazasypkin
jeramysoucy
and
azasypkin
committed
[Write restricted dashboards] Implements upsert for types supporting access control (elastic#247941)
Closes elastic#239686 ## Summary This PR implements the "upsert" case for types supporting access control in the Saved Objects update operation. The default access mode is always used during an upsert. The active user profile becomes the owner. If there is no active user profile, no access control metadata is saved during the upsert. ### Tests - x-pack/platform/test/spaces_api_integration/access_control_objects/apis/spaces/access_control_objects.ts - 'should apply defaults when upserting a supported type' - 'should not write access control metadata when upserting unsupported types' - 'should not write access control metadata when upserting a supported type if there is no active user profile ID' Note: "upserting" is not supported in bulk update. --------- Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com> (cherry picked from commit 7975d47)
1 parent 2c6aebe commit 58fa3de

File tree

4 files changed

+234
-9
lines changed

4 files changed

+234
-9
lines changed

‎src/core/packages/saved-objects/api-server-internal/src/lib/apis/update.test.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,10 @@ import {
4747
createGenericNotFoundErrorPayload,
4848
updateSuccess,
4949
mockTimestampFieldsWithCreated,
50+
ACCESS_CONTROL_TYPE,
51+
MULTI_NAMESPACE_TYPE,
5052
} from '../../test_helpers/repository.test.common';
53+
import { mockAuthenticatedUser } from '@kbn/core-security-common/mocks';
5154

5255
describe('#update', () => {
5356
let client: ReturnType<typeof elasticsearchClientMock.createElasticsearchClient>;
@@ -834,6 +837,127 @@ describe('#update', () => {
834837
})
835838
);
836839
});
840+
841+
describe('access control', () => {
842+
it('should define access control metadata when upserting a supporting type', async () => {
843+
securityExtension.getCurrentUser.mockReturnValue(
844+
mockAuthenticatedUser({ profile_uid: 'u_test_user_version' })
845+
);
846+
847+
const options = { upsert: { title: 'foo', description: 'bar' } };
848+
migrator.migrateDocument.mockImplementationOnce((doc) => ({ ...doc }));
849+
await updateSuccess(
850+
client,
851+
repository,
852+
registry,
853+
ACCESS_CONTROL_TYPE,
854+
id,
855+
attributes,
856+
{
857+
upsert: {
858+
title: 'foo',
859+
description: 'bar',
860+
},
861+
},
862+
{
863+
mockGetResponseAsNotFound: { found: false } as estypes.GetResponse,
864+
}
865+
);
866+
await repository.update(ACCESS_CONTROL_TYPE, id, attributes, options);
867+
expect(client.get).toHaveBeenCalledTimes(2);
868+
const expectedType = {
869+
accessControlType: { description: 'bar', title: 'foo' },
870+
namespaces: ['default'],
871+
type: 'accessControlType',
872+
accessControl: {
873+
accessMode: 'default',
874+
owner: 'u_test_user_version',
875+
},
876+
created_by: 'u_test_user_version',
877+
updated_by: 'u_test_user_version',
878+
...mockTimestampFieldsWithCreated,
879+
};
880+
expect(
881+
(client.create.mock.calls[0][0] as estypes.CreateRequest<SavedObjectsRawDocSource>)
882+
.document!
883+
).toEqual(expectedType);
884+
});
885+
886+
it('should not define access control metadata when upserting a supporting type but there is no active user profile', async () => {
887+
securityExtension.getCurrentUser.mockReturnValue(null);
888+
const options = { upsert: { title: 'foo', description: 'bar' } };
889+
migrator.migrateDocument.mockImplementationOnce((doc) => ({ ...doc }));
890+
await updateSuccess(
891+
client,
892+
repository,
893+
registry,
894+
ACCESS_CONTROL_TYPE,
895+
id,
896+
attributes,
897+
{
898+
upsert: {
899+
title: 'foo',
900+
description: 'bar',
901+
},
902+
},
903+
{
904+
mockGetResponseAsNotFound: { found: false } as estypes.GetResponse,
905+
}
906+
);
907+
await repository.update(ACCESS_CONTROL_TYPE, id, attributes, options);
908+
expect(client.get).toHaveBeenCalledTimes(2);
909+
const expectedType = {
910+
accessControlType: { description: 'bar', title: 'foo' },
911+
namespaces: ['default'],
912+
type: 'accessControlType',
913+
...mockTimestampFieldsWithCreated,
914+
};
915+
expect(
916+
(client.create.mock.calls[0][0] as estypes.CreateRequest<SavedObjectsRawDocSource>)
917+
.document!
918+
).toEqual(expectedType);
919+
});
920+
921+
it('should not define access control metadata when upserting a non-supporting type', async () => {
922+
securityExtension.getCurrentUser.mockReturnValue(
923+
mockAuthenticatedUser({ profile_uid: 'u_test_user_version' })
924+
);
925+
926+
const options = { upsert: { title: 'foo', description: 'bar' } };
927+
migrator.migrateDocument.mockImplementationOnce((doc) => ({ ...doc }));
928+
await updateSuccess(
929+
client,
930+
repository,
931+
registry,
932+
MULTI_NAMESPACE_TYPE,
933+
id,
934+
attributes,
935+
{
936+
upsert: {
937+
title: 'foo',
938+
description: 'bar',
939+
},
940+
},
941+
{
942+
mockGetResponseAsNotFound: { found: false } as estypes.GetResponse,
943+
}
944+
);
945+
await repository.update(MULTI_NAMESPACE_TYPE, id, attributes, options);
946+
expect(client.get).toHaveBeenCalledTimes(2);
947+
const expectedType = {
948+
multiNamespaceType: { description: 'bar', title: 'foo' },
949+
namespaces: ['default'],
950+
type: 'multiNamespaceType',
951+
created_by: 'u_test_user_version',
952+
updated_by: 'u_test_user_version',
953+
...mockTimestampFieldsWithCreated,
954+
};
955+
expect(
956+
(client.create.mock.calls[0][0] as estypes.CreateRequest<SavedObjectsRawDocSource>)
957+
.document!
958+
).toEqual(expectedType);
959+
});
960+
});
837961
});
838962
});
839963
});

‎src/core/packages/saved-objects/api-server-internal/src/lib/apis/update.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
encodeHitVersion,
1919
} from '@kbn/core-saved-objects-base-server-internal';
2020
import type {
21+
SavedObjectAccessControl,
2122
SavedObjectsUpdateOptions,
2223
SavedObjectsUpdateResponse,
2324
} from '@kbn/core-saved-objects-api-server';
@@ -27,6 +28,7 @@ import { DEFAULT_REFRESH_SETTING, DEFAULT_RETRY_COUNT } from '../constants';
2728
import { isValidRequest } from '../utils';
2829
import { getCurrentTime, getSavedObjectFromSource, mergeForUpdate } from './utils';
2930
import type { ApiExecutionContext } from './types';
31+
import { setAccessControl } from './utils/internal_utils';
3032

3133
export interface PerformUpdateParams<T = unknown> {
3234
type: string;
@@ -182,13 +184,18 @@ export const executeUpdate = async <T>(
182184

183185
// UPSERT CASE START
184186
if (shouldPerformUpsert) {
185-
// Note: Upsert does not support adding accessControl properties. To do so, the `create` API should be used.
186-
187-
if (registry.supportsAccessControl(type)) {
188-
throw SavedObjectsErrorHelpers.createBadRequestError(
189-
`"update" does not support "upsert" of objects that support access control. Use "create" or "bulk create" instead.`
190-
);
187+
// Note: Update does not support accessControl parameters. If applicable and possible (type supports access control
188+
// and there is an active user profile), the default access control metadata will be set.
189+
// To explicitly set access control for a new object, the `create` API should be used.
190+
let accessControlToWrite: SavedObjectAccessControl | undefined;
191+
if (securityExtension) {
192+
accessControlToWrite = setAccessControl({
193+
typeSupportsAccessControl: registry.supportsAccessControl(type),
194+
createdBy: updatedBy,
195+
accessMode: 'default',
196+
});
191197
}
198+
192199
// ignore attributes if creating a new doc: only use the upsert attributes
193200
// don't include upsert if the object already exists; ES doesn't allow upsert in combination with version properties
194201
const migratedUpsert = migrationHelper.migrateInputDocument({
@@ -203,6 +210,7 @@ export const executeUpdate = async <T>(
203210
updated_at: time,
204211
...(updatedBy && { created_by: updatedBy, updated_by: updatedBy }),
205212
...(Array.isArray(references) && { references }),
213+
...(accessControlToWrite && { accessControl: accessControlToWrite }),
206214
}) as SavedObjectSanitizedDoc<T>;
207215
validationHelper.validateObjectForCreate(type, migratedUpsert);
208216
const rawUpsert = serializer.savedObjectToRaw(migratedUpsert);

‎x-pack/platform/test/spaces_api_integration/access_control_objects/apis/spaces/access_control_objects.ts‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,94 @@ export default function ({ getService }: FtrProviderContext) {
10541054
expect(updateResponse.body).to.have.property('message');
10551055
expect(updateResponse.body.message).to.contain(`Unable to update ${ACCESS_CONTROL_TYPE}`);
10561056
});
1057+
1058+
it('should apply defaults when upserting a supported type', async () => {
1059+
const { cookie: objectOwnerCookie, profileUid: ownerProfileUid } = await loginAsObjectOwner(
1060+
'test_user',
1061+
'changeme'
1062+
);
1063+
1064+
const objectId = 'upserted-object-1';
1065+
const updateResponse = await supertestWithoutAuth
1066+
.put('/access_control_objects/update')
1067+
.set('kbn-xsrf', 'true')
1068+
.set('cookie', objectOwnerCookie.cookieString())
1069+
.send({ objectId, type: ACCESS_CONTROL_TYPE, upsert: true })
1070+
.expect(200);
1071+
1072+
expect(updateResponse.body.id).to.eql(objectId);
1073+
expect(updateResponse.body.attributes).to.have.property(
1074+
'description',
1075+
'updated description'
1076+
);
1077+
// get the object to verify access control metadata
1078+
const getResponse = await supertestWithoutAuth
1079+
.get(`/access_control_objects/${objectId}`)
1080+
.set('kbn-xsrf', 'true')
1081+
.set('cookie', objectOwnerCookie.cookieString())
1082+
.expect(200);
1083+
expect(getResponse.body).to.have.property('accessControl');
1084+
expect(getResponse.body.accessControl).to.have.property('owner', ownerProfileUid);
1085+
expect(getResponse.body.accessControl).to.have.property('accessMode', 'default');
1086+
});
1087+
1088+
it('should not write access control metadata when upserting unsupported types', async () => {
1089+
const { cookie: objectOwnerCookie } = await loginAsObjectOwner('test_user', 'changeme');
1090+
1091+
const objectId = 'upserted-object-2';
1092+
const updateResponse = await supertestWithoutAuth
1093+
.put('/access_control_objects/update')
1094+
.set('kbn-xsrf', 'true')
1095+
.set('cookie', objectOwnerCookie.cookieString())
1096+
.send({ objectId, type: NON_ACCESS_CONTROL_TYPE, upsert: true })
1097+
.expect(200);
1098+
1099+
expect(updateResponse.body.id).to.eql(objectId);
1100+
expect(updateResponse.body.attributes).to.have.property(
1101+
'description',
1102+
'updated description'
1103+
);
1104+
// get the object to verify access control metadata
1105+
const getResponse = await supertestWithoutAuth
1106+
.get(`/non_access_control_objects/${objectId}`)
1107+
.set('kbn-xsrf', 'true')
1108+
.set('cookie', objectOwnerCookie.cookieString())
1109+
.expect(200);
1110+
expect(getResponse.body).not.to.have.property('accessControl');
1111+
});
1112+
1113+
it('should not write access control metadata when upserting a supported type if there is no active user profile ID', async () => {
1114+
const objectId = 'upserted-object-3';
1115+
const updateResponse = await supertestWithoutAuth
1116+
.put('/access_control_objects/update')
1117+
.set('kbn-xsrf', 'true')
1118+
.set(
1119+
'Authorization',
1120+
`Basic ${Buffer.from(`${adminTestUser.username}:${adminTestUser.password}`).toString(
1121+
'base64'
1122+
)}`
1123+
)
1124+
.send({ objectId, type: ACCESS_CONTROL_TYPE, upsert: true })
1125+
.expect(200);
1126+
1127+
expect(updateResponse.body.id).to.eql(objectId);
1128+
expect(updateResponse.body.attributes).to.have.property(
1129+
'description',
1130+
'updated description'
1131+
);
1132+
// get the object to verify access control metadata
1133+
const getResponse = await supertestWithoutAuth
1134+
.get(`/access_control_objects/${objectId}`)
1135+
.set('kbn-xsrf', 'true')
1136+
.set(
1137+
'Authorization',
1138+
`Basic ${Buffer.from(`${adminTestUser.username}:${adminTestUser.password}`).toString(
1139+
'base64'
1140+
)}`
1141+
)
1142+
.expect(200);
1143+
expect(getResponse.body).not.to.have.property('accessControl');
1144+
});
10571145
});
10581146

10591147
describe('#bulk_update', () => {

‎x-pack/platform/test/spaces_api_integration/common/plugins/access_control_test_plugin/server/plugin.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,16 +254,21 @@ export class AccessControlTestPlugin implements Plugin {
254254
schema.literal(NON_ACCESS_CONTROL_TYPE),
255255
]),
256256
objectId: schema.string(),
257+
upsert: schema.boolean({ defaultValue: false }),
257258
}),
258259
},
259260
},
260261
async (context, request, response) => {
261262
const soClient = (await context.core).savedObjects.client;
262263
try {
263264
const objectType = request.body.type || ACCESS_CONTROL_TYPE;
264-
const result = await soClient.update(objectType, request.body.objectId, {
265-
description: 'updated description',
266-
});
265+
const upsert = request.body.upsert ?? false;
266+
const result = await soClient.update(
267+
objectType,
268+
request.body.objectId,
269+
{ description: 'updated description' },
270+
{ upsert: upsert ? { description: 'updated description' } : undefined }
271+
);
267272

268273
return response.ok({
269274
body: result,

0 commit comments

Comments
 (0)