csos95:

defmodule Coruscant.Accounts.User do
  use Ash.Resource,
    data_layer: AshPostgres.DataLayer,
    extensions: [AshAuthentication, AshGraphql.Resource]

  code_interface do
    define_for Coruscant.Accounts
    define :register_with_password, args: [:name, :password, :password_confirmation]
    define :sign_in_with_password, args: [:name, :password]
  end

  graphql do
    type :user

    queries do
      list :list_users, :read
    end

    mutations do
      create :register, :register_with_password
      # if this is uncommented it causes a compile error
      # create :sign_in, :sign_in_with_password
    end
  end

  actions do
    defaults [:read]
  end

  attributes do
    uuid_primary_key :id

    attribute :name, :string do
      allow_nil? false
    end

    attribute :hashed_password, :string do
      allow_nil? false
      sensitive? true
      private? true
    end
  end

  authentication do
    api Coruscant.Accounts

    strategies do
      password :password do
        identity_field :name
      end
    end

    tokens do
      enabled? true
      token_resource Coruscant.Accounts.Token

      signing_secret fn _, _ ->
        Application.fetch_env(:coruscant, :token_signing_secret)
      end
    end
  end

  postgres do
    table "users"
    repo Coruscant.Repo
  end

  identities do
    identity :unique_name, [:name]
  end
end

ZachDaniel:

What is the error?

csos95:

Compiling 3 files (.ex)

== Compilation error in file lib/coruscant/schema.ex ==
** (KeyError) key :accept not found in: %Ash.Resource.Actions.Read{name: :sign_in_with_password, pagination: false, primary?: false, filter: nil, description: nil, get?: true, manual: nil, modify_query: nil, transaction?: false, arguments: [%Ash.Resource.Actions.Argument{allow_nil?: false, type: Ash.Type.String, name: :name, default: nil, private?: false, sensitive?: false, description: nil, constraints: [allow_empty?: false, trim?: true]}, %Ash.Resource.Actions.Argument{allow_nil?: false, type: Ash.Type.String, name: :password, default: nil, private?: false, sensitive?: true, description: nil, constraints: [allow_empty?: false, trim?: true]}], preparations: [%Ash.Resource.Preparation{preparation: {AshAuthentication.Strategy.Password.SignInPreparation, []}}], touches_resources: [], type: :read}
    (ash_graphql 0.22.4) lib/resource/resource.ex:760: anonymous fn/2 in AshGraphql.Resource.mutation_fields/4
    (elixir 1.14.3) lib/enum.ex:4197: Enum.filter_list/2
    (ash_graphql 0.22.4) lib/resource/resource.ex:759: AshGraphql.Resource.mutation_fields/4
    (ash_graphql 0.22.4) lib/resource/resource.ex:438: anonymous fn/4 in AshGraphql.Resource.mutations/3
    (elixir 1.14.3) lib/enum.ex:1658: Enum."-map/2-lists^map/1-0-"/2
    (elixir 1.14.3) lib/enum.ex:1658: Enum."-map/2-lists^map/1-0-"/2
    (ash_graphql 0.22.4) lib/resource/resource.ex:410: AshGraphql.Resource.mutations/3
    (elixir 1.14.3) lib/enum.ex:4249: Enum.flat_map_list/2
    (coruscant 0.1.0) lib/coruscant/schema.ex:6: Coruscant.Accounts.AshTypes.run/2
    (absinthe 1.7.0) lib/absinthe/pipeline.ex:384: Absinthe.Pipeline.run_phase/3
    (absinthe 1.7.0) lib/absinthe/schema.ex:360: Absinthe.Schema.__after_compile__/2
    (stdlib 4.2) lists.erl:1350: :lists.foldl/3
    (elixir 1.14.3) lib/kernel/parallel_compiler.ex:340: anonymous fn/5 in Kernel.ParallelCompiler.spawn_workers/7
Generated coruscant app

ZachDaniel:

Ah, yeah

ZachDaniel:

So the sign in action is a read action.

csos95:

So do I put that under queries with read_one?

ZachDaniel:

Yep, and if you want it to be a mutation in the actual graphql you can pass as_mutation? true in that declaration.

csos95:

It looks like read_one doesn’t have that flag, but get does.

ZachDaniel:

I think you may want to do it with get

ZachDaniel:

Ha, yeah

ZachDaniel:

Was just double checking

csos95:

Does that not require the id?

ZachDaniel:

Because you also want: https://ash-hq.org/docs/dsl/ash_graphql/0.22.4/ashgraphql-resource/graphql/queries/get#modify_resolution

ZachDaniel:

Somethings off with the docs descriptions on that page…

ZachDaniel:

Either way, pass identity nil

ZachDaniel:

And it won’t require an id

csos95:

Yeah, I noticed the values for the doc column on those pages disappeared recently.

ZachDaniel:

Ill fix it tonight 👍

ZachDaniel:

Hexdocs is a good backup

ZachDaniel:

https://hexdocs.pm/ash_graphql/AshGraphql.Resource.html#module-queries

csos95:

Oh wow, that’s a lot more info! I hadn’t checked hexdocs for ash at all.

ZachDaniel:

Yeah…we should probably have a view like that in ash hq instead of hiding it in the sidebar

ZachDaniel:

fixed the missing DSL docs. <@189522455642112000> Did you get things figured out? If so, please mark this post with the solved tag and close it by right clicking on the thread in the left bar and closing it 🙇

csos95:

Sorry, I had to make dinner and then forgot to follow up. It shows up in my graphql schema now as a mutation with the correct parameters, but if I try to call it I get a forbidden error, but it says there are no policy errors.

{
  "data": {
    "signIn": null
  },
  "errors": [
    {
      "code": "Forbidden",
      "fields": [],
      "locations": [
        {
          "column": 3,
          "line": 2
        }
      ],
      "message": "No policy errors",
      "path": [
        "signIn"
      ],
      "short_message": "forbidden",
      "vars": {}
    }
  ]
}

csos95:

I added a modify_resolution that just prints the state and returns it to see if I can find any errors. I haven’t seen any yet, but there’s a lot of output so I might be missing something.

ZachDaniel:

Do you have policies on your resource?

csos95:

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

    policy always() do
      authorize_if always()
    end
  end

csos95:

I had the second one to let everything through, but I added the first from the ash authentication guide just to be safe so I don’t forget it later.

ZachDaniel:

Hm…well. That ought to let anything happen.

ZachDaniel:

Im not sure about the “no policy errors” thing. I don’t recognize that error message

csos95:

I think it comes from here. https://github.com/ash-project/ash/blob/2f3fcbad13cc23c6ac67af4ad13c31b7751003b2/lib/ash/error/forbidden/policy.ex#L66

ZachDaniel:

Oh okay that’s the breakdown

ZachDaniel:

Can you try calling the action in code?

ZachDaniel:

Then you can see the error itself.

ZachDaniel:

I.e ash.Query.for_read(:sign_in, …)

csos95:

Calling it in code seems to work fine.

iex(4)> Ash.Query.for_read(Coruscant.Accounts.User, :sign_in_with_password, %{name: "foo", password: "password"}) |> Coruscant.Accounts.read()
[debug] QUERY OK source="users" db=0.6ms decode=1.6ms queue=0.7ms idle=1945.9ms
SELECT u0."id", u0."name", u0."hashed_password" FROM "users" AS u0 WHERE (u0."name"::text = $1::text) ["foo"]
↳ AshPostgres.DataLayer.run_query/2, at: lib/data_layer.ex:599
{:ok,
 [
   #Coruscant.Accounts.User<
     __meta__: #Ecto.Schema.Metadata<:loaded, "users">,
     id: "7218de28-9c72-4fa8-82f2-45ea2b06feb9",
     name: "foo",
     aggregates: %{},
     calculations: %{},
     __order__: nil,
     ...
   >
 ]}

ZachDaniel:

Do you have tokens enabled?

ZachDaniel:

You might need to add your catch all policy to your tokens resource too

csos95:

Here’s the authentication section in my user resource.

  authentication do
    api Coruscant.Accounts

    strategies do
      password :password do
        identity_field :name
      end
    end

    tokens do
      enabled? true
      token_resource Coruscant.Accounts.Token

      signing_secret fn _, _ ->
        Application.fetch_env(:coruscant, :token_signing_secret)
      end
    end
  end

And I just added the policy to my token.

defmodule Coruscant.Accounts.Token do
  use Ash.Resource,
    data_layer: AshPostgres.DataLayer,
    extensions: [AshAuthentication.TokenResource],
    authorizers: [Ash.Policy.Authorizer]

  policies do
    policy always() do
      authorize_if always()
    end
  end

  token do
    api Coruscant.Accounts
  end

  postgres do
    table "tokens"
    repo Coruscant.Repo
  end
end

csos95:

Still the same result in the playground.

ZachDaniel:

Hmm…try removing the policy authorizer from both?

ZachDaniel:

Be sure to remove the extension too.

ZachDaniel:

Using it in code ought to be the same as graphql.

ZachDaniel:

Are you requesting related information in the response? Can I see the query you’re making?

csos95:

Here’s the query

mutation SignIn($name: String!, $password: String!) {
  signIn(name: $name, password: $password) {
    id
    name
  }
}

and the variables.

{
  "name": "foo",
  "password": "password"
}

csos95:

Same result in the playground (forbidden error) and iex (no error) with the policy and extension on user and token removed.

ZachDaniel:

Oh, when calling it in code can you add authorize?: true option?

ZachDaniel:

Wait

ZachDaniel:

You get a forbidden error even with all of it removed???

ZachDaniel:

Something else weird is going on here

csos95:

Yeah, I’m thinking I missed something important. I was getting the forbidden error before I added any policy stuff so I though it was defaulting to forbidden unless I had a policy to allow it.

ZachDaniel:

Okay so I think this may actually be something weird in the way we display errors here or something…

csos95:

Here’s the repository https://git.csos95.com/csos95/coruscant

ZachDaniel:

Will take a look soon

ZachDaniel:

Yeah, okay so I have some info on this. The basic problem is that we were treating all forbidden errors as “policy forbidden errors” which is incorrect 🙂

ZachDaniel:

So what I will do first here is ship the fix for that, and then we can take a look. For example, signing in with incorrect credentials shows this

{
  "data": {
    "signIn": null
  },
  "errors": [
    {
      "locations": [
        {
          "column": 3,
          "line": 2
        }
      ],
      "message": "Something went wrong. Unique error id: `e8d2c22a-44ea-4e7a-9bee-9c4a4aba8f80`",
      "path": [
        "signIn"
      ]
    }
  ]
}

ZachDaniel:

Which is the expected thing for “unhandled” errors

ZachDaniel:

Then you get a log message like this:

[warning] `e8d2c22a-44ea-4e7a-9bee-9c4a4aba8f80`: AshGraphql.Error not implemented for error:

** (AshAuthentication.Errors.AuthenticationFailed) Authentication failed
    (ash_authentication 3.7.3) lib/ash_authentication/errors/authentication_failed.ex:13: AshAuthentication.Errors.AuthenticationFailed.exception/1
    (ash_authentication 3.7.3) lib/ash_authentication/strategies/password/sign_in_preparation.ex:55: anonymous fn/3 in AshAuthentication.Strategy.Password.SignInPreparation.prepare/3
    (ash 2.5.9) lib/ash/actions/read.ex:1265: anonymous fn/2 in Ash.Actions.Read.run_after_action/2
    (elixir 1.14.0) lib/enum.ex:4751: Enumerable.List.reduce/3
    (elixir 1.14.0) lib/enum.ex:2514: Enum.reduce_while/3
    (ash 2.5.9) lib/ash/actions/read.ex:1263: Ash.Actions.Read.run_after_action/2
    (ash 2.5.9) lib/ash/actions/read.ex:975: anonymous fn/3 in Ash.Actions.Read.data_field/2
    (ash 2.5.9) lib/ash/engine/engine.ex:468: anonymous fn/2 in Ash.Engine.run_iteration/1
    (ash 2.5.9) lib/ash/engine/engine.ex:623: Ash.Engine.start_pending_tasks/1
    (ash 2.5.9) lib/ash/engine/engine.ex:273: Ash.Engine.run_to_completion/1
    (ash 2.5.9) lib/ash/engine/engine.ex:202: Ash.Engine.do_run/2
    (ash 2.5.9) lib/ash/engine/engine.ex:141: Ash.Engine.run/2
    (ash 2.5.9) lib/ash/actions/read.ex:170: Ash.Actions.Read.do_run/3
    (ash 2.5.9) lib/ash/actions/read.ex:90: Ash.Actions.Read.run/3
    (ash 2.5.9) lib/ash/api/api.ex:1005: Ash.Api.read_one/3
    (coruscant 0.1.0) lib/coruscant/accounts.ex:1: Coruscant.Accounts.read_one/2
    (ash_graphql 0.22.4) lib/graphql/resolver.ex:66: AshGraphql.Graphql.Resolver.resolve/2
    (absinthe 1.7.0) lib/absinthe/phase/document/execution/resolution.ex:230: Absinthe.Phase.Document.Execution.Resolution.reduce_resolution/1

ZachDaniel:

So then what you could do is implement defimpl AshGraphql.Error to add a custom error message and present that to the graphql

ZachDaniel:

And you could also inspect the error in your handler to see what the reason for authentication failure is (we don’t put it in the message, because this stuff is sensitive and we don’t necessarily want errors to surface with the underlying reason)

ZachDaniel:

When you get a chance, can you try ash_graphql main, and implement an error handler for AshAuthentication.Errors.AuthenticationFailed to show an error message in the client and see whats going on?

csos95:

Sure, I’ll try that now.

csos95:

%AshAuthentication.Errors.AuthenticationFailed{
  caused_by: %{
    action: :sign_in,
    message: "Query returned too many users",
    module: AshAuthentication.Strategy.Password.SignInPreparation,
    strategy: %AshAuthentication.Strategy.Password{
      identity_field: :name,
      hashed_password_field: :hashed_password,
      hash_provider: AshAuthentication.BcryptProvider,
      confirmation_required?: true,
      password_field: :password,
      password_confirmation_field: :password_confirmation,
      register_action_name: :register_with_password,
      sign_in_action_name: :sign_in_with_password,
      resettable: [],
      name: :password,
      provider: :password,
      resource: Coruscant.Accounts.User
    }
  },
  changeset: nil,
  query: #Ash.Query<
    resource: Coruscant.Accounts.User,
    arguments: %{name: "foo", password: "**redacted**"},
    filter: #Ash.Filter<name == "foo">,
    select: [:id, :name]
  >,
  error_context: [],
  vars: [],
  path: [],
  stacktrace: #Stacktrace<>,
  class: :forbidden
}

ZachDaniel:

Looks like you need an identity on user name

ZachDaniel:

Although I feel like it should have required that you have that…

csos95:

I had thought that the identities part generated an index, but I guess not.

  identities do
    identity :unique_name, [:name]
  end

ZachDaniel:

Well, it does

ZachDaniel:

Did you run the migration generator after adding the identity?

csos95:

I think so, but I’ll reset it and check again. When I do the register action with the same name it returns a null result and I see the db rollback message in the terminal.

ZachDaniel:

Oh, interesting.

csos95:

Yeah, same result with freshly generated resources/database and only running register once.

ZachDaniel:

😢 that is mega strange

csos95:

And the generated migration does have a unique index.

ZachDaniel:

so there is only one user in the database? And its saying “Query returned too many users”??

csos95:

Yeah

ZachDaniel:

what

ZachDaniel:

okay lemme look at that code, one sec

csos95:

I just pushed a commit with the defimpl added.

ZachDaniel:

Okay, I found the issue, or at least the first issue

ZachDaniel:

 query, [record] when is_binary(:erlang.map_get(strategy.hashed_password_field, record))

ZachDaniel:

oh

ZachDaniel:

yeah this is a bug in ash_authentication 🙂

ZachDaniel:

Should have seen this coming, its usually ash_graphql that highlights errors around not using ensure_selected to make sure the proper fields are present

ZachDaniel:

So because ash_graphql selects a subset of fields, the hashed_password field not being one of them, its coming back as nil (which is not necessarily a “security” bug because all that means is no one can log in)

csos95:

Aha! Yeah, if I remove the private? true and select that field, signing in works.

ZachDaniel:

I think you’d also have to select it, right?

csos95:

Yeah

ZachDaniel:

kk, makes sense

ZachDaniel:

this is an easy fix, but congratulations you’re the first person to setup the sign in action over graphql 🙂

ZachDaniel:

I wouldn’t worry about it overly much, the point of Ash is that these actions/behaviors are the same internally across interfaces

csos95:

🥳

ZachDaniel:

So its not like signing in isn’t vetted

csos95:

So with that sorted out, how would I actually retrieve the token on the client? I see that the token gets put under the __metadata__ field in the user value, but the final return value is a user which has no token field that I can put it in with the resolution modifier.

csos95:

I tried adding a calculation field that’s just an empty string and then setting the token value in the resolution modifier, but it seems like the calculation value overwrites it.

ZachDaniel:

Okay, I pushed a branch up to ash_graphql to do the selection

ZachDaniel:

🤔 I’ll push up another thing to fix your issue

ZachDaniel:

on an action you can add

metadata :token, :string, ....

and AshGraphql will add that to the graphql type

ZachDaniel:

Although…that is made for create/update/destroy actions…lemme poke around

ZachDaniel:

Alright, so there is a few things. This may be an oversight in the way that we do read actions that are marked with as_mutation which is that there isn’t a place to return metadata like there is in an actual mutation.

csos95:

Issues using AshAuthentication’s sign_in_with_password action with AshGraphql

ZachDaniel:

This is a bit dumb, but there is an option for you in the short term I believe

ZachDaniel:

Clearly we need to work a bit on how this interacts with ash_graphql

ZachDaniel:

defmodule GetTokenFromMetadata do
  use Ash.Calculation

  def calculate(users, _, _) do
    Enum.map(users, fn user -> 
      Map.get(user.__metadata__, :token)
    end)
  end
end

calculate :token, :string, GetTokenFromMetadata

ZachDaniel:

The reason that this is dumb is that it will add a token field to your user on all the other actions you have

ZachDaniel:

And it will be nil on those other actions because the token metadata isn’t set

ZachDaniel:

but should make it available to the client

csos95:

Yup, that works for me.

ZachDaniel:

Although you don’t necessarily need the token on the client

csos95:

Do you mean setting a cookie or something else?

ZachDaniel:

Yeah, setting a cookie

ZachDaniel:

Not saying we shouldn’t make it easier to get this information on the client

csos95:

I think now my issue has been solved/worked around so I can use it. Do you want me to mark it as solved and close or wait for that ash authentication fix to be pushed?

ZachDaniel:

So the new branch fixes the main issue of having to select the field right?

ZachDaniel:

So the only remaining issue once that fix is in is that you don’t have a good way to return read-action-specific metadata via the graphql

csos95:

Oh! I didn’t check that. It’s the select-fields branch, right?

ZachDaniel:

select_fields I believe

ZachDaniel:

nope

ZachDaniel:

select-fields was right 😆

csos95:

Yes, that fixed it.

ZachDaniel:

🥳 In that case, if you have a few to create an issue about it that would be great

ZachDaniel:

I might actually suggest doing it in ash core

ZachDaniel:

because read actions don’t support the metadata key like the other actions do

csos95:

Should I use the bug or proposal template?

ZachDaniel:

so I can add it to ash core and then add it to ash_authentication and then add it to ash_graphql to make special types for read actions that have metadata. What it would do is make those actions return something like [{result: {name: "name"}, metadata: {token: "token"}] .

ZachDaniel:

proposal template 👍

ZachDaniel:

doesn’t matter too much

csos95:

https://github.com/ash-project/ash/issues/494

ZachDaniel:

🙇