Fix IDOR vulnerability

lib/cadet/accounts/course_registrations.ex

@@ -100,6 +100,13 @@ defmodule Cadet.Accounts.CourseRegistrations do
     |> Repo.all()
   end
 
+  def get_course_reg_in_course(course_reg_id, course_id)
+      when is_ecto_id(course_reg_id) and is_ecto_id(course_id) do
+    CourseRegistration
+    |> where(id: ^course_reg_id, course_id: ^course_id)
+    |> Repo.one()
+  end
+
   def get_staffs(course_id) do
     CourseRegistration
     |> where(course_id: ^course_id)

lib/cadet/accounts/teams.ex

@@ -39,6 +39,15 @@ defmodule Cadet.Accounts.Teams do
     teams
   end
 
+  def get_team_in_course(team_id, course_id)
+      when is_ecto_id(team_id) and is_ecto_id(course_id) do
+    Team
+    |> where(id: ^team_id)
+    |> join(:inner, [t], a in assoc(t, :assessment))
+    |> where([_, a], a.course_id == ^course_id)
+    |> Repo.one()
+  end
+
   @doc """
   Creates a new team and assigns an assessment and team members to it.
 

lib/cadet/assessments/assessments.ex

@@ -1248,6 +1248,23 @@ defmodule Cadet.Assessments do
     |> Repo.one()
   end
 
+  def get_assessment_in_course(assessment_id, course_id)
+      when is_ecto_id(assessment_id) and is_ecto_id(course_id) do
+    Assessment
+    |> where(id: ^assessment_id, course_id: ^course_id)
+    |> Repo.one()
+  end
+
+  def get_question_in_course(question_id, course_id)
+      when is_ecto_id(question_id) and is_ecto_id(course_id) do
+    Question
+    |> where(id: ^question_id)
+    |> join(:inner, [q], a in assoc(q, :assessment))
+    |> where([_, a], a.course_id == ^course_id)
+    |> preload([_, a], assessment: a)
+    |> Repo.one()
+  end
+
   def delete_question(id) when is_ecto_id(id) do
     Logger.info("Deleting question #{id}")
 
@@ -1405,6 +1422,16 @@ defmodule Cadet.Assessments do
     |> Repo.one()
   end
 
+  def get_submission_in_course(submission_id, course_id)
+      when is_ecto_id(submission_id) and is_ecto_id(course_id) do
+    Submission
+    |> where(id: ^submission_id)
+    |> join(:inner, [s], a in assoc(s, :assessment))
+    |> where([_, a], a.course_id == ^course_id)
+    |> preload([_, a], assessment: a)
+    |> Repo.one()
+  end
+
   def finalise_submission(submission = %Submission{}) do
     Logger.info(
       "Finalizing submission #{submission.id} for assessment #{submission.assessment_id}"
@@ -3093,6 +3120,21 @@ defmodule Cadet.Assessments do
     end
   end
 
+  def get_answer_in_course(answer_id, course_id)
+      when is_ecto_id(answer_id) and is_ecto_id(course_id) do
+    answer_query =
+      Answer
+      |> where(id: ^answer_id)
+      |> join(:inner, [a], q in assoc(a, :question))
+      |> join(:inner, [_, q], ast in assoc(q, :assessment))
+      |> where([_, _, ast], ast.course_id == ^course_id)
+
+    case Repo.exists?(answer_query) do
+      true -> get_answer(answer_id)
+      false -> {:error, {:forbidden, "Forbidden"}}
+    end
+  end
+
   @spec get_answers_in_submission(integer() | String.t()) ::
           {:ok, {[Answer.t()], Assessment.t()}}
           | {:error, {:bad_request, String.t()}}
@@ -3200,7 +3242,7 @@ defmodule Cadet.Assessments do
   def update_grading_info(
         %{submission_id: submission_id, question_id: question_id},
         attrs,
-        cr = %CourseRegistration{id: grader_id}
+        cr = %CourseRegistration{id: grader_id, course_id: course_id}
       )
       when is_ecto_id(submission_id) and is_ecto_id(question_id) do
     attrs = Map.put(attrs, "grader_id", grader_id)
@@ -3213,7 +3255,9 @@ defmodule Cadet.Assessments do
     answer_query =
       answer_query
       |> join(:inner, [a], s in assoc(a, :submission))
-      |> preload([_, s], submission: s)
+      |> join(:inner, [_, s], asst in assoc(s, :assessment))
+      |> where([_, _, asst], asst.course_id == ^course_id)
+      |> preload([_, s, asst], submission: {s, assessment: asst})
 
     answer = Repo.one(answer_query)
 
@@ -3267,10 +3311,16 @@ defmodule Cadet.Assessments do
           {:ok, nil} | {:error, {:forbidden | :not_found, String.t()}}
   def force_regrade_submission(
         submission_id,
-        _requesting_user = %CourseRegistration{id: grader_id}
+        _requesting_user = %CourseRegistration{id: grader_id, course_id: course_id}
       )
       when is_ecto_id(submission_id) do
-    with {:get, sub} when not is_nil(sub) <- {:get, Repo.get(Submission, submission_id)},
+    submission_query =
+      Submission
+      |> where(id: ^submission_id)
+      |> join(:inner, [s], asst in assoc(s, :assessment))
+      |> where([_, asst], asst.course_id == ^course_id)
+
+    with {:get, sub} when not is_nil(sub) <- {:get, Repo.one(submission_query)},
          {:status, true} <- {:status, sub.student_id == grader_id or sub.status == :submitted} do
       GradingJob.force_grade_individual_submission(sub, true)
       {:ok, nil}
@@ -3296,12 +3346,15 @@ defmodule Cadet.Assessments do
   def force_regrade_answer(
         submission_id,
         question_id,
-        _requesting_user = %CourseRegistration{id: grader_id}
+        _requesting_user = %CourseRegistration{id: grader_id, course_id: course_id}
       )
       when is_ecto_id(submission_id) and is_ecto_id(question_id) do
     answer =
       Answer
       |> where(submission_id: ^submission_id, question_id: ^question_id)
+      |> join(:inner, [a], q in assoc(a, :question))
+      |> join(:inner, [_, q], asst in assoc(q, :assessment))
+      |> where([_, _, asst], asst.course_id == ^course_id)
       |> preload([:question, :submission])
       |> Repo.one()
 

lib/cadet_web/admin_controllers/admin_assessments_controller.ex

@@ -9,18 +9,29 @@ defmodule CadetWeb.AdminAssessmentsController do
   alias CadetWeb.AssessmentsHelpers
   alias Cadet.Assessments.{Question, Assessment}
   alias Cadet.{Assessments, Repo}
-  alias Cadet.Accounts.CourseRegistration
 
-  def index(conn, %{"course_reg_id" => course_reg_id}) do
-    course_reg = Repo.get(CourseRegistration, course_reg_id)
+  plug(
+    CadetWeb.Plug.EnsureResourceScope,
+    [resource: :course_reg, param: "course_reg_id", assign: :target_course_reg]
+    when action in [:index, :get_assessment]
+  )
+
+  plug(
+    CadetWeb.Plug.EnsureResourceScope,
+    [resource: :assessment, param: "assessmentid", assign: :scoped_assessment]
+    when action in [:update, :calculate_contest_score, :dispatch_contest_xp]
+  )
+
+  def index(conn, %{"course_reg_id" => _course_reg_id}) do
+    course_reg = conn.assigns.target_course_reg
     {:ok, assessments} = Assessments.all_assessments(course_reg)
     assessments = Assessments.format_all_assessments(assessments)
     render(conn, "index.json", assessments: assessments)
   end
 
-  def get_assessment(conn, %{"course_reg_id" => course_reg_id, "assessmentid" => assessment_id})
+  def get_assessment(conn, %{"course_reg_id" => _course_reg_id, "assessmentid" => assessment_id})
       when is_ecto_id(assessment_id) do
-    course_reg = Repo.get(CourseRegistration, course_reg_id)
+    course_reg = conn.assigns.target_course_reg
 
     case Assessments.assessment_with_questions_and_answers(assessment_id, course_reg) do
       {:ok, assessment} -> render(conn, "show.json", assessment: assessment)
@@ -135,7 +146,7 @@ defmodule CadetWeb.AdminAssessmentsController do
     end
   end
 
-  def calculate_contest_score(conn, %{"assessmentid" => assessment_id, "course_id" => course_id}) do
+  def calculate_contest_score(conn, %{"assessmentid" => assessment_id, "course_id" => _course_id}) do
     voting_questions =
       Question
       |> where(type: :voting)
@@ -150,7 +161,7 @@ defmodule CadetWeb.AdminAssessmentsController do
     end
   end
 
-  def dispatch_contest_xp(conn, %{"assessmentid" => assessment_id, "course_id" => course_id}) do
+  def dispatch_contest_xp(conn, %{"assessmentid" => assessment_id, "course_id" => _course_id}) do
     voting_questions =
       Question
       |> where(type: :voting)

lib/cadet_web/admin_controllers/admin_goals_controller.ex

@@ -4,16 +4,20 @@ defmodule CadetWeb.AdminGoalsController do
   use PhoenixSwagger
 
   alias Cadet.Incentives.Goals
-  alias Cadet.Accounts.CourseRegistration
+
+  plug(
+    CadetWeb.Plug.EnsureResourceScope,
+    [resource: :course_reg, param: "course_reg_id", assign: :target_course_reg]
+    when action in [:index_goals_with_progress, :update_progress]
+  )
 
   def index(conn, _) do
     course_id = conn.assigns.course_reg.course_id
     render(conn, "index.json", goals: Goals.get(course_id))
   end
 
-  def index_goals_with_progress(conn, %{"course_reg_id" => course_reg_id}) do
-    course_id = conn.assigns.course_reg.course_id
-    course_reg = %CourseRegistration{id: String.to_integer(course_reg_id), course_id: course_id}
+  def index_goals_with_progress(conn, %{"course_reg_id" => _course_reg_id}) do
+    course_reg = conn.assigns.target_course_reg
 
     render(conn, "index_goals_with_progress.json", goals: Goals.get_with_progress(course_reg))
   end
@@ -38,10 +42,10 @@ defmodule CadetWeb.AdminGoalsController do
 
   def update_progress(conn, %{
         "uuid" => uuid,
-        "course_reg_id" => course_reg_id,
+        "course_reg_id" => _course_reg_id,
         "progress" => progress
       }) do
-    course_reg_id = String.to_integer(course_reg_id)
+    course_reg_id = conn.assigns.target_course_reg.id
 
     progress
     |> json_to_progress(uuid, course_reg_id)

lib/cadet_web/admin_controllers/admin_grading_controller.ex

@@ -4,6 +4,32 @@ defmodule CadetWeb.AdminGradingController do
 
   alias Cadet.{Assessments, Courses}
 
+  plug(
+    CadetWeb.Plug.EnsureResourceScope,
+    [resource: :submission, param: "submissionid", assign: :scoped_submission]
+    when action in [
+           :show,
+           :update,
+           :unsubmit,
+           :unpublish_grades,
+           :publish_grades,
+           :autograde_submission,
+           :autograde_answer
+         ]
+  )
+
+  plug(
+    CadetWeb.Plug.EnsureResourceScope,
+    [resource: :question, param: "questionid", assign: :scoped_question]
+    when action in [:update, :autograde_answer]
+  )
+
+  plug(
+    CadetWeb.Plug.EnsureResourceScope,
+    [resource: :assessment, param: "assessmentid", assign: :scoped_assessment]
+    when action in [:publish_all_grades, :unpublish_all_grades]
+  )
+
   @doc """
   # Query Parameters
   - `pageSize`: Integer. The number of submissions to return. Default 10.

lib/cadet_web/admin_controllers/admin_teams_controller.ex

@@ -5,6 +5,12 @@ defmodule CadetWeb.AdminTeamsController do
 
   alias Cadet.Accounts.{Teams, Team}
 
+  plug(
+    CadetWeb.Plug.EnsureResourceScope,
+    [resource: :team, param: "teamid", assign: :scoped_team]
+    when action in [:update, :delete]
+  )
+
   def index(conn, %{"course_id" => course_id}) do
     teams = Teams.all_teams_for_course(course_id)
 
@@ -47,14 +53,9 @@ defmodule CadetWeb.AdminTeamsController do
     end
   end
 
-  def update(conn, %{
-        "teamId" => teamId,
-        "assessmentId" => assessmentId,
-        "student_ids" => student_ids
-      }) do
+  def update(conn, %{"assessmentId" => assessmentId, "student_ids" => student_ids}) do
     team =
-      Team
-      |> Repo.get!(teamId)
+      conn.assigns.scoped_team
       |> Repo.preload(assessment: [:config], team_members: [student: [:user]])
 
     case Teams.update_team(team, assessmentId, student_ids) do
@@ -70,8 +71,8 @@ defmodule CadetWeb.AdminTeamsController do
     end
   end
 
-  def delete(conn, %{"teamId" => team_id}) do
-    team = Repo.get(Team, team_id)
+  def delete(conn, %{"teamid" => _team_id}) do
+    team = conn.assigns.scoped_team
 
     if team do
       case Teams.delete_team(team) do
@@ -90,10 +91,6 @@ defmodule CadetWeb.AdminTeamsController do
     end
   end
 
-  def delete(conn, %{"course_id" => course_id, "teamid" => team_id}) do
-    delete(conn, %{"teamId" => team_id})
-  end
-
   swagger_path :index do
     get("/admin/teams")