[User profiles] Strict user profile setting keys#241213
Conversation
|
Pinging @elastic/kibana-security (Team:Security) |
|
@elasticmachine merge upstream |
| // expect(() => bodySchema.validate(null)).toThrowErrorMatchingInlineSnapshot( | ||
| // `"expected a plain object value, but found [null] instead."` | ||
| // ); | ||
| // expect(() => bodySchema.validate(undefined)).toThrowErrorMatchingInlineSnapshot( | ||
| // `"expected value of type [object] but got [undefined]"` | ||
| // ); |
There was a problem hiding this comment.
Commenting these out as for some reason null and undefined are now accepted and the endpoint schema validation does not throw.
There was a problem hiding this comment.
Should we remove these or uncomment?
Also how the route is supposed to react to such requests?
| solutionNavOptOut: schema.maybe(schema.boolean()), | ||
| }) | ||
| ), | ||
| }); |
There was a problem hiding this comment.
Related to https://github.com/elastic/kibana/pull/241213/files#r2503396102
We could add special validation logic here to disallow falsy values.
|
@elasticmachine merge upstream |
| schema.object({ | ||
| initials: schema.nullable(schema.string({ maxLength: MAX_STRING_FIELD_LENGTH })), | ||
| color: schema.nullable(schema.string({ maxLength: MAX_STRING_FIELD_LENGTH })), | ||
| imageUrl: schema.nullable(schema.string()), |
There was a problem hiding this comment.
I've added some defaults for maxLength for these fields.
@azasypkin (tagging you since i saw you assigned to the review 😅) - I'm not sure how to approach restricting imageUrl length.
|
@elasticmachine merge upstream |
|
ACK: I'll review PR today |
azasypkin
left a comment
There was a problem hiding this comment.
LGTM, thanks! Just one question/suggestion.
Also, do we need to update this interface as well
?| 'solutionNavigationTour:completed', // TODO: remove with https://github.com/elastic/kibana/issues/239313 | ||
| ]; | ||
|
|
||
| const MAX_STRING_FIELD_LENGTH = 1024; |
There was a problem hiding this comment.
question/suggestion: Is there any particular reason or use case why we want to support 1024 characters for initials and color instead of, let's say, 100? We can technically go even lower and align with the limits in UI, but I don't have a strong opinion.
If we have a user that has anything in the user profile that's larger than these limits, nothing would break except they'll have to obey the new limits if they decide to update the data, right?
There was a problem hiding this comment.
If we have a user that has anything in the user profile that's larger than these limits, nothing would break except they'll have to obey the new limits if they decide to update the data, right?
That's right. It shouldn't affect any existing data since it's just validation on save.
No particular reason for 1024. Matching the UI limits fromm the form field works for me too.
| avatar: schema.maybe( | ||
| schema.object({ | ||
| initials: schema.nullable(schema.string({ maxLength: MAX_STRING_FIELD_LENGTH })), | ||
| color: schema.nullable(schema.string({ maxLength: MAX_STRING_FIELD_LENGTH })), |
There was a problem hiding this comment.
There was a problem hiding this comment.
Interesting, i hadn't come across this. I'll create an issue for us to look into.
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]
History
|
|
Starting backport for target branches: 8.19, 9.1, 9.2 |
## Summary Adds strict schema validation to the user profile update API endpoint (/internal/security/user_profile/_data) to restrict the request body to just allowed fields. The PR also introduces some reasonable defaults for string length for colors and initials. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] [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 - [ ] [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) --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 90fdc5f)
## Summary Adds strict schema validation to the user profile update API endpoint (/internal/security/user_profile/_data) to restrict the request body to just allowed fields. The PR also introduces some reasonable defaults for string length for colors and initials. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] [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 - [ ] [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) --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 90fdc5f)
## Summary Adds strict schema validation to the user profile update API endpoint (/internal/security/user_profile/_data) to restrict the request body to just allowed fields. The PR also introduces some reasonable defaults for string length for colors and initials. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] [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 - [ ] [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) --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 90fdc5f)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…5267) # Backport This will backport the following commits from `main` to `9.2`: - [[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>
…5266) # 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>
|
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
…45265) # Backport This will backport the following commits from `main` to `8.19`: - [[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>
## Summary Adds strict schema validation to the user profile update API endpoint (/internal/security/user_profile/_data) to restrict the request body to just allowed fields. The PR also introduces some reasonable defaults for string length for colors and initials. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] [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 - [ ] [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) --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Summary
Adds strict schema validation to the user profile update API endpoint (/internal/security/user_profile/_data) to restrict the request body to just allowed fields.
The PR also introduces some reasonable defaults for string length for colors and initials.
Checklist
Check the PR satisfies following conditions.
Reviewers should verify this PR satisfies this list as well.
release_note:*label is applied per the guidelines