Commit f7e9e2e49d5ed4c39816ead940354ad4e911d378
1 parent
3fb4b9aa
Exists in
master
and in
29 other branches
ActionItem790: don't crash with private profile and ssl disabled
Showing
4 changed files
with
28 additions
and
3 deletions
Show diff stats
app/controllers/application.rb
| @@ -38,8 +38,9 @@ class ApplicationController < ActionController::Base | @@ -38,8 +38,9 @@ class ApplicationController < ActionController::Base | ||
| 38 | redirect_to_ssl | 38 | redirect_to_ssl |
| 39 | end | 39 | end |
| 40 | def redirect_to_ssl | 40 | def redirect_to_ssl |
| 41 | - return true if environment.disable_ssl | 41 | + return false if environment.disable_ssl |
| 42 | redirect_to(params.merge(:protocol => 'https://')) | 42 | redirect_to(params.merge(:protocol => 'https://')) |
| 43 | + true | ||
| 43 | end | 44 | end |
| 44 | 45 | ||
| 45 | def self.refuse_ssl(*options) | 46 | def self.refuse_ssl(*options) |
app/controllers/public/content_viewer_controller.rb
| @@ -29,8 +29,7 @@ class ContentViewerController < ApplicationController | @@ -29,8 +29,7 @@ class ContentViewerController < ApplicationController | ||
| 29 | end | 29 | end |
| 30 | 30 | ||
| 31 | if !@page.public? && !request.ssl? | 31 | if !@page.public? && !request.ssl? |
| 32 | - redirect_to_ssl | ||
| 33 | - return | 32 | + return if redirect_to_ssl |
| 34 | end | 33 | end |
| 35 | 34 | ||
| 36 | if !@page.display_to?(user) | 35 | if !@page.display_to?(user) |
test/functional/application_controller_test.rb
| @@ -356,6 +356,21 @@ class ApplicationControllerTest < Test::Unit::TestCase | @@ -356,6 +356,21 @@ class ApplicationControllerTest < Test::Unit::TestCase | ||
| 356 | assert_redirected_to :x => '1', :y => '1', :protocol => 'https://' | 356 | assert_redirected_to :x => '1', :y => '1', :protocol => 'https://' |
| 357 | end | 357 | end |
| 358 | 358 | ||
| 359 | + should 'return true in redirect_to_ssl' do | ||
| 360 | + env = mock | ||
| 361 | + env.expects(:disable_ssl).returns(false) | ||
| 362 | + @controller.expects(:environment).returns(env) | ||
| 363 | + @controller.expects(:params).returns({}) | ||
| 364 | + @controller.expects(:redirect_to).with({:protocol => 'https://'}) | ||
| 365 | + assert_equal true, @controller.redirect_to_ssl | ||
| 366 | + end | ||
| 367 | + should 'return false in redirect_to_ssl when ssl is disabled' do | ||
| 368 | + env = mock | ||
| 369 | + env.expects(:disable_ssl).returns(true) | ||
| 370 | + @controller.expects(:environment).returns(env) | ||
| 371 | + assert_equal false, @controller.redirect_to_ssl | ||
| 372 | + end | ||
| 373 | + | ||
| 359 | should 'not force ssl when ssl is disabled' do | 374 | should 'not force ssl when ssl is disabled' do |
| 360 | env = Environment.default | 375 | env = Environment.default |
| 361 | env.expects(:disable_ssl).returns(true) | 376 | env.expects(:disable_ssl).returns(true) |
test/functional/content_viewer_controller_test.rb
| @@ -451,4 +451,14 @@ class ContentViewerControllerTest < Test::Unit::TestCase | @@ -451,4 +451,14 @@ class ContentViewerControllerTest < Test::Unit::TestCase | ||
| 451 | assert_no_tag :tag => 'a', :attributes => {:href => ('/myprofile/' + prof.identifier + '/cms/publish/' + page.id.to_s)} | 451 | assert_no_tag :tag => 'a', :attributes => {:href => ('/myprofile/' + prof.identifier + '/cms/publish/' + page.id.to_s)} |
| 452 | end | 452 | end |
| 453 | 453 | ||
| 454 | + should 'deny access before trying SSL when SSL is disabled' do | ||
| 455 | + @controller.expects(:redirect_to_ssl).returns(false) | ||
| 456 | + profile = create_user('testuser').person | ||
| 457 | + profile.public_profile = false | ||
| 458 | + profile.save! | ||
| 459 | + | ||
| 460 | + get :view_page, :profile => 'testuser', :page => profile.home_page.explode_path | ||
| 461 | + assert_response 403 | ||
| 462 | + end | ||
| 463 | + | ||
| 454 | end | 464 | end |