From ca3dac3878c44cf114fe7b52e4b35393c2c41900 Mon Sep 17 00:00:00 2001
From: James Harton <59449+jimsynz@users.noreply.github.com>
Date: Sun, 12 Feb 2023 21:14:16 +1300
Subject: [PATCH] fix: don't allow special purpose tokens to be used for sign
 in. (#191)

This fixes a security issue where someone in possession of a special purpose token (reset, confirmation, magic link, etc) would be able to access an API using this token.  We strongly encourage you to upgrade.

Closes #190.
---
 lib/ash_authentication/plug/helpers.ex        |  6 ++--
 test/ash_authentication/plug/helpers_test.exs | 30 ++++++++++++++++++-
 .../example/user_with_token_required.ex       |  9 ++++++
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/lib/ash_authentication/plug/helpers.ex b/lib/ash_authentication/plug/helpers.ex
index 90274c2..7dec2e2 100644
--- a/lib/ash_authentication/plug/helpers.ex
+++ b/lib/ash_authentication/plug/helpers.ex
@@ -75,7 +75,8 @@ defmodule AshAuthentication.Plug.Helpers do
 
         with token when is_binary(token) <-
                Conn.get_session(conn, "#{options.subject_name}_token"),
-             {:ok, %{"sub" => subject, "jti" => jti}, _} <- Jwt.verify(token, otp_app),
+             {:ok, %{"sub" => subject, "jti" => jti} = claims, _}
+             when not is_map_key(claims, "act") <- Jwt.verify(token, otp_app),
              {:ok, [_]} <-
                TokenResource.Actions.get_token(token_resource, %{
                  "jti" => jti,
@@ -116,7 +117,8 @@ defmodule AshAuthentication.Plug.Helpers do
     |> Stream.filter(&String.starts_with?(&1, "Bearer "))
     |> Stream.map(&String.replace_leading(&1, "Bearer ", ""))
     |> Enum.reduce(conn, fn token, conn ->
-      with {:ok, %{"sub" => subject, "jti" => jti}, resource} <- Jwt.verify(token, otp_app),
+      with {:ok, %{"sub" => subject, "jti" => jti} = claims, resource}
+           when not is_map_key(claims, "act") <- Jwt.verify(token, otp_app),
            :ok <- validate_token(resource, jti),
            {:ok, user} <- AshAuthentication.subject_to_user(subject, resource),
            {:ok, subject_name} <- Info.authentication_subject_name(resource),
diff --git a/test/ash_authentication/plug/helpers_test.exs b/test/ash_authentication/plug/helpers_test.exs
index a5c8a39..894ea12 100644
--- a/test/ash_authentication/plug/helpers_test.exs
+++ b/test/ash_authentication/plug/helpers_test.exs
@@ -1,7 +1,7 @@
 defmodule AshAuthentication.Plug.HelpersTest do
   @moduledoc false
   use DataCase, async: true
-  alias AshAuthentication.{Jwt, Plug.Helpers, TokenResource}
+  alias AshAuthentication.{Info, Jwt, Plug.Helpers, Strategy.Password, TokenResource}
   import Plug.Test, only: [conn: 3]
   alias Plug.Conn
 
@@ -106,6 +106,20 @@ defmodule AshAuthentication.Plug.HelpersTest do
 
       refute conn.assigns.current_user_with_token_required
     end
+
+    test "when the token is for another purpose it can't be used for sign in", %{conn: conn} do
+      user = build_user_with_token_required()
+
+      strategy = Info.strategy!(user.__struct__, :password)
+      {:ok, reset_token} = Password.reset_token_for(strategy, user)
+
+      conn =
+        conn
+        |> Conn.put_session("user_with_token_required_token", reset_token)
+        |> Helpers.retrieve_from_session(:ash_authentication)
+
+      refute conn.assigns.current_user_with_token_required
+    end
   end
 
   describe "retrieve_from_bearer/2" do
@@ -163,6 +177,20 @@ defmodule AshAuthentication.Plug.HelpersTest do
 
       refute is_map_key(conn.assigns, :current_user_with_token_required)
     end
+
+    test "when the token is for another purpose, it doesn't let them sign in", %{conn: conn} do
+      user = build_user()
+
+      strategy = Info.strategy!(user.__struct__, :password)
+      {:ok, reset_token} = Password.reset_token_for(strategy, user)
+
+      conn =
+        conn
+        |> Conn.put_req_header("authorization", "Bearer #{reset_token}")
+        |> Helpers.retrieve_from_bearer(:ash_authentication)
+
+      refute is_map_key(conn.assigns, :current_user_with_token_required)
+    end
   end
 
   describe "revoke_bearer_tokens/2" do
diff --git a/test/support/example/user_with_token_required.ex b/test/support/example/user_with_token_required.ex
index 752c6d4..e4cd6ad 100644
--- a/test/support/example/user_with_token_required.ex
+++ b/test/support/example/user_with_token_required.ex
@@ -1,6 +1,7 @@
 defmodule Example.UserWithTokenRequired do
   @moduledoc false
   use Ash.Resource, data_layer: AshPostgres.DataLayer, extensions: [AshAuthentication]
+  require Logger
 
   @type t :: %__MODULE__{
           id: Ecto.UUID.t(),
@@ -32,6 +33,14 @@ defmodule Example.UserWithTokenRequired do
     strategies do
       password do
         identity_field :email
+
+        resettable do
+          sender fn user, token, _opts ->
+            Logger.debug(
+              "Password reset request for user #{user.username}, token #{inspect(token)}"
+            )
+          end
+        end
       end
     end
   end