HiveBrain v1.2.0
Get Started
← Back to all entries
debugMinor

Elixir / Phoenix login controller, allowing multiple attempts

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
allowinglogincontrollerphoenixelixirmultipleattempts

Problem

I have a login method in my controller. I would like to get rid of the imperative "thinking" and write something more functional.

def login(conn, %{"login" => %{"password" => password}}) do

    conn = conn |> fetch_session

    if count_attempt_login(conn)  attempt_login(password)
    else 
      conn 
        |> redirect(to: "/")
    end

  end

  defp count_attempt_login(conn) do
    (conn |> get_session(:count_attempt_login)) || 1
  end

  defp attempt_login(conn, password) when password == @password_login do
      conn
        |> put_session(:is_admin, true)
        |> delete_session(:count_attempt_login)
        |> redirect(to: "/dashboard")
  end

  defp attempt_login(conn, _) do
    conn
      |> put_session(:count_attempt_login, count_attempt_login(conn) + 1)
      |> put_flash(:error, "Impossible to connect with this password")
      |> redirect(to: "/dashboard/auth")
  end

Solution

Personally I would do the inc and count of the attempts in one place,
You can still split it, but you actually need this every time an attempt is going to happen.

You could also replace the if with a case. You don't have to, but I think it looks nicer :)

The next thing is, remove the when password == @password_login and just use @password_login in the function definition

def login(conn, %{"login" => %{"password" => password}}) do
    case inc_and_get_login_attempts(conn)   conn |> try_to_login(password)
      _ ->     conn |> redirect(to: "/")
    end
  end

  defp inc_and_get_login_attempts(conn) do
    attempts = (conn |> get_session(:count_attempt_login)) || 0
    put_session(conn, :count_attempt_login, attempts + 1)
    attempts + 1
  end

  defp try_to_login(conn, @password_login) do
    conn
      |> put_session(:is_admin, true)
      |> delete_session(:count_attempt_login)
      |> redirect(to: "/dashboard")
  end

  defp try_to_login(conn, _) do
    conn
      |> put_flash(:error, "Impossible to connect with this password")
      |> redirect(to: "/dashboard/auth")
  end

Code Snippets

def login(conn, %{"login" => %{"password" => password}}) do
    case inc_and_get_login_attempts(conn) <= 10 do
      true ->  conn |> try_to_login(password)
      _ ->     conn |> redirect(to: "/")
    end
  end

  defp inc_and_get_login_attempts(conn) do
    attempts = (conn |> get_session(:count_attempt_login)) || 0
    put_session(conn, :count_attempt_login, attempts + 1)
    attempts + 1
  end

  defp try_to_login(conn, @password_login) do
    conn
      |> put_session(:is_admin, true)
      |> delete_session(:count_attempt_login)
      |> redirect(to: "/dashboard")
  end

  defp try_to_login(conn, _) do
    conn
      |> put_flash(:error, "Impossible to connect with this password")
      |> redirect(to: "/dashboard/auth")
  end

Context

StackExchange Code Review Q#134405, answer score: 2

Revisions (0)

No revisions yet.