From e1dffc0c0c4fe386dcbffc2038e9f1ad90fcb3aa Mon Sep 17 00:00:00 2001
From: Zach Daniel <zach@zachdaniel.dev>
Date: Sat, 6 Jul 2024 13:41:58 -0400
Subject: [PATCH] fix: properly enforce tenancy on all mutative actions

---
 lib/ash/actions/create/create.ex   | 19 ++++++++++++++++++-
 lib/ash/actions/destroy/destroy.ex | 18 +++++++++++++++++-
 lib/ash/actions/update/update.ex   | 10 +++++++++-
 test/actions/multitenancy_test.exs | 19 +++++++++++++++++++
 4 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/lib/ash/actions/create/create.ex b/lib/ash/actions/create/create.ex
index 1538a83b..3eea7d84 100644
--- a/lib/ash/actions/create/create.ex
+++ b/lib/ash/actions/create/create.ex
@@ -509,6 +509,15 @@ defmodule Ash.Actions.Create do
   defp manage_relationships(other, _, _, _), do: other
 
   defp set_tenant(changeset) do
+    changeset =
+      case changeset.data do
+        %{__metadata__: %{tenant: tenant}} ->
+          Ash.Changeset.set_tenant(changeset, tenant)
+
+        _ ->
+          changeset
+      end
+
     if changeset.tenant &&
          Ash.Resource.Info.multitenancy_strategy(changeset.resource) == :attribute do
       attribute = Ash.Resource.Info.multitenancy_attribute(changeset.resource)
@@ -517,7 +526,15 @@ defmodule Ash.Actions.Create do
 
       Ash.Changeset.force_change_attribute(changeset, attribute, attribute_value)
     else
-      changeset
+      if is_nil(Ash.Resource.Info.multitenancy_strategy(changeset.resource)) ||
+           Ash.Resource.Info.multitenancy_global?(changeset.resource) || changeset.tenant do
+        changeset
+      else
+        Ash.Changeset.add_error(
+          changeset,
+          Ash.Error.Invalid.TenantRequired.exception(resource: changeset.resource)
+        )
+      end
     end
   end
 
diff --git a/lib/ash/actions/destroy/destroy.ex b/lib/ash/actions/destroy/destroy.ex
index fdf5dbed..9d0d718e 100644
--- a/lib/ash/actions/destroy/destroy.ex
+++ b/lib/ash/actions/destroy/destroy.ex
@@ -267,6 +267,14 @@ defmodule Ash.Actions.Destroy do
   end
 
   defp set_tenant(changeset) do
+    changeset =
+      case changeset.data do
+        %{__metadata__: %{tenant: tenant}} ->
+          Ash.Changeset.set_tenant(changeset, tenant)
+
+        _ ->
+          changeset
+      end
     if changeset.tenant &&
          Ash.Resource.Info.multitenancy_strategy(changeset.resource) == :attribute do
       attribute = Ash.Resource.Info.multitenancy_attribute(changeset.resource)
@@ -276,7 +284,15 @@ defmodule Ash.Actions.Destroy do
 
       Ash.Changeset.filter(changeset, [{attribute, attribute_value}])
     else
-      changeset
+      if is_nil(Ash.Resource.Info.multitenancy_strategy(changeset.resource)) ||
+           Ash.Resource.Info.multitenancy_global?(changeset.resource) || changeset.tenant do
+        changeset
+      else
+        Ash.Changeset.add_error(
+          changeset,
+          Ash.Error.Invalid.TenantRequired.exception(resource: changeset.resource)
+        )
+      end
     end
   end
 
diff --git a/lib/ash/actions/update/update.ex b/lib/ash/actions/update/update.ex
index e54dfbdc..9664d650 100644
--- a/lib/ash/actions/update/update.ex
+++ b/lib/ash/actions/update/update.ex
@@ -622,7 +622,15 @@ defmodule Ash.Actions.Update do
 
       Ash.Changeset.filter(changeset, expr(^ref(attribute) == ^attribute_value))
     else
-      changeset
+      if is_nil(Ash.Resource.Info.multitenancy_strategy(changeset.resource)) ||
+           Ash.Resource.Info.multitenancy_global?(changeset.resource) || changeset.tenant do
+        changeset
+      else
+        Ash.Changeset.add_error(
+          changeset,
+          Ash.Error.Invalid.TenantRequired.exception(resource: changeset.resource)
+        )
+      end
     end
   end
 end
diff --git a/test/actions/multitenancy_test.exs b/test/actions/multitenancy_test.exs
index eb951af7..35ceac80 100644
--- a/test/actions/multitenancy_test.exs
+++ b/test/actions/multitenancy_test.exs
@@ -311,6 +311,14 @@ defmodule Ash.Actions.MultitenancyTest do
       |> Ash.create!()
     end
 
+    test "an error is produced when a tenant is not specified" do
+      assert_raise Ash.Error.Invalid, ~r/require a tenant to be specified/, fn ->
+        User
+        |> Ash.Changeset.for_create(:create, %{})
+        |> Ash.create!()
+      end
+    end
+
     test "a record written to one tenant cannot be read from another", %{
       tenant1: tenant1,
       tenant2: tenant2
@@ -411,6 +419,17 @@ defmodule Ash.Actions.MultitenancyTest do
       assert User |> Ash.Query.set_tenant(tenant2) |> Ash.read!() == []
     end
 
+    test "updates require a tenant as well", %{tenant1: tenant1} do
+      assert_raise Ash.Error.Invalid, ~r/require a tenant to be specified/, fn ->
+        User
+        |> Ash.Changeset.for_create(:create, %{}, tenant: tenant1)
+        |> Ash.create!()
+        |> Map.update!(:__metadata__, &Map.delete(&1, :tenant))
+        |> Ash.Changeset.for_update(:update, %{})
+        |> Ash.update!()
+      end
+    end
+
     test "a record for a different tenant cant be updated from the other one", %{
       tenant1: tenant1,
       tenant2: tenant2