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

[Security Solution] Fix assistant settings availability for non-superuser role #182322

Conversation

YulNaumenko
Copy link
Contributor

@YulNaumenko YulNaumenko commented May 2, 2024

Resolves #182209

@YulNaumenko YulNaumenko requested review from a team as code owners May 2, 2024 04:13
@YulNaumenko YulNaumenko requested a review from a team May 2, 2024 04:13
@YulNaumenko YulNaumenko requested review from a team as code owners May 2, 2024 04:13
@YulNaumenko YulNaumenko self-assigned this May 2, 2024
@YulNaumenko YulNaumenko added v8.14.0 bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes labels May 2, 2024
@YulNaumenko YulNaumenko removed the request for review from a team May 2, 2024 05:09
@semd
Copy link
Contributor

semd commented May 2, 2024

Hey @YulNaumenko, the config overall looks good, the original problem is solved (for stateful). I tested the branch locally, it works as expected so I am going to approve it.

However, I noticed some things:

  • License: The AI assistants link in the management navigation is visible, and Security settings are editable, regardless of the license being used (I tested with Platinum license, I could not use the AI assistant, but I could edit its settings). Should we align the behavior regarding the license?

  • Serverless: It is possible to access app/management/kibana/securityAiAssistantManagement from the global search, but there's no link to it anywhere in the entire application, would it make sense to add a "custom" card link to the management landing? (after checking the product type being used)

  • Features: The privilege to use the Security AI assistant is under the "Security" catalog, but to edit the setting a user needs a privilege that is under the "Management" catalog, wouldn't it be more consistent if the Security AI assistant feature enabled the securityAiAssistantManagement management flag. So a user would need both the Security and the Management "AI assistant" features to edit the Security AI assistant settings? (same for O11y)

Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

LGTM!

@YulNaumenko YulNaumenko requested a review from a team as a code owner May 2, 2024 15:25
Copy link
Contributor

@CoenWarmer CoenWarmer left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@kdelemme kdelemme left a comment

Choose a reason for hiding this comment

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

Obs UI changes LGTM

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

LGTM, just left few non-blocking questions.

@@ -7,7 +7,7 @@
"server": true,
"browser": true,
"requiredPlugins": ["management"],
"optionalPlugins": ["home", "serverless"],
"optionalPlugins": ["home", "serverless", "features"],
Copy link
Member

Choose a reason for hiding this comment

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

question: cannot we make features plugin required?

};
});

plugins.features?.registerKibanaFeature({
Copy link
Member

Choose a reason for hiding this comment

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

question: I'm not sure if this plugin is enabled in Serverless, but if it's - should this feature be available in Serverless role management UI? If so, for which project types (Security, Search, Observability)?

@YulNaumenko YulNaumenko enabled auto-merge (squash) May 2, 2024 16:15
@YulNaumenko YulNaumenko requested a review from a team as a code owner May 2, 2024 17:08
@YulNaumenko YulNaumenko removed the request for review from a team May 2, 2024 17:16
@jbudz
Copy link
Member

jbudz commented May 2, 2024

Sorry, updating once more to include #182432 and fix the timeouts shown in the previous build

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
aiAssistantManagementSelection 4.3KB 4.4KB +18.0B
navigation 31.1KB 31.2KB +102.0B
securitySolutionEss 15.5KB 15.5KB +51.0B
total +171.0B

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/saved-objects-service.html#_mappings

id before after diff
_ignored_source 1 - -1
Unknown metric groups

ESLint disabled line counts

id before after diff
aiAssistantManagementSelection 6 5 -1

Total ESLint disabled count

id before after diff
aiAssistantManagementSelection 6 5 -1

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @YulNaumenko

@YulNaumenko YulNaumenko merged commit 9d7dfea into elastic:main May 2, 2024
36 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request May 2, 2024
…user role (elastic#182322)

Resolves elastic#182209

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 9d7dfea)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.14

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request May 2, 2024
…n-superuser role (#182322) (#182528)

# Backport

This will backport the following commits from `main` to `8.14`:
- [[Security Solution] Fix assistant settings availability for
non-superuser role
(#182322)](#182322)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Yuliia
Naumenko","email":"jo.naumenko@gmail.com"},"sourceCommit":{"committedDate":"2024-05-02T21:44:23Z","message":"[Security
Solution] Fix assistant settings availability for non-superuser role
(#182322)\n\nResolves #182209\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"9d7dfeac2fe00ca52398f78e7a7d62b10dd4cfa4","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v8.14.0","v8.15.0"],"title":"[Security
Solution] Fix assistant settings availability for non-superuser
role","number":182322,"url":"https://github.com/elastic/kibana/pull/182322","mergeCommit":{"message":"[Security
Solution] Fix assistant settings availability for non-superuser role
(#182322)\n\nResolves #182209\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"9d7dfeac2fe00ca52398f78e7a7d62b10dd4cfa4"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/182322","number":182322,"mergeCommit":{"message":"[Security
Solution] Fix assistant settings availability for non-superuser role
(#182322)\n\nResolves #182209\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"9d7dfeac2fe00ca52398f78e7a7d62b10dd4cfa4"}}]}]
BACKPORT-->

Co-authored-by: Yuliia Naumenko <jo.naumenko@gmail.com>
@@ -79,6 +86,66 @@ export class AIAssistantManagementSelectionPlugin
},
});

core.capabilities.registerProvider(() => {
Copy link
Member

@azasypkin azasypkin May 3, 2024

Choose a reason for hiding this comment

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

note: forgot to comment on this one yesterday - I don't think we need this here - I believe it's only needed when the feature/plugin can be used with Elastic Stack security disabled which is unlikely to be the case for this plugin.

yuliacech pushed a commit to yuliacech/kibana that referenced this pull request May 3, 2024
…user role (elastic#182322)

Resolves elastic#182209

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes v8.14.0 v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Security GenAI] AI Assistants settings page in Stack Management not available for user with custom role