From 8e4a4a7b05de5689213a53479aa7f471e4e1fe7e Mon Sep 17 00:00:00 2001 From: Andrew Dryga Date: Mon, 26 Aug 2024 15:30:46 -0600 Subject: [PATCH] feat(portal): Pre-check constraint conformation on client connect (#6431) Closes #6216 --- elixir/apps/api/lib/api/client/channel.ex | 46 +++++++++++-------- .../apps/api/test/api/client/channel_test.exs | 35 +++++++++++++- elixir/apps/domain/lib/domain/policies.ex | 22 +++++++++ .../domain/policies/condition/evaluator.ex | 7 +++ 4 files changed, 90 insertions(+), 20 deletions(-) diff --git a/elixir/apps/api/lib/api/client/channel.ex b/elixir/apps/api/lib/api/client/channel.ex index 5fd148251..5d5f5d56d 100644 --- a/elixir/apps/api/lib/api/client/channel.ex +++ b/elixir/apps/api/lib/api/client/channel.ex @@ -74,6 +74,8 @@ defmodule API.Client.Channel do ] ) + resources = Policies.pre_filter_non_conforming_resources(resources, socket.assigns.client) + # We subscribe for all resource events but only care about update events, # where resource might be renamed which should be propagated to the UI. :ok = Enum.each(resources, &Resources.subscribe_to_events_for_resource/1) @@ -222,27 +224,35 @@ 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, - preload: [:gateway_groups] - ) do - {:ok, resource} -> - case map_or_drop_compatible_resource( - resource, - socket.assigns.client.last_seen_version - ) do - {:cont, resource} -> - push( - socket, - "resource_created_or_updated", - Views.Resource.render(resource) - ) - - :drop -> - :ok - end + with {:ok, resource} <- + Resources.fetch_and_authorize_resource_by_id(resource_id, socket.assigns.subject, + preload: [:gateway_groups] + ), + true <- + Policies.client_conforms_any_on_connect?( + socket.assigns.client, + resource.authorized_by_policies + ) do + case map_or_drop_compatible_resource( + resource, + socket.assigns.client.last_seen_version + ) do + {:cont, resource} -> + push( + socket, + "resource_created_or_updated", + Views.Resource.render(resource) + ) + :drop -> + :ok + end + else {:error, _reason} -> :ok + + false -> + :ok end {:noreply, socket} diff --git a/elixir/apps/api/test/api/client/channel_test.exs b/elixir/apps/api/test/api/client/channel_test.exs index 504ecca1d..82ac49d27 100644 --- a/elixir/apps/api/test/api/client/channel_test.exs +++ b/elixir/apps/api/test/api/client/channel_test.exs @@ -53,6 +53,12 @@ defmodule API.Client.ChannelTest do connections: [%{gateway_group_id: gateway_group.id}] ) + nonconforming_resource = + Fixtures.Resources.create_resource( + account: account, + connections: [%{gateway_group_id: gateway_group.id}] + ) + dns_resource_policy = Fixtures.Policies.create_policy( account: account, @@ -72,6 +78,19 @@ defmodule API.Client.ChannelTest do resource: ip_resource ) + Fixtures.Policies.create_policy( + account: account, + actor_group: actor_group, + resource: nonconforming_resource, + conditions: [ + %{ + property: :remote_ip_location_region, + operator: :is_not_in, + values: [client.last_seen_remote_ip_location_region] + } + ] + ) + expires_at = DateTime.utc_now() |> DateTime.add(30, :second) subject = %{subject | expires_at: expires_at} @@ -100,6 +119,7 @@ defmodule API.Client.ChannelTest do cidr_resource: cidr_resource, ip_resource: ip_resource, unauthorized_resource: unauthorized_resource, + nonconforming_resource: nonconforming_resource, dns_resource_policy: dns_resource_policy, socket: socket } @@ -203,12 +223,13 @@ defmodule API.Client.ChannelTest do {:error, %{reason: :invalid_version}} end - test "sends list of resources after join", %{ + test "sends list of available resources after join", %{ client: client, gateway_group: gateway_group, dns_resource: dns_resource, cidr_resource: cidr_resource, - ip_resource: ip_resource + ip_resource: ip_resource, + nonconforming_resource: nonconforming_resource } do assert_push "init", %{ resources: resources, @@ -279,6 +300,8 @@ defmodule API.Client.ChannelTest do ] } in resources + refute Enum.any?(resources, &(&1.id == nonconforming_resource.id)) + assert interface == %{ ipv4: client.ipv4, ipv6: client.ipv6, @@ -732,6 +755,14 @@ defmodule API.Client.ChannelTest do ] } end + + test "does not push resources that can't be access by the client", %{ + nonconforming_resource: resource, + socket: socket + } do + send(socket.channel_pid, {:update_resource, resource.id}) + refute_push "resource_created_or_updated", %{} + end end describe "handle_info/2 :delete_resource" do diff --git a/elixir/apps/domain/lib/domain/policies.ex b/elixir/apps/domain/lib/domain/policies.ex index 57c960614..d743490b5 100644 --- a/elixir/apps/domain/lib/domain/policies.ex +++ b/elixir/apps/domain/lib/domain/policies.ex @@ -165,6 +165,28 @@ defmodule Domain.Policies do {:ok, policies} end + def pre_filter_non_conforming_resources(resources, %Clients.Client{} = client) do + resources + |> Enum.flat_map(fn resource -> + case client_conforms_any_on_connect?(client, resource.authorized_by_policies) do + true -> [resource] + false -> [] + end + end) + end + + def client_conforms_any_on_connect?(%Clients.Client{} = client, policies) do + Enum.any?(policies, fn policy -> + policy.conditions + |> Enum.filter(&Condition.Evaluator.evaluable_on_connect?/1) + |> Condition.Evaluator.ensure_conforms(client) + |> case do + {:ok, _expires_at} -> true + {:error, _violated_properties} -> false + end + end) + end + def ensure_client_conforms_policy_conditions(%Clients.Client{} = client, %Policy{} = policy) do case Condition.Evaluator.ensure_conforms(policy.conditions, client) do {:ok, expires_at} -> diff --git a/elixir/apps/domain/lib/domain/policies/condition/evaluator.ex b/elixir/apps/domain/lib/domain/policies/condition/evaluator.ex index 818a95651..0b59fdeda 100644 --- a/elixir/apps/domain/lib/domain/policies/condition/evaluator.ex +++ b/elixir/apps/domain/lib/domain/policies/condition/evaluator.ex @@ -5,6 +5,13 @@ defmodule Domain.Policies.Condition.Evaluator do @days_of_week ~w[M T W R F S U] + @doc """ + Returns `true` if the condition can be evaluated during connection (eg. for IP address matching + it can't change while socket is open), otherwise `false`. + """ + def evaluable_on_connect?(%Condition{property: "current_utc_datetime"}), do: false + def evaluable_on_connect?(_), do: true + def ensure_conforms([], %Clients.Client{}) do {:ok, nil} end