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

Adds Role Based Access-Control to the Alerting & Action plugins based on Kibana Feature Controls #67157

Merged
merged 196 commits into from
Jul 22, 2020
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
196 commits
Select commit Hold shift + click to select a range
c62a8fc
made SO client unsecure in alerting
gmmorris May 21, 2020
764f515
fixed typing, commented unused authz
gmmorris May 21, 2020
52a153e
fixed unit test
gmmorris May 21, 2020
4b95c81
added rbac in alerting
gmmorris Jun 3, 2020
95da803
made SO client unsecure in alerting
gmmorris May 21, 2020
341afdb
fixed typing, commented unused authz
gmmorris May 21, 2020
c8e23f0
fixed unit test
gmmorris May 21, 2020
1afad8e
added rbac in alerting
gmmorris Jun 3, 2020
06495a6
fixed unit test
gmmorris Jun 4, 2020
77348f1
provide default global privileges over builtin types
gmmorris Jun 4, 2020
412f684
Merge branch 'alerting/consumer-based-rbac' of github.com:gmmorris/ki…
gmmorris Jun 4, 2020
541a871
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 4, 2020
492f78a
fixed lintin errors
gmmorris Jun 4, 2020
076ebdf
moved feature into main alerts plugin
gmmorris Jun 4, 2020
bdd5d28
fixed security tests
gmmorris Jun 4, 2020
711fdba
fixed security unit tests
gmmorris Jun 4, 2020
a15c7d9
added _global namespace before global privileges
gmmorris Jun 4, 2020
87d099f
fixed security acceptance tests
gmmorris Jun 4, 2020
ade2c4c
fixed lint
gmmorris Jun 4, 2020
0ace530
use alerts privileges in the alertsExample feature
gmmorris Jun 5, 2020
efdb521
added more acceptance tests around alert creation and auth
gmmorris Jun 8, 2020
665c427
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 8, 2020
11bbf16
fixed secuirty interface
gmmorris Jun 8, 2020
2b84902
removed unused test fixture
gmmorris Jun 8, 2020
e15946c
added more acceptance tests around alert deletion, enabling, find and…
gmmorris Jun 8, 2020
445710f
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 8, 2020
ee79c9c
expanded acceptance tests around rbac in alerts
gmmorris Jun 9, 2020
168ce21
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 9, 2020
6910165
fixed alerts UI tests
gmmorris Jun 9, 2020
6b789ec
extracted auth function from alerts client
gmmorris Jun 9, 2020
0aaaef5
fixed sapces only suite
gmmorris Jun 10, 2020
86f7d73
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 10, 2020
ba40757
added more unit tests around the extracted auth code
gmmorris Jun 10, 2020
74a886a
fixed lintin issues
gmmorris Jun 10, 2020
f56a849
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 10, 2020
c055216
fixed producer in siem alert types
gmmorris Jun 10, 2020
399493b
added readme
gmmorris Jun 10, 2020
8ecba62
removed unused export
gmmorris Jun 10, 2020
e4e6590
added audit logging
gmmorris Jun 11, 2020
08541d3
added alerting to feature iterator
gmmorris Jun 11, 2020
87bd206
added validation that alert type IDs dont contain invalid privilege c…
gmmorris Jun 11, 2020
e1e560c
added comment around alert type ID char limitations
gmmorris Jun 11, 2020
b012ae1
fixed tests
gmmorris Jun 11, 2020
34b7cf9
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 11, 2020
64e802b
fixed features unit tests
gmmorris Jun 11, 2020
f287766
added index threshold as authorized alert type in example plugin
gmmorris Jun 12, 2020
acd9961
fixed a bunch of styling changes and small fixes
gmmorris Jun 12, 2020
54ad8dd
changed casing of const
gmmorris Jun 12, 2020
867b7c3
added support for fields in find
gmmorris Jun 15, 2020
f34d031
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 15, 2020
289a85b
revert feature ID to legacy ID to prevent old alerts from breaking
gmmorris Jun 15, 2020
c9453f1
corrected unit tests that relied on the new feature id
gmmorris Jun 15, 2020
244874c
corrected acceptance tests that relied on the new feature id
gmmorris Jun 15, 2020
606081b
moved logs alert type to the correct feature
gmmorris Jun 15, 2020
213b330
reverted partial type
gmmorris Jun 15, 2020
35a9971
fixed another place using the alerts consumer
gmmorris Jun 15, 2020
c18ab7f
use constant to make it easier to change i nthe future
gmmorris Jun 15, 2020
9937143
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 15, 2020
f2f3c2b
changed producer on metric alerts to match feature id
gmmorris Jun 15, 2020
ac37d1b
change feature in privileges bac kto alerts
gmmorris Jun 15, 2020
0d2c859
change feature in privileges in basic back to alerts
gmmorris Jun 15, 2020
80fe0fd
fixed consumer fields in siem
gmmorris Jun 15, 2020
270ecb1
cleaned up some fixtures and featur eregestrations
gmmorris Jun 16, 2020
9ff6666
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 18, 2020
d78822c
fixed indentation
gmmorris Jun 18, 2020
3c77b85
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 23, 2020
554e7ce
removed feature registration in alerting
gmmorris Jun 23, 2020
c177be0
ensure alerTypeId and Consumer cant be used for KQL injection
gmmorris Jun 23, 2020
bae77e4
added some missing unit tests
gmmorris Jun 23, 2020
0370e9d
migrated feature to "alerts"
gmmorris Jun 23, 2020
19d38aa
support alerts consumer without a feature backing it
gmmorris Jun 23, 2020
3c66ba0
bump timeout on delete all test as the rbac work has made it a little…
gmmorris Jun 23, 2020
8f82baf
removed alerting feature from privileges
gmmorris Jun 23, 2020
04cd6f5
include alerts in auth consumers for all types
gmmorris Jun 23, 2020
75a5fcb
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 23, 2020
cc06e67
ensure test tag is isolated from other tests
gmmorris Jun 23, 2020
3ccb14f
prevent parens and whitespace in consumer or alerttypeid
gmmorris Jun 24, 2020
a3082b0
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 24, 2020
e00ffe4
improved perf of "find" api by improving the filter
gmmorris Jun 24, 2020
adefb2f
fixed tests broken by merge conflict
gmmorris Jun 24, 2020
8b2a423
reduce features included in auth to those who grant alerting privileges
gmmorris Jun 25, 2020
44a0c4e
incluyde sub features in privilege check
gmmorris Jun 25, 2020
9de574c
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 25, 2020
2e75199
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 25, 2020
b094910
fixed broken index threshold in non metrics users
gmmorris Jun 26, 2020
17fba6a
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 26, 2020
611061e
migrate alerts with consumer "metrics" to be "infrastructure"
gmmorris Jun 26, 2020
bd0f78c
Merge branch 'master' into alerting/consumer-based-rbac
elasticmachine Jun 29, 2020
1a20848
fixed consumer in metrics alert types
gmmorris Jun 29, 2020
970656d
Merge branch 'alerting/consumer-based-rbac' of github.com:gmmorris/ki…
gmmorris Jun 29, 2020
0ffe4a2
Merge branch 'master' into alerting/consumer-based-rbac
elasticmachine Jun 30, 2020
025ed9e
use feature based RBAC for actions instead of api privileges
gmmorris Jun 30, 2020
f4f2f09
temporary security changes until alerting rbac branch is merged
gmmorris Jun 30, 2020
29c9cc7
base execution privileges on access to action_task_params type
gmmorris Jun 30, 2020
99e5ab0
fixed linting
gmmorris Jun 30, 2020
f22c7aa
Merge branch 'alerting/consumer-based-rbac' of github.com:gmmorris/ki…
gmmorris Jun 30, 2020
90d0df4
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jun 30, 2020
036a082
fixed security typing
gmmorris Jul 1, 2020
33ef0b0
ensure save/edit buttons in triggers UI is based on RBAC auth
gmmorris Jul 1, 2020
3c4a7a9
Merge remote-tracking branch 'upstream/master' into alerting/consumer…
gmmorris Jul 1, 2020
353dd25
introduces a feature for built-in alert types
gmmorris Jul 2, 2020
c0d09cc
introduces a feature for built-in alert types
gmmorris Jul 2, 2020
0e001e6
Merge branch 'master' into alerting/built-in-alerts-feature
gmmorris Jul 2, 2020
ee05baa
show prompt if user has no privileges in flyout
gmmorris Jul 2, 2020
6c42c92
fixed list types test
gmmorris Jul 2, 2020
a7d36e4
fixed unit tests in trigegrs UI
gmmorris Jul 2, 2020
ae38572
fix test broken by addition of built-in types feature
gmmorris Jul 2, 2020
a67950e
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 2, 2020
8f30d0f
updated readme and i18n usage
gmmorris Jul 2, 2020
e454e59
added builtInAlerts to feature set test
gmmorris Jul 2, 2020
b3ed832
use alertsclient in task runner
gmmorris Jul 2, 2020
e4a16c7
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 3, 2020
f0f82f3
fixed lodash usage broken by upgrade to lodash 4
gmmorris Jul 3, 2020
541cdfd
prevent rendering alert editing when there are no privileges to edit …
gmmorris Jul 3, 2020
f56574f
Merge branch 'master' into actions/feature
gmmorris Jul 3, 2020
169789a
allow all to see list of action types by default (for now)
gmmorris Jul 3, 2020
c426139
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 3, 2020
1611aff
Merge branch 'actions/feature' into actions/feature-and-rbac
gmmorris Jul 3, 2020
49b40be
fixec privileges feature tests
gmmorris Jul 3, 2020
53916fd
fixed security test
gmmorris Jul 3, 2020
0e0d175
Merge branch 'actions/feature' into actions/feature-and-rbac
gmmorris Jul 3, 2020
e6025ba
disble connector fields when user is read only
gmmorris Jul 3, 2020
e919958
Merge branch 'actions/feature' into actions/feature-and-rbac
gmmorris Jul 3, 2020
d7f0b27
correct capabilities check
gmmorris Jul 3, 2020
d78b918
fixed type errors
gmmorris Jul 6, 2020
14ebe0e
Merge branch 'master' into actions/feature
gmmorris Jul 6, 2020
a2d25cf
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 6, 2020
5671159
Merge branch 'actions/feature' into actions/feature-and-rbac
gmmorris Jul 6, 2020
9cc3753
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 6, 2020
a4f1a7d
fixed security unit test
gmmorris Jul 6, 2020
2ae533a
Merge branch 'actions/feature' into actions/feature-and-rbac
gmmorris Jul 6, 2020
3cc2cb5
fixed some missing typing
gmmorris Jul 6, 2020
7f5099c
show prompt if user has no privileges in actions form
gmmorris Jul 6, 2020
76d2818
added actions feature to features test
gmmorris Jul 6, 2020
a449385
Merge branch 'actions/feature' into actions/feature-and-rbac
gmmorris Jul 6, 2020
6a2b64d
Merge branch 'master' into actions/feature
gmmorris Jul 6, 2020
3fd2309
added missing SO privileges
gmmorris Jul 6, 2020
da1f944
improved copy
gmmorris Jul 7, 2020
ae3c7a7
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 7, 2020
22efe49
Merge branch 'actions/feature' into actions/feature-and-rbac
gmmorris Jul 7, 2020
61626a0
Merge branch 'master' into actions/feature
gmmorris Jul 7, 2020
a73fff5
Merge branch 'actions/feature' into actions/feature-and-rbac
gmmorris Jul 7, 2020
d412665
Merge branch 'alerting/consumer-based-rbac' into actions/feature-and-…
gmmorris Jul 7, 2020
23dafe8
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 7, 2020
a3f6142
added readonly support to servicenow connector
gmmorris Jul 7, 2020
5c5ff2d
Merge branch 'actions/feature' into actions/feature-and-rbac
gmmorris Jul 7, 2020
6c84a9d
removed unused variable in i18n
gmmorris Jul 7, 2020
96dfb5a
Merge branch 'actions/feature' into actions/feature-and-rbac
gmmorris Jul 7, 2020
12f2049
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 7, 2020
84947d2
Merge branch 'alerting/consumer-based-rbac' into actions/feature-and-…
gmmorris Jul 7, 2020
57ffda5
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 7, 2020
4d6d96e
Merge branch 'alerting/consumer-based-rbac' into actions/feature-and-…
gmmorris Jul 7, 2020
a84a263
Merge branch 'master' into actions/feature-and-rbac
gmmorris Jul 8, 2020
5e2f0dd
added bulk audit log api for alerts
gmmorris Jul 8, 2020
abbb2c0
fixed bulk audit log api for alerts
gmmorris Jul 8, 2020
84da270
Merge branch 'alerting/consumer-based-rbac' into actions/feature-and-…
gmmorris Jul 8, 2020
8d8ea54
removed actio nSO privileges from builtin alert types
gmmorris Jul 8, 2020
f33ac25
Merge branch 'master' into actions/feature-and-rbac
gmmorris Jul 8, 2020
7b8cbe2
Merge branch 'master' into alerting/consumer-based-rbac
elasticmachine Jul 8, 2020
9e74773
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 9, 2020
6c0f72c
Merge branch 'alerting/consumer-based-rbac' of github.com:gmmorris/ki…
gmmorris Jul 9, 2020
98a00b6
removed ui capabilities that are no longer in use
gmmorris Jul 9, 2020
4efb5e7
removed ui and api capabilities from built-in alerts that are no long…
gmmorris Jul 9, 2020
053ca76
removed ui capabilities from solutions that are no longer in use
gmmorris Jul 9, 2020
9f004db
improved "no permission" call out in UI
gmmorris Jul 9, 2020
acc5f55
ensure user has authrization to actions when an alert has actions
gmmorris Jul 9, 2020
662e4a2
disabled switches on alert details page when there are no privileges
gmmorris Jul 9, 2020
048e769
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 13, 2020
cd0522e
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 13, 2020
d2c732f
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 14, 2020
46845fc
handle case where security is disabled in ES but enabled in kibana
gmmorris Jul 14, 2020
97b4262
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 14, 2020
67e913a
prevent unknown consumers from being authorized
gmmorris Jul 14, 2020
6b090eb
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 15, 2020
d1cc1cd
moved migration to v7.10.0 as this feature hasnt made it into 7.9.0
gmmorris Jul 15, 2020
e9ac83c
corrected var name
gmmorris Jul 15, 2020
cdef95d
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 15, 2020
d6616db
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 15, 2020
a6989b5
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 16, 2020
07e1a1c
take into account which features available in the active space
gmmorris Jul 17, 2020
cbee849
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 17, 2020
16ab6c6
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 17, 2020
46f0d74
corrected consumer on enable operation
gmmorris Jul 17, 2020
6b14aaf
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 20, 2020
e2fff84
added valifation of the alerting privileges at feature level
gmmorris Jul 20, 2020
d7ecd86
corrected security check for rbac
gmmorris Jul 20, 2020
46f46c7
fixed unit in alerts client factory
gmmorris Jul 20, 2020
a894e5a
allow user to disable alert even if they dont have privileges to the …
gmmorris Jul 20, 2020
81978c3
fixed alerts test
gmmorris Jul 20, 2020
7febd47
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 20, 2020
5582f06
expclude security wrapper in SO client passed to ActionsClient
gmmorris Jul 21, 2020
12f6536
removed uneeded tests
gmmorris Jul 21, 2020
407b09a
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 21, 2020
7bafd5d
includes hidden params type in SO client
gmmorris Jul 21, 2020
e794518
renamed variable to make it clear the SO client is unsecured
gmmorris Jul 21, 2020
53aa8e9
Merge branch 'master' into alerting/consumer-based-rbac
gmmorris Jul 21, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/alerting_example/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
"kibanaVersion": "kibana",
"server": true,
"ui": true,
"requiredPlugins": ["triggers_actions_ui", "charts", "data", "alerts", "actions", "developerExamples"],
"requiredPlugins": ["triggers_actions_ui", "charts", "data", "alerts", "actions", "features", "developerExamples"],
"optionalPlugins": []
}
32 changes: 31 additions & 1 deletion examples/alerting_example/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,49 @@

import { Plugin, CoreSetup } from 'kibana/server';
import { PluginSetupContract as AlertingSetup } from '../../../x-pack/plugins/alerts/server';
import { PluginSetupContract as FeaturesPluginSetup } from '../../../x-pack/plugins/features/server';

import { alertType as alwaysFiringAlert } from './alert_types/always_firing';
import { alertType as peopleInSpaceAlert } from './alert_types/astros';

// this plugin's dependendencies
export interface AlertingExampleDeps {
alerts: AlertingSetup;
features: FeaturesPluginSetup;
}

export class AlertingExamplePlugin implements Plugin<void, void, AlertingExampleDeps> {
public setup(core: CoreSetup, { alerts }: AlertingExampleDeps) {
public setup(core: CoreSetup, { alerts, features }: AlertingExampleDeps) {
alerts.registerType(alwaysFiringAlert);
alerts.registerType(peopleInSpaceAlert);

features.registerFeature({
id: 'alertsExample',
name: 'alertsExample',
gmmorris marked this conversation as resolved.
Show resolved Hide resolved
app: [],
privileges: {
all: {
alerting: {
all: [alwaysFiringAlert.id, peopleInSpaceAlert.id],
gmmorris marked this conversation as resolved.
Show resolved Hide resolved
},
savedObject: {
all: [],
read: [],
},
ui: [],
},
read: {
alerting: {
read: [alwaysFiringAlert.id, peopleInSpaceAlert.id],
},
savedObject: {
all: [],
read: [],
},
ui: [],
},
},
});
}

public start() {}
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/alerting_builtins/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { PluginInitializerContext } from 'src/core/server';
import { AlertingBuiltinsPlugin } from './plugin';
import { configSchema } from './config';
export { ID as IndexThresholdId } from './alert_types/index_threshold/alert_type';
gmmorris marked this conversation as resolved.
Show resolved Hide resolved

export const plugin = (ctx: PluginInitializerContext) => new AlertingBuiltinsPlugin(ctx);

Expand Down
103 changes: 102 additions & 1 deletion x-pack/plugins/alerts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Table of Contents
- [Methods](#methods)
- [Executor](#executor)
- [Example](#example)
- [Role Based Access-Control](#role-based-access-control)
- [Alert Navigation](#alert-navigation)
- [RESTful API](#restful-api)
- [`POST /api/alerts/alert`: Create alert](#post-apialert-create-alert)
Expand Down Expand Up @@ -58,7 +59,8 @@ A Kibana alert detects a condition and executes one or more actions when that co
## Usage

1. Develop and register an alert type (see alert types -> example).
2. Create an alert using the RESTful API (see alerts -> create).
2. Configure feature level privileges using RBAC
3. Create an alert using the RESTful API (see alerts -> create).

## Limitations

Expand Down Expand Up @@ -293,6 +295,105 @@ server.newPlatform.setup.plugins.alerts.registerType({
});
```

## Role Based Access-Control
Once you have registered your AlertType, you need to grant your users privileges to use it.
When registering a feature in Kibana you can specify multiple types of privileges which are granted to users when they're assigned certain roles.

Assuming your feature introduces its own AlertTypes, you'll want to control:
- Which roles have all/read privileges for these AlertTypes when they're inside the feature
- Which roles have all/read privileges for these AlertTypes when they're outside the feature (in another feature or in the global alerts management)

In addition, when users are inside your feature you might want to grant them access to AlertTypes from other features, such as built-in AlertTypes or AlertTypes provided by other features.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sure we've talked about this and I'm sure we will again. But to refresh my memory; I was thinking this was automatic and not controlled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's confusing, no worries.

It works like this:
Producer privilege throughout Kibana & Consumer privilege to create built-in types in Alerts Management are automatic.
Consumer privilege to create built-in types in a consumer that isn't Alerts Management is not automatic.

We did this so that:

  1. Gaining all access to create alerts in siem, for example, doesn't automatically grant you the right to create any built-in with siem as consumer- as that might mean that if they run a find for all siem alerts they'll get back AlertTypes they don't support or expect.
  2. What if siem do actually want to allow a built-in type, but only to a certain role or as a sub-privilege? We'd have to provide for that and doing so automatically now isn't something we can (necessarily) easily dial back, so best to keep this explicit for now (I think).

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I'll process this soon.

It looks like @kobelb has a similar question as well: #43994 (comment).


You can control all of these abilities by assigning privileges to the Alerting Framework from within your own feature, for example:

```
features.registerFeature({
id: 'my-application-id',
name: 'My Application',
app: [],
privileges: {
all: {
alerting: {
all: [
// grant `all` over our own types
'my-application-id.my-alert-type',
'my-application-id.my-restricted-alert-type',
// grant `all` over the built-in IndexThreshold
'.index-threshold',
// grant `all` over Uptime's TLS AlertType
'xpack.uptime.alerts.actionGroups.tls'],
},
},
read: {
alerting: {
read: [
// grant `read` over our own type
'my-application-id.my-alert-type',
// grant `read` over the built-in IndexThreshold
'.index-threshold',
// grant `read` over Uptime's TLS AlertType
'xpack.uptime.alerts.actionGroups.tls'],
},
},
},
});
gmmorris marked this conversation as resolved.
Show resolved Hide resolved
```

In this example we can see the following:
- Our feature grants any user who's assigned the `all` role in our feature the `all` role in the Alerting framework over every alert of the `my-application-id.my-alert-type` type which is created _inside_ the feature. What that means is that this privilege will allow the user to execute any of the `all` operations (listed below) on these alerts as long as their `consumer` is `my-application-id`. Below that you'll notice we've done the same with the `read` role, which is grants the Alerting Framework's `read` role privileges over these very same alerts.
- In addition, our feature grants the same privileges over any alert of type `my-application-id.my-restricted-alert-type`, which is another hypothetical alertType registered by this feature. It's worth noting though that this type has been omitted from the `read` role. What this means is that only users with the `all` role will be able to interact with alerts of this type.
- Next, lets look at the `.index-threshold` and `xpack.uptime.alerts.actionGroups.tls` types. These have been specified in both `read` and `all`, which means that all the users in the feature will gain privileges over alerts of these types (as long as their `consumer` is `my-application-id`). The difference between these two and the previous two is that they are _produced_ by other features! `.index-threshold` is a built-in type, provided by the framework, and specifying it here is all you need in order to grant privileges to use this type. On the other hand, `xpack.uptime.alerts.actionGroups.tls` is an AlertType provided by the _Uptime_ feature. Specifying this type here tells the Alerting Framework that as far as the `my-application-id` feature is concerned, the user is privileged to use this type (with `all` and `read` applied), but that isn't enough. Using another feature's AlertType is only possible if both the producer of the AlertType, and the consumer of the AlertType, explicitly grant privileges to do so. In this case, the _Uptime_ feature would have to explicitly add these privileges to a role and this role would have to be granted to this user.

It's important to note that any role can be granted a mix of `all` and `read` privileges accross multiple type, for example:

```
features.registerFeature({
id: 'my-application-id',
name: 'My Application',
app: [],
privileges: {
all: {
alerting: {
all: ['my-application-id.my-alert-type', 'my-application-id.my-restricted-alert-type'],
},
},
read: {
alerting: {
all:['my-application-id.my-alert-type']
read: ['my-application-id.my-restricted-alert-type'],
},
},
},
});
gmmorris marked this conversation as resolved.
Show resolved Hide resolved
```

In the above example, you note that instead of denying users with the `read` role any access to the `my-application-id.my-restricted-alert-type` type, we've decided that these users _should_ be granted `read` privileges over the _resitricted_ AlertType.
As part of that same change, we also decided that not only should they be allowed to `read` the _restricted_ AlertType, but actually, despite having `read` privileges to the feature as a whole, we do actualyl want to allow them to create our basic 'my-application-id.my-alert-type' AlertType, as we consider it an extension of _reading_ data in our feature, rather than _writing_ it.
gmmorris marked this conversation as resolved.
Show resolved Hide resolved

### `read` privileges vs. `all` privileges
When a user is granted the `read` role in the Alerting Framework, they will be able to execute the following api calls:
- get
- getAlertState
- find

When a user is granted the `all` role in the Alerting Framework, they will be able to execute al lof the `read` privileged api calls, but in addition they'll be granted the following calls:
gmmorris marked this conversation as resolved.
Show resolved Hide resolved
- `create`
- `delete`
- `update`
- `enable`
- `disable`
- `updateApiKey`
- `muteAll`
- `unmuteAll`
- `muteInstance`
- `unmuteInstance`

Finally, all users, whether they're granted any role or not, are privileged to call the following:
- `listAlertTypes`, but the output is limited to displaying the AlertTypes the user is perivileged to `get`

Attempting to execute any operation the user isn't privileged to execute will result in an Authorization error throws by the AlertsClient.
gmmorris marked this conversation as resolved.
Show resolved Hide resolved

## Alert Navigation
When registering an Alert Type, you'll likely want to provide a way of viewing alerts of that type within your own plugin, or perhaps you want to provide a view for all alerts created from within your solution within your own UI.

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/alerts/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ export interface AlertingFrameworkHealth {
}

export const BASE_ALERT_API_PATH = '/api/alerts';
export const AlertsFeatureId = 'alerts';
gmmorris marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion x-pack/plugins/alerts/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
"version": "8.0.0",
"kibanaVersion": "kibana",
"configPath": ["xpack", "alerts"],
"requiredPlugins": ["licensing", "taskManager", "encryptedSavedObjects", "actions", "eventLog"],
"requiredPlugins": ["licensing", "taskManager", "encryptedSavedObjects", "actions", "eventLog", "features"],
"optionalPlugins": ["usageCollection", "spaces", "security"]
}
38 changes: 35 additions & 3 deletions x-pack/plugins/alerts/server/alert_type_registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,38 @@ describe('has()', () => {
});

describe('register()', () => {
test('throws if AlertType Id contains invalid characters', () => {
const alertType = {
id: 'test',
name: 'Test',
actionGroups: [
{
id: 'default',
name: 'Default',
},
],
defaultActionGroupId: 'default',
executor: jest.fn(),
producer: 'alerting',
};
// eslint-disable-next-line @typescript-eslint/no-var-requires
const registry = new AlertTypeRegistry(alertTypeRegistryParams);

const invalidCharacters = [' ', ':', '*', '*', '/'];
for (const char of invalidCharacters) {
expect(() => registry.register({ ...alertType, id: `${alertType.id}${char}` })).toThrowError(
new Error(`expected AlertType Id not to include invalid character: ${char}`)
);
}

const [first, second] = invalidCharacters;
expect(() =>
registry.register({ ...alertType, id: `${first}${alertType.id}${second}` })
).toThrowError(
new Error(`expected AlertType Id not to include invalid characters: ${first}, ${second}`)
);
});

test('registers the executor with the task manager', () => {
const alertType = {
id: 'test',
Expand Down Expand Up @@ -177,7 +209,7 @@ describe('list()', () => {
test('should return empty when nothing is registered', () => {
const registry = new AlertTypeRegistry(alertTypeRegistryParams);
const result = registry.list();
expect(result).toMatchInlineSnapshot(`Array []`);
expect(result).toMatchInlineSnapshot(`Set {}`);
Copy link
Member

Choose a reason for hiding this comment

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

With the change from Array to Set, we'll potentially lose some ordering we had with the Array - although in practice Sets may end up with the same "ordered as added" pattern that ordinary objects have. I don't think we have any dependencies on the order, but does make me wonder if we want them sorted by id in the HTTP response to make things more consistent. Nothing directly actionable, thought I'd mention it in case anyone else knows of any ordering dependency here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm we can... the reason I changed this to a set was that it made it cleaner and easier to check if a type was in there and it is a more "correct" description of what this API returns.

Leaving it as an array ended up making the code around it a bit messier, so this was done mostly to make the code easier to follow.

The effect on the order is interesting as, like you said, Set orders by insertion order, which is exactly what we had in the array before hand, so nothing changed here... we could add some kind of ordering but as it's the same as it was before, I'd defer that to another time.

});

test('should return registered types', () => {
Expand All @@ -197,7 +229,7 @@ describe('list()', () => {
});
const result = registry.list();
expect(result).toMatchInlineSnapshot(`
Array [
Set {
Object {
"actionGroups": Array [
Object {
Expand All @@ -214,7 +246,7 @@ describe('list()', () => {
"name": "Test",
"producer": "alerting",
},
]
}
`);
});

Expand Down
57 changes: 47 additions & 10 deletions x-pack/plugins/alerts/server/alert_type_registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import Boom from 'boom';
import { i18n } from '@kbn/i18n';
import { schema } from '@kbn/config-schema';
import typeDetect from 'type-detect';
import { RunContext, TaskManagerSetupContract } from '../../task_manager/server';
import { TaskRunnerFactory } from './task_runner';
import { AlertType } from './types';
Expand All @@ -15,6 +17,34 @@ interface ConstructorOptions {
taskRunnerFactory: TaskRunnerFactory;
}

export interface RegistryAlertType
extends Pick<
AlertType,
'name' | 'actionGroups' | 'defaultActionGroupId' | 'actionVariables' | 'producer'
> {
id: string;
}

/**
* AlertType IDs are used as part of the authorization strings used to
* grant users privileged operations. There is a limited range of characters
* we can use in these auth strings, so we apply these same limitations to
* the AlertType Ids.
* If you wish to change this, please confer with the Kibana security team.
gmmorris marked this conversation as resolved.
Show resolved Hide resolved
*/
const alertIdSchema = schema.string({
validate(value: string): string | void {
if (typeof value !== 'string') {
return `expected AlertType Id of type [string] but got [${typeDetect(value)}]`;
gmmorris marked this conversation as resolved.
Show resolved Hide resolved
} else if (!value.match(/^[a-zA-Z0-9_\-\.]*$/)) {
const invalid = value.match(/[^a-zA-Z0-9_\-\.]+/g)!;
return `expected AlertType Id not to include invalid character${
invalid.length > 1 ? `s` : ``
}: ${invalid?.join(`, `)}`;
}
},
});

export class AlertTypeRegistry {
private readonly taskManager: TaskManagerSetupContract;
private readonly alertTypes: Map<string, AlertType> = new Map();
Expand All @@ -41,7 +71,7 @@ export class AlertTypeRegistry {
);
}
alertType.actionVariables = normalizedActionVariables(alertType.actionVariables);
this.alertTypes.set(alertType.id, { ...alertType });
this.alertTypes.set(alertIdSchema.validate(alertType.id), { ...alertType });
this.taskManager.registerTaskDefinitions({
[`alerting:${alertType.id}`]: {
title: alertType.name,
Expand All @@ -66,15 +96,22 @@ export class AlertTypeRegistry {
return this.alertTypes.get(id)!;
}

gmmorris marked this conversation as resolved.
Show resolved Hide resolved
public list() {
return Array.from(this.alertTypes).map(([alertTypeId, alertType]) => ({
id: alertTypeId,
name: alertType.name,
actionGroups: alertType.actionGroups,
defaultActionGroupId: alertType.defaultActionGroupId,
actionVariables: alertType.actionVariables,
producer: alertType.producer,
}));
public list(): Set<RegistryAlertType> {
return new Set(
Array.from(this.alertTypes).map(
([id, { name, actionGroups, defaultActionGroupId, actionVariables, producer }]: [
string,
AlertType
]) => ({
id,
name,
actionGroups,
defaultActionGroupId,
actionVariables,
producer,
})
)
);
}
}

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/alerts/server/alerts_client.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const createAlertsClientMock = () => {
unmuteAll: jest.fn(),
muteInstance: jest.fn(),
unmuteInstance: jest.fn(),
listAlertTypes: jest.fn(),
};
return mocked;
};
Expand Down
Loading