Forbidden Field Causing an Issue During Update

[allenwyma]

Describe the bug Forbidden field of Decimal type is erroring out when updating a field.

To Reproduce Connect with me if need.

Expected behavior A user is trying to update a record, that cannot edit/see the field that is forbidden, but the field is directly being passed Decimal.decimal/1

** (EXIT from #PID<0.12463.0>) an exception was raised:
         ** (Ash.Error.Unknown) Unknown Error

     * %FunctionClauseError{module: Decimal, function: :decimal, arity: 1, kind: nil, args: nil, clauses: nil}
       (decimal 2.1.1) Decimal.decimal(#Ash.ForbiddenField<field: :foo, type: :attribute, ...>)
       (decimal 2.1.1) lib/decimal.ex:386: Decimal.compare/2
       (decimal 2.1.1) lib/decimal.ex:447: Decimal.eq?/2
       (ash 3.4.36) lib/ash/changeset/changeset.ex:5168: Ash.Changeset.do_change_attribute/4
       (ash 3.4.36) lib/ash/changeset/changeset.ex:3714: anonymous fn/2 in Ash.Changeset.run_before_actions/1
       (elixir 1.17.3) lib/enum.ex:4858: Enumerable.List.reduce/3
       (elixir 1.17.3) lib/enum.ex:2585: Enum.reduce_while/3
       (ash 3.4.36) lib/ash/changeset/changeset.ex:3689: Ash.Changeset.run_before_actions/1
       (ash 3.4.36) lib/ash/changeset/changeset.ex:3829: Ash.Changeset.run_around_actions/2
       (ash 3.4.36) lib/ash/changeset/changeset.ex:3381: anonymous fn/3 in Ash.Changeset.with_hooks/3
       (ecto_sql 3.12.1) lib/ecto/adapters/sql.ex:1400: anonymous fn/3 in Ecto.Adapters.SQL.checkout_or_transaction/4
       (db_connection 2.7.0) lib/db_connection.ex:1756: DBConnection.run_transaction/4
       (ash 3.4.36) lib/ash/changeset/changeset.ex:3379: anonymous fn/3 in Ash.Changeset.with_hooks/3
       (ash 3.4.36) lib/ash/changeset/changeset.ex:3523: anonymous fn/2 in Ash.Changeset.transaction_hooks/2
       (ash 3.4.36) lib/ash/changeset/changeset.ex:3360: Ash.Changeset.with_hooks/3
       (ash 3.4.36) lib/ash/actions/update/update.ex:403: Ash.Actions.Update.commit/3
       (ash 3.4.36) lib/ash/actions/update/update.ex:283: Ash.Actions.Update.do_run/4
       (ash 3.4.36) lib/ash/actions/update/update.ex:240: Ash.Actions.Update.run/4
       (ash 3.4.36) lib/ash.ex:2665: Ash.update/3
       (ash_phoenix 2.1.6) lib/ash_phoenix/form/form.ex:2099: AshPhoenix.Form.with_changeset/2
             (decimal 2.1.1) Decimal.decimal(#Ash.ForbiddenField<field: :rate, type: :attribute, ...>)
             (decimal 2.1.1) lib/decimal.ex:386: Decimal.compare/2
             (decimal 2.1.1) lib/decimal.ex:447: Decimal.eq?/2
             (ash 3.4.36) lib/ash/changeset/changeset.ex:5168: Ash.Changeset.do_change_attribute/4
             (ash 3.4.36) lib/ash/changeset/changeset.ex:3714: anonymous fn/2 in Ash.Changeset.run_before_actions/1
             (elixir 1.17.3) lib/enum.ex:4858: Enumerable.List.reduce/3
             (elixir 1.17.3) lib/enum.ex:2585: Enum.reduce_while/3
             (ash 3.4.36) lib/ash/changeset/changeset.ex:3689: Ash.Changeset.run_before_actions/1
             (ash 3.4.36) lib/ash/changeset/changeset.ex:3829: Ash.Changeset.run_around_actions/2
             (ash 3.4.36) lib/ash/changeset/changeset.ex:3381: anonymous fn/3 in Ash.Changeset.with_hooks/3
             (ecto_sql 3.12.1) lib/ecto/adapters/sql.ex:1400: anonymous fn/3 in Ecto.Adapters.SQL.checkout_or_transaction/4
             (db_connection 2.7.0) lib/db_connection.ex:1756: DBConnection.run_transaction/4
             (ash 3.4.36) lib/ash/changeset/changeset.ex:3379: anonymous fn/3 in Ash.Changeset.with_hooks/3
             (ash 3.4.36) lib/ash/changeset/changeset.ex:3523: anonymous fn/2 in Ash.Changeset.transaction_hooks/2
             (ash 3.4.36) lib/ash/changeset/changeset.ex:3360: Ash.Changeset.with_hooks/3
             (ash 3.4.36) lib/ash/actions/update/update.ex:403: Ash.Actions.Update.commit/3
             (ash 3.4.36) lib/ash/actions/update/update.ex:283: Ash.Actions.Update.do_run/4
             (ash 3.4.36) lib/ash/actions/update/update.ex:240: Ash.Actions.Update.run/4
             (ash 3.4.36) lib/ash.ex:2665: Ash.update/3
             (ash_phoenix 2.1.6) lib/ash_phoenix/form/form.ex:2099: AshPhoenix.Form.with_changeset/2

Runtime

• Elixir version 1.17.3
• Erlang version 27.1.
• OS Mac OS
• Ash version 3.4.36
• any related extension versions

Additional context Add any other context about the problem here.

[zachdaniel]

Will look into this on Monday. This will require some thinking about what to actually do in this scenario. If the value isn't available, the changeset can't do its update logic, so we have to choose from some options:

1. refetch any forbidden fields. This is hard because we don't know what fields will be used by custom changes, so what we'd actually need to do is refetch all forbidden fields.
2. not allow doing this. It could be considered a forbidden error to update a value when you've given us a record w/ the original value being a forbidden field.
3. assume the new value is distinct from the old value, or act as if the old value is nil in the case that it is a forbidden field.

[allenwyma]

For some context: this is a field that's not reachable to the user, and they are unaware of it, so at least in this case, we shouldn't be updating fields that are forbidden.

Is there a way we can just skip forbidden fields from being updated?

[zachdaniel]

Hmm...is that field accepted by the action? Is it being passed in as input?

[allenwyma]

Okay, so for background: when a user creates this field, i copy this field value from a related field for historical reasons. But the test that's failing, no one will be editing that field as I do not provide a method in that form for editing it. It gets set on creation, and not updated.

Hence, I was thinking if any data is sent in the form, it should (in my understanding) only update that field from the params, and this will never get sent in params cause it doesn't get updated at all after creation.

[zachdaniel]

how do you copy the field value from a related field?

[zachdaniel]

Ultimately, something you're doing is triggering the code that compares old value w/ new value, so at some point you're either passing it in as a parm or using Ash.Changeset.change_attribute or force_change_attribute etc.

[allenwyma]

Useing Changeset.before_action, then I end with Changeset.change_attributes, but I suppose I can switch to using the force change if this is a problem, but the test that is failing and the main problem is the updating, not the creation.

[zachdaniel]

force_change_attribute doesn't bypass the previous value logic, so using that wouldn't change anything. What I'm saying is that on your update action, you are in some way attempting to change the field in question here. Only you could say how. Either by being provided as a parameter and accepting it as input, or somewhere in a change.

[zachdaniel]

And its safe to say that its in a before_action hook given this in the stacktrace: (ash 3.4.36) lib/ash/changeset/changeset.ex:3689: Ash.Changeset.run_before_actions/1

[allenwyma]

hi @zachdaniel you're right, looking closer, looks like i do do an update in that before_action.

Maybe it'd be best if I check if it's forbidden, then load the field from not authorized load?

What do you recommend?

[zachdaniel]

Hmm...it's hard to say. In most cases it doesn't actually matter to compare the old version vs the new version. It's done to avoid changing things "unnecessarily", from the perspective of the caller. i.e in a form if the old value is 10 and the new value is 10, the user meant for it to stay the same, not to "become 10". But in your case, you do mean for it to change regardless. In retrospect...this is how force_change_attribute ought to behave. So maybe that is really the bug here, and we can solve the ergonomics of users being able to update fields they can't see later. But that is also a pretty rare edge case.

[zachdaniel]

Give main of ash a try and see how that does for you.

[allenwyma]

Seems like still the same:

** (EXIT from #PID<0.14919.0>) an exception was raised:
         ** (Ash.Error.Unknown) Unknown Error

     * %FunctionClauseError{module: Decimal, function: :decimal, arity: 1, kind: nil, args: nil, clauses: nil}
       (decimal 2.1.1) Decimal.decimal(#Ash.ForbiddenField<field: :foo, type: :attribute, ...>)
       (decimal 2.1.1) lib/decimal.ex:386: Decimal.compare/2
       (decimal 2.1.1) lib/decimal.ex:447: Decimal.eq?/2
       (ecto 3.12.4) lib/ecto/changeset.ex:1907: Ecto.Changeset.put_change/7
       (stdlib 6.1.1) maps.erl:860: :maps.fold_1/4
       (ecto 3.12.4) lib/ecto/changeset.ex:493: Ecto.Changeset.change/2
       (ash_postgres 2.4.11) lib/data_layer.ex:2081: AshPostgres.DataLayer.ecto_changeset/4
       (ash_postgres 2.4.11) lib/data_layer.ex:1325: AshPostgres.DataLayer.update_query/4
       (ash_postgres 2.4.11) lib/data_layer.ex:2777: AshPostgres.DataLayer.update/2
       (ash 3.4.36) lib/ash/actions/update/update.ex:487: anonymous fn/5 in Ash.Actions.Update.commit/3
       (ash 3.4.36) lib/ash/changeset/changeset.ex:3846: Ash.Changeset.run_around_actions/2
       (ash 3.4.36) lib/ash/changeset/changeset.ex:3381: anonymous fn/3 in Ash.Changeset.with_hooks/3
       (ecto_sql 3.12.1) lib/ecto/adapters/sql.ex:1400: anonymous fn/3 in Ecto.Adapters.SQL.checkout_or_transaction/4
       (db_connection 2.7.0) lib/db_connection.ex:1756: DBConnection.run_transaction/4
       (ash 3.4.36) lib/ash/changeset/changeset.ex:3379: anonymous fn/3 in Ash.Changeset.with_hooks/3
       (ash 3.4.36) lib/ash/changeset/changeset.ex:3523: anonymous fn/2 in Ash.Changeset.transaction_hooks/2
       (ash 3.4.36) lib/ash/changeset/changeset.ex:3360: Ash.Changeset.with_hooks/3
       (ash 3.4.36) lib/ash/actions/update/update.ex:403: Ash.Actions.Update.commit/3
       (ash 3.4.36) lib/ash/actions/update/update.ex:283: Ash.Actions.Update.do_run/4
       (ash 3.4.36) lib/ash/actions/update/update.ex:240: Ash.Actions.Update.run/4
             (decimal 2.1.1) Decimal.decimal(#Ash.ForbiddenField<field: :rate, type: :attribute, ...>)
             (decimal 2.1.1) lib/decimal.ex:386: Decimal.compare/2
             (decimal 2.1.1) lib/decimal.ex:447: Decimal.eq?/2
             (ecto 3.12.4) lib/ecto/changeset.ex:1907: Ecto.Changeset.put_change/7
             (stdlib 6.1.1) maps.erl:860: :maps.fold_1/4
             (ecto 3.12.4) lib/ecto/changeset.ex:493: Ecto.Changeset.change/2
             (ash_postgres 2.4.11) lib/data_layer.ex:2081: AshPostgres.DataLayer.ecto_changeset/4
             (ash_postgres 2.4.11) lib/data_layer.ex:1325: AshPostgres.DataLayer.update_query/4
             (ash_postgres 2.4.11) lib/data_layer.ex:2777: AshPostgres.DataLayer.update/2
             (ash 3.4.36) lib/ash/actions/update/update.ex:487: anonymous fn/5 in Ash.Actions.Update.commit/3
             (ash 3.4.36) lib/ash/changeset/changeset.ex:3846: Ash.Changeset.run_around_actions/2
             (ash 3.4.36) lib/ash/changeset/changeset.ex:3381: anonymous fn/3 in Ash.Changeset.with_hooks/3
             (ecto_sql 3.12.1) lib/ecto/adapters/sql.ex:1400: anonymous fn/3 in Ecto.Adapters.SQL.checkout_or_transaction/4
             (db_connection 2.7.0) lib/db_connection.ex:1756: DBConnection.run_transaction/4
             (ash 3.4.36) lib/ash/changeset/changeset.ex:3379: anonymous fn/3 in Ash.Changeset.with_hooks/3
             (ash 3.4.36) lib/ash/changeset/changeset.ex:3523: anonymous fn/2 in Ash.Changeset.transaction_hooks/2
             (ash 3.4.36) lib/ash/changeset/changeset.ex:3360: Ash.Changeset.with_hooks/3
             (ash 3.4.36) lib/ash/actions/update/update.ex:403: Ash.Actions.Update.commit/3
             (ash 3.4.36) lib/ash/actions/update/update.ex:283: Ash.Actions.Update.do_run/4
             (ash 3.4.36) lib/ash/actions/update/update.ex:240: Ash.Actions.Update.run/4

[zachdaniel]

Oh, right 🤔 Thats an error from ash_postgres 🤔 🤔 one sec.

[zachdaniel]

Okay, so the ash change may not be necessary, but it's still the right way that that should behave. Try out ash_postgres main.

[allenwyma]

same

[allenwyma]

okay, works with both ash and ash_postgres main