fix: don't update tenant on update, instead enforce it

the attribute strategy allowed for overwriting the multitenant attribute
on update. In practice, this can't really happen using any standard pattern
because any record to be updated is read with the tenant context, but it still
represents a small risk (and `schema` based multitenancy would enforce it in this
way anyway, so this is more consistent).
This commit is contained in:
Zach Daniel 2024-06-05 10:43:38 -04:00
parent 51be60cb4b
commit e4980d55ba
4 changed files with 32 additions and 2 deletions

View file

@ -171,6 +171,8 @@ defmodule Ash.Actions.Destroy do
{:ok, new_data, _} ->
changeset = %{changeset | data: new_data}
changeset = set_tenant(changeset)
if changeset.action.manual do
{mod, action_opts} = changeset.action.manual
@ -264,6 +266,20 @@ defmodule Ash.Actions.Destroy do
end
end
defp set_tenant(changeset) do
if changeset.tenant &&
Ash.Resource.Info.multitenancy_strategy(changeset.resource) == :attribute do
attribute = Ash.Resource.Info.multitenancy_attribute(changeset.resource)
{m, f, a} = Ash.Resource.Info.multitenancy_parse_attribute(changeset.resource)
attribute_value = apply(m, f, [changeset.to_tenant | a])
Ash.Changeset.filter(changeset, [{attribute, attribute_value}])
else
changeset
end
end
defp validate_manual_action_return_result!({:ok, %resource{}} = result, resource, _) do
result
end

View file

@ -5,6 +5,7 @@ defmodule Ash.Actions.Update do
require Ash.Tracer
require Logger
import Ash.Expr
@spec run(Ash.Domain.t(), Ash.Resource.record(), Ash.Resource.Actions.action(), Keyword.t()) ::
{:ok, Ash.Resource.record(), list(Ash.Notifier.Notification.t())}
@ -631,7 +632,7 @@ defmodule Ash.Actions.Update do
{m, f, a} = Ash.Resource.Info.multitenancy_parse_attribute(changeset.resource)
attribute_value = apply(m, f, [changeset.to_tenant | a])
Ash.Changeset.force_change_attribute(changeset, attribute, attribute_value)
Ash.Changeset.filter(changeset, expr(^ref(attribute) == ^attribute_value))
else
changeset
end

View file

@ -151,7 +151,7 @@ defmodule Ash.Changeset do
%Ash.Filter{expression: nil} ->
empty()
true ->
_ ->
concat("filter: ", to_doc(changeset.filter, opts))
end

View file

@ -380,6 +380,19 @@ defmodule Ash.Actions.MultitenancyTest do
assert User |> Ash.Query.set_tenant(tenant2) |> Ash.read!() == []
end
test "a record for a different tenant cant be updated from the other one", %{
tenant1: tenant1,
tenant2: tenant2
} do
assert_raise Ash.Error.Invalid, ~r/Attempted to update stale record/, fn ->
User
|> Ash.Changeset.for_create(:create, %{}, tenant: tenant1)
|> Ash.create!()
|> Ash.Changeset.for_update(:update, %{name: "new name"}, tenant: tenant2)
|> Ash.update!()
end
end
test "a record can be destroyed in a tenant", %{tenant1: tenant1} do
User
|> Ash.Changeset.for_create(:create, %{}, tenant: tenant1)