From b03eff9298ffa963ebc951e3476b72fa795fd983 Mon Sep 17 00:00:00 2001 From: "John W. Darrington" Date: Mon, 23 Sep 2024 20:10:22 -0400 Subject: [PATCH] permissions overhaul to use bodyguard, keep people from doing things they shouldn't --- server/lib/ingest/destinations/destination.ex | 17 +++++++++++ server/lib/ingest/projects/project.ex | 21 ++++++++++++++ .../components_live/destination_form.ex | 28 +++++++++++-------- .../live/dashboard_live/destinations_live.ex | 14 ++++++++-- .../live/dashboard_live/projects_live.ex | 14 ++++++++-- server/mix.exs | 1 + server/mix.lock | 1 + 7 files changed, 81 insertions(+), 15 deletions(-) diff --git a/server/lib/ingest/destinations/destination.ex b/server/lib/ingest/destinations/destination.ex index 6a1647e..531f9e7 100644 --- a/server/lib/ingest/destinations/destination.ex +++ b/server/lib/ingest/destinations/destination.ex @@ -4,6 +4,8 @@ defmodule Ingest.Destinations.Destination do Typically this reflects the HTTP uploads, but can hopefully be configured in the future to work with the high-speed UDP file transers as well. """ + @behaviour Bodyguard.Policy + alias Ingest.Destinations.LakeFSConfig alias Ingest.Destinations.TemporaryConfig alias Ingest.Destinations.AzureConfig @@ -64,6 +66,21 @@ defmodule Ingest.Destinations.Destination do |> cast_embed(:temporary_config, required: false) |> validate_required([:name, :type]) end + + def authorize(:create_destination, _user), do: :ok + + # Admins can do anything + def authorize(action, %{roles: :admin} = _user, _destination) + when action in [:update_destination, :delete_destination], + do: :ok + + # Users can manage their own projects + def authorize(action, %{id: user_id} = _user, %{inserted_by: user_id} = _destination) + when action in [:update_destination, :delete_destination], + do: :ok + + # Otherwise, denied + def authorize(_, _, _), do: :error end defmodule Ingest.Destinations.S3Config do diff --git a/server/lib/ingest/projects/project.ex b/server/lib/ingest/projects/project.ex index 3522261..e32db6e 100644 --- a/server/lib/ingest/projects/project.ex +++ b/server/lib/ingest/projects/project.ex @@ -1,4 +1,9 @@ defmodule Ingest.Projects.Project do + @moduledoc """ + Project represents a group of individuals with common access to destinations and templates. + """ + @behaviour Bodyguard.Policy + use Ecto.Schema import Ecto.Changeset alias Ingest.Destinations.Destination @@ -36,4 +41,20 @@ defmodule Ingest.Projects.Project do |> cast(attrs, [:name, :description, :inserted_by]) |> validate_required([:name, :description]) end + + # Anyone can make a project + def authorize(:create_project, _user), do: :ok + + # Admins can do anything + def authorize(action, %{roles: :admin} = _user, _project) + when action in [:update_project, :delete_project], + do: :ok + + # Users can manage their own projects + def authorize(action, %{id: user_id} = _user, %{inserted_by: user_id} = _project) + when action in [:update_project, :delete_project], + do: :ok + + # Otherwise, denied + def authorize(_, _, _), do: :error end diff --git a/server/lib/ingest_web/components_live/destination_form.ex b/server/lib/ingest_web/components_live/destination_form.ex index f2d7902..ed88c4b 100644 --- a/server/lib/ingest_web/components_live/destination_form.ex +++ b/server/lib/ingest_web/components_live/destination_form.ex @@ -442,17 +442,23 @@ defmodule IngestWeb.LiveComponents.DestinationForm do collect_classifications(destination_params) ) - case Ingest.Destinations.update_destination(socket.assigns.destination, destination_params) do - {:ok, destination} -> - notify_parent({:saved, destination}) - - {:noreply, - socket - |> put_flash(:info, "Destination updated successfully") - |> push_patch(to: socket.assigns.patch)} - - {:error, %Ecto.Changeset{} = changeset} -> - {:noreply, assign_form(socket, changeset)} + with :ok <- + Bodyguard.permit( + Ingest.Destinations.Destination, + :update_destination, + socket.assigns.current_user, + socket.assigns.destination + ), + {:ok, destination} <- + Ingest.Destinations.update_destination(socket.assigns.destination, destination_params) do + notify_parent({:saved, destination}) + + {:noreply, + socket + |> put_flash(:info, "Destination updated successfully") + |> push_patch(to: socket.assigns.patch)} + else + _ -> {:noreply, assign_form(socket, socket.assigns.destination)} end end diff --git a/server/lib/ingest_web/live/dashboard_live/destinations_live.ex b/server/lib/ingest_web/live/dashboard_live/destinations_live.ex index 09ae882..eabd828 100644 --- a/server/lib/ingest_web/live/dashboard_live/destinations_live.ex +++ b/server/lib/ingest_web/live/dashboard_live/destinations_live.ex @@ -192,8 +192,18 @@ defmodule IngestWeb.DestinationsLive do @impl true def handle_event("delete", %{"id" => id}, socket) do destination = Ingest.Destinations.get_own_destination!(socket.assigns.current_user, id) - {:ok, _} = Ingest.Destinations.delete_destination(destination) - {:noreply, stream_delete(socket, :destinations, destination)} + with :ok <- + Bodyguard.permit( + Ingest.Destinations.Destination, + :delete_destination, + socket.assigns.current_user, + destination + ), + {:ok, _} <- Ingest.Destinations.delete_destination(destination) do + {:noreply, stream_delete(socket, :destinations, destination)} + else + _ -> {:noreply, socket |> put_flash(:error, "Unable to delete destination")} + end end end diff --git a/server/lib/ingest_web/live/dashboard_live/projects_live.ex b/server/lib/ingest_web/live/dashboard_live/projects_live.ex index 2a1f194..ed1a5ef 100644 --- a/server/lib/ingest_web/live/dashboard_live/projects_live.ex +++ b/server/lib/ingest_web/live/dashboard_live/projects_live.ex @@ -175,8 +175,18 @@ defmodule IngestWeb.ProjectsLive do @impl true def handle_event("delete", %{"id" => id, "count" => count}, socket) do project = Ingest.Projects.get_project!(id) - {:ok, _} = Ingest.Projects.delete_project(project) - {:noreply, stream_delete(socket, :projects, {project, count})} + with :ok <- + Bodyguard.permit( + Ingest.Projects.Project, + :delete_project, + socket.assigns.current_user, + project + ), + {:ok, _} <- Ingest.Projects.delete_project(project) do + {:noreply, stream_delete(socket, :projects, {project, count})} + else + _ -> {:noreply, socket |> put_flash(:error, "Unable to delete project")} + end end end diff --git a/server/mix.exs b/server/mix.exs index e52b882..4dc92fc 100644 --- a/server/mix.exs +++ b/server/mix.exs @@ -69,6 +69,7 @@ defmodule Ingest.MixProject do {:oban, "~> 2.17"}, {:backpex, "~> 0.6.0"}, {:earmark, "~> 1.4"}, + {:bodyguard, "~> 2.4"}, {:credo, "~> 1.7", only: [:dev, :test], runtime: false}, {:mix_audit, "~> 2.1", only: [:dev, :test], runtime: false} ] diff --git a/server/mix.lock b/server/mix.lock index 19761e0..918412d 100644 --- a/server/mix.lock +++ b/server/mix.lock @@ -2,6 +2,7 @@ "argon2_elixir": {:hex, :argon2_elixir, "4.0.0", "7f6cd2e4a93a37f61d58a367d82f830ad9527082ff3c820b8197a8a736648941", [:make, :mix], [{:comeonin, "~> 5.3", [hex: :comeonin, repo: "hexpm", optional: false]}, {:elixir_make, "~> 0.6", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "f9da27cf060c9ea61b1bd47837a28d7e48a8f6fa13a745e252556c14f9132c7f"}, "backpex": {:hex, :backpex, "0.6.0", "f8137e56a57fa7e59f6a12b986c3d8d7385b50d9ac7cd67847ade4be800092f8", [:mix], [{:ecto_sql, "~> 3.6", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:gettext, "~> 0.18", [hex: :gettext, repo: "hexpm", optional: false]}, {:jason, "~> 1.2", [hex: :jason, repo: "hexpm", optional: false]}, {:money, "~> 1.13.1", [hex: :money, repo: "hexpm", optional: false]}, {:number, "~> 1.0.3", [hex: :number, repo: "hexpm", optional: false]}, {:phoenix, "~> 1.7.6", [hex: :phoenix, repo: "hexpm", optional: false]}, {:phoenix_ecto, "~> 4.4", [hex: :phoenix_ecto, repo: "hexpm", optional: false]}, {:phoenix_html, "~> 4.1.1", [hex: :phoenix_html, repo: "hexpm", optional: false]}, {:phoenix_html_helpers, "~> 1.0", [hex: :phoenix_html_helpers, repo: "hexpm", optional: false]}, {:phoenix_live_view, "~> 0.20.0", [hex: :phoenix_live_view, repo: "hexpm", optional: false]}, {:postgrex, ">= 0.0.0", [hex: :postgrex, repo: "hexpm", optional: false]}], "hexpm", "f01e5ca63de47ac39693dfcdc706879ff0cb407f3be958b215a2b06b32874d9c"}, "bandit": {:hex, :bandit, "1.5.7", "6856b1e1df4f2b0cb3df1377eab7891bec2da6a7fd69dc78594ad3e152363a50", [:mix], [{:hpax, "~> 1.0.0", [hex: :hpax, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:thousand_island, "~> 1.0", [hex: :thousand_island, repo: "hexpm", optional: false]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "f2dd92ae87d2cbea2fa9aa1652db157b6cba6c405cb44d4f6dd87abba41371cd"}, + "bodyguard": {:hex, :bodyguard, "2.4.3", "5faec1c7a346b3a6bac0b63aa3b2ae05b8dab6a5e4f8384e21577710498f8b56", [:mix], [{:plug, "~> 1.4", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "5bb6bcc04871e18d97da5822a4d5d25ec38158447cb767b2eea3e2eb99cdc351"}, "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"}, "cachex": {:hex, :cachex, "3.6.0", "14a1bfbeee060dd9bec25a5b6f4e4691e3670ebda28c8ba2884b12fe30b36bf8", [:mix], [{:eternal, "~> 1.2", [hex: :eternal, repo: "hexpm", optional: false]}, {:jumper, "~> 1.0", [hex: :jumper, repo: "hexpm", optional: false]}, {:sleeplocks, "~> 1.1", [hex: :sleeplocks, repo: "hexpm", optional: false]}, {:unsafe, "~> 1.0", [hex: :unsafe, repo: "hexpm", optional: false]}], "hexpm", "ebf24e373883bc8e0c8d894a63bbe102ae13d918f790121f5cfe6e485cc8e2e2"}, "castore": {:hex, :castore, "1.0.8", "dedcf20ea746694647f883590b82d9e96014057aff1d44d03ec90f36a5c0dc6e", [:mix], [], "hexpm", "0b2b66d2ee742cb1d9cb8c8be3b43c3a70ee8651f37b75a8b982e036752983f1"},