From 4e66a402fe9081ede73401b68730d08f2f3aa2ea Mon Sep 17 00:00:00 2001
From: James Harton <james@harton.nz>
Date: Fri, 10 May 2024 14:32:56 +1200
Subject: [PATCH] improvement: Only require tokens to be enabled when using a
 strategy which needs them.

---
 documentation/dsls/DSL:-AshAuthentication.md  |  2 +-
 documentation/topics/custom-strategy.md       | 12 +++-
 documentation/tutorials/confirmation.md       |  2 +-
 .../add_ons/confirmation/strategy.ex          |  4 ++
 lib/ash_authentication/dsl.ex                 |  2 +-
 .../strategies/magic_link/strategy.ex         |  4 ++
 .../strategies/oauth2/strategy.ex             |  4 ++
 .../strategies/password/strategy.ex           |  6 ++
 lib/ash_authentication/strategy.ex            |  6 ++
 lib/ash_authentication/verifier.ex            | 67 ++++++++++++++++++-
 .../example/only_marties_at_the_party.ex      |  2 +
 11 files changed, 104 insertions(+), 7 deletions(-)

diff --git a/documentation/dsls/DSL:-AshAuthentication.md b/documentation/dsls/DSL:-AshAuthentication.md
index e221d06..e16ecb7 100644
--- a/documentation/dsls/DSL:-AshAuthentication.md
+++ b/documentation/dsls/DSL:-AshAuthentication.md
@@ -124,7 +124,7 @@ Configure JWT settings for this resource
 | Name | Type | Default | Docs |
 |------|------|---------|------|
 | [`token_resource`](#authentication-tokens-token_resource){: #authentication-tokens-token_resource .spark-required} | `module \| false` |  | The resource used to store token information, such as in-flight confirmations, revocations, and if `store_all_tokens?` is enabled, authentication tokens themselves. |
-| [`enabled?`](#authentication-tokens-enabled?){: #authentication-tokens-enabled? } | `boolean` | `true` | Should JWTs be generated by this resource? |
+| [`enabled?`](#authentication-tokens-enabled?){: #authentication-tokens-enabled? } | `boolean` | `false` | Should JWTs be generated by this resource? |
 | [`store_all_tokens?`](#authentication-tokens-store_all_tokens?){: #authentication-tokens-store_all_tokens? } | `boolean` | `false` | Store all tokens in the `token_resource`. See the [tokens guide](/documentation/topics/tokens.md) for more. |
 | [`require_token_presence_for_authentication?`](#authentication-tokens-require_token_presence_for_authentication?){: #authentication-tokens-require_token_presence_for_authentication? } | `boolean` | `false` | Require a locally-stored token for authentication. See the [tokens guide](/documentation/topics/tokens.md) for more. |
 | [`signing_algorithm`](#authentication-tokens-signing_algorithm){: #authentication-tokens-signing_algorithm } | `String.t` | `"HS256"` | The algorithm to use for token signing. Available signing algorithms are; EdDSA, Ed448ph, Ed448, Ed25519ph, Ed25519, PS512, PS384, PS256, ES512, ES384, ES256, RS512, RS384, RS256, HS512, HS384 and HS256. |
diff --git a/documentation/topics/custom-strategy.md b/documentation/topics/custom-strategy.md
index c63bd61..422a49d 100644
--- a/documentation/topics/custom-strategy.md
+++ b/documentation/topics/custom-strategy.md
@@ -150,7 +150,7 @@ concepts:
 
 - "phases" - in terms of HTTP, each strategy is likely to have many phases (eg OAuth 2.0's "request" and "callback" phases). Essentially you need one phase for each HTTP endpoint you wish to support with your strategy. In our case we just want one sign in endpoint.
 - "actions" - actions are exactly as they sound - Resource actions which can be executed by the strategy, whether generated by the strategy (as in the password strategy) or typed in by the user (as in the OAuth 2.0 strategy). The reason that we wrap the strategy's actions this way is that all the built-in strategies (and we hope yours too) allow the user to customise the name of the actions that it uses. At the very least it should probably append the strategy name to the action. Using `Strategy.action/4` allows us to refer these by a more generic name rather than via the user-specified one (eg `:register` vs `:register_with_password`).
-- "routes" - `AshAuthentication.Plug` (or [`AshAuthentication.Phoenix.Router.html`](`e:ash_authentication_phoenix:AshAuthentication.Phoenix.Router.html`)) will generate routes using `Plug.Router` (or [`Phoenix.Router`](`e:phoenix:Phoenix.Router.html`)) - the `routes/1` callback is used to retrieve this information from the strategy.
+- "routes" - `AshAuthentication.Plug` (or [`AshAuthentication.Phoenix.Router.html`](https://hexdocs.pm/ash_authentication_phoenix/AshAuthentication.Phoenix.Router.html)) will generate routes using `Plug.Router` (or [`Phoenix.Router`](https://hexdocs.pm/phoenix/Phoenix.Router.html)) - the `routes/1` callback is used to retrieve this information from the strategy.
 
 Given this information, let's implement the strategy. It's quite long, so I'm
 going to break it up into smaller chunks.
@@ -210,7 +210,7 @@ straight into `store_authentication_result/2` from
   end
 ```
 
-Finally, we implement our sign in action. We use `Ash.Query` to find all
+Next, we implement our sign in action. We use `Ash.Query` to find all
 records whose name field matches the input, then constrain it to only records
 whose name field starts with "Marty". Depending on whether the name field has a
 unique identity on it we have to deal with it returning zero or more users, or
@@ -267,6 +267,14 @@ the resource.
 end
 ```
 
+Lastly, we have to implement the `tokens_required?/1` function. This function
+indicates Ash Authentication whether your strategy creates or consumes any
+tokens. Since our strategy does not, we can simply return false:
+
+```elixir
+def token_required?(_), do: false
+```
+
 ## Bonus round - transformers and verifiers
 
 In some cases it may be required for your strategy to modify it's own
diff --git a/documentation/tutorials/confirmation.md b/documentation/tutorials/confirmation.md
index c37fe28..78054d1 100644
--- a/documentation/tutorials/confirmation.md
+++ b/documentation/tutorials/confirmation.md
@@ -84,7 +84,7 @@ defmodule MyApp.NewUserConfirmationSender do
 end
 ```
 
-Provided you have your authentication routes hooked up either via `AshAuthentication.Plug` or [`AshAuthentication.Phoenix.Router`](`e:ash_authentication_phoenix:AshAuthentication.Phoenix.Router.html`) then the user will be confirmed when the token is submitted.
+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.
 
 ## Confirming changes to monitored fields
 
diff --git a/lib/ash_authentication/add_ons/confirmation/strategy.ex b/lib/ash_authentication/add_ons/confirmation/strategy.ex
index 10c9f90..c0f67f4 100644
--- a/lib/ash_authentication/add_ons/confirmation/strategy.ex
+++ b/lib/ash_authentication/add_ons/confirmation/strategy.ex
@@ -51,4 +51,8 @@ defimpl AshAuthentication.Strategy, for: AshAuthentication.AddOn.Confirmation do
   @spec action(Confirmation.t(), action, map, keyword) :: {:ok, Resource.record()} | {:error, any}
   def action(strategy, :confirm, params, options),
     do: Confirmation.Actions.confirm(strategy, params, options)
+
+  @doc false
+  @spec tokens_required?(Confirmation.t()) :: true
+  def tokens_required?(_), do: true
 end
diff --git a/lib/ash_authentication/dsl.ex b/lib/ash_authentication/dsl.ex
index 0d0506a..8a63614 100644
--- a/lib/ash_authentication/dsl.ex
+++ b/lib/ash_authentication/dsl.ex
@@ -77,7 +77,7 @@ defmodule AshAuthentication.Dsl do
                 doc: """
                 Should JWTs be generated by this resource?
                 """,
-                default: true
+                default: false
               ],
               store_all_tokens?: [
                 type: :boolean,
diff --git a/lib/ash_authentication/strategies/magic_link/strategy.ex b/lib/ash_authentication/strategies/magic_link/strategy.ex
index 0ed313d..803d76c 100644
--- a/lib/ash_authentication/strategies/magic_link/strategy.ex
+++ b/lib/ash_authentication/strategies/magic_link/strategy.ex
@@ -45,4 +45,8 @@ defimpl AshAuthentication.Strategy, for: AshAuthentication.Strategy.MagicLink do
 
   def action(strategy, :sign_in, params, options),
     do: MagicLink.Actions.sign_in(strategy, params, options)
+
+  @doc false
+  @spec tokens_required?(MagicLink.t()) :: true
+  def tokens_required?(_), do: true
 end
diff --git a/lib/ash_authentication/strategies/oauth2/strategy.ex b/lib/ash_authentication/strategies/oauth2/strategy.ex
index ebf0762..0c1db53 100644
--- a/lib/ash_authentication/strategies/oauth2/strategy.ex
+++ b/lib/ash_authentication/strategies/oauth2/strategy.ex
@@ -66,4 +66,8 @@ defimpl AshAuthentication.Strategy, for: AshAuthentication.Strategy.OAuth2 do
 
   def action(strategy, :sign_in, params, options),
     do: OAuth2.Actions.sign_in(strategy, params, options)
+
+  @doc false
+  @spec tokens_required?(OAuth2.t()) :: boolean
+  def tokens_required?(_), do: false
 end
diff --git a/lib/ash_authentication/strategies/password/strategy.ex b/lib/ash_authentication/strategies/password/strategy.ex
index fcae87a..7dc3f93 100644
--- a/lib/ash_authentication/strategies/password/strategy.ex
+++ b/lib/ash_authentication/strategies/password/strategy.ex
@@ -94,4 +94,10 @@ defimpl AshAuthentication.Strategy, for: AshAuthentication.Strategy.Password do
 
   def action(strategy, :sign_in_with_token, params, options),
     do: Password.Actions.sign_in_with_token(strategy, params, options)
+
+  @doc false
+  @spec tokens_required?(Password.t()) :: boolean
+  def tokens_required?(strategy) when strategy.sign_in_tokens_enabled?, do: true
+  def tokens_required?(strategy) when is_map(strategy.resettable), do: true
+  def tokens_required?(_), do: false
 end
diff --git a/lib/ash_authentication/strategy.ex b/lib/ash_authentication/strategy.ex
index 23e0029..d2ce027 100644
--- a/lib/ash_authentication/strategy.ex
+++ b/lib/ash_authentication/strategy.ex
@@ -125,4 +125,10 @@ defprotocol AshAuthentication.Strategy do
   @spec action(t, action, params :: map, options :: keyword) ::
           :ok | {:ok, Resource.record()} | {:error, any}
   def action(strategy, action_name, params, options \\ [])
+
+  @doc """
+  Indicates that the strategy creates or consumes tokens.
+  """
+  @spec tokens_required?(t) :: boolean
+  def tokens_required?(strategy)
 end
diff --git a/lib/ash_authentication/verifier.ex b/lib/ash_authentication/verifier.ex
index a7bb469..983b7b0 100644
--- a/lib/ash_authentication/verifier.ex
+++ b/lib/ash_authentication/verifier.ex
@@ -6,7 +6,7 @@ defmodule AshAuthentication.Verifier do
   """
 
   use Spark.Dsl.Verifier
-  alias AshAuthentication.Info
+  alias AshAuthentication.{Info, Strategy, Strategy.Password}
   alias Spark.{Dsl.Transformer, Error.DslError}
   import AshAuthentication.Utils
 
@@ -17,7 +17,8 @@ defmodule AshAuthentication.Verifier do
           | {:error, term}
           | {:warn, String.t() | list(String.t())}
   def verify(dsl_state) do
-    with {:ok, _domain} <- validate_domain_presence(dsl_state) do
+    with {:ok, _domain} <- validate_domain_presence(dsl_state),
+         :ok <- validate_tokens_may_be_required(dsl_state) do
       validate_token_resource(dsl_state)
     end
   end
@@ -46,6 +47,68 @@ defmodule AshAuthentication.Verifier do
     end
   end
 
+  defp validate_tokens_may_be_required(dsl_state) do
+    strategies_requiring_tokens =
+      dsl_state
+      |> Info.authentication_strategies()
+      |> Enum.filter(&Strategy.tokens_required?/1)
+
+    tokens_enabled? =
+      dsl_state
+      |> Info.authentication_tokens_enabled?()
+
+    case {strategies_requiring_tokens, tokens_enabled?} do
+      {[], _} ->
+        :ok
+
+      {_, true} ->
+        :ok
+
+      {[password | _], false}
+      when is_struct(password, Password) and is_map(password.resettable) ->
+        {:error,
+         DslError.exception(
+           path: [:authentication, :tokens, :enabled?],
+           message: """
+           The `#{password.name}` password authentication strategy requires tokens be enabled because reset tokens are in use.
+
+           To fix this error you can either:
+
+             1. disable password resets by removing the `resettable` configuration from your password strategy, or
+             2. enable tokens.
+           """
+         )}
+
+      {[password | _], false}
+      when is_struct(password, Password) and is_map(password.sign_in_tokens_enabled?) ->
+        {:error,
+         DslError.exception(
+           path: [:authentication, :tokens, :enabled?],
+           message: """
+           The `#{password.name}` password authentication strategy requires tokens be enabled because sign-in tokens are in use.
+
+           To fix this error you can either:
+
+             1. disable sign in tokens by setting `sign_in_tokens? false` your password strategy, or
+             2. enable tokens.
+           """
+         )}
+
+      {[strategy | _], false} ->
+        {:error,
+         DslError.exception(
+           path: [:authentication, :tokens, :enabled?],
+           message: """
+           The `#{inspect(strategy.name)}` authentication strategy requires tokens be enabled.
+
+           To fix this error you can either:
+             1. disable the `#{inspect(strategy.name)}` strategy, or
+             2. enable tokens.
+           """
+         )}
+    end
+  end
+
   defp validate_token_resource(dsl_state) do
     if_tokens_enabled(dsl_state, fn dsl_state ->
       with {:ok, resource} when is_truthy(resource) <-
diff --git a/test/support/example/only_marties_at_the_party.ex b/test/support/example/only_marties_at_the_party.ex
index 9b5064c..aba5d6f 100644
--- a/test/support/example/only_marties_at_the_party.ex
+++ b/test/support/example/only_marties_at_the_party.ex
@@ -118,5 +118,7 @@ defmodule Example.OnlyMartiesAtTheParty do
            )}
       end
     end
+
+    def tokens_required?(_), do: false
   end
 end