fix: app env matching in controllers (and logo env foreign key) (#534)

apps/lenra/lib/lenra/apps.ex

@@ -158,6 +158,16 @@ defmodule Lenra.Apps do
  Repo.fetch(Environment, env_id)
  end
 
+ def fetch_app_env(app_id, env_id) do
+ Environment
+ |> Repo.get_by(id: env_id, application_id: app_id)
+ |> Repo.preload(:application)
+ |> case do
+ nil -> BusinessError.no_env_found_tuple()
+ env -> {:ok, env}
+ end
+ end
+
  def create_env(application_id, creator_id, params) do
  Ecto.Multi.new()
  |> Ecto.Multi.put(:inserted_application, %{id: application_id})
@@ -670,7 +680,7 @@ defmodule Lenra.Apps do
  |> Repo.transaction()
  end
 
- defp upsert_logo(transaction, %{inserted_image: image, old_logo: old_logo} = state, %{"app_id" => app_id} = params) do
+ defp upsert_logo(transaction, %{inserted_image: image, old_logo: old_logo}, %{"app_id" => app_id} = params) do
  case old_logo do
  nil ->
  transaction.insert(Logo.new(app_id, params["env_id"], %{image_id: image.id}))

apps/lenra/priv/repo/migrations/20240121000440_fix_logo_foreign_key.exs

@@ -0,0 +1,11 @@
+defmodule Lenra.Repo.Migrations.FixLogoForeignKey do
+ use Ecto.Migration
+
+ def change do
+ drop(constraint(:logos, "logos_environment_id_fkey"))
+
+ alter table(:logos) do
+ modify(:environment_id, references(:environments, on_delete: :delete_all), null: true)
+ end
+ end
+end

apps/lenra_web/lib/lenra_web/controllers/environment_controller.ex

@@ -17,6 +17,16 @@ defmodule LenraWeb.EnvsController do
  end
  end
 
+ defp get_app_env_and_allow(conn, %{"app_id" => app_id_str, "env_id" => env_id_str}) do
+ with {app_id, _} <- Integer.parse(app_id_str),
+ {env_id, _} <- Integer.parse(env_id_str),
+ {:ok, env} <- Apps.fetch_app_env(app_id, env_id),
+ app <- Map.get(env, :application),
+ :ok <- allow(conn, %{app: app, env: env}) do
+ {:ok, app, env}
+ end
+ end
+
  def index(conn, params) do
  with {:ok, app} <- get_app_and_allow(conn, params) do
  conn
@@ -34,8 +44,7 @@ defmodule LenraWeb.EnvsController do
  end
 
  def update(conn, %{"env_id" => env_id, "is_public" => true} = params) do
- with {:ok, app} <- get_app_and_allow(conn, params),
- {:ok, env} <- Apps.fetch_env(env_id),
+ with {:ok, app, env} <- get_app_env_and_allow(conn, params),
  %Subscription{} = _subscription <- Subscriptions.get_subscription_by_app_id(app.id),
  {:ok, %{updated_env: env}} <- Apps.update_env(env, params) do
  conn
@@ -47,8 +56,7 @@ defmodule LenraWeb.EnvsController do
  end
 
  def update(conn, %{"env_id" => env_id} = params) do
- with {:ok, _app} <- get_app_and_allow(conn, params),
- {:ok, env} <- Apps.fetch_env(env_id),
+ with {:ok, _app, env} <- get_app_env_and_allow(conn, params),
  {:ok, %{updated_env: env}} <- Apps.update_env(env, params) do
  conn
  |> reply(env)
@@ -59,12 +67,19 @@ end
 defmodule LenraWeb.EnvsController.Policy do
  alias Lenra.Accounts.User
  alias Lenra.Apps.App
+ alias Lenra.Apps.Environment
  alias Lenra.Subscriptions.Subscription
 
  @impl Bouncer.Policy
  def authorize(:index, %User{id: user_id}, %App{creator_id: user_id}), do: true
  def authorize(:create, %User{id: user_id}, %App{creator_id: user_id}), do: true
- def authorize(:update, %User{id: user_id}, %App{creator_id: user_id}), do: true
+
+ def authorize(:update, %User{id: user_id}, %{
+ app: %App{id: app_id, creator_id: user_id},
+ env: %Environment{application_id: app_id}
+ }),
+ do: true
+
  def authorize(:update, %App{id: app_id}, %Subscription{application_id: app_id}), do: true
 
  # credo:disable-for-next-line Credo.Check.Readability.StrictModuleLayout

apps/lenra_web/lib/lenra_web/controllers/logo_controller.ex

@@ -6,6 +6,24 @@ defmodule LenraWeb.LogosController do
 
  alias Lenra.Apps
 
+ defp get_app_and_allow(conn, %{"app_id" => app_id_str}) do
+ with {app_id, _} <- Integer.parse(app_id_str),
+ {:ok, app} <- Apps.fetch_app(app_id),
+ :ok <- allow(conn, app) do
+ {:ok, app}
+ end
+ end
+
+ defp get_app_env_and_allow(conn, %{"app_id" => app_id_str, "env_id" => env_id_str}) do
+ with {app_id, _} <- Integer.parse(app_id_str),
+ {env_id, _} <- Integer.parse(env_id_str),
+ {:ok, env} <- Apps.fetch_app_env(app_id, env_id),
+ app <- Map.get(env, :application),
+ :ok <- allow(conn, %{app: app, env: env}) do
+ {:ok, app, env}
+ end
+ end
+
  def get_image_content(conn, %{"image_id" => image_id}) do
  {:ok, image} = Apps.fetch_image(image_id)
 
@@ -14,22 +32,19 @@ defmodule LenraWeb.LogosController do
  |> Plug.Conn.send_resp(:ok, image.data)
  end
 
- @spec put_logo(any(), map()) :: any()
- def put_logo(conn, %{"app_id" => _app_id, "env_id" => env_id} = params) do
- with {:ok, env} <- Apps.fetch_env(env_id),
- :ok <- allow(conn, env),
+ def put_logo(conn, %{"app_id" => _app_id, "env_id" => _env_id} = params) do
+ with {:ok, app, env} <- get_app_env_and_allow(conn, params),
  user <- LenraWeb.Auth.current_resource(conn),
  decoded_data <- decode_data(params),
  {:ok, %{new_logo: logo}} <-
- Apps.set_logo(user.id, Map.merge(decoded_data, %{"app_id" => env.application_id, "env_id" => env.id})) do
+ Apps.set_logo(user.id, Map.merge(decoded_data, %{"app_id" => app.id, "env_id" => env.id})) do
  conn
  |> reply(logo)
  end
  end
 
- def put_logo(conn, %{"app_id" => app_id} = params) do
- with {:ok, app} <- Apps.fetch_app(app_id),
- :ok <- allow(conn, app),
+ def put_logo(conn, %{"app_id" => _app_id} = params) do
+ with {:ok, app} <- get_app_and_allow(conn, params),
  user <- LenraWeb.Auth.current_resource(conn),
  decoded_data <- decode_data(params),
  {:ok, %{new_logo: logo}} <- Apps.set_logo(user.id, Map.merge(decoded_data, %{"app_id" => app.id})) do
@@ -49,9 +64,14 @@ defmodule LenraWeb.LogosController.Policy do
  alias Lenra.Apps.Environment
 
  @impl Bouncer.Policy
- def authorize(:put_logo, %User{id: user_id}, %Environment{creator_id: user_id}), do: true
  def authorize(:put_logo, %User{id: user_id}, %App{creator_id: user_id}), do: true
 
+ def authorize(:put_logo, %User{id: user_id}, %{
+ app: %App{id: app_id, creator_id: user_id},
+ env: %Environment{application_id: app_id}
+ }),
+ do: true
+
  # credo:disable-for-next-line Credo.Check.Readability.StrictModuleLayout
  use LenraWeb.Policy.Default
 end

apps/lenra_web/test/controllers/application_main_env_controller_test.exs

@@ -8,19 +8,6 @@ defmodule LenraWeb.ApplicationMainEnvControllerTest do
  {:ok, conn: conn}
  end
 
- def create_app(conn) do
- conn =
- post(conn, Routes.apps_path(conn, :create), %{
- "name" => "test",
- "color" => "ffffff",
- "icon" => 12
- })
-
- app = json_response(conn, 200)
-
- %{conn: conn, app: app}
- end
-
  describe "index" do
  test "application main env controller not authenticated", %{conn: conn} do
  conn = get(conn, Routes.application_main_env_path(conn, :index, 0))

apps/lenra_web/test/controllers/build_controller_test.exs

@@ -8,14 +8,6 @@ defmodule LenraWeb.BuildControllerTest do
  {:ok, conn: conn}
  end
 
- defp create_app(conn) do
- post(conn, Routes.apps_path(conn, :create), %{
- "name" => "test",
- "color" => "ffffff",
- "icon" => 12
- })
- end
-
  defp create_build(conn, app_id) do
  post(
  conn,
@@ -31,8 +23,7 @@ defmodule LenraWeb.BuildControllerTest do
  end
 
  defp create_app_and_build(conn!) do
- conn! = create_app(conn!)
- assert app = json_response(conn!, 200)
+ %{conn: conn!, app: app} = create_app(conn!)
 
  conn! = create_build(conn!, app["id"])
  assert build = json_response(conn!, 200)
@@ -91,8 +82,7 @@ defmodule LenraWeb.BuildControllerTest do
  describe "create" do
  @tag auth_users_with_cgs: [:dev, :user, :dev, :admin]
  test "build controller authenticated", %{users: [creator!, user, other_dev, admin]} do
- creator! = create_app(creator!)
- assert app = json_response(creator!, 200)
+ %{conn: creator!, app: app} = create_app(creator!)
 
  creator! = create_build(creator!, app["id"])
  admin = create_build(admin, app["id"])
@@ -131,9 +121,7 @@ defmodule LenraWeb.BuildControllerTest do
 
  @tag auth_user_with_cgs: :dev
  test "build controller authenticated but invalid params", %{conn: conn!} do
- conn! = create_app(conn!)
-
- assert app = json_response(conn!, 200)
+ %{conn: conn!, app: app} = create_app(conn!)
 
  conn! =
  post(

apps/lenra_web/test/controllers/environment_controller_test.exs

@@ -8,14 +8,6 @@ defmodule LenraWeb.EnvironmentControllerTest do
  {:ok, conn: conn}
  end
 
- defp create_app(conn) do
- post(conn, Routes.apps_path(conn, :create), %{
- "name" => "test",
- "color" => "ffffff",
- "icon" => 12
- })
- end
-
  describe "index" do
  test "environment controller not authenticated", %{conn: conn} do
  conn = get(conn, Routes.envs_path(conn, :index, 0))
@@ -29,9 +21,7 @@ defmodule LenraWeb.EnvironmentControllerTest do
 
  @tag auth_users_with_cgs: [:dev, :user, :dev, :admin]
  test "get environment check authorizations", %{users: [creator!, user, other_dev, admin]} do
- creator! = create_app(creator!)
-
- assert app = json_response(creator!, 200)
+ %{conn: creator!, app: app} = create_app(creator!)
 
  creator! =
  post(creator!, Routes.envs_path(creator!, :create, app["id"]), %{
@@ -103,8 +93,29 @@ defmodule LenraWeb.EnvironmentControllerTest do
  describe "update" do
  @tag auth_users_with_cgs: [:dev, :user, :dev, :admin]
  test "environment controller authenticated", %{users: [creator!, user!, other_dev!, admin!]} do
- creator! = create_app(creator!)
- assert app = json_response(creator!, 200)
+ %{conn: creator!, app: app} = create_app(creator!)
+ %{conn: other_dev!, app: other_app} = create_app(other_dev!, "test2")
+
+ [env] = json_response(get(creator!, Routes.envs_path(creator!, :index, app["id"])), 200)
+
+ update_env_path = Routes.envs_path(creator!, :update, app["id"], env["id"])
+ update_other_app_env_path = Routes.envs_path(other_dev!, :update, other_app["id"], env["id"])
+
+ public_body = %{
+ "is_public" => true
+ }
+
+ private_body = %{
+ "is_public" => false
+ }
+
+ creator! = patch(creator!, update_env_path, public_body)
+
+ assert %{"message" => "You need a subscirption", "reason" => "subscription_required"} =
+ json_response(creator!, 402)
+
+ assert [%{"is_public" => false}] =
+ json_response(get(creator!, Routes.envs_path(creator!, :index, app["id"])), 200)
 
  subscription =
  Subscription.new(%{
@@ -116,50 +127,38 @@ defmodule LenraWeb.EnvironmentControllerTest do
 
  Repo.insert(subscription)
 
- [env] = json_response(get(creator!, Routes.envs_path(creator!, :index, app["id"])), 200)
-
- update_env_path = Routes.envs_path(creator!, :update, app["id"], env["id"])
-
- creator! =
- patch(creator!, update_env_path, %{
- "is_public" => true
- })
-
+ creator! = patch(creator!, update_env_path, public_body)
  assert [%{"is_public" => true}] = json_response(get(creator!, Routes.envs_path(creator!, :index, app["id"])), 200)
 
- patch(admin!, update_env_path, %{
- "is_public" => false
- })
+ patch(admin!, update_env_path, private_body)
 
  assert [%{"is_public" => false}] =
  json_response(get(creator!, Routes.envs_path(creator!, :index, app["id"])), 200)
 
- user! =
- patch(user!, update_env_path, %{
- "is_public" => true
- })
+ user! = patch(user!, update_env_path, public_body)
+ assert %{"message" => "Forbidden", "reason" => "forbidden"} = json_response(user!, 403)
 
  assert [%{"is_public" => false}] =
  json_response(get(creator!, Routes.envs_path(creator!, :index, app["id"])), 200)
 
- other_dev! =
- patch(other_dev!, update_env_path, %{
- "is_public" => true
- })
+ other_dev! = patch(other_dev!, update_env_path, public_body)
+ assert %{"message" => "Forbidden", "reason" => "forbidden"} = json_response(other_dev!, 403)
 
  assert [%{"is_public" => false}] =
  json_response(get(creator!, Routes.envs_path(creator!, :index, app["id"])), 200)
 
- assert %{"message" => "Forbidden", "reason" => "forbidden"} = json_response(user!, 403)
- assert %{"message" => "Forbidden", "reason" => "forbidden"} = json_response(other_dev!, 403)
+ assert %{"message" => "Environment not found", "reason" => "no_env_found"} =
+ json_response(patch(creator!, update_other_app_env_path, private_body), 404)
+
+ assert %{"message" => "Environment not found", "reason" => "no_env_found"} =
+ json_response(patch(other_dev!, update_other_app_env_path, private_body), 404)
  end
  end
 
  describe "create" do
  @tag auth_users_with_cgs: [:dev, :user, :dev, :admin]
  test "environment controller authenticated", %{users: [creator!, user!, other_dev!, admin!]} do
- creator! = create_app(creator!)
- assert app = json_response(creator!, 200)
+ %{conn: creator!, app: app} = create_app(creator!)
 
  create_env_path = Routes.envs_path(creator!, :create, app["id"])
 
@@ -195,9 +194,7 @@ defmodule LenraWeb.EnvironmentControllerTest do
 
  @tag auth_user_with_cgs: :dev
  test "environment controller authenticated but invalid params", %{conn: conn!} do
- conn! = create_app(conn!)
-
- assert app = json_response(conn!, 200)
+ %{conn: conn!, app: app} = create_app(conn!)
 
  conn! =
  post(conn!, Routes.envs_path(conn!, :create, app["id"]), %{