From 3249141588c8f73f1958f782041798fbde05e69f Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Wed, 27 May 2020 09:42:28 +0300 Subject: [PATCH 1/2] validate actor type --- docs/API/admin_api.md | 18 +++++++++- lib/pleroma/user.ex | 5 +-- .../controllers/admin_api_controller.ex | 13 ++++--- .../controllers/admin_api_controller_test.exs | 35 ++++++++++++++++--- 4 files changed, 60 insertions(+), 11 deletions(-) diff --git a/docs/API/admin_api.md b/docs/API/admin_api.md index c455047cc..639c3224d 100644 --- a/docs/API/admin_api.md +++ b/docs/API/admin_api.md @@ -511,7 +511,23 @@ Note: Available `:permission_group` is currently moderator and admin. 404 is ret - `discoverable` - `actor_type` -- Response: none (code `200`) +- Response: + +```json +{"status": "success"} +``` + +```json +{"errors": + {"actor_type": "is invalid"}, + {"email": "has invalid format"}, + ... + } +``` + +```json +{"error": "Unable to update user."} +``` ## `GET /api/pleroma/admin/reports` diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 842b28c06..2684e1139 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -538,9 +538,10 @@ defmodule Pleroma.User do |> delete_change(:also_known_as) |> unique_constraint(:email) |> validate_format(:email, @email_regex) + |> validate_inclusion(:actor_type, ["Person", "Service"]) end - @spec update_as_admin(%User{}, map) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()} + @spec update_as_admin(User.t(), map()) :: {:ok, User.t()} | {:error, Changeset.t()} def update_as_admin(user, params) do params = Map.put(params, "password_confirmation", params["password"]) changeset = update_as_admin_changeset(user, params) @@ -561,7 +562,7 @@ defmodule Pleroma.User do |> put_change(:password_reset_pending, false) end - @spec reset_password(User.t(), map) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()} + @spec reset_password(User.t(), map()) :: {:ok, User.t()} | {:error, Changeset.t()} def reset_password(%User{} = user, params) do reset_password(user, user, params) end diff --git a/lib/pleroma/web/admin_api/controllers/admin_api_controller.ex b/lib/pleroma/web/admin_api/controllers/admin_api_controller.ex index 6b1d64a2e..6aedccec6 100644 --- a/lib/pleroma/web/admin_api/controllers/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/controllers/admin_api_controller.ex @@ -693,7 +693,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do %{assigns: %{user: admin}} = conn, %{"nickname" => nickname} = params ) do - with {_, user} <- {:user, User.get_cached_by_nickname(nickname)}, + with {_, %User{} = user} <- {:user, User.get_cached_by_nickname(nickname)}, {:ok, _user} <- User.update_as_admin(user, params) do ModerationLog.insert_log(%{ @@ -715,11 +715,16 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do json(conn, %{status: "success"}) else {:error, changeset} -> - {_, {error, _}} = Enum.at(changeset.errors, 0) - json(conn, %{error: "New password #{error}."}) + errors = + Enum.reduce(changeset.errors, %{}, fn + {key, {error, _}}, acc -> + Map.put(acc, key, error) + end) + + json(conn, %{errors: errors}) _ -> - json(conn, %{error: "Unable to change password."}) + json(conn, %{error: "Unable to update user."}) end end diff --git a/test/web/admin_api/controllers/admin_api_controller_test.exs b/test/web/admin_api/controllers/admin_api_controller_test.exs index 321840a8c..ead840186 100644 --- a/test/web/admin_api/controllers/admin_api_controller_test.exs +++ b/test/web/admin_api/controllers/admin_api_controller_test.exs @@ -3191,8 +3191,12 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do end describe "PATCH /users/:nickname/credentials" do - test "changes password and email", %{conn: conn, admin: admin} do + setup do user = insert(:user) + [user: user] + end + + test "changes password and email", %{conn: conn, admin: admin, user: user} do assert user.password_reset_pending == false conn = @@ -3222,9 +3226,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do "@#{admin.nickname} forced password reset for users: @#{user.nickname}" end - test "returns 403 if requested by a non-admin" do - user = insert(:user) - + test "returns 403 if requested by a non-admin", %{user: user} do conn = build_conn() |> assign(:user, user) @@ -3236,6 +3238,31 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do assert json_response(conn, :forbidden) end + + test "changes actor type from permitted list", %{conn: conn, user: user} do + assert user.actor_type == "Person" + + assert patch(conn, "/api/pleroma/admin/users/#{user.nickname}/credentials", %{ + "actor_type" => "Service" + }) + |> json_response(200) == %{"status" => "success"} + + updated_user = User.get_by_id(user.id) + + assert updated_user.actor_type == "Service" + + assert patch(conn, "/api/pleroma/admin/users/#{user.nickname}/credentials", %{ + "actor_type" => "Application" + }) + |> json_response(200) == %{"errors" => %{"actor_type" => "is invalid"}} + end + + test "update non existing user", %{conn: conn} do + assert patch(conn, "/api/pleroma/admin/users/non-existing/credentials", %{ + "password" => "new_password" + }) + |> json_response(200) == %{"error" => "Unable to update user."} + end end describe "PATCH /users/:nickname/force_password_reset" do From 047a11c48f2bc88b6b278b6a5acd94807c7e5138 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Wed, 27 May 2020 10:55:42 +0000 Subject: [PATCH 2/2] Apply suggestion to lib/pleroma/web/admin_api/controllers/admin_api_controller.ex --- .../web/admin_api/controllers/admin_api_controller.ex | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/pleroma/web/admin_api/controllers/admin_api_controller.ex b/lib/pleroma/web/admin_api/controllers/admin_api_controller.ex index 6aedccec6..783203c07 100644 --- a/lib/pleroma/web/admin_api/controllers/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/controllers/admin_api_controller.ex @@ -715,11 +715,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do json(conn, %{status: "success"}) else {:error, changeset} -> - errors = - Enum.reduce(changeset.errors, %{}, fn - {key, {error, _}}, acc -> - Map.put(acc, key, error) - end) + errors = Map.new(changeset.errors, fn {key, {error, _}} -> {key, error} end) json(conn, %{errors: errors})