-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Saved Queries] Rework saved query privileges #202863
Changes from 67 commits
be94cfe
82135fc
154ddfa
dc5dd8b
d7c7492
530be40
e283580
9a7669f
ecbbd1b
d9ffd02
f4300c6
342c0d2
5f1aecf
6462941
ecd017a
c6caf36
54530cd
c3631a3
7438ac2
7b80270
eecd961
c285640
257aefb
ecd00f2
d812bf2
694988a
36785cf
5492bda
a4c9658
2cfa7d4
08c2e0f
9ed56a6
adb4b53
ae13ef7
a4d46f7
7b4c8e9
9e38a78
ca08ea1
c0175c9
46a10d1
c399174
3fd0fee
dfed85d
7235ea8
e8edb23
7acd0e2
505cfd7
cdb4da3
f4a565a
049f07d
1eadd4f
42bfe4c
2a5a720
80062a7
9d693c0
59a8dca
2d2c7be
01354de
1ac85f5
2a1431b
392ee3f
9f5d07b
cc2229f
069e8f8
ebf724d
5e78180
2553533
a4e9f98
d81d900
01bd64c
a3cb358
7351b70
71282ba
587233d
034c5db
b137d34
3e0182a
9786c73
547744d
1e9dc0d
d6fe93d
3469d55
82727a4
9cdc87c
e891e17
77f0db9
216697a
5a7a248
83a9528
998f91f
2bbfe3c
d508730
d6e949d
5011208
352aa6b
1cabffb
62b0ab5
efec298
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,7 @@ xpack.features.overrides: | |
- feature: "observability" | ||
privileges: [ "read" ] | ||
### Dashboards feature should be moved from Analytics category to the Observability one. | ||
dashboard.category: "observability" | ||
dashboard_v2.category: "observability" | ||
### Discover feature should be moved from Analytics category to the Observability one and its privileges are | ||
### fine-tuned to grant access to Observability app. | ||
discover: | ||
|
@@ -44,6 +44,17 @@ xpack.features.overrides: | |
read.composedOf: | ||
- feature: "observability" | ||
privileges: [ "read" ] | ||
discover_v2: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Originally I replaced the existing discover/dashboard/visualize/maps overrides with the v2 features, but I encountered authorization errors in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, unfortunately, we have to keep the privileges portion of overrides for the deprecated features, as they are still registered and referenced by existing/old roles. However, we can drop the category property. The confusion probably stems from the fact that roles or privileges are not mutated/migrated in the traditional sense - we keep both old and new features, along with their feature privileges, at the same time. Old roles will continue to refer to the old privileges (unless re-saved by admin), while new roles will refer to the new privileges. We only ensure that the new feature privilege provides at least the same level of access as the deprecated features did. |
||
category: "observability" | ||
privileges: | ||
# Discover `All` feature privilege should implicitly grant `All` access to Observability app. | ||
all.composedOf: | ||
- feature: "observability" | ||
privileges: [ "all" ] | ||
# Discover `Read` feature privilege should implicitly grant `Read` access to Observability app. | ||
read.composedOf: | ||
- feature: "observability" | ||
privileges: [ "read" ] | ||
### Fleet feature privileges are fine-tuned to grant access to Logs app. | ||
fleetv2: | ||
privileges: | ||
|
@@ -224,4 +235,4 @@ xpack.ml.compatibleModuleType: 'observability' | |
console.ui.embeddedEnabled: false | ||
|
||
# Disable role management (custom roles) | ||
xpack.security.roleManagementEnabled: false | ||
xpack.security.roleManagementEnabled: false |
semd marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,8 +16,10 @@ xpack.inventory.enabled: false | |
xpack.features.overrides: | ||
### Dashboard feature is hidden in Role management since it's automatically granted by SIEM feature. | ||
dashboard.hidden: true | ||
dashboard_v2.hidden: true | ||
### Discover feature is hidden in Role management since it's automatically granted by SIEM feature. | ||
discover.hidden: true | ||
discover_v2.hidden: true | ||
kertal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
### Machine Learning feature is moved from Analytics category to the Security one as the last item. | ||
ml: | ||
category: "security" | ||
|
@@ -28,25 +30,29 @@ xpack.features.overrides: | |
### Security's `All` feature privilege should implicitly grant `All` access to Discover, Dashboard, Maps, and | ||
### Visualize features. | ||
all.composedOf: | ||
- feature: "discover" | ||
- feature: "discover_v2" | ||
privileges: [ "all" ] | ||
- feature: "dashboard" | ||
- feature: "dashboard_v2" | ||
privileges: [ "all" ] | ||
- feature: "visualize" | ||
- feature: "visualize_v2" | ||
privileges: [ "all" ] | ||
- feature: "maps" | ||
- feature: "maps_v2" | ||
privileges: [ "all" ] | ||
- feature: "savedQueryManagement" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
privileges: [ "all" ] | ||
# Security's `Read` feature privilege should implicitly grant `Read` access to Discover, Dashboard, Maps, and | ||
# Visualize features. Additionally, it should implicitly grant privilege to create short URLs in Discover, | ||
### Dashboard, and Visualize apps. | ||
read.composedOf: | ||
- feature: "discover" | ||
- feature: "discover_v2" | ||
privileges: [ "read" ] | ||
- feature: "dashboard_v2" | ||
privileges: [ "read" ] | ||
- feature: "dashboard" | ||
- feature: "visualize_v2" | ||
privileges: [ "read" ] | ||
- feature: "visualize" | ||
- feature: "maps_v2" | ||
privileges: [ "read" ] | ||
- feature: "maps" | ||
- feature: "savedQueryManagement" | ||
privileges: [ "read" ] | ||
|
||
## Cloud settings | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,16 +16,40 @@ xpack.features.overrides: | |
privileges: | ||
### Dashboard's `All` feature privilege should implicitly grant `All` access to Maps and Visualize features. | ||
all.composedOf: | ||
- feature: "maps" | ||
- feature: "maps_v2" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason the original override was changed to reference the new V2 privileges? We'd typically expect the previous overrides to stay as they were. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I may have done this when troubleshooting some YML config issues I was running into originally, but I don't think there's a good reason for it. I reverted those changes here: 3469d55. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like some snapshots failed after this, updated them here: 82727a4. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That makes sense. Thanks for updating this! |
||
privileges: [ "all" ] | ||
- feature: "visualize" | ||
- feature: "visualize_v2" | ||
privileges: [ "all" ] | ||
### Dashboard's `Read` feature privilege should implicitly grant `Read` access to Maps and Visualize features. | ||
### Additionally, it should implicitly grant privilege to create short URLs in Visualize app. | ||
read.composedOf: | ||
- feature: "maps" | ||
- feature: "maps_v2" | ||
privileges: [ "read" ] | ||
- feature: "visualize" | ||
- feature: "visualize_v2" | ||
privileges: [ "read" ] | ||
### All Dashboard sub-feature privileges should be hidden: reporting capabilities will be granted via dedicated | ||
### Reporting feature and short URL sub-feature privilege should be granted for both `All` and `Read`. | ||
subFeatures.privileges: | ||
download_csv_report.disabled: true | ||
generate_report.disabled: true | ||
store_search_session.disabled: true | ||
url_create: | ||
disabled: true | ||
includeIn: "read" | ||
dashboard_v2: | ||
privileges: | ||
### Dashboard's `All` feature privilege should implicitly grant `All` access to Maps and Visualize features. | ||
all.composedOf: | ||
- feature: "maps_v2" | ||
privileges: [ "all" ] | ||
- feature: "visualize_v2" | ||
privileges: [ "all" ] | ||
### Dashboard's `Read` feature privilege should implicitly grant `Read` access to Maps and Visualize features. | ||
### Additionally, it should implicitly grant privilege to create short URLs in Visualize app. | ||
read.composedOf: | ||
- feature: "maps_v2" | ||
privileges: [ "read" ] | ||
- feature: "visualize_v2" | ||
privileges: [ "read" ] | ||
### All Dashboard sub-feature privileges should be hidden: reporting capabilities will be granted via dedicated | ||
### Reporting feature and short URL sub-feature privilege should be granted for both `All` and `Read`. | ||
|
@@ -45,23 +69,36 @@ xpack.features.overrides: | |
url_create: | ||
disabled: true | ||
includeIn: "read" | ||
discover_v2: | ||
### All Discover sub-feature privileges should be hidden: reporting capabilities will be granted via dedicated | ||
### Reporting feature and short URL sub-feature privilege should be granted for both `All` and `Read`. | ||
subFeatures.privileges: | ||
generate_report.disabled: true | ||
store_search_session.disabled: true | ||
url_create: | ||
disabled: true | ||
includeIn: "read" | ||
### Shared images feature is hidden in Role management since it's not needed. | ||
filesSharedImage.hidden: true | ||
### Maps feature is hidden in Role management since it's automatically granted by Dashboard feature. | ||
maps.hidden: true | ||
maps_v2.hidden: true | ||
### Reporting feature is supposed to give access to reporting capabilities across different features. | ||
reporting: | ||
privileges: | ||
all.composedOf: | ||
- feature: "dashboard" | ||
- feature: "dashboard_v2" | ||
privileges: [ "download_csv_report" ] | ||
- feature: "discover" | ||
- feature: "discover_v2" | ||
privileges: [ "generate_report" ] | ||
### Visualize feature is hidden in Role management since it's automatically granted by Dashboard feature. | ||
visualize: | ||
hidden: true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: same as above, but we can also keep these too, it won't break anything. |
||
### The short URL sub-feature privilege should be always granted. | ||
subFeatures.privileges.url_create.includeIn: "read" | ||
visualize_v2: | ||
hidden: true | ||
### The short URL sub-feature privilege should be always granted. | ||
subFeatures.privileges.url_create.includeIn: "read" | ||
|
||
# Cloud links | ||
xpack.cloud.base_url: 'https://cloud.elastic.co' | ||
|
@@ -233,4 +270,4 @@ discover.enableUiSettingsValidations: true | |
## Data Usage in stack management | ||
xpack.dataUsage.enabled: true | ||
# This feature is disabled in Serverless until fully tested within a Serverless environment | ||
xpack.dataUsage.enableExperimental: ['dataUsageDisabled'] | ||
xpack.dataUsage.enableExperimental: ['dataUsageDisabled'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: why using snack case here instead of the camel case one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reason in particular. There was a mixture in existing privileges, and the examples in the RFC I followed used snake case, so I just went with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: let's migrate to snack case! dashboard🍫V2 ... all of Kibana!