Merge pull request #346 from firezone/backlog/129/persistent_keepalives

apps/fz_http/lib/fz_http/devices.ex

@@ -103,8 +103,21 @@ defmodule FzHttp.Devices do
     end
   end
 
+  def persistent_keepalives(device) do
+    if device.use_default_persistent_keepalives do
+      Settings.default_device_persistent_keepalives()
+    else
+      device.persistent_keepalives
+    end
+  end
+
   def defaults(changeset) do
-    ~w(use_default_allowed_ips use_default_dns_servers use_default_endpoint)a
+    ~w(
+      use_default_allowed_ips
+      use_default_dns_servers
+      use_default_endpoint
+      use_default_persistent_keepalives
+    )a
     |> Enum.map(fn field -> {field, Device.field(changeset, field)} end)
     |> Map.new()
   end
@@ -122,6 +135,7 @@ defmodule FzHttp.Devices do
     PublicKey = #{device.server_public_key}
     AllowedIPs = #{allowed_ips(device)}
     Endpoint = #{endpoint(device)}:#{wireguard_port}
+    #{persistent_keepalives_config(device)}
     """
   end
 
@@ -136,6 +150,17 @@ defmodule FzHttp.Devices do
     update_device(device, config_token_attrs)
   end
 
+  defp persistent_keepalives_config(device) do
+    pk = persistent_keepalives(device)
+    pk && "PersistentKeepalives = #{pk}"
+
+    if is_nil(pk) do
+      ""
+    else
+      "PersistentKeepalives = #{pk}"
+    end
+  end
+
   defp dns_servers_config(device) when is_struct(device) do
     dns_servers = dns_servers(device)
 

apps/fz_http/lib/fz_http/devices/device.ex

@@ -23,7 +23,9 @@ defmodule FzHttp.Devices.Device do
     field :use_default_allowed_ips, :boolean, read_after_writes: true, default: true
     field :use_default_dns_servers, :boolean, read_after_writes: true, default: true
     field :use_default_endpoint, :boolean, read_after_writes: true, default: true
+    field :use_default_persistent_keepalives, :boolean, read_after_writes: true, default: true
     field :endpoint, :string
+    field :persistent_keepalives, :integer
     field :allowed_ips, :string
     field :dns_servers, :string
     field :private_key, FzHttp.Encrypted.Binary
@@ -62,9 +64,11 @@ defmodule FzHttp.Devices.Device do
       :use_default_allowed_ips,
       :use_default_dns_servers,
       :use_default_endpoint,
+      :use_default_persistent_keepalives,
       :allowed_ips,
       :dns_servers,
       :endpoint,
+      :persistent_keepalives,
       :remote_ip,
       :address,
       :server_public_key,
@@ -86,12 +90,21 @@ defmodule FzHttp.Devices.Device do
       :server_public_key,
       :private_key
     ])
-    |> validate_required_unless_default([:allowed_ips, :dns_servers, :endpoint])
-    |> validate_omitted_if_default([:allowed_ips, :dns_servers, :endpoint])
+    |> validate_required_unless_default([
+      :allowed_ips,
+      :dns_servers,
+      :endpoint,
+      :persistent_keepalives
+    ])
+    |> validate_omitted_if_default([:allowed_ips, :dns_servers, :endpoint, :persistent_keepalives])
     |> validate_list_of_ips_or_cidrs(:allowed_ips)
     |> validate_list_of_ips(:dns_servers)
     |> validate_no_duplicates(:dns_servers)
     |> validate_ip(:endpoint)
+    |> validate_number(:persistent_keepalives,
+      greater_than_or_equal_to: 0,
+      less_than_or_equal_to: 120
+    )
     |> unique_constraint(:address)
     |> validate_number(:address, greater_than_or_equal_to: 2, less_than_or_equal_to: 254)
     |> unique_constraint(:public_key)

apps/fz_http/lib/fz_http/macros.ex

@@ -3,6 +3,9 @@ defmodule FzHttp.Macros do
   Metaprogramming macros
   """
 
+  @doc """
+  Defines getters for all Setting keys as functions on the Settings module.
+  """
   defmacro def_settings(keys) do
     quote bind_quoted: [keys: keys] do
       Enum.each(keys, fn key ->

apps/fz_http/lib/fz_http/settings.ex

@@ -13,6 +13,7 @@ defmodule FzHttp.Settings do
     default.device.allowed_ips
     default.device.dns_servers
     default.device.endpoint
+    default.device.persistent_keepalives
   ))
 
   @doc """

apps/fz_http/lib/fz_http/settings/setting.ex

@@ -15,6 +15,7 @@ defmodule FzHttp.Settings.Setting do
 
   import FzHttp.SharedValidators,
     only: [
+      validate_ip: 2,
       validate_list_of_ips: 2,
       validate_list_of_ips_or_cidrs: 2,
       validate_no_duplicates: 2
@@ -57,8 +58,12 @@ defmodule FzHttp.Settings.Setting do
 
   defp validate_kv_pair(changeset, "default.device.endpoint") do
     changeset
-    |> validate_list_of_ips_or_cidrs(:value)
-    |> validate_no_duplicates(:value)
+    |> validate_ip(:value)
+  end
+
+  defp validate_kv_pair(changeset, "default.device.persistent_keepalives") do
+    changeset
+    |> validate_number(:value, greater_than_or_equal_to: 0, less_than_or_equal_to: 120)
   end
 
   defp validate_kv_pair(changeset, unknown_key) do

apps/fz_http/lib/fz_http_web/live/device_live/form_component.ex

@@ -18,6 +18,10 @@ defmodule FzHttpWeb.DeviceLive.FormComponent do
      |> assign(:default_device_allowed_ips, Settings.default_device_allowed_ips())
      |> assign(:default_device_dns_servers, Settings.default_device_dns_servers())
      |> assign(:default_device_endpoint, default_device_endpoint)
+     |> assign(
+       :default_device_persistent_keepalives,
+       Settings.default_device_persistent_keepalives()
+     )
      |> assign(:changeset, changeset)}
   end
 

apps/fz_http/lib/fz_http_web/live/device_live/form_component.html.heex

@@ -92,6 +92,39 @@
       </p>
     </div>
 
+    <div class="field">
+      <%= label f, :use_default_persistent_keepalives, "Use Default Persistent Keepalives", class: "label" %>
+      <div class="control">
+        <label class="radio">
+          <%= radio_button f, :use_default_persistent_keepalives, true %>
+          Yes
+        </label>
+        <label class="radio">
+          <%= radio_button f, :use_default_persistent_keepalives, false %>
+          No
+        </label>
+      </div>
+      <p class="help">
+        Default: <%= @default_device_persistent_keepalives %>
+      </p>
+    </div>
+
+    <div class="field">
+      <%= label f, :persistent_keepalives, "Persistent Keepalives", class: "label" %>
+      <p>
+        Interval for WireGuard
+        <a href="https://www.wireguard.com/quickstart/#nat-and-firewall-traversal-persistence">
+          persistent keepalives</a>. A value of 0 disables this. Leave this disabled
+          unless you're experiencing NAT or firewall traversal problems.
+      </p>
+      <div class="control">
+        <%= text_input f, :persistent_keepalives, class: "input", disabled: @use_default_persistent_keepalives %>
+      </div>
+      <p class="help is-danger">
+        <%= error_tag f, :persistent_keepalives %>
+      </p>
+    </div>
+
     <div class="field">
       <%= label f, :address, "Interface Address (last octet only)", class: "label" %>
 

apps/fz_http/lib/fz_http_web/live/device_live/show.html.heex

@@ -62,6 +62,17 @@
         <td><%= @endpoint %></td>
       </tr>
 
+      <tr>
+        <td><strong>Persistent Keepalives</strong></td>
+        <td>
+          <%= if @persistent_keepalives == 0 do %>
+            Every <%= @persistent_keepalives %> seconds
+          <% else %>
+            Disabled
+          <% end %>
+        </td>
+      </tr>
+
       <tr>
         <td><strong>Public key</strong></td>
         <td class="code"><%= @device.public_key %></td>
@@ -87,8 +98,13 @@
     </div>
     <div class="level-right">
       <div class="field has-addons">
-        <div class={@dropdown_active_class <> " control dropdown is-right"}
-          phx-click-away="hide_config_token">
+        <div
+          id="shareable-link-trigger"
+          class={@dropdown_active_class <> " control dropdown is-right"}
+          phx-capture-click="close_dropdown"
+          phx-key="escape"
+          phx-click-away="close_dropdown"
+          phx-window-keydown="close_dropdown">
           <div class="dropdown-trigger">
             <button
               id="get-shareable-link"

apps/fz_http/lib/fz_http_web/live/device_live/show_live.ex

@@ -42,7 +42,7 @@ defmodule FzHttpWeb.DeviceLive.Show do
   end
 
   @impl Phoenix.LiveView
-  def handle_event("hide_config_token", _params, socket) do
+  def handle_event("close_dropdown", _params, socket) do
     {:noreply,
      socket
      |> assign(:dropdown_active_class, "")}
@@ -84,6 +84,7 @@ defmodule FzHttpWeb.DeviceLive.Show do
         allowed_ips: Devices.allowed_ips(device),
         dns_servers: Devices.dns_servers(device),
         endpoint: Devices.endpoint(device),
+        persistent_keepalives: Devices.persistent_keepalives(device),
         config: Devices.as_config(device)
       )
     else

apps/fz_http/lib/fz_http_web/live/modal_component.ex

@@ -4,11 +4,11 @@ defmodule FzHttpWeb.ModalComponent do
   """
   use FzHttpWeb, :live_component
 
-  @impl true
+  @impl Phoenix.LiveComponent
   def render(assigns) do
     ~H"""
     <div
-      id={@myself}
+      id={"modal-#{@myself}"}
       class="modal is-active"
       phx-capture-click="close"
       phx-window-keydown="close"
@@ -33,8 +33,18 @@ defmodule FzHttpWeb.ModalComponent do
     """
   end
 
-  @impl true
+  @impl Phoenix.LiveComponent
   def handle_event("close", _, socket) do
     {:noreply, push_patch(socket, to: socket.assigns.return_to)}
   end
+
+  @impl Phoenix.LiveComponent
+  @doc """
+  XXX: This is needed due to a bug on pages with dropdowns.
+  Basically this modal receives the phx-click-away event and the
+  server crashes if this is not implemented.
+  """
+  def handle_event("close_dropdown", _params, socket) do
+    {:noreply, socket}
+  end
 end

apps/fz_http/priv/repo/migrations/20211217003247_add_persistent_keepalives.exs

@@ -0,0 +1,17 @@
+defmodule FzHttp.Repo.Migrations.AddPersistentKeepalives do
+  use Ecto.Migration
+
+  def change do
+    alter table(:devices) do
+      add :persistent_keepalives, :integer
+      add :use_default_persistent_keepalives, :boolean, null: false, default: true
+    end
+
+    now = DateTime.utc_now()
+
+    execute """
+    INSERT INTO settings (key, value, inserted_at, updated_at) VALUES \
+    ('default.device.persistent_keepalives', 0, '#{now}', '#{now}')
+    """
+  end
+end

apps/fz_http/test/fz_http_web/live/device_live/show_test.exs

@@ -25,7 +25,16 @@ defmodule FzHttpWeb.DeviceLive.ShowTest do
       "device" => %{"use_default_endpoint" => "false", "endpoint" => @wireguard_endpoint}
     }
     @endpoint_unchanged %{
-      "device" => %{"use_default_endpoint" => "true", "dns_servers" => @wireguard_endpoint}
+      "device" => %{"use_default_endpoint" => "true", "endpoint" => @wireguard_endpoint}
+    }
+    @persistent_keepalives_change %{
+      "device" => %{
+        "use_default_persistent_keepalives" => "false",
+        "persistent_keepalives" => "120"
+      }
+    }
+    @persistent_keepalives_unchanged %{
+      "device" => %{"use_default_persistent_keepalives" => "true", "persistent_keepalives" => "5"}
     }
     @default_allowed_ips_change %{
       "device" => %{"use_default_allowed_ips" => "false"}
@@ -36,6 +45,9 @@ defmodule FzHttpWeb.DeviceLive.ShowTest do
     @default_endpoint_change %{
       "device" => %{"use_default_endpoint" => "false"}
     }
+    @default_persistent_keepalives_change %{
+      "device" => %{"use_default_persistent_keepalives" => "false"}
+    }
 
     test "shows device details", %{authed_conn: conn, device: device} do
       path = Routes.device_show_path(conn, :show, device)
@@ -112,6 +124,22 @@ defmodule FzHttpWeb.DeviceLive.ShowTest do
       assert test_view =~ "must not be present"
     end
 
+    test "prevents persistent_keepalives changes when use_default_persistent_keepalives is true",
+         %{
+           authed_conn: conn,
+           device: device
+         } do
+      path = Routes.device_show_path(conn, :edit, device)
+      {:ok, view, _html} = live(conn, path)
+
+      test_view =
+        view
+        |> form("#edit-device")
+        |> render_submit(@persistent_keepalives_unchanged)
+
+      assert test_view =~ "must not be present"
+    end
+
     test "allows allowed_ips changes", %{authed_conn: conn, device: device} do
       path = Routes.device_show_path(conn, :edit, device)
       {:ok, view, _html} = live(conn, path)
@@ -157,6 +185,21 @@ defmodule FzHttpWeb.DeviceLive.ShowTest do
       assert html =~ "Endpoint = #{@wireguard_endpoint}:51820"
     end
 
+    test "allows persistent_keepalives changes", %{authed_conn: conn, device: device} do
+      path = Routes.device_show_path(conn, :edit, device)
+      {:ok, view, _html} = live(conn, path)
+
+      view
+      |> form("#edit-device")
+      |> render_submit(@persistent_keepalives_change)
+
+      flash = assert_redirected(view, Routes.device_show_path(conn, :show, device))
+      assert flash["info"] == "Device updated successfully."
+
+      {:ok, _view, html} = live(conn, path)
+      assert html =~ "PersistentKeepalives = 120"
+    end
+
     test "prevents empty names", %{authed_conn: conn, device: device} do
       path = Routes.device_show_path(conn, :edit, device)
       {:ok, view, _html} = live(conn, path)
@@ -210,6 +253,20 @@ defmodule FzHttpWeb.DeviceLive.ShowTest do
              <input class="input" id="edit-device_endpoint" name="device[endpoint]" type="text"/>\
              """
     end
+
+    test "on use_default_persistent_keepalives change", %{authed_conn: conn, device: device} do
+      path = Routes.device_show_path(conn, :edit, device)
+      {:ok, view, _html} = live(conn, path)
+
+      test_view =
+        view
+        |> form("#edit-device")
+        |> render_change(@default_persistent_keepalives_change)
+
+      assert test_view =~ """
+             <input class="input" id="edit-device_persistent_keepalives" name="device[persistent_keepalives]" type="text"/>\
+             """
+    end
   end
 
   describe "delete own device" do