improvement: add require_confirmed_with option to password strategy (…

…#861)

.formatter.exs

@@ -1,6 +1,7 @@
 spark_locals_without_parens = [
   access_token_attribute_name: 1,
   access_token_expires_at_attribute_name: 1,
+  require_confirmed_with: 1,
   apple: 0,
   apple: 1,
   apple: 2,

documentation/dsls/DSL-AshAuthentication.Strategy.Password.md

@@ -140,6 +140,7 @@ end
 | [`sign_in_enabled?`](#authentication-strategies-password-sign_in_enabled?){: #authentication-strategies-password-sign_in_enabled? } | `boolean` | `true` | If you do not want new users to be able to sign in using this strategy, set this to false. |
 | [`sign_in_tokens_enabled?`](#authentication-strategies-password-sign_in_tokens_enabled?){: #authentication-strategies-password-sign_in_tokens_enabled? } | `boolean` | `true` | Whether or not to support generating short lived sign in tokens. Requires the resource to have tokens enabled. |
 | [`sign_in_token_lifetime`](#authentication-strategies-password-sign_in_token_lifetime){: #authentication-strategies-password-sign_in_token_lifetime } | `pos_integer \| {pos_integer, :days \| :hours \| :minutes \| :seconds}` | `{60, :seconds}` | A lifetime for which a generated sign in token will be valid, if `sign_in_tokens_enabled?`. Unit defaults to `:seconds`. |
+| [`require_confirmed_with`](#authentication-strategies-password-require_confirmed_with){: #authentication-strategies-password-require_confirmed_with } | `atom \| nil` |  | Whether a new account must be confirmed in order to log in. |
 
 
 ## authentication.strategies.password.resettable

documentation/tutorials/confirmation.md

@@ -183,6 +183,25 @@ end
 
 Provided you have your authentication routes hooked up either via `AshAuthentication.Plug` or [`AshAuthentication.Phoenix.Router`](https://hexdocs.pm/ash_authentication_phoenix/AshAuthentication.Phoenix.Router.html) then the user will be confirmed when the token is submitted.
 
+## Blocking unconfirmed users from logging in
+
+The above section explains how to confirm an user account. There's a new directive in the [dsl](file://wsl$/UbuntuLatest/home/bruno/dev/ash/ash_authentication/doc/dsl-ashauthentication-strategy-password.html#authentication-strategies-password-require_confirmed_with) which can require the user to be confirmed in order to log in.
+
+So:
+
+```
+strategies do
+  strategy :password do
+    ...
+    require_confirmed_with :confirmed_at
+  end
+end
+```
+
+this will make impossible for unconfirmed users to log in. Note that at the moment it is developer responsibility to handle the scenario, for example redirecting the user to a page that gives some context and maybe offers the chance to require a new confirmation email in case the previous one is lost.
+
+If the field value is `nil` or if the field itself is not present, no confirmation check will be enforced.
+
 ## Confirming changes to monitored fields
 
 You may want to require a user to perform a confirmation when a certain field changes. For example if a user changes their email address we can send them a new confirmation request.

lib/ash_authentication/errors/unconfirmed_user.ex

@@ -0,0 +1,28 @@
+defmodule AshAuthentication.Errors.UnconfirmedUser do
+  @moduledoc """
+    The user is unconfirmed and so the operation cannot be executed.
+  """
+  use Splode.Error,
+    fields: [
+      caused_by: %{},
+      changeset: nil,
+      field: nil,
+      query: nil,
+      strategy: nil
+    ],
+    class: :forbidden
+
+  alias AshAuthentication.Debug
+
+  @type t :: Exception.t()
+
+  @impl true
+  def exception(args) do
+    args
+    |> super()
+    |> Debug.describe()
+  end
+
+  @impl true
+  def message(_), do: "Unconfirmed user"
+end

lib/ash_authentication/strategies/password.ex

@@ -94,7 +94,8 @@ defmodule AshAuthentication.Strategy.Password do
   See the [Testing guide](/documentation/topics/testing.md) for tips on testing resources using this strategy.
   """
 
-  defstruct confirmation_required?: false,
+  defstruct require_confirmed_with: nil,
+            confirmation_required?: false,
             hash_provider: AshAuthentication.BcryptProvider,
             hashed_password_field: :hashed_password_field,
             identity_field: :username,
@@ -128,6 +129,7 @@ defmodule AshAuthentication.Strategy.Password do
   use Custom, entity: Dsl.dsl()
 
   @type t :: %Password{
+          require_confirmed_with: :atom | nil,
           confirmation_required?: boolean,
           hash_provider: module,
           hashed_password_field: atom,

lib/ash_authentication/strategies/password/actions.ex

@@ -33,12 +33,31 @@ defmodule AshAuthentication.Strategy.Password.Actions do
 
     strategy.resource
     |> Query.new()
+    |> Query.ensure_selected(List.wrap(strategy.require_confirmed_with))
     |> Query.set_context(context)
     |> Query.for_read(strategy.sign_in_action_name, params, options)
     |> Ash.read()
     |> case do
       {:ok, [user]} ->
-        {:ok, user}
+        case strategy.require_confirmed_with do
+          nil ->
+            {:ok, user}
+
+          field ->
+            if user_confirmed?(user, field) do
+              {:ok, user}
+            else
+              {:error,
+               Errors.UnconfirmedUser.exception(
+                 strategy: strategy,
+                 caused_by: %{
+                   action: :sign_in,
+                   message: "User must be confirmed to sign in.",
+                   module: __MODULE__
+                 }
+               )}
+            end
+        end
 
       {:ok, []} ->
         {:error,
@@ -279,4 +298,13 @@ defmodule AshAuthentication.Strategy.Password.Actions do
 
   def reset(strategy, _params, _options) when is_struct(strategy, Password),
     do: {:error, NoSuchAction.exception(resource: strategy.resource, action: :reset, type: :read)}
+
+  defp user_confirmed?(user, field) do
+    case Map.get(user, field) do
+      %Ash.NotLoaded{} -> false
+      %Ash.ForbiddenField{} -> false
+      nil -> false
+      _ -> true
+    end
+  end
 end

lib/ash_authentication/strategies/password/dsl.ex

@@ -125,6 +125,12 @@ defmodule AshAuthentication.Strategy.Password.Dsl do
           default: {60, :seconds},
           doc:
             "A lifetime for which a generated sign in token will be valid, if `sign_in_tokens_enabled?`. Unit defaults to `:seconds`."
+        ],
+        require_confirmed_with: [
+          type: {:or, [:atom, nil]},
+          required: false,
+          doc: "Whether a new account must be confirmed in order to log in.",
+          default: nil
         ]
       ],
       entities: [

lib/ash_authentication/strategies/password/sign_in_preparation.ex

@@ -13,7 +13,7 @@ defmodule AshAuthentication.Strategy.Password.SignInPreparation do
   an authentication failed error.
   """
   use Ash.Resource.Preparation
-  alias AshAuthentication.{Errors.AuthenticationFailed, Info, Jwt}
+  alias AshAuthentication.{Errors.AuthenticationFailed, Errors.UnconfirmedUser, Info, Jwt}
   alias Ash.{Error.Unknown, Query, Resource.Preparation}
   require Ash.Query
 
@@ -46,9 +46,23 @@ defmodule AshAuthentication.Strategy.Password.SignInPreparation do
              password,
              Map.get(record, strategy.hashed_password_field)
            ) do
-          token_type = query.context[:token_type] || :user
-
-          {:ok, [maybe_generate_token(token_type, record, strategy)]}
+          if user_confirmed_if_needed(record, strategy) do
+            token_type = query.context[:token_type] || :user
+
+            {:ok, [maybe_generate_token(token_type, record, strategy)]}
+          else
+            {:error,
+             UnconfirmedUser.exception(
+               strategy: strategy,
+               query: query,
+               caused_by: %{
+                 module: __MODULE__,
+                 action: query.action,
+                 resource: query.resource,
+                 message: "User must be confirmed to sign in."
+               }
+             )}
+          end
         else
           {:error,
            AuthenticationFailed.exception(
@@ -135,4 +149,9 @@ defmodule AshAuthentication.Strategy.Password.SignInPreparation do
 
     Ash.Resource.put_metadata(record, :token, token)
   end
+
+  def user_confirmed_if_needed(_user, %{require_confirmed_with: nil} = _strategy), do: true
+
+  def user_confirmed_if_needed(user, %{require_confirmed_with: field} = _strategy),
+    do: Map.get(user, field) != nil
 end

test/ash_authentication/strategies/password/plug_test.exs

@@ -5,6 +5,7 @@ defmodule AshAuthentication.Strategy.Password.PlugTest do
   import Plug.Test
 
   alias AshAuthentication.{
+    Errors.UnconfirmedUser,
     Info,
     Plug.Helpers,
     Strategy.Password,
@@ -115,6 +116,90 @@ defmodule AshAuthentication.Strategy.Password.PlugTest do
 
       assert Exception.message(error) =~ ~r/authentication failed/i
     end
+
+    test "it returns an error when account confirmation is required and instead it is not" do
+      {:ok, strategy} = Info.strategy(Example.User, :password)
+      strategy = %{strategy | require_confirmed_with: :confirmed_at}
+
+      password = password()
+      user = build_user(password: password, password_confirmation: password)
+
+      params = %{
+        "user" => %{
+          "username" => user.username,
+          "password" => password
+        }
+      }
+
+      assert {_conn,
+              {
+                :error,
+                %UnconfirmedUser{
+                  caused_by: %{
+                    message: "User must be confirmed to sign in."
+                  }
+                } = error
+              }} =
+               :post
+               |> conn("/", params)
+               |> Plug.sign_in(strategy)
+               |> Helpers.get_authentication_result()
+
+      assert Exception.message(error) =~ ~r/unconfirmed user/i
+    end
+
+    test "it does NOT return an error if the user is unconfirmed but the confirmation is not required" do
+      {:ok, strategy} = Info.strategy(Example.User, :password)
+      strategy = %{strategy | require_confirmed_with: nil}
+      password = password()
+      user = build_user(password: password, password_confirmation: password)
+
+      params = %{
+        "user" => %{
+          "username" => user.username,
+          "password" => password
+        }
+      }
+
+      assert {_conn, {:ok, signed_in_user}} =
+               :post
+               |> conn("/", params)
+               |> Plug.sign_in(strategy)
+               |> Helpers.get_authentication_result()
+
+      assert signed_in_user.id == user.id
+    end
+
+    test "it does NOT return an error if the user is confirmed, and the confirmation is required" do
+      {:ok, strategy} = Info.strategy(Example.User, :password)
+      strategy = %{strategy | require_confirmed_with: :confirmed_at}
+      password = password()
+
+      # Need to build a confirmed user
+      user =
+        build_user(
+          password: password,
+          password_confirmation: password
+        )
+
+      user =
+        Ash.Seed.update!(user, %{confirmed_at: DateTime.utc_now()})
+
+      params = %{
+        "user" => %{
+          "username" => user.username,
+          "password" => password
+        }
+      }
+
+      assert {_conn, {:ok, signed_in_user}} =
+               :post
+               |> conn("/", params)
+               |> Plug.sign_in(strategy)
+               |> Helpers.get_authentication_result()
+
+      assert signed_in_user.id == user.id
+    end
   end
 
   describe "reset_request/2" do

test/support/example/user.ex

@@ -195,6 +195,7 @@ defmodule Example.User do
       password do
         register_action_accept [:extra_stuff]
         sign_in_tokens_enabled? true
+        require_confirmed_with nil
 
         resettable do
           sender fn user, token, _opts ->