Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Obs AI Assistant] Add security configs to API #201439

Merged

Conversation

viduni94
Copy link
Contributor

@viduni94 viduni94 commented Nov 22, 2024

Summary

Problem

The API /internal/observability/assistant/alert_details_contextual_insights does not provide explicit authorization settings.

Solution

Add access privileges (ai_assistant) to the above API

Checklist

  • Unit or functional tests were updated or added to match the most common scenarios
  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

@viduni94 viduni94 self-assigned this Nov 22, 2024
@viduni94
Copy link
Contributor Author

/ci

@viduni94 viduni94 force-pushed the add-security-configs-to-contextual-insights-api branch 2 times, most recently from feebf41 to 06f85fb Compare November 25, 2024 13:09
@viduni94 viduni94 marked this pull request as ready for review November 25, 2024 13:59
@viduni94 viduni94 requested a review from a team as a code owner November 25, 2024 14:00
@viduni94 viduni94 added enhancement New value added to drive a business result release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:Obs AI Assistant Observability AI Assistant Authz: API migration labels Nov 25, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ai-assistant (Team:Obs AI Assistant)

@viduni94 viduni94 force-pushed the add-security-configs-to-contextual-insights-api branch from db33865 to ea97495 Compare November 25, 2024 14:08
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-management Observability Management User Experience Team labels Nov 25, 2024
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

🤖 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!)

@viduni94 viduni94 force-pushed the add-security-configs-to-contextual-insights-api branch from ea97495 to 97a7965 Compare November 25, 2024 14:08
@viduni94 viduni94 requested a review from a team November 25, 2024 14:13
@viduni94 viduni94 requested a review from a team as a code owner November 25, 2024 15:53
@@ -146,7 +146,7 @@ export class ObservabilityPlugin implements Plugin<ObservabilityPluginSetup> {
all: {
app: [observabilityFeatureId],
catalogue: [observabilityFeatureId],
api: ['rac'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eh, does this mean they never worked before?

Copy link
Contributor Author

@viduni94 viduni94 Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Earlier we didn't have any access privilege specified for this API. So it worked before too as far as I know (and from my testing before adding the feature privilege).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm I actually thought tags were used for this purpose, but I'm not sure if we tested it in any case. Do you mind adding an API test that checks if there's a 403 when using a user with insufficient access privileges?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, now I get it, this is for the Observability plugin. Hmm. I think the outcome of this should be that you only have access to this API if you have the Obs AI Assistant feature privilege. With this change, is that the case?

Copy link
Contributor Author

@viduni94 viduni94 Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, what I mean is that the feature privilege for the AI Assistant is defined here:

api: [OBSERVABILITY_AI_ASSISTANT_FEATURE_ID, 'ai_assistant', 'manage_llm_product_doc'],

If the user has this privilege, they should have access to /internal/observability/assistant/alert_details_contextual_insights. If they don't have this privilege, they shouldn't. If I understand this PR correctly, you're changing a different feature privilege?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm adding the same feature privilege (ai_assistant) to the contextual details endpoint. With the new authz migration, options.tags are not used anymore. It's added under the security.authz.requiredPrivileges property.

Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

access: 'internal',
},
security: {
authz: {
requiredPrivileges: ['ai_assistant', 'rac'],
Copy link
Contributor Author

@viduni94 viduni94 Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: removed rac because this error happens on the main branch too, therefore not related to authz updates.


I added rac here, because I saw the below error:

[2024-11-25T16:42:54.842-05:00][ERROR][plugins.apm] Error while fetching observability alert details context
[2024-11-25T16:42:54.842-05:00][ERROR][plugins.apm] Error: Insufficient privileges to access feature
    at /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/ml/server/lib/capabilities/check_capabilities.ts:64:13
    at getApmServiceSummary (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/apm/server/routes/assistant_functions/get_apm_service_summary/index.ts:75:11)
    at getAnomalies (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/apm/server/routes/assistant_functions/get_apm_service_summary/get_anomalies.ts:24:28)
    at getApmServiceSummary (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/apm/server/routes/assistant_functions/get_apm_service_summary/index.ts:67:39)
    at /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/apm/server/routes/assistant_functions/get_observability_alert_details_context/index.ts:101:88
    at /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/apm/server/routes/assistant_functions/get_observability_alert_details_context/index.ts:272:22
    at /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/apm/server/routes/assistant_functions/get_observability_alert_details_context/index.ts:270:50
    at async /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability/server/services/index.ts:49:16
    at async AlertDetailsContextualInsightsService.getAlertDetailsContext (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability/server/services/index.ts:47:21)
    at async /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:174:20
    at async /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:297:97
    at async getAlertGroupDetails (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:296:30)
    at async getAlertsContext (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:306:73)
    at async executor (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:173:25)
    at async /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/actions/server/lib/action_executor.ts:395:21
    at async ActionExecutor.execute (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/actions/server/lib/action_executor.ts:77:12)
    at async Object.run (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/actions/server/lib/task_runner_factory.ts:91:28)
    at async TaskManagerRunner.run (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/task_manager/server/task_running/task_runner.ts:325:22)
    at /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/apm/server/routes/assistant_functions/get_observability_alert_details_context/index.ts:74:54
    at /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/apm/server/routes/assistant_functions/get_observability_alert_details_context/index.ts:74:54
    at async /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability/server/services/index.ts:49:16
    at async AlertDetailsContextualInsightsService.getAlertDetailsContext (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability/server/services/index.ts:47:21)
    at async /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:174:20
    at async /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:297:97
    at async getAlertGroupDetails (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:296:30)
    at async getAlertsContext (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:306:73)
    at async executor (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:173:25)
    at async /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/actions/server/lib/action_executor.ts:395:21
    at async ActionExecutor.execute (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/actions/server/lib/action_executor.ts:77:12)
    at async Object.run (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/actions/server/lib/task_runner_factory.ts:91:28)
    at async TaskManagerRunner.run (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/task_manager/server/task_running/task_runner.ts:325:22)
    at /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/apm/server/routes/assistant_functions/get_observability_alert_details_context/index.ts:69:118
    at Object.getScopedLogSourcesService (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/logs_data_access/server/services/log_sources_service/index.ts:40:37)
    at /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/apm/server/routes/assistant_functions/get_observability_alert_details_context/index.ts:69:118
    at async /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability/server/services/index.ts:49:16
    at async AlertDetailsContextualInsightsService.getAlertDetailsContext (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability/server/services/index.ts:47:21)
    at async /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:174:20
    at async /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:297:97
    at async getAlertGroupDetails (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:296:30)
    at async getAlertsContext (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:306:73)
    at async executor (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability_ai_assistant_app/server/rule_connector/index.ts:173:25)
    at async /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/actions/server/lib/action_executor.ts:395:21
    at async ActionExecutor.execute (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/actions/server/lib/action_executor.ts:77:12)
    at async Object.run (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/actions/server/lib/task_runner_factory.ts:91:28)
    at async TaskManagerRunner.run (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/task_manager/server/task_running/task_runner.ts:325:22)
    at /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/apm/server/routes/assistant_functions/get_observability_alert_details_context/index.ts:69:75
    at Object.start (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/apm/server/plugin.ts:88:46)
    at /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/apm/server/routes/assistant_functions/get_observability_alert_details_context/index.ts:69:75
    at async /Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability/server/services/index.ts:49:16
    at async AlertDetailsContextualInsightsService.getAlertDetailsContext (/Users/viduni/Workspace/Elastic/kibana/x-pack/plugins/observability_solution/observability/server/services/index.ts:47:21)

access: 'internal',
},
security: {
authz: {
requiredPrivileges: ['ai_assistant', 'rac'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is ai_assistant a privilege? I'd expect something like ai_assistant_contextual_details:read

Copy link
Contributor Author

@viduni94 viduni94 Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the below, ai_assistant is a privilege:

api: [OBSERVABILITY_AI_ASSISTANT_FEATURE_ID, 'ai_assistant', 'manage_llm_product_doc'],

It's the same as options.tags we've had before:

@viduni94 viduni94 force-pushed the add-security-configs-to-contextual-insights-api branch 3 times, most recently from 7b9356e to 00db57c Compare November 26, 2024 21:53
@viduni94 viduni94 force-pushed the add-security-configs-to-contextual-insights-api branch from c0f46cf to 4bebb68 Compare November 28, 2024 15:16
@elasticmachine
Copy link
Contributor

elasticmachine commented Nov 28, 2024

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: 4bebb68
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-201439-4bebb68808b3

Failed CI Steps

Metrics [docs]

✅ unchanged

History

cc @viduni94

@viduni94 viduni94 merged commit 650c5ca into elastic:main Nov 28, 2024
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

You might need to backport the following PRs to 8.x:
- [Streams] App plugin (#200060)

Manual backport

To create the backport manually run:

node scripts/backport --pr 201439

Questions ?

Please refer to the Backport tool documentation

@viduni94
Copy link
Contributor Author

Please fix the conflicts in /Users/viduni/.backport/repositories/elastic/kibana
Hint: Before fixing the conflicts manually you should consider backporting the following pull requests to "8.x":

The streams app plugin backport PR needs to be merged first for me to backport this PR.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 29, 2024
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 201439 locally

5 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 201439 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 201439 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 201439 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 201439 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 201439 locally

@viduni94
Copy link
Contributor Author

💚 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

viduni94 added a commit to viduni94/kibana that referenced this pull request Dec 10, 2024
## Summary

### Problem
The API
`/internal/observability/assistant/alert_details_contextual_insights`
does not provide explicit authorization settings.

### Solution
Add access privileges (`ai_assistant`) to the above API

### Checklist

- [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] 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)

(cherry picked from commit 650c5ca)
@kibanamachine
Copy link
Contributor

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.

viduni94 added a commit that referenced this pull request Dec 11, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [[Obs AI Assistant] Add security configs to API
(#201439)](#201439)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Viduni
Wickramarachchi","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-28T17:20:31Z","message":"[Obs
AI Assistant] Add security configs to API (#201439)\n\n##
Summary\r\n\r\n### Problem\r\nThe
API\r\n`/internal/observability/assistant/alert_details_contextual_insights`\r\ndoes
not provide explicit authorization settings.\r\n\r\n### Solution\r\nAdd
access privileges (`ai_assistant`) to the above API\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] The PR
description includes the appropriate Release Notes section,\r\nand the
correct `release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"650c5ca2122168b6e717e588d3b294bbd8e663ad","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["enhancement","release_note:skip","backport
missing","v9.0.0","Team:Obs AI
Assistant","ci:project-deploy-observability","Team:obs-ux-management","backport:version","Authz:
API
migration","v8.18.0"],"number":201439,"url":"https://github.com/elastic/kibana/pull/201439","mergeCommit":{"message":"[Obs
AI Assistant] Add security configs to API (#201439)\n\n##
Summary\r\n\r\n### Problem\r\nThe
API\r\n`/internal/observability/assistant/alert_details_contextual_insights`\r\ndoes
not provide explicit authorization settings.\r\n\r\n### Solution\r\nAdd
access privileges (`ai_assistant`) to the above API\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] The PR
description includes the appropriate Release Notes section,\r\nand the
correct `release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"650c5ca2122168b6e717e588d3b294bbd8e663ad"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/201439","number":201439,"mergeCommit":{"message":"[Obs
AI Assistant] Add security configs to API (#201439)\n\n##
Summary\r\n\r\n### Problem\r\nThe
API\r\n`/internal/observability/assistant/alert_details_contextual_insights`\r\ndoes
not provide explicit authorization settings.\r\n\r\n### Solution\r\nAdd
access privileges (`ai_assistant`) to the above API\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] The PR
description includes the appropriate Release Notes section,\r\nand the
correct `release_note:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"650c5ca2122168b6e717e588d3b294bbd8e663ad"}},{"branch":"8.x","label":"v8.18.0","labelRegex":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 11, 2024
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
## Summary

### Problem
The API
`/internal/observability/assistant/alert_details_contextual_insights`
does not provide explicit authorization settings.

### Solution
Add access privileges (`ai_assistant`) to the above API

### Checklist

- [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] 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Authz: API migration backport:version Backport to applied version labels ci:project-deploy-observability Create an Observability project enhancement New value added to drive a business result release_note:skip Skip the PR/issue when compiling release notes Team:Obs AI Assistant Observability AI Assistant Team:obs-ux-management Observability Management User Experience Team v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants