From 06791d2d05b63aa3cf90394ea17b538627d8d0ba Mon Sep 17 00:00:00 2001 From: Brian Manifold Date: Thu, 7 Nov 2024 15:45:56 -0500 Subject: [PATCH] refactor(portal): API persistent IDs (#7182) In order for the firezone terraform provider to work properly, the Resources and Policies need to be able to be referenced by their `persistent_id`, specifically in the portal API. --- .../lib/api/controllers/policy_controller.ex | 6 +- .../api/controllers/resource_controller.ex | 7 +- .../api/lib/api/controllers/resource_json.ex | 2 +- .../api/lib/api/schemas/actor_group_schema.ex | 2 +- .../apps/api/lib/api/schemas/actor_schema.ex | 2 +- .../lib/api/schemas/gateway_group_schema.ex | 2 +- .../api/lib/api/schemas/identity_schema.ex | 2 +- .../apps/api/lib/api/schemas/policy_schema.ex | 2 +- .../api/lib/api/schemas/resource_schema.ex | 46 ++++--- elixir/apps/api/mix.exs | 2 +- .../controllers/resource_controller_test.exs | 8 +- elixir/apps/domain/lib/domain/policies.ex | 20 +++ .../lib/domain/policies/policy/query.ex | 8 ++ elixir/apps/domain/lib/domain/resources.ex | 30 +++++ .../lib/domain/resources/resource/query.ex | 8 ++ ...241025152247_add_persistent_id_indexes.exs | 17 +++ .../apps/domain/test/domain/policies_test.exs | 82 ++++++++++++ .../domain/test/domain/resources_test.exs | 117 ++++++++++++++++++ elixir/apps/web/lib/web/live/policies/edit.ex | 2 +- elixir/apps/web/lib/web/live/policies/show.ex | 6 +- .../web/lib/web/live/resources/components.ex | 2 +- .../apps/web/lib/web/live/resources/show.ex | 2 +- elixir/mix.lock | 2 +- 23 files changed, 330 insertions(+), 47 deletions(-) create mode 100644 elixir/apps/domain/priv/repo/migrations/20241025152247_add_persistent_id_indexes.exs diff --git a/elixir/apps/api/lib/api/controllers/policy_controller.ex b/elixir/apps/api/lib/api/controllers/policy_controller.ex index 9f758739c..32e2da09c 100644 --- a/elixir/apps/api/lib/api/controllers/policy_controller.ex +++ b/elixir/apps/api/lib/api/controllers/policy_controller.ex @@ -43,7 +43,7 @@ defmodule API.PolicyController do # Show a specific Policy def show(conn, %{"id" => id}) do - with {:ok, policy} <- Policies.fetch_policy_by_id(id, conn.assigns.subject) do + with {:ok, policy} <- Policies.fetch_policy_by_id_or_persistent_id(id, conn.assigns.subject) do render(conn, :show, policy: policy) end end @@ -91,7 +91,7 @@ defmodule API.PolicyController do def update(conn, %{"id" => id, "policy" => params}) do subject = conn.assigns.subject - with {:ok, policy} <- Policies.fetch_policy_by_id(id, subject) do + with {:ok, policy} <- Policies.fetch_policy_by_id_or_persistent_id(id, subject) do case Policies.update_or_replace_policy(policy, params, subject) do {:updated, updated_policy} -> render(conn, :show, policy: updated_policy) @@ -127,7 +127,7 @@ defmodule API.PolicyController do def delete(conn, %{"id" => id}) do subject = conn.assigns.subject - with {:ok, policy} <- Policies.fetch_policy_by_id(id, subject), + with {:ok, policy} <- Policies.fetch_policy_by_id_or_persistent_id(id, subject), {:ok, policy} <- Policies.delete_policy(policy, subject) do render(conn, :show, policy: policy) end diff --git a/elixir/apps/api/lib/api/controllers/resource_controller.ex b/elixir/apps/api/lib/api/controllers/resource_controller.ex index 3f1d9bf76..02bc10870 100644 --- a/elixir/apps/api/lib/api/controllers/resource_controller.ex +++ b/elixir/apps/api/lib/api/controllers/resource_controller.ex @@ -42,7 +42,8 @@ defmodule API.ResourceController do ] def show(conn, %{"id" => id}) do - with {:ok, resource} <- Resources.fetch_resource_by_id(id, conn.assigns.subject) do + with {:ok, resource} <- + Resources.fetch_resource_by_id_or_persistent_id(id, conn.assigns.subject) do render(conn, :show, resource: resource) end end @@ -91,7 +92,7 @@ defmodule API.ResourceController do subject = conn.assigns.subject attrs = set_param_defaults(params) - with {:ok, resource} <- Resources.fetch_resource_by_id(id, subject) do + with {:ok, resource} <- Resources.fetch_resource_by_id_or_persistent_id(id, subject) do case Resources.update_or_replace_resource(resource, attrs, subject) do {:updated, updated_resource} -> render(conn, :show, resource: updated_resource) @@ -126,7 +127,7 @@ defmodule API.ResourceController do def delete(conn, %{"id" => id}) do subject = conn.assigns.subject - with {:ok, resource} <- Resources.fetch_resource_by_id(id, subject), + with {:ok, resource} <- Resources.fetch_resource_by_id_or_persistent_id(id, subject), {:ok, resource} <- Resources.delete_resource(resource, subject) do render(conn, :show, resource: resource) end diff --git a/elixir/apps/api/lib/api/controllers/resource_json.ex b/elixir/apps/api/lib/api/controllers/resource_json.ex index d479974a5..ec76eb44d 100644 --- a/elixir/apps/api/lib/api/controllers/resource_json.ex +++ b/elixir/apps/api/lib/api/controllers/resource_json.ex @@ -24,7 +24,7 @@ defmodule API.ResourceJSON do id: resource.id, name: resource.name, address: resource.address, - description: resource.address_description, + address_description: resource.address_description, type: resource.type } end diff --git a/elixir/apps/api/lib/api/schemas/actor_group_schema.ex b/elixir/apps/api/lib/api/schemas/actor_group_schema.ex index 8c45be4d8..7f403ac17 100644 --- a/elixir/apps/api/lib/api/schemas/actor_group_schema.ex +++ b/elixir/apps/api/lib/api/schemas/actor_group_schema.ex @@ -31,7 +31,7 @@ defmodule API.Schemas.ActorGroup do description: "POST body for creating an Actor Group", type: :object, properties: %{ - actor_group: %Schema{anyOf: [ActorGroup.Schema]} + actor_group: ActorGroup.Schema }, required: [:actor_group], example: %{ diff --git a/elixir/apps/api/lib/api/schemas/actor_schema.ex b/elixir/apps/api/lib/api/schemas/actor_schema.ex index f3a54d3f1..159bf11df 100644 --- a/elixir/apps/api/lib/api/schemas/actor_schema.ex +++ b/elixir/apps/api/lib/api/schemas/actor_schema.ex @@ -37,7 +37,7 @@ defmodule API.Schemas.Actor do description: "POST body for creating an Actor", type: :object, properties: %{ - actor: %Schema{anyOf: [Actor.Schema]} + actor: Actor.Schema }, required: [:actor], example: %{ diff --git a/elixir/apps/api/lib/api/schemas/gateway_group_schema.ex b/elixir/apps/api/lib/api/schemas/gateway_group_schema.ex index 0f360f954..dc4732d2c 100644 --- a/elixir/apps/api/lib/api/schemas/gateway_group_schema.ex +++ b/elixir/apps/api/lib/api/schemas/gateway_group_schema.ex @@ -31,7 +31,7 @@ defmodule API.Schemas.GatewayGroup do description: "POST body for creating a Gateway Group", type: :object, properties: %{ - gateway_group: %Schema{anyOf: [GatewayGroup.Schema]} + gateway_group: GatewayGroup.Schema }, required: [:gateway_group], example: %{ diff --git a/elixir/apps/api/lib/api/schemas/identity_schema.ex b/elixir/apps/api/lib/api/schemas/identity_schema.ex index bf92053f9..960e67caf 100644 --- a/elixir/apps/api/lib/api/schemas/identity_schema.ex +++ b/elixir/apps/api/lib/api/schemas/identity_schema.ex @@ -35,7 +35,7 @@ defmodule API.Schemas.Identity do description: "POST body for creating a Identity", type: :object, properties: %{ - identity: %Schema{anyOf: [Identity.Schema]} + identity: Identity.Schema }, required: [:identity], example: %{ diff --git a/elixir/apps/api/lib/api/schemas/policy_schema.ex b/elixir/apps/api/lib/api/schemas/policy_schema.ex index 6f154878a..31097f349 100644 --- a/elixir/apps/api/lib/api/schemas/policy_schema.ex +++ b/elixir/apps/api/lib/api/schemas/policy_schema.ex @@ -35,7 +35,7 @@ defmodule API.Schemas.Policy do description: "POST body for creating a Policy", type: :object, properties: %{ - policy: %Schema{anyOf: [Policy.Schema]} + policy: Policy.Schema }, required: [:policy], example: %{ diff --git a/elixir/apps/api/lib/api/schemas/resource_schema.ex b/elixir/apps/api/lib/api/schemas/resource_schema.ex index b63d6ce9e..88b2a520f 100644 --- a/elixir/apps/api/lib/api/schemas/resource_schema.ex +++ b/elixir/apps/api/lib/api/schemas/resource_schema.ex @@ -13,7 +13,7 @@ defmodule API.Schemas.Resource do id: %Schema{type: :string, description: "Resource ID"}, name: %Schema{type: :string, description: "Resource name"}, address: %Schema{type: :string, description: "Resource address"}, - description: %Schema{type: :string, description: "Resource description"}, + address_description: %Schema{type: :string, description: "Resource address description"}, type: %Schema{type: :string, description: "Resource type"} }, required: [:name, :type], @@ -21,7 +21,7 @@ defmodule API.Schemas.Resource do "id" => "42a7f82f-831a-4a9d-8f17-c66c2bb6e205", "name" => "Prod DB", "address" => "10.0.0.10", - "description" => "Production Database", + "address_description" => "Production Database", "type" => "ip" } }) @@ -32,29 +32,27 @@ defmodule API.Schemas.Resource do alias OpenApiSpex.Schema alias API.Schemas.Resource + properties = + Map.merge(Resource.Schema.schema().properties, %{ + connections: %Schema{ + title: "Connections", + description: "Gateway Groups to connect the Resource to", + type: :array, + items: %Schema{ + type: :object, + properties: %{ + gateway_group_id: %Schema{type: :string, description: "Gateway Group ID"} + } + } + } + }) + OpenApiSpex.schema(%{ title: "ResourceRequest", description: "POST body for creating a Resource", type: :object, properties: %{ - resource: %Schema{ - anyOf: [ - Resource.Schema - ], - properties: %{ - connections: %Schema{ - title: "Connections", - description: "Gateway Groups to connect the Resource to", - type: :array, - items: %Schema{ - type: :object, - properties: %{ - gateway_group_id: %Schema{type: :string, description: "Gateway Group ID"} - } - } - } - } - } + resource: %Schema{properties: properties} }, required: [:resource], example: %{ @@ -62,7 +60,7 @@ defmodule API.Schemas.Resource do "id" => "42a7f82f-831a-4a9d-8f17-c66c2bb6e205", "name" => "Prod DB", "address" => "10.0.0.10", - "description" => "Production Database", + "address_description" => "Production Database", "type" => "ip", "connections" => [ %{ @@ -91,7 +89,7 @@ defmodule API.Schemas.Resource do "id" => "42a7f82f-831a-4a9d-8f17-c66c2bb6e205", "name" => "Prod DB", "address" => "10.0.0.10", - "description" => "Production Database", + "address_description" => "Production Database", "type" => "ip" } } @@ -117,14 +115,14 @@ defmodule API.Schemas.Resource do "id" => "42a7f82f-831a-4a9d-8f17-c66c2bb6e205", "name" => "Prod DB", "address" => "10.0.0.10", - "description" => "Production Database", + "address_description" => "Production Database", "type" => "ip" }, %{ "id" => "3b9451c9-5616-48f8-827f-009ace22d015", "name" => "Admin Dashboard", "address" => "10.0.0.20", - "description" => "Production Admin Dashboard", + "address_description" => "Production Admin Dashboard", "type" => "ip" } ], diff --git a/elixir/apps/api/mix.exs b/elixir/apps/api/mix.exs index ce213c29e..fbbf94198 100644 --- a/elixir/apps/api/mix.exs +++ b/elixir/apps/api/mix.exs @@ -59,7 +59,7 @@ defmodule API.MixProject do # Other deps {:jason, "~> 1.2"}, {:remote_ip, "~> 1.1"}, - {:open_api_spex, "~> 3.18"}, + {:open_api_spex, "~> 3.21.2"}, {:ymlr, "~> 5.0"}, # Test deps diff --git a/elixir/apps/api/test/api/controllers/resource_controller_test.exs b/elixir/apps/api/test/api/controllers/resource_controller_test.exs index 17519f368..58287d921 100644 --- a/elixir/apps/api/test/api/controllers/resource_controller_test.exs +++ b/elixir/apps/api/test/api/controllers/resource_controller_test.exs @@ -98,7 +98,7 @@ defmodule API.ResourceControllerTest do assert json_response(conn, 200) == %{ "data" => %{ "address" => resource.address, - "description" => resource.address_description, + "address_description" => resource.address_description, "id" => resource.id, "name" => resource.name, "type" => Atom.to_string(resource.type) @@ -169,7 +169,7 @@ defmodule API.ResourceControllerTest do assert resp = json_response(conn, 201) assert resp["data"]["address"] == attrs["address"] - assert resp["data"]["description"] == nil + assert resp["data"]["address_description"] == nil assert resp["data"]["name"] == attrs["name"] assert resp["data"]["type"] == attrs["type"] end @@ -209,7 +209,7 @@ defmodule API.ResourceControllerTest do assert resp = json_response(conn, 200) assert resp["data"]["address"] == resource.address - assert resp["data"]["description"] == resource.address_description + assert resp["data"]["address_description"] == resource.address_description assert resp["data"]["name"] == attrs["name"] end end @@ -233,7 +233,7 @@ defmodule API.ResourceControllerTest do assert json_response(conn, 200) == %{ "data" => %{ "address" => resource.address, - "description" => resource.address_description, + "address_description" => resource.address_description, "id" => resource.id, "name" => resource.name, "type" => Atom.to_string(resource.type) diff --git a/elixir/apps/domain/lib/domain/policies.ex b/elixir/apps/domain/lib/domain/policies.ex index ca3efbc9b..86a5780c6 100644 --- a/elixir/apps/domain/lib/domain/policies.ex +++ b/elixir/apps/domain/lib/domain/policies.ex @@ -23,6 +23,26 @@ defmodule Domain.Policies do end end + def fetch_policy_by_id_or_persistent_id(id, %Auth.Subject{} = subject, opts \\ []) do + required_permissions = + {:one_of, + [ + Authorizer.manage_policies_permission(), + Authorizer.view_available_policies_permission() + ]} + + with :ok <- Auth.ensure_has_permissions(subject, required_permissions), + true <- Repo.valid_uuid?(id) do + Policy.Query.all() + |> Policy.Query.by_id_or_persistent_id(id) + |> Authorizer.for_subject(subject) + |> Repo.fetch(Policy.Query, opts) + else + false -> {:error, :not_found} + other -> other + end + end + def list_policies(%Auth.Subject{} = subject, opts \\ []) do required_permissions = {:one_of, diff --git a/elixir/apps/domain/lib/domain/policies/policy/query.ex b/elixir/apps/domain/lib/domain/policies/policy/query.ex index 23b564729..e579dce67 100644 --- a/elixir/apps/domain/lib/domain/policies/policy/query.ex +++ b/elixir/apps/domain/lib/domain/policies/policy/query.ex @@ -22,6 +22,14 @@ defmodule Domain.Policies.Policy.Query do where(queryable, [policies: policies], policies.id == ^id) end + def by_id_or_persistent_id(queryable, id) do + where(queryable, [policies: policies], policies.id == ^id) + |> or_where( + [policies: policies], + policies.persistent_id == ^id and is_nil(policies.replaced_by_policy_id) + ) + end + def by_account_id(queryable, account_id) do where(queryable, [policies: policies], policies.account_id == ^account_id) end diff --git a/elixir/apps/domain/lib/domain/resources.ex b/elixir/apps/domain/lib/domain/resources.ex index a2a3d897d..c8081f943 100644 --- a/elixir/apps/domain/lib/domain/resources.ex +++ b/elixir/apps/domain/lib/domain/resources.ex @@ -23,6 +23,26 @@ defmodule Domain.Resources do end end + def fetch_resource_by_id_or_persistent_id(id, %Auth.Subject{} = subject, opts \\ []) do + required_permissions = + {:one_of, + [ + Authorizer.manage_resources_permission(), + Authorizer.view_available_resources_permission() + ]} + + with :ok <- Auth.ensure_has_permissions(subject, required_permissions), + true <- Repo.valid_uuid?(id) do + Resource.Query.all() + |> Resource.Query.by_id_or_persistent_id(id) + |> Authorizer.for_subject(Resource, subject) + |> Repo.fetch(Resource.Query, opts) + else + false -> {:error, :not_found} + other -> other + end + end + def fetch_and_authorize_resource_by_id(id, %Auth.Subject{} = subject, opts \\ []) do with :ok <- Auth.ensure_has_permissions(subject, Authorizer.view_available_resources_permission()), @@ -48,6 +68,16 @@ defmodule Domain.Resources do end end + def fetch_resource_by_id_or_persistent_id!(id) do + if Repo.valid_uuid?(id) do + Resource.Query.not_deleted() + |> Resource.Query.by_id_or_persistent_id(id) + |> Repo.one!() + else + {:error, :not_found} + end + end + def all_authorized_resources(%Auth.Subject{} = subject, opts \\ []) do with :ok <- Auth.ensure_has_permissions(subject, Authorizer.view_available_resources_permission()) do diff --git a/elixir/apps/domain/lib/domain/resources/resource/query.ex b/elixir/apps/domain/lib/domain/resources/resource/query.ex index 3a46a0db0..00a2f6f9d 100644 --- a/elixir/apps/domain/lib/domain/resources/resource/query.ex +++ b/elixir/apps/domain/lib/domain/resources/resource/query.ex @@ -26,6 +26,14 @@ defmodule Domain.Resources.Resource.Query do where(queryable, [resources: resources], resources.id == ^id) end + def by_id_or_persistent_id(queryable, id) do + where(queryable, [resources: resources], resources.id == ^id) + |> or_where( + [resources: resources], + resources.persistent_id == ^id and is_nil(resources.replaced_by_resource_id) + ) + end + def by_account_id(queryable, account_id) do where(queryable, [resources: resources], resources.account_id == ^account_id) end diff --git a/elixir/apps/domain/priv/repo/migrations/20241025152247_add_persistent_id_indexes.exs b/elixir/apps/domain/priv/repo/migrations/20241025152247_add_persistent_id_indexes.exs new file mode 100644 index 000000000..a0de1286c --- /dev/null +++ b/elixir/apps/domain/priv/repo/migrations/20241025152247_add_persistent_id_indexes.exs @@ -0,0 +1,17 @@ +defmodule Domain.Repo.Migrations.AddPersistentIdIndexes do + use Ecto.Migration + + def change do + execute(""" + CREATE INDEX resource_persistent_id_index + ON resources (persistent_id) + WHERE replaced_by_resource_id IS NULL + """) + + execute(""" + CREATE INDEX policy_persistent_id_index + ON policies (persistent_id) + WHERE replaced_by_policy_id IS NULL + """) + end +end diff --git a/elixir/apps/domain/test/domain/policies_test.exs b/elixir/apps/domain/test/domain/policies_test.exs index 7e6584545..a268d10c9 100644 --- a/elixir/apps/domain/test/domain/policies_test.exs +++ b/elixir/apps/domain/test/domain/policies_test.exs @@ -73,6 +73,88 @@ defmodule Domain.PoliciesTest do end end + describe "fetch_policy_by_id_or_persistent_id/3" do + test "returns error when policy does not exist", %{subject: subject} do + assert fetch_policy_by_id_or_persistent_id(Ecto.UUID.generate(), subject) == + {:error, :not_found} + end + + test "returns error when UUID is invalid", %{subject: subject} do + assert fetch_policy_by_id_or_persistent_id("foo", subject) == {:error, :not_found} + end + + test "returns policy when policy exists", %{account: account, subject: subject} do + policy = Fixtures.Policies.create_policy(account: account) + + assert {:ok, fetched_policy} = fetch_policy_by_id_or_persistent_id(policy.id, subject) + assert fetched_policy.id == policy.id + + assert {:ok, fetched_policy} = + fetch_policy_by_id_or_persistent_id(policy.persistent_id, subject) + + assert fetched_policy.id == policy.id + end + + test "returns deleted policies", %{account: account, subject: subject} do + {:ok, policy} = + Fixtures.Policies.create_policy(account: account) + |> delete_policy(subject) + + assert {:ok, fetched_policy} = fetch_policy_by_id_or_persistent_id(policy.id, subject) + assert fetched_policy.id == policy.id + + assert {:ok, fetched_policy} = + fetch_policy_by_id_or_persistent_id(policy.persistent_id, subject) + + assert fetched_policy.id == policy.id + end + + test "does not return policies in other accounts", %{subject: subject} do + policy = Fixtures.Policies.create_policy() + assert fetch_policy_by_id_or_persistent_id(policy.id, subject) == {:error, :not_found} + + assert fetch_policy_by_id_or_persistent_id(policy.persistent_id, subject) == + {:error, :not_found} + end + + test "returns error when subject has no permission to view policies", %{subject: subject} do + subject = Fixtures.Auth.remove_permissions(subject) + + assert fetch_policy_by_id_or_persistent_id(Ecto.UUID.generate(), subject) == + {:error, + {:unauthorized, + reason: :missing_permissions, + missing_permissions: [ + {:one_of, + [ + Policies.Authorizer.manage_policies_permission(), + Policies.Authorizer.view_available_policies_permission() + ]} + ]}} + end + + # TODO: add a test that soft-deleted assocs are not preloaded + test "associations are preloaded when opts given", %{account: account, subject: subject} do + policy = Fixtures.Policies.create_policy(account: account) + + {:ok, policy} = + fetch_policy_by_id_or_persistent_id(policy.id, subject, + preload: [:actor_group, :resource] + ) + + assert Ecto.assoc_loaded?(policy.actor_group) + assert Ecto.assoc_loaded?(policy.resource) + + {:ok, policy} = + fetch_policy_by_id_or_persistent_id(policy.persistent_id, subject, + preload: [:actor_group, :resource] + ) + + assert Ecto.assoc_loaded?(policy.actor_group) + assert Ecto.assoc_loaded?(policy.resource) + end + end + describe "list_policies/2" do test "returns empty list when there are no policies", %{subject: subject} do assert {:ok, [], _metadata} = list_policies(subject) diff --git a/elixir/apps/domain/test/domain/resources_test.exs b/elixir/apps/domain/test/domain/resources_test.exs index 987f5620d..5809b98da 100644 --- a/elixir/apps/domain/test/domain/resources_test.exs +++ b/elixir/apps/domain/test/domain/resources_test.exs @@ -104,6 +104,123 @@ defmodule Domain.ResourcesTest do end end + describe "fetch_resource_by_id_or_persistent_id/3" do + test "returns error when resource does not exist", %{subject: subject} do + assert fetch_resource_by_id_or_persistent_id(Ecto.UUID.generate(), subject) == + {:error, :not_found} + end + + test "returns error when UUID is invalid", %{subject: subject} do + assert fetch_resource_by_id_or_persistent_id("foo", subject) == {:error, :not_found} + end + + test "returns resource for account admin", %{account: account, subject: subject} do + resource = Fixtures.Resources.create_resource(account: account) + + assert {:ok, fetched_resource} = fetch_resource_by_id_or_persistent_id(resource.id, subject) + assert fetched_resource.id == resource.id + + assert {:ok, fetched_resource} = + fetch_resource_by_id_or_persistent_id(resource.persistent_id, subject) + + assert fetched_resource.id == resource.id + end + + test "returns authorized resource for account user", %{ + account: account + } do + actor_group = Fixtures.Actors.create_group(account: account) + actor = Fixtures.Actors.create_actor(type: :account_user, account: account) + Fixtures.Actors.create_membership(account: account, actor: actor, group: actor_group) + + identity = Fixtures.Auth.create_identity(account: account, actor: actor) + subject = Fixtures.Auth.create_subject(identity: identity) + + resource = Fixtures.Resources.create_resource(account: account) + + assert fetch_resource_by_id_or_persistent_id(resource.id, subject) == {:error, :not_found} + + assert fetch_resource_by_id_or_persistent_id(resource.persistent_id, subject) == + {:error, :not_found} + + policy = + Fixtures.Policies.create_policy( + account: account, + actor_group: actor_group, + resource: resource + ) + + assert {:ok, fetched_resource} = fetch_resource_by_id_or_persistent_id(resource.id, subject) + assert fetched_resource.id == resource.id + assert Enum.map(fetched_resource.authorized_by_policies, & &1.id) == [policy.id] + + assert {:ok, fetched_resource} = + fetch_resource_by_id_or_persistent_id(resource.persistent_id, subject) + + assert fetched_resource.id == resource.id + assert Enum.map(fetched_resource.authorized_by_policies, & &1.id) == [policy.id] + end + + test "returns deleted resources", %{account: account, subject: subject} do + {:ok, resource} = + Fixtures.Resources.create_resource(account: account) + |> delete_resource(subject) + + assert {:ok, _resource} = fetch_resource_by_id_or_persistent_id(resource.id, subject) + + assert {:ok, _resource} = + fetch_resource_by_id_or_persistent_id(resource.persistent_id, subject) + end + + test "does not return resources in other accounts", %{subject: subject} do + resource = Fixtures.Resources.create_resource() + assert fetch_resource_by_id_or_persistent_id(resource.id, subject) == {:error, :not_found} + + assert fetch_resource_by_id_or_persistent_id(resource.persistent_id, subject) == + {:error, :not_found} + end + + test "returns error when subject has no permission to view resources", %{subject: subject} do + subject = Fixtures.Auth.remove_permissions(subject) + + assert fetch_resource_by_id_or_persistent_id(Ecto.UUID.generate(), subject) == + {:error, + {:unauthorized, + reason: :missing_permissions, + missing_permissions: [ + {:one_of, + [ + Resources.Authorizer.manage_resources_permission(), + Resources.Authorizer.view_available_resources_permission() + ]} + ]}} + end + + test "associations are preloaded when opts given", %{account: account, subject: subject} do + gateway_group = Fixtures.Gateways.create_group(account: account) + + resource = + Fixtures.Resources.create_resource( + account: account, + connections: [%{gateway_group_id: gateway_group.id}] + ) + + assert {:ok, resource} = + fetch_resource_by_id_or_persistent_id(resource.id, subject, preload: :connections) + + assert Ecto.assoc_loaded?(resource.connections) + assert length(resource.connections) == 1 + + assert {:ok, resource} = + fetch_resource_by_id_or_persistent_id(resource.persistent_id, subject, + preload: :connections + ) + + assert Ecto.assoc_loaded?(resource.connections) + assert length(resource.connections) == 1 + end + end + describe "fetch_and_authorize_resource_by_id/3" do test "returns error when resource does not exist", %{subject: subject} do assert fetch_and_authorize_resource_by_id(Ecto.UUID.generate(), subject) == diff --git a/elixir/apps/web/lib/web/live/policies/edit.ex b/elixir/apps/web/lib/web/live/policies/edit.ex index 78ede475b..b6c40642b 100644 --- a/elixir/apps/web/lib/web/live/policies/edit.ex +++ b/elixir/apps/web/lib/web/live/policies/edit.ex @@ -5,7 +5,7 @@ defmodule Web.Policies.Edit do def mount(%{"id" => id}, _session, socket) do with {:ok, policy} <- - Policies.fetch_policy_by_id(id, socket.assigns.subject, + Policies.fetch_policy_by_id_or_persistent_id(id, socket.assigns.subject, preload: [:actor_group, :resource], filter: [deleted?: false] ) do diff --git a/elixir/apps/web/lib/web/live/policies/show.ex b/elixir/apps/web/lib/web/live/policies/show.ex index b3528cd89..acc06d9df 100644 --- a/elixir/apps/web/lib/web/live/policies/show.ex +++ b/elixir/apps/web/lib/web/live/policies/show.ex @@ -5,7 +5,7 @@ defmodule Web.Policies.Show do def mount(%{"id" => id}, _session, socket) do with {:ok, policy} <- - Policies.fetch_policy_by_id(id, socket.assigns.subject, + Policies.fetch_policy_by_id_or_persistent_id(id, socket.assigns.subject, preload: [ actor_group: [:provider], resource: [], @@ -314,7 +314,9 @@ defmodule Web.Policies.Show do def handle_info({_action, _policy_id}, socket) do {:ok, policy} = - Policies.fetch_policy_by_id(socket.assigns.policy.id, socket.assigns.subject, + Policies.fetch_policy_by_id_or_persistent_id( + socket.assigns.policy.id, + socket.assigns.subject, preload: [ actor_group: [:provider], resource: [], diff --git a/elixir/apps/web/lib/web/live/resources/components.ex b/elixir/apps/web/lib/web/live/resources/components.ex index a43af8f72..bcb87c0c6 100644 --- a/elixir/apps/web/lib/web/live/resources/components.ex +++ b/elixir/apps/web/lib/web/live/resources/components.ex @@ -10,7 +10,7 @@ defmodule Web.Resources.Components do } def fetch_resource_option(id, subject) do - {:ok, resource} = Resources.fetch_resource_by_id(id, subject) + {:ok, resource} = Resources.fetch_resource_by_id_or_persistent_id(id, subject) {:ok, resource_option(resource)} end diff --git a/elixir/apps/web/lib/web/live/resources/show.ex b/elixir/apps/web/lib/web/live/resources/show.ex index b62153ba5..e123f0a10 100644 --- a/elixir/apps/web/lib/web/live/resources/show.ex +++ b/elixir/apps/web/lib/web/live/resources/show.ex @@ -6,7 +6,7 @@ defmodule Web.Resources.Show do def mount(%{"id" => id} = params, _session, socket) do with {:ok, resource} <- - Resources.fetch_resource_by_id(id, socket.assigns.subject, + Resources.fetch_resource_by_id_or_persistent_id(id, socket.assigns.subject, preload: [ :gateway_groups, :created_by_actor, diff --git a/elixir/mix.lock b/elixir/mix.lock index e95c95d75..fa27da0eb 100644 --- a/elixir/mix.lock +++ b/elixir/mix.lock @@ -61,7 +61,7 @@ "nimble_pool": {:hex, :nimble_pool, "1.1.0", "bf9c29fbdcba3564a8b800d1eeb5a3c58f36e1e11d7b7fb2e084a643f645f06b", [:mix], [], "hexpm", "af2e4e6b34197db81f7aad230c1118eac993acc0dae6bc83bac0126d4ae0813a"}, "number": {:hex, :number, "1.0.5", "d92136f9b9382aeb50145782f116112078b3465b7be58df1f85952b8bb399b0f", [:mix], [{:decimal, "~> 1.5 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}], "hexpm", "c0733a0a90773a66582b9e92a3f01290987f395c972cb7d685f51dd927cd5169"}, "observer_cli": {:hex, :observer_cli, "1.7.5", "cf73407c40ba3933a4be8be5cdbfcd647a7ec24b49f1d75e912ae1f2e58bc5d4", [:mix, :rebar3], [{:recon, "~> 2.5.5", [hex: :recon, repo: "hexpm", optional: false]}], "hexpm", "872cf8e833a3a71ebd05420692678ec8aaede8fd96c805a4687398f6b23a3014"}, - "open_api_spex": {:hex, :open_api_spex, "3.21.0", "0582a58d48818260636edffc40a28c378be93f68f0735a62c10ada574dcab5b8", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}, {:poison, "~> 3.0 or ~> 4.0 or ~> 5.0 or ~> 6.0", [hex: :poison, repo: "hexpm", optional: true]}, {:ymlr, "~> 2.0 or ~> 3.0 or ~> 4.0 or ~> 5.0", [hex: :ymlr, repo: "hexpm", optional: true]}], "hexpm", "acb595f268b2bd1ac1c9bd70cc2246de43b8fc86a496f8e525272d240b5f1aa4"}, + "open_api_spex": {:hex, :open_api_spex, "3.21.2", "6a704f3777761feeb5657340250d6d7332c545755116ca98f33d4b875777e1e5", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}, {:poison, "~> 3.0 or ~> 4.0 or ~> 5.0 or ~> 6.0", [hex: :poison, repo: "hexpm", optional: true]}, {:ymlr, "~> 2.0 or ~> 3.0 or ~> 4.0 or ~> 5.0", [hex: :ymlr, repo: "hexpm", optional: true]}], "hexpm", "f42ae6ed668b895ebba3e02773cfb4b41050df26f803f2ef634c72a7687dc387"}, "openid_connect": {:git, "https://github.com/firezone/openid_connect.git", "e4d9dca8ae43c765c00a7d3dfa12d6f24f5b3418", [ref: "e4d9dca8ae43c765c00a7d3dfa12d6f24f5b3418"]}, "opentelemetry": {:hex, :opentelemetry, "1.4.0", "f928923ed80adb5eb7894bac22e9a198478e6a8f04020ae1d6f289fdcad0b498", [:rebar3], [{:opentelemetry_api, "~> 1.3.0", [hex: :opentelemetry_api, repo: "hexpm", optional: false]}, {:opentelemetry_semantic_conventions, "~> 0.2", [hex: :opentelemetry_semantic_conventions, repo: "hexpm", optional: false]}], "hexpm", "50b32ce127413e5d87b092b4d210a3449ea80cd8224090fe68d73d576a3faa15"}, "opentelemetry_api": {:hex, :opentelemetry_api, "1.3.1", "83b4713593f80562d9643c4ab0b6f80f3c5fa4c6d0632c43e11b2ccb6b04dfa7", [:mix, :rebar3], [{:opentelemetry_semantic_conventions, "~> 0.2", [hex: :opentelemetry_semantic_conventions, repo: "hexpm", optional: false]}], "hexpm", "9e8a5cc38671e3ac61be48abe5f6b3afdbbb50a1dc08b7950c56f169611505c1"},