cpjk / canary

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

Abilities for multiple models #52

Open edmaarcosta opened 7 years ago

edmaarcosta commented 7 years ago

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.

kelvinst commented 7 years ago

Well, here we go:

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.

Isn't that possible? Just create the multiple files and define each protocol implementation on the separated files.

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.

Well, if the resource is not found, shouldn't you return a 404?

edmaarcosta commented 7 years ago

Isn't that possible? Just create the multiple files and define each protocol implementation on the separated files.

I think not. The defimpl accept in for, in my case, CompanyMember that is the first argument in can? definition and that's it.

Well, if the resource is not found, shouldn't you return a 404?

Not really. Because the lib return not authorized in this case. Unless I return true in can? definition.

Am I missing something?

kelvinst commented 7 years ago

I think not. The defimpl accept in for, in my case, CompanyMember that is the first argument in can? definition and that's it.

Well, I didn't understand why not. If you have person, company and company_member, you can define defimpl Canada.Can, for: Person in one file, defimpl Canada.Can, for: Company in another and defimpl Canada.Can, for: CompanyMember in another. Or that's not what you asked for?

Not really. Because the lib return not authorized in this case

What I tried to say is that, you can check the resource existence first, and if it does not exist, just return a 404, and only if it exists, run the canary authorize.

codecakes commented 7 years ago

@kelvinst cc: @cpjk

I think not. The defimpl accept in for, in my case, CompanyMember that is the first argument in can? definition and that's it.

Well, I didn't understand why not. If you have person, company and company_member, you can define defimpl Canada.Can, for: Person in one file, defimpl Canada.Can, for: Company in another and defimpl Canada.Can, for: CompanyMember in another. Or that's not what you asked for?

I did the same but as @edmaarcosta mentions, it's only taking in the first file. Lets say all the abilities file are in /lib/abilities/ like:

In each file, the plug is defined as

plug :load_and_authorize_resource, model: <ThatModel>, :whatever_opts

But while the api hits right, the plug it goes to is the user_abilities.ex if that's defined first.

IMP NOTE : However! it doesn't matter what model: I fill in, for multiple models, if I put the same can? function inside the first file i.e. user_abilities.ex regardless, it works.

So maybe i should consolidate all multiple model abilities file into a single abilities.ex ? so the defimpl for: User or defimpl for: Company seems irrelevant there as it's taking any and all models provided a valid can? function is provided.

julien-leclercq commented 7 years ago

The (non really satisfying) I had for this was to wright a abilities.ex file which implement Canada.Can, for: User then I wrote a bunch of resource1_abilities.ex files with the can? function implementation and add in my Canada.Can implementation appropriate pattern matching on each resource to dispatch the function calls in the appropriate module if you have a more elegant solution, I'll be happy to know !

  defmodule Resource1Abilities do
    def can?(%User{} = user, action, %Resource1{} = resource1) do
      ability_logic(user, action, resource)
    end
  end 

  defmodule Resource2Abilities do
    def can?(%User{} = user, action, %Resource2{} = resource1) do
      ability_logic(user, action, resource)
    end
  end 

  defimpl Canada.Can, for: User do
    def can?(%User{} = user, action, %Resource1{} = resource1) do
      Resource1Abilities.can?(user, action, resource1)
    end

    def can?(%User{} = user, action, %Resource2{} = resource2) do
      Resource2Abilities.can?(user, action, resource2)
    end
  end 
bjunc commented 6 years ago

In the little spare time I have these days, I've been working on a different spin to authorization logic; which I call Access Decision Manager.

Instead of "abilities", it uses "voters". The voters take the subjects and attributes (aka "actions") and decide whether or not to :access_abstain from voting, or return :access_denied, or :access_granted. You can pattern match the vote function (or within it); which I think could be what's needed in the scenario above (not 100% sure).

Curious if anyone has any thoughts on it. Still very alpha, but maybe it'd help you out.