Skip to content

Commit

Permalink
fix: ensure token expunger sets context that allows it to run (#921)
Browse files Browse the repository at this point in the history
improvement: authorize with error to make expunge errors clearer
test: add policies to tokens in test to detect issues later
  • Loading branch information
zachdaniel authored Feb 23, 2025
1 parent 2970ab7 commit 8d83fee
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ defmodule AshAuthentication.AddOn.Confirmation.ConfirmationHookChange do
|> case do
{:ok, token} ->
{sender, send_opts} = strategy.sender

send_opts
|> Keyword.put(:tenant, context.tenant)
|> Keyword.put(:changeset, original_changeset)
Expand Down
88 changes: 40 additions & 48 deletions lib/ash_authentication/token_resource/actions.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ defmodule AshAuthentication.TokenResource.Actions do
The code interface for interacting with the token resource.
"""

alias Ash.{Changeset, DataLayer, Notifier, Query, Resource}
alias Ash.{Changeset, Query, Resource}
alias AshAuthentication.{TokenResource, TokenResource.Info}

import AshAuthentication.Utils
Expand Down Expand Up @@ -35,30 +35,45 @@ defmodule AshAuthentication.TokenResource.Actions do
def expunge_expired(resource, opts \\ []) do
expunge_expired_action_name = Info.token_expunge_expired_action_name!(resource)

resource
|> DataLayer.transaction(
fn -> expunge_inside_transaction(resource, expunge_expired_action_name, opts) end,
nil,
%{
type: :bulk_destroy,
metadata: %{
metadata: %{
resource: resource,
action: expunge_expired_action_name
}
}
}
)
|> case do
{:ok, {:ok, notifications}} ->
Notifier.notify(notifications)
:ok

{:ok, {:error, reason}} ->
{:error, reason}

{:error, reason} ->
{:error, reason}
with :ok <- assert_resource_has_extension(resource, TokenResource),
{:ok, read_expired_action_name} <- Info.token_read_expired_action_name(resource) do
opts =
opts
|> Keyword.put_new_lazy(:domain, fn -> Info.token_domain!(resource) end)

authorize_with =
if Ash.DataLayer.data_layer_can?(resource, :expr_error) do
:error
else
:filter
end

resource
|> Query.new()
|> Query.set_context(%{private: %{ash_authentication?: true}})
|> Query.for_read(read_expired_action_name, %{}, opts)
|> Ash.bulk_destroy(
expunge_expired_action_name,
%{},
opts
|> Keyword.update(
:context,
%{private: %{ash_authentication?: true}},
&Ash.Helpers.deep_merge_maps(&1, %{private: %{ash_authentication?: true}})
)
|> Keyword.merge(
strategy: [:atomic, :atomic_batches, :stream],
return_errors?: true,
notify?: true,
return_records?: false,
return_notifications?: false,
authorize_with: authorize_with
)
)
|> case do
%{status: :success} -> :ok
%{errors: errors} -> {:error, Ash.Error.to_class(errors)}
end
end
end

Expand Down Expand Up @@ -321,27 +336,4 @@ defmodule AshAuthentication.TokenResource.Actions do
|> Ash.read()
end
end

defp expunge_inside_transaction(resource, expunge_expired_action_name, opts) do
with :ok <- assert_resource_has_extension(resource, TokenResource),
{:ok, read_expired_action_name} <- Info.token_read_expired_action_name(resource) do
opts =
opts
|> Keyword.put_new_lazy(:domain, fn -> Info.token_domain!(resource) end)

resource
|> Query.new()
|> Query.set_context(%{private: %{ash_authentication?: true}})
|> Query.for_read(read_expired_action_name, %{}, opts)
|> Ash.bulk_destroy(
expunge_expired_action_name,
%{},
Keyword.put_new(opts, :strategy, [:atomic, :atomic_batches, :stream])
)
|> case do
%{status: :success, notifications: notifications} -> {:ok, notifications}
%{errors: errors} -> {:error, Ash.Error.to_class(errors)}
end
end
end
end
1 change: 1 addition & 0 deletions mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ defmodule AshAuthentication.MixProject do
{:plug, "~> 1.13"},
{:spark, "~> 2.0"},
{:splode, "~> 0.2"},
{:simple_sat, "~> 0.1", only: [:dev, :test]},
{:absinthe_plug, "~> 1.5", only: [:dev, :test]},
{:ash_graphql, "~> 1.6.0", only: [:dev, :test]},
{:ash_json_api, "~> 1.4.6", only: [:dev, :test]},
Expand Down
5 changes: 3 additions & 2 deletions mix.lock
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"glob_ex": {:hex, :glob_ex, "0.1.11", "cb50d3f1ef53f6ca04d6252c7fde09fd7a1cf63387714fe96f340a1349e62c93", [:mix], [], "hexpm", "342729363056e3145e61766b416769984c329e4378f1d558b63e341020525de4"},
"ham": {:hex, :ham, "0.3.0", "7cd031b4a55fba219c11553e7b13ba73bd86eab4034518445eff1e038cb9a44d", [:mix], [], "hexpm", "7d6c6b73d7a6a83233876cc1b06a4d9b5de05562b228effda4532f9a49852bf6"},
"hpax": {:hex, :hpax, "1.0.2", "762df951b0c399ff67cc57c3995ec3cf46d696e41f0bba17da0518d94acd4aac", [:mix], [], "hexpm", "2f09b4c1074e0abd846747329eaa26d535be0eb3d189fa69d812bfb8bfefd32f"},
"igniter": {:hex, :igniter, "0.5.24", "d5d275f085485350cb9b57bb54d7bf5510fd453ed294e64683879c0549fcead1", [:mix], [{:glob_ex, "~> 0.1.7", [hex: :glob_ex, repo: "hexpm", optional: false]}, {:inflex, "~> 2.0", [hex: :inflex, repo: "hexpm", optional: false]}, {:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:owl, "~> 0.11", [hex: :owl, repo: "hexpm", optional: false]}, {:phx_new, "~> 1.7", [hex: :phx_new, repo: "hexpm", optional: true]}, {:req, "~> 0.5", [hex: :req, repo: "hexpm", optional: false]}, {:rewrite, ">= 1.1.1 and < 2.0.0-0", [hex: :rewrite, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.4", [hex: :sourceror, repo: "hexpm", optional: false]}, {:spitfire, ">= 0.1.3 and < 1.0.0-0", [hex: :spitfire, repo: "hexpm", optional: false]}], "hexpm", "49f4bbc97590164c51745529142c715b7ced772ced3ead2a4153843b9596e903"},
"igniter": {:hex, :igniter, "0.5.27", "7c633dd99150e9cad68285ec8ad7e15833ff0c72d46774ed3be7728c661ec4cb", [:mix], [{:glob_ex, "~> 0.1.7", [hex: :glob_ex, repo: "hexpm", optional: false]}, {:inflex, "~> 2.0", [hex: :inflex, repo: "hexpm", optional: false]}, {:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:owl, "~> 0.11", [hex: :owl, repo: "hexpm", optional: false]}, {:phx_new, "~> 1.7", [hex: :phx_new, repo: "hexpm", optional: true]}, {:req, "~> 0.5", [hex: :req, repo: "hexpm", optional: false]}, {:rewrite, ">= 1.1.1 and < 2.0.0-0", [hex: :rewrite, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.4", [hex: :sourceror, repo: "hexpm", optional: false]}, {:spitfire, ">= 0.1.3 and < 1.0.0-0", [hex: :spitfire, repo: "hexpm", optional: false]}], "hexpm", "3042a71d4466e9c9b98a23d182eb02014a1c4802a35de0fa8233263d27c99550"},
"inflex": {:hex, :inflex, "2.1.0", "a365cf0821a9dacb65067abd95008ca1b0bb7dcdd85ae59965deef2aa062924c", [:mix], [], "hexpm", "14c17d05db4ee9b6d319b0bff1bdf22aa389a25398d1952c7a0b5f3d93162dd8"},
"iterex": {:hex, :iterex, "0.1.2", "58f9b9b9a22a55cbfc7b5234a9c9c63eaac26d276b3db80936c0e1c60355a5a6", [:mix], [], "hexpm", "2e103b8bcc81757a9af121f6dc0df312c9a17220f302b1193ef720460d03029d"},
"jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"},
Expand All @@ -63,9 +63,10 @@
"plug_crypto": {:hex, :plug_crypto, "2.1.0", "f44309c2b06d249c27c8d3f65cfe08158ade08418cf540fd4f72d4d6863abb7b", [:mix], [], "hexpm", "131216a4b030b8f8ce0f26038bc4421ae60e4bb95c5cf5395e1421437824c4fa"},
"postgrex": {:hex, :postgrex, "0.20.0", "363ed03ab4757f6bc47942eff7720640795eb557e1935951c1626f0d303a3aed", [:mix], [{:db_connection, "~> 2.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "d36ef8b36f323d29505314f704e21a1a038e2dc387c6409ee0cd24144e187c0f"},
"ranch": {:hex, :ranch, "1.8.0", "8c7a100a139fd57f17327b6413e4167ac559fbc04ca7448e9be9057311597a1d", [:make, :rebar3], [], "hexpm", "49fbcfd3682fab1f5d109351b61257676da1a2fdbe295904176d5e521a2ddfe5"},
"reactor": {:hex, :reactor, "0.13.1", "c63ff70183aed425f4dcfc7964c034497db0f230e7e7cb4a86c77a4933c6b9e2", [:mix], [{:igniter, "~> 0.4", [hex: :igniter, repo: "hexpm", optional: true]}, {:iterex, "~> 0.1", [hex: :iterex, repo: "hexpm", optional: false]}, {:libgraph, "~> 0.16", [hex: :libgraph, repo: "hexpm", optional: false]}, {:spark, "~> 2.0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, "~> 0.2", [hex: :splode, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.2", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "d9157c96f35f431468cd57f4cd596bb62cdc7eb1852b26e6a67110bf424a26bb"},
"reactor": {:hex, :reactor, "0.13.3", "8d49362564970c3331ba306213bc2416c682a04bfab0f710ac3c740060bbdc71", [:mix], [{:igniter, "~> 0.4", [hex: :igniter, repo: "hexpm", optional: true]}, {:iterex, "~> 0.1", [hex: :iterex, repo: "hexpm", optional: false]}, {:libgraph, "~> 0.16", [hex: :libgraph, repo: "hexpm", optional: false]}, {:spark, "~> 2.0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, "~> 0.2", [hex: :splode, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.2", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "b8227ed82a2aabaedc24a09e347002bb14c58701989d7383c51e941e03085180"},
"req": {:hex, :req, "0.5.8", "50d8d65279d6e343a5e46980ac2a70e97136182950833a1968b371e753f6a662", [:mix], [{:brotli, "~> 0.3.1", [hex: :brotli, repo: "hexpm", optional: true]}, {:ezstd, "~> 1.0", [hex: :ezstd, repo: "hexpm", optional: true]}, {:finch, "~> 0.17", [hex: :finch, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mime, "~> 2.0.6 or ~> 2.1", [hex: :mime, repo: "hexpm", optional: false]}, {:nimble_csv, "~> 1.0", [hex: :nimble_csv, repo: "hexpm", optional: true]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm", "d7fc5898a566477e174f26887821a3c5082b243885520ee4b45555f5d53f40ef"},
"rewrite": {:hex, :rewrite, "1.1.2", "f5a5d10f5fed1491a6ff48e078d4585882695962ccc9e6c779bae025d1f92eda", [:mix], [{:glob_ex, "~> 0.1", [hex: :glob_ex, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.0", [hex: :sourceror, repo: "hexpm", optional: false]}, {:text_diff, "~> 0.1", [hex: :text_diff, repo: "hexpm", optional: false]}], "hexpm", "7f8b94b1e3528d0a47b3e8b7bfeca559d2948a65fa7418a9ad7d7712703d39d4"},
"simple_sat": {:hex, :simple_sat, "0.1.3", "f650fc3c184a5fe741868b5ac56dc77fdbb428468f6dbf1978e14d0334497578", [:mix], [], "hexpm", "a54305066a356b7194dc81db2a89232bacdc0b3edaef68ed9aba28dcbc34887b"},
"sobelow": {:hex, :sobelow, "0.13.0", "218afe9075904793f5c64b8837cc356e493d88fddde126a463839351870b8d1e", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "cd6e9026b85fc35d7529da14f95e85a078d9dd1907a9097b3ba6ac7ebbe34a0d"},
"sourceror": {:hex, :sourceror, "1.7.1", "599d78f4cc2be7d55c9c4fd0a8d772fd0478e3a50e726697c20d13d02aa056d4", [:mix], [], "hexpm", "cd6f268fe29fa00afbc535e215158680a0662b357dc784646d7dff28ac65a0fc"},
"spark": {:hex, :spark, "2.2.45", "19e3a879e80d02853ded85ed7b4c0a84a5d2e395f9d0c884e1a13afbe026929d", [:mix], [{:igniter, ">= 0.3.64 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: true]}, {:sourceror, "~> 1.2", [hex: :sourceror, repo: "hexpm", optional: true]}], "hexpm", "70b272d0ee16e3c10a4f8cf0ef6152840828152e68f2f8e3046e89567f2b49ad"},
Expand Down
7 changes: 4 additions & 3 deletions test/ash_authentication/strategies/password_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ defmodule AshAuthentication.Strategy.PasswordTest do
user = build_user()
subject = AshAuthentication.user_to_subject(user)

tokens = Example.Token |> Ash.read!() |> Enum.group_by(& &1.purpose)
tokens = Example.Token |> Ash.read!(authorize?: false) |> Enum.group_by(& &1.purpose)
assert [%{subject: ^subject}] = tokens["user"]

token_types = tokens |> Map.keys() |> MapSet.new()
Expand All @@ -45,7 +45,7 @@ defmodule AshAuthentication.Strategy.PasswordTest do
user = build_user()
subject = AshAuthentication.user_to_subject(user)

Example.Token |> Ash.bulk_destroy!(:destroy, %{})
Example.Token |> Ash.bulk_destroy!(:destroy, %{}, authorize?: false)

strategy = AshAuthentication.Info.strategy!(User, :password)

Expand All @@ -60,7 +60,8 @@ defmodule AshAuthentication.Strategy.PasswordTest do
context: [token_type: :sign_in]
)

assert [%{subject: ^subject, purpose: "sign_in"}] = Example.Token |> Ash.read!()
assert [%{subject: ^subject, purpose: "sign_in"}] =
Example.Token |> Ash.read!(authorize?: false)
end

test "sign in tokens can only be used once" do
Expand Down
1 change: 1 addition & 0 deletions test/ash_authentication/token_resource/expunger_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ defmodule AshAuthentication.TokenResource.ExpungerTest do
@moduledoc false
use DataCase, async: false
alias AshAuthentication.{Jwt, TokenResource.Expunger}
import ExUnit.CaptureLog

describe "init/1" do
test "it finds all token resources to expunge" do
Expand Down
7 changes: 7 additions & 0 deletions test/support/example/token.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,20 @@ defmodule Example.Token do
use Ash.Resource,
data_layer: AshPostgres.DataLayer,
extensions: [AshAuthentication.TokenResource],
authorizers: [Ash.Policy.Authorizer],
domain: Example

postgres do
table("tokens")
repo(Example.Repo)
end

policies do
bypass always() do
authorize_if AshAuthentication.Checks.AshAuthenticationInteraction
end
end

actions do
defaults [:read, :destroy]

Expand Down

0 comments on commit 8d83fee

Please sign in to comment.