refactor(portal): Remove Permit all and grey out form when traffic filters disabled (#4887)

- Simplify traffic filters: empty means permit all
- Grey out form instead of hiding when traffic filters disabled, fixes
#4816
- Fix port range population when no ports have been entered
- Update tests
- Add migration to migrate existing prod data
- Add "UPGRADE TO UNLOCK" badge
- Add `inline_errors` attr to show inline error messages
- Remove traffic filters feature flag to allow enable/disable by billing
instead

<img width="757" alt="Screenshot 2024-05-03 at 12 43 24 PM"
src="https://github.com/firezone/firezone/assets/167144/9e9277cb-4653-427c-ade3-4e3b9d479411">

<img width="194" alt="Screenshot 2024-05-03 at 2 03 06 PM"
src="https://github.com/firezone/firezone/assets/167144/06e03314-9010-48a0-8504-0ab49173f0a9">

---------

Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
Co-authored-by: Andrew Dryga <andrew@dryga.com>
This commit is contained in:
Jamil
2024-05-07 09:27:00 -07:00
committed by GitHub
parent 7870dcab25
commit 4a6ff03626
18 changed files with 337 additions and 226 deletions

View File

@@ -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"

View File

@@ -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.
"""

View File

@@ -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

View File

@@ -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

View File

@@ -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
)

View File

@@ -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"""
<p class="mt-3 flex gap-3 text-sm leading-6 text-rose-600 phx-no-feedback:hidden" {@rest}>
<p
class={[
"w-full flex gap-3 text-sm leading-6",
"text-rose-600 phx-no-feedback:hidden",
(@inline && "ml-3") || "mt-3"
]}
{@rest}
>
<.icon name="hero-exclamation-circle-mini" class="mt-0.5 h-5 w-5 flex-none" />
<%= render_slot(@inner_block) %>
</p>
@@ -619,7 +627,7 @@ defmodule Web.CoreComponents do
~H"""
<span
class={[
"text-xs mr-2 px-2.5 py-0.5 rounded whitespace-nowrap",
"text-xs px-2.5 py-0.5 rounded whitespace-nowrap",
@colors[@type],
@class
]}

View File

@@ -40,6 +40,11 @@ defmodule Web.FormComponents do
doc: "a form field struct retrieved from the form, for example: @form[:email]"
attr :errors, :list, default: []
attr :inline_errors, :boolean,
default: false,
doc: "whether to display errors inline instead of below the input"
attr :checked, :boolean, doc: "the checked flag for checkbox and radio inputs"
attr :prompt, :string, default: nil, doc: "the prompt for select inputs"
attr :options, :list, doc: "the options to pass to Phoenix.HTML.Form.options_for_select/2"
@@ -128,7 +133,9 @@ defmodule Web.FormComponents do
/>
<%= @label %>
</label>
<.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %></.error>
<.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}>
<%= msg %>
</.error>
</div>
"""
end
@@ -167,7 +174,9 @@ defmodule Web.FormComponents do
<% end %>
<% end %>
</select>
<.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %></.error>
<.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}>
<%= msg %>
</.error>
</div>
"""
end
@@ -197,7 +206,9 @@ defmodule Web.FormComponents do
<option :if={@prompt} value=""><%= @prompt %></option>
<%= Phoenix.HTML.Form.options_for_select(@options, @value) %>
</select>
<.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %></.error>
<.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}>
<%= msg %>
</.error>
</div>
"""
end
@@ -219,7 +230,9 @@ defmodule Web.FormComponents do
]}
{@rest}
><%= Phoenix.HTML.Form.normalize_value("textarea", @value) %></textarea>
<.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %></.error>
<.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}>
<%= msg %>
</.error>
</div>
"""
end
@@ -265,7 +278,9 @@ defmodule Web.FormComponents do
<.icon name="hero-plus" /> Add
</.button>
<.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %></.error>
<.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}>
<%= msg %>
</.error>
</div>
"""
end
@@ -297,14 +312,16 @@ defmodule Web.FormComponents do
{@rest}
/>
<.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %></.error>
<.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}>
<%= msg %>
</.error>
</div>
"""
end
def input(%{type: "text", prefix: prefix} = assigns) when not is_nil(prefix) do
~H"""
<div phx-feedback-for={@name}>
<div phx-feedback-for={@name} class={@inline_errors && "flex flex-row items-center"}>
<.label :if={not is_nil(@label)} for={@id}><%= @label %></.label>
<div class={[
"flex",
@@ -339,14 +356,16 @@ defmodule Web.FormComponents do
{@rest}
/>
</div>
<.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %></.error>
<.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}>
<%= msg %>
</.error>
</div>
"""
end
def input(assigns) do
~H"""
<div phx-feedback-for={@name}>
<div phx-feedback-for={@name} class={@inline_errors && "flex flex-row items-center"}>
<.label :if={not is_nil(@label)} for={@id}><%= @label %></.label>
<input
type={@type}
@@ -362,7 +381,9 @@ defmodule Web.FormComponents do
]}
{@rest}
/>
<.error :for={msg <- @errors} data-validation-error-for={@name}><%= msg %></.error>
<.error :for={msg <- @errors} inline={@inline_errors} data-validation-error-for={@name}>
<%= msg %>
</.error>
</div>
"""
end

View File

@@ -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"""
<fieldset class="flex flex-col gap-2">
<legend class="mb-2">Traffic Restriction</legend>
<div class="mb-1 flex items-center justify-between">
<legend>Traffic Restriction</legend>
<div>
<.input type="hidden" name={"#{@form.name}[all][protocol]"} value="all" />
<div class="items-center flex flex-row h-12">
<div class="flex-none w-32">
<.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"
/>
</div>
</div>
<%= 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
</.badge>
</.link>
<% end %>
</div>
<div>
<.input type="hidden" name={"#{@form.name}[icmp][protocol]"} value="icmp" />
<p class="text-sm text-neutral-500">
Restrict access to the specified protocols and ports. By default, <strong>all</strong>
protocols and ports are accessible.
</p>
<div class="items-center flex flex-row h-12">
<div class="flex-none w-32">
<.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"
/>
<div class={@traffic_filters_enabled? == false && "opacity-50"}>
<div>
<.input type="hidden" name={"#{@form.name}[icmp][protocol]"} value="icmp" />
<div class="items-center flex flex-row h-12">
<div class="flex-none w-32">
<.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"
/>
</div>
</div>
</div>
</div>
<div>
<.input type="hidden" name={"#{@form.name}[tcp][protocol]"} value="tcp" />
<div>
<.input type="hidden" name={"#{@form.name}[tcp][protocol]"} value="tcp" />
<div class="items-center flex flex-row h-12">
<div class="flex-none w-32">
<.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"
/>
</div>
<div class="items-center flex flex-row h-12">
<div class="flex-none w-32">
<.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"
/>
</div>
<div class="flex-grow">
<% 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."
/>
<div class="flex-grow">
<% 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."
/>
</div>
</div>
</div>
</div>
<div>
<.input type="hidden" name={"#{@form.name}[udp][protocol]"} value="udp" />
<div>
<.input type="hidden" name={"#{@form.name}[udp][protocol]"} value="udp" />
<div class="items-center flex flex-row h-12">
<div class="flex-none w-32">
<.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"
/>
</div>
<div class="items-center flex flex-row h-12">
<div class="flex-none w-32">
<.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"
/>
</div>
<div class="flex-grow">
<% 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."
/>
<div class="flex-grow">
<% 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."
/>
</div>
</div>
</div>
</div>
@@ -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

View File

@@ -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
</p>
</div>
<.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}

View File

@@ -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}

View File

@@ -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
<span :if={not is_nil(@resource.deleted_at)} class="text-red-600">(deleted)</span>
</:title>
<: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
</.edit_button>
</:action>
@@ -162,13 +158,13 @@ defmodule Web.Resources.Show do
</span>
</:value>
</.vertical_table_row>
<.vertical_table_row :if={@traffic_filters_enabled?}>
<.vertical_table_row>
<:label>
Traffic Filtering Rules
Traffic restriction
</:label>
<:value>
<div :if={@resource.filters == []} %>
No traffic filtering rules
All traffic allowed
</div>
<div :for={filter <- @resource.filters} :if={@resource.filters != []} %>
<.filter_description filter={filter} />

View File

@@ -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
</.badge>
</.link>
<% end %>

View File

@@ -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

View File

@@ -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

View File

@@ -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,

View File

@@ -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),

View File

@@ -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

View File

@@ -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