Authorizing context actions where? In each function, or the controller?

I perform validation right on the line before accessing a DB call or refining data based on some permission. I have a lot of lines like this scattered about (copy/pasted):

    conn =
      conn
      |> Perms.verify(%Perms.SomeSection.Report{action: :view, report: report})
      |> ensure!()
      # Strip other things working on `conn`

Interesting.

I prefer not to have to see the same code repeated everywhere, particularly when it’s orthogonal to what the controller is really trying to do.

In a pre 1.3 world, you could probably do pretty well with a solution like this:

plug :load_user
plug :load_resource
plug :authorize

That would assume a fairly standard, RESTful layout, but for most web apps, it wouldn’t be far off. However, in a context world I’m pretty sure the right direction is to enforce security at your context boundaries.

Ok, here’s a compromise option between AoP and functional.

defauth create_user(user_params) do
  User.changeset(%User{}, user_params) |> Repo.insert
end

This would get compiled to this:

  @spec create_user(User.t, map)
  def create_user(%User{}=user, user_params) do
    with :ok <- UserPolicy.authorize(:create_user, user, user_params) do
      User.changeset(%User{}, user_params) |> Repo.insert
    end
  end

That way, the controller and everyone else is forced to provide that user for authorization, but doesn’t hide the user parameter and obfuscate how things work. And the context method gets to have a single responsibility.

Oh I don’t have it in the controller’s, it is in the modules that handle that necessary work. ^.^

Yeah that would not even remotely work here. I have to verify they have access to very specific things. Like even when I get rows back from a database I have to verify that they have permission to access specific rows so I Enum.filter a lot. Plugs entirely fail there.

Just to add to the point: I feel like I’ve gained superpowers the day I discovered I could check for permissions wherever I wanted and that a permission check is just an function that returns true or false (this was a long time ago, in python-land).

“Declarative permissions” and friends and all fine and good when it makes sense, but sometimes you just have to ask (the DB, the rules, the world, etc) if this user can do that, and often you only know exactly which question to ask just before you ask it.

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.

@schrockwell, if I took a stab at the macro described above (msg 4), and had it check the policies the way you have laid out, would you be interested in incorporating it into bodyguard? I believe it would meet the pros of both strategies with out the cons of either.

Yeah, I’d definitely be interested. I’ve opened an issue: Add macro for incorporating authorization checks directly in contexts · Issue #30 · schrockwell/bodyguard · GitHub

Thanks! I’ll give it a shot soon.

Implementation question. I can implement this as 1 method where the auth is baked in as in my description above, or I can implement it like this:

defauth create_user(user_params), do: stuff

compiling to:

def create_user(%User{} = user, user_params) do
  with :ok <- authorize(:create_user, user, user_params) do
    __create_user(user_params)
  end
end
def __create_user__(user_params) do
  User.changeset(%User{}, user_params) |> Repo.insert
end

Pros

Testing is easier, because you can write specs against __create_user__ for that functionality, and test the policy for the security separately.

Cons

It is possible to skip the auth check, but it’s pretty obvious you’re doing it.

I’m leaning toward this approach because I don’t worry about malicious developers using my libraries, and it feels like a reasonable balance of productivity and security, but I’m open to critiques.