From 980246ae3beeeef6ed38abf9bc00325320c5a60f Mon Sep 17 00:00:00 2001 From: Andrew Dryga Date: Fri, 9 Feb 2024 08:36:37 -0600 Subject: [PATCH] feat(portal): Broadcast resource sites (#3466) Adds `client_address` field which should be passed down to clients to nicely render copy-pasteable address. Client address MUST contain full `address` field to prevent users from shooting themselves in the foot by creating broken resources. We also now broadcast a list of connected sites (gateway_groups) to the client. When a `connection_request` response is sent a new field `gateway_group_id` is added, this way the client can know which site it's using, and if a resource is updated and `gateway_group_id` is removed - it should restart the connection. The portal can not make such a decision as it doesn't track active connections. Screenshot 2024-01-31 at 16 26 50 Screenshot 2024-01-31 at 16 27 01 Screenshot 2024-01-31 at 16 27 27 Screenshot 2024-01-31 at 16 27 49 This PR will be reabsed on `main` once #2240 is merged. --- elixir/apps/api/lib/api/client/channel.ex | 20 +++++-- .../api/lib/api/client/views/gateway_group.ex | 15 +++++ .../apps/api/lib/api/client/views/resource.ex | 9 ++- .../apps/api/test/api/client/channel_test.exs | 52 ++++++++++++++-- elixir/apps/domain/lib/domain/config.ex | 9 ++- elixir/apps/domain/lib/domain/resources.ex | 29 +++++---- .../domain/lib/domain/resources/resource.ex | 1 + .../domain/resources/resource/changeset.ex | 7 ++- elixir/apps/domain/lib/domain/validator.ex | 21 +++++++ ...131212533_add_resources_client_address.exs | 13 ++++ elixir/apps/domain/priv/repo/seeds.exs | 25 ++++++++ .../resources/resource/changeset_test.exs | 8 ++- .../domain/test/domain/resources_test.exs | 54 +++++++++++++++-- .../domain/test/support/fixtures/resources.ex | 4 +- .../apps/web/lib/web/live/resources/edit.ex | 13 ++++ elixir/apps/web/lib/web/live/resources/new.ex | 59 ++++++++++++++++++- .../apps/web/lib/web/live/resources/show.ex | 21 +++++++ .../web/test/web/live/resources/edit_test.exs | 7 ++- .../web/test/web/live/resources/new_test.exs | 15 ++++- 19 files changed, 341 insertions(+), 41 deletions(-) create mode 100644 elixir/apps/api/lib/api/client/views/gateway_group.ex create mode 100644 elixir/apps/domain/priv/repo/migrations/20240131212533_add_resources_client_address.exs diff --git a/elixir/apps/api/lib/api/client/channel.ex b/elixir/apps/api/lib/api/client/channel.ex index 7536436d2..d35419aa1 100644 --- a/elixir/apps/api/lib/api/client/channel.ex +++ b/elixir/apps/api/lib/api/client/channel.ex @@ -54,7 +54,12 @@ defmodule API.Client.Channel do # Subscribe for config updates :ok = Config.subscribe_to_events_in_account(socket.assigns.client.account_id) - {:ok, resources} = Resources.list_authorized_resources(socket.assigns.subject) + {:ok, resources} = + Resources.list_authorized_resources(socket.assigns.subject, + preload: [ + :gateway_groups + ] + ) # We subscribe for all resource events but only care about update events, # where resource might be renamed which should be propagated to the UI. @@ -163,7 +168,9 @@ defmodule API.Client.Channel do OpenTelemetry.Tracer.with_span "client.resource_updated", attributes: %{resource_id: resource_id} do - case Resources.fetch_and_authorize_resource_by_id(resource_id, socket.assigns.subject) do + case Resources.fetch_and_authorize_resource_by_id(resource_id, socket.assigns.subject, + preload: [:gateway_groups] + ) do {:ok, resource} -> push(socket, "resource_created_or_updated", Views.Resource.render(resource)) @@ -206,7 +213,9 @@ defmodule API.Client.Channel do } do :ok = Resources.subscribe_to_events_for_resource(resource_id) - case Resources.fetch_and_authorize_resource_by_id(resource_id, socket.assigns.subject) do + case Resources.fetch_and_authorize_resource_by_id(resource_id, socket.assigns.subject, + preload: [:gateway_groups] + ) do {:ok, resource} -> push(socket, "resource_created_or_updated", Views.Resource.render(resource)) @@ -237,7 +246,9 @@ defmodule API.Client.Channel do # and the recreate it right away if there is another allowing access to it. push(socket, "resource_deleted", resource_id) - case Resources.fetch_and_authorize_resource_by_id(resource_id, socket.assigns.subject) do + case Resources.fetch_and_authorize_resource_by_id(resource_id, socket.assigns.subject, + preload: [:gateway_groups] + ) do {:ok, resource} -> push(socket, "resource_created_or_updated", Views.Resource.render(resource)) @@ -315,6 +326,7 @@ defmodule API.Client.Channel do relay_connection_type ), resource_id: resource_id, + gateway_group_id: gateway.group_id, gateway_id: gateway.id, gateway_remote_ip: gateway.last_seen_remote_ip }} diff --git a/elixir/apps/api/lib/api/client/views/gateway_group.ex b/elixir/apps/api/lib/api/client/views/gateway_group.ex new file mode 100644 index 000000000..636738909 --- /dev/null +++ b/elixir/apps/api/lib/api/client/views/gateway_group.ex @@ -0,0 +1,15 @@ +defmodule API.Client.Views.GatewayGroup do + alias Domain.Gateways + + def render_many(gateway_groups) do + Enum.map(gateway_groups, &render/1) + end + + def render(%Gateways.Group{} = gateway_group) do + %{ + id: gateway_group.id, + name: gateway_group.name, + routing: gateway_group.routing + } + end +end diff --git a/elixir/apps/api/lib/api/client/views/resource.ex b/elixir/apps/api/lib/api/client/views/resource.ex index 4eb2febad..4cbf1a5fd 100644 --- a/elixir/apps/api/lib/api/client/views/resource.ex +++ b/elixir/apps/api/lib/api/client/views/resource.ex @@ -1,4 +1,5 @@ defmodule API.Client.Views.Resource do + alias API.Client.Views alias Domain.Resources def render_many(resources) do @@ -14,7 +15,9 @@ defmodule API.Client.Views.Resource do id: resource.id, type: :cidr, address: address, - name: resource.name + address_description: resource.address_description, + name: resource.name, + gateway_groups: Views.GatewayGroup.render_many(resource.gateway_groups) } end @@ -23,7 +26,9 @@ defmodule API.Client.Views.Resource do id: resource.id, type: resource.type, address: resource.address, - name: resource.name + address_description: resource.address_description, + name: resource.name, + gateway_groups: Views.GatewayGroup.render_many(resource.gateway_groups) } end end diff --git a/elixir/apps/api/test/api/client/channel_test.exs b/elixir/apps/api/test/api/client/channel_test.exs index 835291332..197c3dc1d 100644 --- a/elixir/apps/api/test/api/client/channel_test.exs +++ b/elixir/apps/api/test/api/client/channel_test.exs @@ -138,6 +138,7 @@ defmodule API.Client.ChannelTest do test "sends list of resources after join", %{ client: client, + gateway_group: gateway_group, dns_resource: dns_resource, cidr_resource: cidr_resource, ip_resource: ip_resource @@ -149,21 +150,45 @@ defmodule API.Client.ChannelTest do id: dns_resource.id, type: :dns, name: dns_resource.name, - address: dns_resource.address + address: dns_resource.address, + address_description: dns_resource.address_description, + gateway_groups: [ + %{ + id: gateway_group.id, + name: gateway_group.name, + routing: gateway_group.routing + } + ] } in resources assert %{ id: cidr_resource.id, type: :cidr, name: cidr_resource.name, - address: cidr_resource.address + address: cidr_resource.address, + address_description: cidr_resource.address_description, + gateway_groups: [ + %{ + id: gateway_group.id, + name: gateway_group.name, + routing: gateway_group.routing + } + ] } in resources assert %{ id: ip_resource.id, type: :cidr, name: ip_resource.name, - address: "#{ip_resource.address}/32" + address: "#{ip_resource.address}/32", + address_description: ip_resource.address_description, + gateway_groups: [ + %{ + id: gateway_group.id, + name: gateway_group.name, + routing: gateway_group.routing + } + ] } in resources assert interface == %{ @@ -275,6 +300,7 @@ defmodule API.Client.ChannelTest do describe "handle_info/2 :update_resource" do test "pushes message to the socket for authorized clients", %{ + gateway_group: gateway_group, dns_resource: resource, socket: socket } do @@ -286,7 +312,11 @@ defmodule API.Client.ChannelTest do id: resource.id, type: :dns, name: resource.name, - address: resource.address + address: resource.address, + address_description: resource.address_description, + gateway_groups: [ + %{id: gateway_group.id, name: gateway_group.name, routing: gateway_group.routing} + ] } end end @@ -339,6 +369,7 @@ defmodule API.Client.ChannelTest do describe "handle_info/2 :allow_access" do test "pushes message to the socket", %{ account: account, + gateway_group: gateway_group, dns_resource: resource, socket: socket } do @@ -359,7 +390,11 @@ defmodule API.Client.ChannelTest do id: resource.id, type: :dns, name: resource.name, - address: resource.address + address: resource.address, + address_description: resource.address_description, + gateway_groups: [ + %{id: gateway_group.id, name: gateway_group.name, routing: gateway_group.routing} + ] } end end @@ -397,6 +432,7 @@ defmodule API.Client.ChannelTest do test "broadcasts a message to re-add the resource if other policy is found", %{ account: account, + gateway_group: gateway_group, dns_resource: resource, socket: socket } do @@ -420,7 +456,11 @@ defmodule API.Client.ChannelTest do id: resource.id, type: :dns, name: resource.name, - address: resource.address + address: resource.address, + address_description: resource.address_description, + gateway_groups: [ + %{id: gateway_group.id, name: gateway_group.name, routing: gateway_group.routing} + ] } end end diff --git a/elixir/apps/domain/lib/domain/config.ex b/elixir/apps/domain/lib/domain/config.ex index a08a251a1..b5911f863 100644 --- a/elixir/apps/domain/lib/domain/config.ex +++ b/elixir/apps/domain/lib/domain/config.ex @@ -83,7 +83,14 @@ defmodule Domain.Config do def update_config(%Configuration{} = configuration, attrs, %Auth.Subject{} = subject) do with :ok <- Auth.ensure_has_permissions(subject, Authorizer.manage_permission()) do - update_config(configuration, attrs) + case update_config(configuration, attrs) do + {:ok, configuration} -> + :ok = broadcast_update_to_account(configuration) + {:ok, configuration} + + {:error, changeset} -> + {:error, changeset} + end end end diff --git a/elixir/apps/domain/lib/domain/resources.ex b/elixir/apps/domain/lib/domain/resources.ex index 6ece59839..f34c5c380 100644 --- a/elixir/apps/domain/lib/domain/resources.ex +++ b/elixir/apps/domain/lib/domain/resources.ex @@ -1,6 +1,6 @@ defmodule Domain.Resources do alias Domain.{Repo, Validator, Auth, PubSub} - alias Domain.{Accounts, Gateways, Policies} + alias Domain.{Accounts, Gateways, Policies, Flows} alias Domain.Resources.{Authorizer, Resource, Connection} def fetch_resource_by_id(id, %Auth.Subject{} = subject, opts \\ []) do @@ -157,17 +157,24 @@ defmodule Domain.Resources do def update_resource(%Resource{} = resource, attrs, %Auth.Subject{} = subject) do with :ok <- Auth.ensure_has_permissions(subject, Authorizer.manage_resources_permission()) do - resource - |> Resource.Changeset.update(attrs, subject) - |> Repo.update() - |> case do - {:ok, resource} -> - :ok = broadcast_resource_events(:update, resource) - {:ok, resource} + Resource.Query.by_id(resource.id) + |> Authorizer.for_subject(Resource, subject) + |> Repo.fetch_and_update( + with: fn resource -> + resource = Repo.preload(resource, :connections) + changeset = Resource.Changeset.update(resource, attrs, subject) - {:error, reason} -> - {:error, reason} - end + cb = fn _resource -> + if Map.has_key?(changeset.changes, :connections) do + {:ok, _flows} = Flows.expire_flows_for(resource, subject) + end + + :ok = broadcast_resource_events(:update, resource) + end + + {changeset, execute_after_commit: cb} + end + ) end end diff --git a/elixir/apps/domain/lib/domain/resources/resource.ex b/elixir/apps/domain/lib/domain/resources/resource.ex index 81fcdfb9d..6e2f74828 100644 --- a/elixir/apps/domain/lib/domain/resources/resource.ex +++ b/elixir/apps/domain/lib/domain/resources/resource.ex @@ -3,6 +3,7 @@ defmodule Domain.Resources.Resource do schema "resources" do field :address, :string + field :address_description, :string field :name, :string field :type, Ecto.Enum, values: [:cidr, :ip, :dns] diff --git a/elixir/apps/domain/lib/domain/resources/resource/changeset.ex b/elixir/apps/domain/lib/domain/resources/resource/changeset.ex index 7d360a767..d48511454 100644 --- a/elixir/apps/domain/lib/domain/resources/resource/changeset.ex +++ b/elixir/apps/domain/lib/domain/resources/resource/changeset.ex @@ -3,9 +3,9 @@ defmodule Domain.Resources.Resource.Changeset do alias Domain.{Auth, Accounts, Network} alias Domain.Resources.{Resource, Connection} - @fields ~w[address name type]a - @update_fields ~w[name]a - @required_fields ~w[address type]a + @fields ~w[address address_description name type]a + @update_fields ~w[name address_description]a + @required_fields ~w[address address_description type]a def create(%Accounts.Account{} = account, attrs, %Auth.Subject{} = subject) do %Resource{connections: []} @@ -151,6 +151,7 @@ defmodule Domain.Resources.Resource.Changeset do defp changeset(changeset) do changeset |> validate_length(:name, min: 1, max: 255) + |> validate_length(:address_description, min: 1, max: 512) |> cast_embed(:filters, with: &cast_filter/2) |> unique_constraint(:ipv4, name: :resources_account_id_ipv4_index) |> unique_constraint(:ipv6, name: :resources_account_id_ipv6_index) diff --git a/elixir/apps/domain/lib/domain/validator.ex b/elixir/apps/domain/lib/domain/validator.ex index 946d83ebd..aabe4f327 100644 --- a/elixir/apps/domain/lib/domain/validator.ex +++ b/elixir/apps/domain/lib/domain/validator.ex @@ -37,6 +37,27 @@ defmodule Domain.Validator do end) end + def validate_contains(changeset, field, [{:field, source_field} | opts]) do + case fetch_field(changeset, source_field) do + {_data_or_changes, value} when is_binary(value) -> + validate_contains(changeset, field, value, opts) + + _ -> + changeset + end + end + + def validate_contains(changeset, field, substring, opts \\ []) do + validate_change(changeset, field, fn _current_field, value -> + if String.contains?(value, substring) do + [] + else + message = Keyword.get(opts, :message, "should contain #{inspect(substring)}") + [{field, message}] + end + end) + end + def validate_does_not_end_with(changeset, field, suffix, opts \\ []) do validate_change(changeset, field, fn _current_field, value -> if String.ends_with?(value, suffix) do diff --git a/elixir/apps/domain/priv/repo/migrations/20240131212533_add_resources_client_address.exs b/elixir/apps/domain/priv/repo/migrations/20240131212533_add_resources_client_address.exs new file mode 100644 index 000000000..84dd47bde --- /dev/null +++ b/elixir/apps/domain/priv/repo/migrations/20240131212533_add_resources_client_address.exs @@ -0,0 +1,13 @@ +defmodule Domain.Repo.Migrations.AddResourcesClientAddress do + use Ecto.Migration + + def change do + alter table(:resources) do + add(:address_description, :text) + end + + execute("UPDATE resources SET address_description = address") + + execute("ALTER TABLE resources ALTER COLUMN address_description SET NOT NULL") + end +end diff --git a/elixir/apps/domain/priv/repo/seeds.exs b/elixir/apps/domain/priv/repo/seeds.exs index 4ce87bd6a..d80963214 100644 --- a/elixir/apps/domain/priv/repo/seeds.exs +++ b/elixir/apps/domain/priv/repo/seeds.exs @@ -539,6 +539,7 @@ IO.puts("") type: :dns, name: "google.com", address: "google.com", + address_description: "https://google.com/", connections: [%{gateway_group_id: gateway_group.id}], filters: [%{protocol: :all}] }, @@ -551,6 +552,7 @@ IO.puts("") type: :dns, name: "*.firez.one", address: "*.firez.one", + address_description: "https://firez.one/", connections: [%{gateway_group_id: gateway_group.id}], filters: [%{protocol: :all}] }, @@ -563,6 +565,7 @@ IO.puts("") type: :dns, name: "?.firezone.dev", address: "?.firezone.dev", + address_description: "https://firezone.dev/", connections: [%{gateway_group_id: gateway_group.id}], filters: [%{protocol: :all}] }, @@ -575,6 +578,7 @@ IO.puts("") type: :dns, name: "example.com", address: "example.com", + address_description: "https://example.com:1234/", connections: [%{gateway_group_id: gateway_group.id}], filters: [%{protocol: :all}] }, @@ -587,6 +591,7 @@ IO.puts("") type: :dns, name: "ip6only", address: "ip6only.me", + address_description: "https://ip6only.me/", connections: [%{gateway_group_id: gateway_group.id}], filters: [%{protocol: :all}] }, @@ -599,6 +604,24 @@ IO.puts("") type: :dns, name: "gitlab.mycorp.com", address: "gitlab.mycorp.com", + address_description: "https://gitlab.mycorp.com/", + connections: [%{gateway_group_id: gateway_group.id}], + filters: [ + %{ports: ["80", "433"], protocol: :tcp}, + %{ports: ["53"], protocol: :udp}, + %{protocol: :icmp} + ] + }, + admin_subject + ) + +{:ok, ip_resource} = + Resources.create_resource( + %{ + type: :dns, + name: "CloudFlare DNS", + address: "1.1.1.1", + address_description: "http://1.1.1.1:3000/", connections: [%{gateway_group_id: gateway_group.id}], filters: [ %{ports: ["80", "433"], protocol: :tcp}, @@ -615,6 +638,7 @@ IO.puts("") type: :cidr, name: "MyCorp Network", address: "172.20.0.1/16", + address_description: "172.20.0.1/16", connections: [%{gateway_group_id: gateway_group.id}], filters: [%{protocol: :all}] }, @@ -627,6 +651,7 @@ IO.puts(" #{dns_gitlab_resource.address} - DNS - gateways: #{gateway_name}") IO.puts(" #{firez_one.address} - DNS - gateways: #{gateway_name}") IO.puts(" #{firezone_dev.address} - DNS - gateways: #{gateway_name}") IO.puts(" #{example_dns.address} - DNS - gateways: #{gateway_name}") +IO.puts(" #{ip_resource.address} - IP - gateways: #{gateway_name}") IO.puts(" #{cidr_resource.address} - CIDR - gateways: #{gateway_name}") IO.puts("") diff --git a/elixir/apps/domain/test/domain/resources/resource/changeset_test.exs b/elixir/apps/domain/test/domain/resources/resource/changeset_test.exs index 3c1f76ed1..36f038432 100644 --- a/elixir/apps/domain/test/domain/resources/resource/changeset_test.exs +++ b/elixir/apps/domain/test/domain/resources/resource/changeset_test.exs @@ -11,7 +11,7 @@ defmodule Domain.Resources.Resource.ChangesetTest do {"192.168.1.255/32", "192.168.1.255/32"}, {"2607:f8b0:4012:0::200e/128", "2607:f8b0:4012::200e/128"} ] do - changeset = create(%{type: :cidr, address: string}) + changeset = create(%{type: :cidr, address: string, address_description: string}) assert changeset.changes[:address] == cidr assert changeset.valid? end @@ -36,7 +36,7 @@ defmodule Domain.Resources.Resource.ChangesetTest do {"192.168.1.255", "192.168.1.255"}, {"2607:f8b0:4012:0::200e", "2607:f8b0:4012::200e"} ] do - changeset = create(%{type: :ip, address: string}) + changeset = create(%{type: :ip, address: string, address_description: string}) assert changeset.changes[:address] == ip assert changeset.valid? end @@ -63,7 +63,9 @@ defmodule Domain.Resources.Resource.ChangesetTest do "такі.справи", "subdomain.subdomain2.example.space" ] do - changeset = create(%{type: :dns, address: valid_address}) + changeset = + create(%{type: :dns, address: valid_address, address_description: valid_address}) + assert changeset.valid? end diff --git a/elixir/apps/domain/test/domain/resources_test.exs b/elixir/apps/domain/test/domain/resources_test.exs index 3d1fff70a..d17af34d8 100644 --- a/elixir/apps/domain/test/domain/resources_test.exs +++ b/elixir/apps/domain/test/domain/resources_test.exs @@ -877,17 +877,25 @@ defmodule Domain.ResourcesTest do assert errors_on(changeset) == %{ address: ["can't be blank"], + address_description: ["can't be blank"], type: ["can't be blank"], connections: ["can't be blank"] } end test "returns error on invalid attrs", %{subject: subject} do - attrs = %{"name" => String.duplicate("a", 256), "filters" => :foo, "connections" => :bar} + attrs = %{ + "name" => String.duplicate("a", 256), + "address_description" => String.duplicate("a", 513), + "filters" => :foo, + "connections" => :bar + } + assert {:error, changeset} = create_resource(attrs, subject) assert errors_on(changeset) == %{ address: ["can't be blank"], + address_description: ["should be at most 512 character(s)"], name: ["should be at most 255 character(s)"], type: ["can't be blank"], filters: ["is invalid"], @@ -964,6 +972,7 @@ defmodule Domain.ResourcesTest do assert {:ok, resource} = create_resource(attrs, subject) assert resource.address == attrs.address + assert resource.address_description == attrs.address_description assert resource.name == attrs.address assert resource.account_id == account.id @@ -994,12 +1003,14 @@ defmodule Domain.ResourcesTest do ], type: :cidr, name: "mycidr", - address: "192.168.1.1/28" + address: "192.168.1.1/28", + address_description: "192.168.1.1/28" ) assert {:ok, resource} = create_resource(attrs, subject) assert resource.address == "192.168.1.0/28" + assert resource.address_description == attrs.address_description assert resource.name == attrs.name assert resource.account_id == account.id @@ -1073,11 +1084,18 @@ defmodule Domain.ResourcesTest do end test "returns error on invalid attrs", %{resource: resource, subject: subject} do - attrs = %{"name" => String.duplicate("a", 256), "filters" => :foo, "connections" => :bar} + attrs = %{ + "name" => String.duplicate("a", 256), + "address_description" => String.duplicate("a", 513), + "filters" => :foo, + "connections" => :bar + } + assert {:error, changeset} = update_resource(resource, attrs, subject) assert errors_on(changeset) == %{ name: ["should be at most 255 character(s)"], + address_description: ["should be at most 512 character(s)"], filters: ["is invalid"], connections: ["is invalid"] } @@ -1116,14 +1134,35 @@ defmodule Domain.ResourcesTest do assert resource.name == "foo" end + test "allows to update client address", %{resource: resource, subject: subject} do + attrs = %{"address_description" => "http://#{resource.address}:1234/foo"} + assert {:ok, resource} = update_resource(resource, attrs, subject) + assert resource.address_description == attrs["address_description"] + end + test "allows to update filters", %{resource: resource, subject: subject} do attrs = %{"filters" => []} assert {:ok, resource} = update_resource(resource, attrs, subject) assert resource.filters == [] end + test "does not expire flows when connections are not updated", %{ + account: account, + resource: resource, + subject: subject + } do + flow = Fixtures.Flows.create_flow(account: account, resource: resource, subject: subject) + :ok = Domain.Flows.subscribe_to_flow_expiration_events(flow) + + attrs = %{"name" => "foo"} + assert {:ok, _resource} = update_resource(resource, attrs, subject) + + refute_receive {:expire_flow, _flow_id, _client_id, _resource_id} + end + test "allows to update connections", %{account: account, resource: resource, subject: subject} do - gateway1 = Fixtures.Gateways.create_gateway(account: account) + group = Fixtures.Gateways.create_group(account: account, subject: subject) + gateway1 = Fixtures.Gateways.create_gateway(account: account, group: group) attrs = %{"connections" => [%{gateway_group_id: gateway1.group_id}]} assert {:ok, resource} = update_resource(resource, attrs, subject) @@ -1132,6 +1171,9 @@ defmodule Domain.ResourcesTest do gateway2 = Fixtures.Gateways.create_gateway(account: account) + flow = Fixtures.Flows.create_flow(account: account, resource: resource, subject: subject) + :ok = Domain.Flows.subscribe_to_flow_expiration_events(flow) + attrs = %{ "connections" => [ %{gateway_group_id: gateway1.group_id}, @@ -1147,6 +1189,10 @@ defmodule Domain.ResourcesTest do assert {:ok, resource} = update_resource(resource, attrs, subject) gateway_group_ids = Enum.map(resource.connections, & &1.gateway_group_id) assert gateway_group_ids == [gateway2.group_id] + + flow_id = flow.id + resource_id = resource.id + assert_receive {:expire_flow, ^flow_id, _client_id, ^resource_id} end test "does not allow to remove all connections", %{resource: resource, subject: subject} do diff --git a/elixir/apps/domain/test/support/fixtures/resources.ex b/elixir/apps/domain/test/support/fixtures/resources.ex index 64cac3b4c..ea1ecb048 100644 --- a/elixir/apps/domain/test/support/fixtures/resources.ex +++ b/elixir/apps/domain/test/support/fixtures/resources.ex @@ -2,10 +2,12 @@ defmodule Domain.Fixtures.Resources do use Domain.Fixture def resource_attrs(attrs \\ %{}) do - address = "admin-#{unique_integer()}.mycorp.com" + attrs = Enum.into(attrs, %{}) + address = Map.get(attrs, :address, "admin-#{unique_integer()}.mycorp.com") Enum.into(attrs, %{ address: address, + address_description: "http://#{address}/", name: address, type: :dns, filters: [ diff --git a/elixir/apps/web/lib/web/live/resources/edit.ex b/elixir/apps/web/lib/web/live/resources/edit.ex index 6e578ee48..fb9289b08 100644 --- a/elixir/apps/web/lib/web/live/resources/edit.ex +++ b/elixir/apps/web/lib/web/live/resources/edit.ex @@ -55,6 +55,19 @@ defmodule Web.Resources.Edit do required /> +
+ <.input + field={@form[:address_description]} + type="text" + label="Address Description" + placeholder={@form[:address].value || "http://example.com/"} + required + /> +

+ This will be displayed in client applications to assist users in understanding how to access the resource. +

+
+ <.filters_form :if={@traffic_filters_enabled?} form={@form[:filters]} /> <.connections_form diff --git a/elixir/apps/web/lib/web/live/resources/new.ex b/elixir/apps/web/lib/web/live/resources/new.ex index 263c4e277..8b52601da 100644 --- a/elixir/apps/web/lib/web/live/resources/new.ex +++ b/elixir/apps/web/lib/web/live/resources/new.ex @@ -11,6 +11,7 @@ defmodule Web.Resources.New do assign( socket, gateway_groups: gateway_groups, + address_description_changed?: false, name_changed?: false, form: to_form(changeset), params: Map.take(params, ["site_id"]), @@ -111,6 +112,19 @@ defmodule Web.Resources.New do

+
+ <.input + field={@form[:address_description]} + type="text" + label="Address Description" + placeholder={@form[:address].value || "http://example.com/"} + required + /> +

+ This will be displayed in client applications to assist users in understanding how to access the resource. +

+
+ <.input field={@form[:name]} type="text" @@ -139,11 +153,18 @@ defmodule Web.Resources.New do end def handle_event("change", %{"resource" => attrs} = payload, socket) do - name_changed? = socket.assigns.name_changed? || payload["_target"] == ["resource", "name"] + name_changed? = + socket.assigns.name_changed? || + payload["_target"] == ["resource", "name"] + + address_description_changed? = + socket.assigns.address_description_changed? || + payload["_target"] == ["resource", "address_description"] attrs = attrs |> maybe_put_default_name(name_changed?) + |> maybe_put_default_address_description(address_description_changed?) |> map_filters_form_attrs() |> map_connections_form_attrs() |> maybe_put_connections(socket.assigns.params) @@ -152,13 +173,21 @@ defmodule Web.Resources.New do Resources.new_resource(socket.assigns.account, attrs) |> Map.put(:action, :validate) - {:noreply, assign(socket, form: to_form(changeset), name_changed?: name_changed?)} + socket = + assign(socket, + form: to_form(changeset), + name_changed?: name_changed?, + address_description_changed?: address_description_changed? + ) + + {:noreply, socket} end def handle_event("submit", %{"resource" => attrs}, socket) do attrs = attrs |> maybe_put_default_name() + |> maybe_put_default_address_description() |> map_filters_form_attrs() |> map_connections_form_attrs() |> maybe_put_connections(socket.assigns.params) @@ -186,6 +215,32 @@ defmodule Web.Resources.New do Map.put(attrs, "name", attrs["address"]) end + defp maybe_put_default_address_description(attrs, address_description_changed? \\ true) + + defp maybe_put_default_address_description( + %{"type" => "dns", "address" => address} = attrs, + false + ) + when is_binary(address) do + Map.put(attrs, "address_description", "http://#{address}/") + end + + defp maybe_put_default_address_description( + %{"type" => "ip", "address" => address} = attrs, + false + ) + when is_binary(address) do + Map.put(attrs, "address_description", "http://#{address}/") + end + + defp maybe_put_default_address_description(attrs, false) do + Map.put(attrs, "address_description", "") + end + + defp maybe_put_default_address_description(attrs, true) do + attrs + end + defp maybe_put_connections(attrs, params) do if site_id = params["site_id"] do Map.put(attrs, "connections", %{ diff --git a/elixir/apps/web/lib/web/live/resources/show.ex b/elixir/apps/web/lib/web/live/resources/show.ex index 8f0179678..7332be966 100644 --- a/elixir/apps/web/lib/web/live/resources/show.ex +++ b/elixir/apps/web/lib/web/live/resources/show.ex @@ -73,6 +73,27 @@ defmodule Web.Resources.Show do <%= @resource.address %> + <.vertical_table_row> + <:label> + Address Description + + <:value> + @resource.address_description + end + } + target="_blank" + class={link_style()} + > + <%= @resource.address_description %> + <.icon name="hero-arrow-top-right-on-square" class="mb-3 w-3 h-3" /> + + + <.vertical_table_row> <:label> Connected Sites 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 358c255b4..618e46574 100644 --- a/elixir/apps/web/test/web/live/resources/edit_test.exs +++ b/elixir/apps/web/test/web/live/resources/edit_test.exs @@ -100,6 +100,7 @@ defmodule Web.Live.Resources.EditTest do expected_inputs = (connection_inputs ++ [ + "resource[address_description]", "resource[filters][all][enabled]", "resource[filters][all][protocol]", "resource[filters][icmp][enabled]", @@ -132,6 +133,7 @@ defmodule Web.Live.Resources.EditTest do form = form(lv, "form") assert find_inputs(form) == [ + "resource[address_description]", "resource[filters][all][enabled]", "resource[filters][all][protocol]", "resource[filters][icmp][enabled]", @@ -333,7 +335,10 @@ defmodule Web.Live.Resources.EditTest do form = form(lv, "form") - assert find_inputs(form) == ["resource[name]"] + assert find_inputs(form) == [ + "resource[address_description]", + "resource[name]" + ] end test "updates a resource on valid attrs when traffic filters disabled", %{ 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 94022fc8d..a93106a16 100644 --- a/elixir/apps/web/test/web/live/resources/new_test.exs +++ b/elixir/apps/web/test/web/live/resources/new_test.exs @@ -70,6 +70,7 @@ defmodule Web.Live.Resources.NewTest do (connection_inputs ++ [ "resource[address]", + "resource[address_description]", "resource[filters][all][enabled]", "resource[filters][all][protocol]", "resource[filters][icmp][enabled]", @@ -103,6 +104,7 @@ 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]", @@ -183,7 +185,8 @@ defmodule Web.Live.Resources.NewTest do |> render_submit() |> form_validation_errors() == %{ "resource[name]" => ["should be at most 255 character(s)"], - "connections" => ["can't be blank"] + "connections" => ["can't be blank"], + "resource[address_description]" => ["can't be blank"] } end @@ -217,7 +220,8 @@ defmodule Web.Live.Resources.NewTest do |> form("form", resource: attrs) |> render_submit() |> form_validation_errors() == %{ - "resource[address]" => ["can't be blank"] + "resource[address]" => ["can't be blank"], + "resource[address_description]" => ["can't be blank"] } end @@ -231,6 +235,7 @@ defmodule Web.Live.Resources.NewTest do attrs = %{ address: "foobar.com", + address_description: "http://foobar.com:3000/", connections: %{connection.gateway_group_id => %{enabled: false}} } @@ -262,6 +267,7 @@ defmodule Web.Live.Resources.NewTest do name: "foobar.com", type: "dns", address: "foobar.com", + address_description: "http://foobar.com:3000/", filters: %{ icmp: %{enabled: true}, tcp: %{ports: "80, 443"}, @@ -297,6 +303,7 @@ defmodule Web.Live.Resources.NewTest do attrs = %{ name: "foobar.com", address: "foobar.com", + address_description: "http://foobar.com:3000/", filters: %{ icmp: %{enabled: true}, tcp: %{ports: "80, 443"}, @@ -339,6 +346,7 @@ defmodule Web.Live.Resources.NewTest do assert find_inputs(form) == [ "resource[address]", + "resource[address_description]", "resource[name]", "resource[type]" ] @@ -352,7 +360,8 @@ defmodule Web.Live.Resources.NewTest do } do attrs = %{ name: "foobar.com", - address: "foobar.com" + address: "foobar.com", + address_description: "foobar.com" } {:ok, lv, _html} =