Action registration / framework level license check

[peterschretlen]

We anticipate making a wider variety of action types available ( see #45023 ) which may be available at different license levels.

In addition, 3rd parties can create and register custom actions, and the ability to register custom actions should also be subject to a license check. This implies:

• an additional check at the framework level, in addition to checks on an action-by-action basis.
• a way to differentiate built-in actions from custom ones (such that built-in actions could be offered at a lower license level than custom ones)
• making sure the license is available at startup when actions are registered. I believe it is, but this needs to be validated. [Updated: a better option may be to do a runtime check, which may provide more flexibility for trial and license changes ]

[elasticmachine]

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

[pmuellr]

Getting a list of existing built-in action types should be easy.

I think we'll then have a couple of different flavors of "availability":

• each builtin actionType will have a license level where it can be used

• 3rd party actionTypes will have a single license level at which they can all be used - or does it make sense to have 3rd party actionTypes that could be used at different license levels? Ignoring a 3rd party actionType that might be indirectly dependent on a license level because of a feature they depend on.

For actionTypes that aren't "built-in" in that they aren't defined in the action plugin, but in some other plugin, I think we want to consider these as "built-in", so I think we'll have to hard-code these somehow in the actions plugin. We don't have any of these yet ...

[peterschretlen]

There was some discussion that an execution-time check might be better than a registration time check, because it simplifies cases like trial licenses, license upgrade/downgrade - you don't have to restart Kibana. So you'd be able to register any action you want, but it may not execute based on the default action license level.

So maybe the simplest option is just to have a "default" action license level? Any action that gets registered can optionally set itself higher than the default, but not lower. If we want it to be lower we have to put it in a list of exceptions in the actions framework.

[pmuellr]

Ya, seems like execution-time check would be good, would it make sense to do both? Seems confusing to be able to create an action that would fail execution because of license checks. In the UI, we could provide some kind of warning, but we don't really have a concept of doing that with the REST API.

I guess the question is, what scenario breaks if we do both creation-time and execution-time checks?

[peterschretlen]

Seems confusing to be able to create an action that would fail execution because of license checks.

Yeah good point - I guess the checks have to be applied in a few places (action creation, action update, listing action types) and not just execution.

I guess the question is, what scenario breaks if we do both creation-time and execution-time checks?

Upgrade and start-trial scenarios don't work if we enforce at both startup/registration time & execution-time. You would upgrade your license, but not have access to any of the new actions until a restart? Downgrade and end-trial scenarios should would work OK though.

[pmuellr]

Ya, seems like any kind of check we'd do at startup/registration would make the "live upgrade" scenario not work. The checks should be made for any activity that would mutate or execute an action. So that when a customer upgrades, the check will be made after that, and should work with the upgraded privileges.

If you don't have the "sufficient privileges" seems like you should only be allowed to read/delete actions. If you end up getting downgrade privileges (trial expires), you shouldn't be able to create, update, or execute an action, but I think it makes sense for a customer to see them and probably delete them. They certainly shouldn't be deleted automagically or anything, in case the customer goes from trial -> trial expired -> buys a gold license; they'd just have some "action down time" from when the trial expired to buying a license.

[mikecote]

I've put some thought into this and wrote up a summary below of what we should confirm before implementing this. Looking forward to everyone's thoughts!

1. Add a minimum license attribute to action type definition
2. Define a minimum license requirement in each built in action type
3. Modify plugins.registerType(...) to enforce a gold+ license on every registered action type (this will force on startup that each 3rd party action type requires a gold+ license minimum regardless of the current license level)
4. Doing a POST /api/action/{id}/_execute should either throw an error about action not found or an error about failed license check?
5. Calling plugins.execute(…) (ex: from alerting) should either throw an error about action not found or failed license check? (see item # 7 to prevent these errors)
6. Task runner should fail silently and log to the console? (actions that are scheduled to run but didn't yet)
7. Alerting framework should not call plugins.execute(…) on actions the alert doesn’t support at runtime. Log a message as well? (this allows for example trial -> basic -> paid subscription to resume usage of those actions)
8. Alerting APIs should hide from actions array the actions that aren't within their license level? (so they don't show up in the UIs). Then delete the out of license actions on next save? or keep those there?
9. Doing a POST /api/action should throw an error about actionTypeId not found like if it really wasn’t there
10. Doing a DELETE /api/action should let them delete their action?
11. Doing a GET /api/action/{id} should return not found error
12. Doing a GET /api/action should exclude actions out of license level
13. Doing a GET /api/action/types should exclude action types out of their license level
14. Doing a PUT /api/action/{id} should return a not found error?

Feel free to ask for any clarifications on the points above.

cc @elastic/kibana-alerting-services @peterschretlen @bmcconaghy

[pmuellr]

3. Modify plugins.registerType(...) to enforce a gold+ license on every registered action type (this will force on startup that each 3rd party action type requires a gold+ license minimum regardless of the current license level)

You mean after the built-ins have been registered? Presumably server-log and index are basic, so should NOT be registered as gold. So 3rd party action types are always gold? Would we ever have plugins that want to register Basic actions? Guessing the answer is no, or we would move those into the the actions plugin, as they are presumably fairly simple. We'll need to doc this behavior, could be confusing for plugin authors writing their own actions.

[pmuellr]

Things like "GET /api/action/{id} should return not found error" don't feel right at all.

We already maintain a list of "disabled" actionTypes, via config, and that state is returned in the actionsClient and HTTP "list" functions. I believe it has some of the same constraints that we need here - if you had an action whose actionType is later disabled, it of course won't run it, but is it an error, or not found, or ??? We probably want the same thing if the action can't be used because of the license.

Here's the config:

kibana/x-pack/plugins/actions/server/config.ts

Lines 18 to 22 in 77fe83e

	enabledActionTypes: schema.arrayOf(
	schema.oneOf([schema.string(), schema.literal(EnabledActionTypes.Any)]),
	{
	defaultValue: [WhitelistedHosts.Any],
	}

We've already added checks for config-based disablement, here, which is used in presumably the same places we'd be making license checks:

kibana/x-pack/plugins/actions/server/actions_config.ts

Lines 74 to 82 in 7e08763

	function isActionTypeEnabledInConfig(
	{ enabledActionTypes }: ActionsConfigType,
	actionType: string
	): boolean {
	const enabled = new Set(enabledActionTypes);
	if (enabled.has(EnabledActionTypes.Any)) return true;
	if (enabled.has(actionType)) return true;
	return false;
	}

It seems like we could extend the notion of "disabled" here to include the license check as well, enable the appropriate error to be logged if it's disabled (via config, or via license). We'd probably want the list types function to return separate boolean values for disabled via config and disabled via license.

Logging license issues would be good, but one of those things that we probably just want to log once per Kibana process, per actionType we find someone trying to use, that has license issues.

[mikecote]

3. Modify plugins.registerType(...) to enforce a gold+ license on every registered action type (this will force on startup that each 3rd party action type requires a gold+ license minimum regardless of the current license level)

You mean after the built-ins have been registered? Presumably server-log and index are basic, so should NOT be registered as gold. So 3rd party action types are always gold? Would we ever have plugins that want to register Basic actions? Guessing the answer is no, or we would move those into the the actions plugin, as they are presumably fairly simple. We'll need to doc this behavior, could be confusing for plugin authors writing their own actions.

plugins.registerType(...) sits on top of actionTypeRegistry.register(...) where external plugins call to register their extra action types. The built ins register directly with the actionTypeRegistry. I can see what you mean though about other plugins within Kibana wanting to offer their action types within Basic. Maybe we'll need some sort of whitelist or a hardcoded list of directories we register ourselves. 🤔

We already maintain a list of "disabled" actionTypes, via config, and that state is returned in the actionsClient and HTTP "list" functions. I believe it has some of the same constraints that we need here - if you had an action whose actionType is later disabled, it of course won't run it, but is it an error, or not found, or ??? We probably want the same thing if the action can't be used because of the license.

I totally forgot about those! I see nothing wrong piggy backing on this feature.

[peterschretlen]

• Doing a POST /api/action/{id}/_execute should either throw an error about action not found or an error about failed license check?
• Doing a POST /api/action should throw an error about actionTypeId not found like if it really wasn’t there
• Doing a PUT /api/action/{id} should return a not found error?

For APIs, rather than returning 404 not found when accessing actions outside the current license, could we return 403 forbidden instead ? I think that provides more clarity about the failure.

• Doing a GET /api/action/{id} should return not found error

I don't think there's any harm in being able to fetch this data? I think it would be strange to have a unauthorized connector still exist yet its details are inaccessible.

• Alerting APIs should hide from actions array the actions that aren't within their license level? (so they don't show up in the UIs). Then delete the out of license actions on next save? or keep those there?
• Doing a GET /api/action should exclude actions out of license level
• Doing a GET /api/action/types should exclude action types out of their license level

I think we should let API clients filter the list, and not filter it ourselves. Or we provide a flag to the API to include unauthorized actions.

In the UI I think whether we show them or not will depend on the context

• In the alert flyout, it makes little sense to include unauthorized actions in the dropdown, since they can't be used.
• The connectors management view, it would be very helpful to see which connectors have been disabled, and how many alerts they are still attached to.
• In the action type list I think we could go either way. If they can be shown as disabled, there's value in showing what actions people could have access to in a trial or upgrade. @arisonl may have thoughts here. Again I think we can leave this decision to the API client, and not the API.

[mikecote]

Thanks for the feedback @pmuellr and @peterschretlen. This all makes sense to me and will use it as a starting point 👍