feat: allow projects editors to provision and merge sandboxes (#4424)

* feat: allow editors to provision and merge sandboxes (#4384)

Expand sandbox permissions so that editors (not just admin/owner) on a
project can fork new sandboxes and merge them back. Merge authorization
now checks the user's role on the target project rather than only the
source sandbox.

- Add :editor to provision_sandbox allowed roles
- Add :merge_sandbox policy action checking editor+ on target project
- Update check_manage_permissions so merge uses editor+ on root
- Filter merge target options by user's role on each target
- Add server-side merge authorization check in confirm-merge handler

* chore: add changelog entry for #4384

* test: cover nil branch in user_role_on_project/2

Add a no-membership sandbox to the target filtering test so the nil
return path is exercised.

* refactor: clean up check_manage_permissions logic

Remove unreachable is_superuser check in the else branch and derive
is_root_editor_plus from is_root_owner_or_admin to avoid a redundant
scan of project_users.

Author: elias-ba

CHANGELOG.md

@@ -19,6 +19,8 @@ and this project adheres to
 
 ### Changed
 
+- Editors can now provision and merge sandboxes; merge checks editor+ role on
+  the target project [#4384](https://github.com/OpenFn/lightning/issues/4384)
 - Show specific workflow names in sandbox merge dialog when target project has
   diverged, instead of generic warning message
   [#4001](https://github.com/OpenFn/lightning/issues/4001)

lib/lightning/policies/sandboxes.ex

@@ -5,6 +5,8 @@ defmodule Lightning.Policies.Sandboxes do
   Sandboxes have different authorization rules than regular projects:
   - Sandbox owners/admins can manage their own sandboxes
   - Root project owners/admins can manage any sandbox in their workspace
+  - Editors (and above) on the root project can merge sandboxes
+  - Editors (and above) on the parent project can provision sandboxes
   - Superusers can manage any sandbox anywhere
   """
   @behaviour Bodyguard.Policy
@@ -17,6 +19,7 @@ defmodule Lightning.Policies.Sandboxes do
           :delete_sandbox
           | :update_sandbox
           | :provision_sandbox
+          | :merge_sandbox
 
   @doc """
   Authorize sandbox operations based on user role and project hierarchy.
@@ -31,18 +34,31 @@ defmodule Lightning.Policies.Sandboxes do
 
   ### `:provision_sandbox`
   User can create sandboxes if they are:
-  - Owner/admin of the parent project they're creating the sandbox under
+  - Editor/admin/owner of the parent project they're creating the sandbox under
+
+  ### `:merge_sandbox`
+  User can merge a sandbox into a target project if they are:
+  - Editor/admin/owner on the target project
+  - Superuser
 
   ## Parameters
   - `action` - The action being attempted
   - `user` - The user attempting the action
-  - `project` - The sandbox project (for delete/update) or parent project (for provision)
+  - `project` - The sandbox project (for delete/update), parent project (for provision),
+    or target project (for merge)
   """
   @spec authorize(actions(), User.t(), Project.t()) :: boolean
 
   def authorize(:provision_sandbox, %User{} = user, %Project{} = parent_project) do
     case Projects.get_project_user_role(user, parent_project) do
-      role when role in [:owner, :admin] -> true
+      role when role in [:owner, :admin, :editor] -> true
+      _ -> user.role == :superuser
+    end
+  end
+
+  def authorize(:merge_sandbox, %User{} = user, %Project{} = target_project) do
+    case Projects.get_project_user_role(user, target_project) do
+      role when role in [:owner, :admin, :editor] -> true
       _ -> user.role == :superuser
     end
   end
@@ -79,14 +95,24 @@ defmodule Lightning.Policies.Sandboxes do
             binary() => %{update: boolean(), delete: boolean(), merge: boolean()}
           }
   def check_manage_permissions(sandboxes, %User{} = user, root_project) do
-    has_root_privileges =
-      user.role == :superuser or
+    is_superuser = user.role == :superuser
+
+    is_root_owner_or_admin =
+      Enum.any?(
+        root_project.project_users,
+        &(&1.user_id == user.id and &1.role in [:owner, :admin])
+      )
+
+    is_root_editor_plus =
+      is_root_owner_or_admin or
         Enum.any?(
           root_project.project_users,
-          &(&1.user_id == user.id and &1.role in [:owner, :admin])
+          &(&1.user_id == user.id and &1.role == :editor)
         )
 
-    if has_root_privileges do
+    has_full_privileges = is_superuser or is_root_owner_or_admin
+
+    if has_full_privileges do
       Map.new(sandboxes, &{&1.id, %{update: true, delete: true, merge: true}})
     else
       Map.new(sandboxes, fn sandbox ->
@@ -96,14 +122,11 @@ defmodule Lightning.Policies.Sandboxes do
             &(&1.user_id == user.id and &1.role in [:owner, :admin])
           )
 
-        # Merge follows the same rule as update
-        # Today update/delete/merge share the same rule.
-        # If they ever diverge, split here (e.g., compute separate booleans).
         {sandbox.id,
          %{
            update: is_owner_or_admin_here?,
            delete: is_owner_or_admin_here?,
-           merge: is_owner_or_admin_here?
+           merge: is_root_editor_plus
          }}
       end)
     end

lib/lightning/projects/sandboxes.ex

@@ -23,7 +23,8 @@ defmodule Lightning.Projects.Sandboxes do
 
   ## Authorization
 
-  * **Provisioning**: Requires `:owner` or `:admin` role on the parent project or superuser
+  * **Provisioning**: Requires `:editor`, `:admin`, or `:owner` role on the parent project, or superuser
+  * **Merge**: Requires `:editor`, `:admin`, or `:owner` role on the target project, or superuser
   * **Updates/Deletion**: Requires `:owner` or `:admin` role on the sandbox itself,
                           or `:owner` or `:admin` on the root project, or superuser
 
@@ -88,7 +89,7 @@ defmodule Lightning.Projects.Sandboxes do
 
   ## Parameters
   * `parent` - Project to clone from
-  * `actor` - User creating the sandbox (needs `:owner` or `:admin` role on parent)
+  * `actor` - User creating the sandbox (needs `:editor`, `:admin`, or `:owner` role on parent)
   * `attrs` - Creation attributes (see `t:provision_attrs/0`)
 
   ## Returns

lib/lightning_web/live/sandbox_live/index.ex

@@ -2,6 +2,7 @@ defmodule LightningWeb.SandboxLive.Index do
   use LightningWeb, :live_view
 
   alias Ecto.Changeset
+  alias Lightning.Policies.Permissions
   alias Lightning.Projects
   alias Lightning.Projects.MergeProjects
   alias Lightning.Projects.ProjectLimiter
@@ -276,9 +277,24 @@ defmodule LightningWeb.SandboxLive.Index do
             |> noreply()
 
           target ->
-            source
-            |> perform_merge(target, actor)
-            |> handle_merge_result(socket, source, target, root_project, actor)
+            if Permissions.can?(
+                 :sandboxes,
+                 :merge_sandbox,
+                 actor,
+                 target
+               ) do
+              source
+              |> perform_merge(target, actor)
+              |> handle_merge_result(socket, source, target, root_project, actor)
+            else
+              socket
+              |> put_flash(
+                :error,
+                "You are not authorized to merge into this project"
+              )
+              |> reset_merge_modal_state()
+              |> noreply()
+            end
         end
     end
   end
@@ -390,7 +406,7 @@ defmodule LightningWeb.SandboxLive.Index do
     current_user = socket.assigns.current_user
 
     can_create_sandbox =
-      Lightning.Policies.Permissions.can?(
+      Permissions.can?(
         :sandboxes,
         :provision_sandbox,
         current_user,
@@ -518,13 +534,18 @@ defmodule LightningWeb.SandboxLive.Index do
   end
 
   defp get_merge_target_options(socket, source_sandbox) do
+    current_user = socket.assigns.current_user
     root_project = socket.assigns.root_project
 
     socket.assigns.workspace_projects
     |> Enum.reject(fn potential_target ->
       potential_target.id == source_sandbox.id or
         Projects.descendant_of?(potential_target, source_sandbox, root_project)
     end)
+    |> Enum.filter(fn project ->
+      user_role_on_project(project, current_user) in [:owner, :admin, :editor] or
+        current_user.role == :superuser
+    end)
     |> Enum.map(fn project ->
       %{
         value: project.id,
@@ -533,6 +554,13 @@ defmodule LightningWeb.SandboxLive.Index do
     end)
   end
 
+  defp user_role_on_project(project, user) do
+    case Enum.find(project.project_users, &(&1.user_id == user.id)) do
+      nil -> nil
+      pu -> pu.role
+    end
+  end
+
   defp get_all_descendants(sandbox, workspace_projects) do
     project_map = Map.new(workspace_projects, &{&1.id, &1})
 

test/lightning/policies/sandbox_permissions_test.exs

@@ -337,6 +337,79 @@ defmodule Lightning.Policies.SandboxesTest do
     end
   end
 
+  describe "merge_sandbox permissions" do
+    test "editors on the target project can merge sandboxes", %{
+      root_project: root_project,
+      user: user
+    } do
+      insert(:project_user, user: user, project: root_project, role: :editor)
+
+      root_project =
+        Lightning.Repo.preload(root_project, :project_users, force: true)
+
+      assert Sandboxes
+             |> Permissions.can?(:merge_sandbox, user, root_project)
+    end
+
+    test "admins on the target project can merge sandboxes", %{
+      root_project: root_project,
+      user: user
+    } do
+      insert(:project_user, user: user, project: root_project, role: :admin)
+
+      root_project =
+        Lightning.Repo.preload(root_project, :project_users, force: true)
+
+      assert Sandboxes
+             |> Permissions.can?(:merge_sandbox, user, root_project)
+    end
+
+    test "owners on the target project can merge sandboxes", %{
+      root_project_owner: owner,
+      root_project: root_project
+    } do
+      assert Sandboxes
+             |> Permissions.can?(:merge_sandbox, owner, root_project)
+    end
+
+    test "viewers on the target project cannot merge sandboxes", %{
+      root_project: root_project,
+      user: user
+    } do
+      insert(:project_user, user: user, project: root_project, role: :viewer)
+
+      root_project =
+        Lightning.Repo.preload(root_project, :project_users, force: true)
+
+      refute Sandboxes
+             |> Permissions.can?(:merge_sandbox, user, root_project)
+    end
+
+    test "users without project access cannot merge sandboxes", %{
+      root_project: root_project,
+      user: user
+    } do
+      refute Sandboxes
+             |> Permissions.can?(:merge_sandbox, user, root_project)
+    end
+
+    test "superusers can merge sandboxes into any project", %{
+      superuser: superuser,
+      root_project: root_project,
+      other_root_project: other_root_project
+    } do
+      assert Sandboxes
+             |> Permissions.can?(:merge_sandbox, superuser, root_project)
+
+      assert Sandboxes
+             |> Permissions.can?(
+               :merge_sandbox,
+               superuser,
+               other_root_project
+             )
+    end
+  end
+
   describe "check_manage_permissions/3 bulk operation" do
     setup %{
       root_project: root_project,
@@ -456,7 +529,30 @@ defmodule Lightning.Policies.SandboxesTest do
       assert map_size(permissions) == 4
 
       for sandbox <- sandboxes do
-        assert %{update: false, delete: false} = permissions[sandbox.id]
+        assert %{update: false, delete: false, merge: false} =
+                 permissions[sandbox.id]
+      end
+    end
+
+    test "root project editor gets merge but not update/delete on all sandboxes",
+         %{
+           root_project: root_project,
+           sandboxes: sandboxes,
+           user: user
+         } do
+      insert(:project_user, user: user, project: root_project, role: :editor)
+
+      root_project =
+        Lightning.Repo.preload(root_project, :project_users, force: true)
+
+      permissions =
+        Sandboxes.check_manage_permissions(sandboxes, user, root_project)
+
+      assert map_size(permissions) == 4
+
+      for sandbox <- sandboxes do
+        assert %{update: false, delete: false, merge: true} =
+                 permissions[sandbox.id]
       end
     end
   end
@@ -493,7 +589,7 @@ defmodule Lightning.Policies.SandboxesTest do
       refute Sandboxes |> Permissions.can?(:delete_sandbox, user, sandbox)
     end
 
-    test "provision_sandbox with editor role", %{
+    test "provision_sandbox with editor role is allowed", %{
       root_project: root_project,
       user: user
     } do
@@ -502,7 +598,7 @@ defmodule Lightning.Policies.SandboxesTest do
       root_project =
         Lightning.Repo.preload(root_project, :project_users, force: true)
 
-      refute Sandboxes
+      assert Sandboxes
              |> Permissions.can?(:provision_sandbox, user, root_project)
     end
 
@@ -536,8 +632,10 @@ defmodule Lightning.Policies.SandboxesTest do
       permissions =
         Sandboxes.check_manage_permissions(sandboxes, user, root_project)
 
+      # Editor on root gets merge but not update/delete
       for sandbox <- sandboxes do
-        assert %{update: false, delete: false} = permissions[sandbox.id]
+        assert %{update: false, delete: false, merge: true} =
+                 permissions[sandbox.id]
       end
     end
   end

test/lightning/sandboxes_test.exs

@@ -179,11 +179,18 @@ defmodule Lightning.Projects.SandboxesTest do
   end
 
   describe "authorization" do
-    test "rejects non-admin/editor" do
-      %{actor: actor, parent: parent} = build_parent_fixture!(:editor)
+    test "rejects viewer" do
+      %{actor: actor, parent: parent} = build_parent_fixture!(:viewer)
 
       assert {:error, :unauthorized} =
-               Sandboxes.provision(parent, actor, %{name: "SB"})
+               Sandboxes.provision(parent, actor, %{name: "sb-viewer"})
+    end
+
+    test "allows editor" do
+      %{actor: actor, parent: parent} = build_parent_fixture!(:editor)
+
+      assert {:ok, %Project{}} =
+               Sandboxes.provision(parent, actor, %{name: "sb-editor"})
     end
 
     test "allows admin/owner" do