diff --git a/CHANGELOG.md b/CHANGELOG.md index 67d39bb6ec..ed557881f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ and this project adheres to ### Changed +- PromEx metrics endpoint returns 401 on unauthorized requests. + [#2823](https://github.com/OpenFn/lightning/issues/2823) + ### Fixed ## [v2.10.11] - 2025-01-21 diff --git a/lib/lightning/config.ex b/lib/lightning/config.ex index 8084b63806..888155f416 100644 --- a/lib/lightning/config.ex +++ b/lib/lightning/config.ex @@ -227,6 +227,25 @@ defmodule Lightning.Config do defp kafka_trigger_config do Application.get_env(:lightning, :kafka_triggers, []) end + + @impl true + def promex_metrics_endpoint_authorization_required? do + promex_config() |> Keyword.get(:metrics_endpoint_authorization_required) + end + + @impl true + def promex_metrics_endpoint_scheme do + promex_config() |> Keyword.get(:metrics_endpoint_scheme) + end + + @impl true + def promex_metrics_endpoint_token do + promex_config() |> Keyword.get(:metrics_endpoint_token) + end + + defp promex_config do + Application.get_env(:lightning, Lightning.PromEx, []) + end end @callback apollo(key :: atom() | nil) :: map() @@ -247,6 +266,9 @@ defmodule Lightning.Config do @callback kafka_number_of_processors() :: integer() @callback kafka_triggers_enabled?() :: boolean() @callback oauth_provider(key :: atom()) :: keyword() | nil + @callback promex_metrics_endpoint_authorization_required?() :: boolean() + @callback promex_metrics_endpoint_scheme() :: String.t() + @callback promex_metrics_endpoint_token() :: String.t() @callback purge_deleted_after_days() :: integer() @callback repo_connection_token_signer() :: Joken.Signer.t() @callback reset_password_token_validity_in_days() :: integer() @@ -413,6 +435,18 @@ defmodule Lightning.Config do impl().kafka_number_of_processors() end + def promex_metrics_endpoint_authorization_required? do + impl().promex_metrics_endpoint_authorization_required?() + end + + def promex_metrics_endpoint_scheme do + impl().promex_metrics_endpoint_scheme() + end + + def promex_metrics_endpoint_token do + impl().promex_metrics_endpoint_token() + end + defp impl do Application.get_env(:lightning, __MODULE__, API) end diff --git a/lib/lightning_web/endpoint.ex b/lib/lightning_web/endpoint.ex index a30d19bea1..74aef40ba6 100644 --- a/lib/lightning_web/endpoint.ex +++ b/lib/lightning_web/endpoint.ex @@ -50,9 +50,7 @@ defmodule LightningWeb.Endpoint do plug Plug.RequestId - plug Unplug, - if: {LightningWeb.PromExPlugAuthorization, nil}, - do: {PromEx.Plug, prom_ex_module: Lightning.PromEx} + plug Plugs.PromexWrapper @plug_extensions Application.compile_env( :lightning, diff --git a/lib/lightning_web/plugs/metrics_auth.ex b/lib/lightning_web/plugs/metrics_auth.ex new file mode 100644 index 0000000000..abd3fade6e --- /dev/null +++ b/lib/lightning_web/plugs/metrics_auth.ex @@ -0,0 +1,55 @@ +defmodule LightningWeb.Plugs.MetricsAuth do + @moduledoc """ + Implements Bearer token authorization for /metrics endpoint that is managed by + PromEx. + """ + import Plug.Conn + + def init(opts), do: opts + + def call(conn, _opts) do + if metrics_path?(conn) && authorization_required?() do + if valid_token?(auth_header(conn)) && valid_scheme?(conn) do + conn + else + halt_as_unauthorized(conn) + end + else + conn + end + end + + defp metrics_path?(conn) do + conn.path_info == ["metrics"] + end + + defp authorization_required? do + Lightning.Config.promex_metrics_endpoint_authorization_required?() + end + + defp auth_header(conn) do + Plug.Conn.get_req_header(conn, "authorization") + end + + defp valid_token?(["Bearer " <> provided_token]) do + expected_token = Lightning.Config.promex_metrics_endpoint_token() + Plug.Crypto.secure_compare(provided_token, expected_token) + end + + defp valid_token?(_auth_header) do + false + end + + defp valid_scheme?(conn) do + provided_scheme = Atom.to_string(conn.scheme) + expected_scheme = Lightning.Config.promex_metrics_endpoint_scheme() + provided_scheme == expected_scheme + end + + defp halt_as_unauthorized(conn) do + conn + |> put_resp_header("www-authenticate", "Bearer") + |> send_resp(401, "Unauthorized") + |> halt() + end +end diff --git a/lib/lightning_web/plugs/promex_wrapper.ex b/lib/lightning_web/plugs/promex_wrapper.ex new file mode 100644 index 0000000000..2e2b2222ff --- /dev/null +++ b/lib/lightning_web/plugs/promex_wrapper.ex @@ -0,0 +1,9 @@ +defmodule LightningWeb.Plugs.PromexWrapper do + @moduledoc """ + Ensures that the MetricsAuth plug always comes before the PromEx plug. + """ + use Plug.Builder + + plug LightningWeb.Plugs.MetricsAuth + plug PromEx.Plug, prom_ex_module: Lightning.PromEx +end diff --git a/lib/lightning_web/prom_ex_plug_authorization.ex b/lib/lightning_web/prom_ex_plug_authorization.ex deleted file mode 100644 index a9e86590a9..0000000000 --- a/lib/lightning_web/prom_ex_plug_authorization.ex +++ /dev/null @@ -1,37 +0,0 @@ -defmodule LightningWeb.PromExPlugAuthorization do - @moduledoc """ - Implement custom authorization for PromEx metrics endpoint as per - https://hexdocs.pm/prom_ex/1.1.1/PromEx.Plug.html - """ - @behaviour Unplug.Predicate - - @impl true - def call(conn, _vars) do - config = Application.get_env(:lightning, Lightning.PromEx) - - if config[:metrics_endpoint_authorization_required] do - valid_token?( - Plug.Conn.get_req_header(conn, "authorization"), - config[:metrics_endpoint_token] - ) && - valid_scheme?( - Atom.to_string(conn.scheme), - config[:metrics_endpoint_scheme] - ) - else - true - end - end - - defp valid_token?(["Bearer " <> provided_token], expected_token) do - Plug.Crypto.secure_compare(provided_token, expected_token) - end - - defp valid_token?(_auth_header, _expected_token) do - false - end - - defp valid_scheme?(provided_scheme, expected_scheme) do - provided_scheme == expected_scheme - end -end diff --git a/mix.exs b/mix.exs index 6d9f4e4f34..5db096255f 100644 --- a/mix.exs +++ b/mix.exs @@ -137,7 +137,6 @@ defmodule Lightning.MixProject do {:telemetry_poller, "~> 1.0"}, {:tesla, "~> 1.13"}, {:timex, "~> 3.7"}, - {:unplug, "~> 1.0"}, {:replug, "~> 0.1.0"}, {:phoenix_swoosh, "~> 1.2.1"}, {:hammer_backend_mnesia, "~> 0.6"}, diff --git a/test/lightning/config_test.exs b/test/lightning/config_test.exs new file mode 100644 index 0000000000..c4bd252e2c --- /dev/null +++ b/test/lightning/config_test.exs @@ -0,0 +1,41 @@ +defmodule Lightning.Configtest do + use ExUnit.Case, async: true + + alias Lightning.Config.API + + describe "API" do + test "returns the appropriate PromEx endpoint auth setting" do + expected = + extract_from_config( + Lightning.PromEx, + :metrics_endpoint_authorization_required + ) + + actual = API.promex_metrics_endpoint_authorization_required?() + + assert expected == actual + end + + test "returns the appropriate Promex endpoint token" do + expected = + extract_from_config(Lightning.PromEx, :metrics_endpoint_token) + + actual = API.promex_metrics_endpoint_token() + + assert expected == actual + end + + test "returns the appropriate PromEx endpoint scheme" do + expected = + extract_from_config(Lightning.PromEx, :metrics_endpoint_scheme) + + actual = API.promex_metrics_endpoint_scheme() + + assert expected == actual + end + end + + defp extract_from_config(config, key) do + Application.get_env(:lightning, config) |> Keyword.get(key) + end +end diff --git a/test/lightning_web/plugs/metrics_auth_test.exs b/test/lightning_web/plugs/metrics_auth_test.exs new file mode 100644 index 0000000000..c3ba5167d7 --- /dev/null +++ b/test/lightning_web/plugs/metrics_auth_test.exs @@ -0,0 +1,147 @@ +defmodule LightningWeb.Plugs.MetricsAuthTest do + use LightningWeb.ConnCase, async: true + + import Plug.Test + + alias LightningWeb.Plugs.MetricsAuth + + setup do + token = "test_token" + + Mox.stub(Lightning.MockConfig, :promex_metrics_endpoint_token, fn -> + token + end) + + Mox.stub(Lightning.MockConfig, :promex_metrics_endpoint_scheme, fn -> + "http" + end) + + %{token: token} + end + + describe "init" do + test "returns the provided options as they are" do + assert MetricsAuth.init(a: 1, b: 2) == [a: 1, b: 2] + end + end + + describe "metrics request and authorization required" do + setup do + Mox.stub( + Lightning.MockConfig, + :promex_metrics_endpoint_authorization_required?, + fn -> true end + ) + + conn = conn(:get, "/metrics") + + %{conn: conn} + end + + test "passes if the token and scheme match", %{conn: conn, token: token} do + conn = + conn + |> put_req_header("authorization", "Bearer #{token}") + |> MetricsAuth.call([]) + + assert_passed_request?(conn) + end + + test "is unauthorized if no authorization header", %{conn: conn} do + conn = + conn + |> MetricsAuth.call([]) + + assert_unauthorized_request?(conn) + end + + test "is unauthorized if no bearer token", %{conn: conn, token: token} do + conn = + conn + |> put_req_header("authorization", "Basic #{token}") + |> MetricsAuth.call([]) + + assert_unauthorized_request?(conn) + end + + test "is unauthorized if bearer token does not match", %{ + conn: conn, + token: token + } do + conn = + conn + |> put_req_header("authorization", "Bearer not-#{token}") + |> MetricsAuth.call([]) + + assert_unauthorized_request?(conn) + end + + test "is unauthorised if the scheme does not match", + %{conn: conn, token: token} do + Mox.stub(Lightning.MockConfig, :promex_metrics_endpoint_scheme, fn -> + "https" + end) + + conn = + conn + |> put_req_header("authorization", "Bearer #{token}") + |> MetricsAuth.call([]) + + assert_unauthorized_request?(conn) + end + end + + describe "not metrics and authorization required" do + setup do + Mox.stub( + Lightning.MockConfig, + :promex_metrics_endpoint_authorization_required?, + fn -> true end + ) + + conn = conn(:get, "/not-metrics") + + %{conn: conn} + end + + test "passes the request regardless of what is provided", %{conn: conn} do + conn = conn |> MetricsAuth.call([]) + + assert_passed_request?(conn) + end + end + + describe "metrics request but authorization not required" do + setup do + Mox.stub( + Lightning.MockConfig, + :promex_metrics_endpoint_authorization_required?, + fn -> false end + ) + + conn = conn(:get, "/metrics") + + %{conn: conn} + end + + test "passes the request regardless of what is provided", %{conn: conn} do + conn = conn |> MetricsAuth.call([]) + + assert_passed_request?(conn) + end + end + + def assert_passed_request?(conn) do + assert conn.status == nil + assert get_resp_header(conn, "www-authenticate") == [] + assert conn.resp_body == nil + refute conn.halted + end + + def assert_unauthorized_request?(conn) do + assert conn.status == 401 + assert get_resp_header(conn, "www-authenticate") == ["Bearer"] + assert conn.resp_body == "Unauthorized" + assert conn.halted + end +end diff --git a/test/lightning_web/prom_ex/metrics_endpoint_test.exs b/test/lightning_web/prom_ex/metrics_endpoint_test.exs new file mode 100644 index 0000000000..eaad7c8873 --- /dev/null +++ b/test/lightning_web/prom_ex/metrics_endpoint_test.exs @@ -0,0 +1,21 @@ +defmodule LightningWeb.PromEx.MetricsEndpointTest do + use LightningWeb.ConnCase, async: true + + describe "unauthorized request to GET /metrics" do + setup do + Mox.stub( + Lightning.MockConfig, + :promex_metrics_endpoint_authorization_required?, + fn -> true end + ) + + :ok + end + + test "returns 401", %{conn: conn} do + conn = get(conn, "/metrics") + + assert conn.status == 401 + end + end +end diff --git a/test/lightning_web/prom_ex_plug_authorization_test.exs b/test/lightning_web/prom_ex_plug_authorization_test.exs deleted file mode 100644 index 76132c31f8..0000000000 --- a/test/lightning_web/prom_ex_plug_authorization_test.exs +++ /dev/null @@ -1,88 +0,0 @@ -defmodule LightningWeb.PromExPlugAuthorizationTest do - use LightningWeb.ConnCase, async: true - - setup %{conn: conn} do - token = "foo-bar-baz" - - update_promex_config( - metrics_endpoint_authorization_required: true, - metrics_endpoint_scheme: "http", - metrics_endpoint_token: token - ) - - %{conn: conn, token: token} - end - - test "returns true if bearer token and scheme match", - %{conn: conn, token: token} do - new_conn = - conn |> Plug.Conn.put_req_header("authorization", "Bearer #{token}") - - result = LightningWeb.PromExPlugAuthorization.call(new_conn, nil) - - assert result - end - - test "returns false if there is no authorization header", %{conn: conn} do - result = LightningWeb.PromExPlugAuthorization.call(conn, nil) - - refute result - end - - test "returns false if authorization header does not contain a bearer token", - %{conn: conn, token: token} do - new_conn = - conn |> Plug.Conn.put_req_header("authorization", "Basic #{token}") - - result = LightningWeb.PromExPlugAuthorization.call(new_conn, nil) - - refute result - end - - test "returns false if authorization header contains incorrect bearer token", - %{conn: conn, token: token} do - new_conn = - conn |> Plug.Conn.put_req_header("authorization", "Bearer not-#{token}") - - result = LightningWeb.PromExPlugAuthorization.call(new_conn, nil) - - refute result - end - - test "returns false if scheme does not match", %{conn: conn, token: token} do - update_promex_config( - metrics_endpoint_token: token, - metrics_endpoint_scheme: "https" - ) - - new_conn = - conn |> Plug.Conn.put_req_header("authorization", "Bearer #{token}") - - result = LightningWeb.PromExPlugAuthorization.call(new_conn, nil) - - refute result - end - - test "does not validate token and scheme if auth is not required", - %{conn: conn, token: token} do - update_promex_config( - metrics_endpoint_authorization_required: false, - metrics_endpoint_scheme: "https" - ) - - new_conn = - conn |> Plug.Conn.put_req_header("authorization", "Bearer not-#{token}") - - result = LightningWeb.PromExPlugAuthorization.call(new_conn, nil) - - assert result - end - - defp update_promex_config(overrides) do - new_config = - Application.get_env(:lightning, Lightning.PromEx) - |> Keyword.merge(overrides) - - Application.put_env(:lightning, Lightning.PromEx, new_config) - end -end