From 8fa280691d26a3a6aa22f8bac0474aad09e1320d Mon Sep 17 00:00:00 2001 From: Jamil Date: Mon, 26 Dec 2022 15:19:08 -0600 Subject: [PATCH] Conditionally append port to endpoint (#1247) Fixes a bug where generated configs have an extra port added in some cases. --- apps/fz_http/lib/fz_http/devices.ex | 17 +++++++- apps/fz_http/lib/fz_http/devices/device.ex | 2 - apps/fz_http/lib/fz_http/sites/site.ex | 2 - apps/fz_http/lib/fz_http/validators/common.ex | 14 ------- apps/fz_http/test/fz_http/devices_test.exs | 42 ------------------- .../live/setting_live/site_test.exs | 16 ------- 6 files changed, 15 insertions(+), 78 deletions(-) diff --git a/apps/fz_http/lib/fz_http/devices.ex b/apps/fz_http/lib/fz_http/devices.ex index 0cf272ac8..1b7f0489c 100644 --- a/apps/fz_http/lib/fz_http/devices.ex +++ b/apps/fz_http/lib/fz_http/devices.ex @@ -249,12 +249,25 @@ defmodule FzHttp.Devices do defp endpoint_config(device) do ep = endpoint(device) - wireguard_port = Application.fetch_env!(:fz_vpn, :wireguard_port) if field_empty?(ep) do "" else - "Endpoint = #{ep}:#{wireguard_port}" + "Endpoint = #{maybe_add_port(ep)}" + end + end + + # Finds a port in IPv6-formatted address, e.g. [2001::1]:51820 + @capture_port ~r/\[.*]:(?[\d]+)/ + defp maybe_add_port(endpoint) do + wireguard_port = Application.fetch_env!(:fz_vpn, :wireguard_port) + colon_count = endpoint |> String.graphemes() |> Enum.count(&(&1 == ":")) + + if colon_count == 1 or !is_nil(Regex.named_captures(@capture_port, endpoint)) do + endpoint + else + # No port found + "#{endpoint}:#{wireguard_port}" end end diff --git a/apps/fz_http/lib/fz_http/devices/device.ex b/apps/fz_http/lib/fz_http/devices/device.ex index 2faa185b2..fb8b6b5ae 100644 --- a/apps/fz_http/lib/fz_http/devices/device.ex +++ b/apps/fz_http/lib/fz_http/devices/device.ex @@ -10,7 +10,6 @@ defmodule FzHttp.Devices.Device do import FzHttp.Validators.Common, only: [ trim: 2, - validate_fqdn_or_ip: 2, validate_omitted: 2, validate_no_duplicates: 2, validate_no_mask: 2, @@ -138,7 +137,6 @@ defmodule FzHttp.Devices.Device do ]) |> validate_list_of_ips_or_cidrs(:allowed_ips) |> validate_no_duplicates(:dns) - |> validate_fqdn_or_ip(:endpoint) |> validate_number(:persistent_keepalive, greater_than_or_equal_to: 0, less_than_or_equal_to: 120 diff --git a/apps/fz_http/lib/fz_http/sites/site.ex b/apps/fz_http/lib/fz_http/sites/site.ex index 2b999fa6a..5cfbf9d6c 100644 --- a/apps/fz_http/lib/fz_http/sites/site.ex +++ b/apps/fz_http/lib/fz_http/sites/site.ex @@ -9,7 +9,6 @@ defmodule FzHttp.Sites.Site do import FzHttp.Validators.Common, only: [ trim: 2, - validate_fqdn_or_ip: 2, validate_list_of_ips_or_cidrs: 2, validate_no_duplicates: 2 ] @@ -58,7 +57,6 @@ defmodule FzHttp.Sites.Site do |> validate_no_duplicates(:dns) |> validate_list_of_ips_or_cidrs(:allowed_ips) |> validate_no_duplicates(:allowed_ips) - |> validate_fqdn_or_ip(:endpoint) |> validate_number(:mtu, greater_than_or_equal_to: @min_mtu, less_than_or_equal_to: @max_mtu) |> validate_number(:persistent_keepalive, greater_than_or_equal_to: @min_persistent_keepalive, diff --git a/apps/fz_http/lib/fz_http/validators/common.ex b/apps/fz_http/lib/fz_http/validators/common.ex index 7f46bb20e..1b8d3340a 100644 --- a/apps/fz_http/lib/fz_http/validators/common.ex +++ b/apps/fz_http/lib/fz_http/validators/common.ex @@ -8,8 +8,6 @@ defmodule FzHttp.Validators.Common do import FzCommon.FzNet, only: [ valid_ip?: 1, - valid_fqdn?: 1, - valid_hostname?: 1, valid_cidr?: 1 ] @@ -58,18 +56,6 @@ defmodule FzHttp.Validators.Common do end) end - def validate_fqdn_or_ip(changeset, field) when is_atom(field) do - validate_change(changeset, field, fn _current_field, value -> - value - |> split_comma_list() - |> Enum.find(&(not (valid_ip?(&1) or valid_fqdn?(&1) or valid_hostname?(&1)))) - |> error_if( - &(!is_nil(&1)), - &{field, "is invalid: #{&1} is not a valid FQDN or IPv4 / IPv6 address"} - ) - end) - end - def validate_list_of_ips(changeset, field) when is_atom(field) do validate_change(changeset, field, fn _current_field, value -> value diff --git a/apps/fz_http/test/fz_http/devices_test.exs b/apps/fz_http/test/fz_http/devices_test.exs index 5df052716..14dc773cd 100644 --- a/apps/fz_http/test/fz_http/devices_test.exs +++ b/apps/fz_http/test/fz_http/devices_test.exs @@ -219,21 +219,6 @@ defmodule FzHttp.DevicesTest do endpoint: "valid-endpoint.example.com" } - @invalid_endpoint_ipv4_attrs %{ - use_site_endpoint: false, - endpoint: "-265.1.1.1" - } - - @invalid_endpoint_ipv6_attrs %{ - use_site_endpoint: false, - endpoint: "deadbeef::1" - } - - @invalid_endpoint_host_attrs %{ - use_site_endpoint: false, - endpoint: "can't have this" - } - @empty_endpoint_attrs %{ use_site_endpoint: false, endpoint: "" @@ -308,15 +293,6 @@ defmodule FzHttp.DevicesTest do assert @valid_endpoint_host_attrs = test_device end - test "prevents updating device with invalid ipv4 endpoint", %{device: device} do - {:error, changeset} = Devices.update_device(device, @invalid_endpoint_ipv4_attrs) - - assert changeset.errors[:endpoint] == { - "is invalid: -265.1.1.1 is not a valid FQDN or IPv4 / IPv6 address", - [] - } - end - test "prevents updating fields if use_site_", %{device: device} do for attrs <- @fields_use_site do field = @@ -338,24 +314,6 @@ defmodule FzHttp.DevicesTest do assert {:ok, _device} = Devices.update_device(device, attrs) end - test "prevents updating device with invalid ipv6 endpoint", %{device: device} do - {:error, changeset} = Devices.update_device(device, @invalid_endpoint_ipv6_attrs) - - assert changeset.errors[:endpoint] == { - "is invalid: deadbeef::1 is not a valid FQDN or IPv4 / IPv6 address", - [] - } - end - - test "prevents updating device with invalid host endpoint", %{device: device} do - {:error, changeset} = Devices.update_device(device, @invalid_endpoint_host_attrs) - - assert changeset.errors[:endpoint] == { - "is invalid: can't have this is not a valid FQDN or IPv4 / IPv6 address", - [] - } - end - test "prevents updating device with empty endpoint", %{device: device} do {:error, changeset} = Devices.update_device(device, @empty_endpoint_attrs) diff --git a/apps/fz_http/test/fz_http_web/live/setting_live/site_test.exs b/apps/fz_http/test/fz_http_web/live/setting_live/site_test.exs index 85df4f089..4863b7e1f 100644 --- a/apps/fz_http/test/fz_http_web/live/setting_live/site_test.exs +++ b/apps/fz_http/test/fz_http_web/live/setting_live/site_test.exs @@ -23,9 +23,6 @@ defmodule FzHttpWeb.SettingLive.SiteTest do @invalid_allowed_ips %{ "site" => %{"allowed_ips" => "foobar"} } - @invalid_endpoint %{ - "site" => %{"endpoint" => "-foobar"} - } @invalid_persistent_keepalive %{ "site" => %{"persistent_keepalive" => "-1"} } @@ -133,19 +130,6 @@ defmodule FzHttpWeb.SettingLive.SiteTest do """ end - test "prevents invalid endpoint", %{view: view} do - test_view = - view - |> element("#site_form_component") - |> render_submit(@invalid_endpoint) - - assert test_view =~ "is invalid" - - assert test_view =~ """ - \ - """ - end - test "prevents invalid persistent_keepalive", %{view: view} do test_view = view