In Log module now quoting values that contain spaces or = sign

lib/openmaize/confirm/base.ex

@@ -39,18 +39,23 @@ defmodule Openmaize.Confirm.Base do
   alias Openmaize.Database, as: DB
   alias Openmaize.Log
 
+  @doc """
+  Function to confirm email by checking the token.
+
+  This function is used by the Openmaize.ConfirmEmail and
+  Openmaize.ResetPassword Plugs.
+  """
   def check_confirm(conn, {uniq, user_id, key, password},
     {repo, user_model, {key_expiry, mail_func}}) when byte_size(key) == 32 do
     repo.get_by(user_model, [{uniq, user_id}])
     |> check_key(repo, key, key_expiry * 60, password)
     |> finalize(conn, user_id, mail_func, password)
   end
   def check_confirm(conn, _, _) do
-    log_entry = %Log{
-      message: "invalid query string.",
-      meta: [{"query", conn.query_string}]}
-
-    conn |> Log.logfmt(log_entry) |> Logger.warn
+    Log.logfmt(conn, %Log{
+                 message: "invalid query string",
+                 meta: [{"query", conn.query_string}]})
+    |> Logger.warn
     put_private(conn, :openmaize_error, "Invalid credentials")
   end
 
@@ -69,20 +74,17 @@ defmodule Openmaize.Confirm.Base do
 
   defp finalize({:ok, user}, conn, user_id, mail_func, password) do
     message = if password == :nopass, do: "account confirmed", else: "password reset"
-    log_entry = %Log{user: user_id, message: message}
-    conn |> Log.logfmt(log_entry) |> Logger.info
+    Log.logfmt(conn, %Log{user: user_id, message: message}) |> Logger.info
 
     mail_func.(user.email)
     put_private(conn, :openmaize_info, String.capitalize(message))
   end
   defp finalize({:error, message}, conn, user_id, _, _) do
-    current_user_id = conn |> Log.current_user_id
-    log_entry = %Log{
-      user: user_id,
-      message: message,
-      meta: [{"current_user_id", current_user_id}]}
-
-    conn |> Log.logfmt(log_entry) |> Logger.warn
+    Log.logfmt(conn, %Log{
+                 user: user_id,
+                 message: message,
+                 meta: [{"current_user_id", Log.current_user_id(conn)}]})
+    |> Logger.warn
     put_private(conn, :openmaize_error, "Invalid credentials")
   end
 end

lib/openmaize/log.ex

@@ -30,9 +30,20 @@ defmodule Openmaize.Log do
   def logfmt(%Plug.Conn{request_path: request_path},
     %Openmaize.Log{user: user, message: message, meta: meta}) do
     log = [{"path", request_path}, {"user", user}, {"message", message}] ++ meta
-    Enum.map_join(log, " ", fn {k, v} -> "#{k}=#{v}" end)
+    Enum.map_join(log, " ", &format/1)
   end
 
+  @doc """
+  Returns the id of the currently logged-in user, if present.
+  """
   def current_user_id(%Plug.Conn{assigns: %{current_user: %{id: id}}}), do: "#{id}"
   def current_user_id(_), do: "nil"
+
+  defp format({key, val}) do
+    if String.contains?(val, [" ", "="]) do
+      ~s(#{key}="#{val}")
+    else
+      ~s(#{key}=#{val})
+    end
+  end
 end

lib/openmaize/login.ex

@@ -100,19 +100,11 @@ defmodule Openmaize.Login do
     put_private(conn, :openmaize_user, Map.drop(user, Config.drop_user_keys))
   end
   defp handle_auth({:error, "acc" <> _ = message}, conn, user_id) do
-    log_entry = %Log{
-      user: user_id,
-      message: message}
-
-    conn |> Log.logfmt(log_entry) |> Logger.warn
+    Log.logfmt(conn, %Log{user: user_id, message: message}) |> Logger.warn
     put_private(conn, :openmaize_error, "You have to confirm your account")
   end
   defp handle_auth({:error, message}, conn, user_id) do
-    log_entry = %Log{
-      user: user_id,
-      message: message}
-
-    conn |> Log.logfmt(log_entry) |> Logger.warn
+    Log.logfmt(conn, %Log{user: user_id, message: message}) |> Logger.warn
     put_private(conn, :openmaize_error, "Invalid credentials")
   end
 end

lib/openmaize/onetime_pass.ex

@@ -94,9 +94,7 @@ defmodule Openmaize.OnetimePass do
   end
 
   defp handle_auth({:error, message}, conn) do
-    log_entry = %Log{message: message}
-
-    conn |> Log.logfmt(log_entry) |> Logger.warn
+    Log.logfmt(conn, %Log{message: message}) |> Logger.warn
     put_private(conn, :openmaize_error, "Invalid credentials")
   end
   defp handle_auth(user, conn) do

test/log_test.exs

@@ -12,22 +12,32 @@ defmodule Openmaize.LogTest do
   test "logs to console in standard logfmt" do
     assert capture_log(fn ->
       conn = conn(:get, "/confirm") |> assign(:current_user, @user)
-      current_user_id = conn |> Log.current_user_id
-      log_entry = %Log{
-        user: "[email protected]",
-        message: "account confirmed",
-        meta: [{"current_user_id", current_user_id}]}
-      conn |> Log.logfmt(log_entry) |> Logger.warn
-    end) =~ "path=/confirm [email protected] message=account confirmed current_user_id=1"
+      Log.logfmt(conn, %Log{
+                   user: "[email protected]",
+                   message: "account confirmed",
+                   meta: [{"current_user_id", Log.current_user_id(conn)}]})
+      |> Logger.warn
+    end) =~ ~s(path=/confirm [email protected] message="account confirmed" current_user_id=1)
   end
 
   test "logs to console in standard logfmt for nil current_user" do
     assert capture_log(fn ->
       conn = conn(:get, "/login") |> assign(:current_user, nil)
-      log_entry = %Log{
-        user: "[email protected]",
-        message: "failed login"}
-      conn |> Log.logfmt(log_entry) |> Logger.warn
-    end) =~ "path=/login [email protected] message=failed login"
+      Log.logfmt(conn, %Log{
+                   user: "[email protected]",
+                   message: "failed login"})
+      |> Logger.warn
+    end) =~ ~s(path=/login [email protected] message="failed login")
   end
+
+  test "quotes values containing '='" do
+    assert capture_log(fn ->
+      conn = conn(:get, "/confirm")
+      Log.logfmt(conn, %Log{
+                   message: "invalid query string",
+                   meta: [{"query", "email=wrong%40mail.com"}]})
+      |> Logger.warn
+    end) =~ ~s(path=/confirm user=nil message="invalid query string" query="email=wrong%40mail.com")
+  end
+
 end