activeadmin / activeadmin

The administration framework for Ruby on Rails applications.
https://activeadmin.info
MIT License
9.5k stars 3.31k forks source link

Should we differentiate authorization for new and create? #3802

Closed bamorim closed 1 year ago

bamorim commented 9 years ago

(This is more like a discussion than an issue. :smile:)

Well, I'm using Pundit for my authorization layer and I have some issues (not critical, easy workaround).

The question is whether we should have different actions for new and create. Let me exemplify:

A Theme has many Subjects An User can be the coordinator of a given Theme User can create a Subject only for the Theme he is the coordinator.

If I were impementing my own admin, I'd create a Pundit Policy like this:

class SubjectPolicy < ApplicationPolicy
  def new?
    # Every user should be able to reach the new page
    true
  end

  def create?
    # However, the user can only create subjects belonging to themes he is the coordinator
    record.theme.coordinator_id == user.id
  end
end

But, ActiveAdmin uses the create? for showing the new form as well for creating the subject itself.

The workaround I did was checking whether the theme was present:

  def create?
    user.admin? or
      record.theme.nil? or
      record.theme.coordinator_id == user.id
  end

What you guys think? If you'd agree I can make some effort to create a PR for this.

Thank you for your attention. :smile:

timoschilling commented 9 years ago

double of / related to #3405

timoschilling commented 9 years ago

Feel free to create a PR

bamorim commented 9 years ago

@timoschilling Okay, I'll create a Pull Request, but do you think that should be backwards compatible?

I can try to call new? and if it's not present then call create? Or maybe that should be a configuration.

What do you think?

timoschilling commented 9 years ago

The next release will be a 1.0.0, so we can break thinks.

finishingmove commented 9 years ago

I just think that new? and create? should not be connected in any way by default. The same for edit? and update?. It would be easy enough for the end-users to reproduce the current behavior if they wanted it, but the way it is now, it does more bad than good.

gkop commented 8 years ago

I have a (much) more specific related issue, which is that the Pundit adapter is not aware of belongs_to when initializing the policy; see https://github.com/activeadmin/activeadmin/blob/d01155f73a0e2cc27d712792270357e2b3759d05/lib/active_admin/pundit_adapter.rb#L34 , because it doesn't passing the "parent" record in the belongs to association, at subject.new, I am unable to configure authorization in Pundit in context of the parent record.

wvteijlingen commented 7 years ago

I'm running into exactly the same issue. What is the status on this? Was there any progress made on a PR?