Stronger credential stuffing protection for 1.2 release series

config/dev.exs

@@ -72,6 +72,9 @@ config :philomena, PhilomenaWeb.Endpoint,
     ]
   ]
 
+# Disable Pwned Passwords API check in development
+config :philomena, pwned_passwords: false
+
 # Relax CSP rules in development
 config :philomena, csp_relaxed: true
 

lib/philomena/users.ex

@@ -407,6 +407,14 @@ defmodule Philomena.Users do
     :ok
   end
 
+  @doc """
+  Deletes every active session, including incomplete TOTP login sessions, for a user.
+  """
+  def delete_user_sessions(user) do
+    Repo.delete_all(UserToken.user_and_contexts_query(user, ["session", "totp"]))
+    :ok
+  end
+
   @doc """
   Deletes the signed token with the given context.
   """

lib/philomena_proxy/http.ex

@@ -39,7 +39,7 @@ defmodule PhilomenaProxy.Http do
   """
   @spec get(url(), header_list()) :: result()
   def get(url, headers \\ []) do
-    request(:get, url, [], headers)
+    request(:get, url, nil, headers)
   end
 
   @doc ~S"""
@@ -56,7 +56,7 @@ defmodule PhilomenaProxy.Http do
   """
   @spec head(url(), header_list()) :: result()
   def head(url, headers \\ []) do
-    request(:head, url, [], headers)
+    request(:head, url, nil, headers)
   end
 
   @doc ~S"""
@@ -76,7 +76,7 @@ defmodule PhilomenaProxy.Http do
     request(:post, url, body, headers)
   end
 
-  @spec request(atom(), String.t(), iodata(), header_list()) :: result()
+  @spec request(atom(), String.t(), iodata() | nil, header_list()) :: result()
   defp request(method, url, body, headers) do
     Req.new(
       method: method,

lib/philomena_web/controllers/session_controller.ex

@@ -3,6 +3,10 @@ defmodule PhilomenaWeb.SessionController do
 
   alias Philomena.Users
   alias PhilomenaWeb.UserAuth
+  alias PhilomenaWeb.CompromisedPasswordCheckPlug
+
+  plug PhilomenaWeb.CaptchaPlug when action in [:new, :create]
+  plug PhilomenaWeb.CheckCaptchaPlug when action in [:create]
 
   def new(conn, _params) do
     render(conn, "new.html", error_message: nil)
@@ -19,6 +23,16 @@ defmodule PhilomenaWeb.SessionController do
       )
 
     cond do
+      not is_nil(user) and CompromisedPasswordCheckPlug.password_compromised?(password) ->
+        Users.delete_user_sessions(user)
+
+        conn
+        |> put_flash(
+          :error,
+          "We've detected that the password you entered has been compromised during a data breach of another website. Please reset your password before signing in again."
+        )
+        |> redirect(to: ~p"/passwords/new")
+
       not is_nil(user) and is_nil(user.confirmed_at) ->
         render(
           conn,

lib/philomena_web/plugs/compromised_password_check_plug.ex

@@ -5,11 +5,7 @@ defmodule PhilomenaWeb.CompromisedPasswordCheckPlug do
   def init(opts), do: opts
 
   def call(conn, _opts) do
-    if pwned_passwords_enabled?() do
-      error_if_password_compromised(conn, conn.params)
-    else
-      conn
-    end
+    error_if_password_compromised(conn, conn.params)
   end
 
   defp error_if_password_compromised(conn, %{"user" => %{"password" => password}}) do
@@ -29,14 +25,33 @@ defmodule PhilomenaWeb.CompromisedPasswordCheckPlug do
   defp error_if_password_compromised(conn, _params),
     do: conn
 
-  defp password_compromised?(password) do
+  @doc """
+  Returns whether a password appears in the Pwned Passwords database.
+
+  The range query only sends the first five characters of the password's SHA-1
+  hash. If the check is disabled or unavailable, passwords are allowed through.
+  """
+  def password_compromised?(password) when is_binary(password) do
+    if pwned_passwords_enabled?() do
+      password_compromised_in_breach?(password)
+    else
+      false
+    end
+  end
+
+  def password_compromised?(_password), do: false
+
+  defp password_compromised_in_breach?(password) do
     <<prefix::binary-size(5), rest::binary>> =
       :crypto.hash(:sha, password)
       |> Base.encode16()
 
     case PhilomenaProxy.Http.get(make_api_url(prefix)) do
-      {:ok, %{body: body, status: 200}} -> String.contains?(body, rest)
-      _ -> false
+      {:ok, %{body: body, status: 200}} ->
+        Enum.any?(String.split(body, "\n"), &String.starts_with?(&1, rest <> ":"))
+
+      _ ->
+        false
     end
   end
 

lib/philomena_web/templates/password/new.html.slime

@@ -1,4 +1,4 @@
-h1 Forgot your password?
+h1 Reset password
 p
   ' Provide the email address you signed up with and we will email you
   ' password reset instructions.

lib/philomena_web/templates/session/new.html.slime

@@ -21,6 +21,8 @@ h1 Sign in
     => checkbox f, :remember_me
     = label f, :remember_me, "Remember me"
 
+  = render PhilomenaWeb.CaptchaView, "_captcha.html", name: "session", conn: @conn
+
   .actions
     = submit "Sign in", class: "button"
 

mix.exs

@@ -64,7 +64,9 @@ defmodule Philomena.MixProject do
       {:redix, "~> 1.4"},
       {:remote_ip, "~> 1.2"},
       {:briefly, "~> 0.5"},
-      {:req, "~> 0.5"},
+      # ExAws signs empty-body GET requests that Req 0.7 rewrites as POST.
+      # https://github.com/ex-aws/ex_aws/issues/1246
+      {:req, "0.6.3"},
       {:exq, "~> 0.21"},
       {:ex_aws, "~> 2.5"},
       {:ex_aws_s3, "~> 2.5"},

test/philomena/users_test.exs

@@ -378,6 +378,20 @@ defmodule Philomena.UsersTest do
     end
   end
 
+  describe "delete_user_sessions/1" do
+    test "deletes all of the user's sessions" do
+      user = user_fixture()
+      first_token = Users.generate_user_session_token(user)
+      second_token = Users.generate_user_session_token(user)
+      totp_token = Users.generate_user_totp_token(user)
+
+      assert Users.delete_user_sessions(user) == :ok
+      refute Users.get_user_by_session_token(first_token)
+      refute Users.get_user_by_session_token(second_token)
+      refute Users.user_totp_token_valid?(user, totp_token)
+    end
+  end
+
   describe "generate_user_totp_token/1" do
     setup do
       %{user: user_fixture()}

test/philomena_web/controllers/session_controller_test.exs

@@ -1,5 +1,7 @@
 defmodule PhilomenaWeb.SessionControllerTest do
-  use PhilomenaWeb.ConnCase, async: true
+  use PhilomenaWeb.ConnCase, async: false
+
+  alias Philomena.Users
 
   import Philomena.UsersFixtures
 
@@ -20,6 +22,10 @@ defmodule PhilomenaWeb.SessionControllerTest do
   end
 
   describe "POST /sessions" do
+    test "adds a captcha to the sign-in form", %{conn: conn} do
+      assert html_response(get(conn, ~p"/sessions/new"), 200) =~ "I am not a robot!"
+    end
+
     test "logs the user in", %{conn: conn, user: user} do
       conn =
         post(conn, ~p"/sessions", %{
@@ -60,6 +66,35 @@ defmodule PhilomenaWeb.SessionControllerTest do
       response = html_response(conn, 200)
       assert response =~ "Invalid email or password"
     end
+
+    test "invalidates all sessions and requires a reset for a compromised password", %{
+      conn: conn,
+      user: user
+    } do
+      Application.put_env(:philomena, :pwned_passwords, true)
+
+      on_exit(fn -> Application.put_env(:philomena, :pwned_passwords, false) end)
+
+      password = valid_user_password()
+
+      <<prefix::binary-size(5), suffix::binary>> = :crypto.hash(:sha, password) |> Base.encode16()
+
+      Req.Test.stub(PhilomenaProxy.Http, fn request ->
+        assert request.request_path == "/range/#{prefix}"
+        Req.Test.text(request, "#{suffix}:1\r\n")
+      end)
+
+      existing_session = Users.generate_user_session_token(user)
+
+      conn =
+        post(conn, ~p"/sessions", %{
+          "user" => %{"email" => user.email, "password" => password}
+        })
+
+      assert redirected_to(conn) == ~p"/passwords/new"
+      assert Phoenix.Flash.get(conn.assigns.flash, :error) =~ "Please reset your password"
+      refute Users.get_user_by_session_token(existing_session)
+    end
   end
 
   describe "DELETE /sessions" do