Skip to content

Commit 2e280bc

Browse files
kibanamachineSiddharthMantrielasticmachine
authored
[9.1] [User profiles] Strict user profile setting keys (#241213) (#245266)
# Backport This will backport the following commits from `main` to `9.1`: - [[User profiles] Strict user profile setting keys (#241213)](#241213) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Sid","email":"siddharthmantri1@gmail.com"},"sourceCommit":{"committedDate":"2025-12-04T15:33:51Z","message":"[User profiles] Strict user profile setting keys (#241213)\n\n## Summary\n\nAdds strict schema validation to the user profile update API endpoint\n(/internal/security/user_profile/_data) to restrict the request body to\njust allowed fields.\n\nThe PR also introduces some reasonable defaults for string length for\ncolors and initials.\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n\n---------\n\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"90fdc5f8bbbb90640c08d531c3c52556ec324ec7","branchLabelMapping":{"^v9.3.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Security","release_note:skip","Feature:Security/User Profile","backport:all-open","v9.3.0"],"title":"[User profiles] Strict user profile setting keys","number":241213,"url":"https://github.com/elastic/kibana/pull/241213","mergeCommit":{"message":"[User profiles] Strict user profile setting keys (#241213)\n\n## Summary\n\nAdds strict schema validation to the user profile update API endpoint\n(/internal/security/user_profile/_data) to restrict the request body to\njust allowed fields.\n\nThe PR also introduces some reasonable defaults for string length for\ncolors and initials.\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n\n---------\n\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"90fdc5f8bbbb90640c08d531c3c52556ec324ec7"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.3.0","branchLabelMappingKey":"^v9.3.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/241213","number":241213,"mergeCommit":{"message":"[User profiles] Strict user profile setting keys (#241213)\n\n## Summary\n\nAdds strict schema validation to the user profile update API endpoint\n(/internal/security/user_profile/_data) to restrict the request body to\njust allowed fields.\n\nThe PR also introduces some reasonable defaults for string length for\ncolors and initials.\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n\n---------\n\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"90fdc5f8bbbb90640c08d531c3c52556ec324ec7"}}]}] BACKPORT--> Co-authored-by: Sid <siddharthmantri1@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
1 parent 95a3860 commit 2e280bc

7 files changed

Lines changed: 263 additions & 36 deletions

File tree

‎x-pack/platform/plugins/shared/security/server/routes/user_profile/update.test.ts‎

Lines changed: 164 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,25 +67,178 @@ describe('Update profile routes', () => {
6767
it('correctly defines route.', () => {
6868
const bodySchema = (routeConfig.validate as any).body as ObjectType;
6969
expect(() => bodySchema.validate(0)).toThrowErrorMatchingInlineSnapshot(
70-
`"expected value of type [object] but got [number]"`
70+
`"expected a plain object value, but found [number] instead."`
7171
);
7272
expect(() => bodySchema.validate('avatar')).toThrowErrorMatchingInlineSnapshot(
73-
`"could not parse record value from json input"`
73+
`"could not parse object value from json input"`
7474
);
7575
expect(() => bodySchema.validate(true)).toThrowErrorMatchingInlineSnapshot(
76-
`"expected value of type [object] but got [boolean]"`
77-
);
78-
expect(() => bodySchema.validate(null)).toThrowErrorMatchingInlineSnapshot(
79-
`"expected value of type [object] but got [null]"`
80-
);
81-
expect(() => bodySchema.validate(undefined)).toThrowErrorMatchingInlineSnapshot(
82-
`"expected value of type [object] but got [undefined]"`
76+
`"expected a plain object value, but found [boolean] instead."`
8377
);
8478

8579
expect(bodySchema.validate({})).toEqual({});
8680
expect(
87-
bodySchema.validate({ title: 'some-title', content: { deepProperty: { type: 'basic' } } })
88-
).toEqual({ title: 'some-title', content: { deepProperty: { type: 'basic' } } });
81+
bodySchema.validate({
82+
avatar: { initials: 'some-initials', color: 'some-color', imageUrl: 'some-image-url' },
83+
userSettings: { darkMode: 'dark', contrastMode: 'high' },
84+
})
85+
).toEqual({
86+
avatar: { initials: 'some-initials', color: 'some-color', imageUrl: 'some-image-url' },
87+
userSettings: { darkMode: 'dark', contrastMode: 'high' },
88+
});
89+
});
90+
91+
it('rejects invalid darkMode enum values.', () => {
92+
const bodySchema = (routeConfig.validate as any).body as ObjectType;
93+
94+
// Valid values should pass
95+
expect(
96+
bodySchema.validate({
97+
userSettings: { darkMode: 'system' },
98+
})
99+
).toEqual({ userSettings: { darkMode: 'system' } });
100+
101+
expect(
102+
bodySchema.validate({
103+
userSettings: { darkMode: 'dark' },
104+
})
105+
).toEqual({ userSettings: { darkMode: 'dark' } });
106+
107+
expect(
108+
bodySchema.validate({
109+
userSettings: { darkMode: 'light' },
110+
})
111+
).toEqual({ userSettings: { darkMode: 'light' } });
112+
113+
expect(
114+
bodySchema.validate({
115+
userSettings: { darkMode: 'space_default' },
116+
})
117+
).toEqual({ userSettings: { darkMode: 'space_default' } });
118+
119+
// Invalid values should fail
120+
expect(() =>
121+
bodySchema.validate({
122+
userSettings: { darkMode: 'invalid' },
123+
})
124+
).toThrow();
125+
126+
expect(() =>
127+
bodySchema.validate({
128+
userSettings: { darkMode: 'INVALID' },
129+
})
130+
).toThrow();
131+
132+
expect(() =>
133+
bodySchema.validate({
134+
userSettings: { darkMode: 123 },
135+
})
136+
).toThrow();
137+
});
138+
139+
it('rejects invalid contrastMode enum values.', () => {
140+
const bodySchema = (routeConfig.validate as any).body as ObjectType;
141+
142+
// Valid values should pass
143+
expect(
144+
bodySchema.validate({
145+
userSettings: { contrastMode: 'system' },
146+
})
147+
).toEqual({ userSettings: { contrastMode: 'system' } });
148+
149+
expect(
150+
bodySchema.validate({
151+
userSettings: { contrastMode: 'standard' },
152+
})
153+
).toEqual({ userSettings: { contrastMode: 'standard' } });
154+
155+
expect(
156+
bodySchema.validate({
157+
userSettings: { contrastMode: 'high' },
158+
})
159+
).toEqual({ userSettings: { contrastMode: 'high' } });
160+
161+
// Invalid values should fail
162+
expect(() =>
163+
bodySchema.validate({
164+
userSettings: { contrastMode: 'invalid' },
165+
})
166+
).toThrow();
167+
168+
expect(() =>
169+
bodySchema.validate({
170+
userSettings: { contrastMode: 'INVALID' },
171+
})
172+
).toThrow();
173+
174+
expect(() =>
175+
bodySchema.validate({
176+
userSettings: { contrastMode: 123 },
177+
})
178+
).toThrow();
179+
});
180+
181+
it('rejects avatar initials exceeding max length.', () => {
182+
const bodySchema = (routeConfig.validate as any).body as ObjectType;
183+
const MAX_STRING_FIELD_LENGTH = 1024;
184+
185+
// Valid length should pass
186+
const validInitials = 'a'.repeat(MAX_STRING_FIELD_LENGTH);
187+
expect(
188+
bodySchema.validate({
189+
avatar: { initials: validInitials },
190+
})
191+
).toEqual({ avatar: { color: null, imageUrl: null, initials: validInitials } });
192+
193+
const invalidInitials = 'a'.repeat(MAX_STRING_FIELD_LENGTH + 1);
194+
expect(() =>
195+
bodySchema.validate({
196+
avatar: { initials: invalidInitials },
197+
})
198+
).toThrowErrorMatchingInlineSnapshot(`
199+
"[avatar.initials]: types that failed validation:
200+
- [avatar.initials.0]: value has length [1025] but it must have a maximum length of [1024].
201+
- [avatar.initials.1]: expected value to equal [null]"
202+
`);
203+
});
204+
205+
it('rejects avatar color exceeding max length.', () => {
206+
const bodySchema = (routeConfig.validate as any).body as ObjectType;
207+
const MAX_STRING_FIELD_LENGTH = 1024;
208+
209+
const validColor = 'a'.repeat(MAX_STRING_FIELD_LENGTH);
210+
expect(
211+
bodySchema.validate({
212+
avatar: { color: validColor },
213+
})
214+
).toEqual({ avatar: { color: validColor, imageUrl: null, initials: null } });
215+
216+
const invalidColor = 'a'.repeat(MAX_STRING_FIELD_LENGTH + 1);
217+
expect(() =>
218+
bodySchema.validate({
219+
avatar: { color: invalidColor },
220+
})
221+
).toThrowErrorMatchingInlineSnapshot(`
222+
"[avatar.color]: types that failed validation:
223+
- [avatar.color.0]: value has length [1025] but it must have a maximum length of [1024].
224+
- [avatar.color.1]: expected value to equal [null]"
225+
`);
226+
});
227+
228+
it('allows null values for avatar initials and color.', () => {
229+
const bodySchema = (routeConfig.validate as any).body as ObjectType;
230+
231+
expect(
232+
bodySchema.validate({
233+
avatar: { initials: null, color: null },
234+
})
235+
).toEqual({ avatar: { imageUrl: null, initials: null, color: null } });
236+
});
237+
238+
it('validates body size limit is configured correctly.', () => {
239+
const MAX_USER_PROFILE_DATA_SIZE_BYTES = 1000 * 1024;
240+
241+
expect(routeConfig.options?.body?.maxBytes).toBe(MAX_USER_PROFILE_DATA_SIZE_BYTES);
89242
});
90243

91244
it('fails if session is not found.', async () => {

‎x-pack/platform/plugins/shared/security/server/routes/user_profile/update.ts‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,36 @@ import { createLicensedRouteHandler } from '../licensed_route_handler';
1717
/** User profile data keys that are allowed to be updated by Cloud users */
1818
const ALLOWED_KEYS_UPDATE_CLOUD = ['userSettings.darkMode', 'userSettings.contrastMode'];
1919

20+
const MAX_STRING_FIELD_LENGTH = 1024;
21+
22+
const MAX_USER_PROFILE_DATA_SIZE_BYTES = 1000 * 1024;
23+
24+
const userProfileUpdateSchema = schema.object({
25+
avatar: schema.maybe(
26+
schema.object({
27+
initials: schema.nullable(schema.string({ maxLength: MAX_STRING_FIELD_LENGTH })),
28+
color: schema.nullable(schema.string({ maxLength: MAX_STRING_FIELD_LENGTH })),
29+
imageUrl: schema.nullable(schema.string()),
30+
})
31+
),
32+
userSettings: schema.maybe(
33+
schema.object({
34+
darkMode: schema.maybe(
35+
schema.oneOf([
36+
schema.literal('system'),
37+
schema.literal('dark'),
38+
schema.literal('light'),
39+
schema.literal('space_default'),
40+
])
41+
),
42+
contrastMode: schema.maybe(
43+
schema.oneOf([schema.literal('system'), schema.literal('standard'), schema.literal('high')])
44+
),
45+
'solutionNavigationTour:completed': schema.maybe(schema.boolean()),
46+
})
47+
),
48+
});
49+
2050
export function defineUpdateUserProfileDataRoute({
2151
router,
2252
getSession,
@@ -35,7 +65,12 @@ export function defineUpdateUserProfileDataRoute({
3565
},
3666
},
3767
validate: {
38-
body: schema.recordOf(schema.string(), schema.any()),
68+
body: userProfileUpdateSchema,
69+
},
70+
options: {
71+
body: {
72+
maxBytes: MAX_USER_PROFILE_DATA_SIZE_BYTES,
73+
},
3974
},
4075
},
4176
createLicensedRouteHandler(async (context, request, response) => {

‎x-pack/platform/test/cases_api_integration/security_and_spaces/tests/common/internal/user_actions_get_users.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -470,11 +470,10 @@ export default ({ getService }: FtrProviderContext): void => {
470470
supertest,
471471
req: {
472472
// @ts-expect-error: types are not correct
473-
initials: 4,
474-
// @ts-expect-error: types are not correct
475-
color: true,
473+
initials: null,
476474
// @ts-expect-error: types are not correct
477-
imageUrl: [],
475+
color: null,
476+
imageUrl: null,
478477
},
479478
headers: superUserHeaders,
480479
});

‎x-pack/platform/test/security_api_integration/tests/user_profiles/bulk_get.ts‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,13 @@ export default function ({ getService }: FtrProviderContext) {
104104
.post('/internal/security/user_profile/_data')
105105
.set('kbn-xsrf', 'xxx')
106106
.set('Cookie', usersSessions.get(`user_${userPrefix}`)!.cookie.cookieString())
107-
.send({ some: `data-${userPrefix}` })
107+
.send({
108+
avatar: {
109+
initials: `some-initials-${userPrefix}`,
110+
color: `some-color-${userPrefix}`,
111+
},
112+
userSettings: { darkMode: `dark`, contrastMode: `high` },
113+
})
108114
.expect(200)
109115
)
110116
);
@@ -145,7 +151,7 @@ export default function ({ getService }: FtrProviderContext) {
145151
.set('kbn-xsrf', 'xxx')
146152
.send({
147153
uids: [usersSessions.get('user_one')!.uid, usersSessions.get('user_two')!.uid],
148-
dataPath: 'some',
154+
dataPath: 'avatar',
149155
})
150156
.expect(200);
151157
expect(profiles.body).to.have.length(2);
@@ -155,7 +161,11 @@ export default function ({ getService }: FtrProviderContext) {
155161
Array [
156162
Object {
157163
"data": Object {
158-
"some": "data-one",
164+
"avatar": Object {
165+
"color": "some-color-one",
166+
"imageUrl": null,
167+
"initials": "some-initials-one",
168+
},
159169
},
160170
"user": Object {
161171
"email": "one@elastic.co",
@@ -165,7 +175,11 @@ export default function ({ getService }: FtrProviderContext) {
165175
},
166176
Object {
167177
"data": Object {
168-
"some": "data-two",
178+
"avatar": Object {
179+
"color": "some-color-two",
180+
"imageUrl": null,
181+
"initials": "some-initials-two",
182+
},
169183
},
170184
"user": Object {
171185
"email": "two@elastic.co",

‎x-pack/platform/test/security_api_integration/tests/user_profiles/get_current.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,10 @@ export default function ({ getService }: FtrProviderContext) {
5151
.post('/internal/security/user_profile/_data')
5252
.set('kbn-xsrf', 'xxx')
5353
.set('Cookie', sessionCookie.cookieString())
54-
.send({ some: 'data', another: 'another-data' })
54+
.send({
55+
avatar: { initials: 'some-initials', color: 'some-color' },
56+
userSettings: { darkMode: 'dark', contrastMode: 'high' },
57+
})
5558
.expect(200);
5659

5760
const { body: profileWithoutData } = await supertestWithoutAuth
@@ -96,8 +99,15 @@ export default function ({ getService }: FtrProviderContext) {
9699
expectSnapshot(profileWithAllData).toMatchInline(`
97100
Object {
98101
"data": Object {
99-
"another": "another-data",
100-
"some": "data",
102+
"avatar": Object {
103+
"color": "some-color",
104+
"imageUrl": null,
105+
"initials": "some-initials",
106+
},
107+
"userSettings": Object {
108+
"contrastMode": "high",
109+
"darkMode": "dark",
110+
},
101111
},
102112
"enabled": true,
103113
"labels": Object {},
@@ -119,9 +129,7 @@ export default function ({ getService }: FtrProviderContext) {
119129
`);
120130
expectSnapshot(profileWithSomeData).toMatchInline(`
121131
Object {
122-
"data": Object {
123-
"some": "data",
124-
},
132+
"data": Object {},
125133
"enabled": true,
126134
"labels": Object {},
127135
"uid": "u_K1WXIRQbRoHiuJylXp842IEhAO_OdqT7SDHrJSzUIjU_0",

0 commit comments

Comments
 (0)