AccessDenied exception params #40

Closed
fastcatch opened this Issue Feb 21, 2010 · 9 comments

Comments

Projects
None yet
4 participants
@fastcatch

It would be quite convenient to have the denied controller & action names in the exception, so that it would be

  • possile to log what failed
  • let the user know what failed exactly
  • and apply proper return_to routing

While the first two points seem to be also addressed by #33 (http://github.com/ryanb/cancan/issues/#issue/33) -- albeit not fully, the thrid one remains entirely on my wish-list.

@shetler

This comment has been minimized.

Show comment Hide comment
@shetler

shetler Feb 22, 2010

I second the ability to specify a redirect! The only thing this plugin is missing is the ability to recover from AccessDenied by being able to redirect to a specific page. I don't just mean overriding rescue_from in the controller.

Say you have a controller that has an Admin-only action and other actions that only logged-in users can access. If a non-Admin tries to access the admin only action, it should redirect to a 404, if a non-logged in user tries to access one of the other actions it should redirect to some other page.

shetler commented Feb 22, 2010

I second the ability to specify a redirect! The only thing this plugin is missing is the ability to recover from AccessDenied by being able to redirect to a specific page. I don't just mean overriding rescue_from in the controller.

Say you have a controller that has an Admin-only action and other actions that only logged-in users can access. If a non-Admin tries to access the admin only action, it should redirect to a 404, if a non-logged in user tries to access one of the other actions it should redirect to some other page.

@shetler

This comment has been minimized.

Show comment Hide comment
@shetler

shetler Feb 22, 2010

IE:

in my app's Ability.rb:

if user.role != :guest
can :edit, User do |requested_user|
requested_user && requested_user == user
end
end

recover_from :edit, User, :show, :users, true do |requested_user|
requested_user
end

here I am specify with recover_from that if the test on can :edit, User fails, it should be caught by recover_from and redirected to :controller => :users, :action => :show with an :id => of the failed noun (the indicated by the 'true') is the block evaluates to be true.

shetler commented Feb 22, 2010

IE:

in my app's Ability.rb:

if user.role != :guest
can :edit, User do |requested_user|
requested_user && requested_user == user
end
end

recover_from :edit, User, :show, :users, true do |requested_user|
requested_user
end

here I am specify with recover_from that if the test on can :edit, User fails, it should be caught by recover_from and redirected to :controller => :users, :action => :show with an :id => of the failed noun (the indicated by the 'true') is the block evaluates to be true.

@shetler

This comment has been minimized.

Show comment Hide comment
@shetler

shetler Feb 27, 2010

I've made a fork to address this issue: http://github.com/kshet26/cancan. Basically it allows you to recover from the exception with:

rescue_from CanCan::AccessDenied, :with => :access_denied

def access_denied(exception)
return redirect_to :action => :show, :id => exception.denied_noun if exception.action == :edit
end

shetler commented Feb 27, 2010

I've made a fork to address this issue: http://github.com/kshet26/cancan. Basically it allows you to recover from the exception with:

rescue_from CanCan::AccessDenied, :with => :access_denied

def access_denied(exception)
return redirect_to :action => :show, :id => exception.denied_noun if exception.action == :edit
end

@ryanb

This comment has been minimized.

Show comment Hide comment
@ryanb

ryanb Apr 16, 2010

Owner

It makes sense for the AccessDenied exception to hold as much information as it can. I'll work on getting this in.

Owner

ryanb commented Apr 16, 2010

It makes sense for the AccessDenied exception to hold as much information as it can. I'll work on getting this in.

@ryanb

This comment has been minimized.

Show comment Hide comment
@ryanb

ryanb Apr 16, 2010

Owner

removing unauthorized! in favor of authorize! and including more information in AccessDenied exception - closed by 8903fee

Owner

ryanb commented Apr 16, 2010

removing unauthorized! in favor of authorize! and including more information in AccessDenied exception - closed by 8903fee

@junaid

This comment has been minimized.

Show comment Hide comment
@junaid

junaid Jun 23, 2010

Hi kshet26 and ryanb,

I think it would be more simpler if we could give redirect url and message with the ability like this

can :index, Tender, :message => "Only logged_in user can see this page.", redirect_url =>root_path

junaid commented Jun 23, 2010

Hi kshet26 and ryanb,

I think it would be more simpler if we could give redirect url and message with the ability like this

can :index, Tender, :message => "Only logged_in user can see this page.", redirect_url =>root_path

@junaid

This comment has been minimized.

Show comment Hide comment
@junaid

junaid Jul 5, 2010

I have added this feature and also sent request to plugin author to review it. If you want same functionality then you can try out http://github.com/junaid/cancan, you can check related example here http://wiki.github.com/junaid/cancan/.

Junaid

junaid commented Jul 5, 2010

I have added this feature and also sent request to plugin author to review it. If you want same functionality then you can try out http://github.com/junaid/cancan, you can check related example here http://wiki.github.com/junaid/cancan/.

Junaid

@ryanb

This comment has been minimized.

Show comment Hide comment
@ryanb

ryanb Jul 16, 2010

Owner

@junaid, do you mean as options to the can definitions in the Ability class? I think the redirect_url lies outside of the scope of the Ability class since it is at the model layer. However I could see customizing the messages in that layer. Please add a separate ticket on the messages issue if you are still interested in having it pulled in. Thanks for your feedback!

Owner

ryanb commented Jul 16, 2010

@junaid, do you mean as options to the can definitions in the Ability class? I think the redirect_url lies outside of the scope of the Ability class since it is at the model layer. However I could see customizing the messages in that layer. Please add a separate ticket on the messages issue if you are still interested in having it pulled in. Thanks for your feedback!

@junaid

This comment has been minimized.

Show comment Hide comment
@junaid

junaid Jul 24, 2010

Hi Ryan,

yes i want to add redirect_url option for each can definition so that if some thing goes wrong then we can redirect user to the given redirect_url and this way we can give all related things in ability file otherwise we have to have checks in global exception handler of cancan in application controller file like if action is 'A' and controller is 'B' then redirect on 'C' page and so on. Could you please have a look on the code i implemented for this feature. Yes i will add a ticket for custom message option for each can definition. Thanks Ryan.

BR
Junaid

junaid commented Jul 24, 2010

Hi Ryan,

yes i want to add redirect_url option for each can definition so that if some thing goes wrong then we can redirect user to the given redirect_url and this way we can give all related things in ability file otherwise we have to have checks in global exception handler of cancan in application controller file like if action is 'A' and controller is 'B' then redirect on 'C' page and so on. Could you please have a look on the code i implemented for this feature. Yes i will add a ticket for custom message option for each can definition. Thanks Ryan.

BR
Junaid

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment