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

@schrockwell - I wasn’t sure if it’d be better to ask this here, or as a github issues, but thought more input would be sourced this way.

v2 of Bodyguard was recently released https://github.com/schrockwell/bodyguard

It’s AWESOME for authorization in phoenix 1.3. Highly recommended.

I do have one question that’s more about the docs and the 1.3 Phoenix release than anything else though.

Where do you CALL your authorization logic? The examples for bodyguard suggest doing so in the controller. That’s how things were done before, and it certainly keeps methods that otherwise don’t have any need for the user object to not require it. But it also makes it really easy to skip your authorization calls (inside or outside of your phoenix app). So it seems like forcing authorization would be ideal.

One other option would be to look at authorization from an AoP point of view. To do that effectively in phoenix, I think you’d need to find a way to store the current user on each call, and then wrap all of your auth-requiring methods in macros that will check your policies before executing the underlying code.

I saw a great article on Function Decorators here that could be used.

The negative to this approach is just that part about having to set ‘invisible’ data as context to a function. It never bothered me in OOP land, but it feels anti-functional here.

2 Likes

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.

1 Like

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.

1 Like

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.

3 Likes

@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.

1 Like

Yeah, I’d definitely be interested. I’ve opened an issue: https://github.com/schrockwell/bodyguard/issues/30

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.

I like method B as well. Maybe name it __create_user__ to match Elixir’s convention for “magic” identifiers.

:thumbsup: You got it.

If anyone with more macro-writing experience wouldn’t mind critiquing what I have, here it is:
https://github.com/schrockwell/bodyguard/pull/31/files

There’s plenty of cleanup to do and docs to write, but the macro itself is the part I’m least confident of. Specifically, why did I have to var!(user) on line 80?

Is this for per-function testing before the body is called? I often have to check permissions inside the body, not knowing what I can check for until I get the query/queries/remote_response/etc back.

Wow. Well with a use case like that, Bodyguard wouldn’t be a great match for your app even before introducing this macro, I guess.

This is pretty common here due to interactions with an utterly ancient database with incredibly fine-grained permissions. ^.^;

It is a pretty cool setup though that I have to deal with, should be a use-case that bodyguard should support. :slight_smile:

I think it is pretty common to have multiple layers of security. You’ve got high-level perimeter security - is this a known person and can he even access these resource types - and then you may have business logic that governs what you can do with an individual record. For example in an HR system - you are a manager so you can access the compensation page - but which employees can you see? Just the employees who report up to you.

Putting perimeter security in the controller - where we deserialize and sanitize all the untrusted input coming from the world - doesn’t sound like the wrong thing to me.

What about when you need to do the same security on a websocket connection? Or a TCP connection? Or something else? You start duplicating code. Better to put all work into a context (as phoenix calls it) including the authentication, then just call those.

An interesting article about authorization: https://betterment.engineering/whats-the-best-authorization-framework-none-at-all-92e6711ee41b

They make some very good points about keeping it simple—even to the point where they don’t use frameworks at all. Particularly, I like:

  • Don’t create an endpoint that you don’t need. Eg. if you want to be sure the user can only see their own account, don’t create a GET /accounts/:id endpoint.
  • Build a separate app for different audiences. Eg. public facing app and a separate customer service app.