feat(slo): Assert user has correct source index privileges when creating, updating or reseting an SLO by kdelemme · Pull Request #199233 · elastic/kibana

kdelemme · 2024-11-06T21:32:25Z

Resolves #195254

🚗 Summary

Since we use the secondary auth to create and start the transform, a user without the read index privileges is able to create an SLO. But the SLO will never be able to work correctly: the related rollup transform will be failing due to missing privileges.

This PR asserts the user has the correct [read, view_index_metadata] index privileges on the SLO indicator index, i.e. the source index when creating, editing or reseting an SLO.

Testing

Create a user with a role that has no read privilege on the index you want to create an SLO with.
Create an SLO using the API (replace the authorization header with the new user credentials)

curl --request POST \
  --url http://localhost:5601/kibana/api/observability/slos \
  --header 'Authorization: Basic c2xvX21pc3Npbmc6Y2hhbmdlbWU=' \
  --header 'Content-Type: application/json' \
  --header 'kbn-xsrf: oui' \
  --data '{
  "name": "test",
  "description": "",
  "indicator": {
    "type": "sli.kql.custom",
    "params": {
      "index": "kbn-data-forge-fake_stack.admin-console-*",
      "filter": "",
      "good": "http.response.status_code <500",
      "total": "http.response.status_code :*",
      "timestampField": "@timestamp",
      "dataViewId": "e7744dbe-a7a4-457b-83aa-539e9c88764c"
    }
  },
  "budgetingMethod": "occurrences",
  "timeWindow": {
    "duration": "7d",
    "type": "rolling"
  },
  "objective": {
    "target": 0.98
  },
  "tags": [],
  "groupBy": [
    "*"
  ],
  "settings": {
    "preventInitialBackfill": false
  }
}'

This request should return a 403:

{
	"statusCode": 403,
	"error": "Forbidden",
	"message": "Missing ['read', 'view_index_metadata'] privileges on the source index [kbn-data-forge-fake_stack.admin-console-*]"
}

elasticmachine · 2024-11-06T21:33:59Z

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

x-pack/plugins/observability_solution/slo/server/services/reset_slo.ts

github-actions · 2024-11-07T17:43:36Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

shahzad31 · 2024-11-12T13:26:55Z

x-pack/plugins/observability_solution/slo/server/services/create_slo.ts

+    await this.assertSLOInexistant(slo);
+    await assertExpectedIndicatorSourceIndexPrivileges(slo, this.esClient);


could be done in parallel.

shahzad31

LGTM !!

elasticmachine · 2024-11-12T19:21:26Z

💚 Build Succeeded

Buildkite Build
Commit: ee0dd91
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-199233-ee0dd91bc1ce

Metrics [docs]

✅ unchanged

History

💔 Build #250366 failed 6e344d9
💚 Build #249358 succeeded 1ac3682
💚 Build #249343 succeeded 34f3cdb
💚 Build #249037 succeeded cb992ae

kibanamachine · 2024-11-12T20:09:01Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11804893316

…ing, updating or reseting an SLO (elastic#199233) (cherry picked from commit da85efe)

kibanamachine · 2024-11-12T20:14:39Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine · 2024-11-14T20:48:56Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kibanamachine · 2024-11-15T20:49:10Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

…ing, updating or reseting an SLO (elastic#199233)

kibanamachine · 2024-11-18T21:48:53Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kibanamachine · 2024-11-19T21:49:09Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

… creating, updating or reseting an SLO (#199233) (#199875) # Backport This will backport the following commits from `main` to `8.x`: - [feat(slo): Assert user has correct source index privileges when creating, updating or reseting an SLO (#199233)](#199233)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Kevin Delemme <kevin.delemme@elastic.co> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

kdelemme added 4 commits November 6, 2024 15:53

Assert user has expected source index privileges

4ac3fa6

update unit ests

36bef65

Assert user has expected privileges on source index during update

ba6b898

Assert user has expected privielges on source index during reset

cb992ae

kdelemme marked this pull request as ready for review November 6, 2024 21:33

kdelemme requested a review from a team as a code owner November 6, 2024 21:33

kdelemme added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 v8.17.0 backport:prev-minor Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. and removed v9.0.0 labels Nov 6, 2024

dominiqueclarke reviewed Nov 7, 2024

View reviewed changes

x-pack/plugins/observability_solution/slo/server/services/reset_slo.ts Outdated Show resolved Hide resolved

botelastic bot added the ci:project-deploy-observability Create an Observability project label Nov 7, 2024

kdelemme added 2 commits November 7, 2024 12:53

Extract function

34f3cdb

Organize imports

1ac3682

kdelemme requested a review from dominiqueclarke November 8, 2024 15:51

shahzad31 reviewed Nov 12, 2024

View reviewed changes

shahzad31 approved these changes Nov 12, 2024

View reviewed changes

shahzad31 and others added 2 commits November 12, 2024 14:29

Merge branch 'main' into slo/check-source-index-privileges

6e344d9

Merge branch 'main' into slo/check-source-index-privileges

ee0dd91

kdelemme merged commit da85efe into elastic:main Nov 12, 2024

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 12, 2024

feat(slo): Assert user has correct source index privileges when creat…

8e1413c

…ing, updating or reseting an SLO (elastic#199233) (cherry picked from commit da85efe)

kibanamachine mentioned this pull request Nov 12, 2024

[8.x] feat(slo): Assert user has correct source index privileges when creating, updating or reseting an SLO (#199233) #199875

Merged

This was referenced Nov 12, 2024

chore(slo): Migrate to server-route-repository #198726

Merged

[ES|QL] Improve STATS command summary extraction #199796

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 14, 2024

CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024

feat(slo): Assert user has correct source index privileges when creat…

84c7dbe

…ing, updating or reseting an SLO (elastic#199233)

CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024

feat(slo): Assert user has correct source index privileges when creat…

711c1dc

…ing, updating or reseting an SLO (elastic#199233)

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(slo): Assert user has correct source index privileges when creating, updating or reseting an SLO#199233

feat(slo): Assert user has correct source index privileges when creating, updating or reseting an SLO#199233
kdelemme merged 8 commits intoelastic:mainfrom
kdelemme:slo/check-source-index-privileges

kdelemme commented Nov 6, 2024 •

edited by kibanamachine

Loading

Uh oh!

elasticmachine commented Nov 6, 2024

Uh oh!

Uh oh!

github-actions bot commented Nov 7, 2024

Uh oh!

shahzad31 Nov 12, 2024

Uh oh!

shahzad31 left a comment

Uh oh!

elasticmachine commented Nov 12, 2024 •

edited

Loading

Uh oh!

kibanamachine commented Nov 12, 2024

Uh oh!

kibanamachine commented Nov 12, 2024

Uh oh!

kibanamachine commented Nov 14, 2024

Uh oh!

kibanamachine commented Nov 15, 2024

Uh oh!

kibanamachine commented Nov 18, 2024

Uh oh!

kibanamachine commented Nov 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		await this.assertSLOInexistant(slo);
		await assertExpectedIndicatorSourceIndexPrivileges(slo, this.esClient);

Conversation

kdelemme commented Nov 6, 2024 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🚗 Summary

Testing

Uh oh!

elasticmachine commented Nov 6, 2024

Uh oh!

Uh oh!

github-actions bot commented Nov 7, 2024

🤖 GitHub comments

Uh oh!

shahzad31 Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

shahzad31 left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Nov 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Metrics [docs]

History

Uh oh!

kibanamachine commented Nov 12, 2024

Uh oh!

kibanamachine commented Nov 12, 2024

💚 All backports created successfully

Questions ?

Uh oh!

kibanamachine commented Nov 14, 2024

Uh oh!

kibanamachine commented Nov 15, 2024

Uh oh!

kibanamachine commented Nov 18, 2024

Uh oh!

kibanamachine commented Nov 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kdelemme commented Nov 6, 2024 •

edited by kibanamachine

Loading

elasticmachine commented Nov 12, 2024 •

edited

Loading