From 9711cf56c1879f7a139a5849cb3d1c8d1ce9f9c2 Mon Sep 17 00:00:00 2001
From: Brian Manifold <bmanifold@users.noreply.github.com>
Date: Thu, 12 Dec 2024 14:51:28 -0800
Subject: [PATCH] fix(portal): Fix update API endpoint for resources (#7493)

Why:

* The API endpoint for updating Resources was using
`Resources.fetch_resource_by_id_or_persistent_id`, however that function
was fetching all Resources, which included deleted Resources. In order
to prevent an API user from attempting to update a Resource that is
deleted, a new function was added to fetch active Resources only.

Fixes: #7492
---
 .../api/controllers/resource_controller.ex    |  2 +-
 .../controllers/resource_controller_test.exs  | 27 ++++++++++++++++++-
 elixir/apps/domain/lib/domain/resources.ex    | 20 ++++++++++++++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/elixir/apps/api/lib/api/controllers/resource_controller.ex b/elixir/apps/api/lib/api/controllers/resource_controller.ex
index 02bc10870..74ef30e05 100644
--- a/elixir/apps/api/lib/api/controllers/resource_controller.ex
+++ b/elixir/apps/api/lib/api/controllers/resource_controller.ex
@@ -92,7 +92,7 @@ defmodule API.ResourceController do
     subject = conn.assigns.subject
     attrs = set_param_defaults(params)
 
-    with {:ok, resource} <- Resources.fetch_resource_by_id_or_persistent_id(id, subject) do
+    with {:ok, resource} <- Resources.fetch_active_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)
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 58287d921..c1e2ce028 100644
--- a/elixir/apps/api/test/api/controllers/resource_controller_test.exs
+++ b/elixir/apps/api/test/api/controllers/resource_controller_test.exs
@@ -5,10 +5,14 @@ defmodule API.ResourceControllerTest do
   setup do
     account = Fixtures.Accounts.create_account()
     actor = Fixtures.Actors.create_actor(type: :api_client, account: account)
+    identity = Fixtures.Auth.create_identity(account: account, actor: actor)
+    subject = Fixtures.Auth.create_subject(identity: identity)
 
     %{
       account: account,
-      actor: actor
+      actor: actor,
+      identity: identity,
+      subject: subject
     }
   end
 
@@ -195,6 +199,27 @@ defmodule API.ResourceControllerTest do
       assert resp == %{"error" => %{"reason" => "Bad Request"}}
     end
 
+    test "returns not found when resource is deleted", %{
+      conn: conn,
+      account: account,
+      actor: actor,
+      subject: subject
+    } do
+      resource = Fixtures.Resources.create_resource(%{account: account})
+      Domain.Resources.delete_resource(resource, subject)
+
+      attrs = %{"name" => "Google"}
+
+      conn =
+        conn
+        |> authorize_conn(actor)
+        |> put_req_header("content-type", "application/json")
+        |> put("/resources/#{resource.id}", resource: attrs)
+
+      assert resp = json_response(conn, 404)
+      assert resp == %{"error" => %{"reason" => "Not Found"}}
+    end
+
     test "updates a resource", %{conn: conn, account: account, actor: actor} do
       resource = Fixtures.Resources.create_resource(%{account: account})
 
diff --git a/elixir/apps/domain/lib/domain/resources.ex b/elixir/apps/domain/lib/domain/resources.ex
index c8081f943..a11dd27d4 100644
--- a/elixir/apps/domain/lib/domain/resources.ex
+++ b/elixir/apps/domain/lib/domain/resources.ex
@@ -43,6 +43,26 @@ defmodule Domain.Resources do
     end
   end
 
+  def fetch_active_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.not_deleted()
+      |> 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()),