cpjk / canary

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

Add ability to check permission based on pid #26

Closed timoteus closed 8 years ago

timoteus commented 8 years ago

Ability to base Canada.Can permission check on parent resource, i.e., can user create action based on the resource action is a sub-resource of.

Adds opts[:parent_to] and relies on ability to specify name of id (using opts[:id_name] see https://github.com/cpjk/canary/pull/22).

Resolves https://github.com/cpjk/canary/issues/23

(I do understand if this is not something you want to support.)

timoteus commented 8 years ago

I may have a better solution than this.

cpjk commented 8 years ago

The way this is worded is kind of confusing to me. In particular, the fact that we are really authorizing for the child resource, but the model as stated in the plug opts is the parent. I think it would make more sense from a user perspective to have a parent opt rather than parent_to. I think this would make it more clear which resource is being authorized for.

For example, I like plug :authorize_resource, model: Post, parent: Thread, parent_id_name: thread_id, which authorizes for Post based on it's parent, instead of plug :authorize_resource, model: Thread, parent_to: Post, id_name: thread_id, which authorizes for a Post as well.

I think the first example makes it clear that the plug is authorizing the action on the Post resource rather than the Thread resource.

I also think that it might be more clear how to declare the rules if the resource that canary passes to Canada.Can.can?/3 was be %{child, parent: parent}, rather than %{child, parent}.

cpjk commented 8 years ago

You mentioned that you might have a better solution. If you still think it is a good solution, would you care to elaborate?

cpjk commented 8 years ago

@timoteus I'm just wondering what your status is with respect to this PR. :smiley:

cpjk commented 8 years ago

Closing as this PR has been inactive for around 6 months