Merge pull request #28 from firezone/andrew/refactor

Simplify the codebase and remove the issues with timeouts, race conditions and dead code

.github/workflows/ci.yml

@@ -2,9 +2,9 @@ name: Elixir CI
 
 on:
   push:
-    branches: [ "master" ]
+    branches: ["main"]
   pull_request:
-    branches: [ "master" ]
+    branches: ["main"]
 
 permissions:
   contents: read

config/config.exs

@@ -43,6 +43,10 @@ config :probe, Probe.Endpoint,
   pubsub_server: Probe.PubSub,
   live_view: [signing_salt: "0PeI+5mX"]
 
+config :probe, :session,
+  signing_salt: "p//dOtPa",
+  encryption_salt: "p//dOtPa"
+
 # Configure esbuild (the version is required)
 config :esbuild,
   version: "0.17.11",

config/runtime.exs

@@ -70,36 +70,4 @@ if config_env() == :prod do
       port: port
     ],
     secret_key_base: secret_key_base
-
-  # ## SSL Support
-  #
-  # To get SSL working, you will need to add the `https` key
-  # to your endpoint configuration:
-  #
-  #     config :probe, Probe.Endpoint,
-  #       https: [
-  #         ...,
-  #         port: 443,
-  #         cipher_suite: :strong,
-  #         keyfile: System.get_env("SOME_APP_SSL_KEY_PATH"),
-  #         certfile: System.get_env("SOME_APP_SSL_CERT_PATH")
-  #       ]
-  #
-  # The `cipher_suite` is set to `:strong` to support only the
-  # latest and more secure SSL ciphers. This means old browsers
-  # and clients may not be supported. You can set it to
-  # `:compatible` for wider support.
-  #
-  # `:keyfile` and `:certfile` expect an absolute path to the key
-  # and cert in disk or a relative path inside priv, for example
-  # "priv/ssl/server.key". For all supported SSL configuration
-  # options, see https://hexdocs.pm/plug/Plug.SSL.html#configure/1
-  #
-  # We also recommend setting `force_ssl` in your config/prod.exs,
-  # ensuring no data is ever sent via http, always redirecting to https:
-  #
-  #     config :probe, Probe.Endpoint,
-  #       force_ssl: [hsts: true]
-  #
-  # Check `Plug.SSL` for all available options in `force_ssl`.
 end

lib/probe/components/list_components.ex

@@ -27,10 +27,12 @@ defmodule Probe.ListComponents do
                 <%= stat.country %>
               </td>
               <td class="px-6 py-4 whitespace-nowrap dark:text-white">
-                <%= stat.num_runs %>
+                <%= stat.num_completed %>
               </td>
               <td class="px-6 py-4 whitespace-nowrap dark:text-white">
-                <%= :erlang.float_to_binary(stat.num_succeeded * 100 / stat.num_runs, decimals: 2) %>%
+                <%= :erlang.float_to_binary(stat.num_succeeded * 100 / stat.num_completed,
+                  decimals: 2
+                ) %>%
               </td>
             </tr>
           <% end %>

lib/probe/components/map_components.ex

@@ -10106,19 +10106,19 @@ defmodule Probe.MapComponents do
     stats
     |> Enum.map_join("\n", fn %{
                                 country: country,
-                                num_runs: num_runs,
+                                num_completed: num_completed,
                                 num_succeeded: num_succeeded
                               } ->
       """
       .#{String.downcase(country)} {
-        fill: #{fill(num_runs, num_succeeded)};
+        fill: #{fill(num_completed, num_succeeded)};
       }
       """
     end)
   end
 
-  defp fill(num_runs, num_succeeded) do
-    case num_succeeded / num_runs do
+  defp fill(num_completed, num_succeeded) do
+    case num_succeeded / num_completed do
       rate when rate < 0.1 -> "#8b0000"
       rate when rate < 0.2 -> "#B22222"
       rate when rate < 0.3 -> "#DC143C"

lib/probe/controllers/run.ex

@@ -1,59 +1,65 @@
 defmodule Probe.Controllers.Run do
   use Probe, :controller
+  alias Probe.Token
+  alias Probe.Runs
   require Logger
-  alias Probe.Runs.UdpServer
-
-  @run_timeout 15_000
-
-  # 1 hour
-  @token_max_age 3600
 
   action_fallback Probe.Controllers.Fallback
 
   def start(conn, %{"token" => token}) do
-    with {:ok, %{topic: topic, port: port} = attrs} <-
-           Phoenix.Token.verify(Probe.Endpoint, "topic", token, max_age: @token_max_age) do
-      Probe.PubSub.broadcast("run:#{topic}", :started)
-
-      {city, region, country, latitude, longitude, provider} = get_remote_ip_location(conn)
-
-      attrs =
-        Map.merge(attrs, %{
-          port: port,
-          topic: topic,
-          checks: %{},
-          remote_ip_location_country: country,
-          remote_ip_location_region: region,
-          remote_ip_location_city: city,
-          remote_ip_location_lat: latitude,
-          remote_ip_location_lon: longitude,
-          remote_ip_provider: provider
-        })
-
-      {:ok, run} = Probe.Runs.start_run(attrs)
-
-      Task.start(fn ->
-        Process.sleep(@run_timeout)
-        Probe.PubSub.broadcast("run:#{topic}", {:completed, run.id})
-      end)
+    with {:ok, %{session_id: session_id, pid: pid, port: port}} <- Token.verify(token),
+         true <- Process.alive?(pid) do
+      remote_ip = get_client_ip(conn)
+      anonymized_id = get_anonymized_id(session_id, remote_ip)
+      {city, region, country, latitude, longitude, provider} = geolocate_ip(remote_ip)
+
+      attrs = %{
+        anonymized_id: anonymized_id,
+        port: port,
+        remote_ip_location_country: country,
+        remote_ip_location_region: region,
+        remote_ip_location_city: city,
+        remote_ip_location_lat: latitude,
+        remote_ip_location_lon: longitude,
+        remote_ip_provider: provider
+      }
+
+      {:ok, run} = Probe.Runs.start_run(pid, attrs)
 
       send_resp(conn, 200, init_data(run))
     else
+      false ->
+        send_resp(
+          conn,
+          401,
+          """
+          Error: You are using an invalid or expired token.
+
+          Please visit the https://probe.sh website to generate a new token
+          and don't close the page until the test is completed.
+          """
+        )
+
       error ->
-        Logger.error("Failed to start run: #{inspect(error)}")
-        send_resp(conn, 401, "invalid or expired token")
+        Logger.warning("Failed to start run: #{inspect(error)}")
+
+        send_resp(conn, 401, """
+        Error: You are using an invalid or expired token.
+
+        Please visit the https://probe.sh website to generate a new token.
+        """)
     end
   end
 
   def complete(conn, %{"id" => id}) do
     {:ok, run} = Probe.Runs.fetch_run(id)
-    Probe.PubSub.broadcast("run:#{run.topic}", {:completed, run.id})
+    Probe.Runs.complete_run(run)
     send_resp(conn, 200, "")
   end
 
   def cancel(conn, %{"id" => id}) do
     {:ok, run} = Probe.Runs.fetch_run(id)
-    Probe.PubSub.broadcast("run:#{run.topic}", {:canceled, run.id})
+    Probe.Runs.cancel_run(run)
     send_resp(conn, 200, "")
   end
 
@@ -75,8 +81,22 @@ defmodule Probe.Controllers.Run do
     """)
   end
 
-  defp get_remote_ip_location(conn) do
-    remote_ip = get_client_ip(conn)
+  defp get_anonymized_id(session_id, remote_ip) do
+    anonymized_remote_ip =
+      remote_ip
+      |> Tuple.to_list()
+      |> Enum.sum()
+      |> to_string()
+
+    today =
+      Date.utc_today()
+      |> Date.to_iso8601()
+
+    :crypto.hash(:sha256, session_id <> anonymized_remote_ip <> today)
+    |> Base.encode64(padding: false)
+  end
+
+  defp geolocate_ip(remote_ip) do
     result = Geolix.lookup(remote_ip, [])
     region = get_in(result.city.continent.name)
     country = get_in(result.city.country.iso_code) || "Unknown"
@@ -89,7 +109,7 @@ defmodule Probe.Controllers.Run do
 
   defp get_client_ip(conn) do
     case Plug.Conn.get_req_header(conn, "fly-client-ip") do
-      [ip | _] -> ip
+      [ip | _] -> :inet.parse_address(Kernel.to_charlist(ip))
       [] -> conn.remote_ip
     end
   end
@@ -108,14 +128,14 @@ defmodule Probe.Controllers.Run do
     #{url(~p"/runs/#{run.id}")}
     #{run.port}
     #{:inet.ntoa(ip)}
-    #{Base.encode64(UdpServer.generate_handshake_initiation_payload(run))}
-    #{Base.encode64(UdpServer.generate_handshake_response_payload(run))}
-    #{Base.encode64(UdpServer.generate_cookie_reply_payload(run))}
-    #{Base.encode64(UdpServer.generate_data_message_payload(run))}
-    #{Base.encode64(UdpServer.generate_turn_handshake_initiation_payload(run))}
-    #{Base.encode64(UdpServer.generate_turn_handshake_response_payload(run))}
-    #{Base.encode64(UdpServer.generate_turn_cookie_reply_payload(run))}
-    #{Base.encode64(UdpServer.generate_turn_data_message_payload(run))}
+    #{Base.encode64(Runs.UDPServer.generate_handshake_initiation_payload(run))}
+    #{Base.encode64(Runs.UDPServer.generate_handshake_response_payload(run))}
+    #{Base.encode64(Runs.UDPServer.generate_cookie_reply_payload(run))}
+    #{Base.encode64(Runs.UDPServer.generate_data_message_payload(run))}
+    #{Base.encode64(Runs.UDPServer.generate_turn_handshake_initiation_payload(run))}
+    #{Base.encode64(Runs.UDPServer.generate_turn_handshake_response_payload(run))}
+    #{Base.encode64(Runs.UDPServer.generate_turn_cookie_reply_payload(run))}
+    #{Base.encode64(Runs.UDPServer.generate_turn_data_message_payload(run))}
     """
   end
 end

lib/probe/controllers/run_json.ex

@@ -1,25 +1,12 @@
 defmodule Probe.Controllers.RunJSON do
   use Probe, :controller
-  alias Probe.Runs
 
   def show(%{run: run, address: address}) do
     %{
       run_id: run.id,
       address: address,
-      checks: Enum.map(run.checks, &show_check/1)
-    }
-  end
-
-  defp show_check(check) do
-    packets =
-      for {type, binary} <- Runs.Adapters.client_packets(check), into: %{} do
-        {type, Base.encode64(binary)}
-      end
-
-    %{
-      adapter: check.adapter,
-      port: Runs.Adapters.port(check),
-      packets: packets
+      port: run.port,
+      checks: run.checks
     }
   end
 end

lib/probe/endpoint.ex

@@ -4,8 +4,8 @@ defmodule Probe.Endpoint do
   @session_options [
     store: :cookie,
     key: "_probe_key",
-    signing_salt: "p//dOtPa",
-    encryption_salt: "p//dOtPa",
+    signing_salt: Application.compile_env!(:probe, [:session, :signing_salt]),
+    encryption_salt: Application.compile_env!(:probe, [:session, :encryption_salt]),
     same_site: "Lax"
   ]
 
@@ -47,5 +47,15 @@ defmodule Probe.Endpoint do
   plug Plug.MethodOverride
   plug Plug.Head
   plug Plug.Session, @session_options
+  plug :fetch_session
+  plug :put_session_id
   plug Probe.Router
+
+  def put_session_id(conn, _opts) do
+    if Plug.Conn.get_session(conn, :session_id) do
+      conn
+    else
+      Plug.Conn.put_session(conn, :session_id, Ecto.UUID.generate())
+    end
+  end
 end

lib/probe/live/component/run.ex

@@ -20,7 +20,7 @@ defmodule Probe.Live.Component.Run do
           Test your WireGuard connectivity
         </h1>
         <p class="underline text-gray-600 dark:text-gray-400 mt-4">
-          <i>No WireGuard client required!</i>
+          <i>No WireGuard® client required!</i>
         </p>
       </div>
       <%= if @os && @os =~ ~r/(Mac OS X|Windows|Linux|FreeBSD|OpenBSD)/ do %>
@@ -290,29 +290,29 @@ defmodule Probe.Live.Component.Run do
                       type="Handshake initiation"
                       header="0x01"
                       description="First message to initiate a tunnel"
-                      status={@checks.handshake_initiation}
-                      turn_status={@checks.turn_handshake_initiation}
+                      status={get_in(@run.checks.handshake_initiation)}
+                      turn_status={get_in(@run.checks.turn_handshake_initiation)}
                     />
                     <.check_row
                       type="Handshake response"
                       header="0x02"
                       description="Reply to the handshake initiation message"
-                      status={@checks.handshake_response}
-                      turn_status={@checks.turn_handshake_response}
+                      status={get_in(@run.checks.handshake_response)}
+                      turn_status={get_in(@run.checks.turn_handshake_response)}
                     />
                     <.check_row
                       type="Cookie reply"
                       header="0x03"
                       description="Used to mitigate DoS attacks"
-                      status={@checks.cookie_reply}
-                      turn_status={@checks.turn_cookie_reply}
+                      status={get_in(@run.checks.cookie_reply)}
+                      turn_status={get_in(@run.checks.turn_cookie_reply)}
                     />
                     <.check_row
                       type="Data message"
                       header="0x04"
                       description="The encrypted payload used to transport application data."
-                      status={@checks.data_message}
-                      turn_status={@checks.turn_data_message}
+                      status={get_in(@run.checks.data_message)}
+                      turn_status={get_in(@run.checks.turn_data_message)}
                     />
                   </tbody>
                 </table>

lib/probe/live/component/stats.ex

@@ -1,10 +1,10 @@
 defmodule Probe.Live.Component.Results do
   use Probe, :live_component
-  alias Probe.Stats
+  alias Probe.Runs
 
   def mount(socket) do
     if connected?(socket) do
-      stats = Stats.country_stats()
+      stats = Runs.country_stats()
       {:ok, assign(socket, :stats, stats)}
     else
       {:ok, assign(socket, :stats, nil)}