Skip to content

Commit

Permalink
2823 metrics authorization return 401 (#2836)
Browse files Browse the repository at this point in the history
* Expose subset of Promex config

* Working - pre refactor

* refactor

* Remove obsolete PromEx auth

* Additional tests for Config.API

* Config clean up

* Extra tests for MetricsAuth

* Update CHANGELOG
  • Loading branch information
rorymckinley authored Jan 21, 2025
1 parent cca74c3 commit 3e63380
Show file tree
Hide file tree
Showing 11 changed files with 311 additions and 129 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions lib/lightning/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions lib/lightning_web/endpoint.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
55 changes: 55 additions & 0 deletions lib/lightning_web/plugs/metrics_auth.ex
Original file line number Diff line number Diff line change
@@ -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
9 changes: 9 additions & 0 deletions lib/lightning_web/plugs/promex_wrapper.ex
Original file line number Diff line number Diff line change
@@ -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
37 changes: 0 additions & 37 deletions lib/lightning_web/prom_ex_plug_authorization.ex

This file was deleted.

1 change: 0 additions & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down
41 changes: 41 additions & 0 deletions test/lightning/config_test.exs
Original file line number Diff line number Diff line change
@@ -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
147 changes: 147 additions & 0 deletions test/lightning_web/plugs/metrics_auth_test.exs
Original file line number Diff line number Diff line change
@@ -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
21 changes: 21 additions & 0 deletions test/lightning_web/prom_ex/metrics_endpoint_test.exs
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit 3e63380

Please sign in to comment.