Giter Site home page Giter Site logo

cpjk / canary Goto Github PK

View Code? Open in Web Editor NEW
473.0 10.0 51.0 199 KB

:hatching_chick: Elixir authorization and resource-loading library for Plug applications.

License: MIT License

Elixir 100.00%
elixir plug phoenix-application authorization phoenix-framework

canary's Introduction

Canary

Build Status Hex pm

An authorization library in Elixir for Plug applications that restricts what resources the current user is allowed to access, and automatically loads resources for the current request.

Inspired by CanCan for Ruby on Rails.

Read the docs

Installation

For the latest master:

defp deps do
  {:canary, github: "cpjk/canary"}
end

For the latest release:

defp deps do
  {:canary, "~> 1.1.1"}
end

Then run mix deps.get to fetch the dependencies.

Usage

Canary provides three functions to be used as plugs to load and authorize resources:

load_resource/2, authorize_resource/2, and load_and_authorize_resource/2.

load_resource/2 and authorize_resource/2 can be used by themselves, while load_and_authorize_resource/2 combines them both.

In order to use Canary, you will need, at minimum:

Then, just import Canary.Plugs in order to use the plugs. In a Phoenix app the best place would probably be inside controller/0 in your web/web.ex, in order to make the functions available in all of your controllers.

load_resource/2

Loads the resource having the id given in conn.params["id"] from the database using the given Ecto repo and model, and assigns the resource to conn.assigns.<resource_name>, where resource_name is inferred from the model name.

For example,

plug :load_resource, model: Project.Post

Will load the Project.Post having the id given in conn.params["id"] through YourApp.Repo, into conn.assigns.post

authorize_resource/2

Checks whether or not the current_user for the request can perform the given action on the given resource and assigns the result (true/false) to conn.assigns.authorized. It is up to you to decide what to do with the result.

For Phoenix applications, Canary determines the action automatically.

For non-Phoenix applications, or to override the action provided by Phoenix, simply ensure that conn.assigns.canary_action contains an atom specifying the action.

In order to authorize resources, you must specify permissions by implementing the Canada.Can protocol for your User model (Canada is included as a light weight dependency).

load_and_authorize_resource/2

Authorizes the resource and then loads it if authorization succeeds. Again, the resource is loaded into conn.assigns.<resource_name>.

In the following example, the Post with the same user_id as the current_user is only loaded if authorization succeeds.

Usage Example

Let's say you have a Phoenix application with a Post model, and you want to authorize the current_user for accessing Post resources.

Let's suppose that you have a file named lib/abilities.ex that contains your Canada authorization rules like so:

defimpl Canada.Can, for: User do
  def can?(%User{ id: user_id }, action, %Post{ user_id: user_id })
    when action in [:show], do: true

  def can?(%User{ id: user_id }, _, _), do: false
end

and in your web/router.ex: you have:

get "/posts/:id", PostController, :show
delete "/posts/:id", PostController, :delete

To automatically load and authorize on the Post having the id given in the params, you would add the following plug to your PostController:

plug :load_and_authorize_resource, model: Post

In this case, on GET /posts/12 authorization succeeds, and the Post specified by conn.params["id] will be loaded into conn.assigns.post.

However, on DELETE /posts/12, authorization fails and the Post resource is not loaded.

Excluding actions

To exclude an action from any of the plugs, pass the :except key, with a single action or list of actions.

For example,

Single action form:

plug :load_and_authorize_resource, model: Post, except: :show

List form:

plug :load_and_authorize_resource, model: Post, except: [:show, :create]

Authorizing only specific actions

To specify that a plug should be run only for a specific list of actions, pass the :only key, with a single action or list of actions.

For example,

Single action form:

plug :load_and_authorize_resource, model: Post, only: :show

List form:

plug :load_and_authorize_resource, model: Post, only: [:show, :create]

Note: Passing both :only and :except to a plug is invalid. Canary will simply pass the Conn along unchanged.

Overriding the default user

Globally, the default key for finding the user to authorize can be set in your configuration as follows:

config :canary, current_user: :some_current_user

In this case, canary will look for the current user record in conn.assigns.some_current_user.

The current user key can also be overridden for individual plugs as follows:

plug :load_and_authorize_resource, model: Post, current_user: :current_admin

Specifying resource_name

To specify the name under which the loaded resource is stored, pass the :as flag in the plug declaration.

For example,

plug :load_and_authorize_resource, model: Post, as: :new_post

will load the post into conn.assigns.new_post

Preloading associations

Associations can be preloaded with Repo.preload by passing the :preload option with the name of the association:

plug :load_and_authorize_resource, model: Post, preload: :comments

Non-id actions

For the :index, :new, and :create actions, the resource passed to the Canada.Can implementation should be the module name of the model rather than a struct.

For example, when authorizing access to the Post resource,

you should use

def can?(%User{}, :index, Post), do: true

instead of

def can?(%User{}, :index, %Post{}), do: true

You can specify additional actions for which Canary will authorize based on the model name, by passing the non_id_actions opt to the plug.

For example,

plug :authorize_resource, model: Post, non_id_actions: [:find_by_name]

Implementing Canada.Can for an anonymous user

You may wish to define permissions for when there is no logged in current user (when conn.assigns.current_user is nil). In this case, you should implement Canada.Can for nil like so:

defimpl Canada.Can, for: Atom do
  # When the user is not logged in, all they can do is read Posts
  def can?(nil, :show, %Post{}), do: true
  def can?(nil, _, _), do: false
end

Nested associations

Sometimes you need to load and authorize a parent resource when you have a relationship between two resources and you are creating a new one or listing all the children of that parent. By specifying the :persisted option with true you can load and/or authorize a nested resource. Specifying this option overrides the default loading behavior of the :index, :new, and :create actions by loading an individual resource. It also overrides the default authorization behavior of the :index, :new, and create actions by loading a struct instead of a module name for the call to Canada.can?.

For example, when loading and authorizing a Post resource which can have one or more Comment resources, use

plug :load_and_authorize_resource, model: Post, id_name: "post_id", persisted: true, only: [:create]

to load and authorize the parent Post resource using the post_id in /posts/:post_id/comments before you create the Comment resource using its parent.

Specifing database field

You can tell Canary to search for a resource using a field other than the default :id by using the :id_field option. Note that the specified field must be able to uniquely identify any resource in the specified table.

For example, if you want to access your posts using a string field called slug, you can use

plug :load_and_authorize_resource, model: Post, id_name: "slug", id_field: "slug"

to load and authorize the resource Post with the slug specified by conn.params["slug"] value.

If you are using Phoenix, your web/router.ex should contain something like:

resources "/posts", PostController, param: "slug"

Then your URLs will look like:

/posts/my-new-post

instead of

/posts/1

Handling unauthorized actions

By default, when an action is unauthorized, Canary simply sets conn.assigns.authorized to false. However, you can configure a handler function to be called when authorization fails. Canary will pass the Plug.Conn to the given function. The handler should accept a Plug.Conn as its only argument, and should return a Plug.Conn.

For example, to have Canary call Helpers.handle_unauthorized/1:

config :canary, unauthorized_handler: {Helpers, :handle_unauthorized}

Handling resource not found

By default, when a resource is not found, Canary simply sets the resource in conn.assigns to nil. Like unauthorized action handling , you can configure a function to which Canary will pass the conn when a resource is not found:

config :canary, not_found_handler: {Helpers, :handle_not_found}

You can also specify handlers on an individual basis (which will override the corresponding configured handler, if any) by specifying the corresponding opt in the plug call:

plug :load_and_authorize_resource Post,
  unauthorized_handler: {Helpers, :handle_unauthorized},
  not_found_handler: {Helpers, :handle_not_found}

Tip: If you would like the request handling to stop after the handler function exits, e.g. when redirecting, be sure to call Plug.Conn.halt/1 within your handler like so:

def handle_unauthorized(conn) do
  conn
  |> put_flash(:error, "You can't access that page!")
  |> redirect(to: "/")
  |> halt
end

Note: 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.

License

MIT License. Copyright 2016 Chris Kelly.

canary's People

Contributors

cpjk avatar davydog187 avatar ghatighorias avatar jmufugi avatar jordantdavis-metova avatar juljimm avatar kagemusha avatar lpmi-13 avatar mgwidmann avatar samhamilton avatar shhavel avatar simonprev avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

canary's Issues

Use of non_id_actions: is not documented

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

Should unauthorized_handler really trigger before not_found_handler?

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.

Compatibility with Phoenix 1.4

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?

Extend list of actions that require model name resource

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.

Improve docs

Canary.Plugs should have an @moduledoc block, and the @doc block for each plug should be expanded and updated for readability on hexdocs

Allowing loading of resource on :new and :create actions.

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?

Allow configuration of user object

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

Tries to preload association on non-existant resource

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

Possible solution

Add a guard in preload_if_needed/3 when records is nil.

Adding permissions for nested models

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.

search for a resource using a field other than the default :id and multiple fields are required to define the resource

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

Index action should be more customizable

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.

Conflict with `conn.assigns.action` and unrelated resource

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!

non_id_actions option on Hex

Shouldn't :non_id_actions option be on Hex documentation ? There is a reference on the Readme but nothing on Hex.

Error when building for prod

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.

Update hex.pm and version

Issue:

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)

Suggestion:

Update version in mix.exs and update hex.pm too

load_and_authorize_resource loads the resource twice

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.

Authorize singleton resources

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?

Actions :index, :new, :create passes module, not struct

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?

not_found_handler not work

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?

does canary integrate with guardian db?

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

Compile warnings for elixir 1.4

warning: variable "package" does not exist and is being expanded to "package()", please use parentheses to remove the ambiguity or change the variable name

Abilities for multiple models

Thanks for the lib.

I got a little problem to work with multiple models in me project.

  1. Organization for abilities (not so important)
    I want work with one file for each model abilities. For example: person_abilities.ex and product_abilities.ex.
  2. Really work with multiple models
    Today I have permission for two controllers: 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/a26b60932e2e5b06ea6db1fb4506209f

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

Infer model name unless specified

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.

Version out of date?

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

Update readme

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?

nil comparison forbidden

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.

Readme does not match warning - Anonymous Users

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

Ecto.Query.CastError when :id is not an integer / does not match the field type

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.

nil given for :id

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)

Ability to check permission (using canada) based on parent id

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!

Weird application warnings and sometimes compile issues

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?

The parent nested resource does not handle errors

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.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.