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

[Actions] Do not return system actions from GET APIs #161845

Merged
merged 12 commits into from
Jul 20, 2023
387 changes: 354 additions & 33 deletions x-pack/plugins/actions/server/actions_client.test.ts

Large diffs are not rendered by default.

54 changes: 48 additions & 6 deletions x-pack/plugins/actions/server/actions_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,15 @@ export interface UpdateOptions {
action: ActionUpdate;
}

interface GetAllOptions {
includeSystemActions?: boolean;
}

interface ListTypesOptions {
featureId?: string;
includeSystemActionTypes?: boolean;
}

export class ActionsClient {
private readonly logger: Logger;
private readonly kibanaIndices: string[];
Expand Down Expand Up @@ -410,7 +419,14 @@ export class ActionsClient {

const foundInMemoryConnector = this.inMemoryConnectors.find((connector) => connector.id === id);

if (foundInMemoryConnector !== undefined) {
/**
* Getting system connector is not allowed
*/
if (foundInMemoryConnector !== undefined && foundInMemoryConnector.isSystemAction) {
throw Boom.notFound(`Connector ${id} not found`);
}

if (foundInMemoryConnector !== undefined && foundInMemoryConnector.isPreconfigured) {
this.auditLogger?.log(
connectorAuditEvent({
action: ConnectorAuditAction.GET,
Expand Down Expand Up @@ -452,7 +468,9 @@ export class ActionsClient {
/**
* Get all actions with in-memory connectors
*/
public async getAll(): Promise<FindActionResult[]> {
public async getAll({ includeSystemActions = false }: GetAllOptions = {}): Promise<
FindActionResult[]
> {
try {
await this.authorization.ensureAuthorized('get');
} catch (error) {
Expand Down Expand Up @@ -483,9 +501,13 @@ export class ActionsClient {
)
);

const inMemoryConnectorsFiltered = includeSystemActions
? this.inMemoryConnectors
: this.inMemoryConnectors.filter((connector) => !connector.isSystemAction);

const mergedResult = [
...savedObjectsActions,
...this.inMemoryConnectors.map((inMemoryConnector) => ({
...inMemoryConnectorsFiltered.map((inMemoryConnector) => ({
id: inMemoryConnector.id,
actionTypeId: inMemoryConnector.actionTypeId,
name: inMemoryConnector.name,
Expand Down Expand Up @@ -523,7 +545,14 @@ export class ActionsClient {
(inMemoryConnector) => inMemoryConnector.id === actionId
);

if (action !== undefined) {
/**
* Getting system connector is not allowed
*/
if (action !== undefined && action.isSystemAction) {
throw Boom.notFound(`Connector ${action.id} not found`);
}

if (action !== undefined && action.isPreconfigured) {
actionResults.push(action);
}
}
Expand Down Expand Up @@ -795,8 +824,21 @@ export class ActionsClient {
return this.ephemeralExecutionEnqueuer(this.unsecuredSavedObjectsClient, options);
}

public async listTypes(featureId?: string): Promise<ActionType[]> {
return this.actionTypeRegistry.list(featureId);
/**
* Return all available action types
* expect system action types
*/
public async listTypes({
featureId,
includeSystemActionTypes = false,
}: ListTypesOptions = {}): Promise<ActionType[]> {
const actionTypes = this.actionTypeRegistry.list(featureId);

const filteredActionTypes = includeSystemActionTypes
? actionTypes
: actionTypes.filter((actionType) => !Boolean(actionType.isSystemActionType));

return filteredActionTypes;
}

public isActionTypeEnabled(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,9 @@ describe('connectorTypesRoute', () => {
expect(actionsClient.listTypes).toHaveBeenCalledTimes(1);
expect(actionsClient.listTypes.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"alerting",
Object {
"featureId": "alerting",
},
]
`);

Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/actions/server/routes/connector_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const connectorTypesRoute = (
verifyAccessAndContext(licenseState, async function (context, req, res) {
const actionsClient = (await context.actions).getActionsClient();
return res.ok({
body: rewriteBodyRes(await actionsClient.listTypes(req.query?.feature_id)),
body: rewriteBodyRes(await actionsClient.listTypes({ featureId: req.query?.feature_id })),
});
})
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import { BASE_ACTION_API_PATH } from '../../../common';
import { ActionsRequestHandlerContext } from '../../types';
import { trackLegacyRouteUsage } from '../../lib/track_legacy_route_usage';

/**
* Return all available action types
* expect system action types
*/
export const listActionTypesRoute = (
router: IRouter<ActionsRequestHandlerContext>,
licenseState: ILicenseState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ export default function getActionTests({ getService }: FtrProviderContext) {
}
});

it('should handle get system action request appropriately', async () => {
it('should throw when requesting a system action', async () => {
const response = await supertestWithoutAuth
.get(
`${getUrlPrefix(space.id)}/api/actions/connector/system-connector-test.system-action`
Expand All @@ -183,14 +183,11 @@ export default function getActionTests({ getService }: FtrProviderContext) {
case 'superuser at space1':
case 'space_1_all at space1':
case 'space_1_all_with_restricted_fixture at space1':
expect(response.statusCode).to.eql(200);
expect(response.statusCode).to.eql(404);
expect(response.body).to.eql({
id: 'system-connector-test.system-action',
connector_type_id: 'test.system-action',
name: 'System action: test.system-action',
is_preconfigured: false,
is_system_action: true,
is_deprecated: false,
statusCode: 404,
error: 'Not Found',
message: 'Connector system-connector-test.system-action not found',
});
break;
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,6 @@ export default function getAllActionTests({ getService }: FtrProviderContext) {
name: 'Slack#xyz',
referenced_by_count: 0,
},
{
connector_type_id: 'test.system-action',
id: 'system-connector-test.system-action',
is_deprecated: false,
is_preconfigured: false,
is_system_action: true,
name: 'System action: test.system-action',
referenced_by_count: 0,
},
{
id: 'custom-system-abc-connector',
is_preconfigured: true,
Expand Down Expand Up @@ -294,15 +285,6 @@ export default function getAllActionTests({ getService }: FtrProviderContext) {
name: 'Slack#xyz',
referenced_by_count: 0,
},
{
connector_type_id: 'test.system-action',
id: 'system-connector-test.system-action',
is_deprecated: false,
is_preconfigured: false,
is_system_action: true,
name: 'System action: test.system-action',
referenced_by_count: 0,
},
{
id: 'custom-system-abc-connector',
is_preconfigured: true,
Expand Down Expand Up @@ -426,15 +408,6 @@ export default function getAllActionTests({ getService }: FtrProviderContext) {
name: 'Slack#xyz',
referenced_by_count: 0,
},
{
connector_type_id: 'test.system-action',
id: 'system-connector-test.system-action',
is_deprecated: false,
is_preconfigured: false,
is_system_action: true,
name: 'System action: test.system-action',
referenced_by_count: 0,
},
{
id: 'custom-system-abc-connector',
is_preconfigured: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@ export default function listActionTypesTests({ getService }: FtrProviderContext)
).to.be(true);
});

it('should filter out system action types', async () => {
const response = await supertest.get(
`${getUrlPrefix(Spaces.space1.id)}/api/actions/connector_types`
);

const actionTypes = response.body as Array<{ is_system_action_type: boolean }>;

expect(actionTypes.every((actionType) => !actionType.is_system_action_type)).to.be(true);
});

describe('legacy', () => {
it('should return 200 with list of action types containing defaults', async () => {
const response = await supertest.get(
Expand All @@ -53,6 +63,16 @@ export default function listActionTypesTests({ getService }: FtrProviderContext)
response.body.some(createActionTypeMatcher('test.index-record', 'Test: Index Record'))
).to.be(true);
});

it('should filter out system action types', async () => {
const response = await supertest.get(
`${getUrlPrefix(Spaces.space1.id)}/api/actions/list_action_types`
);

const actionTypes = response.body as Array<{ is_system_action_type: boolean }>;

expect(actionTypes.every((actionType) => !actionType.is_system_action_type)).to.be(true);
});
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,14 @@ export default function getActionTests({ getService }: FtrProviderContext) {
});
});

it('should handle get a system connector', async () => {
it('should return 404 when trying to get a system connector', async () => {
await supertest
.get(
`${getUrlPrefix(
Spaces.space1.id
)}/api/actions/connector/system-connector-test.system-action`
)
.expect(200, {
id: 'system-connector-test.system-action',
connector_type_id: 'test.system-action',
name: 'System action: test.system-action',
is_preconfigured: false,
is_system_action: true,
is_deprecated: false,
});
.expect(404);
});

it('should handle get a deprecated connector', async () => {
Expand Down Expand Up @@ -206,21 +199,14 @@ export default function getActionTests({ getService }: FtrProviderContext) {
});
});

it('should handle get a system connector', async () => {
it('should return 404 when trying to get a system connector', async () => {
await supertest
.get(
`${getUrlPrefix(
Spaces.space1.id
)}/api/actions/action/system-connector-test.system-action`
)
.expect(200, {
id: 'system-connector-test.system-action',
actionTypeId: 'test.system-action',
name: 'System action: test.system-action',
isPreconfigured: false,
isSystemAction: true,
isDeprecated: false,
});
.expect(404);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,6 @@ export default function getAllActionTests({ getService }: FtrProviderContext) {
name: 'Slack#xyz',
referenced_by_count: 0,
},
{
connector_type_id: 'test.system-action',
id: 'system-connector-test.system-action',
is_deprecated: false,
is_preconfigured: false,
is_system_action: true,
name: 'System action: test.system-action',
referenced_by_count: 0,
},
{
id: 'custom-system-abc-connector',
is_preconfigured: true,
Expand Down Expand Up @@ -235,15 +226,6 @@ export default function getAllActionTests({ getService }: FtrProviderContext) {
name: 'Slack#xyz',
referenced_by_count: 0,
},
{
connector_type_id: 'test.system-action',
id: 'system-connector-test.system-action',
is_deprecated: false,
is_preconfigured: false,
is_system_action: true,
name: 'System action: test.system-action',
referenced_by_count: 0,
},
{
id: 'custom-system-abc-connector',
is_preconfigured: true,
Expand Down Expand Up @@ -370,15 +352,6 @@ export default function getAllActionTests({ getService }: FtrProviderContext) {
name: 'Slack#xyz',
referencedByCount: 0,
},
{
actionTypeId: 'test.system-action',
id: 'system-connector-test.system-action',
isDeprecated: false,
isPreconfigured: false,
isSystemAction: true,
name: 'System action: test.system-action',
referencedByCount: 0,
},
{
id: 'custom-system-abc-connector',
isPreconfigured: true,
Expand Down