aaronrenner / phx_gen_auth

An authentication system generator for Phoenix 1.5 applications.
772 stars 55 forks source link

Don't redirect in UserAuth.log_in_user #99

Closed samu closed 3 years ago

samu commented 3 years ago

The standard UserSessionController.create looks like this:

  def create(conn, %{"user" => user_params}) do
    %{"email" => email, "password" => password} = user_params

    if user = Accounts.get_user_by_email_and_password(email, password) do
      UserAuth.log_in_user(conn, user, user_params)
    else
      render(conn, "new.html", error_message: "Invalid email or password")
    end
  end

I wanted to create a log-in endpoint which can be used to log in via an Ajax request. The main difference would be that it doesn't redirect and instead returns a 200 status code and a JSON compatible payload:

  def create(conn, params = %{"email" => email, "password" => password}) do
    user = Accounts.get_user_by_email_and_password(email, password)

    if user do
      UserAuth.log_in_user(conn, user, params)
      |> put_status(:ok)
      |> json(%{id: user.id})
    else
      conn
      |> put_status(:unauthorized)
      |> text("")
    end
  end

The problem is, that UserAuth.log_in_user does a redirect right at the end of it's processing:

  def log_in_user(conn, user, params \\ %{}) do
    token = Accounts.generate_user_session_token(user)
    user_return_to = get_session(conn, :user_return_to)

    conn
    |> renew_session()
    |> put_session(:user_token, token)
    |> put_session(:live_socket_id, "users_sessions:#{Base.url_encode64(token)}")
    |> maybe_write_remember_me_cookie(token, params)
    |> redirect(to: user_return_to || signed_in_path(conn))
  end

Now i'm wondering if this redirect really belongs into the UserAuth module. One could argue that the decision to redirect should go into the respective controller.

I of course understand that i could simply tweak the generated code, but i think it would be optimal if i had to do as little changes to the generated code as possible.

josevalim commented 3 years ago

This is tricky because for some APIs, you won’t even use the session but just return the token, so you would reuse very little of this function.

So generally speaking we prefer to keep the code simple rather than prepare it for all possible scenarios, because there are just too many to predict. Thanks!