Recommend storing current_user in assigns

[danielberkompas]

Mithril's authentication system (powered by Authority) is a token-based system. You exchange a username and password for a token.

MyApp.Accounts.tokenize({"my@email.com", "password"})
# => %MyApp.Accounts.Token{token: "d88fb81c-1843-4696-932c-a5b5d779d854"}

You can then later use that token to verify and fetch a user's identity.

MyApp.Accounts.authenticate(token)
# => {:ok, %MyApp.Accounts.User{...}}

Current Recommendations

Mithril recommends that you store only this token in your Phoenix session. It specifically forbids you to store user IDs in the session. This allows MyApp to revoke tokens and kill Phoenix sessions without relying on Phoenix.

In addition, Mithril currently recommends that you use the token instead of a user_id for function calls. This means that every function that needs to know which user is logged in would accept a token argument.

MyApp.CMS.create_page(token, params)

Internally, the function would have to exchange this token for a user_id, either by calling authenticate directly or using some kind of protocol.

# Using authenticate directly
def create_page(token, params) do
  with {:ok, user} <- Accounts.authenticate(token) do
    # ...
  end
end

# Using an Identity protocol
def create_page(identity, params) do
  with {:ok, user_id} <- Identity.user_id(identity) do
    # ...
  end
end

This adds a lot of complexity to functions, and it can be inefficient. Each function call revalidates and refetches the current user.

Proposed Recommendations

1. Continue forbidding storing current_user or current_user_id in the session
2. Encourage storing current_user or current_user_id in the assigns

This would have several benefits:

• The logic app can still cancel sessions, because only tokens are stored in the Phoenix session, not the current user
• Efficiency: domain functions don't have to accept tokens or refetch user ids. MyApp.CMS.create_page(conn.assigns.current_user.id, params)
• Simplicity: it's more the way people expect to do things, and there's no real harm in it.
• We could integrate rzane/authenticator, perhaps making it into authority_plug.

[zberkom]

I think this new approach makes total sense. The current convention could mean loading the current user from the DB an unknown number of times. It also disrupts pipeline flows.

Two 👍 👍 from me.

[rzane]

I'd be happy to rename authenticator 👍

[silasjmatson]

On the mithril-based project I've been working on, I've started following the second approach, after a fashion.

I think to ensure that the user is always loaded using the proper method (w/authentication), we could add a function to the phoenix side (e.g. authenticated_user/1) that pulls the user from the assigns.

def authenticated_user(conn) do
  conn.assigns.current_user
end

We could also add a requirement that any user passed to a domain function has a virtual attribute authenticated with a value of true, which can be set whenever a user is authenticated. So Repo.get(User, id) will not return an 'authenticated' user, and a user loaded in that method cannot be passed to a domain function requiring an authed user.

defmodule MyApp.Conversations do
  def archive_conversation(%{authenticated: true} = user, conversation_id) do
    # archive conversation
  end

  def archive_conversation(_user, _id), {:error, :unauthorized}
end

@danielberkompas Thoughts?

I've run into the pain points of passing tokens everywhere, and I like the second approach. There are really two concerns at play regarding authenticating an action:

1. Ensuring that the user is authenticated
2. Ensuring that the user has permissions to perform the action

I think it makes sense to have domain functions only concern themselves with the second, as the first is handled by authority. An 'archive conversations' function should not need to know how to authenticate a user, for example, only whether any given user can archive a given conversation.

[danielberkompas]

@silasjmatson good thoughts! I agree that domain functions should only concern themselves with permissions, not authentication.

For that reason, it probably isn't necessary to have a virtual authenticated attribute. As long as Phoenix isn't storing the user_id in the session, the domain function can be confident that the user it receives is being authenticated, because the only way Phoenix will have a user ID is if it authenticated the token.

I'm thinking we'll probably use rzane/authenticator once it's renamed to authority_plug for basic plugs that ensure that current_user is present in the assigns.

[rzane]

Seeking advice. Currently, Authenticator is a generic solution for this sort of Plug level authentication. It doesn't really care if you're using Authority or Guardian. As such, it feels weird to rename it to "Authority.Plug". I'm still not entirely opposed to the idea though, because I really like the idea of forming a community around authority. 🤷‍♂️

However, I added Authenticator.Authority, which provides a macro that should handle the 90% case of using Authenticator in combination with Authority:

• https://github.com/rzane/authenticator#usage-with-authority
• https://github.com/rzane/authenticator/blob/3abe1b24140941bf01500c0a3e9f8d0d4d7f3e12/lib/authenticator/authority.ex

[rzane]

On second thought, I'm back to thinking renaming is a fine idea. Authenticator takes kind of a similar approach to Authority, so I think this seems fitting. And, I think I can rename Authenticator.Authority to Authority.Plug.Template.