Getting nil as actor in SimpleCheck during AshAuthentication sign-in

Ash Forum Ash Questions / Help
,
1

My user resource thas the following relationship:

    many_to_many :schools, School do
      through UserSchool
      join_relationship :user_schools

      source_attribute_on_join_resource :user_id
      destination_attribute_on_join_resource :school_id
    end

In my School resource, I have the following policy:

  policies do
    alias School.Checks

    policy always() do
      authorize_if Checks.IsConfirmed
    end
  end

And the IsConfirmed check is implemented as this:

defmodule Core.Ash.Checks.IsConfirmed do
  @moduledoc false

  use Ash.Policy.SimpleCheck

  def describe(_), do: "actor account is confirmed"

  def match?(%{confirmed_at: nil}, _, _), do: false
  def match?(%{confirmed_at: _}, _, _), do: true
end

After upgrading to Ash 3.0, when I try to sign-in using the sign_in_with_password action, the actor field from match?/3 always arrive as nil which will fail the match of the check.

Did something changed that I need to update to make this work again? I check both Ash upgrade guide and Ash authentication one but didn’t find anything that would help.

2

Hm…the actor is never set when calling sign_in_with_password AFAIK. It’s not set because we don’t know who it is yet, because the user isn’t authenticated. Are you saying this is happening in the :sign_in_with_password action? Or is this perhaps happening elsewhere?

Actually, I bet I know what changed :slight_smile: The default for authorize? is now true (see the upgrade guide for more on that). Either

  1. you have an action where you need to pass authorize?: false
  2. you need to add the ash authentication bypass. This should be documented in the ash_authentication getting started guide.
3

Hmm, shouldn’t authorize? be false when calling the sign_in_with_password action since, as you correctly said, there is no actor?

I could change it to false manually, but that would apply to every call the School.read action correctly?

What I would prefer is to know if the parent action is a ash_authentication action and handle that accordingly in my policy, but looking at the context, I couldn’t find anything that indicates that the sign_in_with_password action was called.

I also tried to add the ash_authentication bypass rule to School but that doesn’t seem to work either.

Any idea on how I can achieve this? Otherwise I think I will just have to create a match? case where if actor is nil it will authorize, but that doesn’t seem very secure.

4

authorize? will be true when calling sign_in_with_password. It’s not sufficient to know that a given action is an action added by ash authentication, because ash authentication isn’t the only thing capable of calling it.

When you say “parent action”. Definitely don’t create a match allowing a non-logged in user to do the action you’re writing.

I think the missing context here is that you haven’t explained yet exactly what is being called and from where. It’s reasonable to hook into the actions you’re hooking into, but how are you doing it? What is triggering a read of School when a user signs in?

5

Ah, sorry if I was not clear.

The School.read action is being called by this many_to_many relationship:

    many_to_many :schools, School do
      through UserSchool
      join_relationship :user_schools

      source_attribute_on_join_resource :user_id
      destination_attribute_on_join_resource :school_id
    end

So, basically, when I run the sign_in_with_password action, the :school relationship will be loaded by default which will call School.read action and will call my IsConfirmed check.

6

How are you accomplishing the loading by default? A global preparation?

7

Yep, like this:

  preparations do
    prepare build(
              load: [
                :full_name,
                :subscription,
                user_schools: [:school],
                user_organizations: [:organization]
              ]
            )
  end
8

Gotcha. :thinking:. Do you need those loaded on the registration action?

9

So, I use these loads to handle next steps after an user signs-in (or register). For example, I will check the subscription relationship to see if there a plan already active. Or I will check if the school page I’m in right now is one that the user should have access (checking the schools relationship), etc.

i guess I could split this into a manual Ash.load function call after the user sign-in, but in that case I would need to be very careful to always add these whenever I’m updating the user.

If that is the correct/better way to handle it, I will do it, but at the same time I was wondering if there is workarounds :slight_smile:

10

There are workarounds, yes :slight_smile:

Something like this might do it?

prepare fn query, _ -> 
  load_statement = [...]
  if query.action.name == :sign_in_with_password do
    Ash.Query.after_action(query, fn query, [user] = results -> 
      Ash.load(query, load_statement, actor: user)

      query, results ->
        # I don't actually think you need to do this, but `ash_authentication`
        # would translate the "multiple users matching" scenario into an error
        # so if you get more than one result you can ignore it
        {:ok, results}
    end)
  else
    Ash.Query.load(query, load_statement)
  end
end
11

Shouldn’t Ash.load(query, load_statement, actor: user) be Ash.load(user, load_statement, actor: user) and Ash.Query.load(query, load_statement) be Ash.Query.load(user, load_statement)?

12

The first correction you made is correct, the second one is not.

prepare fn query, _ -> 
  load_statement = [...]
  if query.action.name == :sign_in_with_password do
    Ash.Query.after_action(query, fn query, [user] = results -> 
      Ash.load(user, load_statement, actor: user)

      query, results ->
        # I don't actually think you need to do this, but `ash_authentication`
        # would translate the "multiple users matching" scenario into an error
        # so if you get more than one result you can ignore it
        {:ok, results}
    end)
  else
    Ash.Query.load(query, load_statement)
  end
end
1 Like
13

Makes sense.

Thanks a bunch Zach, that worked great!

1 Like