Skip to content

Commit d2fbd27

Browse files
rgodfrey-elasticclaudekibanamachine
authored
[9.4] Bound and memoize Kibana privilege validation in role schema (#282111) (#290944)
# Backport This will backport the following commits from `main` to `9.4`: - [Bound and memoize Kibana privilege validation in role schema (#282111)](#282111) Manual conflict resolution: the generated OAS files (`oas_docs/output/kibana.yaml`, `kibana.serverless.yaml`) did not apply cleanly because the request-body layout differs from `main`. Took the 9.4 versions and added `maxItems: 1000` on the PUT and bulk role-schema Kibana privilege request arrays only (not response schemas). `role_schema.ts` applied cleanly: memoize `getBasePrivilegeNames` with `_.once`, bound the Kibana privileges array to `maxSize: 1000`, and replace the O(n²) space-overlap check with a `Set`. Existing 9.4 `maxSize` bounds on base privilege arrays were left in place. <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) Made with [Cursor](https://cursor.com) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
1 parent b084eae commit d2fbd27

7 files changed

Lines changed: 132 additions & 10 deletions

File tree

‎oas_docs/output/kibana.serverless.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56733,6 +56733,7 @@ paths:
5673356733
- '*'
5673456734
required:
5673556735
- base
56736+
maxItems: 1000
5673656737
type: array
5673756738
metadata:
5673856739
additionalProperties: {}
@@ -56970,6 +56971,7 @@ paths:
5697056971
- '*'
5697156972
required:
5697256973
- base
56974+
maxItems: 1000
5697356975
type: array
5697456976
metadata:
5697556977
additionalProperties: {}

‎oas_docs/output/kibana.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60738,6 +60738,7 @@ paths:
6073860738
- '*'
6073960739
required:
6074060740
- base
60741+
maxItems: 1000
6074160742
type: array
6074260743
metadata:
6074360744
additionalProperties: {}
@@ -60975,6 +60976,7 @@ paths:
6097560976
- '*'
6097660977
required:
6097760978
- base
60979+
maxItems: 1000
6097860980
type: array
6097960981
metadata:
6098060982
additionalProperties: {}

‎x-pack/platform/packages/shared/security/plugin_types_server/jest.config.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
module.exports = {
9+
preset: '@kbn/test/jest_node',
10+
rootDir: '../../../../../..',
11+
roots: ['<rootDir>/x-pack/platform/packages/shared/security/plugin_types_server'],
12+
};

‎x-pack/platform/packages/shared/security/plugin_types_server/moon.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,11 @@ tags:
2828
- prod
2929
- group-platform
3030
- shared
31+
- jest-unit-tests
3132
fileGroups:
3233
src:
3334
- '**/*.ts'
3435
- '!target/**/*'
36+
jest-config:
37+
- jest.config.js
3538
tasks: {}

‎x-pack/platform/packages/shared/security/plugin_types_server/src/authorization/role_schema.test.ts‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
import { getKibanaRoleSchema } from './role_schema';
9+
10+
const basePrivilegeNamesMap = {
11+
global: ['all', 'read'],
12+
space: ['all', 'read'],
13+
};
14+
15+
describe('getKibanaRoleSchema', () => {
16+
describe('input hardening', () => {
17+
test('resolves base privilege names at most once regardless of entry/privilege count', () => {
18+
const getBasePrivilegeNames = jest.fn(() => basePrivilegeNamesMap);
19+
const kibana = Array.from({ length: 25 }, (_unused, i) => ({
20+
spaces: [`space-${i}`],
21+
base: ['all', 'read'],
22+
}));
23+
24+
expect(() => getKibanaRoleSchema(getBasePrivilegeNames).validate(kibana)).not.toThrow();
25+
26+
// Without memoization this would be invoked once per base privilege entry (50 times here);
27+
// the expensive privilege-map resolution must happen at most once per schema.
28+
expect(getBasePrivilegeNames).toHaveBeenCalledTimes(1);
29+
});
30+
31+
test('rejects more than 1000 Kibana privilege entries', () => {
32+
const kibana = Array.from({ length: 1001 }, (_unused, i) => ({
33+
spaces: [`space-${i}`],
34+
base: ['all'],
35+
}));
36+
37+
expect(() =>
38+
getKibanaRoleSchema(() => basePrivilegeNamesMap).validate(kibana)
39+
).toThrowErrorMatchingInlineSnapshot(
40+
`"array size is [1001], but cannot be greater than [1000]"`
41+
);
42+
});
43+
44+
test('allows up to 1000 Kibana privilege entries', () => {
45+
const kibana = Array.from({ length: 1000 }, (_unused, i) => ({
46+
spaces: [`space-${i}`],
47+
base: ['all'],
48+
}));
49+
50+
expect(() => getKibanaRoleSchema(() => basePrivilegeNamesMap).validate(kibana)).not.toThrow();
51+
});
52+
});
53+
54+
describe('space overlap', () => {
55+
test('rejects the same space claimed by two entries', () => {
56+
expect(() =>
57+
getKibanaRoleSchema(() => basePrivilegeNamesMap).validate([
58+
{ feature: { foo: ['foo-privilege-1'] }, spaces: ['marketing'] },
59+
{ feature: { bar: ['bar-privilege-1'] }, spaces: ['sales', 'marketing'] },
60+
])
61+
).toThrowErrorMatchingInlineSnapshot(
62+
`"more than one privilege is applied to the following spaces: [marketing]"`
63+
);
64+
});
65+
66+
test('reports the first duplicate when two entries share multiple spaces', () => {
67+
expect(() =>
68+
getKibanaRoleSchema(() => basePrivilegeNamesMap).validate([
69+
{ base: ['all'], spaces: ['alpha', 'beta'] },
70+
{ base: ['read'], spaces: ['alpha', 'beta'] },
71+
])
72+
).toThrowErrorMatchingInlineSnapshot(
73+
`"more than one privilege is applied to the following spaces: [alpha]"`
74+
);
75+
});
76+
77+
test('rejects duplicate space IDs within a single entry', () => {
78+
expect(() =>
79+
getKibanaRoleSchema(() => basePrivilegeNamesMap).validate([
80+
{ base: ['all'], spaces: ['marketing', 'marketing'] },
81+
])
82+
).toThrowErrorMatchingInlineSnapshot(
83+
`"more than one privilege is applied to the following spaces: [marketing]"`
84+
);
85+
});
86+
87+
test('allows disjoint spaces across entries', () => {
88+
expect(() =>
89+
getKibanaRoleSchema(() => basePrivilegeNamesMap).validate([
90+
{ base: ['all'], spaces: ['marketing'] },
91+
{ base: ['read'], spaces: ['sales'] },
92+
])
93+
).not.toThrow();
94+
});
95+
});
96+
});

‎x-pack/platform/packages/shared/security/plugin_types_server/src/authorization/role_schema.ts‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,11 @@ const FEATURE_NAME_VALUE_REGEX = /^[a-zA-Z0-9_-]+$/;
285285
*/
286286
export const getKibanaRoleSchema = (
287287
getBasePrivilegeNames: () => { global: string[]; space: string[] }
288-
) =>
289-
schema.arrayOf(
288+
) => {
289+
// Resolving base privilege names rebuilds the full privilege map; do it once per schema.
290+
const getMemoizedBasePrivilegeNames = _.once(getBasePrivilegeNames);
291+
292+
return schema.arrayOf(
290293
schema.object(
291294
{
292295
/**
@@ -309,7 +312,7 @@ export const getKibanaRoleSchema = (
309312
schema.string({
310313
meta: { description: 'A base privilege that grants applies to all spaces.' },
311314
validate(value) {
312-
const globalPrivileges = getBasePrivilegeNames().global;
315+
const globalPrivileges = getMemoizedBasePrivilegeNames().global;
313316
if (!globalPrivileges.some((privilege) => privilege === value)) {
314317
return `unknown global privilege "${value}", must be one of [${globalPrivileges}]`;
315318
}
@@ -321,7 +324,7 @@ export const getKibanaRoleSchema = (
321324
schema.string({
322325
meta: { description: 'A base privilege that applies to specific spaces.' },
323326
validate(value) {
324-
const spacePrivileges = getBasePrivilegeNames().space;
327+
const spacePrivileges = getMemoizedBasePrivilegeNames().space;
325328
if (!spacePrivileges.some((privilege) => privilege === value)) {
326329
return `unknown space privilege "${value}", must be one of [${spacePrivileges}]`;
327330
}
@@ -383,18 +386,21 @@ export const getKibanaRoleSchema = (
383386
}
384387
),
385388
{
389+
maxSize: 1000,
386390
validate(value) {
387-
for (const [indexA, valueA] of value.entries()) {
388-
for (const valueB of value.slice(indexA + 1)) {
389-
const spaceIntersection = _.intersection(valueA.spaces, valueB.spaces);
390-
if (spaceIntersection.length !== 0) {
391-
return `more than one privilege is applied to the following spaces: [${spaceIntersection}]`;
391+
const claimed = new Set<string>();
392+
for (const { spaces } of value) {
393+
for (const space of spaces) {
394+
if (claimed.has(space)) {
395+
return `more than one privilege is applied to the following spaces: [${space}]`;
392396
}
397+
claimed.add(space);
393398
}
394399
}
395400
},
396401
}
397402
);
403+
};
398404

399405
export type ElasticsearchPrivilegesType = TypeOf<typeof elasticsearchRoleSchema>;
400406
export type KibanaPrivilegesType = TypeOf<ReturnType<typeof getKibanaRoleSchema>>;

‎x-pack/platform/packages/shared/security/plugin_types_server/tsconfig.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
{
22
"extends": "@kbn/tsconfig-base/tsconfig.json",
33
"compilerOptions": {
4-
"outDir": "target/types"
4+
"outDir": "target/types",
5+
"types": ["jest", "node"]
56
},
67
"include": [
78
"**/*.ts",

0 commit comments

Comments
 (0)