Add a handle_authorize hook which is executed on all actions (before logic)

[archseer]

Hi! I've butchered the code a bit to fit our specific usecase (integrating with an authorization library, like bodyguard)

My goal was to run authorization on the actual record, for all actions. It's currently a bit hard to do, because the handle_ callbacks are executed at various stages (show fetches the record, update gets the already fetched record, etc.) and I did not want to manually do this in handle_ calls, since it's very repetitive and error-prone (as I add more routes and hooks, I must remember to add authorization).

The closest I could get was patching record/2, but that doesn't take care of index and create; which makes it even easier to make a mistake and forget to manually do it:

  def records(conn) do
    scope(conn, model())
  end

  # this solves the auth for show, update and delete (but not index or create)
  def record(conn, id) do
    r = super(conn, id)
    authorize!(conn, r, policy: Post.Policy)
    r
  end

  def handle_index(conn, attributes) do
    authorize!(conn, Post)
    super(conn, attributes)
  end

  def handle_create(conn, attributes) do
    authorize!(conn, Post)
    super(conn, attributes)
  end

So I added a new callback. I don't expect it to be mergeable in the current state, but I wanted to showcase this usecase and get the discussion going to see if we can get it integrated in some form.

Show, delete and update will call the method with the actual model to be operated on (but before anything is done to the model!). index and create actions will pass in the model() module instead (since we're operating on a collection/adding a record not yet in the db).

This is the new callback:

  def handle_authorize(model, conn) do
    authorize!(conn, model)
  end

The return value currently doesn't matter, but maybe we could return a Plug.Conn to use later down the pipeline (some of the auth frameworks will do assigns on the conn).

[archseer]

handle_authorize might not be the best name, maybe handle_before or handle_setup.

[alanpeabody]

Thanks @archSeer. On comment is that the result of the handle_authorize does nothing. Are you expecting exceptions to be raised?

Related is #32

Let me think about this a bit, I realize this is something people want, I just want to make sure it fits right. /cc @beerlington

[archseer]

Bodyguard has two versions, authorize!/3 will raise which will then render a 403 up the stack, and the non-bang version which just returns a bool.

The reason why I'm suggesting to return a conn is related: bodyguard has a verify_authorized plug to check that all the actions on the controller are verified (so we don't miss any), it does so by marking the conn with a boolean after the method runs. Since I'm not passing the connection further down the pipeline, that functionality currently does not work.

Regarding #32, we've ran into that usecase as well, but I think the correct solution there is to just use an Ecto.Multi module. Another thing I was evaluating is the permitted_attributes. We're currently using them, but I think Ecto 2.x is slowly leaning towards multiple changesets (create_changeset/2, update_changeset/2, you share the code between the two by calling changeset/2)

I agree that it's getting a bit messy as we're adding more callbacks and hooks, especially since a lot of the handle_ ones have different arities and arguments. It reduces the amount of boilerplate needed, but it's a bit less straightforward. I do like the simple filter & sort implementation, one of the main reasons we switched 👍 👍