diff --git a/lib/cadet/accounts/accounts.ex b/lib/cadet/accounts/accounts.ex index fa3bce754..6a6acbd46 100644 --- a/lib/cadet/accounts/accounts.ex +++ b/lib/cadet/accounts/accounts.ex @@ -8,6 +8,7 @@ defmodule Cadet.Accounts do alias Cadet.Accounts.{Query, User, CourseRegistration} alias Cadet.Auth.Provider + alias Cadet.Assessments.{Answer, Submission} @doc """ Register new User entity using Cadet.Accounts.Form.Registration @@ -109,4 +110,91 @@ defmodule Cadet.Accounts do {:error, changeset} -> {:error, {:internal_server_error, full_error_messages(changeset)}} end end + + @update_role_roles ~w(admin)a + def update_role( + _admin_course_reg = %CourseRegistration{ + id: admin_course_reg_id, + course_id: admin_course_id, + role: admin_role + }, + role, + coursereg_id + ) do + with {:validate_role, true} <- {:validate_role, admin_role in @update_role_roles}, + {:validate_not_self, true} <- {:validate_not_self, admin_course_reg_id != coursereg_id}, + {:get_cr, user_course_reg} when not is_nil(user_course_reg) <- + {:get_cr, CourseRegistration |> where(id: ^coursereg_id) |> Repo.one()}, + {:validate_same_course, true} <- + {:validate_same_course, user_course_reg.course_id == admin_course_id}, + {:update_db, {:ok, _} = result} <- + {:update_db, + user_course_reg |> CourseRegistration.changeset(%{role: role}) |> Repo.update()} do + result + else + {:validate_role, false} -> + {:error, {:forbidden, "User is not permitted to change others' roles"}} + + {:validate_not_self, false} -> + {:error, {:bad_request, "Admin not allowed to downgrade own role"}} + + {:get_cr, _} -> + {:error, {:bad_request, "User course registration does not exist"}} + + {:validate_same_course, false} -> + {:error, {:forbidden, "Wrong course"}} + + {:update_db, {:error, changeset}} -> + {:error, {:bad_request, full_error_messages(changeset)}} + end + end + + @delete_user_roles ~w(admin)a + def delete_user( + _admin_course_reg = %CourseRegistration{ + id: admin_course_reg_id, + course_id: admin_course_id, + role: admin_role + }, + coursereg_id + ) do + with {:validate_role, true} <- {:validate_role, admin_role in @delete_user_roles}, + {:validate_not_self, true} <- {:validate_not_self, admin_course_reg_id != coursereg_id}, + {:get_cr, user_course_reg} when not is_nil(user_course_reg) <- + {:get_cr, CourseRegistration |> where(id: ^coursereg_id) |> Repo.one()}, + {:prevent_delete_admin, true} <- {:prevent_delete_admin, user_course_reg.role != :admin}, + {:validate_same_course, true} <- + {:validate_same_course, user_course_reg.course_id == admin_course_id} do + # TODO: Handle deletions of achievement entries, etc. too + + # Delete submissions and answers before deleting user + Submission + |> where(student_id: ^user_course_reg.id) + |> Repo.all() + |> Enum.each(fn x -> + Answer + |> where(submission_id: ^x.id) + |> Repo.delete_all() + + Repo.delete(x) + end) + + Repo.delete(user_course_reg) + else + {:validate_role, false} -> + {:error, {:forbidden, "User is not permitted to delete other users"}} + + {:validate_not_self, false} -> + {:error, {:bad_request, "Admin not allowed to delete ownself from course"}} + + {:get_cr, _} -> + {:error, {:bad_request, "User course registration does not exist"}} + + {:prevent_delete_admin, false} -> + {:error, {:bad_request, "Admins cannot be deleted"}} + + {:validate_same_course, false} -> + {:error, {:forbidden, "Wrong course"}} + end + end end diff --git a/lib/cadet_web/admin_controllers/admin_user_controller.ex b/lib/cadet_web/admin_controllers/admin_user_controller.ex index 0ed85caf9..fb80a779c 100644 --- a/lib/cadet_web/admin_controllers/admin_user_controller.ex +++ b/lib/cadet_web/admin_controllers/admin_user_controller.ex @@ -13,6 +13,30 @@ defmodule CadetWeb.AdminUserController do render(conn, "users.json", users: users) end + def update_role(conn, %{"role" => role, "crId" => coursereg_id}) do + case Accounts.update_role(conn.assigns.course_reg, role, coursereg_id) do + {:ok, %{}} -> + text(conn, "OK") + + {:error, {status, message}} -> + conn + |> put_status(status) + |> text(message) + end + end + + def delete_user(conn, %{"crId" => coursereg_id}) do + case Accounts.delete_user(conn.assigns.course_reg, coursereg_id) do + {:ok, %{}} -> + text(conn, "OK") + + {:error, {status, message}} -> + conn + |> put_status(status) + |> text(message) + end + end + swagger_path :index do get("/v2/courses/{course_id}/admin/users") @@ -24,6 +48,56 @@ defmodule CadetWeb.AdminUserController do response(401, "Unauthorised") end + swagger_path :update_role do + put("/v2/courses/{course_id}/admin/users/role") + + summary("Updates the role of the given user in the the course") + security([%{JWT: []}]) + consumes("application/json") + + parameters do + course_id(:path, :integer, "Course ID", required: true) + role(:body, :role, "The new role", required: true) + + crId(:body, :integer, "The course registration of the user whose role is to be updated", + required: true + ) + end + + response(200, "OK") + + response( + 400, + "Bad Request. User course registration does not exist or admin not allowed to downgrade own role" + ) + + response(403, "Forbidden. User is in different course, or you are not an admin") + end + + swagger_path :delete_user do + delete("/v2/courses/{course_id}/admin/users") + + summary("Deletes a user from a course") + consumes("application/json") + + parameters do + course_id(:path, :integer, "Course ID", required: true) + + crId(:body, :integer, "The course registration of the user whose role is to be updated", + required: true + ) + end + + response(200, "OK") + + response( + 400, + "Bad Request. User course registration does not exist or admin not allowed to delete ownself from course or admins cannot be deleted" + ) + + response(403, "Forbidden. User is in different course, or you are not an admin") + end + def swagger_definitions do %{ AdminUserInfo: diff --git a/lib/cadet_web/admin_views/admin_user_view.ex b/lib/cadet_web/admin_views/admin_user_view.ex index 4b48af13d..70b9d8a51 100644 --- a/lib/cadet_web/admin_views/admin_user_view.ex +++ b/lib/cadet_web/admin_views/admin_user_view.ex @@ -10,6 +10,7 @@ defmodule CadetWeb.AdminUserView do crId: cr.id, course_id: cr.course_id, name: cr.user.name, + username: cr.user.username, role: cr.role, group: case cr.group do diff --git a/lib/cadet_web/router.ex b/lib/cadet_web/router.ex index ac6aaabf9..83f63901f 100644 --- a/lib/cadet_web/router.ex +++ b/lib/cadet_web/router.ex @@ -127,6 +127,8 @@ defmodule CadetWeb.Router do ) get("/users", AdminUserController, :index) + put("/users/role", AdminUserController, :update_role) + delete("/users", AdminUserController, :delete_user) post("/users/:userid/goals/:uuid/progress", AdminGoalsController, :update_progress) put("/achievements", AdminAchievementsController, :bulk_update) diff --git a/lib/cadet_web/views/user_view.ex b/lib/cadet_web/views/user_view.ex index bd3d3dff3..15c5c24b9 100644 --- a/lib/cadet_web/views/user_view.ex +++ b/lib/cadet_web/views/user_view.ex @@ -65,6 +65,7 @@ defmodule CadetWeb.UserView do _ -> %{ + crId: latest.id, courseId: latest.course_id, role: latest.role, group: diff --git a/test/cadet/accounts/accounts_test.exs b/test/cadet/accounts/accounts_test.exs index aa75438b5..83dd511fe 100644 --- a/test/cadet/accounts/accounts_test.exs +++ b/test/cadet/accounts/accounts_test.exs @@ -155,4 +155,127 @@ defmodule Cadet.AccountsTest do assert length(all_stu_in_c1g2) == 0 end end + + describe "update_role" do + setup do + c1 = insert(:course, %{course_name: "c1"}) + c2 = insert(:course, %{course_name: "c2"}) + admin1 = insert(:course_registration, %{course: c1, role: :admin}) + staff1 = insert(:course_registration, %{course: c1, role: :staff}) + student1 = insert(:course_registration, %{course: c1, role: :student}) + student2 = insert(:course_registration, %{course: c2, role: :student}) + + {:ok, %{a1: admin1, s1: student1, s2: student2, st1: staff1}} + end + + test "successful when admin is admin of the course the user is in (student)", %{ + a1: admin1, + s1: %{id: coursereg_id} + } do + {:ok, updated_coursereg} = Accounts.update_role(admin1, "student", coursereg_id) + assert updated_coursereg.role == :student + end + + test "successful when admin is admin of the course the user is in (staff)", %{ + a1: admin1, + s1: %{id: coursereg_id} + } do + {:ok, updated_coursereg} = Accounts.update_role(admin1, "staff", coursereg_id) + assert updated_coursereg.role == :staff + end + + test "successful when admin is admin of the course the user is in (admin)", %{ + a1: admin1, + s1: %{id: coursereg_id} + } do + {:ok, updated_coursereg} = Accounts.update_role(admin1, "admin", coursereg_id) + assert updated_coursereg.role == :admin + end + + test "fails when admin tries to downgrade own role", %{a1: %{id: coursereg_id} = admin1} do + assert {:error, {:bad_request, "Admin not allowed to downgrade own role"}} == + Accounts.update_role(admin1, "staff", coursereg_id) + end + + test "fails when user course registration does not exist", %{ + a1: admin1, + s2: %{id: coursereg_id} + } do + assert {:error, {:bad_request, "User course registration does not exist"}} == + Accounts.update_role(admin1, "staff", coursereg_id + 1) + end + + test "admin is not admin of the course the user is in", %{a1: admin1, s2: %{id: coursereg_id}} do + assert {:error, {:forbidden, "Wrong course"}} == + Accounts.update_role(admin1, "staff", coursereg_id) + end + + test "invalid role provided", %{a1: admin1, s1: %{id: coursereg_id}} do + assert {:error, {:bad_request, "role is invalid"}} == + Accounts.update_role(admin1, "invalidrole", coursereg_id) + end + + test "fails when staff makes changes", %{st1: staff1, s1: %{id: coursereg_id}} do + assert {:error, {:forbidden, "User is not permitted to change others' roles"}} == + Accounts.update_role(staff1, "staff", coursereg_id) + end + end + + describe "delete_user" do + setup do + c1 = insert(:course, %{course_name: "c1"}) + c2 = insert(:course, %{course_name: "c2"}) + admin1 = insert(:course_registration, %{course: c1, role: :admin}) + admin2 = insert(:course_registration, %{course: c1, role: :admin}) + staff1 = insert(:course_registration, %{course: c1, role: :staff}) + student1 = insert(:course_registration, %{course: c1, role: :student}) + student2 = insert(:course_registration, %{course: c2, role: :student}) + + {:ok, %{a1: admin1, a2: admin2, s1: student1, s2: student2, st1: staff1}} + end + + test "successful when admin is admin of the course the user is in (student)", %{ + a1: admin1, + s1: %{id: coursereg_id} + } do + {:ok, deleted_entry} = Accounts.delete_user(admin1, coursereg_id) + assert deleted_entry.id == coursereg_id + end + + test "successful when admin is admin of the course the user is in (staff)", %{ + a1: admin1, + st1: %{id: coursereg_id} + } do + {:ok, deleted_entry} = Accounts.delete_user(admin1, coursereg_id) + assert deleted_entry.id == coursereg_id + end + + test "fails when staff tries to delete user", %{st1: staff1, s1: %{id: coursereg_id}} do + assert {:error, {:forbidden, "User is not permitted to delete other users"}} == + Accounts.delete_user(staff1, coursereg_id) + end + + test "fails when deleting own self", %{a1: %{id: coursereg_id} = admin1} do + assert {:error, {:bad_request, "Admin not allowed to delete ownself from course"}} == + Accounts.delete_user(admin1, coursereg_id) + end + + test "fails when user course registration does not exist", %{ + a1: admin1, + s2: %{id: coursereg_id} + } do + assert {:error, {:bad_request, "User course registration does not exist"}} == + Accounts.delete_user(admin1, coursereg_id + 1) + end + + test "fails when deleting an admin", %{a1: admin1, a2: %{id: coursereg_id}} do + assert {:error, {:bad_request, "Admins cannot be deleted"}} == + Accounts.delete_user(admin1, coursereg_id) + end + + test "fails when deleting a user from another course", %{a1: admin1, s2: %{id: coursereg_id}} do + assert {:error, {:forbidden, "Wrong course"}} == + Accounts.delete_user(admin1, coursereg_id) + end + end end diff --git a/test/cadet_web/admin_controllers/admin_user_controller_test.exs b/test/cadet_web/admin_controllers/admin_user_controller_test.exs index 74d56d6f8..10ffb0089 100644 --- a/test/cadet_web/admin_controllers/admin_user_controller_test.exs +++ b/test/cadet_web/admin_controllers/admin_user_controller_test.exs @@ -2,10 +2,12 @@ defmodule CadetWeb.AdminUserControllerTest do use CadetWeb.ConnCase import Cadet.Factory + import Ecto.Query alias CadetWeb.AdminUserController alias Cadet.Repo alias Cadet.Courses.Course + alias Cadet.Accounts.CourseRegistration test "swagger" do assert is_map(AdminUserController.swagger_definitions()) @@ -23,7 +25,7 @@ defmodule CadetWeb.AdminUserControllerTest do resp = conn - |> get(build_url(course_id)) + |> get(build_url_users(course_id)) |> json_response(200) assert 3 == Enum.count(resp) @@ -39,7 +41,7 @@ defmodule CadetWeb.AdminUserControllerTest do resp = conn - |> get(build_url(course_id) <> "?role=student") + |> get(build_url_users(course_id) <> "?role=student") |> json_response(200) assert 1 == Enum.count(resp) @@ -56,7 +58,7 @@ defmodule CadetWeb.AdminUserControllerTest do resp = conn - |> get(build_url(course_id) <> "?group=#{group.name}") + |> get(build_url_users(course_id) <> "?group=#{group.name}") |> json_response(200) assert 2 == Enum.count(resp) @@ -68,7 +70,7 @@ defmodule CadetWeb.AdminUserControllerTest do course_id = conn.assigns[:course_id] assert conn - |> get(build_url(course_id)) + |> get(build_url_users(course_id)) |> response(403) end @@ -80,5 +82,262 @@ defmodule CadetWeb.AdminUserControllerTest do # end end - defp build_url(course_id), do: "/v2/courses/#{course_id}/admin/users" + describe "PUT /v2/courses/{course_id}/admin/users/role" do + @tag authenticate: :admin + test "success (student to staff), when admin is admin of the course the user is in", %{ + conn: conn + } do + course_id = conn.assigns[:course_id] + course = Repo.get(Course, course_id) + user_course_reg = insert(:course_registration, %{role: :student, course: course}) + + params = %{ + "role" => "staff", + "crId" => user_course_reg.id + } + + resp = put(conn, build_url_users_role(course_id), params) + + assert response(resp, 200) == "OK" + updated_course_reg = Repo.get(CourseRegistration, user_course_reg.id) + assert updated_course_reg.role == :staff + end + + @tag authenticate: :admin + test "success (staff to student), when admin is admin of the course the user is in", %{ + conn: conn + } do + course_id = conn.assigns[:course_id] + course = Repo.get(Course, course_id) + user_course_reg = insert(:course_registration, %{role: :staff, course: course}) + + params = %{ + "role" => "student", + "crId" => user_course_reg.id + } + + resp = put(conn, build_url_users_role(course_id), params) + + assert response(resp, 200) == "OK" + updated_course_reg = Repo.get(CourseRegistration, user_course_reg.id) + assert updated_course_reg.role == :student + end + + @tag authenticate: :admin + test "success (admin to staff), when admin is admin of the course the user is in", %{ + conn: conn + } do + course_id = conn.assigns[:course_id] + course = Repo.get(Course, course_id) + user_course_reg = insert(:course_registration, %{role: :admin, course: course}) + + params = %{ + "role" => "staff", + "crId" => user_course_reg.id + } + + resp = put(conn, build_url_users_role(course_id), params) + + assert response(resp, 200) == "OK" + updated_course_reg = Repo.get(CourseRegistration, user_course_reg.id) + assert updated_course_reg.role == :staff + end + + @tag authenticate: :admin + test "fails, when course registration does not exist", %{conn: conn} do + course_id = conn.assigns[:course_id] + + params = %{ + "role" => "staff", + "crId" => 1 + } + + conn = put(conn, build_url_users_role(course_id), params) + + assert response(conn, 400) == "User course registration does not exist" + end + + @tag authenticate: :admin + test "fails, when admin is NOT admin of the course the user is in", %{conn: conn} do + course_id = conn.assigns[:course_id] + user_course_reg = insert(:course_registration, %{role: :student}) + + params = %{ + "role" => "staff", + "crId" => user_course_reg.id + } + + conn = put(conn, build_url_users_role(course_id), params) + + assert response(conn, 403) == "Wrong course" + unchanged_course_reg = Repo.get(CourseRegistration, user_course_reg.id) + assert unchanged_course_reg.role == :student + end + + @tag authenticate: :staff + test "fails, when staff attempts to make a role change", %{conn: conn} do + course_id = conn.assigns[:course_id] + course = Repo.get(Course, course_id) + user_course_reg = insert(:course_registration, %{role: :student, course: course}) + + params = %{ + "role" => "staff", + "crId" => user_course_reg.id + } + + conn = put(conn, build_url_users_role(course_id), params) + + assert response(conn, 403) == "User is not permitted to change others' roles" + unchanged_course_reg = Repo.get(CourseRegistration, user_course_reg.id) + assert unchanged_course_reg.role == :student + end + + @tag authenticate: :admin + test "fails, when invalid role is provided", %{conn: conn} do + course_id = conn.assigns[:course_id] + course = Repo.get(Course, course_id) + user_course_reg = insert(:course_registration, %{role: :student, course: course}) + + params = %{ + "role" => "avenger", + "crId" => user_course_reg.id + } + + conn = put(conn, build_url_users_role(course_id), params) + + assert response(conn, 400) == "role is invalid" + unchanged_course_reg = Repo.get(CourseRegistration, user_course_reg.id) + assert unchanged_course_reg.role == :student + end + end + + describe "DELETE /v2/courses/{course_id}/admin/users" do + @tag authenticate: :admin + test "success (delete student), when admin is admin of the course the user is in", %{ + conn: conn + } do + course_id = conn.assigns[:course_id] + course = Repo.get(Course, course_id) + user_course_reg = insert(:course_registration, %{role: :student, course: course}) + + params = %{ + "crId" => user_course_reg.id + } + + resp = delete(conn, build_url_users(course_id), params) + + assert response(resp, 200) == "OK" + assert Repo.get(CourseRegistration, user_course_reg.id) == nil + end + + @tag authenticate: :admin + test "success (delete staff), when admin is admin of the course the user is in", %{ + conn: conn + } do + course_id = conn.assigns[:course_id] + course = Repo.get(Course, course_id) + user_course_reg = insert(:course_registration, %{role: :staff, course: course}) + + params = %{ + "crId" => user_course_reg.id + } + + resp = delete(conn, build_url_users(course_id), params) + + assert response(resp, 200) == "OK" + assert Repo.get(CourseRegistration, user_course_reg.id) == nil + end + + @tag authenticate: :staff + test "fails when staff tries to delete user", %{ + conn: conn + } do + course_id = conn.assigns[:course_id] + course = Repo.get(Course, course_id) + user_course_reg = insert(:course_registration, %{role: :student, course: course}) + + params = %{ + "crId" => user_course_reg.id + } + + conn = delete(conn, build_url_users(course_id), params) + + assert response(conn, 403) == "User is not permitted to delete other users" + assert Repo.get(CourseRegistration, user_course_reg.id) != nil + end + + @tag authenticate: :admin + test "fails when admin tries to delete ownself", %{ + conn: conn + } do + course_id = conn.assigns[:course_id] + current_user = conn.assigns[:current_user] + + own_course_reg = + CourseRegistration + |> where(user_id: ^current_user.id) + |> where(course_id: ^course_id) + |> Repo.one() + + params = %{ + "crId" => own_course_reg.id + } + + conn = delete(conn, build_url_users(course_id), params) + + assert response(conn, 400) == "Admin not allowed to delete ownself from course" + assert Repo.get(CourseRegistration, own_course_reg.id) != nil + end + + @tag authenticate: :admin + test "fails when user course registration does not exist", %{ + conn: conn + } do + course_id = conn.assigns[:course_id] + + params = %{ + "crId" => 1 + } + + conn = delete(conn, build_url_users(course_id), params) + + assert response(conn, 400) == "User course registration does not exist" + end + + @tag authenticate: :admin + test "fails when deleting an admin", %{ + conn: conn + } do + course_id = conn.assigns[:course_id] + course = Repo.get(Course, course_id) + user_course_reg = insert(:course_registration, %{role: :admin, course: course}) + + params = %{ + "crId" => user_course_reg.id + } + + conn = delete(conn, build_url_users(course_id), params) + + assert response(conn, 400) == "Admins cannot be deleted" + end + + @tag authenticate: :admin + test "fails when deleting a user from another course", %{ + conn: conn + } do + course_id = conn.assigns[:course_id] + user_course_reg = insert(:course_registration, %{role: :student}) + + params = %{ + "crId" => user_course_reg.id + } + + conn = delete(conn, build_url_users(course_id), params) + + assert response(conn, 403) == "Wrong course" + end + end + + defp build_url_users(course_id), do: "/v2/courses/#{course_id}/admin/users" + defp build_url_users_role(course_id), do: build_url_users(course_id) <> "/role" end