Stuck in Programming Phoenix: Password Hash Not Saved when Inserting New Users into Ecto

Hello! I have been learning Elixir for the last couple of months, and this past week I have started learning Phoenix with Programming Phoenix by Chris McCord, Bruce Tate, and Jose Valim.

Elixir Version: 1.7.3
Phoenix Version: 1.2.5

I am currently stuck on Creating Users in Chapter 5 (Authenticating Users). In Rumbl.UserController.create(), a user was successfully added to the database by passing in the according changeset to Rumbl.Repo.insert(changeset). When checking the Ecto database with Rumbl.Repo.all(Rumbl.User), the only populated fields within the User structs are for name and username. I don’t expect password to be populated as that field was set to virtual in the schema, no problem there. However, the issue is the password_hash field is not populated (set to nil). Because of this, I cannot use Comeonin.Bcrypt.checkpw() to properly authenticate a user.

I have been struggling with this issue for a couple of hours now and can’t seem to find the right resource to help solve the issue.

Rumbl.UserController.create()

def create(conn, %{"user" => user_params}) do        
    user_params = for {key, val} <- user_params, into: %{}, do: {String.to_atom(key), val}        
    changeset = User.registration_changeset(%User{}, user_params)        
    IO.inspect(changeset)
    case Repo.insert(changeset) do
        {:ok, user} ->
            IO.inspect(user)
            conn
            |> Rumbl.Auth.login(user)
            |> put_flash(:info, "#{user.name} created!")
            |> redirect(to: user_path(conn, :index))
        {:error, changeset} ->
            render(conn, "new.html", changeset: changeset)
    end        
end

Rumbl.User

defmodule Rumbl.User do
    use Rumbl.Web, :model

schema "users" do
    field :name, :string
    field :username, :string
    field :password, :string, virtual: true # virtual fields are NOT persisted to the database
    field :password_hash, :string

    timestamps()
end

def changeset(model, params \\ :invalid) do
    model
    |> cast(params, ~w(name username), [])
    |> validate_length(:username, min: 1, max: 20)
end

def registration_changeset(model, params) do
    model
    |> changeset(params)
    |> cast(params, ~w(password), [])
    |> validate_length(:password, min: 6, max: 100)
    |> put_pass_hash()
end

defp put_pass_hash(changeset) do
    case changeset do
        %Ecto.Changeset{valid?: true, changes: %{password: pass}} ->
            put_change(changeset, :pasword_hash, Comeonin.Bcrypt.hashpwsalt(pass))
        _ ->
            changeset
    end
end
end

Rumbl.Auth

defmodule Rumbl.Auth do
            import Plug.Conn
            import Comeonin.Bcrypt, only: [checkpw: 2, dummy_checkpw: 0]
        
def init(opts) do
    Keyword.fetch!(opts, :repo)
end

def call(conn, repo) do
    user_id = get_session(conn, :user_id)
    user = user_id && repo.get(Rumbl.User, user_id)
    assign(conn, :current_user, user)
end

def login(conn, user) do
    conn
    |> assign(:current_user, user)
    |> put_session(:user_id, user.id)
    |> configure_session(renew: true)
end

def login_by_username_and_pass(conn, username, given_pass, opts) do
    repo = Keyword.fetch!(opts, :repo)
    user = repo.get_by(Rumbl.User, username: username)
    IO.puts("Given Pass: #{given_pass}")
    IO.puts("Password Hash: #{inspect user.password_hash}")
    cond do
        user && checkpw(given_pass, user.password_hash) ->
            {:ok, login(conn, user)}
        user ->
            {:error, :unauthorized, conn}
        true ->
            dummy_checkpw()
            {:error, :not_found, conn}
    end
end
end

What does your registration changes etc look like?

Also there is no need to manually atomify the input parameters. Ecto will do this in a more secure manor.

I have updated my post with all the code that I think is relevant to this issue.

In regards to converting the keys from strings to atoms, I was trying to resolve the following warning:

warning: non-atom keys in cast/3 is deprecated. All keys must be atoms, got: `"username"`
  (ecto) lib/ecto/changeset.ex:552: Ecto.Changeset.cast_key/1
  (elixir) lib/enum.ex:765: Enum."-each/2-lists^foreach/1-0-"/2
  (elixir) lib/enum.ex:765: Enum.each/2
  (ecto) lib/ecto/changeset.ex:486: Ecto.Changeset.cast/6
  (rumbl) web/models/user.ex:15: Rumbl.User.changeset/2
  (rumbl) web/controllers/user_controller.ex:18: Rumbl.UserController.new/2
  (rumbl) web/controllers/user_controller.ex:1: Rumbl.UserController.action/2
  (rumbl) web/controllers/user_controller.ex:1: Rumbl.UserController.phoenix_controller_pipeline/2
  (rumbl) lib/rumbl/endpoint.ex:1: Rumbl.Endpoint.instrument/4
  (rumbl) lib/phoenix/router.ex:261: Rumbl.Router.dispatch/2
  (rumbl) web/router.ex:1: Rumbl.Router.do_call/2
  (rumbl) lib/rumbl/endpoint.ex:1: Rumbl.Endpoint.phoenix_pipeline/1
  (rumbl) lib/plug/debugger.ex:123: Rumbl.Endpoint."call (overridable 3)"/2
  (rumbl) lib/rumbl/endpoint.ex:1: Rumbl.Endpoint.call/2
  (plug) lib/plug/adapters/cowboy/handler.ex:15: Plug.Adapters.Cowboy.Handler.upgrade/4

You should not use this, but…

~w(name username)a

The difference lies at the end, a means atom. This should be changed in all your cast.

iex(1)> ~w(koko)
["koko"]
iex(2)> ~w(koko)a
[:koko]

This is why it does not cast correctly…

You should not use atom key for data coming from the outside, as mentionned previously.

And then, it’s a good idea to use context to hide complexity to controller.

For example…

  def create(conn, %{"user" => user_params}) do
    case Accounts.register_user(user_params) do
      {:ok, user} ->
        conn
        |> RumblWeb.Auth.login(user)
        |> put_flash(:info, "#{user.name} created!")
        |> redirect(to: Routes.user_path(conn, :index))
      {:error, changeset} ->
        render(conn, "new.html", changeset: changeset)
    end
  end

And the corresponding context function

  def register_user(attrs \\ %{}) do
    %User{}
    |> User.registration_changeset(attrs)
    |> Repo.insert()
  end

The logic is hidden in the Accounts context.

Hope this helps…

PS: :password is empty (your real value is in the “password” key, because of the wrong casting), it will return an empty hash…

2 Likes

According to cast/4s documentation a string should be fine either as a key in the params, as well as in the permitted list.

If there occurs a warning in either place, then the documentation lies, which is worth a bug report in my opinion.

2 Likes

I have made the suggested change where any instance of ~w(koko) is replaced with ~w(koko)a. I have removed the line in Rumbl.UserController.create() that transform the keys of the incoming params into atoms. However, before the case Repo.insert(changeset) do line in Rumbl.UserController.create(), the IO.inspect() on the preceding line returns a changeset where all four of the changes (name, username, password, password_hash) are populated correctly. When I look at the recently added entry in Ecto, still both the password and password_hash are set to nil.

This sounds incorrect behaviour to me, can you please publish the full project in its current state to github or some place like that ans share the link?

What do You have at the console?

As You are using Phoenix 1.2.x, You do not have contexts.

I tried typing this in mine… (Phoenix 1.4, Ecto 3.0, and the name of my project is CmsEx)

iex(1)> alias CmsEx.{Accounts, Repo}                                                                                                                  
[CmsEx.Accounts, CmsEx.Repo]
iex(2)> changeset = Accounts.User.registration_changeset(%Accounts.User{}, %{"name" => "koko", "email" => "koko@gmail.com", "password" => "MyPassword"})
#Ecto.Changeset<
  action: nil,
  changes: %{
    email: "koko@gmail.com",
    name: "koko",
    password: "MyPassword",
    password_hash: "$2b$12$..NtUYRb7tgFOpS1OhrWp.4JnN69NbVfZMzmVKQhljl0oCOK6hlIq"
  },
  errors: [],
  data: #CmsEx.Accounts.User<>,
  valid?: true
>
iex(3)> changeset |> Repo.insert                                                                                                                        
[debug] QUERY OK db=4.2ms
INSERT INTO "users" ("email","name","password_hash","inserted_at","updated_at") VALUES ($1,$2,$3,$4,$5) RETURNING "id" ["koko@gmail.com", "koko", "$2b$12$..NtUYRb7tgFOpS1OhrWp.4JnN69NbVfZMzmVKQhljl0oCOK6hlIq", {{2018, 12, 9}, {23, 14, 52, 59770}}, {{2018, 12, 9}, {23, 14, 52, 62294}}]
{:ok,
 %CmsEx.Accounts.User{
   __meta__: #Ecto.Schema.Metadata<:loaded, "users">,
   current_sign_in_at: nil,
   current_sign_in_ip: nil,
   email: "koko@gmail.com",
   id: 2,
   inserted_at: ~N[2018-12-09 23:14:52.059770],
   last_sign_in_at: nil,
   last_sign_in_ip: nil,
   name: "koko",
   password: "MyPassword",
   password_hash: "$2b$12$..NtUYRb7tgFOpS1OhrWp.4JnN69NbVfZMzmVKQhljl0oCOK6hlIq",
   sign_in_count: nil,
   updated_at: ~N[2018-12-09 23:14:52.062294]
 }}

By the way, is there a reason why You are using a release candidate for Ecto? and an old Phoenix release?

Ecto is now 3.0.4, and Phoenix is 1.4.x

He doesn’t mention hus ecto version, and he is probably using the phoenix version that matches the book he is reading.

I assume the Ecto version from cross posting :slight_smile:

In the other OP, about telemetry, from the same poster

{:ecto_sql, "~> 3.0-rc.1"},

Yeah I see that now, and I’d suggest to use ecto 2.x it my assumption about the phoenix version used is correct. I do not think though that this is related to the problem.

You spelt password_hash incorrectly in the put_pass_hash function. It should work once you fix that :+1:.

5 Likes

Oh, nice spotting this :slight_smile:

I just gave myself the biggest facepalm. Thanks for pointing that out, alexgaribay, everything now works! Thanks for the help, NobbZ and kokolegorille, onward I go on my Phoenix journey.

2 Likes

Are you aware that you can buy Programming Phoenix >= 1.4 right now as an ebook? I got tired of waiting while watching the publication date get continually pushed into the future, and then I realized that I could just buy the ebook now rather than attempt to learn Phoenix with the old print book. I prefer print books, but the pdf file for the ebook allows you to paste notes onto the text, so I find that I can make notations just like with a print book, and it’s working out pretty well. I’m working on the same Rumbl example that you are. However, I made so many typos typing Rumble instead of Rumbl, that I changed the name of my project to Rum.:+1:

2 Likes