Commit bf8cce15ad642444772e447fb3ec516423321d23
Committed by
Antonio Terceiro
1 parent
226e4e28
Exists in
master
and in
22 other branches
ActionItem890: "access denied" page is too ugly
Showing
9 changed files
with
41 additions
and
24 deletions
Show diff stats
app/controllers/application.rb
| @@ -96,6 +96,11 @@ class ApplicationController < ActionController::Base | @@ -96,6 +96,11 @@ class ApplicationController < ActionController::Base | ||
| 96 | render :template => 'shared/not_found.rhtml', :status => 404 | 96 | render :template => 'shared/not_found.rhtml', :status => 404 |
| 97 | end | 97 | end |
| 98 | 98 | ||
| 99 | + def render_access_denied(message = nil) | ||
| 100 | + @message = message | ||
| 101 | + render :template => 'shared/access_denied.rhtml', :status => 403 | ||
| 102 | + end | ||
| 103 | + | ||
| 99 | def user | 104 | def user |
| 100 | current_user.person if logged_in? | 105 | current_user.person if logged_in? |
| 101 | end | 106 | end |
app/controllers/my_profile_controller.rb
| @@ -18,8 +18,7 @@ class MyProfileController < ApplicationController | @@ -18,8 +18,7 @@ class MyProfileController < ApplicationController | ||
| 18 | def self.requires_profile_class(some_class) | 18 | def self.requires_profile_class(some_class) |
| 19 | before_filter do |controller| | 19 | before_filter do |controller| |
| 20 | unless controller.send(:profile).kind_of?(some_class) | 20 | unless controller.send(:profile).kind_of?(some_class) |
| 21 | - controller.instance_variable_set('@message', _("This action is not available for \"%s\".") % controller.send(:profile).name) | ||
| 22 | - controller.send(:render, :file => File.join(RAILS_ROOT, 'app', 'views', 'shared', 'access_denied.rhtml'), :layout => true, :status => 403) | 21 | + controller.send(:render_access_denied, _("This action is not available for \"%s\".") % controller.send(:profile).name) |
| 23 | end | 22 | end |
| 24 | end | 23 | end |
| 25 | end | 24 | end |
app/controllers/public/content_viewer_controller.rb
| @@ -49,8 +49,7 @@ class ContentViewerController < ApplicationController | @@ -49,8 +49,7 @@ class ContentViewerController < ApplicationController | ||
| 49 | end | 49 | end |
| 50 | 50 | ||
| 51 | if !@page.display_to?(user) | 51 | if !@page.display_to?(user) |
| 52 | - # FIXME find a nice "access denied" layout | ||
| 53 | - render :action => 'access_denied', :status => 403, :layout => false | 52 | + render_access_denied(_('You are not allowed to view this content. You can contact the owner of this profile to request access then.')) |
| 54 | end | 53 | end |
| 55 | 54 | ||
| 56 | # At this point the page will be showed | 55 | # At this point the page will be showed |
app/views/content_viewer/access_denied.rhtml
app/views/shared/access_denied.rhtml
| 1 | -<h2> <%= _('Access denied') %> </h2> | 1 | +<div id='access-denied'> |
| 2 | 2 | ||
| 3 | -<% unless @message.nil? %> | ||
| 4 | - <p> | ||
| 5 | - <%= @message %> | ||
| 6 | - </p> | ||
| 7 | -<% end %> | 3 | + <h1> <%= _('Access denied') %> </h1> |
| 4 | + | ||
| 5 | + <% unless @message.nil? %> | ||
| 6 | + <p><%= @message %></p> | ||
| 7 | + <% else %> | ||
| 8 | + <p><%= _('You are not allowed to view this page.') %></p> | ||
| 9 | + <% end %> | ||
| 10 | + | ||
| 11 | + <ul> | ||
| 12 | + <li><%= link_to _('Go to the site home page'), :controller => 'home' %></li> | ||
| 13 | + <li><%= link_to _('Go back'), :back %></li> | ||
| 14 | + </ul> | ||
| 15 | + | ||
| 16 | +</div> |
public/stylesheets/common.css
| @@ -419,14 +419,16 @@ div.pending-tasks { | @@ -419,14 +419,16 @@ div.pending-tasks { | ||
| 419 | margin: 1em; | 419 | margin: 1em; |
| 420 | } | 420 | } |
| 421 | 421 | ||
| 422 | -#content #not-found { | 422 | +#content #not-found, |
| 423 | +#content #access-denied { | ||
| 423 | padding: 20px; | 424 | padding: 20px; |
| 424 | margin: 20px; | 425 | margin: 20px; |
| 425 | border: 1px solid #DDD; | 426 | border: 1px solid #DDD; |
| 426 | -moz-border-radius: 6px; | 427 | -moz-border-radius: 6px; |
| 427 | } | 428 | } |
| 428 | 429 | ||
| 429 | -#content #not-found h1 { | 430 | +#content #not-found h1, |
| 431 | +#content #access-denied h1 { | ||
| 430 | text-align: left; | 432 | text-align: left; |
| 431 | background: url(../images/icons-app/alert-icon.png) no-repeat; | 433 | background: url(../images/icons-app/alert-icon.png) no-repeat; |
| 432 | padding-left: 30px; | 434 | padding-left: 30px; |
test/functional/content_viewer_controller_test.rb
| @@ -410,7 +410,7 @@ class ContentViewerControllerTest < Test::Unit::TestCase | @@ -410,7 +410,7 @@ class ContentViewerControllerTest < Test::Unit::TestCase | ||
| 410 | @request.stubs(:ssl?).returns(true) | 410 | @request.stubs(:ssl?).returns(true) |
| 411 | get :view_page, :profile => 'test_profile', :page => [ 'my-intranet' ] | 411 | get :view_page, :profile => 'test_profile', :page => [ 'my-intranet' ] |
| 412 | 412 | ||
| 413 | - assert_template 'access_denied' | 413 | + assert_template 'access_denied.rhtml' |
| 414 | end | 414 | end |
| 415 | 415 | ||
| 416 | should 'not give access to private articles if logged in but not member' do | 416 | should 'not give access to private articles if logged in but not member' do |
| @@ -421,7 +421,7 @@ class ContentViewerControllerTest < Test::Unit::TestCase | @@ -421,7 +421,7 @@ class ContentViewerControllerTest < Test::Unit::TestCase | ||
| 421 | @request.stubs(:ssl?).returns(true) | 421 | @request.stubs(:ssl?).returns(true) |
| 422 | get :view_page, :profile => 'test_profile', :page => [ 'my-intranet' ] | 422 | get :view_page, :profile => 'test_profile', :page => [ 'my-intranet' ] |
| 423 | 423 | ||
| 424 | - assert_template 'access_denied' | 424 | + assert_template 'access_denied.rhtml' |
| 425 | end | 425 | end |
| 426 | 426 | ||
| 427 | should 'give access to private articles if logged in and member' do | 427 | should 'give access to private articles if logged in and member' do |
vendor/plugins/access_control/lib/permission_check.rb
| @@ -21,16 +21,18 @@ module PermissionCheck | @@ -21,16 +21,18 @@ module PermissionCheck | ||
| 21 | accessor = accessor_method.kind_of?(Symbol) ? c.send(accessor_method) : accessor_method | 21 | accessor = accessor_method.kind_of?(Symbol) ? c.send(accessor_method) : accessor_method |
| 22 | unless accessor && accessor.has_permission?(permission.to_s, target) | 22 | unless accessor && accessor.has_permission?(permission.to_s, target) |
| 23 | # c.instance_variable_set('@b', [accessor, permission, target]) | 23 | # c.instance_variable_set('@b', [accessor, permission, target]) |
| 24 | - c.send(:render, :file => access_denied_template_path, :status => 403) && false | 24 | + c.send(:render, :template => access_denied_template_path, :status => 403) && false |
| 25 | end | 25 | end |
| 26 | end | 26 | end |
| 27 | end | 27 | end |
| 28 | 28 | ||
| 29 | def access_denied_template_path | 29 | def access_denied_template_path |
| 30 | - if File.exists?(File.join(RAILS_ROOT, 'app', 'views','access_control' ,'access_denied.rhtml')) | ||
| 31 | - file_path = File.join(RAILS_ROOT, 'app', 'views','access_control' ,'access_denied.rhtml') | 30 | + if File.exists?(File.join(RAILS_ROOT, 'app', 'views', 'access_control', 'access_denied.rhtml')) |
| 31 | + File.join(RAILS_ROOT, 'app', 'views', 'access_control', 'access_denied.rhtml') | ||
| 32 | + elsif File.exists?(File.join(RAILS_ROOT, 'app','views', 'shared', 'access_denied.rhtml')) | ||
| 33 | + File.join('shared', 'access_denied.rhtml') | ||
| 32 | else | 34 | else |
| 33 | - file_path = File.join(File.dirname(__FILE__),'..', 'views','access_denied.rhtml') | 35 | + File.join(File.dirname(__FILE__), '..', 'views', 'access_denied.rhtml') |
| 34 | end | 36 | end |
| 35 | end | 37 | end |
| 36 | end | 38 | end |
vendor/plugins/access_control/test/permission_check_test.rb
| @@ -36,6 +36,12 @@ class PermissionCheckTest < Test::Unit::TestCase | @@ -36,6 +36,12 @@ class PermissionCheckTest < Test::Unit::TestCase | ||
| 36 | get :other_stuff, :user => user.id, :resource => resource.id | 36 | get :other_stuff, :user => user.id, :resource => resource.id |
| 37 | assert_response :success | 37 | assert_response :success |
| 38 | assert_template nil | 38 | assert_template nil |
| 39 | - | ||
| 40 | end | 39 | end |
| 40 | + | ||
| 41 | + def test_try_render_shared_access_denied_view | ||
| 42 | + File.expects(:exists?).with(File.join(RAILS_ROOT, 'app', 'views', 'access_control', 'access_denied.rhtml')) | ||
| 43 | + File.expects(:exists?).with(File.join(RAILS_ROOT, 'app', 'views', 'shared', 'access_denied.rhtml')) | ||
| 44 | + AccessControlTestController.access_denied_template_path | ||
| 45 | + end | ||
| 46 | + | ||
| 41 | end | 47 | end |