diff --git a/js/console/src/components/settings/notifications/sinks/UpsertNotificationSinkModal.tsx b/js/console/src/components/settings/notifications/sinks/UpsertNotificationSinkModal.tsx index 761b86164f..0c5b98daa6 100644 --- a/js/console/src/components/settings/notifications/sinks/UpsertNotificationSinkModal.tsx +++ b/js/console/src/components/settings/notifications/sinks/UpsertNotificationSinkModal.tsx @@ -1,6 +1,7 @@ import { ComponentProps, FormEvent, useCallback, useMemo } from 'react' import { Button, FormField, Input2, Modal } from '@pluralsh/design-system' +import { GqlError } from 'components/utils/Alert' import { ModalMountTransition } from 'components/utils/ModalMountTransition' import { Body2P } from 'components/utils/typography/Text' import { useTheme } from 'styled-components' @@ -17,10 +18,8 @@ import { appendConnection, updateCache } from 'utils/graphql' import { sinkTypeToIcon } from './NotificationSinksColumns' -const hookUrlMatch = [ - [SinkType.Slack, /^https:\/\/[^/]*?slack/], - [SinkType.Teams, /^https:\/\/[^/]*?office/], -] as const satisfies [SinkType, RegExp][] +// URL validity is enforced server-side. This only preserves Slack vs Teams payload selection. +const slackUrlTypeHintRegex = /^https:\/\/[^/]*?slack/ type ModalBaseProps = { mode: 'edit' | 'create' @@ -39,6 +38,10 @@ function UpsertNotificationSinkModal({ ...props }: ModalProps) { const sink = mode === 'edit' ? props.sink : undefined + const sinkName = sink?.name + const sinkType = sink?.type + const slackUrl = sink?.configuration.slack?.url + const teamsUrl = sink?.configuration.teams?.url const theme = useTheme() const initialState = useMemo( () => ({ @@ -46,28 +49,25 @@ function UpsertNotificationSinkModal({ hookUrl: '', ...(mode === 'edit' ? { - name: sink?.name, - hookUrl: - sink?.configuration.slack?.url || sink?.configuration.teams?.url, + name: sinkName, + hookUrl: slackUrl || teamsUrl, } : {}), }), - [ - mode, - sink?.configuration.slack?.url, - sink?.configuration.teams?.url, - sink?.name, - ] + [mode, sinkName, slackUrl, teamsUrl] ) const { state, update, hasUpdates } = useUpdateState<{ name: string hookUrl: string }>(initialState) - const hookType = hookUrlMatch.find(([_, regex]) => - regex.test(state.hookUrl) - )?.[0] + const hookType = + mode === 'edit' && sinkType + ? sinkType + : slackUrlTypeHintRegex.test(state.hookUrl) + ? SinkType.Slack + : SinkType.Teams - const [mutation, { loading }] = useUpsertNotificationSinkMutation({ + const [mutation, { loading, error }] = useUpsertNotificationSinkMutation({ onCompleted: () => onClose?.(), update: (cache, { data }) => updateCache(cache, { @@ -81,7 +81,7 @@ function UpsertNotificationSinkModal({ }), }) - const allowSubmit = hookType && state.name && state.hookUrl && hasUpdates + const allowSubmit = state.name && state.hookUrl && hasUpdates const onSubmit = useCallback( (e: FormEvent) => { @@ -166,6 +166,7 @@ function UpsertNotificationSinkModal({ {' '} webhook url to send this event alert to your team. + {error && }
{mode !== 'edit' && ( @@ -178,7 +179,7 @@ function UpsertNotificationSinkModal({ )} update({ hookUrl: e.target.value })} placeholder="https://hooks.provider.com/..." css={{ flex: '1 1 100%' }} diff --git a/lib/console/schema/notification_sink.ex b/lib/console/schema/notification_sink.ex index 5765c52333..83f4d48b45 100644 --- a/lib/console/schema/notification_sink.ex +++ b/lib/console/schema/notification_sink.ex @@ -44,6 +44,8 @@ defmodule Console.Schema.NotificationSink do end @valid ~w(type name)a + @slack_webhook_hosts ~w(slack.com slack-gov.com) + @teams_webhook_hosts ~w(office.com office365.com powerautomate.com powerplatform.com logic.azure.com) def changeset(model, attrs \\ %{}) do model @@ -57,8 +59,8 @@ defmodule Console.Schema.NotificationSink do defp config_changeset(model, attrs) do model |> cast(attrs, []) - |> cast_embed(:slack, with: &url_changeset/2) - |> cast_embed(:teams, with: &url_changeset/2) + |> cast_embed(:slack, with: &slack_changeset/2) + |> cast_embed(:teams, with: &teams_changeset/2) |> cast_embed(:plural, with: &plural_changeset/2) end @@ -68,6 +70,47 @@ defmodule Console.Schema.NotificationSink do |> validate_required([:url]) end + defp slack_changeset(model, attrs) do + model + |> url_changeset(attrs) + |> validate_change( + :url, + &validate_webhook_url(&1, &2, @slack_webhook_hosts, "must be a valid Slack webhook URL") + ) + end + + defp teams_changeset(model, attrs) do + model + |> url_changeset(attrs) + |> validate_change( + :url, + &validate_webhook_url( + &1, + &2, + @teams_webhook_hosts, + "must be a valid Microsoft Teams webhook URL" + ) + ) + end + + defp validate_webhook_url(:url, url, hosts, message) when is_binary(url) do + with {:ok, %URI{scheme: "https", host: host, userinfo: nil}} when is_binary(host) <- + URI.new(url), + true <- host_matches?(host, hosts) do + [] + else + _ -> [url: message] + end + end + + defp validate_webhook_url(:url, _, _, message), do: [url: message] + + defp host_matches?(host, hosts) do + host = String.downcase(host) + + Enum.any?(hosts, &(host == &1 || String.ends_with?(host, ".#{&1}"))) + end + defp plural_changeset(model, attrs) do model |> cast(attrs, ~w(priority urgent)a) diff --git a/test/console/schema/notification_sink_test.exs b/test/console/schema/notification_sink_test.exs new file mode 100644 index 0000000000..d206c7533e --- /dev/null +++ b/test/console/schema/notification_sink_test.exs @@ -0,0 +1,61 @@ +defmodule Console.Schema.NotificationSinkTest do + use Console.DataCase, async: true + alias Console.Schema.NotificationSink + + describe "changeset/2" do + test "accepts standard Slack and GovSlack webhook URLs" do + for url <- [ + "https://hooks.slack.com/services/T00000000/B00000000/test", + "https://hooks.slack-gov.com/services/T00000000/B00000000/test" + ] do + assert valid_sink?(:slack, url) + end + end + + test "accepts Microsoft Teams webhook URL hosts" do + for url <- [ + "https://outlook.office.com/webhook/test", + "https://environment.api.powerplatform.com:443/powerautomate/automations/direct/workflows/test/triggers/manual/paths/invoke?sig=test", + "https://prod-01.westus.logic.azure.com:443/workflows/test/triggers/manual/paths/invoke?sig=test" + ] do + assert valid_teams_sink?(url) + end + end + + test "rejects userinfo hostname spoofing" do + refute valid_sink?(:slack, "https://hooks.slack.com@example.com/services/test") + + refute valid_sink?( + :teams, + "https://environment.api.powerplatform.com@example.com/workflows/test" + ) + end + + test "rejects suffix hostname spoofing" do + refute valid_sink?(:slack, "https://hooks.slack.com.example.com/services/test") + refute valid_sink?(:slack, "https://hooks.slack-gov.com.example.com/services/test") + + refute valid_sink?( + :teams, + "https://environment.api.powerplatform.com.example.com/workflows/test" + ) + end + + test "rejects non-HTTPS URLs" do + refute valid_sink?(:slack, "http://hooks.slack.com/services/test") + refute valid_sink?(:teams, "http://environment.api.powerplatform.com/workflows/test") + end + end + + defp valid_teams_sink?(url), do: valid_sink?(:teams, url) + + defp valid_sink?(type, url) do + %NotificationSink{} + |> NotificationSink.changeset(%{ + name: "#{type}-sink", + type: type, + configuration: %{type => %{url: url}} + }) + |> then(& &1.valid?) + end +end