From a626cb682cc8fd6cad91484db064ed22646960af Mon Sep 17 00:00:00 2001 From: fence Date: Mon, 27 Apr 2020 17:55:33 +0200 Subject: [PATCH 1/6] secure mongoose auth endpoint --- .../web/mongooseim/mongoose_im_controller.ex | 37 +++++++++++++------ 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/lib/pleroma/web/mongooseim/mongoose_im_controller.ex b/lib/pleroma/web/mongooseim/mongoose_im_controller.ex index 04d823b36..744cf5227 100644 --- a/lib/pleroma/web/mongooseim/mongoose_im_controller.ex +++ b/lib/pleroma/web/mongooseim/mongoose_im_controller.ex @@ -26,21 +26,36 @@ def user_exists(conn, %{"user" => username}) do end def check_password(conn, %{"user" => username, "pass" => password}) do - with %User{password_hash: password_hash} <- - Repo.get_by(User, nickname: username, local: true), - true <- Pbkdf2.checkpw(password, password_hash) do - conn - |> json(true) - else - false -> - conn - |> put_status(:forbidden) - |> json(false) + user = Repo.get_by(User, nickname: username, local: true) - _ -> + case User.account_status(user) do + :deactivated -> conn |> put_status(:not_found) |> json(false) + + :confirmation_pending -> + conn + |> put_status(:not_found) + |> json(false) + + _ -> + with %User{password_hash: password_hash} <- + user, + true <- Pbkdf2.checkpw(password, password_hash) do + conn + |> json(true) + else + false -> + conn + |> put_status(:forbidden) + |> json(false) + + _ -> + conn + |> put_status(:not_found) + |> json(false) + end end end end From 5c7cc109172c84b991fad7eebbdd51e75f0c5382 Mon Sep 17 00:00:00 2001 From: fence Date: Mon, 27 Apr 2020 18:31:00 +0200 Subject: [PATCH 2/6] add tests for deactivated users for mongoose auth --- lib/pleroma/web/mongooseim/mongoose_im_controller.ex | 7 ++++++- test/web/mongooseim/mongoose_im_controller_test.exs | 9 +++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/mongooseim/mongoose_im_controller.ex b/lib/pleroma/web/mongooseim/mongoose_im_controller.ex index 744cf5227..c15b4bfb8 100644 --- a/lib/pleroma/web/mongooseim/mongoose_im_controller.ex +++ b/lib/pleroma/web/mongooseim/mongoose_im_controller.ex @@ -27,8 +27,13 @@ def user_exists(conn, %{"user" => username}) do def check_password(conn, %{"user" => username, "pass" => password}) do user = Repo.get_by(User, nickname: username, local: true) + + state = case user do + nil -> nil + _ -> User.account_status(user) + end - case User.account_status(user) do + case state do :deactivated -> conn |> put_status(:not_found) diff --git a/test/web/mongooseim/mongoose_im_controller_test.exs b/test/web/mongooseim/mongoose_im_controller_test.exs index 291ae54fc..5987111e5 100644 --- a/test/web/mongooseim/mongoose_im_controller_test.exs +++ b/test/web/mongooseim/mongoose_im_controller_test.exs @@ -34,6 +34,7 @@ test "/user_exists", %{conn: conn} do test "/check_password", %{conn: conn} do user = insert(:user, password_hash: Comeonin.Pbkdf2.hashpwsalt("cool")) + _deactivated_user = insert(:user, nickname: "konata", local: false, deactivated: true) res = conn @@ -49,6 +50,14 @@ test "/check_password", %{conn: conn} do assert res == false + res = + conn + |> get(mongoose_im_path(conn, :check_password), user: "konata", pass: "1337") + |> json_response(404) + + assert res == false + + res = conn |> get(mongoose_im_path(conn, :check_password), user: "nobody", pass: "cool") From cc1e2e8d0f5fa27d051a0a21740a8052b95ce1a5 Mon Sep 17 00:00:00 2001 From: fence Date: Mon, 27 Apr 2020 19:11:03 +0200 Subject: [PATCH 3/6] requested changes to mongoose_im_controller.ex --- .../web/mongooseim/mongoose_im_controller.ex | 45 ++++++------------- 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/lib/pleroma/web/mongooseim/mongoose_im_controller.ex b/lib/pleroma/web/mongooseim/mongoose_im_controller.ex index c15b4bfb8..7123153c5 100644 --- a/lib/pleroma/web/mongooseim/mongoose_im_controller.ex +++ b/lib/pleroma/web/mongooseim/mongoose_im_controller.ex @@ -14,7 +14,7 @@ defmodule Pleroma.Web.MongooseIM.MongooseIMController do plug(RateLimiter, [name: :authentication, params: ["user"]] when action == :check_password) def user_exists(conn, %{"user" => username}) do - with %User{} <- Repo.get_by(User, nickname: username, local: true) do + with %User{} <- Repo.get_by(User, nickname: username, local: true, deactivated: false) do conn |> json(true) else @@ -26,41 +26,22 @@ def user_exists(conn, %{"user" => username}) do end def check_password(conn, %{"user" => username, "pass" => password}) do - user = Repo.get_by(User, nickname: username, local: true) - - state = case user do - nil -> nil - _ -> User.account_status(user) - end - - case state do - :deactivated -> + with %User{password_hash: password_hash, deactivated: false} <- + Repo.get_by(User, nickname: username, local: true), + true <- Pbkdf2.checkpw(password, password_hash) do + conn + |> json(true) + else + false -> conn - |> put_status(:not_found) - |> json(false) - - :confirmation_pending -> - conn - |> put_status(:not_found) + |> put_status(:forbidden) |> json(false) _ -> - with %User{password_hash: password_hash} <- - user, - true <- Pbkdf2.checkpw(password, password_hash) do - conn - |> json(true) - else - false -> - conn - |> put_status(:forbidden) - |> json(false) - - _ -> - conn - |> put_status(:not_found) - |> json(false) - end + conn + |> put_status(:not_found) + |> json(false) end end end + From 935ca2c1329fd4b4f2b40a5ed7f2c1fa5f95b9bd Mon Sep 17 00:00:00 2001 From: fence Date: Mon, 27 Apr 2020 19:16:05 +0200 Subject: [PATCH 4/6] requested changes to mongoose test --- test/web/mongooseim/mongoose_im_controller_test.exs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/web/mongooseim/mongoose_im_controller_test.exs b/test/web/mongooseim/mongoose_im_controller_test.exs index 5987111e5..2b4d124af 100644 --- a/test/web/mongooseim/mongoose_im_controller_test.exs +++ b/test/web/mongooseim/mongoose_im_controller_test.exs @@ -9,6 +9,7 @@ defmodule Pleroma.Web.MongooseIMController do test "/user_exists", %{conn: conn} do _user = insert(:user, nickname: "lain") _remote_user = insert(:user, nickname: "alice", local: false) + _deactivated_user = insert(:user, nickname: "konata", deactivated: true) res = conn @@ -30,11 +31,18 @@ test "/user_exists", %{conn: conn} do |> json_response(404) assert res == false + + res = + conn + |> get(mongoose_im_path(conn, :user_exists), user: "konata") + |> json_response(404) + + assert res == false end test "/check_password", %{conn: conn} do user = insert(:user, password_hash: Comeonin.Pbkdf2.hashpwsalt("cool")) - _deactivated_user = insert(:user, nickname: "konata", local: false, deactivated: true) + _deactivated_user = insert(:user, nickname: "konata", deactivated: true) res = conn @@ -52,7 +60,7 @@ test "/check_password", %{conn: conn} do res = conn - |> get(mongoose_im_path(conn, :check_password), user: "konata", pass: "1337") + |> get(mongoose_im_path(conn, :check_password), user: "konata", pass: "cool") |> json_response(404) assert res == false From d607b4d84002ff14f51713f1ac74a4971e2dffac Mon Sep 17 00:00:00 2001 From: fence Date: Mon, 27 Apr 2020 19:32:58 +0200 Subject: [PATCH 5/6] mongooseim test: explicitly set password for the deactivated used --- test/web/mongooseim/mongoose_im_controller_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/web/mongooseim/mongoose_im_controller_test.exs b/test/web/mongooseim/mongoose_im_controller_test.exs index 2b4d124af..d17f8dbb4 100644 --- a/test/web/mongooseim/mongoose_im_controller_test.exs +++ b/test/web/mongooseim/mongoose_im_controller_test.exs @@ -42,7 +42,7 @@ test "/user_exists", %{conn: conn} do test "/check_password", %{conn: conn} do user = insert(:user, password_hash: Comeonin.Pbkdf2.hashpwsalt("cool")) - _deactivated_user = insert(:user, nickname: "konata", deactivated: true) + _deactivated_user = insert(:user, nickname: "konata", deactivated: true, password_hash: Comeonin.Pbkdf2.hashpwsalt("cool")) res = conn From 5ff20793e739daa962cdc1623c01dc6ec1ff8a61 Mon Sep 17 00:00:00 2001 From: fence Date: Tue, 28 Apr 2020 01:29:31 +0200 Subject: [PATCH 6/6] formating --- lib/pleroma/web/mongooseim/mongoose_im_controller.ex | 1 - test/web/mongooseim/mongoose_im_controller_test.exs | 9 +++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/web/mongooseim/mongoose_im_controller.ex b/lib/pleroma/web/mongooseim/mongoose_im_controller.ex index 7123153c5..1ed6ee521 100644 --- a/lib/pleroma/web/mongooseim/mongoose_im_controller.ex +++ b/lib/pleroma/web/mongooseim/mongoose_im_controller.ex @@ -44,4 +44,3 @@ def check_password(conn, %{"user" => username, "pass" => password}) do end end end - diff --git a/test/web/mongooseim/mongoose_im_controller_test.exs b/test/web/mongooseim/mongoose_im_controller_test.exs index d17f8dbb4..1ac2f2c27 100644 --- a/test/web/mongooseim/mongoose_im_controller_test.exs +++ b/test/web/mongooseim/mongoose_im_controller_test.exs @@ -42,7 +42,13 @@ test "/user_exists", %{conn: conn} do test "/check_password", %{conn: conn} do user = insert(:user, password_hash: Comeonin.Pbkdf2.hashpwsalt("cool")) - _deactivated_user = insert(:user, nickname: "konata", deactivated: true, password_hash: Comeonin.Pbkdf2.hashpwsalt("cool")) + + _deactivated_user = + insert(:user, + nickname: "konata", + deactivated: true, + password_hash: Comeonin.Pbkdf2.hashpwsalt("cool") + ) res = conn @@ -65,7 +71,6 @@ test "/check_password", %{conn: conn} do assert res == false - res = conn |> get(mongoose_im_path(conn, :check_password), user: "nobody", pass: "cool")