From 0f6849ea10de947935218fffb5a02f7cedb2ff81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gr=C3=A9goire=20Charvet=20=E9=BB=91=E7=93=9C?=
 <greg@geekingfrog.com>
Date: Tue, 4 Jun 2024 05:08:32 +0100
Subject: [PATCH] Deny email change if email already taken (#306)

---
 lib/teiserver/data/cache_user.ex                    | 13 ++++++++++---
 lib/teiserver/protocols/spring/spring_in.ex         | 10 +++++-----
 .../teiserver/protocols/spring/spring_auth_test.exs |  9 +++++++++
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/lib/teiserver/data/cache_user.ex b/lib/teiserver/data/cache_user.ex
index 7a7e34566..c3bc15a5b 100644
--- a/lib/teiserver/data/cache_user.ex
+++ b/lib/teiserver/data/cache_user.ex
@@ -499,11 +499,18 @@ defmodule Teiserver.CacheUser do
     :ok
   end
 
-  def request_email_change(nil, _), do: nil
+  @spec request_email_change(T.user() | nil, String.t()) :: {:ok, T.user()} | {:error, String.t()}
+  def request_email_change(nil, _), do: {:error, "no user"}
 
   def request_email_change(user, new_email) do
-    code = :rand.uniform(899_999) + 100_000
-    update_user(%{user | email_change_code: ["#{code}", new_email]})
+    case get_user_by_email(new_email) do
+      nil ->
+        code = :rand.uniform(899_999) + 100_000
+        {:ok, update_user(%{user | email_change_code: ["#{code}", new_email]})}
+
+      _ ->
+        {:error, "Email already in use"}
+    end
   end
 
   @spec change_email(T.user(), String.t()) :: T.user()
diff --git a/lib/teiserver/protocols/spring/spring_in.ex b/lib/teiserver/protocols/spring/spring_in.ex
index 060d6d26a..8c6d42c18 100644
--- a/lib/teiserver/protocols/spring/spring_in.ex
+++ b/lib/teiserver/protocols/spring/spring_in.ex
@@ -392,14 +392,14 @@ defmodule Teiserver.Protocols.SpringIn do
   end
 
   defp do_handle("CHANGEEMAILREQUEST", new_email, msg_id, state) do
-    new_user = CacheUser.request_email_change(state.user, new_email)
+    result = CacheUser.request_email_change(state.user, new_email)
 
-    case new_user do
-      nil ->
-        reply(:change_email_request_denied, "no user", msg_id, state)
+    case result do
+      {:error, reason} ->
+        reply(:change_email_request_denied, reason, msg_id, state)
         state
 
-      _ ->
+      {:ok, new_user} ->
         reply(:change_email_request_accepted, nil, msg_id, state)
         %{state | user: new_user}
     end
diff --git a/test/teiserver/protocols/spring/spring_auth_test.exs b/test/teiserver/protocols/spring/spring_auth_test.exs
index 8a03b75e2..7dd4290bf 100644
--- a/test/teiserver/protocols/spring/spring_auth_test.exs
+++ b/test/teiserver/protocols/spring/spring_auth_test.exs
@@ -560,6 +560,15 @@ CLIENTS test_room #{user.name}\n"
     assert new_user.email_change_code == [nil, nil]
   end
 
+  test "CHANGEEMAIL to email already taken", %{socket: socket, user: user} do
+    other_user = Teiserver.TeiserverTestLib.new_user()
+
+    # Make the request
+    _send_raw(socket, "CHANGEEMAILREQUEST #{other_user.email}\n")
+    reply = _recv_raw(socket)
+    assert reply == "CHANGEEMAILREQUESTDENIED Email already in use\n"
+  end
+
   test "CREATEBOTACCOUNT - no mod", %{socket: socket, user: user} do
     _send_raw(socket, "CREATEBOTACCOUNT test_bot_account_no_mod #{user.name}\n")
     reply = _recv_raw(socket)