From c1c46707ba8da9796eec247d03f8047f7c8c91d8 Mon Sep 17 00:00:00 2001 From: Mike Cifelli Date: Mon, 1 Apr 2024 10:40:35 -0400 Subject: [PATCH] Clean up code --- lib/chronoscope/nts.ex | 13 +- .../v1/nts/key_establishment_controller.ex | 9 +- test/chronoscope/nts/certificate_test.exs | 13 +- .../nts/key_establishment_controller_test.exs | 134 ++++++++++-------- test/support/case.ex | 8 ++ test/support/conn_case.ex | 1 + test/support/mocks.ex | 9 +- 7 files changed, 115 insertions(+), 72 deletions(-) create mode 100644 test/support/case.ex diff --git a/lib/chronoscope/nts.ex b/lib/chronoscope/nts.ex index 9315101..cd4edf9 100644 --- a/lib/chronoscope/nts.ex +++ b/lib/chronoscope/nts.ex @@ -27,13 +27,14 @@ defmodule Chronoscope.NTS do GenServer.call(pid, :key_establishment) [] -> - NTS.DynamicSupervisor - |> DynamicSupervisor.start_child({ - NTS.Client, - host: host, port: port, name: {:via, Registry, {NTS.Registry, name}} - }) + {:ok, pid} = + NTS.DynamicSupervisor + |> DynamicSupervisor.start_child({ + NTS.Client, + host: host, port: port, name: {:via, Registry, {NTS.Registry, name}} + }) - key_establishment(host, port) + GenServer.call(pid, :key_establishment) end end end diff --git a/lib/chronoscope_web/controllers/api/v1/nts/key_establishment_controller.ex b/lib/chronoscope_web/controllers/api/v1/nts/key_establishment_controller.ex index 3892e07..3a1f4e0 100644 --- a/lib/chronoscope_web/controllers/api/v1/nts/key_establishment_controller.ex +++ b/lib/chronoscope_web/controllers/api/v1/nts/key_establishment_controller.ex @@ -6,6 +6,7 @@ defmodule ChronoscopeWeb.API.V1.NTS.KeyEstablishmentController do alias Chronoscope.NTS @default_port 4460 + @max_host_length 255 def get(conn, %{"host" => host, "port" => port}) do try do @@ -24,7 +25,7 @@ defmodule ChronoscopeWeb.API.V1.NTS.KeyEstablishmentController do end defp handle_get(conn, %{host: host, port: port}) when port > 0 and port < 65536 do - case nts_behaviour().key_establishment(host, port) do + case key_establishment_response(host, port) do {:ok, response} -> json(conn, %{status: :ok, response: format_response(response)}) @@ -37,6 +38,12 @@ defmodule ChronoscopeWeb.API.V1.NTS.KeyEstablishmentController do bad_request_response(conn, "port out of range") end + defp key_establishment_response(host, port) do + host + |> String.slice(0, @max_host_length) + |> nts_behaviour().key_establishment(port) + end + defp format_response(response) do response |> Map.take([:aead_algorithms, :cert_expiration, :cookie_length, :cookies, :next_protocols, :port, :server]) diff --git a/test/chronoscope/nts/certificate_test.exs b/test/chronoscope/nts/certificate_test.exs index e78cfb9..2d5f4da 100644 --- a/test/chronoscope/nts/certificate_test.exs +++ b/test/chronoscope/nts/certificate_test.exs @@ -1,5 +1,7 @@ defmodule Chronoscope.NTS.CertificateTest do - use ExUnit.Case + use Chronoscope.Case + + alias Chronoscope.NTS.DateTimeMock import Chronoscope.NTS.Certificate import Mox @@ -7,9 +9,6 @@ defmodule Chronoscope.NTS.CertificateTest do setup :verify_on_exit! test "parses the expiration date of a certificate" do - Chronoscope.NTS.DateTimeMock - |> stub(:utc_now, &DateTime.utc_now/0) - {:ok, expiration, _} = :secp256r1 |> X509.PrivateKey.new_ec() @@ -22,21 +21,21 @@ defmodule Chronoscope.NTS.CertificateTest do end test "converts certificate datetime to iso8601" do - Chronoscope.NTS.DateTimeMock + DateTimeMock |> expect(:utc_now, fn -> ~U[2024-03-31 01:23:45Z] end) assert cert_time_to_iso8601("240326110000Z") == "2024-03-26T11:00:00Z" end test "handles century rollover" do - Chronoscope.NTS.DateTimeMock + DateTimeMock |> expect(:utc_now, fn -> ~U[2024-03-31 01:23:45Z] end) assert cert_time_to_iso8601("010326110000Z") == "2101-03-26T11:00:00Z" end test "handles millenium rollover" do - Chronoscope.NTS.DateTimeMock + DateTimeMock |> expect(:utc_now, fn -> ~U[2999-03-31 01:23:45Z] end) assert cert_time_to_iso8601("010326110000Z") == "3001-03-26T11:00:00Z" diff --git a/test/chronoscope_web/controllers/api/v1/nts/key_establishment_controller_test.exs b/test/chronoscope_web/controllers/api/v1/nts/key_establishment_controller_test.exs index c86b993..03703a6 100644 --- a/test/chronoscope_web/controllers/api/v1/nts/key_establishment_controller_test.exs +++ b/test/chronoscope_web/controllers/api/v1/nts/key_establishment_controller_test.exs @@ -5,77 +5,99 @@ defmodule ChronoscopeWeb.API.V1.NTS.KeyEstablishmentControllerTest do setup :verify_on_exit! - test "requires a host name", %{conn: conn} do - response = - conn - |> get(~p"/api/v1/nts/key-establishment") - |> json_response(400) + describe "/api/v1/nts/key-establishment" do + test "requires a host name", %{conn: conn} do + response = + conn + |> get(~p"/api/v1/nts/key-establishment") + |> json_response(400) - assert %{"error" => "missing host"} == response - end + assert %{"error" => "missing host"} == response + end - test "uses the given port number", %{conn: conn} do - Chronoscope.NTS.BehaviourMock - |> expect(:key_establishment, fn "localhost", 4461 -> {:ok, %{status: :ok}} end) + test "truncates the host name", %{conn: conn} do + Chronoscope.NTS.BehaviourMock + |> expect( + :key_establishment, + fn "test.example.com.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456", + 4460 -> + {:ok, %{status: :ok}} + end + ) - response = - conn - |> get(~p"/api/v1/nts/key-establishment?host=localhost&port=4461") - |> json_response(200) + response = + conn + |> get( + ~p"/api/v1/nts/key-establishment?host=test.example.com.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789.0123456789" + ) + |> json_response(200) - assert %{"status" => "ok", "response" => %{"cookies" => 0}} == response - end + assert %{"status" => "ok", "response" => %{"cookies" => 0}} == response + end - test "handles an out of range port number", %{conn: conn} do - response = - conn - |> get(~p"/api/v1/nts/key-establishment?host=localhost&port=65536") - |> json_response(400) + test "uses the given port number", %{conn: conn} do + Chronoscope.NTS.BehaviourMock + |> expect(:key_establishment, fn "localhost", 4461 -> {:ok, %{status: :ok}} end) - assert %{"error" => "port out of range"} == response - end + response = + conn + |> get(~p"/api/v1/nts/key-establishment?host=localhost&port=4461") + |> json_response(200) - test "handles a negative port number", %{conn: conn} do - response = - conn - |> get(~p"/api/v1/nts/key-establishment?host=localhost&port=-4460") - |> json_response(400) + assert %{"status" => "ok", "response" => %{"cookies" => 0}} == response + end - assert %{"error" => "port out of range"} == response - end + test "handles an out of range port number", %{conn: conn} do + response = + conn + |> get(~p"/api/v1/nts/key-establishment?host=localhost&port=65536") + |> json_response(400) - test "handles a bad port number", %{conn: conn} do - response = - conn - |> get(~p"/api/v1/nts/key-establishment?host=localhost&port=AA60") - |> json_response(400) + assert %{"error" => "port out of range"} == response + end - assert %{"error" => "invalid port"} == response - end + test "handles a negative port number", %{conn: conn} do + response = + conn + |> get(~p"/api/v1/nts/key-establishment?host=localhost&port=-4460") + |> json_response(400) - test "returns an empty successful response", %{conn: conn} do - Chronoscope.NTS.BehaviourMock - |> expect(:key_establishment, fn "localhost", 4460 -> {:ok, %{status: :ok}} end) + assert %{"error" => "port out of range"} == response + end - response = - conn - |> get(~p"/api/v1/nts/key-establishment?host=localhost") - |> json_response(200) + test "handles a bad port number", %{conn: conn} do + response = + conn + |> get(~p"/api/v1/nts/key-establishment?host=localhost&port=AA60") + |> json_response(400) - assert %{"status" => "ok", "response" => %{"cookies" => 0}} == response - end + assert %{"error" => "invalid port"} == response + end - test "returns a full successful response", %{conn: conn} do - Chronoscope.NTS.BehaviourMock - |> expect(:key_establishment, fn "localhost", 4460 -> - {:ok, %{cookies: [[], [], []], cookie_length: 300}} - end) + test "returns an empty successful response", %{conn: conn} do + Chronoscope.NTS.BehaviourMock + |> expect(:key_establishment, fn "localhost", 4460 -> {:ok, %{status: :ok}} end) - response = - conn - |> get(~p"/api/v1/nts/key-establishment?host=localhost") - |> json_response(200) + response = + conn + |> get(~p"/api/v1/nts/key-establishment?host=localhost") + |> json_response(200) - assert %{"status" => "ok", "response" => %{"cookies" => 3, "cookie_length" => 300}} == response + assert %{"status" => "ok", "response" => %{"cookies" => 0}} == response + end + + test "returns a full successful response", %{conn: conn} do + Chronoscope.NTS.BehaviourMock + |> expect(:key_establishment, fn "localhost", 4460 -> + {:ok, %{cookies: [[], [], []], cookie_length: 300}} + end) + + response = + conn + |> get(~p"/api/v1/nts/key-establishment?host=localhost") + |> json_response(200) + + assert %{"status" => "ok", "response" => %{"cookies" => 3, "cookie_length" => 300}} == response + end end end diff --git a/test/support/case.ex b/test/support/case.ex new file mode 100644 index 0000000..18a5228 --- /dev/null +++ b/test/support/case.ex @@ -0,0 +1,8 @@ +defmodule Chronoscope.Case do + use ExUnit.CaseTemplate + + setup _tags do + Mox.stub_with(Chronoscope.NTS.DateTimeMock, Chronoscope.DateTime.Stub) + :ok + end +end diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index a214180..dcfe251 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -32,6 +32,7 @@ defmodule ChronoscopeWeb.ConnCase do end setup _tags do + Mox.stub_with(Chronoscope.NTS.DateTimeMock, Chronoscope.DateTime.Stub) {:ok, conn: Phoenix.ConnTest.build_conn()} end end diff --git a/test/support/mocks.ex b/test/support/mocks.ex index 1730f51..ee25f62 100644 --- a/test/support/mocks.ex +++ b/test/support/mocks.ex @@ -1,6 +1,11 @@ -defmodule DateTime.Behaviour do +defmodule Chronoscope.DateTime.Behaviour do @callback utc_now :: DateTime.t() end +defmodule Chronoscope.DateTime.Stub do + @behaviour Chronoscope.DateTime.Behaviour + def utc_now(), do: DateTime.utc_now() +end + Mox.defmock(Chronoscope.NTS.BehaviourMock, for: Chronoscope.NTS.Behaviour) -Mox.defmock(Chronoscope.NTS.DateTimeMock, for: DateTime.Behaviour) +Mox.defmock(Chronoscope.NTS.DateTimeMock, for: Chronoscope.DateTime.Behaviour)