Skip to content

Commit bd8ad32

Browse files
georgianaonoleata1904adcoelhocnasikas
authored
[ResponseOps][Connectors] Change the Slack config and params to support channel names instead of channel IDs (elastic#238585)
Closes elastic#230967 ## Summary This PR introduces schema changes which update the Slack connector to allow users to send messages using channelNames How to test: - create a Slack api connector using UI/Postman - Since the updates affect only the backend, use Postman to send a message using the new `channelNames` parameter (in addition to `channelIds`). - Verify the following: - messages are sent successfully when using valid channel names that: - starts with `#` - are included in the config.allowedChannels.name list - requests fail with a validation error when: - the channel name does not start with `#` - the channel name is not included in the allowed channel list - Ensure the UI still works correctly when using only channelIds --------- Co-authored-by: Antonio <antoniodcoelho@gmail.com> Co-authored-by: Christos Nasikas <xristosnasikas@gmail.com> Co-authored-by: Christos Nasikas <christos.nasikas@elastic.co> Co-authored-by: Antonio <antonio.coelho@elastic.co>
1 parent ae0d9f1 commit bd8ad32

12 files changed

Lines changed: 855 additions & 105 deletions

File tree

‎src/platform/packages/shared/kbn-connector-schemas/slack_api/schemas/v1.test.ts‎

Lines changed: 176 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,56 +8,196 @@
88
*/
99

1010
import type { z } from '@kbn/zod';
11-
import { validateBlockkit } from './v1';
11+
import {
12+
validateBlockkit,
13+
validateChannelName,
14+
PostMessageSubActionParamsSchema,
15+
SlackApiConfigSchema,
16+
MAX_ALLOWED_CHANNELS,
17+
} from './v1';
1218

1319
const ctx = {
1420
addIssue: jest.fn(),
1521
} as unknown as z.RefinementCtx;
1622

17-
describe('validateBlockkit', () => {
18-
beforeEach(() => {
19-
jest.clearAllMocks();
23+
describe('Slack Api Schema validation', () => {
24+
describe('validateBlockkit', () => {
25+
beforeEach(() => {
26+
jest.clearAllMocks();
27+
});
28+
29+
test('should add error for invalid json', () => {
30+
validateBlockkit('', ctx);
31+
validateBlockkit('abc', ctx);
32+
33+
expect(ctx.addIssue).toHaveBeenCalledTimes(2);
34+
expect(ctx.addIssue).toHaveBeenNthCalledWith(1, {
35+
code: 'custom',
36+
message: 'block kit body is not valid JSON - Unexpected end of JSON input',
37+
});
38+
expect(ctx.addIssue).toHaveBeenNthCalledWith(2, {
39+
code: 'custom',
40+
message:
41+
'block kit body is not valid JSON - Unexpected token \'a\', "abc" is not valid JSON',
42+
});
43+
});
44+
45+
test('should add error for json that does not contain the "blocks" field', () => {
46+
validateBlockkit(JSON.stringify({ foo: 'bar' }), ctx);
47+
expect(ctx.addIssue).toHaveBeenCalledTimes(1);
48+
expect(ctx.addIssue).toHaveBeenNthCalledWith(1, {
49+
code: 'custom',
50+
message: `block kit body must contain field \"blocks\"`,
51+
});
52+
});
53+
54+
test('should add nothing for valid blockkit text', () => {
55+
validateBlockkit(
56+
JSON.stringify({
57+
blocks: [
58+
{
59+
type: 'section',
60+
text: {
61+
type: 'mrkdwn',
62+
text: 'Hello',
63+
},
64+
},
65+
],
66+
}),
67+
ctx
68+
);
69+
expect(ctx.addIssue).not.toHaveBeenCalled();
70+
});
2071
});
2172

22-
test('should add error for invalid json', () => {
23-
validateBlockkit('', ctx);
24-
validateBlockkit('abc', ctx);
73+
describe('Validate channel name', () => {
74+
beforeEach(() => {
75+
jest.clearAllMocks();
76+
});
2577

26-
expect(ctx.addIssue).toHaveBeenCalledTimes(2);
27-
expect(ctx.addIssue).toHaveBeenNthCalledWith(1, {
28-
code: 'custom',
29-
message: 'block kit body is not valid JSON - Unexpected end of JSON input',
78+
test('should add error if the channel name does not start with #', () => {
79+
validateChannelName('general', ctx);
80+
expect(ctx.addIssue).toHaveBeenNthCalledWith(1, {
81+
code: 'custom',
82+
message: 'Channel name must start with #',
83+
});
3084
});
31-
expect(ctx.addIssue).toHaveBeenNthCalledWith(2, {
32-
code: 'custom',
33-
message: 'block kit body is not valid JSON - Unexpected token \'a\', "abc" is not valid JSON',
85+
86+
test('should add nothing for valid channel names starting with #', () => {
87+
validateChannelName('#general', ctx);
88+
validateChannelName('#channel-123', ctx);
89+
90+
expect(ctx.addIssue).not.toHaveBeenCalled();
91+
});
92+
93+
test('should add nothing for channel names with special characters', () => {
94+
validateChannelName('#test-team', ctx);
95+
validateChannelName('#incident-*', ctx);
96+
97+
expect(ctx.addIssue).not.toHaveBeenCalled();
98+
});
99+
100+
test('should add error for empty strings', () => {
101+
validateChannelName('', ctx);
102+
expect(ctx.addIssue).toHaveBeenNthCalledWith(1, {
103+
code: 'custom',
104+
message: 'Channel name cannot be empty',
105+
});
106+
});
107+
108+
test('should add error for undefined values', () => {
109+
validateChannelName(undefined, ctx);
110+
expect(ctx.addIssue).toHaveBeenNthCalledWith(1, {
111+
code: 'custom',
112+
message: 'Channel name cannot be empty',
113+
});
34114
});
35115
});
36116

37-
test('should add error for json that does not contain the "blocks" field', () => {
38-
validateBlockkit(JSON.stringify({ foo: 'bar' }), ctx);
39-
expect(ctx.addIssue).toHaveBeenCalledTimes(1);
40-
expect(ctx.addIssue).toHaveBeenNthCalledWith(1, {
41-
code: 'custom',
42-
message: `block kit body must contain field \"blocks\"`,
117+
describe('SlackApiConfigSchema', () => {
118+
test('should not throw if all properties are missing', () => {
119+
expect(() => SlackApiConfigSchema.parse({})).not.toThrow();
120+
});
121+
122+
test('should throw with excess fields', () => {
123+
expect(() =>
124+
SlackApiConfigSchema.parse({ allowedChannels: [{ name: '#channel-name' }], foo: 'bar' })
125+
).toThrow();
126+
});
127+
128+
test('should throw if allowedChannels.id is missing', () => {
129+
expect(() =>
130+
SlackApiConfigSchema.parse({ allowedChannels: [{ name: '#channel-name' }] })
131+
).not.toThrow();
132+
});
133+
134+
test('should throw if allowedChannels.name is missing', () => {
135+
expect(() => SlackApiConfigSchema.parse({ allowedChannels: [{}] })).toThrow();
136+
});
137+
138+
test('should throw if allowedChannels.id is empty', () => {
139+
expect(() =>
140+
SlackApiConfigSchema.parse({ allowedChannels: [{ id: '', name: '#channel-name' }] })
141+
).toThrow();
142+
});
143+
144+
test('should throw if allowedChannels.name is empty', () => {
145+
expect(() =>
146+
SlackApiConfigSchema.parse({ allowedChannels: [{ id: 'channel-id', name: '' }] })
147+
).toThrow();
148+
});
149+
150+
test('should throw with excess fields in allowedChannels', () => {
151+
expect(() =>
152+
SlackApiConfigSchema.parse({
153+
allowedChannels: [{ name: '#channel-name', foo: 'bar' }],
154+
})
155+
).toThrow();
156+
});
157+
158+
test(`should throw if allowedChannels is more than ${MAX_ALLOWED_CHANNELS}`, () => {
159+
expect(() =>
160+
SlackApiConfigSchema.parse({
161+
allowedChannels: Array.from({ length: MAX_ALLOWED_CHANNELS + 1 }, (_, i) => ({
162+
id: `channel-id-${i}`,
163+
name: `#channel-name-${i}`,
164+
})),
165+
})
166+
).toThrow();
43167
});
44168
});
45169

46-
test('should add nothing for valid blockkit text', () => {
47-
validateBlockkit(
48-
JSON.stringify({
49-
blocks: [
50-
{
51-
type: 'section',
52-
text: {
53-
type: 'mrkdwn',
54-
text: 'Hello',
55-
},
56-
},
57-
],
58-
}),
59-
ctx
60-
);
61-
expect(ctx.addIssue).not.toHaveBeenCalled();
170+
describe('PostMessageSubActionParamsSchema', () => {
171+
test('should throw if text is missing', () => {
172+
expect(() => PostMessageSubActionParamsSchema.parse({})).toThrow();
173+
});
174+
175+
test('should not throw if text is not missing', () => {
176+
expect(() => PostMessageSubActionParamsSchema.parse({ text: 'hello' })).not.toThrow();
177+
});
178+
179+
test('should throw if channelNames is too short', () => {
180+
expect(() =>
181+
PostMessageSubActionParamsSchema.parse({ text: 'hello', channelNames: ['#'] })
182+
).toThrow();
183+
});
184+
185+
test('should throw if channelNames is too long', () => {
186+
expect(() =>
187+
PostMessageSubActionParamsSchema.parse({ text: 'hello', channelNames: ['#'.repeat(201)] })
188+
).toThrow();
189+
});
190+
191+
test('should throw if channelNames does not start with #', () => {
192+
expect(() =>
193+
PostMessageSubActionParamsSchema.parse({ text: 'hello', channelNames: ['general'] })
194+
).toThrow();
195+
});
196+
197+
test('should not throw with valid names', () => {
198+
expect(() =>
199+
PostMessageSubActionParamsSchema.parse({ text: 'hello', channelNames: ['#general'] })
200+
).not.toThrow();
201+
});
62202
});
63203
});

‎src/platform/packages/shared/kbn-connector-schemas/slack_api/schemas/v1.ts‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
*/
99
import { z } from '@kbn/zod';
1010

11+
export const MAX_ALLOWED_CHANNELS = 500;
12+
1113
export const SlackApiSecretsSchema = z
1214
.object({
1315
token: z.string().min(1),
@@ -20,12 +22,12 @@ export const SlackApiConfigSchema = z
2022
.array(
2123
z
2224
.object({
23-
id: z.string().min(1),
25+
id: z.string().min(1).optional(),
2426
name: z.string().min(1),
2527
})
2628
.strict()
2729
)
28-
.max(500)
30+
.max(MAX_ALLOWED_CHANNELS)
2931
.optional(),
3032
})
3133
.strict();
@@ -45,8 +47,17 @@ export const ValidChannelIdParamsSchema = z
4547

4648
export const PostMessageSubActionParamsSchema = z
4749
.object({
50+
/**
51+
* @deprecated Use `channelNames` or `channelIds` instead
52+
* `channelNames` takes priority over `channelIds` and `channels`
53+
*/
4854
channels: z.array(z.string()).max(1).optional(),
4955
channelIds: z.array(z.string()).max(1).optional(),
56+
channelNames: z
57+
// min of two characters to account for '#' prefix
58+
.array(z.string().min(2).max(200).superRefine(validateChannelName))
59+
.max(1)
60+
.optional(),
5061
text: z.string().min(1),
5162
})
5263
.strict();
@@ -69,10 +80,32 @@ export function validateBlockkit(text: string, ctx: z.RefinementCtx) {
6980
}
7081
}
7182

83+
export function validateChannelName(value: string | undefined, ctx: z.RefinementCtx) {
84+
if (!value || value.length === 0) {
85+
ctx.addIssue({
86+
code: z.ZodIssueCode.custom,
87+
message: 'Channel name cannot be empty',
88+
});
89+
return;
90+
}
91+
92+
if (!value.startsWith('#')) {
93+
ctx.addIssue({
94+
code: z.ZodIssueCode.custom,
95+
message: 'Channel name must start with #',
96+
});
97+
}
98+
}
99+
72100
export const PostBlockkitSubActionParamsSchema = z
73101
.object({
102+
/**
103+
* @deprecated Use `channelNames` or `channelIds` instead
104+
* `channelNames` takes priority over `channelIds` and `channels`
105+
*/
74106
channels: z.array(z.string()).max(1).optional(),
75107
channelIds: z.array(z.string()).max(1).optional(),
108+
channelNames: z.array(z.string().superRefine(validateChannelName)).max(1).optional(),
76109
text: z.string().superRefine(validateBlockkit),
77110
})
78111
.strict();

‎x-pack/platform/plugins/shared/actions/server/integration_tests/__snapshots__/connector_types.test.ts.snap‎

Lines changed: 16 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎x-pack/platform/plugins/shared/stack_connectors/common/slack_api/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,13 @@ export interface SlackApiService {
7474
postMessage: ({
7575
channels,
7676
channelIds,
77+
channelNames,
7778
text,
7879
}: PostMessageSubActionParams) => Promise<ConnectorTypeExecutorResult<unknown>>;
7980
postBlockkit: ({
8081
channels,
8182
channelIds,
83+
channelNames,
8284
text,
8385
}: PostBlockkitSubActionParams) => Promise<ConnectorTypeExecutorResult<unknown>>;
8486
}

‎x-pack/platform/plugins/shared/stack_connectors/server/connector_types/slack_api/api.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,19 @@ const validChannelIdHandler = async ({
2222

2323
const postMessageHandler = async ({
2424
externalService,
25-
params: { channelIds, channels, text },
25+
params: { channelIds, channels, text, channelNames },
2626
}: {
2727
externalService: SlackApiService;
2828
params: PostMessageSubActionParams;
29-
}) => await externalService.postMessage({ channelIds, channels, text });
29+
}) => await externalService.postMessage({ channelIds, channels, channelNames, text });
3030

3131
const postBlockkitHandler = async ({
3232
externalService,
33-
params: { channelIds, channels, text },
33+
params: { channelIds, channels, channelNames, text },
3434
}: {
3535
externalService: SlackApiService;
3636
params: PostBlockkitSubActionParams;
37-
}) => await externalService.postBlockkit({ channelIds, channels, text });
37+
}) => await externalService.postBlockkit({ channelIds, channels, channelNames, text });
3838

3939
export const api = {
4040
validChannelId: validChannelIdHandler,

0 commit comments

Comments
 (0)