Editing a user requires them to set a password

[blatyo]

So, imagine this scenario.

1. User creates account with Google Oauth using pow_assent
2. User goes to edit page to change their email
3. User gets an error, because they didn't set a password

Because password_hash is nil, the code decides that a password is required. I was thinking about the best way for pow to handle this case and what I came up with is that this code should check if data contains an id. If it does, it means that the user was created without a password and shouldn't require one.

Thoughts?

[blatyo]

This is my workaround right now:

  defp maybe_validate_password(changeset, params, config) do
    new_user? = !Changeset.get_field(changeset, :id)
    has_password_change? = Map.get(params, "password")

    if new_user? || has_password_change? do
      Pow.Ecto.Schema.Changeset.new_password_changeset(changeset, params, config)
    else
      changeset
    end
  end

[danschultzer]

Yeah, I see the issue. To update credentials (the email is the user id here), the current password is needed. If no password has been set, you can still update, but it'll require you to fill out a password so next time it'll require the password when updating. I'm not sure if this requirement makes sense, I'll think about it and check a few other libraries to see what is the most common way of handling it.

Also, I plan to introduce password-less sign in with WebAuthn in #6, so I in any case I need to find some solution to this.

The way to currently fix this is with a custom changeset. I imagine something like this would work:

defmodule MyApp.Users.User do
  use Ecto.Schema
  use Pow.Ecto.Schema

  import Pow.Ecto.Schema.Changeset, only: [password_changeset: 3]

  def changeset(user_or_changeset, attrs) do
    user_or_changeset
    |> pow_user_id_field_changeset(attrs)
    |> pow_current_password_changeset(attrs)
    |> maybe_new_password(attrs, @pow_config)
  end

  defp maybe_new_password(changeset, attrs, config) do
    case new_user?(changeset) or password_changed?(attrs) do
      true  -> password_changeset(user_or_changeset, params, config)
      false -> changeset
    end
  end

  defp new_user?(%{data: data}}), do: Ecto.get_meta(data, :state) == :built

  defp password_changed?(params), do: Map.get(params, "password", "") != ""
end

[danschultzer]

Devise disables password changes completely.

I've been thinking about this and any potential security issues. I agree with your suggestion. It shouldn't validate the password when no password has been set, and no updates to password has been submitted. However, it also makes sense to require auth to update auth details (why current password is required when the user has a password set).

So I'm thinking of refactoring how auth methods are handled in Pow. When you update your user details, Pow should look up what auth methods are available for the current user, e.g. if you have used PowAssent to auth then upon update of the auth details it should redo auth through the provider. Current password will be optional in this case. It should only require a password to be set if no other auth methods are present (so also when you create a new user, the changeset will work as long as there is at least one set of auth details)

This makes sense with WebAuthn where the browser will handle auth. You'll be able to sign up, sign in and update details without a password, and no need to set a password. You just have to authenticate with your device.

I've been pretty busy the last weeks, but I'll get to this ASAP, and see if the above makes sense to implement. Stay tuned :smile:

[blatyo]

Don't feel any need to rush on this for my behalf. The workaround I have works fine.

What you're proposing sounds interesting.