Skip to content

Commit 27c9862

Browse files
SiddharthMantrielasticmachineCopilotkibanamachineazasypkin
authored
[Write restricted SOs] Allow creation, update, delete and transfer ownership (elastic#224411)
Closes elastic#221753 ---- > [!IMPORTANT] > To support requirements, this feature is restricted to SO types that are `multiple` and `multiple-isolated` namespace types only. ---- ## Summary Summarize your PR. If it involves visual changes include a screenshot or gif. - **Change Ownership Endpoints**: Adds server-side logic, client methods, and repository support to transfer object ownership. - **Extended Create Options** : Allow specifying `accessMode: 'read_only'` when creating an object. - **Security & Authorization**: Integrates with the security extension to enforce owner-based restrictions. - **Mappings & Migrations**: Elasticsearch mappings and migrations updated to persist `accessControl`. - **Tests**: New FTR integration tests and a dedicated test plugin for read-only object scenarios. ## Ownership-Based Authorization Logic All ownership checks are now centralized in `SavedObjectsAccessControlService.enforceAccessControl(operation, args…)`. The general flow is: 1. **Load existing object** to read its `accessControl` metadata. 2. **Determine required privilege** based on the operation: - `create` - `read` - `modify` (covers both update & delete) - `changeOwnership` 3. **If** `object.accessControl.accessMode === 'read_only'`, **also enforce** `user === object.accessControl.owner`. 4. **Always** verify the caller holds the corresponding privilege. | Operation | Privilege Key | Read-Only Constraint | Notes | |-----------------------|-----------------------|---------------------------------|----------------------------------------------------------| | **Create** | `create` | Must supply `accessControl.owner` | `accessControl.accessMode` passed in create options | | **Read** | `read` | N/A | Falls back to standard read privileges | | **Modify** (update/delete) | `modify` | Caller must be the current owner or admin | Single hook for both update & delete | | **Change Ownership** | `changeOwnership` | Caller must be the current owner or admin| Performs a preflight multi-get, then bulk-updates owner based on result | ## Assumptions 1. Update operations will not support changing accessControl metadata. 2. Updates (and in turn upserts) will also not allow objects to be created as write restricted types. We throw an error and ask the consumer to use `create` instead. 3. Bulk Delete operations will behave as expected with the new changes. Superusers/owners can bulk delete objects. Non-owners cannot bulk delete objects that are in write restricted mode. (Assuming that they otherwise have the permission to do so) 4. Bulk delete with `force` option works the same way. ## Expected behavior of SOR operations All operations in the SOR on accessControl types follow the same rules. After the existing RBAC rules for these operations, for a user to be granted permission to perform an action on an SO Type supporting access control, they must be superuser or owner of the object. 1. Create Objects supporting access control will be created with the accessControl metadata as follows: `owner`: User profile ID of creating user `accessMode`: `read_only` | `default` based on the incoming option If objects supporting access control are created with no access control metadata, then they are created just as they are today - no change to how that object type will function. 2. Read No changes to read. Must have RBAC granted access for SOs. 3. Update Update operations will not support changing accessControl metadata. Updates (and in turn upserts) will also not allow objects to be created as write restricted types. We throw an error and ask the consumer to use `create` instead. Updates are restricted by the access control rules stated above 4. Delete Delete operations will be allowed/disallowed based on the rules above. 6. Bulk update Objects supporting access control but not owned by the current user (unless superuser) will not be updated in this operation. 7. Bulk delete Bulk delete (with and without the force option) work the same way. If an object in the payload is of a type supporting access control, then the rules will be the same. We will allow delete only if the user is a superuser or owner of said object - all other instances will be rejected. 8. Delete by namespace Delete by namespace is a workaround for bulk deleting all objects in a space. This function isn't exposed on the SO client but rather used on the repository which is used by the SpacesClient to be able to delete _all_ objects in a space. For users with a role granting this permission, we will continue to allow them to do so even if there are objects owned by others in that space. This is product decision based on the Editor role being granted this privilege and if removed, would be considered a breaking change that's out of the scope of this project. Misc: Saved objects like dashboards that are shareable across spaces have the same rules assigned to them. A user cannot update/delete a dashboard in a space where they might have RBAC access but don't own said dashboard. ## How to test Since there's no associated UI changes in this PR, you can use the test `read only objects` plugin to test behavior of the different flows. Start the plugin in the integration test as: ``` node scripts/functional_tests_server --config x-pack/platform/test/spaces_api_integration/read_only_objects/config.ts ``` Once done, you can open Kibana on `localhost:5620` and set up a simple user with this role: `kibana_savedobjects_editor`. For your tests you can use three users: 1. elastic:changeme `superuser` 2. test_user:changeme `non-superuser` 3. The user just created above with role `kibana_savedobjects_editor` With each of these users, you can try the following calls in Dev Tools 1. Create a read only object / non-read only ``` POST kbn:/read_only_objects/create {"isReadOnly": true} // switch to false if creating regular SO type ``` Creates and responds with an object owned by the creating user. Using the ID in the response, you can now perform the different operations and verify how access control works You can run ``` GET kbn:/read_only_objects/<ID> ``` at any time to see the current shape of the object including the access control metadata 2. Updates - Run this as admin/owner/non-owner to test and verify the different responses ``` PUT kbn:/read_only_objects/update { "objectId": "<ID FROM CREATE>", "type": "read_only_type" } ``` 3. Delete - Run this as admin/owner/non-owner to test and verify the different responses ``` DELETE kbn:/read_only_objects/<ID FROM CREATE> ``` 4. Bulk delete - Run this as admin/owner/non-owner to test and verify the different responses ``` POST kbn:/read_only_objects/bulk_delete { "objects": [ { "id": "<ID1>", "type": "read_only_type" }, { "id": "<ID2>", "type": "read_only_type" }, // ... ] } ``` 5. Bulk delete with force - same as above with force option ``` POST kbn:/read_only_objects/bulk_delete { "force": true, "objects": [ { "id": "<ID1>", "type": "read_only_type" }, { "id": "<ID2>", "type": "read_only_type" }, // ... ] } ``` ## Note for reviewers: - There are a lot of changes owned by the core team. Although most of the authz and ownership changes are in the security extension owned by Kibana platform security, surfacing these APIs to the SO client touches a lot of interfaces owned by core. ### Follow up tasks We have a few different follow up tasks documented on elastic#230991 ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Risk matrix attached on parent PR: elastic#224552 (comment) --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Aleh Zasypkin <[email protected]> Co-authored-by: Christiane (Tina) Heiligers <[email protected]> Co-authored-by: “jeramysoucy” <[email protected]>
1 parent 3bacb47 commit 27c9862

File tree

67 files changed

+5112
-133
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

67 files changed

+5112
-133
lines changed

.buildkite/ftr_platform_stateful_configs.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,7 @@ enabled:
334334
- x-pack/platform/test/spaces_api_integration/security_and_spaces/config_basic.ts
335335
- x-pack/platform/test/spaces_api_integration/security_and_spaces/config_trial.ts
336336
- x-pack/platform/test/spaces_api_integration/spaces_only/config.ts
337+
- x-pack/platform/test/spaces_api_integration/read_only_objects/config.ts
337338
- x-pack/platform/test/task_manager_claimer_update_by_query/config.ts
338339
- x-pack/platform/test/ui_capabilities/security_and_spaces/config.ts
339340
- x-pack/platform/test/ui_capabilities/spaces_only/config.ts

.github/CODEOWNERS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,6 +1081,7 @@ x-pack/platform/test/security_api_integration/plugins/oidc_provider @elastic/kib
10811081
x-pack/platform/test/security_api_integration/plugins/saml_provider @elastic/kibana-security
10821082
x-pack/platform/test/security_api_integration/plugins/user_profiles_consumer @elastic/kibana-security
10831083
x-pack/platform/test/security_functional/plugins/test_endpoints @elastic/kibana-security
1084+
x-pack/platform/test/spaces_api_integration/common/plugins/read_only_objects_test_plugin @elastic/kibana-security
10841085
x-pack/platform/test/spaces_api_integration/common/plugins/spaces_test_plugin @elastic/kibana-security
10851086
x-pack/platform/test/task_manager_claimer_update_by_query/plugins/sample_task_plugin_mget @elastic/response-ops
10861087
x-pack/platform/test/ui_capabilities/common/plugins/foo_plugin @elastic/kibana-security

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,7 @@
817817
"@kbn/react-kibana-context-theme": "link:src/platform/packages/shared/react/kibana_context/theme",
818818
"@kbn/react-kibana-mount": "link:src/platform/packages/shared/react/kibana_mount",
819819
"@kbn/react-mute-legacy-root-warning": "link:src/platform/packages/private/kbn-react-mute-legacy-root-warning",
820+
"@kbn/read-only-objects-test-plugin": "link:x-pack/platform/test/spaces_api_integration/common/plugins/read_only_objects_test_plugin",
820821
"@kbn/recently-accessed": "link:src/platform/packages/shared/kbn-recently-accessed",
821822
"@kbn/reindex-service-plugin": "link:x-pack/platform/plugins/private/reindex_service",
822823
"@kbn/remote-clusters-plugin": "link:x-pack/platform/plugins/private/remote_clusters",

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

Lines changed: 243 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ import {
1919

2020
import type { Payload } from '@hapi/boom';
2121

22-
import type { SavedObjectsBulkCreateObject } from '@kbn/core-saved-objects-api-server';
22+
import type {
23+
SavedObjectAccessControl,
24+
SavedObjectsBulkCreateObject,
25+
} from '@kbn/core-saved-objects-api-server';
2326
import {
2427
type SavedObjectsRawDoc,
2528
type SavedObjectUnsanitizedDoc,
@@ -58,6 +61,7 @@ import {
5861
} from '../../test_helpers/repository.test.common';
5962
import type { ISavedObjectsSecurityExtension } from '@kbn/core-saved-objects-server';
6063
import { savedObjectsExtensionsMock } from '../../mocks/saved_objects_extensions.mock';
64+
import type { AuthenticatedUser } from '@kbn/core-security-common';
6165

6266
// so any breaking changes to this repository are considered breaking changes to the SavedObjectsClient.
6367

@@ -1086,6 +1090,244 @@ describe('#bulkCreate', () => {
10861090
})
10871091
);
10881092
});
1093+
1094+
describe('access control', () => {
1095+
const CURRENT_USER_PROFILE_ID = 'current_user_profile_id';
1096+
const READ_ONLY_TYPE = 'read-only-type';
1097+
1098+
beforeEach(() => {
1099+
securityExtension.getCurrentUser.mockReturnValue({
1100+
authentication_realm: {
1101+
name: 'authentication_realm',
1102+
type: 'authentication_realm_type',
1103+
},
1104+
lookup_realm: {
1105+
name: 'lookup_realm',
1106+
type: 'lookup_realm_type',
1107+
},
1108+
authentication_provider: {
1109+
name: 'authentication_provider',
1110+
type: 'authentication_provider_type',
1111+
},
1112+
authentication_type: 'realm',
1113+
elastic_cloud_user: false,
1114+
profile_uid: CURRENT_USER_PROFILE_ID,
1115+
} as AuthenticatedUser);
1116+
});
1117+
1118+
registry.registerType({
1119+
name: READ_ONLY_TYPE,
1120+
hidden: false,
1121+
namespaceType: 'multiple-isolated',
1122+
supportsAccessControl: true,
1123+
mappings: {
1124+
dynamic: false,
1125+
properties: {
1126+
description: { type: 'text' },
1127+
},
1128+
},
1129+
management: {
1130+
importableAndExportable: true,
1131+
},
1132+
});
1133+
1134+
afterEach(() => {
1135+
securityExtension.getCurrentUser.mockClear();
1136+
});
1137+
1138+
it('applies access control options only to applicable types', async () => {
1139+
const obj1NoAccessControl = {
1140+
type: 'config',
1141+
id: '6.0.0-alpha1',
1142+
attributes: { title: 'Test One' },
1143+
references: [{ name: 'ref_0', type: 'test', id: '1' }],
1144+
};
1145+
const obj2AccessControl = {
1146+
type: READ_ONLY_TYPE,
1147+
id: 'has-read-only-metadata',
1148+
attributes: { title: 'Test Two' },
1149+
references: [{ name: 'ref_0', type: 'test', id: '2' }],
1150+
};
1151+
await bulkCreateSuccess(client, repository, [obj1NoAccessControl, obj2AccessControl], {
1152+
accessControl: { accessMode: 'read_only' },
1153+
});
1154+
1155+
expect(securityExtension.authorizeBulkCreate).toHaveBeenCalledWith(
1156+
expect.objectContaining({
1157+
objects: expect.arrayContaining([
1158+
{
1159+
type: 'config',
1160+
id: '6.0.0-alpha1',
1161+
name: 'Test One',
1162+
existingNamespaces: [],
1163+
initialNamespace: undefined,
1164+
// explicitly confirm there is no accessControl for non-supporting type
1165+
},
1166+
{
1167+
type: READ_ONLY_TYPE,
1168+
id: 'has-read-only-metadata',
1169+
name: 'Test Two',
1170+
existingNamespaces: [],
1171+
initialNamespace: undefined,
1172+
accessControl: {
1173+
owner: CURRENT_USER_PROFILE_ID,
1174+
accessMode: 'read_only',
1175+
},
1176+
},
1177+
]),
1178+
})
1179+
);
1180+
});
1181+
1182+
it('overrides access control options with incoming object property only for applicable types', async () => {
1183+
const obj1NoAccessControl = {
1184+
type: 'config',
1185+
id: '6.0.0-alpha1',
1186+
attributes: { title: 'Test One' },
1187+
references: [{ name: 'ref_0', type: 'test', id: '1' }],
1188+
accessControl: {
1189+
accessMode: 'read_only',
1190+
} as Pick<SavedObjectAccessControl, 'accessMode'>,
1191+
};
1192+
const obj2AccessControl = {
1193+
type: READ_ONLY_TYPE,
1194+
id: 'has-read-only-metadata',
1195+
attributes: { title: 'Test Two' },
1196+
references: [{ name: 'ref_0', type: 'test', id: '2' }],
1197+
accessControl: {
1198+
accessMode: 'default', // default === RBAC-only
1199+
} as Pick<SavedObjectAccessControl, 'accessMode'>,
1200+
};
1201+
const obj3AccessControl = {
1202+
type: READ_ONLY_TYPE,
1203+
id: 'has-read-only-metadata',
1204+
attributes: { title: 'Test Three' },
1205+
references: [{ name: 'ref_0', type: 'test', id: '3' }],
1206+
};
1207+
await bulkCreateSuccess(
1208+
client,
1209+
repository,
1210+
[obj1NoAccessControl, obj2AccessControl, obj3AccessControl],
1211+
{
1212+
accessControl: { accessMode: 'read_only' },
1213+
}
1214+
);
1215+
1216+
expect(securityExtension.authorizeBulkCreate).toHaveBeenCalledWith(
1217+
expect.objectContaining({
1218+
objects: expect.arrayContaining([
1219+
{
1220+
type: 'config',
1221+
id: '6.0.0-alpha1',
1222+
name: 'Test One',
1223+
existingNamespaces: [],
1224+
initialNamespace: undefined,
1225+
// explicitly confirm there is no accessControl for non-supporting type
1226+
},
1227+
{
1228+
type: READ_ONLY_TYPE,
1229+
id: 'has-read-only-metadata',
1230+
name: 'Test Two',
1231+
existingNamespaces: [],
1232+
initialNamespace: undefined,
1233+
accessControl: {
1234+
owner: CURRENT_USER_PROFILE_ID,
1235+
accessMode: 'default', // explicitly confirm the mode is overriden
1236+
},
1237+
},
1238+
{
1239+
type: READ_ONLY_TYPE,
1240+
id: 'has-read-only-metadata',
1241+
name: 'Test Three',
1242+
existingNamespaces: [],
1243+
initialNamespace: undefined,
1244+
accessControl: {
1245+
owner: CURRENT_USER_PROFILE_ID,
1246+
accessMode: 'read_only', // explicitly confirm the mode is NOT overriden
1247+
},
1248+
},
1249+
]),
1250+
})
1251+
);
1252+
});
1253+
1254+
it('does not create objects with access control when there is no active user profile', async () => {
1255+
securityExtension.getCurrentUser.mockReturnValueOnce(null);
1256+
1257+
const obj1NoAccessControl = {
1258+
type: 'config',
1259+
id: '6.0.0-alpha1',
1260+
attributes: { title: 'Test One' },
1261+
references: [{ name: 'ref_0', type: 'test', id: '1' }],
1262+
};
1263+
const obj2AccessControl = {
1264+
type: READ_ONLY_TYPE,
1265+
id: 'has-read-only-metadata',
1266+
attributes: { title: 'Test Two' },
1267+
references: [{ name: 'ref_0', type: 'test', id: '2' }],
1268+
};
1269+
await bulkCreateSuccess(client, repository, [obj1NoAccessControl, obj2AccessControl], {
1270+
accessControl: { accessMode: 'read_only' },
1271+
});
1272+
1273+
expect(securityExtension.authorizeBulkCreate).toHaveBeenCalledWith(
1274+
expect.objectContaining({
1275+
objects: expect.arrayContaining([
1276+
{
1277+
type: 'config',
1278+
id: '6.0.0-alpha1',
1279+
name: 'Test One',
1280+
existingNamespaces: [],
1281+
initialNamespace: undefined,
1282+
// explicitly confirm there is no accessControl for non-supporting type
1283+
},
1284+
]),
1285+
})
1286+
);
1287+
});
1288+
1289+
// regression test
1290+
it('creates objects supporting access control with no access control metadata when there is no active user profile and no access mode is provided', async () => {
1291+
securityExtension.getCurrentUser.mockReturnValueOnce(null);
1292+
1293+
const obj1NoAccessControl = {
1294+
type: 'config',
1295+
id: '6.0.0-alpha1',
1296+
attributes: { title: 'Test One' },
1297+
references: [{ name: 'ref_0', type: 'test', id: '1' }],
1298+
};
1299+
const obj2AccessControl = {
1300+
type: READ_ONLY_TYPE,
1301+
id: 'could-have-read-only-metadata',
1302+
attributes: { title: 'Test Two' },
1303+
references: [{ name: 'ref_0', type: 'test', id: '2' }],
1304+
};
1305+
await bulkCreateSuccess(client, repository, [obj1NoAccessControl, obj2AccessControl]); // no accessControl options
1306+
1307+
expect(securityExtension.authorizeBulkCreate).toHaveBeenCalledWith(
1308+
expect.objectContaining({
1309+
objects: [
1310+
{
1311+
type: 'config',
1312+
id: '6.0.0-alpha1',
1313+
name: 'Test One',
1314+
existingNamespaces: [],
1315+
initialNamespace: undefined,
1316+
// explicitly confirm there is no accessControl for non-supporting type
1317+
},
1318+
{
1319+
type: READ_ONLY_TYPE,
1320+
id: 'could-have-read-only-metadata',
1321+
name: 'Test Two',
1322+
existingNamespaces: [],
1323+
initialNamespace: undefined,
1324+
// explicitly confirm there is no accessControl for supporting type
1325+
},
1326+
],
1327+
})
1328+
);
1329+
});
1330+
});
10891331
});
10901332
});
10911333
});

0 commit comments

Comments
 (0)