diff --git a/docker-compose.yml b/docker-compose.yml index a09b91063..26a3179ac 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -98,7 +98,6 @@ services: # Feature flags FLOW_ACTIVITIES_ENABLED: "true" MULTI_SITE_RESOURCES_ENABLED: "true" - TRAFFIC_FILTERS_ENABLED: "true" SELF_HOSTED_RELAYS_ENABLED: "true" IDP_SYNC_ENABLED: "true" REST_API_ENABLED: "true" @@ -167,7 +166,6 @@ services: # Feature flags FLOW_ACTIVITIES_ENABLED: "true" MULTI_SITE_RESOURCES_ENABLED: "true" - TRAFFIC_FILTERS_ENABLED: "true" SELF_HOSTED_RELAYS_ENABLED: "true" IDP_SYNC_ENABLED: "true" REST_API_ENABLED: "true" @@ -230,7 +228,6 @@ services: # Feature flags FLOW_ACTIVITIES_ENABLED: "true" MULTI_SITE_RESOURCES_ENABLED: "true" - TRAFFIC_FILTERS_ENABLED: "true" SELF_HOSTED_RELAYS_ENABLED: "true" IDP_SYNC_ENABLED: "true" REST_API_ENABLED: "true" @@ -299,7 +296,6 @@ services: # Feature flags FLOW_ACTIVITIES_ENABLED: "true" MULTI_SITE_RESOURCES_ENABLED: "true" - TRAFFIC_FILTERS_ENABLED: "true" SELF_HOSTED_RELAYS_ENABLED: "true" IDP_SYNC_ENABLED: "true" REST_API_ENABLED: "true" diff --git a/elixir/apps/domain/lib/domain/config/definitions.ex b/elixir/apps/domain/lib/domain/config/definitions.ex index df86dfc08..761ae4da1 100644 --- a/elixir/apps/domain/lib/domain/config/definitions.ex +++ b/elixir/apps/domain/lib/domain/config/definitions.ex @@ -635,11 +635,6 @@ defmodule Domain.Config.Definitions do """ defconfig(:feature_flow_activities_enabled, :boolean, default: false) - @doc """ - Boolean flag to turn Resource traffic filters on/off for all accounts. - """ - defconfig(:feature_traffic_filters_enabled, :boolean, default: false) - @doc """ Boolean flag to turn Account relays admin functionality on/off for all accounts. """ diff --git a/elixir/apps/domain/lib/domain/resources/resource.ex b/elixir/apps/domain/lib/domain/resources/resource.ex index 6e2f74828..41d9b49c5 100644 --- a/elixir/apps/domain/lib/domain/resources/resource.ex +++ b/elixir/apps/domain/lib/domain/resources/resource.ex @@ -9,7 +9,7 @@ defmodule Domain.Resources.Resource do field :type, Ecto.Enum, values: [:cidr, :ip, :dns] embeds_many :filters, Filter, on_replace: :delete, primary_key: false do - field :protocol, Ecto.Enum, values: [tcp: 6, udp: 17, icmp: 1, all: -1] + field :protocol, Ecto.Enum, values: [tcp: 6, udp: 17, icmp: 1] field :ports, {:array, Domain.Types.Int4Range}, default: [] end diff --git a/elixir/apps/domain/priv/repo/migrations/20240503205508_reset_traffic_filters.exs b/elixir/apps/domain/priv/repo/migrations/20240503205508_reset_traffic_filters.exs new file mode 100644 index 000000000..e7edc4f18 --- /dev/null +++ b/elixir/apps/domain/priv/repo/migrations/20240503205508_reset_traffic_filters.exs @@ -0,0 +1,11 @@ +defmodule Domain.Repo.Migrations.ResetTrafficFilters do + use Ecto.Migration + + def change do + execute(""" + UPDATE resources + SET filters = '[]' + WHERE filters = '[{"ports":[],"protocol":"all"}]' + """) + end +end diff --git a/elixir/apps/domain/priv/repo/seeds.exs b/elixir/apps/domain/priv/repo/seeds.exs index 107e13420..0d5c46650 100644 --- a/elixir/apps/domain/priv/repo/seeds.exs +++ b/elixir/apps/domain/priv/repo/seeds.exs @@ -628,7 +628,7 @@ IO.puts("") address: "google.com", address_description: "https://google.com/", connections: [%{gateway_group_id: gateway_group.id}], - filters: [%{protocol: :all}] + filters: [] }, admin_subject ) @@ -641,7 +641,7 @@ IO.puts("") address: "*.firez.one", address_description: "https://firez.one/", connections: [%{gateway_group_id: gateway_group.id}], - filters: [%{protocol: :all}] + filters: [] }, admin_subject ) @@ -654,7 +654,7 @@ IO.puts("") address: "?.firezone.dev", address_description: "https://firezone.dev/", connections: [%{gateway_group_id: gateway_group.id}], - filters: [%{protocol: :all}] + filters: [] }, admin_subject ) @@ -667,7 +667,7 @@ IO.puts("") address: "example.com", address_description: "https://example.com:1234/", connections: [%{gateway_group_id: gateway_group.id}], - filters: [%{protocol: :all}] + filters: [] }, admin_subject ) @@ -680,7 +680,7 @@ IO.puts("") address: "ip6only.me", address_description: "https://ip6only.me/", connections: [%{gateway_group_id: gateway_group.id}], - filters: [%{protocol: :all}] + filters: [] }, admin_subject ) @@ -727,7 +727,7 @@ IO.puts("") address: "172.20.0.1/16", address_description: "172.20.0.1/16", connections: [%{gateway_group_id: gateway_group.id}], - filters: [%{protocol: :all}] + filters: [] }, admin_subject ) diff --git a/elixir/apps/web/lib/web/components/core_components.ex b/elixir/apps/web/lib/web/components/core_components.ex index f33824adc..1a6694260 100644 --- a/elixir/apps/web/lib/web/components/core_components.ex +++ b/elixir/apps/web/lib/web/components/core_components.ex @@ -343,10 +343,18 @@ defmodule Web.CoreComponents do """ attr :rest, :global slot :inner_block, required: true + attr :inline, :boolean, default: false def error(assigns) do ~H""" -

+

<.icon name="hero-exclamation-circle-mini" class="mt-0.5 h-5 w-5 flex-none" /> <%= render_slot(@inner_block) %>

@@ -619,7 +627,7 @@ defmodule Web.CoreComponents do ~H""" <%= @label %> - <.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %> + <.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}> + <%= msg %> + """ end @@ -167,7 +174,9 @@ defmodule Web.FormComponents do <% end %> <% end %> - <.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %> + <.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}> + <%= msg %> + """ end @@ -197,7 +206,9 @@ defmodule Web.FormComponents do <%= Phoenix.HTML.Form.options_for_select(@options, @value) %> - <.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %> + <.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}> + <%= msg %> + """ end @@ -219,7 +230,9 @@ defmodule Web.FormComponents do ]} {@rest} ><%= Phoenix.HTML.Form.normalize_value("textarea", @value) %> - <.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %> + <.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}> + <%= msg %> + """ end @@ -265,7 +278,9 @@ defmodule Web.FormComponents do <.icon name="hero-plus" /> Add - <.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %> + <.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}> + <%= msg %> + """ end @@ -297,14 +312,16 @@ defmodule Web.FormComponents do {@rest} /> - <.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %> + <.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}> + <%= msg %> + """ end def input(%{type: "text", prefix: prefix} = assigns) when not is_nil(prefix) do ~H""" -
+
<.label :if={not is_nil(@label)} for={@id}><%= @label %>
- <.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %> + <.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}> + <%= msg %> +
""" end def input(assigns) do ~H""" -
+
<.label :if={not is_nil(@label)} for={@id}><%= @label %> - <.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %> + <.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}> + <%= msg %> +
""" end diff --git a/elixir/apps/web/lib/web/live/resources/components.ex b/elixir/apps/web/lib/web/live/resources/components.ex index d79d73c86..93a6d8b90 100644 --- a/elixir/apps/web/lib/web/live/resources/components.ex +++ b/elixir/apps/web/lib/web/live/resources/components.ex @@ -6,7 +6,7 @@ defmodule Web.Resources.Components do if Domain.Accounts.traffic_filters_enabled?(account) do attrs else - Map.put(attrs, "filters", %{"all" => %{"enabled" => "true", "protocol" => "all"}}) + Map.put(attrs, "filters", %{}) end Map.update(attrs, "filters", [], fn filters -> @@ -21,11 +21,7 @@ defmodule Web.Resources.Components do }} end - if Map.has_key?(filters, "all") do - %{"all" => %{"protocol" => "all"}} - else - filters - end + filters end) end @@ -38,6 +34,7 @@ defmodule Web.Resources.Components do end attr :form, :any, required: true + attr :account, :any, required: true def filters_form(assigns) do # Code is taken from https://github.com/phoenixframework/phoenix_live_view/blob/v0.19.5/lib/phoenix_component.ex#L2356 @@ -57,103 +54,113 @@ defmodule Web.Resources.Components do {id, new_form} end - assigns = Map.put(assigns, :forms_by_protocol, forms_by_protocol) + assigns = + assigns + |> Map.put(:forms_by_protocol, forms_by_protocol) + |> Map.put( + :traffic_filters_enabled?, + Domain.Accounts.traffic_filters_enabled?(assigns.account) + ) ~H"""
- Traffic Restriction +
+ Traffic Restriction -
- <.input type="hidden" name={"#{@form.name}[all][protocol]"} value="all" /> - -
-
- <.input - type="checkbox" - field={@forms_by_protocol[:all]} - name={"#{@form.name}[all][enabled]"} - checked={Map.has_key?(@forms_by_protocol, :all)} - value="true" - label="Permit All" - /> -
-
+ <%= if @traffic_filters_enabled? == false do %> + <.link navigate={~p"/#{@account}/settings/billing"} class="text-sm text-primary-500"> + <.badge type="primary" title="Feature available on a higher pricing plan"> + <.icon name="hero-lock-closed" class="w-3.5 h-3.5 mr-1" /> UPGRADE TO UNLOCK + + + <% end %>
-
- <.input type="hidden" name={"#{@form.name}[icmp][protocol]"} value="icmp" /> +

+ Restrict access to the specified protocols and ports. By default, all + protocols and ports are accessible. +

-
-
- <.input - type="checkbox" - field={@forms_by_protocol[:icmp]} - name={"#{@form.name}[icmp][enabled]"} - checked={Map.has_key?(@forms_by_protocol, :icmp)} - value="true" - disabled={Map.has_key?(@forms_by_protocol, :all)} - label="ICMP" - /> +
+
+ <.input type="hidden" name={"#{@form.name}[icmp][protocol]"} value="icmp" /> + +
+
+ <.input + title="Allow ICMP traffic" + type="checkbox" + field={@forms_by_protocol[:icmp]} + name={"#{@form.name}[icmp][enabled]"} + checked={Map.has_key?(@forms_by_protocol, :icmp)} + value="true" + disabled={!@traffic_filters_enabled?} + label="ICMP" + /> +
-
-
- <.input type="hidden" name={"#{@form.name}[tcp][protocol]"} value="tcp" /> +
+ <.input type="hidden" name={"#{@form.name}[tcp][protocol]"} value="tcp" /> -
-
- <.input - type="checkbox" - field={@forms_by_protocol[:tcp]} - name={"#{@form.name}[tcp][enabled]"} - checked={Map.has_key?(@forms_by_protocol, :tcp)} - value="true" - disabled={Map.has_key?(@forms_by_protocol, :all)} - label="TCP" - /> -
+
+
+ <.input + title="Restrict traffic to TCP traffic" + type="checkbox" + field={@forms_by_protocol[:tcp]} + name={"#{@form.name}[tcp][enabled]"} + checked={Map.has_key?(@forms_by_protocol, :tcp)} + value="true" + disabled={!@traffic_filters_enabled?} + label="TCP" + /> +
-
- <% ports = (@forms_by_protocol[:tcp] || %{ports: %{value: []}})[:ports] %> - <.input - type="text" - field={ports} - name={"#{@form.name}[tcp][ports]"} - value={pretty_print_ports(ports.value)} - disabled={Map.has_key?(@forms_by_protocol, :all)} - placeholder="Enter comma-separated port range(s), eg. 433, 80, 90-99. Matches all ports if empty." - /> +
+ <% ports = (@forms_by_protocol[:tcp] || %{ports: %{value: []}})[:ports] %> + <.input + type="text" + inline_errors={true} + field={ports} + name={"#{@form.name}[tcp][ports]"} + value={Enum.any?(ports.value) && pretty_print_ports(ports.value)} + disabled={!@traffic_filters_enabled? || !Map.has_key?(@forms_by_protocol, :tcp)} + placeholder="Comma-separated port range(s), eg. 433, 80, 90-99. Matches all ports if empty." + /> +
-
-
- <.input type="hidden" name={"#{@form.name}[udp][protocol]"} value="udp" /> +
+ <.input type="hidden" name={"#{@form.name}[udp][protocol]"} value="udp" /> -
-
- <.input - type="checkbox" - field={@forms_by_protocol[:udp]} - name={"#{@form.name}[udp][enabled]"} - checked={Map.has_key?(@forms_by_protocol, :udp)} - value="true" - disabled={Map.has_key?(@forms_by_protocol, :all)} - label="UDP" - /> -
+
+
+ <.input + type="checkbox" + field={@forms_by_protocol[:udp]} + name={"#{@form.name}[udp][enabled]"} + checked={Map.has_key?(@forms_by_protocol, :udp)} + value="true" + disabled={!@traffic_filters_enabled?} + label="UDP" + /> +
-
- <% ports = (@forms_by_protocol[:udp] || %{ports: %{value: []}})[:ports] %> - <.input - type="text" - field={ports} - name={"#{@form.name}[udp][ports]"} - value={pretty_print_ports(ports.value)} - disabled={Map.has_key?(@forms_by_protocol, :all)} - placeholder="Enter comma-separated port range(s), eg. 433, 80, 90-99. Matches all ports if empty." - /> +
+ <% ports = (@forms_by_protocol[:udp] || %{ports: %{value: []}})[:ports] %> + <.input + type="text" + inline_errors={true} + field={ports} + name={"#{@form.name}[udp][ports]"} + value={Enum.any?(ports.value) && pretty_print_ports(ports.value)} + disabled={!@traffic_filters_enabled? || !Map.has_key?(@forms_by_protocol, :udp)} + placeholder="Comma-separated port range(s), eg. 433, 80, 90-99. Matches all ports if empty." + /> +
@@ -161,15 +168,6 @@ defmodule Web.Resources.Components do """ end - attr :form, :any, required: true - - def beta_filters_form(assigns) do - ~H""" - <.input type="hidden" name={"#{@form.name}[all][protocol]"} value="all" /> - <.input type="hidden" name={"#{@form.name}[all][enabled]"} value="true" /> - """ - end - attr :filter, :any, required: true def filter_description(assigns) do @@ -178,9 +176,6 @@ defmodule Web.Resources.Components do """ end - defp pretty_print_filter(%{protocol: :all}), - do: "All Traffic Allowed" - defp pretty_print_filter(%{protocol: :icmp}), do: "ICMP: Allowed" @@ -190,7 +185,7 @@ defmodule Web.Resources.Components do defp pretty_print_filter(%{protocol: :udp, ports: ports}), do: "UDP: #{pretty_print_ports(ports)}" - defp pretty_print_ports([]), do: "any port" + defp pretty_print_ports([]), do: "All ports allowed" defp pretty_print_ports(ports), do: Enum.join(ports, ", ") def map_connections_form_attrs(attrs) do diff --git a/elixir/apps/web/lib/web/live/resources/edit.ex b/elixir/apps/web/lib/web/live/resources/edit.ex index 13cf98101..9fb3b7351 100644 --- a/elixir/apps/web/lib/web/live/resources/edit.ex +++ b/elixir/apps/web/lib/web/live/resources/edit.ex @@ -21,7 +21,6 @@ defmodule Web.Resources.Edit do gateway_groups: gateway_groups, form: form, params: Map.take(params, ["site_id"]), - traffic_filters_enabled?: Accounts.traffic_filters_enabled?(socket.assigns.account), page_title: "Edit #{resource.name}" ) @@ -72,12 +71,12 @@ defmodule Web.Resources.Edit do

- <.filters_form :if={@traffic_filters_enabled?} form={@form[:filters]} /> + <.filters_form account={@account} form={@form[:filters]} /> <.connections_form :if={is_nil(@params["site_id"])} id="connections_form" - multiple={Domain.Accounts.multi_site_resources_enabled?(@account)} + multiple={Accounts.multi_site_resources_enabled?(@account)} form={@form[:connections]} account={@account} resource={@resource} diff --git a/elixir/apps/web/lib/web/live/resources/new.ex b/elixir/apps/web/lib/web/live/resources/new.ex index efd30564c..dd912108d 100644 --- a/elixir/apps/web/lib/web/live/resources/new.ex +++ b/elixir/apps/web/lib/web/live/resources/new.ex @@ -15,7 +15,6 @@ defmodule Web.Resources.New do name_changed?: false, form: to_form(changeset), params: Map.take(params, ["site_id"]), - traffic_filters_enabled?: Accounts.traffic_filters_enabled?(socket.assigns.account), page_title: "New Resource" ) @@ -130,11 +129,11 @@ defmodule Web.Resources.New do required /> - <.filters_form :if={@traffic_filters_enabled?} form={@form[:filters]} /> + <.filters_form account={@account} form={@form[:filters]} /> <.connections_form :if={is_nil(@params["site_id"])} - multiple={Domain.Accounts.multi_site_resources_enabled?(@account)} + multiple={Accounts.multi_site_resources_enabled?(@account)} form={@form[:connections]} account={@account} gateway_groups={@gateway_groups} diff --git a/elixir/apps/web/lib/web/live/resources/show.ex b/elixir/apps/web/lib/web/live/resources/show.ex index 7f1cb0a63..0632cba0e 100644 --- a/elixir/apps/web/lib/web/live/resources/show.ex +++ b/elixir/apps/web/lib/web/live/resources/show.ex @@ -19,7 +19,6 @@ defmodule Web.Resources.Show do assign( socket, page_title: "Resource #{resource.name}", - traffic_filters_enabled?: Accounts.traffic_filters_enabled?(socket.assigns.account), flow_activities_enabled?: Accounts.flow_activities_enabled?(socket.assigns.account), resource: resource, actor_groups_peek: Map.fetch!(actor_groups_peek, resource.id), @@ -96,10 +95,7 @@ defmodule Web.Resources.Show do (deleted) <:action :if={is_nil(@resource.deleted_at)}> - <.edit_button - :if={Domain.Accounts.multi_site_resources_enabled?(@account)} - navigate={~p"/#{@account}/resources/#{@resource.id}/edit?#{@params}"} - > + <.edit_button navigate={~p"/#{@account}/resources/#{@resource.id}/edit?#{@params}"}> Edit Resource @@ -162,13 +158,13 @@ defmodule Web.Resources.Show do - <.vertical_table_row :if={@traffic_filters_enabled?}> + <.vertical_table_row> <:label> - Traffic Filtering Rules + Traffic restriction <:value>
- No traffic filtering rules + All traffic allowed
<.filter_description filter={filter} /> diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/new.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/new.ex index 766472457..6f0210514 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/new.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/new.ex @@ -136,7 +136,7 @@ defmodule Web.Settings.IdentityProviders.New do <%= if @opts[:enabled] == false do %> <.link navigate={~p"/#{@account}/settings/billing"} class="ml-2 text-sm text-primary-500"> <.badge class="ml-2" type="primary" title="Feature available on a higher pricing plan"> - UPGRADE TO UNLOCK + <.icon name="hero-lock-closed" class="w-3.5 h-3.5 mr-1" /> UPGRADE TO UNLOCK <% end %> diff --git a/elixir/apps/web/test/web/live/resources/edit_test.exs b/elixir/apps/web/test/web/live/resources/edit_test.exs index 6b4c01a48..fe64350fb 100644 --- a/elixir/apps/web/test/web/live/resources/edit_test.exs +++ b/elixir/apps/web/test/web/live/resources/edit_test.exs @@ -101,8 +101,6 @@ defmodule Web.Live.Resources.EditTest do (connection_inputs ++ [ "resource[address_description]", - "resource[filters][all][enabled]", - "resource[filters][all][protocol]", "resource[filters][icmp][enabled]", "resource[filters][icmp][protocol]", "resource[filters][tcp][enabled]", @@ -134,8 +132,6 @@ defmodule Web.Live.Resources.EditTest do assert find_inputs(form) == [ "resource[address_description]", - "resource[filters][all][enabled]", - "resource[filters][all][protocol]", "resource[filters][icmp][enabled]", "resource[filters][icmp][protocol]", "resource[filters][tcp][enabled]", @@ -273,39 +269,7 @@ defmodule Web.Live.Resources.EditTest do {:live_redirect, %{to: ~p"/#{account}/sites/#{group}?#resources", kind: :push}}} end - test "disables all filters on a resource when 'Permit All' filter is selected", %{ - account: account, - identity: identity, - resource: resource, - conn: conn - } do - attrs = %{ - filters: %{ - all: %{enabled: true} - } - } - - {:ok, lv, _html} = - conn - |> authorize_conn(identity) - |> live(~p"/#{account}/resources/#{resource}/edit") - - assert lv - |> form("form", resource: attrs) - |> render_submit() == - {:error, {:live_redirect, %{to: ~p"/#{account}/resources/#{resource}", kind: :push}}} - - assert saved_resource = Repo.get_by(Domain.Resources.Resource, id: resource.id) - - saved_filters = - for filter <- saved_resource.filters, into: %{} do - {filter.protocol, %{ports: Enum.join(filter.ports, ", ")}} - end - - assert saved_filters == %{all: %{ports: ""}} - end - - test "does not render traffic filter form when traffic filters disabled", %{ + test "shows disabled traffic filter form when traffic filters disabled", %{ account: account, group: group, identity: identity, @@ -314,7 +278,7 @@ defmodule Web.Live.Resources.EditTest do } do Domain.Config.feature_flag_override(:traffic_filters, false) - {:ok, lv, _html} = + {:ok, lv, html} = conn |> authorize_conn(identity) |> live(~p"/#{account}/resources/#{resource}/edit?site_id=#{group}") @@ -323,8 +287,18 @@ defmodule Web.Live.Resources.EditTest do assert find_inputs(form) == [ "resource[address_description]", + "resource[filters][icmp][enabled]", + "resource[filters][icmp][protocol]", + "resource[filters][tcp][enabled]", + "resource[filters][tcp][ports]", + "resource[filters][tcp][protocol]", + "resource[filters][udp][enabled]", + "resource[filters][udp][ports]", + "resource[filters][udp][protocol]", "resource[name]" ] + + assert html =~ "UPGRADE TO UNLOCK" end test "updates a resource on valid attrs when traffic filters disabled", %{ @@ -354,8 +328,31 @@ defmodule Web.Live.Resources.EditTest do assert saved_resource = Repo.get_by(Domain.Resources.Resource, id: resource.id) assert saved_resource.name == attrs.name - assert saved_resource.filters == [ - %Domain.Resources.Resource.Filter{protocol: :all, ports: []} - ] + assert saved_resource.filters == [] + end + + test "disables traffic filters form fields when traffic filters disabled", %{ + account: account, + group: group, + identity: identity, + resource: resource, + conn: conn + } do + Domain.Config.feature_flag_override(:traffic_filters, false) + + {:ok, lv, _html} = + conn + |> authorize_conn(identity) + |> live(~p"/#{account}/resources/#{resource}/edit?site_id=#{group}") + + assert has_element?(lv, "input[name='resource[filters][icmp][enabled]'][disabled]") + assert has_element?(lv, "input[name='resource[filters][icmp][enabled]'][disabled]") + assert has_element?(lv, "input[name='resource[filters][tcp][enabled]'][disabled]") + assert has_element?(lv, "input[name='resource[filters][tcp][ports]'][disabled]") + assert has_element?(lv, "input[name='resource[filters][udp][enabled]'][disabled]") + assert has_element?(lv, "input[name='resource[filters][udp][ports]'][disabled]") + assert saved_resource = Repo.get_by(Domain.Resources.Resource, id: resource.id) + assert saved_resource.name == resource.name + assert saved_resource.filters == resource.filters end end diff --git a/elixir/apps/web/test/web/live/resources/new_test.exs b/elixir/apps/web/test/web/live/resources/new_test.exs index eadd6faee..366ede235 100644 --- a/elixir/apps/web/test/web/live/resources/new_test.exs +++ b/elixir/apps/web/test/web/live/resources/new_test.exs @@ -76,6 +76,14 @@ defmodule Web.Live.Resources.NewTest do expected_inputs = (connection_inputs ++ [ + "resource[filters][icmp][enabled]", + "resource[filters][icmp][protocol]", + "resource[filters][tcp][enabled]", + "resource[filters][tcp][protocol]", + "resource[filters][tcp][ports]", + "resource[filters][udp][enabled]", + "resource[filters][udp][protocol]", + "resource[filters][udp][ports]", "resource[address]", "resource[address_description]", "resource[name]", @@ -117,16 +125,14 @@ defmodule Web.Live.Resources.NewTest do [ "resource[address]", "resource[address_description]", - "resource[filters][all][enabled]", - "resource[filters][all][protocol]", "resource[filters][icmp][enabled]", "resource[filters][icmp][protocol]", "resource[filters][tcp][enabled]", - "resource[filters][tcp][ports]", "resource[filters][tcp][protocol]", + "resource[filters][tcp][ports]", "resource[filters][udp][enabled]", - "resource[filters][udp][ports]", "resource[filters][udp][protocol]", + "resource[filters][udp][ports]", "resource[name]", "resource[type]" ]) @@ -162,6 +168,14 @@ defmodule Web.Live.Resources.NewTest do expected_inputs = (connection_inputs ++ [ + "resource[filters][icmp][enabled]", + "resource[filters][icmp][protocol]", + "resource[filters][tcp][enabled]", + "resource[filters][tcp][protocol]", + "resource[filters][tcp][ports]", + "resource[filters][udp][enabled]", + "resource[filters][udp][protocol]", + "resource[filters][udp][ports]", "resource[address]", "resource[address_description]", "resource[name]", @@ -188,8 +202,6 @@ defmodule Web.Live.Resources.NewTest do assert find_inputs(form) == [ "resource[address]", "resource[address_description]", - "resource[filters][all][enabled]", - "resource[filters][all][protocol]", "resource[filters][icmp][enabled]", "resource[filters][icmp][protocol]", "resource[filters][tcp][enabled]", @@ -224,7 +236,10 @@ defmodule Web.Live.Resources.NewTest do lv |> form("form") - |> render_change(resource: %{type: :dns}) + # Generate onchange to trigger visible elements, otherwise the form won't be valid + |> render_change( + resource: %{type: :dns, filters: %{tcp: %{enabled: true}, udp: %{enabled: true}}} + ) lv |> form("form", resource: attrs) @@ -261,7 +276,10 @@ defmodule Web.Live.Resources.NewTest do lv |> form("form") - |> render_change(resource: %{type: :dns}) + # Generate onchange to trigger visible elements, otherwise the form won't be valid + |> render_change( + resource: %{type: :dns, filters: %{tcp: %{enabled: true}, udp: %{enabled: true}}} + ) assert lv |> form("form", resource: attrs) @@ -298,7 +316,10 @@ defmodule Web.Live.Resources.NewTest do lv |> form("form") - |> render_change(resource: %{type: :dns}) + # Generate onchange to trigger visible elements, otherwise the form won't be valid + |> render_change( + resource: %{type: :dns, filters: %{tcp: %{enabled: true}, udp: %{enabled: true}}} + ) assert lv |> form("form", resource: attrs) @@ -330,6 +351,7 @@ defmodule Web.Live.Resources.NewTest do lv |> form("form") + # Generate onchange to trigger visible elements, otherwise the form won't be valid |> render_change(resource: %{type: :dns}) assert lv @@ -354,8 +376,8 @@ defmodule Web.Live.Resources.NewTest do address_description: "http://foobar.com:3000/", filters: %{ icmp: %{enabled: true}, - tcp: %{ports: "80, 443"}, - udp: %{ports: "4000 - 5000"} + tcp: %{ports: "80, 443", enabled: true}, + udp: %{ports: "4000 - 5000", enabled: true} }, connections: %{gateway_group.id => %{enabled: true}} } @@ -367,7 +389,13 @@ defmodule Web.Live.Resources.NewTest do lv |> form("form") - |> render_change(resource: %{type: :dns}) + # Generate onchange to trigger visible elements, otherwise the form won't be valid + |> render_change( + resource: %{ + type: :dns, + filters: %{tcp: %{enabled: true}, udp: %{enabled: true}, icmp: %{enabled: true}} + } + ) lv |> form("form", resource: attrs) @@ -390,8 +418,8 @@ defmodule Web.Live.Resources.NewTest do address_description: "http://foobar.com:3000/", filters: %{ icmp: %{enabled: true}, - tcp: %{ports: "80, 443"}, - udp: %{ports: "4000 - 5000"} + tcp: %{ports: "80, 443", enabled: true}, + udp: %{ports: "4000 - 5000", enabled: true} } } @@ -402,7 +430,13 @@ defmodule Web.Live.Resources.NewTest do lv |> form("form") - |> render_change(resource: %{type: :dns}) + # Generate onchange to trigger visible elements, otherwise the form won't be valid + |> render_change( + resource: %{ + type: :dns, + filters: %{tcp: %{enabled: true}, udp: %{enabled: true}, icmp: %{enabled: true}} + } + ) lv |> form("form", resource: attrs) @@ -413,7 +447,7 @@ defmodule Web.Live.Resources.NewTest do assert assert_redirect(lv, ~p"/#{account}/resources/#{resource}?site_id=#{group.id}") end - test "does not render traffic filter form", %{ + test "shows disabled traffic filter form when traffic filters disabled", %{ account: account, group: group, identity: identity, @@ -421,7 +455,7 @@ defmodule Web.Live.Resources.NewTest do } do Domain.Config.feature_flag_override(:traffic_filters, false) - {:ok, lv, _html} = + {:ok, lv, html} = conn |> authorize_conn(identity) |> live(~p"/#{account}/resources/new?site_id=#{group}") @@ -431,9 +465,19 @@ defmodule Web.Live.Resources.NewTest do assert find_inputs(form) == [ "resource[address]", "resource[address_description]", + "resource[filters][icmp][enabled]", + "resource[filters][icmp][protocol]", + "resource[filters][tcp][enabled]", + "resource[filters][tcp][ports]", + "resource[filters][tcp][protocol]", + "resource[filters][udp][enabled]", + "resource[filters][udp][ports]", + "resource[filters][udp][protocol]", "resource[name]", "resource[type]" ] + + assert html =~ "UPGRADE TO UNLOCK" end test "creates a resource on valid attrs when traffic filter form disabled", %{ @@ -455,6 +499,7 @@ defmodule Web.Live.Resources.NewTest do lv |> form("form") + # Generate onchange to trigger visible elements, otherwise the form won't be valid |> render_change(resource: %{type: :dns}) lv @@ -467,4 +512,36 @@ defmodule Web.Live.Resources.NewTest do assert assert_redirect(lv, ~p"/#{account}/resources/#{resource}?site_id=#{group.id}") end + + test "prevents saving resource if traffic filters set when traffic filters disabled", %{ + account: account, + group: group, + identity: identity, + conn: conn + } do + Domain.Config.feature_flag_override(:traffic_filters, false) + + attrs = %{ + name: "foobar.com", + filters: %{ + icmp: %{enabled: true}, + tcp: %{ports: "8080, 4443", enabled: true}, + udp: %{ports: "4000 - 5000", enabled: true} + } + } + + {:ok, lv, _html} = + conn + |> authorize_conn(identity) + |> live(~p"/#{account}/resources/new?site_id=#{group}") + + # ** (ArgumentError) could not find non-disabled input, select or textarea with name "resource[filters][tcp][ports]" within: + assert_raise ArgumentError, fn -> + lv + |> form("form", resource: attrs) + |> render_submit() + end + + assert Repo.all(Domain.Resources.Resource) == [] + end end diff --git a/elixir/apps/web/test/web/live/resources/show_test.exs b/elixir/apps/web/test/web/live/resources/show_test.exs index ef3672712..576b6d62c 100644 --- a/elixir/apps/web/test/web/live/resources/show_test.exs +++ b/elixir/apps/web/test/web/live/resources/show_test.exs @@ -97,22 +97,6 @@ defmodule Web.Live.Resources.ShowTest do {:live_redirect, %{to: ~p"/#{account}/resources/#{resource}/edit", kind: :push}}} end - test "hides edit resource button when feature is disabled", %{ - account: account, - resource: resource, - identity: identity, - conn: conn - } do - Domain.Config.feature_flag_override(:multi_site_resources, false) - - {:ok, lv, _html} = - conn - |> authorize_conn(identity) - |> live(~p"/#{account}/resources/#{resource}") - - refute has_element?(lv, "a", "Edit Resource") - end - test "renders resource details", %{ account: account, actor: actor, @@ -136,10 +120,52 @@ defmodule Web.Live.Resources.ShowTest do assert table["created"] =~ actor.name for filter <- resource.filters do - assert String.downcase(table["traffic filtering rules"]) =~ Atom.to_string(filter.protocol) + assert String.downcase(table["traffic restriction"]) =~ Atom.to_string(filter.protocol) end end + test "renders traffic filters on show page even when traffic filters disabled", %{ + account: account, + identity: identity, + conn: conn + } do + Domain.Config.feature_flag_override(:traffic_filters, false) + + resource = Fixtures.Resources.create_resource(account: account, filters: []) + + {:ok, lv, _html} = + conn + |> authorize_conn(identity) + |> live(~p"/#{account}/resources/#{resource}") + + table = + lv + |> element("#resource") + |> render() + |> vertical_table_to_map() + + assert table["traffic restriction"] == "All traffic allowed" + + resource = + Fixtures.Resources.create_resource( + account: account, + filters: [%{protocol: :tcp, ports: []}] + ) + + {:ok, lv, _html} = + conn + |> authorize_conn(identity) + |> live(~p"/#{account}/resources/#{resource}") + + table = + lv + |> element("#resource") + |> render() + |> vertical_table_to_map() + + assert table["traffic restriction"] == "TCP: All ports allowed" + end + test "renders policies table", %{ account: account, identity: identity, diff --git a/elixir/config/runtime.exs b/elixir/config/runtime.exs index 7402331b8..b4a9393a8 100644 --- a/elixir/config/runtime.exs +++ b/elixir/config/runtime.exs @@ -55,7 +55,6 @@ if config_env() == :prod do config :domain, :enabled_features, idp_sync: compile_config!(:feature_idp_sync_enabled), - traffic_filters: compile_config!(:feature_traffic_filters_enabled), sign_up: compile_config!(:feature_sign_up_enabled), flow_activities: compile_config!(:feature_flow_activities_enabled), self_hosted_relays: compile_config!(:feature_self_hosted_relays_enabled), diff --git a/terraform/environments/production/portal.tf b/terraform/environments/production/portal.tf index 44088050b..658b37b92 100644 --- a/terraform/environments/production/portal.tf +++ b/terraform/environments/production/portal.tf @@ -334,10 +334,6 @@ locals { name = "FEATURE_FLOW_ACTIVITIES_ENABLED" value = true }, - { - name = "FEATURE_TRAFFIC_FILTERS_ENABLED" - value = true - }, { name = "FEATURE_SELF_HOSTED_RELAYS_ENABLED" value = true diff --git a/terraform/environments/staging/portal.tf b/terraform/environments/staging/portal.tf index 451ea8c0a..749703e01 100644 --- a/terraform/environments/staging/portal.tf +++ b/terraform/environments/staging/portal.tf @@ -317,10 +317,6 @@ locals { name = "FEATURE_FLOW_ACTIVITIES_ENABLED" value = true }, - { - name = "FEATURE_TRAFFIC_FILTERS_ENABLED" - value = true - }, { name = "FEATURE_SELF_HOSTED_RELAYS_ENABLED" value = true