diff --git a/lib/algora/integrations/github/webhook.ex b/lib/algora/integrations/github/webhook.ex index 773c22277..a6e233f85 100644 --- a/lib/algora/integrations/github/webhook.ex +++ b/lib/algora/integrations/github/webhook.ex @@ -22,16 +22,20 @@ defmodule Algora.Github.Webhook do defstruct @enforce_keys - def new(conn, payload) do + def new(conn) do secret = Algora.Github.webhook_secret() with {:ok, headers} <- parse_headers(conn), - {:ok, _} <- verify_signature(headers.signature_256, conn.assigns[:raw_body], secret) do - build_webhook(headers, payload) + {:ok, payload, conn} = Plug.Conn.read_body(conn), + {:ok, _} <- verify_signature(headers.signature_256, payload, secret), + {:ok, webhook} <- build_webhook(headers, payload) do + {:ok, webhook, conn} end end defp build_webhook(headers, payload) do + payload = Jason.decode!(payload) + params = headers |> Map.put(:payload, payload) diff --git a/lib/algora_web/controllers/webhooks/github_controller.ex b/lib/algora_web/controllers/webhooks/github_controller.ex index bfc9593c6..2d447daf8 100644 --- a/lib/algora_web/controllers/webhooks/github_controller.ex +++ b/lib/algora_web/controllers/webhooks/github_controller.ex @@ -19,34 +19,6 @@ defmodule AlgoraWeb.Webhooks.GithubController do # TODO: persist & alert about failed deliveries # TODO: auto-retry failed deliveries with exponential backoff - def new(conn, payload) do - with {:ok, webhook} <- Webhook.new(conn, payload), - :ok <- process_delivery(webhook) do - conn |> put_status(:accepted) |> json(%{status: "ok"}) - else - {:error, :bot_event} -> - conn |> put_status(:ok) |> json(%{status: "ok"}) - - {:error, :missing_header} -> - conn |> put_status(:bad_request) |> json(%{error: "Missing header"}) - - {:error, :signature_mismatch} -> - conn |> put_status(:unauthorized) |> json(%{error: "Signature mismatch"}) - - {:error, reason} -> - Logger.error("Error processing webhook: #{inspect(reason)}") - conn |> put_status(:internal_server_error) |> json(%{error: "Internal server error"}) - - error -> - Logger.error("Error processing webhook: #{inspect(error)}") - conn |> put_status(:internal_server_error) |> json(%{error: "Internal server error"}) - end - rescue - e -> - Logger.error(Exception.format(:error, e, __STACKTRACE__)) - conn |> put_status(:internal_server_error) |> json(%{error: "Internal server error"}) - end - def process_delivery(webhook) do with :ok <- ensure_human_author(webhook), {:ok, commands} <- process_commands(webhook), diff --git a/lib/algora_web/endpoint.ex b/lib/algora_web/endpoint.ex index 5eceae756..03476b53a 100644 --- a/lib/algora_web/endpoint.ex +++ b/lib/algora_web/endpoint.ex @@ -46,32 +46,9 @@ defmodule AlgoraWeb.Endpoint do handler: AlgoraWeb.Webhooks.StripeController, secret: {Algora, :config, [[:stripe, :webhook_secret]]} - plug :parse_body - - opts = [ - parsers: [:urlencoded, :multipart, :json], - pass: ["*/*"], - json_decoder: Phoenix.json_library() - ] - - defmodule BodyReader do - @moduledoc false - def cache_raw_body(conn, opts) do - with {:ok, body, conn} <- Plug.Conn.read_body(conn, opts) do - conn = update_in(conn.assigns[:raw_body], &[body | &1 || []]) - - {:ok, body, conn} - end - end - end - - @parser_without_cache Plug.Parsers.init(opts) - @parser_with_cache Plug.Parsers.init([body_reader: {BodyReader, :cache_raw_body, []}] ++ opts) - - # All endpoints that start with "webhooks" have their body cached. - defp parse_body(%{path_info: ["webhooks" | _]} = conn, _), do: Plug.Parsers.call(conn, @parser_with_cache) - - defp parse_body(conn, _), do: Plug.Parsers.call(conn, @parser_without_cache) + plug AlgoraWeb.Plugs.GithubWebhooks, + at: "/webhooks/github", + handler: AlgoraWeb.Webhooks.GithubController plug Plug.MethodOverride plug Plug.Head diff --git a/lib/algora_web/plugs/github_webhooks.ex b/lib/algora_web/plugs/github_webhooks.ex new file mode 100644 index 000000000..11cfa3f0b --- /dev/null +++ b/lib/algora_web/plugs/github_webhooks.ex @@ -0,0 +1,56 @@ +defmodule AlgoraWeb.Plugs.GithubWebhooks do + @moduledoc false + @behaviour Plug + + import Plug.Conn + + alias Algora.Github.Webhook + alias AlgoraWeb.Webhooks.GithubController + alias Plug.Conn + + require Logger + + @impl true + def init(opts) do + path_info = String.split(opts[:at], "/", trim: true) + + opts + |> Map.new() + |> Map.put_new(:path_info, path_info) + end + + @impl true + def call(%Conn{method: "POST", path_info: path_info} = conn, %{path_info: path_info} = _opts) do + with {:ok, webhook, conn} <- Webhook.new(conn), + :ok <- GithubController.process_delivery(webhook) do + conn |> send_resp(200, "Webhook received.") |> halt() + else + {:error, :bot_event} -> + conn |> send_resp(200, "Webhook received.") |> halt() + + {:error, :missing_header} -> + Logger.error("Missing header") + conn |> send_resp(400, "Bad request.") |> halt() + + {:error, :signature_mismatch} -> + Logger.error("Signature mismatch") + conn |> send_resp(400, "Bad request.") |> halt() + + error -> + Logger.error("Bad request: #{inspect(error)}") + conn |> send_resp(400, "Bad request.") |> halt() + end + rescue + e -> + Logger.error(Exception.format(:error, e, __STACKTRACE__)) + conn |> send_resp(400, "Bad request.") |> halt() + end + + @impl true + def call(%Conn{path_info: path_info} = conn, %{path_info: path_info}) do + conn |> send_resp(400, "Bad request.") |> halt() + end + + @impl true + def call(conn, _), do: conn +end diff --git a/lib/algora_web/router.ex b/lib/algora_web/router.ex index 79bef9a45..e682928f4 100644 --- a/lib/algora_web/router.ex +++ b/lib/algora_web/router.ex @@ -20,12 +20,6 @@ defmodule AlgoraWeb.Router do plug :accepts, ["json"] end - scope "/webhooks", AlgoraWeb do - pipe_through :api - - post "/github", Webhooks.GithubController, :new - end - scope "/", AlgoraWeb do pipe_through [:browser]