diff --git a/elixir/apps/domain/lib/domain/resources/resource/query.ex b/elixir/apps/domain/lib/domain/resources/resource/query.ex index a8d0c6d96..4a00d2a2f 100644 --- a/elixir/apps/domain/lib/domain/resources/resource/query.ex +++ b/elixir/apps/domain/lib/domain/resources/resource/query.ex @@ -25,14 +25,17 @@ defmodule Domain.Resources.Resource.Query do end def by_authorized_actor_id(queryable \\ not_deleted(), actor_id) do - subquery = Domain.Policies.Policy.Query.by_actor_id(actor_id) + subquery = + Domain.Policies.Policy.Query.by_actor_id(actor_id) + |> where([policies: policies], policies.resource_id == parent_as(:resources).id) + |> limit(1) queryable |> join( - :inner, + :inner_lateral, [resources: resources], policies in subquery(subquery), - on: policies.resource_id == resources.id, + on: true, as: :authorized_by_policies ) # Note: this will only write one of policies to a map, which means that diff --git a/elixir/apps/domain/test/domain/resources_test.exs b/elixir/apps/domain/test/domain/resources_test.exs index c007ae81b..9bc6fd58d 100644 --- a/elixir/apps/domain/test/domain/resources_test.exs +++ b/elixir/apps/domain/test/domain/resources_test.exs @@ -49,15 +49,16 @@ defmodule Domain.ResourcesTest do assert fetch_resource_by_id(resource.id, subject) == {:error, :not_found} - Fixtures.Policies.create_policy( - account: account, - actor_group: actor_group, - resource: resource - ) + policy = + Fixtures.Policies.create_policy( + account: account, + actor_group: actor_group, + resource: resource + ) assert {:ok, fetched_resource} = fetch_resource_by_id(resource.id, subject) assert fetched_resource.id == resource.id - refute is_nil(fetched_resource.authorized_by_policy) + assert fetched_resource.authorized_by_policy.id == policy.id end test "returns deleted resources", %{account: account, subject: subject} do @@ -152,15 +153,49 @@ defmodule Domain.ResourcesTest do assert fetch_and_authorize_resource_by_id(resource.id, subject) == {:error, :not_found} - Fixtures.Policies.create_policy( - account: account, - actor_group: actor_group, - resource: resource - ) + policy = + Fixtures.Policies.create_policy( + account: account, + actor_group: actor_group, + resource: resource + ) assert {:ok, fetched_resource} = fetch_and_authorize_resource_by_id(resource.id, subject) assert fetched_resource.id == resource.id - refute is_nil(fetched_resource.authorized_by_policy) + assert fetched_resource.authorized_by_policy.id == policy.id + end + + test "returns authorized resource using one of multiple policies for account user", %{ + account: account + } do + actor = Fixtures.Actors.create_actor(type: :account_user, account: account) + identity = Fixtures.Auth.create_identity(account: account, actor: actor) + subject = Fixtures.Auth.create_subject(identity: identity) + resource = Fixtures.Resources.create_resource(account: account) + + actor_group1 = Fixtures.Actors.create_group(account: account) + Fixtures.Actors.create_membership(account: account, actor: actor, group: actor_group1) + + policy1 = + Fixtures.Policies.create_policy( + account: account, + actor_group: actor_group1, + resource: resource + ) + + actor_group2 = Fixtures.Actors.create_group(account: account) + Fixtures.Actors.create_membership(account: account, actor: actor, group: actor_group2) + + policy2 = + Fixtures.Policies.create_policy( + account: account, + actor_group: actor_group2, + resource: resource + ) + + assert {:ok, fetched_resource} = fetch_and_authorize_resource_by_id(resource.id, subject) + assert fetched_resource.id == resource.id + assert fetched_resource.authorized_by_policy.id in [policy1.id, policy2.id] end test "does not return deleted resources", %{account: account, actor: actor, subject: subject} do @@ -180,6 +215,66 @@ defmodule Domain.ResourcesTest do assert fetch_and_authorize_resource_by_id(resource.id, subject) == {:error, :not_found} end + test "does not authorize using deleted policies", %{ + account: account, + actor: actor, + subject: subject + } do + resource = Fixtures.Resources.create_resource(account: account) + actor_group = Fixtures.Actors.create_group(account: account) + Fixtures.Actors.create_membership(account: account, actor: actor, group: actor_group) + + Fixtures.Policies.create_policy( + account: account, + actor_group: actor_group, + resource: resource + ) + |> Fixtures.Policies.delete_policy() + + assert fetch_and_authorize_resource_by_id(resource.id, subject) == {:error, :not_found} + end + + test "does not authorize using deleted group membership", %{ + account: account, + subject: subject + } do + resource = Fixtures.Resources.create_resource(account: account) + actor_group = Fixtures.Actors.create_group(account: account) + + # memberships are not soft deleted + # Fixtures.Actors.create_membership(account: account, actor: actor, group: actor_group) + + Fixtures.Policies.create_policy( + account: account, + actor_group: actor_group, + resource: resource + ) + + assert fetch_and_authorize_resource_by_id(resource.id, subject) == {:error, :not_found} + end + + test "does not authorize using deleted group", %{ + account: account, + actor: actor, + subject: subject + } do + resource = Fixtures.Resources.create_resource(account: account) + + actor_group = + Fixtures.Actors.create_group(account: account) + |> Fixtures.Actors.delete_group() + + Fixtures.Actors.create_membership(account: account, actor: actor, group: actor_group) + + Fixtures.Policies.create_policy( + account: account, + actor_group: actor_group, + resource: resource + ) + + assert fetch_and_authorize_resource_by_id(resource.id, subject) == {:error, :not_found} + end + test "does not return resources in other accounts", %{subject: subject} do resource = Fixtures.Resources.create_resource() assert fetch_and_authorize_resource_by_id(resource.id, subject) == {:error, :not_found} @@ -288,14 +383,17 @@ defmodule Domain.ResourcesTest do assert length(resources) == 1 assert hd(resources).authorized_by_policy.id == policy.id - Fixtures.Policies.create_policy( - account: account, - actor_group: actor_group, - resource: resource2 - ) + policy2 = + Fixtures.Policies.create_policy( + account: account, + actor_group: actor_group, + resource: resource2 + ) - assert {:ok, resources} = list_authorized_resources(subject) - assert length(resources) == 2 + assert {:ok, resources2} = list_authorized_resources(subject) + assert length(resources2) == 2 + + assert hd(resources2 -- resources).authorized_by_policy.id == policy2.id end test "returns authorized resources for account admin subject", %{