diff --git a/.formatter.exs b/.formatter.exs index a3efe543..c4373853 100644 --- a/.formatter.exs +++ b/.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, diff --git a/documentation/dsls/DSL-AshAuthentication.Strategy.Password.md b/documentation/dsls/DSL-AshAuthentication.Strategy.Password.md index 81493bd7..984eb50f 100644 --- a/documentation/dsls/DSL-AshAuthentication.Strategy.Password.md +++ b/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 diff --git a/documentation/tutorials/confirmation.md b/documentation/tutorials/confirmation.md index dd4f8faf..813df891 100644 --- a/documentation/tutorials/confirmation.md +++ b/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. diff --git a/lib/ash_authentication/errors/unconfirmed_user.ex b/lib/ash_authentication/errors/unconfirmed_user.ex new file mode 100644 index 00000000..bad53490 --- /dev/null +++ b/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 diff --git a/lib/ash_authentication/strategies/password.ex b/lib/ash_authentication/strategies/password.ex index ea9cb1eb..74014f55 100644 --- a/lib/ash_authentication/strategies/password.ex +++ b/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, diff --git a/lib/ash_authentication/strategies/password/actions.ex b/lib/ash_authentication/strategies/password/actions.ex index 82da6c82..4d4d6b35 100644 --- a/lib/ash_authentication/strategies/password/actions.ex +++ b/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 diff --git a/lib/ash_authentication/strategies/password/dsl.ex b/lib/ash_authentication/strategies/password/dsl.ex index af336c6d..3ca9d63d 100644 --- a/lib/ash_authentication/strategies/password/dsl.ex +++ b/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: [ diff --git a/lib/ash_authentication/strategies/password/sign_in_preparation.ex b/lib/ash_authentication/strategies/password/sign_in_preparation.ex index 19030679..44234b03 100644 --- a/lib/ash_authentication/strategies/password/sign_in_preparation.ex +++ b/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 diff --git a/test/ash_authentication/strategies/password/plug_test.exs b/test/ash_authentication/strategies/password/plug_test.exs index 13a1ee00..99c0fe40 100644 --- a/test/ash_authentication/strategies/password/plug_test.exs +++ b/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 diff --git a/test/support/example/user.ex b/test/support/example/user.ex index bf46b346..d678c0c8 100644 --- a/test/support/example/user.ex +++ b/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 ->