Ash Framework•3y ago

How to run :request_password_reset_with_password action?

So, looking at my request_password_reset_with_password action. it expects a changeset of an actual record. But that would mean that I would need an changeset of the user I want to reset the password. How would I get that user if the whole point of doing the reset is because the user is not able to login to that user? I tried this:

params = %{reset_token: "...", password: "87654321", password_confirmation: "87654321"}

Marketplace.Accounts.User.password_reset_with_password(Marketplace.Accounts.User |> Ash.Changeset.new(), params)

params = %{reset_token: "...", password: "87654321", password_confirmation: "87654321"}

Marketplace.Accounts.User.password_reset_with_password(Marketplace.Accounts.User |> Ash.Changeset.new(), params)

But this will complain that the changeset is of action type.

22 Replies

BlibsOP•3y ago

Hmm, I'm wondering if I am expected to use the reset_token as an authentication token (basically using it in the Authorization bearer header).

ZachDaniel•3y ago

What is the type of that request password reset action? You can use Ash.Resource.Info.action(resource, name).type to see

BlibsOP•3y ago

:update It is the default function AshAuthentication created when I add the resettable option So, to be more specific, I'm running this from my graphql API. So, if I pass the reset token to both the input but also as a bearer http header, the system will be able to fetch the actor and then the call will work. But to be honest this for me seem pretty bizarre... Not only I can use that reset token to do whatever I want in the system (I can use to query other apis that expect the user is logged, basically that reset token acts as if the user was logged, it is not only restricted to the reset_password API), I also need to pass it both as the authorization beare http header, but also as an input called reset_token.

ZachDaniel•3y ago

That doesn’t sound right to me 🙂

BlibsOP•3y ago

I will paste the code I have here, 1 minute These two are my API's to handle password reset, the first one will send an email with the reset token and the second one will reset the password using the provided token:

    field :request_password_reset, type: :request_password_reset_output do
      arg :email, non_null(:string)

      resolve fn _, args, _ ->
        User.request_password_reset_with_password(args) |> IO.inspect(label: ">>>>>>> reply")

        {:ok, :ok}
      end
    end

    field :password_reset_with_password, type: :password_reset_with_password_output do
      arg :input, non_null(:password_reset_with_password_input)

      resolve fn a, %{input: args} = b, %{context: %{actor: actor}} ->
        with {:ok, user} <- User.password_reset_with_password(actor, args) do
          {:ok, %{user: user, token: user.__metadata__.token}}
        else
          {:error, %{errors: errors}} ->
            errors = Enum.map(errors, &AshGraphql.Error.to_error/1)

            {:ok, %{errors: errors}}
        end
      end
    end

    field :request_password_reset, type: :request_password_reset_output do
      arg :email, non_null(:string)

      resolve fn _, args, _ ->
        User.request_password_reset_with_password(args) |> IO.inspect(label: ">>>>>>> reply")

        {:ok, :ok}
      end
    end

    field :password_reset_with_password, type: :password_reset_with_password_output do
      arg :input, non_null(:password_reset_with_password_input)

      resolve fn a, %{input: args} = b, %{context: %{actor: actor}} ->
        with {:ok, user} <- User.password_reset_with_password(actor, args) do
          {:ok, %{user: user, token: user.__metadata__.token}}
        else
          {:error, %{errors: errors}} ->
            errors = Enum.map(errors, &AshGraphql.Error.to_error/1)

            {:ok, %{errors: errors}}
        end
      end
    end

ZachDaniel•3y ago

So the idea is that you're supposed to email them that token

BlibsOP•3y ago

With the token provided by the request_password_reset_with_password action, I can just add it as an authorization bearer http header and I can call any API that expects the user to be logged in. In my view, this is a security issue, I expected that token to be specific to the reset password API.

ZachDaniel•3y ago

you're not supposed to give them the token back over the API

BlibsOP•3y ago

I'm not, I'm just printing now in the terminal But they will have access to it via their e-mail, so they can use it whatever way they want

ZachDaniel•3y ago

Yeah, I see what you mean, but I don't know I'd actually consider that a security issue We can prevent using that token for anything other than resetting the password But if you have that token, you already have "all the keys to the castle", so-to-speak having that token means you can go reset the users password and get back a token to authenticate as that user, meaning they are effectively the same We can talk with @jart about adding some metadata to the token to ensure its not used as an authentication token, purely from a principle of least surprise perspective.

BlibsOP•3y ago

Sure, that is true, but it is nice to have it only be usable in a limited scope. For example, if someone else has access to it, if the token was only available to the reset API, and that reset API sent an e-mail notifying the user that the password was successfully changed, that would shrink the window of possible damage that the attack could do I think

ZachDaniel•3y ago

Yeah, it could close the window a bit potentially. That user with that token could still go reset the password and then continue on, but you're right that it could cause an email to be sent allowing the user to maybe do something to address it. Main thing being I don't think that is a "critical" issue necessarily. @jart will have some thoughts and if its high priority after his evaluation then we can fix it ASAP in the next few days.

BlibsOP•3y ago

But going back to the API, for the password_reset_with_password to work, I need to have the user that I will reset the password already fetched. For my case what I did was put the reset token as the authorization HTTP header so the route pipeline would be able to fetch the actor from the token and I would have it in the absinthe context as you can see in the code above. But the password_reset_with_password action also needs that token to be passed as an argument to the function itself, so I also had to send the same token as an input of my graphQL query meaning that args variable in the code above contains something like this: %{reset_token: "...", password: "...", password_confirmation: "..."}. I'm ok with that, but at the same time seems very odd to be that I need to have the user already fetched to call that action, I would expect that I only needed to call the action with the args variable directly and the action itself would fetch the user from the token and update the password.

ZachDaniel•3y ago

That still seems strange to me I'm honestly not the expert on that implementation, but I believe there should be a read action that you can use or something like that to look the user up by the token instead of using the actor

BlibsOP•3y ago

Should I create a ticket in the ash_authentication repo requesting the change of that action to be a read?

ZachDaniel•3y ago

  @doc """
  Attempt to change a user's password using a reset token.
  """
  @spec reset(Password.t(), map, keyword) :: {:ok, Resource.record()} | {:error, any}
  def reset(
        %Password{resettable: [%Password.Resettable{} = resettable]} = strategy,
        params,
        options
      ) do
    with {:ok, token} <- Map.fetch(params, "reset_token"),
         {:ok, %{"sub" => subject}, resource} <- Jwt.verify(token, strategy.resource),
         {:ok, user} <- AshAuthentication.subject_to_user(subject, resource) do
      api = Info.authentication_api!(resource)

      user
      |> Changeset.new()
      |> Changeset.set_context(%{
        private: %{
          ash_authentication?: true
        }
      })
      |> Changeset.for_update(resettable.password_reset_action_name, params)
      |> api.update(options)
    else
      {:error, %Changeset{} = changeset} -> {:error, changeset}
      _ -> {:error, Errors.InvalidToken.exception(type: :reset)}
    end
  end

  def reset(%Password{} = strategy, _params, _options),
    do: {:error, NoSuchAction.exception(resource: strategy.resource, action: :reset, type: :read)}

  @doc """
  Attempt to change a user's password using a reset token.
  """
  @spec reset(Password.t(), map, keyword) :: {:ok, Resource.record()} | {:error, any}
  def reset(
        %Password{resettable: [%Password.Resettable{} = resettable]} = strategy,
        params,
        options
      ) do
    with {:ok, token} <- Map.fetch(params, "reset_token"),
         {:ok, %{"sub" => subject}, resource} <- Jwt.verify(token, strategy.resource),
         {:ok, user} <- AshAuthentication.subject_to_user(subject, resource) do
      api = Info.authentication_api!(resource)

      user
      |> Changeset.new()
      |> Changeset.set_context(%{
        private: %{
          ash_authentication?: true
        }
      })
      |> Changeset.for_update(resettable.password_reset_action_name, params)
      |> api.update(options)
    else
      {:error, %Changeset{} = changeset} -> {:error, changeset}
      _ -> {:error, Errors.InvalidToken.exception(type: :reset)}
    end
  end

  def reset(%Password{} = strategy, _params, _options),
    do: {:error, NoSuchAction.exception(resource: strategy.resource, action: :reset, type: :read)}

That is how ash_authentication does it under the hood The idea is that you look up the user with the token

BlibsOP•3y ago

Yeah, I saw that code, I can adapt it to my case, I just expected that the action itself would do that, so I thought if I did something like this it would probably break in the future (by some API change) But I will try that out, seems to have everything I need

ZachDaniel•3y ago

Well, I'd suggest calling that function directly strategy = AshAuthentication.Info.strategy!(UserResource, :password) and then

AshAuthentication.Strategy.Password.Actions.reset(strategy, %{"reset_token" => reset_token}, [])

AshAuthentication.Strategy.Password.Actions.reset(strategy, %{"reset_token" => reset_token}, [])

jart•3y ago

This is definitely a bug and shouldn’t happen. My bad. I know exactly where the issue is. Can you please open an issue if you haven’t already. Bug is “only tokens with no act claim should be useable for sign in”

BlibsOP•3y ago

Worked like a charm 😄

    field :password_reset_with_password, type: :password_reset_with_password_output do
      arg :input, non_null(:password_reset_with_password_input)

      resolve fn _, %{input: args}, _ ->
        strategy = AshAuthentication.Info.strategy!(User, :password)
        args = args |> Enum.map(fn {k, v} -> {Atom.to_string(k), v} end) |> Enum.into(%{})

        with {:ok, user} <- AshAuthentication.Strategy.Password.Actions.reset(strategy, args, []) do
          {:ok, %{user: user, token: user.__metadata__.token}}
        else
          {:error, %{errors: errors}} ->
            errors = Enum.map(errors, &AshGraphql.Error.to_error/1)

          {:ok, %{errors: errors}}

          {:error, %AshAuthentication.Errors.InvalidToken{}} ->
            {:ok, %{errors: [%{code: "invalid_token"}]}}
        end
      end
    end

    field :password_reset_with_password, type: :password_reset_with_password_output do
      arg :input, non_null(:password_reset_with_password_input)

      resolve fn _, %{input: args}, _ ->
        strategy = AshAuthentication.Info.strategy!(User, :password)
        args = args |> Enum.map(fn {k, v} -> {Atom.to_string(k), v} end) |> Enum.into(%{})

        with {:ok, user} <- AshAuthentication.Strategy.Password.Actions.reset(strategy, args, []) do
          {:ok, %{user: user, token: user.__metadata__.token}}
        else
          {:error, %{errors: errors}} ->
            errors = Enum.map(errors, &AshGraphql.Error.to_error/1)

          {:ok, %{errors: errors}}

          {:error, %AshAuthentication.Errors.InvalidToken{}} ->
            {:ok, %{errors: [%{code: "invalid_token"}]}}
        end
      end
    end

Yes, I will do that ASAP

jart•3y ago

Thanks mate. I’m up a hill in the bush walking the dog.

BlibsOP•3y ago

@jart https://github.com/team-alembic/ash_authentication/issues/190

Gaming

Programming

How to run :request_password_reset_with_password action?

Did you find this page helpful?