How to run :request_password_reset_with_password action?
Eduardo B. Alexandre:
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)
But this will complain that the changeset is of action type.
Eduardo B. Alexandre:
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:
What is the type of that request password reset action?
ZachDaniel:
You can use
Ash.Resource.Info.action(resource, name).type
to see
Eduardo B. Alexandre:
:update
Eduardo B. Alexandre:
It is the default function AshAuthentication created when I add the resettable option
Eduardo B. Alexandre:
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:
That doesn’t sound right to me 🙂
Eduardo B. Alexandre:
I will paste the code I have here, 1 minute
Eduardo B. Alexandre:
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
ZachDaniel:
So the idea is that you’re supposed to email them that token
Eduardo B. Alexandre:
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:
you’re not supposed to give them the token back over the API
Eduardo B. Alexandre:
I’m not, I’m just printing now in the terminal
Eduardo B. Alexandre:
But they will have access to it via their e-mail, so they can use it whatever way they want
ZachDaniel:
Yeah, I see what you mean, but I don’t know I’d actually consider that a security issue
ZachDaniel:
We can prevent using that token for anything other than resetting the password
ZachDaniel:
But if you have that token, you already have “all the keys to the castle”, so-to-speak
ZachDaniel:
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
ZachDaniel:
We can talk with <@346791515923939328> about adding some metadata to the token to ensure its not used as an authentication token, purely from a principle of least surprise perspective.
Eduardo B. Alexandre:
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:
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. <@346791515923939328> will have some thoughts and if its high priority after his evaluation then we can fix it ASAP in the next few days.
Eduardo B. Alexandre:
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:
That still seems strange to me
ZachDaniel:
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
ZachDaniel:
instead of using the actor
Eduardo B. Alexandre:
Should I create a ticket in the ash_authentication repo requesting the change of that action to be a read?
ZachDaniel:
@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)}
ZachDaniel:
That is how ash_authentication does it under the hood
ZachDaniel:
The idea is that you look up the user with the token
Eduardo B. Alexandre:
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)
Eduardo B. Alexandre:
But I will try that out, seems to have everything I need
ZachDaniel:
Well, I’d suggest calling that function directly
ZachDaniel:
strategy = AshAuthentication.Info.strategy!(UserResource, :password)
ZachDaniel:
and then
AshAuthentication.Strategy.Password.Actions.reset(strategy, %{"reset_token" => reset_token}, [])
jart:
This is definitely a bug and shouldn’t happen. My bad. I know exactly where the issue is.
jart:
Can you please open an issue if you haven’t already.
jart:
Bug is “only tokens with no
act
claim should be useable for sign in”
Eduardo B. Alexandre:
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
Eduardo B. Alexandre:
Yes, I will do that ASAP
jart:
Thanks mate. I’m up a hill in the bush walking the dog.
Eduardo B. Alexandre:
<@346791515923939328> https://github.com/team-alembic/ash_authentication/issues/190