Elixir Phoenix 1.6 - trying to pass token to browser, function nil.id/0 is undefined or private

Question

I am working on a Phoenix 1.6 app. I used Ueberauth for authenication via GitHub, which was working. I set up a channel to post topics and comments, which authenticated users can do. Next, I tried to add a user token for use in the channel. I was following the instructions in the boilerplate code created using mix phx.new.socket User. In the user_socket.js file, there are instructions for creating the token in a template, which worked. The token is verified in user_socket.ex in the connect function. I created a new plug put_user_token in router.ex to add the token to the conn, which also worked. However, I had a problem with the logic in the plug. This is my code:

  defp put_user_token(conn, _) do
    if conn.assigns.user do
      token = Phoenix.Token.sign(conn, "user socket", conn.assigns.user.id)
      assign(conn, :user_token, token)
    else
      conn
    end

This works as long as I am logged in. The problem is when I log out and try to log back in. The boilerplate code for the if statement in the plug is:

if current_user = conn.assigns[:current_user] do

My code differs because the current user is defined as user, with the value equal to the id of the user record in the database. I think the problem is that when setting up Ueberauth, I created another plug SetUser that comes before the put_user_token plug. This is the call function for SetUser:

 def call(conn, _opts) do
        user_id = get_session(conn, :user_id)

        cond do
            user = user_id && Repo.get(User, user_id) ->
                assign(conn, :user, user)
            true ->
                assign(conn, :user, nil)
        end
    end

What seems to be happening is that, after I log out, the app redirects to the home page, which causes the plugs to execute, and the value of conn.assigns.user is set to nil. Then the error occurs and I cant' log back in.

I need to come up with a way to have the if statement in the put_user_token plug be able to handle a nil value. I tried is_integer(conn.assigns.user), and a couple of other comparisons, but if a value of nil is present, the app crashes.

Answer 1

I think this question would be easier to answer if you could simplify it -- I suspect once you simplified the problem it would be clear to you where wires were getting crossed. However, let me attempt a couple clarifications.

First, if statements are somewhat unidiomatic in Elixir -- you'll find that most often your execution flows can be defined without them and code is are usually easier to read when it doesn't rely on if statements.

Related, be very careful when using if to check the "truthiness" of a value. This isn't just an Elixir thing, the behavior is a potential gotcha in any language. I seem to remember PHP, for example, which evaluated an empty object as false in one version but true in another (!!). In Elixir, 0 or an empty object both are "truthy", but not true... so all that to say that it pays to be more explicit.

For example, consider refactoring this code:

defp put_user_token(conn, _) do
  if conn.assigns.user do
    token = Phoenix.Token.sign(conn, "user socket", conn.assigns.user.id)
    assign(conn, :user_token, token)
  else
    conn
  end
end

To something more explicit, perhaps:

defp put_user_token(conn, _) do
  case conn.assigns.user do
    nil -> conn
    user -> 
      token = Phoenix.Token.sign(conn, "user socket", conn.assigns.user.id)
      assign(conn, :user_token, token)
  end
end

or consider pushing the pattern-matching all the way into the function signature, something like this (I'm not sure of the exact shape of the conn, but hopefully you get the idea):

defp put_user_token(%{assigns: %{user: nil}} = conn, _), do: conn
defp put_user_token(%{assigns: %{user: user}} = conn, _) do 
  token = Phoenix.Token.sign(conn, "user socket", conn.assigns.user.id)
  assign(conn, :user_token, token)
end

You may find it more straight-forward to assign a simple boolean value like :is_logged_in? as the pivot point in your code because doing a bunch of checking for a value that MAYBE is a map/struct or MAYBE is nil can be confusing and is harder to read.

Lastly, double-check the other side of this code where you fetch the user data:

def call(conn, _opts) do
  case get_session(conn, :user_id) do
    nil -> conn
    user_id -> user = Repo.get(User, user_id)
      assign(conn, :user, user)
  end
end

or to be a bit strict and handle the possibility that the user ID in the session doesn't exist in the database, you could refactor this as a with statement something like:

def call(conn, _opts) do
  with user_id when !is_nil(user_id) <- get_session(conn, :user_id) 
   user when !is_nil(user) <- Repo.get(User, user_id)
      assign(conn, :user, user)
  else
    _ -> conn
  end
end

I think it would be easier to read if you moved the component steps into their own named private-functions which returned something more explicit than nil.

I would also take a moment to re-evaluate the flow here -- the app will not perform well if you need to hit the database for every request. You should probably write the required user-data to the session only after successful login.

All code samples are untested.