`require_actor? true` in domain doesn't affect GraphQL mutations

[sb8244]

Describe the bug

I have require_actor? true in my Ash.Domain. My functions generally require the user / tenant it's operating on, so I see this being the norm.

I am doing TDD to implement the GQL layer, and haven't yet set the user/tenant in the context.

Ash is attempting to process the request and is raising an error due to null field value:

23:31:57.513 request_id=F87u13iXaWqJozoAAKZh [warning] `a84e58cc-d453-491e-83ab-b54450664d65`: AshGraphql.Error not implemented for error:

** (Ash.Error.Unknown.UnknownError) ** (Postgrex.Error) ERROR 23502 (not_null_violation) null value in column "team_id" violates not-null constraint

    table: home_page_templates
    column: team_id

Failing row contains (4b0a780d-a1ef-41b4-b58c-d44fbddc2980, null, [{"opaque": 1}], 2024-05-13 03:31:57.472125, 2024-05-13 03:31:57.472125, {2,1}).

To Reproduce

defmodule Super.BackroomAsh do
  use Ash.Domain,
    extensions: [
      AshGraphql.Domain
    ]

  authorization do
    require_actor? true
  end
end

defmodule Super.Backroom.Resource.HomePageTemplate do
  use Ash.Resource,
    domain: Super.BackroomAsh,
    data_layer: AshPostgres.DataLayer,
    extensions: [
      AshGraphql.Resource
    ]

  postgres do
    table "home_page_templates"
    repo Super.Repo
  end

  attributes do
    uuid_primary_key :id, public?: true

    attribute :team_id, :uuid
    attribute :components, {:array, :map}, public?: true
    attribute :column_spans, {:array, :integer}, default: [2, 1], public?: true

    create_timestamp :inserted_at, public?: true
    update_timestamp :updated_at, public?: true
  end

  multitenancy do
    strategy :attribute
    attribute :team_id
  end

  identities do
    identity :unique_team_id, [:team_id]
  end

  validations do
    validate present(:components)
  end

  actions do
    create :create do
      accept [:components]

      upsert? true
      upsert_identity :unique_team_id

      change fn cs, %{actor: user} ->
        Ash.Changeset.set_tenant(cs, user)
      end
    end
  end

  graphql do
    type :ash_home_page_template

    mutations do
      create :ash_create_home_page_template, :create
    end
  end
end

Expected behavior

I expect a GQL error is returned (maybe generic), and a logged error from the domain, like function calls:

     * The domain Super.BackroomAsh requires that an actor is provided at all times and none was provided.

Runtime

Ash 3.0.0 AshGraphql 1.0.0

Additional context Very real chance I'm doing something wrong here and there's a root cause that's my fault.

[zachdaniel]

In Ash, nil is a valid actor. Currently, require_actor?'s job is to require that all places pass the actor option, not to ensure that it is not nil. This is to prevent typos, basically. AshGraphql always sets the actor option, even if one was not provided to it (but it may be nil). A good argument could be made for that being not the best behavior, but it is currently the designed behavior.

Typically this would be implemented somewhere in the authentication flow, where if authentication fails or no authentication information is present you'd reject the user's request.

It is too late to change the semantics of require_actor?, unfortunately, but we could potentially add something like validate_actor/1 that gives you the actor and a chance to provide an arbitrary error message about why that actor is invalid.

Then you could accomplish what you want with something like this:

require_actor? true
validate_actor fn nil -> 
    {:error, "...."}
  %User{} = user ->
     {:ok, user}
end

[sb8244]

Thanks Zach. That's helpful to understand. You're right that my auth flow handles this, it was more of an interim state that felt off. But now I understand so all good.

I didn't have the AshGraphql plug in my pipeline at the time. Not sure if that changes anything.

[zachdaniel]

Shouldn't affect things. I'll close this for now, but if it looks like validate_actor or some similar set of behavior will help in other cases as well then we'll add it to the roadmap :)