Merge branch 'bugfix/captcha-nil-answer_data' into 'develop'

Bugfix: return invalid when answer_data is nil

Closes #1585

See merge request pleroma/pleroma!2236

lib/pleroma/captcha/captcha.ex

@@ -1,5 +1,5 @@
 # Pleroma: A lightweight social networking server
-# Copyright © 2017-2019 Pleroma Authors <https://pleroma.social/>
+# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/>
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Captcha do
@@ -50,7 +50,7 @@ def handle_call(:new, _from, state) do
       token = new_captcha[:token]
       secret = KeyGenerator.generate(secret_key_base, token <> "_encrypt")
       sign_secret = KeyGenerator.generate(secret_key_base, token <> "_sign")
-      # Basicallty copy what Phoenix.Token does here, add the time to
+      # Basically copy what Phoenix.Token does here, add the time to
       # the actual data and make it a binary to then encrypt it
       encrypted_captcha_answer =
         %{
@@ -62,7 +62,7 @@ def handle_call(:new, _from, state) do
 
       {
         :reply,
-        # Repalce the answer with the encrypted answer
+        # Replace the answer with the encrypted answer
         %{new_captcha | answer_data: encrypted_captcha_answer},
         state
       }
@@ -82,7 +82,8 @@ def handle_call({:validate, token, captcha, answer_data}, _from, state) do
     valid_if_after = DateTime.subtract!(DateTime.now_utc(), seconds_valid)
 
     result =
-      with {:ok, data} <- MessageEncryptor.decrypt(answer_data, secret, sign_secret),
+      with false <- is_nil(answer_data),
+           {:ok, data} <- MessageEncryptor.decrypt(answer_data, secret, sign_secret),
            %{at: at, answer_data: answer_md5} <- :erlang.binary_to_term(data) do
         try do
           if DateTime.before?(at, valid_if_after),

test/captcha_test.exs

@@ -1,17 +1,20 @@
 # Pleroma: A lightweight social networking server
-# Copyright © 2017-2019 Pleroma Authors <https://pleroma.social/>
+# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/>
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.CaptchaTest do
-  use ExUnit.Case
+  use Pleroma.DataCase
 
   import Tesla.Mock
 
+  alias Pleroma.Captcha
   alias Pleroma.Captcha.Kocaptcha
   alias Pleroma.Captcha.Native
 
   @ets_options [:ordered_set, :private, :named_table, {:read_concurrency, true}]
 
+  clear_config([Pleroma.Captcha, :enabled])
+
   describe "Kocaptcha" do
     setup do
       ets_name = Kocaptcha.Ets
@@ -31,17 +34,18 @@ defmodule Pleroma.CaptchaTest do
 
     test "new and validate" do
       new = Kocaptcha.new()
-      assert new[:type] == :kocaptcha
-      assert new[:token] == "afa1815e14e29355e6c8f6b143a39fa2"
 
-      assert new[:url] ==
-               "https://captcha.kotobank.ch/captchas/afa1815e14e29355e6c8f6b143a39fa2.png"
+      token = "afa1815e14e29355e6c8f6b143a39fa2"
+      url = "https://captcha.kotobank.ch/captchas/afa1815e14e29355e6c8f6b143a39fa2.png"
 
-      assert Kocaptcha.validate(
-               new[:token],
-               "7oEy8c",
-               new[:answer_data]
-             ) == :ok
+      assert %{
+               answer_data: answer,
+               token: ^token,
+               url: ^url,
+               type: :kocaptcha
+             } = new
+
+      assert Kocaptcha.validate(token, "7oEy8c", answer) == :ok
     end
   end
 
@@ -61,4 +65,52 @@ test "new and validate" do
       assert {:error, "Invalid CAPTCHA"} == Native.validate(token, answer, answer <> "foobar")
     end
   end
+
+  describe "Captcha Wrapper" do
+    test "validate" do
+      Pleroma.Config.put([Pleroma.Captcha, :enabled], true)
+
+      new = Captcha.new()
+
+      assert %{
+               answer_data: answer,
+               token: token
+             } = new
+
+      assert is_binary(answer)
+      assert :ok = Captcha.validate(token, "63615261b77f5354fb8c4e4986477555", answer)
+    end
+
+    test "doesn't validate invalid answer" do
+      Pleroma.Config.put([Pleroma.Captcha, :enabled], true)
+
+      new = Captcha.new()
+
+      assert %{
+               answer_data: answer,
+               token: token
+             } = new
+
+      assert is_binary(answer)
+
+      assert {:error, "Invalid answer data"} =
+               Captcha.validate(token, "63615261b77f5354fb8c4e4986477555", answer <> "foobar")
+    end
+
+    test "nil answer_data" do
+      Pleroma.Config.put([Pleroma.Captcha, :enabled], true)
+
+      new = Captcha.new()
+
+      assert %{
+               answer_data: answer,
+               token: token
+             } = new
+
+      assert is_binary(answer)
+
+      assert {:error, "Invalid answer data"} =
+               Captcha.validate(token, "63615261b77f5354fb8c4e4986477555", nil)
+    end
+  end
 end

test/support/captcha_mock.ex

@@ -1,5 +1,5 @@
 # Pleroma: A lightweight social networking server
-# Copyright © 2017-2019 Pleroma Authors <https://pleroma.social/>
+# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/>
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Captcha.Mock do
@@ -7,8 +7,17 @@ defmodule Pleroma.Captcha.Mock do
   @behaviour Service
 
   @impl Service
-  def new, do: %{type: :mock}
+  def new,
+    do: %{
+      type: :mock,
+      token: "afa1815e14e29355e6c8f6b143a39fa2",
+      answer_data: "63615261b77f5354fb8c4e4986477555",
+      url: "https://example.org/captcha.png"
+    }
 
   @impl Service
-  def validate(_token, _captcha, _data), do: :ok
+  def validate(_token, captcha, captcha) when not is_nil(captcha), do: :ok
+
+  def validate(_token, captcha, answer),
+    do: {:error, "Invalid CAPTCHA captcha: #{inspect(captcha)} ; answer: #{inspect(answer)}"}
 end