Skip to content

Commit

Permalink
Enhancements
Browse files Browse the repository at this point in the history
  • Loading branch information
cnasikas committed Jul 9, 2023
1 parent eca005d commit 0020ee9
Show file tree
Hide file tree
Showing 12 changed files with 47 additions and 104 deletions.
2 changes: 1 addition & 1 deletion x-pack/plugins/actions/server/action_type_registry.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const createActionTypeRegistryMock = () => {
isActionExecutable: jest.fn(),
isSystemActionType: jest.fn(),
getUtils: jest.fn(),
getSystemActionRequiredKibanaPrivileges: jest.fn(),
getSystemActionKibanaPrivileges: jest.fn(),
};
return mocked;
};
Expand Down
33 changes: 27 additions & 6 deletions x-pack/plugins/actions/server/action_type_registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ describe('actionTypeRegistry', () => {
});
});

describe('getSystemActionRequiredKibanaPrivileges()', () => {
describe('getSystemActionKibanaPrivileges()', () => {
it('should get the kibana privileges correctly for system actions', () => {
const registry = new ActionTypeRegistry(actionTypeRegistryParams);

Expand All @@ -699,7 +699,7 @@ describe('actionTypeRegistry', () => {
name: 'Cases',
minimumLicenseRequired: 'platinum',
supportedFeatureIds: ['alerting'],
kibanaPrivileges: ['cases/create'],
kibanaPrivileges: ['test/create'],
validate: {
config: { schema: schema.object({}) },
secrets: { schema: schema.object({}) },
Expand All @@ -709,8 +709,8 @@ describe('actionTypeRegistry', () => {
executor,
});

const result = registry.getSystemActionRequiredKibanaPrivileges('.cases');
expect(result).toEqual(['cases/create']);
const result = registry.getSystemActionKibanaPrivileges('.cases');
expect(result).toEqual(['test/create']);
});

it('should return an empty array if the system action does not define any kibana privileges', () => {
Expand All @@ -730,7 +730,7 @@ describe('actionTypeRegistry', () => {
executor,
});

const result = registry.getSystemActionRequiredKibanaPrivileges('.cases');
const result = registry.getSystemActionKibanaPrivileges('.cases');
expect(result).toEqual([]);
});

Expand All @@ -750,7 +750,28 @@ describe('actionTypeRegistry', () => {
executor,
});

const result = registry.getSystemActionRequiredKibanaPrivileges('foo');
const result = registry.getSystemActionKibanaPrivileges('foo');
expect(result).toEqual([]);
});

it('should return an empty array if the action type is not a system action but defines kibana privileges', () => {
const registry = new ActionTypeRegistry(actionTypeRegistryParams);

registry.register({
id: 'foo',
name: 'Foo',
minimumLicenseRequired: 'basic',
supportedFeatureIds: ['alerting'],
kibanaPrivileges: ['test/create'],
validate: {
config: { schema: schema.object({}) },
secrets: { schema: schema.object({}) },
params: { schema: schema.object({}) },
},
executor,
});

const result = registry.getSystemActionKibanaPrivileges('foo');
expect(result).toEqual([]);
});
});
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/actions/server/action_type_registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export class ActionTypeRegistry {
/**
* Returns the kibana privileges of an action type
*/
public getSystemActionRequiredKibanaPrivileges(actionTypeId: string): string[] {
public getSystemActionKibanaPrivileges(actionTypeId: string): string[] {
const actionType = this.actionTypes.get(actionTypeId);

if (!actionType?.isSystemActionType) {
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/actions/server/actions_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2831,7 +2831,7 @@ describe('execute()', () => {
name: 'Cases',
minimumLicenseRequired: 'platinum',
supportedFeatureIds: ['alerting'],
kibanaPrivileges: ['cases/create'],
kibanaPrivileges: ['test/create'],
validate: {
config: { schema: schema.object({}) },
secrets: { schema: schema.object({}) },
Expand All @@ -2848,7 +2848,7 @@ describe('execute()', () => {

expect(authorization.ensureAuthorized).toHaveBeenCalledWith({
operation: 'execute',
additionalPrivileges: ['cases/create'],
additionalPrivileges: ['test/create'],
});
});

Expand Down Expand Up @@ -2896,7 +2896,7 @@ describe('execute()', () => {
name: 'Cases',
minimumLicenseRequired: 'platinum',
supportedFeatureIds: ['alerting'],
kibanaPrivileges: ['cases/create'],
kibanaPrivileges: ['test/create'],
validate: {
config: { schema: schema.object({}) },
secrets: { schema: schema.object({}) },
Expand Down
14 changes: 7 additions & 7 deletions x-pack/plugins/actions/server/actions_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -718,15 +718,13 @@ export class ActionsClient {
return await this.unsecuredSavedObjectsClient.delete('action', id);
}

private getSystemActionRequiredKibanaPrivileges(actionId: string) {
private getSystemActionKibanaPrivileges(actionId: string) {
const inMemoryConnector = this.inMemoryConnectors.find(
(connector) => connector.id === actionId
);

const additionalPrivileges = inMemoryConnector?.isSystemAction
? this.actionTypeRegistry.getSystemActionRequiredKibanaPrivileges(
inMemoryConnector.actionTypeId
)
? this.actionTypeRegistry.getSystemActionKibanaPrivileges(inMemoryConnector.actionTypeId)
: [];

return additionalPrivileges;
Expand All @@ -744,7 +742,7 @@ export class ActionsClient {
(await getAuthorizationModeBySource(this.unsecuredSavedObjectsClient, source)) ===
AuthorizationMode.RBAC
) {
const additionalPrivileges = this.getSystemActionRequiredKibanaPrivileges(actionId);
const additionalPrivileges = this.getSystemActionKibanaPrivileges(actionId);
await this.authorization.ensureAuthorized({ operation: 'execute', additionalPrivileges });
} else {
trackLegacyRBACExemption('execute', this.usageCounter);
Expand All @@ -768,7 +766,8 @@ export class ActionsClient {
) {
/**
* For scheduled executions the additional authorization check
* will be performed inside the ActionExecutor at execution time
* for system actions (kibana privileges) will be performed
* inside the ActionExecutor at execution time
*/
await this.authorization.ensureAuthorized({ operation: 'execute' });
} else {
Expand All @@ -791,7 +790,8 @@ export class ActionsClient {
if (authCounts[AuthorizationMode.RBAC] > 0) {
/**
* For scheduled executions the additional authorization check
* will be performed inside the ActionExecutor at execution time
* for system actions (kibana privileges) will be performed
* inside the ActionExecutor at execution time
*/
await this.authorization.ensureAuthorized({ operation: 'execute' });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ describe('ensureAuthorized', () => {
await actionsAuthorization.ensureAuthorized({
operation: 'execute',
actionTypeId: 'myType',
additionalPrivileges: ['cases/create'],
additionalPrivileges: ['test/create'],
});

expect(authorization.actions.savedObject.get).toHaveBeenCalledWith(
Expand All @@ -224,7 +224,7 @@ describe('ensureAuthorized', () => {
kibana: [
mockAuthorizationAction(ACTION_SAVED_OBJECT_TYPE, 'get'),
mockAuthorizationAction(ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE, 'create'),
'cases/create',
'test/create',
],
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ export interface ConstructorOptions {
}

const operationAlias: Record<string, (authorization: SecurityPluginSetup['authz']) => string[]> = {
execute: (authorization, additionalPrivileges: string[] = []) => [
execute: (authorization) => [
authorization.actions.savedObject.get(ACTION_SAVED_OBJECT_TYPE, 'get'),
authorization.actions.savedObject.get(ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE, 'create'),
...additionalPrivileges,
],
list: (authorization) => [
authorization.actions.savedObject.get(ACTION_SAVED_OBJECT_TYPE, 'find'),
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/actions/server/lib/action_executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ test('successfully authorize system actions', async () => {
name: 'Cases',
minimumLicenseRequired: 'platinum',
supportedFeatureIds: ['alerting'],
kibanaPrivileges: ['cases/create'],
kibanaPrivileges: ['test/create'],
isSystemActionType: true,
validate: {
config: { schema: schema.any() },
Expand All @@ -824,13 +824,13 @@ test('successfully authorize system actions', async () => {

actionTypeRegistry.get.mockReturnValueOnce(actionType);
actionTypeRegistry.isSystemActionType.mockReturnValueOnce(true);
actionTypeRegistry.getSystemActionRequiredKibanaPrivileges.mockReturnValueOnce(['cases/create']);
actionTypeRegistry.getSystemActionKibanaPrivileges.mockReturnValueOnce(['test/create']);

await actionExecutor.execute({ ...executeParams, actionId: 'system-connector-.cases' });

expect(authorizationMock.ensureAuthorized).toBeCalledWith({
operation: 'execute',
additionalPrivileges: ['cases/create'],
additionalPrivileges: ['test/create'],
});
});

Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/actions/server/lib/action_executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ export class ActionExecutor {
*/
if (actionTypeRegistry.isSystemActionType(actionTypeId)) {
const additionalPrivileges =
actionTypeRegistry.getSystemActionRequiredKibanaPrivileges(actionTypeId);
actionTypeRegistry.getSystemActionKibanaPrivileges(actionTypeId);
authorization.ensureAuthorized({ operation: 'execute', additionalPrivileges });
}

Expand Down
29 changes: 0 additions & 29 deletions x-pack/plugins/cases/common/api/saved_object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,32 +19,3 @@ export const NumberFromString = new rt.Type<number, string, unknown>(
}),
String
);

export const PaginationSchema = new rt.Type<
{ page?: number; perPage?: number },
{ page?: number; perPage?: number },
unknown
>(
'Test',
rt.partial({ page: rt.number, perPage: rt.number }).is,
(u, c) =>
either.chain(rt.partial({ page: rt.number, perPage: rt.number }).validate(u, c), (params) => {
if (params.page == null && params.perPage) {
return rt.success(params);
}

const pageAsNumber = params.page ?? 0;
const perPageAsNumber = params.perPage ?? 0;

if (Math.max(pageAsNumber, pageAsNumber * perPageAsNumber) > 10) {
return rt.failure(
u,
c,
`The number of documents is too high. Paginating through more than ${10} documents is not possible.`
);
}

return rt.success(params);
}),
rt.identity
);
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ export default function ({ getService }: FtrProviderContext) {
}
});

it('should authenticate correctly system actions with kibana privileges', async () => {
it('should authorize system actions correctly', async () => {
const connectorId = 'system-connector-test.system-action-kibana-privileges';
const name = 'System action: test.system-action-kibana-privileges';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,30 +150,6 @@ export default function updateActionTests({ getService }: FtrProviderContext) {
});
});

it(`shouldn't update a system connector`, async () => {
await supertest
.put(
`${getUrlPrefix(
Spaces.space1.id
)}/api/actions/connector/system-connector-test.system-action`
)
.set('kbn-xsrf', 'foo')
.send({
name: 'My action updated',
config: {
unencrypted: `This value shouldn't get encrypted`,
},
secrets: {
encrypted: 'This value should be encrypted',
},
})
.expect(400, {
statusCode: 400,
error: 'Bad Request',
message: 'System action system-connector-test.system-action is not allowed to update.',
});
});

it('should notify feature usage when editing a gold action type', async () => {
const { body: createdAction } = await supertest
.post(`${getUrlPrefix(Spaces.space1.id)}/api/actions/connector`)
Expand Down Expand Up @@ -362,30 +338,6 @@ export default function updateActionTests({ getService }: FtrProviderContext) {
});
});

it(`shouldn't update a system connector`, async () => {
await supertest
.put(
`${getUrlPrefix(
Spaces.space1.id
)}/api/actions/action/system-connector-test.system-action`
)
.set('kbn-xsrf', 'foo')
.send({
name: 'My action updated',
config: {
unencrypted: `This value shouldn't get encrypted`,
},
secrets: {
encrypted: 'This value should be encrypted',
},
})
.expect(400, {
statusCode: 400,
error: 'Bad Request',
message: 'System action system-connector-test.system-action is not allowed to update.',
});
});

it('should notify feature usage when editing a gold action type', async () => {
const { body: createdAction } = await supertest
.post(`${getUrlPrefix(Spaces.space1.id)}/api/actions/action`)
Expand Down

0 comments on commit 0020ee9

Please sign in to comment.