Hi @jeffdeville – you’ve raised a good question. I went back-and-forth internally for a while about this. I think we can at least agree that, whether called internally or externally, the authorize/3 callback should exist directly on the context module itself, since it’s a context-specific API that determines what user can do.

Off the top of my head, here are some arguments for performing authorization in a controller action:

• Reinforces the concept of controllers being the interface INTO your app – the first line of defense
• Easier to call context methods from a privileged position (e.g. scheduled task, testing, etc) where we don’t care about authorization
• Don’t need to pass the user (e.g. conn.assigns[:current_user]) into every context function if not needed (although lots of the time, we end up doing that anyway)
• Easier to compose multiple context actions together
• Leaner context functions that just “do the thing”

… and some arguments for performing authorization in a context function:

• Strictly enforces authorization rules at application level – can’t skip it, no matter what. Arguably this is better design, and forces you to think about non-user-account cases like “guest” or “background task” users
• Leaner controller actions
• More flexibility to perform complex authorization rules, or change authorization rules without having to track down every context caller

So while I did pick controller-level authorization for the code examples, I don’t think it’s the One True Way, and the overall design is certainly up to you.

I think there is room in Bodyguard for a design element that gives you the best of both worlds – a way to perform authorization from within a context function (better design) but doesn’t require repetitive auth checks and passing the user model around everywhere (more convenient). I haven’t thought it through so I’m open to suggestions.

Since this is a frequent and important question that has come up a lot recently, I’ve written a blog post around it outlining my thoughts on where to handle authorization under different use cases:

I do something pretty similar for authorization. I have very fine grained permissions (requested of rme) and I have a singular Permission module that just has a few helpers on it like is_logged_in/1 and can/2 and so forth.

Just about every single one of my ‘contexts’ (though they predate that phoenix concept by a great deal) take an env as their first argument and return an env as their return type (either alone if nothing else is needed or as a tuple with other values, or an %Exception{} structure is returned, yes returned not thrown). An example is, let me just grab a random one since I have a file open:

  def get_requirements(env) do
    query =
      from r in DB.SomeModule.Requirement,
      where: is_nil(r.removed_at)

    reqs =
      query
      ~> Repo.all()
      ~> Enum.filter(&can?(env, %Permissions.SomeModule.Requirement{action: :index, id: &1.id}))

    {env, reqs}
  end

So a permission itself is just a struct that fulfills a behaviour for some helper functions on it (introspection of them, like valid values and so forth for given keys).
I can test a permission with either can(env, somePermission) -> env | %Exception{} or can(env, somePermission) -> true | false, in the above case I’m using the can? variant (actually not commonly used overall, the can is the most often used) to test each of these records to test if the current user has access for each record. The above function is used as such from a controller like:

  def index(conn, _params) do
    conn
    ~> can(%Permissions.SomeModule.Requirement{action: :index})
    ~> SomeModule.get_requirements()
    ~> case do {conn, reqs} ->
      render(conn, :index, requirements: reqs)
    end
  end

As you can see a conn is an ‘environment’ (as are sockets and a few other things depending on access, I only have to implement a couple things to support something else as an ‘environment’) and as such it gets threaded through all the calls.

Another pattern I often use would be like using it as such:

  def index(conn, _params) do
    conn
    ~> can(%Permissions.SomeModule.Requirement{action: :index})
    ~> SomeModule.get_requirements()
    ~> pipe2(do_something(42))
    ~> ...
  end

Where pipe2 just takes the tuple that is passed as the value and folds it into the argument positions, so the above is the same thing as doing (it literally compiles into this):

  def index(conn, _params) do
    conn
    ~> can(%Permissions.SomeModule.Requirement{action: :index})
    ~> SomeModule.get_requirements()
    ~> case do {v0, v1} -> do_something(v0, v1, 42) end
    ~> ...
  end

And since I try to follow the same ‘pattern’ of things around then it fits in quite well so everything just becomes easily pipeable. Most of the code everywhere exists within long streams of ~>'s (a few |>'s still around too). It is very pleasant to keep up and maintain.

Only thing I wish for is a static type checker to better decorate things. ^.^

I keep trying to use with, but I really really hate this format that most seem to use:

with blah <- do_something1(),
     bloop when is_atom(bloop) <- do_something2(),
     bleep <- do_something3(bloop, blah),
     do: {:ok, bleep},
     else: err -> handle_error(err)

Or:

with blah <- do_something1(),
     bloop when is_atom(bloop) <- do_something2(),
     bleep <- do_something3(bloop, blah) do
  {:ok, bleep}
else
  err -> handle_error(err)
end

Or:

with\
  blah <- do_something1(),
  bloop when is_atom(bloop) <- do_something2(),
  bleep <- do_something3(bloop, blah),
  do: {:ok, bleep},
  else: err -> handle_error(err)

Or other variants, they all just itch me very very wrong. The last one is the one I use when I am forced to use with due to too lazy to dep-in something better as the everything having comma’s at least makes copy/pasting and moving lines around easier, but holy heck it looks horrible. I’d still prefer:

with do
  blah <- do_something1()
  bloop when is_atom(bloop) <- do_something2()
  bleep <- do_something3(bloop, blah)
  {:ok, bleep} # Oh looky, last expression is the returned one, like everywhere else in elixir-land...
else
  err -> handle_error(err)
end

That style just makes so much more sense to me than arbitrary and undefined amount of argument passing in that you have to split among multiple lines to get it even remotely readable… And yes, packages exist that do precisely this, which just makes the built-in with soooo weird, this weird oddity or wart on an otherwise more sensical syntax (excepting for, but I have my own notes on how it should have been done too, they are both weird).