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

[gmmorris]

Summary

This PR adds Role Based Access-Control to the Alerting framework & Actions feature using Kibana Feature Controls, addressing most of the Meta issue: #43994

This also closes #62438

This PR includes the following:

1. Adds alerting specific Security Actions (not to be confused with Alerting Actions) to the security plugin which allows us to assign alerting specific privileges to users of other plugins using the features plugin.
2. Removes the security wrapper from the savedObjectsClient in AlertsClient and instead plugs in the new AlertsAuthorization which performs the privilege checks on each api call made to the AlertsClient.
3. Adds privileges in each plugin that is already using the Alerting Framework which mirror (as closely as possible) the existing api-level tag-based privileges and plugs them into the AlertsClient.
4. Adds feature granted privileges arounds Actions (by relying on Saved Object privileges under the hood) and plugs them into the ActionsClient
5. Removes the legacy api-level tag-based privilege system from both the Alerts and Action HTTP APIs

DevDocs

This PR allows you to assign privileges to the Alerting framework when defining your feature in Kibana. When registering your feature you can add a list of AlertTypes under your read and all keys of the privileges object, like so:

features.registerFeature({
      id: 'alertsExample',
      name: 'alertsExample',
      app: [],
      privileges: {
        all: {
          alerting: {
            all: ['example.always-firing', 'example.people-in-space'],
          },
        },
        read: {
          alerting: {
            read: ['example.always-firing', 'example.people-in-space'],
          },
        },
      },
    });

What this specifies, in detail, is that:

1. If a user has been granted the all privilege to the alertsExample feature, then they are also granted all privileges to the example.always-firing and example.people-in-space AlertTypes under the alertsExample consumer
2. If a user has been granted the read privilege to the alertsExample feature, then they are also granted read privileges to the example.always-firing and example.people-in-space AlertTypes under the alertsExample consumer

That means that an all user will be able to, for example, create an example.always-firing alert with the alertsExample as consumer. This will also automatically grant them the right to create an example.always-firing alert from within alerts management where alerts is the consumer (we special case Alerts management).
What this doesn't grant the user is the ability to create an example.always-firing alert under any other consumer. For that the specific consumer will have to grant the user explicit rights through their privilege system.

For example, if Uptime wanted to allow users to create an example.people-in-space alert inside of the Uptime solution, then they will have to do the following:

features.registerFeature({
      id: 'uptime',
      name: 'Uptime',
      app: [],
      privileges: {
        all: {
          alerting: {
            all: ['xpack.uptime.alerts.actionGroups.tls', 'example.people-in-space'],
          },
        },
        read: {
          alerting: {
            read: ['xpack.uptime.alerts.actionGroups.tls', 'example.people-in-space'],
          },
        },
      },
    });

This, assuming it's added by Uptime, would grant uptime users the privilege to create both their own xpack.uptime.alerts.actionGroups.tls alert and the example.people-in-space alert with uptime as the consumer.

You might ask, will this allow any Uptime user with all privileges to create an example.people-in-space alert? No.
In order to create an example.people-in-space alert the Uptime user needs both all in Uptime and in AlertsExample as we always check whether the user is privileged to execute an operation (create/enable/delete etc.) in both the alert's consumer and its producer.

The one exception to this is when the producer is alerts, which represents a built-in AlertType, in which case we only check for consumer privileges as all users are privileged to create built-in types by definition.

Checklist

Delete any items that are not applicable to this PR.

• [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• This was checked for keyboard-only and screenreader accessibility
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
• This was checked for cross-browser compatibility, including a check against IE11

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[gmmorris]

I wasn't able to get as far as I wanted with this review today, but will pick back up on Monday.

Doing some quick testing with user having all access to the Security Solution, and no privileges to the Actions feature. When trying to create an alert within the solution, I'm unable to load the list of available connectors, despite having one configured:

Hmm this is an interesting one.
This will likely require some dev work on SIEM's side, as they aren't using the flyout (I hadn't realised this) and they're probably better off waiting until this is merged and addressing it then. This is why we decided to wait until after 7.9FF to merge this - giving solutions time to make any small fixes they might need to in response to the change.
cc @dhurley14 - I can guide you through fixing this.

[gmmorris]

cc @dhurley14 - I can guide you through fixing this.

I had a chat with @dhurley14 about this and we'll move forward with merging the code despite this known regression as it'll be easier for the SIEM team to address it once it's merged into master.

[pmuellr]

reviewed the files that have changed since my last review; LGTM

[pmuellr]

re: Duplicate authorization checks

You can already see some of this now when you peek at our APM results - in practice, I think it's fine for duplicate checks, except where it's really going to hurt, likely in GET requests (viewing alerts in the web UI) and alert function execution. I'm not seeing these take very long in practice, and it's pretty easy to see them from the APM transaction log, so I'm not worried.

[apmmachine]

💚 Build Succeeded

Expand to view the summary

Build stats

• Build Cause: [Pull request #67157 updated]

• Start Time: 2020-07-21T14:44:48.551+0000

• Duration: 24 min 0 sec

Test stats 🧪

Test	Results
Failed	0
Passed	9
Skipped	0
Total	9

[gmmorris]

re: Duplicate authorization checks

I've now removed the securityWrapper in Actions - we only use it for the SO client we pass to the action itself.

@legrego @pmuellr

[legrego]

Awesome work! 🏅 🥇 👏

I don't want to block your merge on this, but I would really like to see a security_only functional test suite within your API integration tests as well. Some users choose to disable Spaces while still securing their cluster, and having assurances that we don't break anything here would be valuable.

[gmmorris]

Awesome work! 🏅 🥇 👏

I don't want to block your merge on this, but I would really like to see a security_only functional test suite within your API integration tests as well. Some users choose to disable Spaces while still securing their cluster, and having assurances that we don't break anything here would be valuable.

We agree! :)
It's in the backlog: #49825

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 53aa8e9

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
triggers_actions_ui	152	+1	151

async chunks size

id	value	diff	baseline
infra	3.6MB	+14.0B	3.6MB
triggers_actions_ui	709.4KB	+6.6KB	702.8KB
total	-	+6.6KB	-

page load bundle size

id	value	diff	baseline
alerts	88.6KB	+124.0B	88.5KB
features	13.4KB	+65.0B	13.4KB
triggers_actions_ui	266.8KB	+2.1KB	264.7KB
total	-	+2.3KB	-

History

• 💚 Build #63350 succeeded e794518
• 💚 Build #63307 succeeded 7bafd5d
• 💔 Build #63272 failed 407b09a
• 💚 Build #63071 succeeded 7febd47
• 💔 Build #62969 failed a894e5a

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