aaronrenner / phx_gen_auth

An authentication system generator for Phoenix 1.5 applications.
774 stars 56 forks source link

Rationale for not hashing all stored tokens #65

Closed liamwhite closed 4 years ago

liamwhite commented 4 years ago

The generator creates two functions along the lines of the following (copying from my app's source code):

  @doc """
  Generates a token that will be stored in a signed place,
  such as session or cookie. As they are signed, those
  tokens do not need to be hashed.
  """
  def build_session_token(user) do
    token = :crypto.strong_rand_bytes(@rand_size)
    {token, %Philomena.Users.UserToken{token: token, context: "session", user_id: user.id}}
  end

  @doc """
  Builds a token with a hashed counter part.

  The non-hashed token is sent to the user email while the
  hashed part is stored in the database, to avoid reconstruction.
  The token is valid for a week as long as users don't change
  their email.
  """
  def build_email_token(user, context) do
    build_hashed_token(user, context, user.email)
  end

  defp build_hashed_token(user, context, sent_to) do
    token = :crypto.strong_rand_bytes(@rand_size)
    hashed_token = :crypto.hash(@hash_algorithm, token)

    {Base.url_encode64(token, padding: false),
     %Philomena.Users.UserToken{
       token: hashed_token,
       context: context,
       sent_to: sent_to,
       user_id: user.id
     }}
  end

I am curious as to why the decision was made not to simply hash the token stored in the database in all cases. There does not appear to be any concrete downside to doing so, as the tokens never need to be reconstructed by the application.

josevalim commented 4 years ago

You are welcome to make the change yourself. But since the session itself is signed (and often also persisted in the database), it is not really necessary to hash that.