Fix teams permissions

lib/query_canary/checks.ex

@@ -97,13 +97,19 @@ defmodule QueryCanary.Checks do
   end
 
   def get_check!(%Scope{} = scope, id) do
-    Repo.get_by!(Check, id: id, user_id: scope.user.id)
-    |> Repo.preload(:server)
+    Check
+    |> where([c], c.id == ^id)
+    |> accessible_by_user(scope.user.id)
+    |> Repo.one!()
+    |> Repo.preload(server: [:team])
   end
 
   def get_check(%Scope{} = scope, id) do
-    Repo.get_by(Check, id: id, user_id: scope.user.id)
-    |> Repo.preload(:server)
+    Check
+    |> where([c], c.id == ^id)
+    |> accessible_by_user(scope.user.id)
+    |> Repo.one()
+    |> Repo.preload(server: [:team])
   end
 
   def get_check_for_system!(id) do
@@ -151,7 +157,7 @@ defmodule QueryCanary.Checks do
 
   """
   def update_check(%Scope{} = scope, %Check{} = check, attrs) do
-    true = check.user_id == scope.user.id
+    true = can_perform?(:edit, scope, check)
 
     with {:ok, check = %Check{}} <-
            check
@@ -175,7 +181,7 @@ defmodule QueryCanary.Checks do
 
   """
   def delete_check(%Scope{} = scope, %Check{} = check) do
-    true = check.user_id == scope.user.id
+    true = can_perform?(:edit, scope, check)
 
     with {:ok, check = %Check{}} <-
            Repo.delete(check) do
@@ -194,7 +200,7 @@ defmodule QueryCanary.Checks do
 
   """
   def change_check(%Scope{} = scope, %Check{} = check, attrs \\ %{}) do
-    true = check.user_id == scope.user.id
+    true = can_perform?(:edit, scope, check)
 
     Check.changeset(check, attrs, scope)
   end

lib/query_canary/checks/check.ex

@@ -1,6 +1,10 @@
 defmodule QueryCanary.Checks.Check do
   use Ecto.Schema
   import Ecto.Changeset
+  import Ecto.Query
+
+  alias QueryCanary.Accounts.TeamUser
+  alias QueryCanary.Servers.Server
 
   schema "checks" do
     field :name, :string
@@ -25,7 +29,9 @@ defmodule QueryCanary.Checks.Check do
     |> validate_required([:name, :schedule, :enabled, :query, :server_id])
     |> validate_cron_expression(:schedule)
     |> foreign_key_constraint(:server_id)
-    |> put_change(:user_id, user_scope.user.id)
+    |> validate_server_id_unchanged()
+    |> validate_server_access(user_scope)
+    |> put_user_id_on_insert(user_scope)
   end
 
   defp validate_cron_expression(changeset, field) do
@@ -40,4 +46,49 @@ defmodule QueryCanary.Checks.Check do
       end
     end)
   end
+
+  defp validate_server_access(changeset, user_scope) do
+    server_id = get_field(changeset, :server_id)
+
+    cond do
+      Keyword.has_key?(changeset.errors, :server_id) ->
+        changeset
+
+      is_nil(server_id) ->
+        changeset
+
+      true ->
+        user_id = user_scope.user.id
+
+        query =
+          from s in Server,
+            left_join: tu in TeamUser,
+            on: tu.team_id == s.team_id,
+            where: s.id == ^server_id and (s.user_id == ^user_id or tu.user_id == ^user_id)
+
+        if QueryCanary.Repo.exists?(query) do
+          changeset
+        else
+          add_error(changeset, :server_id, "is not accessible to the current user")
+        end
+    end
+  end
+
+  defp validate_server_id_unchanged(%Ecto.Changeset{data: %__MODULE__{id: nil}} = changeset) do
+    changeset
+  end
+
+  defp validate_server_id_unchanged(changeset) do
+    case get_change(changeset, :server_id) do
+      nil -> changeset
+      server_id when server_id == changeset.data.server_id -> changeset
+      _server_id -> add_error(changeset, :server_id, "cannot be changed after creation")
+    end
+  end
+
+  defp put_user_id_on_insert(%Ecto.Changeset{data: %__MODULE__{id: nil}} = changeset, user_scope) do
+    put_change(changeset, :user_id, user_scope.user.id)
+  end
+
+  defp put_user_id_on_insert(changeset, _user_scope), do: changeset
 end

test/query_canary/checks_test.exs

@@ -30,8 +30,9 @@ defmodule QueryCanary.ChecksTest do
   alias QueryCanary.Checks
   alias QueryCanary.Checks.Check
   alias QueryCanary.Checks.CheckResult
+  alias QueryCanary.Accounts
 
-  import QueryCanary.AccountsFixtures, only: [user_scope_fixture: 0]
+  import QueryCanary.AccountsFixtures, only: [team_fixture: 1, user_scope_fixture: 0]
   import QueryCanary.ChecksFixtures
   import QueryCanary.ServersFixtures
 
@@ -55,6 +56,16 @@ defmodule QueryCanary.ChecksTest do
       assert_raise Ecto.NoResultsError, fn -> Checks.get_check!(other_scope, check.id) end
     end
 
+    test "get_check!/2 returns a team check for a team member" do
+      owner_scope = user_scope_fixture()
+      team = team_fixture(owner_scope)
+      member_scope = add_team_member(owner_scope, team)
+      server = server_fixture(owner_scope, %{team_id: team.id})
+      check = check_fixture(owner_scope, %{server_id: server.id})
+
+      assert Checks.get_check!(member_scope, check.id).id == check.id
+    end
+
     test "create_check/2 with valid data creates a check" do
       scope = user_scope_fixture()
       server = server_fixture(scope)
@@ -87,6 +98,32 @@ defmodule QueryCanary.ChecksTest do
       assert check.query == "some updated query"
     end
 
+    test "update_check/3 allows a team member to update a check they did not create" do
+      owner_scope = user_scope_fixture()
+      team = team_fixture(owner_scope)
+      member_scope = add_team_member(owner_scope, team)
+      server = server_fixture(owner_scope, %{team_id: team.id})
+      check = check_fixture(owner_scope, %{server_id: server.id})
+
+      assert {:ok, %Check{} = check} =
+               Checks.update_check(member_scope, check, %{query: "some updated query"})
+
+      assert check.query == "some updated query"
+      assert check.user_id == owner_scope.user.id
+    end
+
+    test "update_check/3 rejects changing a check's server" do
+      scope = user_scope_fixture()
+      other_scope = user_scope_fixture()
+      check = check_fixture(scope)
+      other_server = server_fixture(other_scope)
+
+      assert {:error, %Ecto.Changeset{} = changeset} =
+               Checks.update_check(scope, check, %{server_id: other_server.id})
+
+      assert {"cannot be changed after creation", _} = changeset.errors[:server_id]
+    end
+
     test "update_check/3 with invalid scope raises" do
       scope = user_scope_fixture()
       other_scope = user_scope_fixture()
@@ -111,6 +148,17 @@ defmodule QueryCanary.ChecksTest do
       assert_raise Ecto.NoResultsError, fn -> Checks.get_check!(scope, check.id) end
     end
 
+    test "delete_check/2 allows a team member to delete a check they did not create" do
+      owner_scope = user_scope_fixture()
+      team = team_fixture(owner_scope)
+      member_scope = add_team_member(owner_scope, team)
+      server = server_fixture(owner_scope, %{team_id: team.id})
+      check = check_fixture(owner_scope, %{server_id: server.id})
+
+      assert {:ok, %Check{}} = Checks.delete_check(member_scope, check)
+      assert_raise Ecto.NoResultsError, fn -> Checks.get_check!(owner_scope, check.id) end
+    end
+
     test "delete_check/2 with invalid scope raises" do
       scope = user_scope_fixture()
       other_scope = user_scope_fixture()
@@ -124,6 +172,16 @@ defmodule QueryCanary.ChecksTest do
       assert %Ecto.Changeset{} = Checks.change_check(scope, check)
     end
 
+    test "change_check/2 allows a team member to edit a check they did not create" do
+      owner_scope = user_scope_fixture()
+      team = team_fixture(owner_scope)
+      member_scope = add_team_member(owner_scope, team)
+      server = server_fixture(owner_scope, %{team_id: team.id})
+      check = check_fixture(owner_scope, %{server_id: server.id})
+
+      assert %Ecto.Changeset{} = Checks.change_check(member_scope, check)
+    end
+
     test "run_check/1 gives checks a 30 second query timeout" do
       scope = user_scope_fixture()
       server = server_fixture(scope)
@@ -176,4 +234,13 @@ defmodule QueryCanary.ChecksTest do
     #   assert {:ok, :notification_sent} = Checks.maybe_send_check_notification(check, result)
     # end
   end
+
+  defp add_team_member(owner_scope, team) do
+    member_scope = user_scope_fixture()
+
+    {:ok, _user} = Accounts.invite_user_to_team(owner_scope, team, member_scope.user.email)
+    {:ok, _team_user} = Accounts.accept_team_invite(member_scope, team)
+
+    member_scope
+  end
 end

test/query_canary_web/live/check_live_test.exs

@@ -4,6 +4,9 @@ defmodule QueryCanaryWeb.CheckLiveTest do
   import Phoenix.LiveViewTest
   import QueryCanary.ChecksFixtures
   import QueryCanary.AccountsFixtures
+  import QueryCanary.ServersFixtures
+
+  alias QueryCanary.Accounts
 
   @update_attrs %{
     name: "Updated Check",
@@ -123,5 +126,47 @@ defmodule QueryCanaryWeb.CheckLiveTest do
 
       assert redirected_to =~ "/"
     end
+
+    test "can edit a team check created by another user", %{conn: conn, scope: member_scope} do
+      check = team_check_for_member(member_scope)
+
+      assert {:ok, form_live, html} = live(conn, ~p"/checks/#{check}/edit")
+
+      assert html =~ "Edit Check"
+
+      assert {:ok, index_live, _html} =
+               form_live
+               |> form("#check-form", check: @update_attrs)
+               |> render_submit()
+               |> follow_redirect(conn, ~p"/checks")
+
+      html = render(index_live)
+      assert html =~ "Check updated successfully"
+      assert html =~ "Updated Check"
+    end
+
+    test "can delete a team check created by another user from the listing", %{
+      conn: conn,
+      scope: member_scope
+    } do
+      check = team_check_for_member(member_scope)
+
+      {:ok, index_live, html} = live(conn, ~p"/checks")
+
+      assert html =~ check.name
+      assert index_live |> element("#checks-#{check.id} a", "Delete") |> render_click()
+      refute has_element?(index_live, "#checks-#{check.id}")
+    end
+  end
+
+  defp team_check_for_member(member_scope) do
+    owner_scope = user_scope_fixture()
+    team = team_fixture(owner_scope)
+
+    {:ok, _user} = Accounts.invite_user_to_team(owner_scope, team, member_scope.user.email)
+    {:ok, _team_user} = Accounts.accept_team_invite(member_scope, team)
+
+    server = server_fixture(owner_scope, %{team_id: team.id})
+    check_fixture(owner_scope, %{server_id: server.id})
   end
 end