Merge pull request #735 from code-corps/717-change-project-permissions

Updated policies to rely on ProjectUser records

Author: joshsmith

lib/code_corps/helpers/policy.ex

@@ -4,82 +4,89 @@ defmodule CodeCorps.Helpers.Policy do
   authorization policies.
   """
 
-  import Ecto.Query
-
   alias CodeCorps.{
-    Organization, OrganizationMembership,
-    Project, ProjectUser, Repo, StripeConnectAccount,
+    Organization, ProjectUser,
+    Project, ProjectUser, Repo,
     TaskSkill, Task, User, UserTask
   }
   alias Ecto.Changeset
 
   @doc """
-  Retrieves the specified user's membership record, from a `CodeCorps.Project` struct or an `Ecto.Changeset`,
-  containing an `organization_id` field, or from a `CodeCorps.Organization` struct
+  Determines if the provided organization or project is owned by the provided user
+  """
+  @spec owned_by?(nil | Organization.t | Project.t, User.t) :: boolean
+  def owned_by?(%{owner_id: owner_id}, %User{id: user_id}), do: owner_id == user_id
+  def owned_by?(nil, _), do: false
 
-  Returns `CodeCorps.OrganizationMembership`
+  @doc """
+  Determines if the provided project is being administered by the provided User
+
+  Returns `true` if
+    - the user is the owner of the project
+    - the user is an admin or higher member of the project
   """
-  @spec get_membership(nil | Changeset.t | Project.t | Organization.t | StripeConnectAccount.t, User.t) :: nil | OrganizationMembership.t
-  def get_membership(nil, %User{}), do: nil
-  def get_membership(%Changeset{changes: %{organization_id: organization_id}}, %User{id: user_id}), do: do_get_membership(organization_id, user_id)
-  def get_membership(%Project{organization_id: organization_id}, %User{id: user_id}), do: do_get_membership(organization_id, user_id)
-  def get_membership(%Organization{id: organization_id}, %User{id: user_id}), do: do_get_membership(organization_id, user_id)
-  def get_membership(%StripeConnectAccount{organization_id: organization_id}, %User{id: user_id}), do: do_get_membership(organization_id, user_id)
-  defp do_get_membership(organization_id, user_id) do
-    OrganizationMembership
-    |> where([m], m.member_id == ^user_id and m.organization_id == ^organization_id)
-    |> Repo.one
+  @spec administered_by?(nil | Project.t, User.t) :: boolean
+  def administered_by?(%Project{} = project, %User{} = user) do
+    case owned_by?(project, user) do
+      true -> true
+      false -> project |> get_membership(user) |> get_role |> admin_or_higher?
+    end
   end
+  def administered_by?(nil, _), do: false
 
   @doc """
-  Determines if the provided organization is owned by the provided user
+  Determines if the provided project is being contributed to by the provided User
+
+  Returns `true` if
+    - the user is the owner of the project
+    - the user is a contributor or higher member of the project
   """
-  @spec organization_owned_by?(Organization.t, User.t) :: boolean
-  def organization_owned_by?(%Organization{owner_id: owner_id}, %User{id: user_id}) do
-    owner_id == user_id
+  @spec contributed_by?(nil | Project.t, User.t) :: boolean
+  def contributed_by?(%Project{} = project, %User{} = user) do
+    case owned_by?(project, user) do
+      true -> true
+      false -> project |> get_membership(user) |> get_role |> contributor_or_higher?
+    end
   end
+  def contributed_by?(nil, _), do: false
+
+  @doc """
+  Retrieves an organization record, from a model struct, or an `Ecto.Changeset`
+  containing an `organization_id` field
+
+  Returns `CodeCorps.Organization`
+  """
+  @spec get_organization(struct | Changeset.t | any) :: Organization.t
+  def get_organization(%{organization_id: id}), do: Organization |> Repo.get(id)
+  def get_organization(%Changeset{changes: %{organization_id: id}}), do: Organization |> Repo.get(id)
+  def get_organization(_), do: nil
 
   @doc """
-  Retrieves a project record, from a model struct, or an `Ecto.Changeset` containing a `project_id` field
+  Retrieves a project record, from a model struct, or an `Ecto.Changeset`
+  containing a `project_id` field
 
   Returns `CodeCorps.Project`
   """
   @spec get_project(struct | Changeset.t | any) :: Project.t
-  def get_project(%{project_id: project_id}), do: Project |> Repo.get(project_id)
-  def get_project(%Changeset{changes: %{project_id: project_id}}), do: Project |> Repo.get(project_id)
+  def get_project(%{project_id: id}), do: Project |> Repo.get(id)
+  def get_project(%Changeset{changes: %{project_id: id}}), do: Project |> Repo.get(id)
   def get_project(_), do: nil
 
   @doc """
-  Retrieves the role field, from a `CodeCorps.OrganizationMembership` struct or an `Ecto.Changeset`
-
-  Returns `:string`
+  Retrieves the role field from a `CodeCorps.ProjectUser` struct or an `Ecto.Changeset`
   """
-  @spec get_role(nil | OrganizationMembership.t | ProjectUser.t | Changeset.t) :: String.t | nil
+  @spec get_role(nil | ProjectUser.t | Changeset.t) :: String.t | nil
   def get_role(nil), do: nil
-  def get_role(%OrganizationMembership{role: role}), do: role
   def get_role(%ProjectUser{role: role}), do: role
   def get_role(%Changeset{} = changeset), do: changeset |> Changeset.get_field(:role)
 
-  @doc """
-  Determines if provided string is equal to "owner"
-  """
-  @spec owner?(String.t) :: boolean
-  def owner?("owner"), do: true
-  def owner?(_), do: false
-
-  @doc """
-  Determines if provided string is equal to one of `["admin", "owner"]`
-  """
   @spec admin_or_higher?(String.t) :: boolean
-  def admin_or_higher?(role) when role in ["admin", "owner"], do: true
-  def admin_or_higher?(_), do: false
+  defp admin_or_higher?(role) when role in ["admin", "owner"], do: true
+  defp admin_or_higher?(_), do: false
 
-  @doc """
-  Determines if provided string is equal to one of `["contributor", "admin", "owner"]`
-  """
   @spec contributor_or_higher?(String.t) :: boolean
-  def contributor_or_higher?(role) when role in ["contributor", "admin", "owner"], do: true
-  def contributor_or_higher?(_), do: false
+  defp contributor_or_higher?(role) when role in ["contributor", "admin", "owner"], do: true
+  defp contributor_or_higher?(_), do: false
 
   @doc """
   Retrieves task from associated record
@@ -96,4 +103,9 @@ defmodule CodeCorps.Helpers.Policy do
   def task_authored_by?(%Task{user_id: author_id}, %User{id: user_id}), do: user_id == author_id
 
 
+  # Returns `CodeCorps.ProjectUser` for specified `CodeCorps.Project`
+  # and `CodeCorps.User`, or nil
+  @spec get_membership(Project.t, User.t) :: nil | ProjectUser.t
+  defp get_membership(%Project{id: project_id}, %User{id: user_id}),
+    do: ProjectUser |> Repo.get_by(project_id: project_id, user_id: user_id)
 end

test/controllers/donation_goal_controller_test.exs

@@ -44,20 +44,21 @@ defmodule CodeCorps.DonationGoalControllerTest do
   describe "create" do
     @tag :authenticated
     test "creates and renders resource when data is valid", %{conn: conn, current_user: current_user} do
-      organization = insert(:organization)
-      insert(:organization_membership, member: current_user, organization: organization, role: "owner")
-      project = insert(:project, organization: organization)
+      project = insert(:project, owner: current_user)
 
-      attrs = @valid_attrs |> Map.merge(%{project: project})
+      attrs = @valid_attrs |> Map.merge(%{project_id: project.id})
       assert conn |> request_create(attrs) |> json_response(201)
 
       user_id = current_user.id
       assert_received {:track, ^user_id, "Created Donation Goal", %{}}
     end
 
-    @tag authenticated: :admin
-    test "does not create resource and renders errors when data is invalid", %{conn: conn} do
-      assert conn |> request_create(@invalid_attrs) |> json_response(422)
+    @tag :authenticated
+    test "does not create resource and renders errors when data is invalid", %{conn: conn, current_user: current_user} do
+      project = insert(:project, owner: current_user)
+
+      attrs = @invalid_attrs |> Map.merge(%{project_id: project.id})
+      assert conn |> request_create(attrs) |> json_response(422)
     end
 
     test "renders 401 when not authenticated", %{conn: conn} do
@@ -73,22 +74,23 @@ defmodule CodeCorps.DonationGoalControllerTest do
   describe "update" do
     @tag :authenticated
     test "updates and renders chosen resource when data is valid", %{conn: conn, current_user: current_user} do
-      organization = insert(:organization)
-      insert(:organization_membership, member: current_user, organization: organization, role: "owner")
-      project = insert(:project, organization: organization)
+      project = insert(:project, owner: current_user)
 
       donation_goal = insert(:donation_goal, project: project)
 
-      attrs = @valid_attrs |> Map.merge(%{project: project})
+      attrs = @valid_attrs |> Map.merge(%{project_id: project.id})
       assert conn |> request_update(donation_goal, attrs) |> json_response(200)
 
       user_id = current_user.id
       assert_received {:track, ^user_id, "Updated Donation Goal", %{}}
     end
 
     @tag authenticated: :admin
-    test "does not update chosen resource and renders errors when data is invalid", %{conn: conn} do
-      assert conn |> request_update(@invalid_attrs) |> json_response(422)
+    test "does not update chosen resource and renders errors when data is invalid", %{conn: conn, current_user: current_user} do
+      project = insert(:project, owner: current_user)
+      donation_goal = insert(:donation_goal, project: project)
+
+      assert conn |> request_update(donation_goal, @invalid_attrs) |> json_response(422)
     end
 
     @tag authenticated: :admin
@@ -107,9 +109,10 @@ defmodule CodeCorps.DonationGoalControllerTest do
   end
 
   describe "delete" do
-    @tag authenticated: :admin
-    test "deletes chosen resource", %{conn: conn} do
-      donation_goal = insert(:donation_goal)
+    @tag :authenticated
+    test "deletes chosen resource", %{conn: conn, current_user: current_user} do
+      project = insert(:project, owner: current_user)
+      donation_goal = insert(:donation_goal, project: project)
       assert conn |> request_delete(donation_goal) |> response(204)
     end
 

test/controllers/organization_membership_controller_test.exs

@@ -42,19 +42,19 @@ defmodule CodeCorps.ProjectCategoryControllerTest do
   end
 
   describe "create" do
-    @tag authenticated: :admin
-    test "creates and renders resource when data is valid", %{conn: conn} do
-      organization = insert(:organization)
+    @tag :authenticated
+    test "creates and renders resource when data is valid", %{conn: conn, current_user: current_user} do
       category = insert(:category)
-      project = insert(:project, organization: organization)
+      project = insert(:project, owner: current_user)
 
       attrs = %{category: category, project: project}
       assert conn |> request_create(attrs) |> json_response(201)
     end
 
-    @tag authenticated: :admin
-    test "renders 422 when data is invalid", %{conn: conn} do
-      invalid_attrs = %{}
+    @tag :authenticated
+    test "renders 422 when data is invalid", %{conn: conn, current_user: current_user} do
+      project = insert(:project, owner: current_user)
+      invalid_attrs = %{project: project}
       assert conn |> request_create(invalid_attrs) |> json_response(422)
     end
 
@@ -69,9 +69,11 @@ defmodule CodeCorps.ProjectCategoryControllerTest do
   end
 
   describe "delete" do
-    @tag authenticated: :admin
-    test "deletes resource", %{conn: conn} do
-      assert conn |> request_delete |> response(204)
+    @tag :authenticated
+    test "deletes resource", %{conn: conn, current_user: current_user} do
+      project = insert(:project, owner: current_user)
+      project_category = insert(:project_category, project: project)
+      assert conn |> request_delete(project_category) |> response(204)
     end
 
     test "renders 401 when unauthenticated", %{conn: conn} do