Merge branch 'bugfix/change_password' into 'develop'

TwitterAPI: Make change_password require body params instead of query

Closes #2740

See merge request pleroma/pleroma!3503

lib/pleroma/web/api_spec/operations/twitter_util_operation.ex

@@ -8,6 +8,8 @@ defmodule Pleroma.Web.ApiSpec.TwitterUtilOperation do
   alias Pleroma.Web.ApiSpec.Schemas.ApiError
   alias Pleroma.Web.ApiSpec.Schemas.BooleanLike
 
+  import Pleroma.Web.ApiSpec.Helpers
+
   def open_api_operation(action) do
     operation = String.to_existing_atom("#{action}_operation")
     apply(__MODULE__, operation, [])
@@ -63,17 +65,7 @@ def change_password_operation do
       summary: "Change account password",
       security: [%{"oAuth" => ["write:accounts"]}],
       operationId: "UtilController.change_password",
-      parameters: [
+      requestBody: request_body("Parameters", change_password_request(), required: true),
-        Operation.parameter(:password, :query, :string, "Current password", required: true),
-        Operation.parameter(:new_password, :query, :string, "New password", required: true),
-        Operation.parameter(
-          :new_password_confirmation,
-          :query,
-          :string,
-          "New password, confirmation",
-          required: true
-        )
-      ],
       responses: %{
         200 =>
           Operation.response("Success", "application/json", %Schema{
@@ -86,17 +78,30 @@ def change_password_operation do
     }
   end
 
+  defp change_password_request do
+    %Schema{
+      title: "ChangePasswordRequest",
+      description: "POST body for changing the account's passowrd",
+      type: :object,
+      required: [:password, :new_password, :new_password_confirmation],
+      properties: %{
+        password: %Schema{type: :string, description: "Current password"},
+        new_password: %Schema{type: :string, description: "New password"},
+        new_password_confirmation: %Schema{
+          type: :string,
+          description: "New password, confirmation"
+        }
+      }
+    }
+  end
+
   def change_email_operation do
     %Operation{
       tags: ["Account credentials"],
       summary: "Change account email",
       security: [%{"oAuth" => ["write:accounts"]}],
       operationId: "UtilController.change_email",
-      parameters: [
+      requestBody: request_body("Parameters", change_email_request(), required: true),
-        Operation.parameter(:password, :query, :string, "Current password", required: true),
-        Operation.parameter(:email, :query, :string, "New email", required: true)
-      ],
-      requestBody: nil,
       responses: %{
         200 =>
           Operation.response("Success", "application/json", %Schema{
@@ -109,6 +114,19 @@ def change_email_operation do
     }
   end
 
+  defp change_email_request do
+    %Schema{
+      title: "ChangeEmailRequest",
+      description: "POST body for changing the account's email",
+      type: :object,
+      required: [:email, :password],
+      properties: %{
+        email: %Schema{type: :string, description: "New email"},
+        password: %Schema{type: :string, description: "Current password"}
+      }
+    }
+  end
+
   def update_notificaton_settings_operation do
     %Operation{
       tags: ["Accounts"],

lib/pleroma/web/twitter_api/controllers/util_controller.ex

@@ -81,17 +81,13 @@ def update_notificaton_settings(%{assigns: %{user: user}} = conn, params) do
     end
   end
 
-  def change_password(%{assigns: %{user: user}} = conn, %{
+  def change_password(%{assigns: %{user: user}, body_params: body_params} = conn, %{}) do
-        password: password,
+    case CommonAPI.Utils.confirm_current_password(user, body_params.password) do
-        new_password: new_password,
-        new_password_confirmation: new_password_confirmation
-      }) do
-    case CommonAPI.Utils.confirm_current_password(user, password) do
       {:ok, user} ->
         with {:ok, _user} <-
                User.reset_password(user, %{
-                 password: new_password,
+                 password: body_params.new_password,
-                 password_confirmation: new_password_confirmation
+                 password_confirmation: body_params.new_password_confirmation
                }) do
           json(conn, %{status: "success"})
         else
@@ -108,10 +104,10 @@ def change_password(%{assigns: %{user: user}} = conn, %{
     end
   end
 
-  def change_email(%{assigns: %{user: user}} = conn, %{password: password, email: email}) do
+  def change_email(%{assigns: %{user: user}, body_params: body_params} = conn, %{}) do
-    case CommonAPI.Utils.confirm_current_password(user, password) do
+    case CommonAPI.Utils.confirm_current_password(user, body_params.password) do
       {:ok, user} ->
-        with {:ok, _user} <- User.change_email(user, email) do
+        with {:ok, _user} <- User.change_email(user, body_params.email) do
           json(conn, %{status: "success"})
         else
           {:error, changeset} ->

test/pleroma/web/twitter_api/util_controller_test.exs

@@ -261,11 +261,8 @@ test "without permissions", %{conn: conn} do
       conn =
         conn
         |> assign(:token, nil)
-        |> post(
+        |> put_req_header("content-type", "multipart/form-data")
-          "/api/pleroma/change_email?#{
+        |> post("/api/pleroma/change_email", %{password: "hi", email: "test@test.com"})
-            URI.encode_query(%{password: "hi", email: "test@test.com"})
-          }"
-        )
 
       assert json_response_and_validate_schema(conn, 403) == %{
                "error" => "Insufficient permissions: write:accounts."
@@ -274,12 +271,9 @@ test "without permissions", %{conn: conn} do
 
     test "with proper permissions and invalid password", %{conn: conn} do
       conn =
-        post(
+        conn
-          conn,
+        |> put_req_header("content-type", "multipart/form-data")
-          "/api/pleroma/change_email?#{
+        |> post("/api/pleroma/change_email", %{password: "hi", email: "test@test.com"})
-            URI.encode_query(%{password: "hi", email: "test@test.com"})
-          }"
-        )
 
       assert json_response_and_validate_schema(conn, 200) == %{"error" => "Invalid password."}
     end
@@ -288,10 +282,9 @@ test "with proper permissions, valid password and invalid email", %{
       conn: conn
     } do
       conn =
-        post(
+        conn
-          conn,
+        |> put_req_header("content-type", "multipart/form-data")
-          "/api/pleroma/change_email?#{URI.encode_query(%{password: "test", email: "foobar"})}"
+        |> post("/api/pleroma/change_email", %{password: "test", email: "foobar"})
-        )
 
       assert json_response_and_validate_schema(conn, 200) == %{
                "error" => "Email has invalid format."
@@ -301,7 +294,10 @@ test "with proper permissions, valid password and invalid email", %{
     test "with proper permissions, valid password and no email", %{
       conn: conn
     } do
-      conn = post(conn, "/api/pleroma/change_email?#{URI.encode_query(%{password: "test"})}")
+      conn =
+        conn
+        |> put_req_header("content-type", "multipart/form-data")
+        |> post("/api/pleroma/change_email", %{password: "test"})
 
       assert %{"error" => "Missing field: email."} = json_response_and_validate_schema(conn, 400)
     end
@@ -310,10 +306,9 @@ test "with proper permissions, valid password and blank email", %{
       conn: conn
     } do
       conn =
-        post(
+        conn
-          conn,
+        |> put_req_header("content-type", "multipart/form-data")
-          "/api/pleroma/change_email?#{URI.encode_query(%{password: "test", email: ""})}"
+        |> post("/api/pleroma/change_email", %{password: "test", email: ""})
-        )
 
       assert json_response_and_validate_schema(conn, 200) == %{"error" => "Email can't be blank."}
     end
@@ -324,10 +319,9 @@ test "with proper permissions, valid password and non unique email", %{
       user = insert(:user)
 
       conn =
-        post(
+        conn
-          conn,
+        |> put_req_header("content-type", "multipart/form-data")
-          "/api/pleroma/change_email?#{URI.encode_query(%{password: "test", email: user.email})}"
+        |> post("/api/pleroma/change_email", %{password: "test", email: user.email})
-        )
 
       assert json_response_and_validate_schema(conn, 200) == %{
                "error" => "Email has already been taken."
@@ -338,12 +332,9 @@ test "with proper permissions, valid password and valid email", %{
       conn: conn
     } do
       conn =
-        post(
+        conn
-          conn,
+        |> put_req_header("content-type", "multipart/form-data")
-          "/api/pleroma/change_email?#{
+        |> post("/api/pleroma/change_email", %{password: "test", email: "cofe@foobar.com"})
-            URI.encode_query(%{password: "test", email: "cofe@foobar.com"})
-          }"
-        )
 
       assert json_response_and_validate_schema(conn, 200) == %{"status" => "success"}
     end
@@ -356,15 +347,12 @@ test "without permissions", %{conn: conn} do
       conn =
         conn
         |> assign(:token, nil)
-        |> post(
+        |> put_req_header("content-type", "multipart/form-data")
-          "/api/pleroma/change_password?#{
+        |> post("/api/pleroma/change_password", %{
-            URI.encode_query(%{
+          "password" => "hi",
-              password: "hi",
+          "new_password" => "newpass",
-              new_password: "newpass",
+          "new_password_confirmation" => "newpass"
-              new_password_confirmation: "newpass"
+        })
-            })
-          }"
-        )
 
       assert json_response_and_validate_schema(conn, 403) == %{
                "error" => "Insufficient permissions: write:accounts."
@@ -373,16 +361,13 @@ test "without permissions", %{conn: conn} do
 
     test "with proper permissions and invalid password", %{conn: conn} do
       conn =
-        post(
+        conn
-          conn,
+        |> put_req_header("content-type", "multipart/form-data")
-          "/api/pleroma/change_password?#{
+        |> post("/api/pleroma/change_password", %{
-            URI.encode_query(%{
+          "password" => "hi",
-              password: "hi",
+          "new_password" => "newpass",
-              new_password: "newpass",
+          "new_password_confirmation" => "newpass"
-              new_password_confirmation: "newpass"
+        })
-            })
-          }"
-        )
 
       assert json_response_and_validate_schema(conn, 200) == %{"error" => "Invalid password."}
     end
@@ -392,16 +377,13 @@ test "with proper permissions, valid password and new password and confirmation
            conn: conn
          } do
       conn =
-        post(
+        conn
-          conn,
+        |> put_req_header("content-type", "multipart/form-data")
-          "/api/pleroma/change_password?#{
+        |> post("/api/pleroma/change_password", %{
-            URI.encode_query(%{
+          "password" => "test",
-              password: "test",
+          "new_password" => "newpass",
-              new_password: "newpass",
+          "new_password_confirmation" => "notnewpass"
-              new_password_confirmation: "notnewpass"
+        })
-            })
-          }"
-        )
 
       assert json_response_and_validate_schema(conn, 200) == %{
                "error" => "New password does not match confirmation."
@@ -412,12 +394,13 @@ test "with proper permissions, valid password and invalid new password", %{
       conn: conn
     } do
       conn =
-        post(
+        conn
-          conn,
+        |> put_req_header("content-type", "multipart/form-data")
-          "/api/pleroma/change_password?#{
+        |> post("/api/pleroma/change_password", %{
-            URI.encode_query(%{password: "test", new_password: "", new_password_confirmation: ""})
+          password: "test",
-          }"
+          new_password: "",
-        )
+          new_password_confirmation: ""
+        })
 
       assert json_response_and_validate_schema(conn, 200) == %{
                "error" => "New password can't be blank."
@@ -429,15 +412,15 @@ test "with proper permissions, valid password and matching new password and conf
       user: user
     } do
       conn =
-        post(
+        conn
-          conn,
+        |> put_req_header("content-type", "multipart/form-data")
-          "/api/pleroma/change_password?#{
+        |> post(
-            URI.encode_query(%{
+          "/api/pleroma/change_password",
-              password: "test",
+          %{
-              new_password: "newpass",
+            password: "test",
-              new_password_confirmation: "newpass"
+            new_password: "newpass",
-            })
+            new_password_confirmation: "newpass"
-          }"
+          }
         )
 
       assert json_response_and_validate_schema(conn, 200) == %{"status" => "success"}