How do you handle password hashing?

[aaronrenner]

I have a User model that hashes the user's password when you call User.changeset/2(gist here). How do I create a factory that correctly hashes the password when I do the following?

    MyApp.Factory.build(:user, password: "supersecret")

I'm new to elixir, so if there is a better approach for automatically hashing the password, please let me know.

[paulcsmith]

@aaronrenner That is a great question. There is a proposed PR for using changesets when creating factories (#78), but I'm thinking that may add more complexity than is necessary. Here's what I've done:

def factory(:user) do
  %User{
    email: Sequence.next(:email, &"email-#{&1}@example.com"),
    encrypted_password: "something",
  }
end

def set_password(user, password) do
  hashed_password = Comeonin.Bcrypt.hashpwsalt(password)
  %{user | encrypted_password: hashed_password}
end

# In a test

test "signs in the user if information  is correct", %{conn: conn} do
  user = build(:user, email: "foo@example.com") |> set_password("pass") |> create

  conn = post conn, session_path(conn, :create), user: %{email: "foo@example.com", password: "pass"}

  assert signed_in?(conn, user) # Test that the conn returns the right status code, sets assigns or whatever
end

So I only use the set_password function when I really need to set the password. Generally this is only for a couple tests where I test the actual sign in implementation. The rest would set the user on the conn and bypass the sign in controller.

So for this to work it assumes:

1. Your password is only used for test authentication. If that's the case (which it usually is), then it doesn't really matter what the hashed password is for the tests.
2. Your auth plug does not check the password if the current user is already assigned. If you are checking the username and password for every single test then this method won't work quite as well.

For example if you do something like this:

post(sign_in_path(conn, :create), username: "me", pass: "pass")
|> get(thing_path(conn, :index))

Then it won't work since you're actually hitting the real sign in controller for all your tests. Instead you could do something more like this:

  test "list things" do
    user = create(:user)
    # You'd probably extract this to a `sign_in` function or do it in the `setup` block
    conn = conn() |> Plug.Conn.assign(:user, user) 
    things = create_pair(:things)

    # Works because the conn already has a user
    conn = get conn, experiment_path(conn, :index)

    for thing <- things do
      assert html_response(conn, 200) =~ thing.title
    end
  end

# Auth plug

# If the conn already has a user, assume they're signed in
def authenticate(conn, _opts) do
  if already_signed_in?(conn) do
    conn
  else
    get_and_assign_user
  end
end

defp already_signed_in?(conn) do
  !!conn.assigns.user
end

I recommend this anyway because it makes tests simpler and faster since you don't have to hit the sign in controller every time.

LMK if that answers your question. It may have been too much, but if someone else finds this on google I'd rather have a more complete answer that they can use :)

[aaronrenner]

Thanks @paulcsmith for a great response.

The only potential drawback I see with adding set_password/2 to your factory is now the logic to hash a password is stored in 2 places. In the application, the password hashing logic is encapsulated in the changeset and the details of how the password is hashed can be hidden in a private method. If I add a set_password/2 method to my Factory, I have to duplicate the knowledge of how the password is hashed.

I really appreciate seeing how you handle this, and I don't have a better solution at the moment. I'll keep pondering it and let you know if I come up with an alternative solution.

[paulcsmith]

I'm glad it helped. That is a really good point! One thing you could do is make the password hashing function public in your model and call that from within the factory. Just delegate it. Haven't tried it myself, but it might work well.

Let me know if you figure something else out :)

On Jan 8, 2016, at 11:26 PM, Aaron Renner notifications@github.com wrote:

Thanks @paulcsmith for a great response.

The only potential drawback I see with adding set_password/2 to your factory is now the logic to hash a password is stored in 2 places. In the application, the password hashing logic is encapsulated in the changeset and the details of how the password is hashed can be hidden in a private method. If I add a set_password/2 method to my Factory, I have to duplicate the knowledge of how the password is hashed.

I really appreciate seeing how you handle this, and I don't have a better solution at the moment. I'll keep pondering it and let you know if I come up with an alternative solution.

—
Reply to this email directly or view it on GitHub.

[aaronrenner]

@paulcsmith After thinking about this for a while, here's what I came up with...

My User.changeset/2 function already contained the logic for setting the encrypted password behind the scenes, and I didn't really want to expose an additional public function just to set the encrypted password in tests. Instead, here's my implementation of Factory.set_password/2 that leverages the existing password hasing behavior of User.changeset/2.

def set_password(user, password) do
  user
  |> User.changeset(%{"password" =>password})
  |> Ecto.Changeset.apply_changes()
end

[paulcsmith]

@aaronrenner Ah that's interesting! Thanks for posting for others to check it out as well. I'll give this a try when I need it again :)

[dustinfarris]

@aaronrenner thanks for that tip. I had to change apply_changes to update:

def set_password(user, password) do
  user
  |> User.changeset(%{"password" =>password})
  |> Repo.update!
end

[aaronrenner]

@dustinfarris I don't think you need to use Repo.update!, if you start your pipeline with build(:user) instead of insert(:user).

build(:user) |> set_password("secret") |> insert

[dustinfarris]

Ah, ok. That works better. Thanks!

On Jul 19, 2016, at 3:04 AM, Aaron Renner notifications@github.com wrote:

@dustinfarris https://github.com/dustinfarris I don't think you need to use Repo.update!, if you start your pipeline with build(:user) instead of insert(:user).

build(:user) |> set_password("secret") |> insert
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #82 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/ABCWvef9Y1Qswq1uQ5Gk1MH2D6pfUNSIks5qW84tgaJpZM4HA8g0.

Dustin Farris

[seanculver]

For anyone else who uses this thread to get password encryption working. Here's what I've done.

if using coherance 0.3.0, ecto 2.0.5, phoenix 1.2.1. I had to do the following.

In factory

def set_password(user, password) do
    user
      |> User.changeset(%{password: password})
      |> Ecto.Changeset.apply_changes()
  end

Note: Repo.update! did not seem to work for me.

In test file

build(:user) |> set_password("secret") |> insert

Which is really combination from comments above. Thanks @aaronrenner @dustinfarris

[stratigos]

Given that I find it hard to come by documentation on items such as this, Id like to add an update that may help contemporary Phoenix devs (ca. 2024 / phx 1.7 / ecto_sql 3.10 / ex_machina ~2.7). Ill also mention that I am coming back to Phoenix after not touching Elixir for ~5 years, and likely have a fair amount of naivety.

Using anything that returns a changeset produced an error for me:

(ArgumentError) #Ecto.Changeset<action: nil, changes: %{hashed_password: "**redacted**"}, errors: [], data: #MyAppElixir.Accounts.Admin<>, valid?: true> is not an Ecto model. Use `build` instead

I was able to accomplish this goal - avoiding duplication of my app's pw hashing code - by extracting the changeset changes and then rebuilding a map of my data model's fields and values:

# test/support/factories/admin_factories.ex

defmodule MyAppElixir.Factories.AdminFactory do
  use ExMachina.Ecto, repo: MyAppElixir.Repo
  
  def admin_factory do
    %Admin{
      name: sequence(:name, &"Testy McTesterson #{&1}"),
      email: sequence(:email, &"email-#{&1}@example.com"),
      hashed_password: "_"
    }
  end

  def set_password(admin, password) do
    admin_changeset_with_pw_hash =
      admin
      |> Admin.registration_changeset(%{"password" => password})

    %{hashed_password: hashed_pw} = admin_changeset_with_pw_hash.changes

    %{admin | hashed_password: hashed_pw}
  end
end

And I call this in tests as:

    build(:admin, %{email: "adminbot@example.com"})
    |> set_password("1ValidPassword!")
    |> insert()