From 4a8e9765bc8834aa191da427861b4225579f0942 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Fri, 4 Nov 2016 19:41:58 +0900 Subject: [PATCH 1/3] Add a handle_authorize hook which is executed on all actions (before 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. --- lib/ja_resource/authorize.ex | 24 ++++++++++++++++++++++++ lib/ja_resource/create.ex | 3 +++ lib/ja_resource/delete.ex | 2 ++ lib/ja_resource/index.ex | 2 ++ lib/ja_resource/model.ex | 1 + lib/ja_resource/show.ex | 8 +++++--- lib/ja_resource/update.ex | 2 ++ 7 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 lib/ja_resource/authorize.ex diff --git a/lib/ja_resource/authorize.ex b/lib/ja_resource/authorize.ex new file mode 100644 index 0000000..b29ea78 --- /dev/null +++ b/lib/ja_resource/authorize.ex @@ -0,0 +1,24 @@ +defmodule JaResource.Authorize do + use Behaviour + + @moduledoc """ + Provides the `handle_authorize/0` callback used to authorize the resource. + + This behaviour is used by all JaResource actions. + """ + + @doc """ + Called before all the actions with the model. Useful for authorizing. + """ + @callback handle_authorize(Plug.Conn.t, JaResource.record) :: any + + defmacro __using__(_) do + quote do + @behaviour JaResource.Authorize + + def handle_authorize(model, _conn), do: model + + defoverridable [handle_authorize: 2] + end + end +end diff --git a/lib/ja_resource/create.ex b/lib/ja_resource/create.ex index 77d2de2..b7e7af0 100644 --- a/lib/ja_resource/create.ex +++ b/lib/ja_resource/create.ex @@ -65,6 +65,9 @@ defmodule JaResource.Create do def call(controller, conn) do merged = JaResource.Attributes.from_params(conn.params) attributes = controller.permitted_attributes(conn, merged, :create) + + controller.handle_authorize(controller.model(), conn) + conn |> controller.handle_create(attributes) |> JaResource.Create.insert(controller) diff --git a/lib/ja_resource/delete.ex b/lib/ja_resource/delete.ex index 59e852e..e535525 100644 --- a/lib/ja_resource/delete.ex +++ b/lib/ja_resource/delete.ex @@ -59,6 +59,8 @@ defmodule JaResource.Delete do def call(controller, conn) do model = controller.record(conn, conn.params["id"]) + controller.handle_authorize(model, conn) + conn |> controller.handle_delete(model) |> JaResource.Delete.respond(conn) diff --git a/lib/ja_resource/index.ex b/lib/ja_resource/index.ex index 89bfaa6..e8161da 100644 --- a/lib/ja_resource/index.ex +++ b/lib/ja_resource/index.ex @@ -119,6 +119,8 @@ defmodule JaResource.Index do Execute the index action on a given module implementing Index behaviour and conn. """ def call(controller, conn) do + controller.handle_authorize(controller.model(), conn) + conn |> controller.handle_index(conn.params) |> JaResource.Index.filter(conn, controller) diff --git a/lib/ja_resource/model.ex b/lib/ja_resource/model.ex index 0db75c5..b8a825d 100644 --- a/lib/ja_resource/model.ex +++ b/lib/ja_resource/model.ex @@ -25,6 +25,7 @@ defmodule JaResource.Model do defmacro __using__(_) do quote do @behaviour JaResource.Model + use JaResource.Authorize @inferred_model JaResource.Model.model_from_controller(__MODULE__) def model(), do: @inferred_model diff --git a/lib/ja_resource/show.ex b/lib/ja_resource/show.ex index e37e95f..c03b18c 100644 --- a/lib/ja_resource/show.ex +++ b/lib/ja_resource/show.ex @@ -57,9 +57,11 @@ defmodule JaResource.Show do Execute the show action on a given module implementing Show behaviour and conn. """ def call(controller, conn) do - conn - |> controller.handle_show(conn.params["id"]) - |> JaResource.Show.respond(conn, controller) + model = controller.handle_show(conn, conn.params["id"]) + + controller.handle_authorize(model, conn) + + JaResource.Show.respond(model, conn, controller) end @doc false diff --git a/lib/ja_resource/update.ex b/lib/ja_resource/update.ex index 9d8f29a..d9c9b68 100644 --- a/lib/ja_resource/update.ex +++ b/lib/ja_resource/update.ex @@ -72,6 +72,8 @@ defmodule JaResource.Update do merged = JaResource.Attributes.from_params(conn.params) attributes = controller.permitted_attributes(conn, merged, :update) + controller.handle_authorize(model, conn) + conn |> controller.handle_update(model, attributes) |> JaResource.Update.update(controller) From 7cf05209f10b35cf547e5cdd769ee590405fd000 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Fri, 11 Nov 2016 20:12:02 +0900 Subject: [PATCH 2/3] Properly respond to Ecto.Multi errors. --- lib/ja_resource/create.ex | 1 + lib/ja_resource/delete.ex | 1 + lib/ja_resource/update.ex | 1 + 3 files changed, 3 insertions(+) diff --git a/lib/ja_resource/create.ex b/lib/ja_resource/create.ex index b7e7af0..9a8ee3e 100644 --- a/lib/ja_resource/create.ex +++ b/lib/ja_resource/create.ex @@ -87,6 +87,7 @@ defmodule JaResource.Create do @doc false def respond(%Plug.Conn{} = conn, _old_conn), do: conn + def respond({:error, _name, errors, _changes}, conn), do: invalid(conn, errors) def respond({:error, errors}, conn), do: invalid(conn, errors) def respond({:ok, model}, conn), do: created(conn, model) def respond(model, conn), do: created(conn, model) diff --git a/lib/ja_resource/delete.ex b/lib/ja_resource/delete.ex index e535525..9fba99d 100644 --- a/lib/ja_resource/delete.ex +++ b/lib/ja_resource/delete.ex @@ -70,6 +70,7 @@ defmodule JaResource.Delete do def respond(nil, conn), do: not_found(conn) def respond(%Plug.Conn{} = conn, _old_conn), do: conn def respond({:ok, _model}, conn), do: deleted(conn) + def respond({:error, _name, errors, _changes}, conn), do: invalid(conn, errors) def respond({:errors, errors}, conn), do: invalid(conn, errors) def respond(_model, conn), do: deleted(conn) diff --git a/lib/ja_resource/update.ex b/lib/ja_resource/update.ex index d9c9b68..6590f19 100644 --- a/lib/ja_resource/update.ex +++ b/lib/ja_resource/update.ex @@ -94,6 +94,7 @@ defmodule JaResource.Update do @doc false def respond(%Plug.Conn{} = conn, _oldconn), do: conn def respond(nil, conn), do: send_resp(conn, :not_found, "") + def respond({:error, _name, errors, _changes}, conn), do: invalid(conn, errors) def respond({:error, errors}, conn), do: invalid(conn, errors) def respond({:ok, model}, conn), do: updated(conn, model) def respond(model, conn), do: updated(conn, model) From aac8cf270c8d192301c6684cb00d9dae18fb12ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Tue, 15 Nov 2016 01:01:46 +0900 Subject: [PATCH 3/3] Make it work with normal responses too... --- lib/ja_resource/create.ex | 1 + lib/ja_resource/delete.ex | 1 + lib/ja_resource/model.ex | 9 +++++++++ lib/ja_resource/update.ex | 1 + 4 files changed, 12 insertions(+) diff --git a/lib/ja_resource/create.ex b/lib/ja_resource/create.ex index 9a8ee3e..73df86f 100644 --- a/lib/ja_resource/create.ex +++ b/lib/ja_resource/create.ex @@ -89,6 +89,7 @@ defmodule JaResource.Create do def respond(%Plug.Conn{} = conn, _old_conn), do: conn def respond({:error, _name, errors, _changes}, conn), do: invalid(conn, errors) def respond({:error, errors}, conn), do: invalid(conn, errors) + def respond({:ok, %{} = map}, conn), do: created(conn, Map.fetch(map, controller.atom())) def respond({:ok, model}, conn), do: created(conn, model) def respond(model, conn), do: created(conn, model) diff --git a/lib/ja_resource/delete.ex b/lib/ja_resource/delete.ex index 9fba99d..2ec7bd8 100644 --- a/lib/ja_resource/delete.ex +++ b/lib/ja_resource/delete.ex @@ -69,6 +69,7 @@ defmodule JaResource.Delete do @doc false def respond(nil, conn), do: not_found(conn) def respond(%Plug.Conn{} = conn, _old_conn), do: conn + def respond({:ok, %{} = map}, conn), do: created(conn, Map.fetch(map, controller.atom())) def respond({:ok, _model}, conn), do: deleted(conn) def respond({:error, _name, errors, _changes}, conn), do: invalid(conn, errors) def respond({:errors, errors}, conn), do: invalid(conn, errors) diff --git a/lib/ja_resource/model.ex b/lib/ja_resource/model.ex index b8a825d..5484258 100644 --- a/lib/ja_resource/model.ex +++ b/lib/ja_resource/model.ex @@ -30,6 +30,15 @@ defmodule JaResource.Model do @inferred_model JaResource.Model.model_from_controller(__MODULE__) def model(), do: @inferred_model + def atom() do + model() + |> Atom.to_string + |> String.split(".") + |> List.last + |> String.downcase + |> String.to_atom + end + defoverridable [model: 0] end end diff --git a/lib/ja_resource/update.ex b/lib/ja_resource/update.ex index 6590f19..5917dcf 100644 --- a/lib/ja_resource/update.ex +++ b/lib/ja_resource/update.ex @@ -96,6 +96,7 @@ defmodule JaResource.Update do def respond(nil, conn), do: send_resp(conn, :not_found, "") def respond({:error, _name, errors, _changes}, conn), do: invalid(conn, errors) def respond({:error, errors}, conn), do: invalid(conn, errors) + def respond({:ok, %{} = map}, conn), do: created(conn, Map.fetch(map, controller.atom())) def respond({:ok, model}, conn), do: updated(conn, model) def respond(model, conn), do: updated(conn, model)