cpjk / canary Goto Github PK
View Code? Open in Web Editor NEW:hatching_chick: Elixir authorization and resource-loading library for Plug applications.
License: MIT License
:hatching_chick: Elixir authorization and resource-loading library for Plug applications.
License: MIT License
I'm not sure if this is intentional or not, but I suspect it isn't: when using load_and_authorize_resource
the requested resource will be fetched two times. In the case of models backed by a database this is certainly undesirable.
load_and_authorize_resource
calls authorize_resource
which, in the case of actions like :show
calls fetch_resource
which loads the model. Then load_if_authorized
is called. Assuming the current_user was authorized load_resource
is called. In the case of actions like :show
fetch_resource
is called again.
Additionally, even when just using authorize_resource
by itself, for actions like :show
you still end up with a double load if you later load the model yourself in (for example) your controller. authorize_resource
doesn't put the resource in the assigns so there's no way to reuse what was just loaded.
I can think of several ways around this. One might be storing the fetched resource in conn assigns. The documentation alludes to a fetched_resource
assign, but nowhere is that found in the code itself. If you put the fetched resource there you could then just copy it to loaded_resource
.
I'd do a pull request, but I'm not entirely sure this isn't the desired behavior.
I have this:
def can?(%User{ id: user_id }, action, %Post{ user_id: user_id })
when action in [:show], do: true
But this does not prevent users from seeing other user's posts.
Hi!
Using Elixir 1.14.5 and OTP 25.2.2
I get these warning when canary is compiled as dependency to my project:
warning: Canada.Can.can?/3 defined in application :canada is used by the current application but the current application does not depend on :canada. To fix this, you must do one of:
1. If :canada is part of Erlang/Elixir, you must include it under :extra_applications inside "def application" in your mix.exs
2. If :canada is a dependency, make sure it is listed under "def deps" in your mix.exs
3. In case you don't want to add a requirement to :canada, you may optionally skip this warning by adding [xref: [exclude: [Canada.Can]]] to your "def project" in mix.exs
lib/canary/plugs.ex:213: Canary.Plugs.do_authorize_resource/2
warning: Ecto.Query.from/1 defined in application :ecto is used by the current application but the current application does not depend on :ecto. To fix this, you must do one of:
1. If :ecto is part of Erlang/Elixir, you must include it under :extra_applications inside "def application" in your mix.exs
2. If :ecto is a dependency, make sure it is listed under "def deps" in your mix.exs
3. In case you don't want to add a requirement to :ecto, you may optionally skip this warning by adding [xref: [exclude: [Ecto.Query]]] to your "def project" in mix.exs
Invalid call found at 2 locations:
lib/canary/plugs.ex:319: Canary.Plugs.fetch_all/2
lib/canary/plugs.ex:324: Canary.Plugs.fetch_all/2
Indeed canary mix.exs
does not list :ecto
or :canada
:
def application do
[applications: [:logger]]
end
The documentation says that ecto and canada should be inferred from deps, where they are listed.
The side-effect of this warning is that sometimes canary will fail to compile โ and one needs to remove _build
dir to force another attempt, and it usually works.
Any ideas what's the best way here?
When I want to access one of my custom routes, I get the following error
nil given for :id. Comparison with nil is forbidden as it is unsafe.
Instead write a query with is_nil/1, for example: is_nil(s.id)
That's probably the interesting part of the call log:
lib/canary/plugs.ex:296 Canary.Plugs.fetch_resource/2
lib/canary/plugs.ex:112 Canary.Plugs.do_load_resource/2
lib/canary/plugs.ex:92 Canary.Plugs.load_resource/2
lib/canary/plugs.ex:271 Canary.Plugs.do_load_and_authorize_resource
I thought that was solved by non_id_actions
but it does not help.
For example, plug :load_resource, model: Project.User
would load the user resource into conn.assigns.user,
which would then be available in views as @user
I think it would be crucial to document this specially when it comes to singleton resources.
where one can have Show, edit or even update routes as none_id_actions and if not specified as such then the permission fails cause cannot resolve the route
hi
does Canary support Channels?
would it be possible to have an example in the plug test file?
I asked this on Canada and its apparently possible
jarednorman/canada#14
Canary.Plugs should have an @moduledoc block, and the @doc block for each plug should be expanded and updated for readability on hexdocs
I'm not yet really familiar with versioning in Elixir yet, but mix hex.outdated
shows that all my top-level dependencies are up-to-date. But when I add {:canary, "~> 0.14.1"}
, mix deps.get
gives me this:
Failed to use "ecto" (version 2.0.2) because
canary (version 0.14.1) requires ~> 1.1
phoenix_ecto (version 3.0.0) requires ~> 2.0.0-rc
Locked to 2.0.2 in your mix.lock
If someone fetches a resource when the action is :new or :create, then they get nil back every time.
If we have a nested resource such as:
/api/v1/questions/:question_id/answers
where questions are owned by users, then we can never load and/or authorize a resource on these creation actions.
Should we allow resources to be loaded on :new and :create actions so we can deal with nested resources? Thoughts?
We have the Network
model which belongs to the Hypervisor
, and /networks
resources are nested like this:
resources "/hypervisors", HypervisorController do
resources "/networks", NetworkController, only: [:new, :create, :index]
end
According to the docs persisted
option is a bless for nested resources.
plug :load_resource,
model: Hypervisor,
id_name: "hypervisor_id",
only: [:index, :create, :new],
preload: [:hypervisor_type],
persisted: true
This is almost perfect. For :index
, :new
or :create
action the handle_not_found/2
will never call error handler. For our case it's essential to allow visit nested pages only if parent resource exists.
So now when I visit /hypervisors/999/networks
the conn.assigns[:hypervisor
will be nil
, I'd expect a 404 error.
I was going through the documentation, and this didn't make sense to me:
If both an :unauthorized_handler and a :not_found_handler are specified for load_and_authorize_resource, and the request meets the criteria for both, the :unauthorized_handler will be called first.
Why would I care if a user is unauthorized for a resource if that resource doesn't exist? And how can authorization status be determined against a nil resource? A cursory glance at the code shows some checking to get around this, so it's possible I'm missing something.
Before I put the time in to refactor this, I'm curious to hear your thoughts.
https://github.com/cpjk/canary/blob/master/lib/canary.ex#L4 this using is unnecessary as all it does is import. Update the README and tell user's to just import it :)
I got the following warnings from my Canada.Can
file but these changes were encouraged in the README for Anonymous Users
.
warning: the Canada.Can protocol has already been consolidated, an implementation for App.User has no effect
lib/app/abilities.ex:2
warning: the Canada.Can protocol has already been consolidated, an implementation for Atom has no effect
lib/app/abilities.ex:27
After deploying to a server, I am getting:
Canada.Can.can?/3 is undefined (module Canada.Can is not available)
Canada.Can.can?(%App.User{...}, :update, %App.Location{...})
Abilities file:
defmodule Canary.Abilities do
defimpl Canada.Can, for: User do
def can?(%User{} = user, :update, %Location{}), do: LocationPolicy.update?(user)
Any thoughts on why this is failing after building and deploying (using distillery) to a server, but working locally? Not much to go on, but if you have encountered something similar, it would be great to hear! Otherwise will close this issue.
I've seen some comments around the codebase concerning :index
and :create
actions.
What if you were able to pass the parent id all the way down to the Canada.Can
implementation so said function can check if current_user
is authorized to :create
a resource of type: for: Type
without an id of Type
?
This only concerns sub-resources with relations to a parent, i.e., "Can I create a new post based on the thread that post belongs to?" (and current_user of course).
Thoughts? Do tell if I should make myself more clear or if this can already be solved in some other way!
Hi! I wrote in config.exs:
config :canary, repo: MyApp.Repo,
unauthorized_handler: {MyApp.Helpers, :handle_unauthorized},
not_found_handler: {MyApp.Helpers, :handle_not_found}
and created helper:
defmodule MyApp.Helpers do
use Phoenix.Controller
require Logger
import Plug.Conn
def handle_unauthorized(conn) do
conn
|> put_status(401)
|> put_view(MyApp.ErrorView)
|> render("401.json")
|> halt
end
def handle_not_found(conn) do
conn
|> put_status(404)
|> put_view(MyApp.ErrorView)
|> render("404.json")
|> halt
end
end
my abilities.ex:
defimpl Canada.Can, for: MyApp.User do
alias MyApp.Operation
alias MyApp.User
def can?(user, action, operation = %Operation{}) when action in [:show, :update, :delete] do
IO.puts "check ability for operation"
if operation.user_id == user.id do
IO.puts "success"
true
else
false
end
end
def can?(subject, action, resource) do
raise """
Unimplemented authorization check for User! To fix see below...
Please implement `can?` for User in #{__ENV__.file}.
The function should match:
subject: #{inspect subject}
action: #{action}
resource: #{inspect resource}
"""
end
end
And unauthorized_handler works fine. But not_found_handler does not work.
[error] #PID<0.709.0> running MyApp.Endpoint terminated
Server: localhost:4000 (http)
Request: GET /api/v1/operations/11
** (exit) an exception was raised:
** (RuntimeError) Unimplemented authorization check for User! To fix see below...
Please implement `can?` for User in /home/mars/phoenix_projects/my_app/lib/abilities.ex.
What could be the reason?
hey there
am wondering if Canary works ok with Guardian DB?
this is my first Phoenix app and so am unsure exactly if this is a Canary issue BUT
when my controller is only loading resources via Canary I am ok
will follow-up with file info
been going round in circles on this for a few days now
[info] GET /spaces
[debug] QUERY OK source="guardian_tokens" db=0.7ms
SELECT g0."jti", g0."typ", g0."aud", g0."iss", g0."sub", g0."exp", g0."jwt", g0."claims", g0."inserted_at", g0."updated_at" FROM "guardian_tokens" AS g0 WHERE ((g0."jti" = $1) AND (g0."aud" = $2)) ["bbe8075b-b204-4cd3-bb04-8871dc18e7d8", "User:1"]
[debug] QUERY OK source="users" db=0.3ms queue=0.3ms
SELECT u0."id", u0."name", u0."email", u0."bio", u0."zipcode", u0."address", u0."phone", u0."inserted_at", u0."updated_at" FROM "users" AS u0 WHERE (u0."id" = $1) [1]
[debug] Processing by SavesonomacountyOrg.SpaceController.index/2
Parameters: %{}
Pipelines: [:browser, :browser_auth, :require_login]
[debug] QUERY OK source="spaces" db=0.2ms
SELECT s0."id", s0."name", s0."user_id", s0."inserted_at", s0."updated_at" FROM "spaces" AS s0 []
[debug] QUERY OK source="spaces" db=0.1ms
SELECT s0."id", s0."name", s0."user_id", s0."inserted_at", s0."updated_at" FROM "spaces" AS s0 []
when my controller is trying to authorize I get Guardian DB errors
[error] #PID<0.1392.0> running SavesonomacountyOrg.Endpoint terminated
Server: website.io:4000 (http)
Request: GET /spaces
** (exit) an exception was raised:
** (KeyError) key :guardian_default_resource not found in: %{current_user: %SavesonomacountyOrg.User{__meta__: #Ecto.Schema.Metadata<:loaded, "users">, address: nil, authorizations: #Ecto.Association.NotLoaded<association :authorizations is not loaded>, bio: nil, email: "[email protected]", id: 1, inserted_at: #Ecto.DateTime<2016-11-06 21:46:22>, name: "Nicholas Roberts", phone: nil, spaces: #Ecto.Association.NotLoaded<association :spaces is not loaded>, updated_at: #Ecto.DateTime<2016-11-06 21:46:22>, zipcode: nil}, spaces: [%SavesonomacountyOrg.Space{__meta__: #Ecto.Schema.Metadata<:loaded, "spaces">, id: 2, inserted_at: #Ecto.DateTime<2016-11-06 21:58:34>, name: "rr2", updated_at: #Ecto.DateTime<2016-11-06 21:58:44>, user: #Ecto.Association.NotLoaded<association :user is not loaded>, user_id: 1}]}
(elixir) lib/map.ex:164: Map.fetch!/2
lib/canary/plugs.ex:193: Canary.Plugs._authorize_resource/2
lib/canary/plugs.ex:186: Canary.Plugs.authorize_resource/2
lib/canary/plugs.ex:267: Canary.Plugs._load_and_authorize_resource/2
(savesonomacounty_org) web/controllers/space_controller.ex:1: SavesonomacountyOrg.SpaceController.phoenix_controller_pipeline/2
(savesonomacounty_org) lib/savesonomacounty_org/endpoint.ex:1: SavesonomacountyOrg.Endpoint.instrument/4
(savesonomacounty_org) lib/phoenix/router.ex:261: SavesonomacountyOrg.Router.dispatch/2
(savesonomacounty_org) web/router.ex:1: SavesonomacountyOrg.Router.do_call/2
(savesonomacounty_org) lib/savesonomacounty_org/endpoint.ex:1: SavesonomacountyOrg.Endpoint.phoenix_pipeline/1
(savesonomacounty_org) lib/plug/debugger.ex:123: SavesonomacountyOrg.Endpoint."call (overridable 3)"/2
(savesonomacounty_org) lib/savesonomacounty_org/endpoint.ex:1: SavesonomacountyOrg.Endpoint.call/2
(plug) lib/plug/adapters/cowboy/handler.ex:15: Plug.Adapters.Cowboy.Handler.upgrade/4
(cowboy) src/cowboy_protocol.erl:442: :cowboy_protocol.execute/4
Sorry if this has been addressed in the docs, but I can't seem to find it.
How do we add a permission for 'nested' models? For example, imagine a User, a Project, and Tasks:
task.project_id -> project.user_id -> user.id
I only want the user to be able to see, edit, etc. tasks if he owns the project.
Hi
Would you accept a PR regarding the readme? Getting started on Phoenix was not that clear to me and I bumped into some issues. Be happy do document a bit to get other users up and running. What do you think?
When defining abilities for :index
, :new
, and :create
, _authorize_resource/2
passes the module to can?/3
, not a struct.
This means defining abilities requires doing this:
def can?(%User{}, action, Item) when action in [:index, :new, :create], do: true
def can?(%User{}, action, %Item{}) when action in [:show], do: true
Rather than just:
def can?(%User{}, action, %Item{}) when action in [:index, :new, :create, :show], do: true
It makes sense in that at index, new, and create, you have no instance of the model so it would not pass a struct.
Maybe this needs to be mentioned in the docs or change the behavior?
If there has been a single field, then I would have used id_name and id_field to get the resource, but a single field is not defining the resource, I need 2 fields to define it uniquely. How can I do that?
for example:
a user is defined uniquely by fieldA and fieldB, conn.path_params will give those parameters, and we will need to use Repo.get_by on those field combined. As of now, there is no way to define 2 fields simultaneously
The current repo in github differs from hex.pm's version in terms of content but the version is the same (1.1.1)
hex.pm has version 1.1.1 that was uploaded at 2017-Apr-16. Current mix.exs
is 1.1.1 too but new things have been introduced since (like authorize_controller
)
Update version in mix.exs
and update hex.pm too
The check in get_action
:
defp get_action(conn) do
conn.assigns
|> Map.fetch(:action)
|> case do
{:ok, action} -> action
_ -> conn.private.phoenix_action
end
end
wrongfully assumes that an unrelated resource conn.assigns.action
is me specifying the action
manually ๐
Maybe use a namespace (i.e. canary_action
) or a different name (i.e. controller_action
, router_action
) or something? conn.assigns.action
is rather generic!
I would like to use the :authorize_resource
plug in a controller for a singleton resource (/user
).
This means that there is no :id
parameter for the show
action. However canary always tries to fetch the resource for the show
action by id. I'm not sure what a good solution would be? Maybe allow overwriting the fetch_resource/2
method?
Can you consider custom-extending list of actions that require model name in Canada.Can.can? implementation. (List is currently hard-coded to [:index, :new, :create]).
For authorize_resource plug, and perhaps for other plugs, this can be defined by inserting additional option, say require_model: [:custom_action1, :custom_action2, ...]
Then you can modify authorize_resource/2 function in plugs.ex. As follows round about line 195:
`
require_model_list = if opts[:require_model], do: ([:index, :new, :create] ++ opts[:require_model]), else: [:index, :new, :create]
resource = cond do
is_persisted ->
fetch_resource(conn, opts)
# action in [:index, :new, :create] ->
action in require_model_list ->
opts[:model]
true ->
fetch_resource(conn, opts)
end
`
handle_not_found/2 may also need to be amended.
In my page controller
plug :authorize_resource, model: User
In my router
scope "/admin", as: :admin, alias: Soranus do
pipe_through [:browser, :browser_session, :browser_auth]
get "/", PageController, :admin_index
end
My test
describe "logged in admin" do
setup do
user =
params_for(:user)
|> user_with_registration_changeset()
conn =
guardian_login(build_conn(), user)
{:ok, conn: conn, user: user}
end
test "get admin index when user_type = :admin", %{conn: conn} do
conn = get conn, admin_page_path(conn, :admin_index)
assert html_response(conn, 200)
end
end
The function guardian_login
sets the user in conn, and in my pipline browser_auth
i use a Plug to set the current_user to conn from Guardian.Plug.current_resource(conn)
Like this
defmodule Soranus.Authentication.PlugCurrentUser do
def init(opts), do: opts
@lint {Credo.Check.Design.AliasUsage, false}
def call(conn, _opts) do
current_user = Guardian.Plug.current_resource(conn)
Plug.Conn.assign(conn, :current_user, current_user)
end
end
Error in conn = get conn, admin_page_path(conn, :admin_index)
in my tests (above)
Error:
2) test logged in admin get admin index when user_type = :admin (Soranus.PageControllerTest)
test/controllers/page_controller_test.exs:25
** (ArgumentError) nil given for :id. Comparison with nil is forbidden as it is unsafe. Instead write a query with is_nil/1, for example: is_nil(s.id)
stacktrace:
(ecto) lib/ecto/query/builder/filter.ex:107: Ecto.Query.Builder.Filter.runtime!/6
(ecto) lib/ecto/query/builder/filter.ex:98: Ecto.Query.Builder.Filter.runtime!/2
(ecto) lib/ecto/repo/queryable.ex:304: Ecto.Repo.Queryable.query_for_get_by/3
(ecto) lib/ecto/repo/queryable.ex:52: Ecto.Repo.Queryable.get_by/5
lib/canary/plugs.ex:298: Canary.Plugs.fetch_resource/2
lib/canary/plugs.ex:203: Canary.Plugs._authorize_resource/2
lib/canary/plugs.ex:186: Canary.Plugs.authorize_resource/2
(soranus) web/controllers/page_controller.ex:1: Soranus.PageController.phoenix_controller_pipeline/2
(soranus) lib/soranus/endpoint.ex:1: Soranus.Endpoint.instrument/4
(soranus) lib/phoenix/router.ex:261: Soranus.Router.dispatch/2
(soranus) web/router.ex:1: Soranus.Router.do_call/2
(soranus) lib/soranus/endpoint.ex:1: Soranus.Endpoint.phoenix_pipeline/1
(soranus) lib/soranus/endpoint.ex:1: Soranus.Endpoint.call/2
(phoenix) lib/phoenix/test/conn_test.ex:224: Phoenix.ConnTest.dispatch/5
test/controllers/page_controller_test.exs:26: (test)
Shouldn't :non_id_actions option be on Hex documentation ? There is a reference on the Readme but nothing on Hex.
I explained why I need it here: jarednorman/canada#11
Do you have any suggestion or alternative way of doing that?
Currently the index action must specify something like:
def can?(%User{}, :index, Post), do: true
Which means the user object can access the post object. But theres no way to specify that the user should only be able to access their own posts. Ideally, if the index action looped through the result set and ran can?/3
on each item, it would be simpler and more flexible to configure. I'm proposing something like this instead:
def can?(%User{id: id}, action, %Post{user_id: id}) when action in [:index, :show], do: true
To clearly say a user may access their own post for the :index
and :show
actions.
Assuming current_user
will work for a little bit, but often apps have stuff like current_user
and current_admin_user
, ect. It would be nice to be able to configure this globally in config.exs
as well as override it per plug like plug :load_resource, model: User, user: :current_admin_user
.
If I have time I'd love to help out on this, for whoever gets to it first...
Hi,
Is there an way to store the authorization rules in a permanent datastore, maybe using Ecto?
Updated the question to make it more clear.
Hey! I noticed you removed the note that this project is unmaintained, so I assume it makes sense to open issues again :)
We are using this library in an older project that I intend to update to Phoenix 1.4, but the dependency on the ecto library currently stops me from doing that.
I already tried bumping plug to 1.7 and changing ecto to ecto_sql ~> 3.0, but after that a ton of tests failed.
Are you perhaps already working on making this compatible to Phoenix 1.4?
fetch_resource/2
attempts to preload the provided associations whether the repo is able to retrieve a resource or not.
repo.get/2
returns nil if the resource does not exist.
This ends up creating the following stacktrace:
** (FunctionClauseError) no function clause matching in Ecto.Repo.Preloader.preload/3
stacktrace:
(ecto) lib/ecto/repo/preloader.ex:35: Ecto.Repo.Preloader.preload(nil, MyApp.Repo, :my_association)
lib/canary/plugs.ex:82: Canary.Plugs._load_resource/2
# ...
# canary/plugs.ex
defp fetch_resource(conn, opts) do
repo = Application.get_env(:canary, :repo)
conn
|> Map.fetch(resource_name(conn, opts))
|> case do
:error ->
repo.get(opts[:model], conn.params["id"])
|> preload_if_needed(repo, opts)
# **Here**:
{:ok, nil} ->
repo.get(opts[:model], conn.params["id"])
|> preload_if_needed(repo, opts)
{:ok, resource} -> # if there is already a resource loaded onto the nn
case (resource.__struct__ == opts[:model]) do
true ->
resource
false ->
# **And here**:
repo.get(opts[:model], conn.params["id"])
|> preload_if_needed(repo, opts)
end
end
end
Add a guard in preload_if_needed/3
when records
is nil
.
warning: variable "package" does not exist and is being expanded to "package()", please use parentheses to remove the ambiguity or change the variable name
Or am I blind? Consider adding a file LICENSE
to the root of your project!
Can be great with a sample Phoenix application ๐ Thank you.
Plugs should infer the model name from the requested URL based on a commonly accepted convention, unless the model is explicitly stated in the plug call.
If I put in "/users/foo", Canary will throw an exception in trying to access the resource as "foo" is clearly not an acceptable where
query for id
.
** (Ecto.Query.CastError) /my_app/deps/ecto/lib/ecto/repo/queryable.ex:331: value `"foo"` in `where` cannot be cast to type :id in query:
from g in MyApp.Account.User,
where: g.id == ^"foo",
select: g
This exception should be caught and we should have an :unprocessable_entity
handler in the error controller, rather than letting the application implode.
Thanks for the lib.
I got a little problem to work with multiple models in me project.
person_abilities.ex
and product_abilities.ex
.PersonController
and ProductController
. In abilities.ex
I have permission only for Person
. That's works if I add %Product{}
, but I have some pattern matching for nil models because when the user search for a information that not exists the last argument in can?
is always nil so I can't blocking the process for him. So, a have the following: https://gist.github.com/edmaarcosta/a26b60932e2e5b06ea6db1fb4506209fWith this approach to control the pattern matching for Product
doesn't works because the Canada don't know what model should manipulate.
Thoughts?
Sorry my english is terrible.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.