feat!: Configure accepted fields on register

also fixes some .formatter.exs issues and standardizes it using spark tools

This is a breaking change because users of previous versions may be relying on this
behavior. To upgrade, add `register_action_accept [:additional, :fields]` to your
password strategy. This prevents users accidentally adding common security flaw which
is accepting more than intended on a given action.

.formatter.exs

@@ -1,4 +1,6 @@
 spark_locals_without_parens = [
+  access_token_attribute_name: 1,
+  access_token_expires_at_attribute_name: 1,
   api: 1,
   auth0: 0,
   auth0: 1,
@@ -16,12 +18,16 @@ spark_locals_without_parens = [
   confirmation: 2,
   confirmation_required?: 1,
   confirmed_at_field: 1,
+  destroy_action_name: 1,
   enabled?: 1,
   expunge_expired_action_name: 1,
   expunge_interval: 1,
   get_by_subject_action_name: 1,
   get_changes_action_name: 1,
   get_token_action_name: 1,
+  github: 0,
+  github: 1,
+  github: 2,
   hash_provider: 1,
   hashed_password_field: 1,
   identity_field: 1,
@@ -30,6 +36,9 @@ spark_locals_without_parens = [
   identity_resource: 1,
   inhibit_updates?: 1,
   is_revoked_action_name: 1,
+  magic_link: 0,
+  magic_link: 1,
+  magic_link: 2,
   monitor_fields: 1,
   oauth2: 0,
   oauth2: 1,
@@ -41,27 +50,41 @@ spark_locals_without_parens = [
   password_field: 1,
   password_reset_action_name: 1,
   private_key: 1,
+  read_action_name: 1,
   read_expired_action_name: 1,
   redirect_uri: 1,
+  refresh_token_attribute_name: 1,
+  register_action_accept: 1,
   register_action_name: 1,
   registration_enabled?: 1,
+  request_action_name: 1,
   request_password_reset_action_name: 1,
   require_token_presence_for_authentication?: 1,
   resettable: 0,
   resettable: 1,
   revoke_token_action_name: 1,
+  select_for_senders: 1,
   sender: 1,
   sign_in_action_name: 1,
+  sign_in_enabled?: 1,
   signing_algorithm: 1,
   signing_secret: 1,
+  single_use_token?: 1,
   site: 1,
   store_all_tokens?: 1,
   store_changes_action_name: 1,
   store_token_action_name: 1,
+  strategy_attribute_name: 1,
   subject_name: 1,
   token_lifetime: 1,
-  token_url: 1,
+  token_param_name: 1,
   token_resource: 1,
+  token_url: 1,
+  uid_attribute_name: 1,
+  upsert_action_name: 1,
+  user_id_attribute_name: 1,
+  user_relationship_name: 1,
+  user_resource: 1,
   user_url: 1
 ]
 

.github/workflows/elixir_lib.yml

@@ -40,6 +40,17 @@ jobs:
         with:
           mix-env: test
 
+  spark-formatter:
+    name: mix spark.formatter --check
+    runs-on: ubuntu-latest
+    needs: build-test
+    steps:
+      - uses: actions/checkout@v3
+      - uses: team-alembic/staple-actions/actions/mix-task@main
+        with:
+          mix-env: test
+          task: spark.formatter --check
+
   credo:
     name: mix credo --strict
     runs-on: ubuntu-latest
@@ -113,6 +124,7 @@ jobs:
       - credo
       - doctor
       - formatter
+      - spark-formatter
       - auditor
       - test
       - dialyzer

lib/ash_authentication/strategies/password.ex

@@ -106,6 +106,7 @@ defmodule AshAuthentication.Strategy.Password do
             registration_enabled?: true,
             sign_in_enabled?: true,
             resettable: [],
+            register_action_accept: [],
             name: nil,
             provider: :password,
             resource: nil
@@ -130,6 +131,7 @@ defmodule AshAuthentication.Strategy.Password do
           confirmation_required?: boolean,
           password_field: atom,
           password_confirmation_field: atom,
+          register_action_accept: [atom],
           register_action_name: atom,
           sign_in_action_name: atom,
           registration_enabled?: boolean,

lib/ash_authentication/strategies/password/dsl.ex

@@ -72,6 +72,11 @@ defmodule AshAuthentication.Strategy.Password.Dsl do
           """,
           default: true
         ],
+        register_action_accept: [
+          type: {:list, :atom},
+          default: [],
+          doc: "A list of additional fields to be accepted in the register action."
+        ],
         password_field: [
           type: :atom,
           doc: """

lib/ash_authentication/strategies/password/transformer.ex

@@ -144,8 +144,14 @@ defmodule AshAuthentication.Strategy.Password.Transformer do
         []
       end
 
+    accept =
+      [strategy.identity_field]
+      |> Enum.concat(List.wrap(strategy.register_action_accept))
+      |> Enum.uniq()
+
     Transformer.build_entity(Resource.Dsl, [:actions], :create,
       name: strategy.register_action_name,
+      accept: accept,
       arguments: arguments,
       changes: changes,
       metadata: metadata,

mix.exs

@@ -179,6 +179,9 @@ defmodule AshAuthentication.MixProject do
         "hex.audit",
         "test"
       ],
+      "spark.formatter": [
+        "spark.formatter --extensions AshAuthentication,AshAuthentication.TokenResource,AshAuthentication.UserIdentity,AshAuthentication.Strategy.MagicLink,AshAuthentication.AddOn.Confirmation,AshAuthentication.Strategy.Auth0,AshAuthentication.Strategy.Github,AshAuthentication.Strategy.OAuth2,AshAuthentication.Strategy.Password"
+      ],
       docs: ["docs", "ash.replace_doc_links"],
       test: ["ecto.create --quiet", "ecto.migrate --quiet", "test"]
     ]

mix.lock

@@ -53,7 +53,7 @@
   "postgrex": {:hex, :postgrex, "0.16.5", "fcc4035cc90e23933c5d69a9cd686e329469446ef7abba2cf70f08e2c4b69810", [:mix], [{:connection, "~> 1.1", [hex: :connection, repo: "hexpm", optional: false]}, {: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", "edead639dc6e882618c01d8fc891214c481ab9a3788dfe38dd5e37fd1d5fb2e8"},
   "ranch": {:hex, :ranch, "1.8.0", "8c7a100a139fd57f17327b6413e4167ac559fbc04ca7448e9be9057311597a1d", [:make, :rebar3], [], "hexpm", "49fbcfd3682fab1f5d109351b61257676da1a2fdbe295904176d5e521a2ddfe5"},
   "sourceror": {:hex, :sourceror, "0.12.1", "239a98ae2ed191528d64e079eaa355f6f1f69318dbb51796e08497dd3b24d10e", [:mix], [], "hexpm", "b5e310385813d0c791e8a481516654a4e10b7a0fdb55b4fc4ef915fbc0899b8f"},
-  "spark": {:hex, :spark, "0.4.5", "fbb6e7b30ca38b75a2af4b2c3d167d038c9f8f35ea6d0a014dad6256089f0d62", [:mix], [{:nimble_options, "~> 0.5", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:sourceror, "~> 0.1", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "948f4370521d04c4f99c46b3c4bb781fa00e1bc5172e76540a37eb1f2b5e75a8"},
+  "spark": {:hex, :spark, "0.4.7", "72e2288eb813fd8f84c5339fb99421776512c2a5ed7dc5c191041595208f8559", [:mix], [{:nimble_options, "~> 0.5", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:sourceror, "~> 0.1", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "ea99174f2d5047f75322a0a2590462c973bb3cf95f31725e5ec1c232ef244d9f"},
   "stream_data": {:hex, :stream_data, "0.5.0", "b27641e58941685c75b353577dc602c9d2c12292dd84babf506c2033cd97893e", [:mix], [], "hexpm", "012bd2eec069ada4db3411f9115ccafa38540a3c78c4c0349f151fc761b9e271"},
   "telemetry": {:hex, :telemetry, "1.2.1", "68fdfe8d8f05a8428483a97d7aab2f268aaff24b49e0f599faa091f1d4e7f61c", [:rebar3], [], "hexpm", "dad9ce9d8effc621708f99eac538ef1cbe05d6a874dd741de2e689c47feafed5"},
   "typable": {:hex, :typable, "0.3.0", "0431e121d124cd26f312123e313d2689b9a5322b15add65d424c07779eaa3ca1", [:mix], [], "hexpm", "880a0797752da1a4c508ac48f94711e04c86156f498065a83d160eef945858f8"},

priv/repo/migrations/20230302020116_migrate_resources2.exs

@@ -0,0 +1,23 @@
+defmodule Example.Repo.Migrations.MigrateResources2 do
+  @moduledoc """
+  Updates resources based on their most recent snapshots.
+
+  This file was autogenerated with `mix ash_postgres.generate_migrations`
+  """
+
+  use Ecto.Migration
+
+  def up do
+    alter table(:user) do
+      add :extra_stuff, :text
+      add :not_accepted_extra_stuff, :text
+    end
+  end
+
+  def down do
+    alter table(:user) do
+      remove :not_accepted_extra_stuff
+      remove :extra_stuff
+    end
+  end
+end

priv/resource_snapshots/repo/user/20230302020116.json

@@ -0,0 +1,108 @@
+{
+  "attributes": [
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "primary_key?": false,
+      "references": null,
+      "size": null,
+      "source": "confirmed_at",
+      "type": "utc_datetime_usec"
+    },
+    {
+      "allow_nil?": false,
+      "default": "fragment(\"uuid_generate_v4()\")",
+      "generated?": false,
+      "primary_key?": true,
+      "references": null,
+      "size": null,
+      "source": "id",
+      "type": "uuid"
+    },
+    {
+      "allow_nil?": false,
+      "default": "nil",
+      "generated?": false,
+      "primary_key?": false,
+      "references": null,
+      "size": null,
+      "source": "username",
+      "type": "citext"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "primary_key?": false,
+      "references": null,
+      "size": null,
+      "source": "extra_stuff",
+      "type": "text"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "primary_key?": false,
+      "references": null,
+      "size": null,
+      "source": "not_accepted_extra_stuff",
+      "type": "text"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "primary_key?": false,
+      "references": null,
+      "size": null,
+      "source": "hashed_password",
+      "type": "text"
+    },
+    {
+      "allow_nil?": false,
+      "default": "fragment(\"now()\")",
+      "generated?": false,
+      "primary_key?": false,
+      "references": null,
+      "size": null,
+      "source": "created_at",
+      "type": "utc_datetime_usec"
+    },
+    {
+      "allow_nil?": false,
+      "default": "fragment(\"now()\")",
+      "generated?": false,
+      "primary_key?": false,
+      "references": null,
+      "size": null,
+      "source": "updated_at",
+      "type": "utc_datetime_usec"
+    }
+  ],
+  "base_filter": null,
+  "check_constraints": [],
+  "custom_indexes": [],
+  "custom_statements": [],
+  "has_create_action": true,
+  "hash": "D59416503FF032F48041F4E1793CD42EE01FF6AB3D5C4FF4A9D9CABA836E96C1",
+  "identities": [
+    {
+      "base_filter": null,
+      "index_name": "user_username_index",
+      "keys": [
+        "username"
+      ],
+      "name": "username"
+    }
+  ],
+  "multitenancy": {
+    "attribute": null,
+    "global": null,
+    "strategy": null
+  },
+  "repo": "Elixir.Example.Repo",
+  "schema": null,
+  "table": "user"
+}

test/ash_authentication/strategies/password/actions_test.exs

@@ -78,6 +78,59 @@ defmodule AshAuthentication.Strategy.Password.ActionsTest do
       assert claims["sub"] =~ "user?id=#{user.id}"
     end
 
+    test "it can register a new user with additional accepted fields" do
+      {:ok, strategy} = Info.strategy(Example.User, :password)
+
+      username = username()
+      password = password()
+
+      assert {:ok, user} =
+               Actions.register(
+                 strategy,
+                 %{
+                   "username" => username,
+                   "password" => password,
+                   "password_confirmation" => password,
+                   "extra_stuff" => "Extra"
+                 },
+                 []
+               )
+
+      assert user.extra_stuff == "Extra"
+
+      assert strategy.hash_provider.valid?(password, user.hashed_password)
+
+      assert {:ok, claims} = Jwt.peek(user.__metadata__.token)
+      assert claims["sub"] =~ "user?id=#{user.id}"
+    end
+
+    test "it cant set unaccepted fields" do
+      {:ok, strategy} = Info.strategy(Example.User, :password)
+
+      username = username()
+      password = password()
+
+      assert {:error,
+              %Ash.Error.Invalid{
+                errors: [
+                  %Ash.Error.Changes.InvalidAttribute{
+                    message: "cannot be changed",
+                    field: :not_accepted_extra_stuff
+                  }
+                ]
+              }} =
+               Actions.register(
+                 strategy,
+                 %{
+                   "username" => username,
+                   "password" => password,
+                   "password_confirmation" => password,
+                   "not_accepted_extra_stuff" => "Extra"
+                 },
+                 []
+               )
+    end
+
     test "it returns an error if the user already exists" do
       user = build_user()
       {:ok, strategy} = Info.strategy(Example.User, :password)

test/support/data_case.ex

@@ -72,17 +72,19 @@ defmodule DataCase do
   def build_user(attrs \\ []) do
     password = password()
 
-    attrs =
+    {force_change_attrs, attrs} =
       attrs
       |> Map.new()
       |> Map.put_new(:username, username())
       |> Map.put_new(:password, password)
       |> Map.put_new(:password_confirmation, password)
+      |> Map.split([:id])
 
     user =
       Example.User
       |> Ash.Changeset.new()
       |> Ash.Changeset.for_create(:register_with_password, attrs)
+      |> Ash.Changeset.force_change_attributes(force_change_attrs)
       |> Example.create!()
 
     attrs

test/support/example/user.ex

@@ -23,6 +23,8 @@ defmodule Example.User do
     uuid_primary_key :id, writable?: true
 
     attribute :username, :ci_string, allow_nil?: false
+    attribute :extra_stuff, :string
+    attribute :not_accepted_extra_stuff, :string
     attribute :hashed_password, :string, allow_nil?: true, sensitive?: true, private?: true
 
     create_timestamp :created_at
@@ -165,6 +167,8 @@ defmodule Example.User do
 
     strategies do
       password do
+        register_action_accept [:extra_stuff]
+
         resettable do
           sender fn user, token, _opts ->
             Logger.debug(