Add a handle_authorize hook which is executed on all actions (before logic) #36
Conversation
|
|
|
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 |
|
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 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 ( 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 👍 👍 |
…anything).
This way I can simply add authorization on the record that's properly
loaded and scoped by ja_resource for all routes at once.
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 was intended for integration with bodyguard. I had to do the
following:
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: Midori.Bot.Policy)
r
end
def handle_index(conn, attributes) do
authorize!(conn, Bot)
super(conn, attributes)
end
def handle_create(conn, attributes) do
authorize!(conn, Bot)
super(conn, attributes)
end
I wanted a solution that would take care of index and create as well,
because having to do it manually action by action is error-prone.
| def respond({:error, errors}, conn), do: invalid(conn, errors) | ||
| def respond({:ok, %{} = map}, conn), do: created(conn, Map.fetch(map, controller.atom())) |
There was a problem hiding this comment.
This is going to match regular models structs from changesets as well.
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:
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:
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).