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

[alerting] need to use encodeURIComponent() when making http calls with user-data in the path #97852

Closed
pmuellr opened this issue Apr 21, 2021 · 1 comment · Fixed by #97854
Closed
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Actions Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@pmuellr
Copy link
Member

pmuellr commented Apr 21, 2021

originating from: #97747

In the following code, and other similar places, we aren't encoding the instanceId with encodeURIComponent(), which means if the instanceId contains "problematic" characters for URLs, the request is likely to fail:

export async function muteAlertInstance({
id,
instanceId,
http,
}: {
id: string;
instanceId: string;
http: HttpSetup;
}): Promise<void> {
await http.post(`${BASE_ALERTING_API_PATH}/rule/${id}/alert/${instanceId}/_mute`);
}

I was able to get this to fail with an alert that generated instanceId's of the form host/1, host/2, and saw 404's being returned when attempting to mute the instance. After adding theencodeURIComponent() wrapper, worked fine.

Rule type ids and connector type ids should not need to be encoded, today, since we are in control of them. However, we probably want to do that anyway, in case we find outselves at a point where users can create new rule types / connector types (eg, via expressions).

Likewise rule ids do not need to be encoded, since we generate them and they're always UUIDs. Connector ids however can be named by a customer for preconfigured connectors, and will need to be encoded. We'll probably end up, someday, having customer-named rule ids, so we should should go ahead and encode those as well.

That implies almost all of our http requests from the web ui should will need this encoding change. Luckily, they seem to all be centralized here:

@pmuellr pmuellr added bug Fixes for quality problems that affect the customer experience Feature:Alerting Feature:Actions Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Apr 21, 2021
@pmuellr pmuellr self-assigned this Apr 21, 2021
@elasticmachine
Copy link
Contributor

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

@pmuellr pmuellr added this to In Progress in Kibana Alerting Apr 21, 2021
pmuellr added a commit to pmuellr/kibana that referenced this issue Apr 21, 2021
…ing UI

resolves: elastic#97852

Adds `encodeURIComponent()` wrappers around references to rule, alert, and
connector ids.  Without this fix, if an alert id (which can contain
customer-generated data) contains a character that needs to be URL
encoded, the resulting API call from the web UI will fail.
Kibana Alerting automation moved this from In Progress to Done (Ordered by most recent) Apr 23, 2021
pmuellr added a commit that referenced this issue Apr 23, 2021
…ing UI (#97854)

resolves: #97852

Adds `encodeURIComponent()` wrappers around references to rule, alert, and
connector ids.  Without this fix, if an alert id (which can contain
customer-generated data) contains a character that needs to be URL
encoded, the resulting API call from the web UI will fail.
pmuellr added a commit to pmuellr/kibana that referenced this issue Apr 23, 2021
…ing UI (elastic#97854)

resolves: elastic#97852

Adds `encodeURIComponent()` wrappers around references to rule, alert, and
connector ids.  Without this fix, if an alert id (which can contain
customer-generated data) contains a character that needs to be URL
encoded, the resulting API call from the web UI will fail.
pmuellr added a commit to pmuellr/kibana that referenced this issue Apr 23, 2021
…ing UI (elastic#97854)

resolves: elastic#97852

Adds `encodeURIComponent()` wrappers around references to rule, alert, and
connector ids.  Without this fix, if an alert id (which can contain
customer-generated data) contains a character that needs to be URL
encoded, the resulting API call from the web UI will fail.
pmuellr added a commit to pmuellr/kibana that referenced this issue Apr 23, 2021
…ing UI (elastic#97854)

resolves: elastic#97852

Adds `encodeURIComponent()` wrappers around references to rule, alert, and
connector ids.  Without this fix, if an alert id (which can contain
customer-generated data) contains a character that needs to be URL
encoded, the resulting API call from the web UI will fail.
# Conflicts:
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/api.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/api.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/resilient/api.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/resilient/api.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/servicenow/api.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/servicenow/api.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/delete.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/delete.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/execute.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/execute.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/update.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/update.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/alert_summary.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/alert_summary.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/delete.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/delete.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/disable.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/disable.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/enable.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/enable.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/get_rule.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/get_rule.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/mute.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/mute.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/mute_alert.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/mute_alert.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/unmute.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/unmute.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/unmute_alert.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/unmute_alert.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/update.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/update.ts
#	x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts
pmuellr added a commit that referenced this issue Apr 23, 2021
…ing UI (#97854) (#98208)

resolves: #97852

Adds `encodeURIComponent()` wrappers around references to rule, alert, and
connector ids.  Without this fix, if an alert id (which can contain
customer-generated data) contains a character that needs to be URL
encoded, the resulting API call from the web UI will fail.
pmuellr added a commit that referenced this issue Apr 23, 2021
…ing UI (#97854) (#98209)

resolves: #97852

Adds `encodeURIComponent()` wrappers around references to rule, alert, and
connector ids.  Without this fix, if an alert id (which can contain
customer-generated data) contains a character that needs to be URL
encoded, the resulting API call from the web UI will fail.
pmuellr added a commit that referenced this issue Apr 26, 2021
…m alerting UI (#97854) (#98211)

* [alerting] encode rule/connector ids in http requests made from alerting UI (#97854)

resolves: #97852

Adds `encodeURIComponent()` wrappers around references to rule, alert, and
connector ids.  Without this fix, if an alert id (which can contain
customer-generated data) contains a character that needs to be URL
encoded, the resulting API call from the web UI will fail.
# Conflicts:
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/api.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/api.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/resilient/api.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/resilient/api.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/servicenow/api.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/servicenow/api.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/delete.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/delete.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/execute.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/execute.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/update.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/update.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/alert_summary.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/alert_summary.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/delete.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/delete.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/disable.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/disable.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/enable.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/enable.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/get_rule.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/get_rule.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/mute.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/mute.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/mute_alert.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/mute_alert.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/unmute.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/unmute.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/unmute_alert.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/unmute_alert.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/update.test.ts
#	x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api/update.ts
#	x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts

* fix merge conflicts

In 7.13.0, the structure of the connector and rules API libraries in
triggers_actions_ui changed, where in 7.12 they were all in a single
file - one for connectors, one for rules - but in 7.13 they are split
out into separate files in a directory for connectors and one for rules.

To cut down on the noise, I decided to not use the `encodeURIComponent()`
wrappers on rule ids, just connector ids and alert ids, since it's not
possible in 7.12 to have rule ids which are not UUIDs, and so don't need
the encoding.

* fix prettier errors

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
madirey pushed a commit to madirey/kibana that referenced this issue May 11, 2021
…ing UI (elastic#97854)

resolves: elastic#97852

Adds `encodeURIComponent()` wrappers around references to rule, alert, and
connector ids.  Without this fix, if an alert id (which can contain
customer-generated data) contains a character that needs to be URL
encoded, the resulting API call from the web UI will fail.
@kobelb kobelb added the needs-team Issues missing a team label label Jan 31, 2022
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Actions Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
No open projects
Kibana Alerting
Done (Ordered by most recent)
Development

Successfully merging a pull request may close this issue.

3 participants