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

Add ability to provide a CA cert when making requests to SUMA #2391

Merged
merged 5 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,36 @@ defmodule Trento.Infrastructure.SoftwareUpdates.Suma.HttpExecutor do
SUMA Http requests executor
"""

@callback login(base_url :: String.t(), username :: String.t(), password :: String.t()) ::
alias Trento.Infrastructure.SoftwareUpdates.SumaApi

@callback login(
base_url :: String.t(),
username :: String.t(),
password :: String.t(),
use_ca_cert :: boolean()
) ::
{:ok, HTTPoison.Response.t()} | {:error, HTTPoison.Error.t()}

@callback get_system_id(
base_url :: String.t(),
auth :: String.t(),
fully_qualified_domain_name :: String.t()
fully_qualified_domain_name :: String.t(),
use_ca_cert :: boolean()
) ::
{:ok, HTTPoison.Response.t()} | {:error, HTTPoison.Error.t()}

@callback get_relevant_patches(
base_url :: String.t(),
auth :: String.t(),
system_id :: pos_integer()
system_id :: pos_integer(),
use_ca_cert :: boolean()
) ::
{:ok, HTTPoison.Response.t()} | {:error, HTTPoison.Error.t()}

@behaviour Trento.Infrastructure.SoftwareUpdates.Suma.HttpExecutor

@impl true
def login(base_url, username, password) do
def login(base_url, username, password, use_ca_cert \\ false) do
payload =
Jason.encode!(%{
"login" => username,
Expand All @@ -34,27 +43,30 @@ defmodule Trento.Infrastructure.SoftwareUpdates.Suma.HttpExecutor do
"#{base_url}/auth/login",
payload,
[{"Content-type", "application/json"}],
ssl: [verify: :verify_none]
ssl_options(use_ca_cert)
)
end

@impl true
def get_system_id(base_url, auth, fully_qualified_domain_name) do
def get_system_id(base_url, auth, fully_qualified_domain_name, use_ca_cert \\ false) do
HTTPoison.get(
"#{base_url}/system/getId?name=#{fully_qualified_domain_name}",
[{"Content-type", "application/json"}],
hackney: [cookie: [auth]],
ssl: [verify: :verify_none]
hackney: [cookie: [auth]] ++ ssl_options(use_ca_cert)
)
end

@impl true
def get_relevant_patches(base_url, auth, system_id) do
def get_relevant_patches(base_url, auth, system_id, use_ca_cert \\ false) do
HTTPoison.get(
"#{base_url}/system/getRelevantErrata?sid=#{system_id}",
[{"Content-type", "application/json"}],
hackney: [cookie: [auth]],
ssl: [verify: :verify_none]
hackney: [cookie: [auth]] ++ ssl_options(use_ca_cert)
)
end

defp ssl_options(true),
do: [ssl: [verify: :verify_peer, certfile: SumaApi.ca_cert_path()]]

defp ssl_options(_), do: []
end
30 changes: 22 additions & 8 deletions lib/trento/infrastructure/software_updates/suma.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.Suma do
use GenServer, restart: :transient

alias Trento.Infrastructure.SoftwareUpdates.{Suma.State, SumaApi}
alias Trento.Infrastructure.SoftwareUpdates.SumaApi
alias Trento.SoftwareUpdates

require Logger
Expand All @@ -25,7 +26,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.Suma do
def identify(server_name \\ @default_name),
do:
server_name
|> identificaton_tuple
|> identification_tuple
nelsonkopliku marked this conversation as resolved.
Show resolved Hide resolved
|> :global.whereis_name()

def setup(server_name \\ @default_name),
Expand Down Expand Up @@ -95,24 +96,27 @@ defmodule Trento.Infrastructure.SoftwareUpdates.Suma do

defp do_handle({:get_system_id, fully_qualified_domain_name}, %State{
url: url,
auth: auth_cookie
auth: auth_cookie,
ca_cert: ca_cert
}),
do: SumaApi.get_system_id(url, auth_cookie, fully_qualified_domain_name)
do: SumaApi.get_system_id(url, auth_cookie, fully_qualified_domain_name, ca_cert != nil)

defp do_handle({:get_relevant_patches, system_id}, %State{
url: url,
auth: auth_cookie
auth: auth_cookie,
ca_cert: ca_cert
}),
do: SumaApi.get_relevant_patches(url, auth_cookie, system_id)
do: SumaApi.get_relevant_patches(url, auth_cookie, system_id, ca_cert != nil)

defp process_identifier(server_name), do: {:global, identificaton_tuple(server_name)}
defp process_identifier(server_name), do: {:global, identification_tuple(server_name)}

defp identificaton_tuple(server_name), do: {__MODULE__, server_name}
defp identification_tuple(server_name), do: {__MODULE__, server_name}

defp setup_auth(%State{} = state) do
with {:ok, %{url: url, username: username, password: password, ca_cert: ca_cert}} <-
SoftwareUpdates.get_settings(),
{:ok, auth_cookie} <- SumaApi.login(url, username, password) do
:ok <- write_ca_cert_file(ca_cert),
{:ok, auth_cookie} <- SumaApi.login(url, username, password, ca_cert != nil) do
{:ok,
%State{
state
Expand All @@ -124,4 +128,14 @@ defmodule Trento.Infrastructure.SoftwareUpdates.Suma do
}}
end
end

defp write_ca_cert_file(nil), do: File.rm(SumaApi.ca_cert_path())

defp write_ca_cert_file(ca_cert) do
SumaApi.ca_cert_path()
|> Path.dirname()
|> File.mkdir_p!()

File.write(SumaApi.ca_cert_path(), ca_cert)
end
end
51 changes: 35 additions & 16 deletions lib/trento/infrastructure/software_updates/suma_api.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,35 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do

@login_retries 5

@spec login(url :: String.t(), username :: String.t(), password :: String.t()) ::
@ca_cert_path "/tmp/suma_ca_cert.crt"

def ca_cert_path, do: @ca_cert_path

@spec login(
url :: String.t(),
username :: String.t(),
password :: String.t(),
use_ca_cert :: boolean()
) ::
{:ok, any()} | {:error, :max_login_retries_reached | any()}
def login(url, username, password),
def login(url, username, password, use_ca_cert),
do:
url
|> get_suma_api_url()
|> try_login(username, password, @login_retries)

@spec get_system_id(url :: String.t(), auth :: any(), fully_qualified_domain_name :: String.t()) ::
|> try_login(username, password, use_ca_cert, @login_retries)

@spec get_system_id(
url :: String.t(),
auth :: any(),
fully_qualified_domain_name :: String.t(),
use_ca_cert :: boolean()
) ::
{:ok, pos_integer()} | {:error, :system_id_not_found | :authentication_error}
def get_system_id(url, auth, fully_qualified_domain_name) do
def get_system_id(url, auth, fully_qualified_domain_name, use_ca_cert) do
response =
url
|> get_suma_api_url()
|> http_executor().get_system_id(auth, fully_qualified_domain_name)
|> http_executor().get_system_id(auth, fully_qualified_domain_name, use_ca_cert)

with {:ok, %HTTPoison.Response{status_code: 200, body: body}} <- response,
{:ok, %{success: true, result: result}} <- Jason.decode(body, keys: :atoms),
Expand All @@ -41,14 +55,19 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do
end
end

@spec get_relevant_patches(url :: String.t(), auth :: any(), system_id :: pos_integer()) ::
@spec get_relevant_patches(
url :: String.t(),
auth :: any(),
system_id :: pos_integer(),
use_ca_cert :: boolean()
) ::
{:ok, [map()]}
| {:error, :error_getting_patches | :authentication_error}
def get_relevant_patches(url, auth, system_id) do
def get_relevant_patches(url, auth, system_id, use_ca_cert) do
response =
url
|> get_suma_api_url()
|> http_executor().get_relevant_patches(auth, system_id)
|> http_executor().get_relevant_patches(auth, system_id, use_ca_cert)

with {:ok, %HTTPoison.Response{status_code: 200, body: body}} <- response,
{:ok, %{success: true, result: result}} <- Jason.decode(body, keys: :atoms) do
Expand All @@ -70,24 +89,24 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do
defp get_suma_api_url(base_url),
do: String.trim_trailing(base_url, "/") <> "/rhn/manager/api"

defp try_login(_, _, _, 0) do
defp try_login(_, _, _, _, 0) do
Logger.error("Failed to Log into SUSE Manager. Max retries reached.")
{:error, :max_login_retries_reached}
end

defp try_login(url, username, password, retry) do
case do_login(url, username, password) do
defp try_login(url, username, password, use_ca_cert, retry) do
case do_login(url, username, password, use_ca_cert) do
{:ok, _} = successful_login ->
successful_login

{:error, reason} ->
Logger.error("Failed to Log into SUSE Manager, retrying...", error: inspect(reason))
try_login(url, username, password, retry - 1)
try_login(url, username, password, use_ca_cert, retry - 1)
end
end

defp do_login(url, username, password) do
case http_executor().login(url, username, password) do
defp do_login(url, username, password, use_ca_cert) do
case http_executor().login(url, username, password, use_ca_cert) do
{:ok, %HTTPoison.Response{headers: headers, status_code: 200} = response} ->
Logger.debug("Successfully logged into suma #{inspect(response)}")
{:ok, get_session_cookies(headers)}
Expand Down
59 changes: 43 additions & 16 deletions test/trento/infrastructure/software_updates/suma_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,41 @@
assert :sys.get_state(Suma.identify(@test_integration_name)) == expected_state
end

test "should save existing CA certificate to local file", %{
settings: %Settings{ca_cert: ca_cert}
} do
assert {:ok, _} = start_supervised({Suma, @test_integration_name})

expect(SumaApiMock, :login, fn _, _, _, true -> successful_login_response() end)

assert :ok = Suma.setup(@test_integration_name)

cert_file_path = "/tmp/suma_ca_cert.crt"

assert File.exists?(cert_file_path)
^ca_cert = File.read!(cert_file_path)
end

test "should not save CA certificate file if no cert is provided" do

Check failure on line 65 in test/trento/infrastructure/software_updates/suma_test.exs

View workflow job for this annotation

GitHub Actions / Test (Elixir 1.15.7, OTP 26)

test Process start up and identification should not save CA certificate file if no cert is provided (Trento.Infrastructure.SoftwareUpdates.SumaTest)
insert_software_updates_settings(ca_cert: nil, ca_uploaded_at: nil)

assert {:ok, _} = start_supervised({Suma, @test_integration_name})

expect(SumaApiMock, :login, fn _, _, _, false -> successful_login_response() end)

assert :ok = Suma.setup(@test_integration_name)

refute File.exists?("/tmp/suma_ca_cert.crt")
end

test "should redact sensitive data in SUMA state", %{
settings: %Settings{url: url, username: username, password: password}
} do
{:ok, _} = start_supervised({Suma, @test_integration_name})

base_api_url = "#{url}/rhn/manager/api"

expect(SumaApiMock, :login, fn ^base_api_url, ^username, ^password ->
expect(SumaApiMock, :login, fn ^base_api_url, ^username, ^password, _ ->
successful_login_response()
end)

Expand Down Expand Up @@ -88,7 +115,7 @@
base_api_url = "#{url}/rhn/manager/api"
auth_cookie = "pxt-session-cookie=4321"

expect(SumaApiMock, :login, fn ^base_api_url, ^username, ^password ->
expect(SumaApiMock, :login, fn ^base_api_url, ^username, ^password, _ ->
successful_login_response()
end)

Expand Down Expand Up @@ -117,7 +144,7 @@
]

for error_cause <- error_causes do
expect(SumaApiMock, :login, 5, fn _, _, _ -> error_cause end)
expect(SumaApiMock, :login, 5, fn _, _, _, _ -> error_cause end)

assert {:error, :max_login_retries_reached} = Suma.setup(@test_integration_name)

Expand Down Expand Up @@ -148,7 +175,7 @@

{:ok, _} = Agent.start_link(fn -> 0 end, name: :login_call_iteration)

expect(SumaApiMock, :login, 3, fn _, _, _ ->
expect(SumaApiMock, :login, 3, fn _, _, _, _ ->
iteration = Agent.get(:login_call_iteration, & &1)

iteration_response = Enum.at(responses, iteration)
Expand All @@ -169,7 +196,7 @@
end

describe "clearing up integration service" do
test "should clear service state", %{

Check failure on line 199 in test/trento/infrastructure/software_updates/suma_test.exs

View workflow job for this annotation

GitHub Actions / Test (Elixir 1.15.7, OTP 26)

test clearing up integration service should clear service state (Trento.Infrastructure.SoftwareUpdates.SumaTest)
settings: %Settings{url: url, username: username, password: password, ca_cert: ca_cert}
} do
{:ok, _} = start_supervised({Suma, @test_integration_name})
Expand Down Expand Up @@ -226,7 +253,7 @@

fqdn = "machine.fqdn.internal"

expect(SumaApiMock, :login, 1, fn _, _, _ -> successful_login_response() end)
expect(SumaApiMock, :login, 1, fn _, _, _, _ -> successful_login_response() end)

error_causes = [
{:ok, %HTTPoison.Response{status_code: 200, body: ~s({"success":true,"result":[]})}},
Expand All @@ -236,7 +263,7 @@
]

for error_cause <- error_causes do
expect(SumaApiMock, :get_system_id, 1, fn _, _, ^fqdn -> error_cause end)
expect(SumaApiMock, :get_system_id, 1, fn _, _, ^fqdn, _ -> error_cause end)

assert {:error, :system_id_not_found} = Suma.get_system_id(fqdn, @test_integration_name)
end
Expand All @@ -247,9 +274,9 @@

fqdn = "machine.fqdn.internal"

expect(SumaApiMock, :login, 1, fn _, _, _ -> successful_login_response() end)
expect(SumaApiMock, :login, 1, fn _, _, _, _ -> successful_login_response() end)

expect(SumaApiMock, :get_system_id, 1, fn _, _, ^fqdn ->
expect(SumaApiMock, :get_system_id, 1, fn _, _, ^fqdn, _ ->
{:ok,
%HTTPoison.Response{
status_code: 200,
Expand All @@ -265,7 +292,7 @@

system_id = 1_000_010_001

expect(SumaApiMock, :login, 1, fn _, _, _ -> successful_login_response() end)
expect(SumaApiMock, :login, 1, fn _, _, _, _ -> successful_login_response() end)

error_causes = [
{:ok,
Expand All @@ -291,7 +318,7 @@
]

for error_cause <- error_causes do
expect(SumaApiMock, :get_relevant_patches, 1, fn _, _, ^system_id -> error_cause end)
expect(SumaApiMock, :get_relevant_patches, 1, fn _, _, ^system_id, _ -> error_cause end)

assert {:error, :error_getting_patches} =
Suma.get_relevant_patches(system_id, @test_integration_name)
Expand Down Expand Up @@ -326,9 +353,9 @@

suma_response_body = %{success: true, result: patches}

expect(SumaApiMock, :login, 1, fn _, _, _ -> successful_login_response() end)
expect(SumaApiMock, :login, 1, fn _, _, _, _ -> successful_login_response() end)

expect(SumaApiMock, :get_relevant_patches, 1, fn _, _, ^system_id ->
expect(SumaApiMock, :get_relevant_patches, 1, fn _, _, ^system_id, _ ->
{:ok, %HTTPoison.Response{status_code: 200, body: Jason.encode!(suma_response_body)}}
end)

Expand Down Expand Up @@ -378,7 +405,7 @@
for %{final_response: final_response, expected_result: expected_result} <- scenarios do
initial_auth_cookie = "pxt-session-cookie=INITIAL-COOKIE"

expect(SumaApiMock, :login, fn _, _, _ ->
expect(SumaApiMock, :login, fn _, _, _, _ ->
successful_login_response(initial_auth_cookie)
end)

Expand All @@ -392,11 +419,11 @@
new_auth_cookie = "pxt-session-cookie=NEW-COOKIE"

SumaApiMock
|> expect(:get_system_id, fn _, ^initial_auth_cookie, _ ->
|> expect(:get_system_id, fn _, ^initial_auth_cookie, _, _ ->
{:ok, %HTTPoison.Response{status_code: 401}}
end)
|> expect(:login, fn _, _, _ -> successful_login_response(new_auth_cookie) end)
|> expect(:get_system_id, fn _, ^new_auth_cookie, _ -> {:ok, final_response} end)
|> expect(:login, fn _, _, _, _ -> successful_login_response(new_auth_cookie) end)
|> expect(:get_system_id, fn _, ^new_auth_cookie, _, _ -> {:ok, final_response} end)

assert ^expected_result = Suma.get_system_id("fqdn", @test_integration_name)

Expand Down
Loading