cpjk / canary

:hatching_chick: Elixir authorization and resource-loading library for Plug applications.
MIT License
474 stars 51 forks source link

not_found_handler not work #57

Closed mvalitov closed 7 years ago

mvalitov commented 7 years ago

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?

cpjk commented 7 years ago

Please implement can? for User in /home/mars/phoenix_projects/my_app/lib/abilities.ex.

Looks like the function call is not matching the first implementation of can?

Maybe the action is not in the [:show, :update, :delete] from your guard clause. Try removing the guard clause from can?, or maybe put a binding in there to see what's going on.

mvalitov commented 7 years ago

No, I see in the log that action show

[debug] Processing by MyApp.V1.OperationController.show/2
...
[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.
mvalitov commented 7 years ago

If I remove guard

[debug] Processing by MyApp.V1.OperationController.show/2
  Parameters: %{"id" => "11"}
  Pipelines: [:api]
[debug] QUERY OK source="sessions" db=3.1ms decode=2.0ms
...
[info] Sent 500 in 72ms
[error] #PID<0.381.0> running MyApp.Endpoint terminated
Server: localhost:4000 (http)
Request: GET /api/v1/operations/11
** (exit) an exception was raised:
    ** (FunctionClauseError) no function clause matching in Canada.Can.MyApp.User.can?/3
...
cpjk commented 7 years ago

** (FunctionClauseError) no function clause matching in Canada.Can.MyApp.User.can?/3

It looks like can? is being called, but no function clause is matching.

I would put an IEx.pry in the code were the canary function is being called to see what's going on.

Why do you think this is a problem with not_found_handler ?

mvalitov commented 7 years ago

I think I understand you. No function clause matching. How can I fix this? If I add a function

  def can?(user, action, nil) do
    IO.puts "check ability for nil"
    false
  end

It will always call handle_unauthorized

cpjk commented 7 years ago

It will always call handle_unauthorized

That would happen if that is the only can? function implementation, since can? will then always return false, meaning the user is always unauthorized.

I would try putting an IEx.pry in authorize_resource in canary's code to see what is going on.

cpjk commented 7 years ago

Maybe around here: https://github.com/cpjk/canary/blob/master/lib/canary/plugs.ex#L212

mvalitov commented 7 years ago

That would happen if that is the only can? function implementation, since can? will then always return false, meaning the user is always unauthorized.

No, I meant only the case when the resource was not found

mvalitov commented 7 years ago
pry(2)> resource
nil     
pry(3)> opts
[model: MyApp.Operation, preload: :users, only: [:show, :update, :delete]]

That's what gives IEx.pry

mvalitov commented 7 years ago

From here I do not understand how I need to write a function for this case. If I add this function

  def can?(user, action, nil) do
    IO.puts "check ability for nil"
    false
  end

  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

If the resource is not found, I will be called handle_unauthorized

If I leave only def can?(user, action, operation = %Operation{}) when action in [:show, :update, :delete] do Then I will have an error (FunctionClauseError) no function clause matching in Canada.Can.MyApp.User.can?/3

mvalitov commented 7 years ago

It was necessary to return the true

  def can?(user, action, nil) do
    true
  end

So it works correctly