Commit cfac63da247a50480416991245ffc5cbcaa5027a
1 parent
91226c8b
Exists in
master
and in
29 other branches
ActionItem1165: fixing SSL support
* removing logic inversion in ssl support * using environment's default hostname instead of the request hostname for SSL links
Showing
8 changed files
with
42 additions
and
29 deletions
Show diff stats
app/controllers/application.rb
@@ -50,18 +50,24 @@ class ApplicationController < ActionController::Base | @@ -50,18 +50,24 @@ class ApplicationController < ActionController::Base | ||
50 | redirect_to_ssl | 50 | redirect_to_ssl |
51 | end | 51 | end |
52 | def redirect_to_ssl | 52 | def redirect_to_ssl |
53 | - return false if environment.disable_ssl | ||
54 | - redirect_to(params.merge(:protocol => 'https://')) | ||
55 | - true | 53 | + if environment.enable_ssl |
54 | + redirect_to(params.merge(:protocol => 'https://')) | ||
55 | + true | ||
56 | + else | ||
57 | + false | ||
58 | + end | ||
56 | end | 59 | end |
57 | 60 | ||
58 | def self.refuse_ssl(*options) | 61 | def self.refuse_ssl(*options) |
59 | before_filter :avoid_ssl, *options | 62 | before_filter :avoid_ssl, *options |
60 | end | 63 | end |
61 | def avoid_ssl | 64 | def avoid_ssl |
62 | - return true if (!request.ssl? || ENV['RAILS_ENV'] == 'development') | ||
63 | - redirect_to(params.merge(:protocol => 'http://')) | ||
64 | - false | 65 | + if (!request.ssl? || ENV['RAILS_ENV'] == 'development') |
66 | + true | ||
67 | + else | ||
68 | + redirect_to(params.merge(:protocol => 'http://')) | ||
69 | + false | ||
70 | + end | ||
65 | end | 71 | end |
66 | 72 | ||
67 | before_init_gettext :maybe_save_locale | 73 | before_init_gettext :maybe_save_locale |
app/helpers/application_helper.rb
@@ -792,12 +792,16 @@ module ApplicationHelper | @@ -792,12 +792,16 @@ module ApplicationHelper | ||
792 | 792 | ||
793 | def login_url | 793 | def login_url |
794 | options = Noosfero.url_options.merge({ :controller => 'account', :action => 'login' }) | 794 | options = Noosfero.url_options.merge({ :controller => 'account', :action => 'login' }) |
795 | - if !environment.disable_ssl && (ENV['RAILS_ENV'] != 'development') | ||
796 | - options.merge!(:protocol => 'https://', :host => request.host) | 795 | + if environment.enable_ssl && (ENV['RAILS_ENV'] != 'development') |
796 | + options.merge!(:protocol => 'https://', :host => ssl_hostname) | ||
797 | end | 797 | end |
798 | url_for(options) | 798 | url_for(options) |
799 | end | 799 | end |
800 | 800 | ||
801 | + def ssl_hostname | ||
802 | + environment.default_hostname | ||
803 | + end | ||
804 | + | ||
801 | def base_url | 805 | def base_url |
802 | environment.top_url(request.ssl?) | 806 | environment.top_url(request.ssl?) |
803 | end | 807 | end |
app/models/environment.rb
@@ -513,12 +513,12 @@ class Environment < ActiveRecord::Base | @@ -513,12 +513,12 @@ class Environment < ActiveRecord::Base | ||
513 | result | 513 | result |
514 | end | 514 | end |
515 | 515 | ||
516 | - def disable_ssl | ||
517 | - settings[:disable_ssl] | 516 | + def enable_ssl |
517 | + settings[:enable_ssl] | ||
518 | end | 518 | end |
519 | 519 | ||
520 | - def disable_ssl=(value) | ||
521 | - settings[:disable_ssl] = value | 520 | + def enable_ssl=(value) |
521 | + settings[:enable_ssl] = value | ||
522 | end | 522 | end |
523 | 523 | ||
524 | def to_s | 524 | def to_s |
app/views/layouts/application.rhtml
@@ -87,7 +87,7 @@ | @@ -87,7 +87,7 @@ | ||
87 | :id=>"menu_link_to_envhome", | 87 | :id=>"menu_link_to_envhome", |
88 | :title=>@environment.name %> | 88 | :title=>@environment.name %> |
89 | <% unless environment.enabled?(:disable_categories) %> | 89 | <% unless environment.enabled?(:disable_categories) %> |
90 | - <% cache(environment.name.id.to_s + '_categories_menu') do %> | 90 | + <% cache(environment.id.to_s + '_categories_menu') do %> |
91 | <%= render :file => 'shared/categories_menu' %> | 91 | <%= render :file => 'shared/categories_menu' %> |
92 | <% end %> | 92 | <% end %> |
93 | <% end %> | 93 | <% end %> |
test/functional/application_controller_test.rb
@@ -267,6 +267,7 @@ class ApplicationControllerTest < Test::Unit::TestCase | @@ -267,6 +267,7 @@ class ApplicationControllerTest < Test::Unit::TestCase | ||
267 | end | 267 | end |
268 | 268 | ||
269 | should 'require ssl when told to' do | 269 | should 'require ssl when told to' do |
270 | + Environment.default.update_attribute(:enable_ssl, true) | ||
270 | @request.expects(:ssl?).returns(false).at_least_once | 271 | @request.expects(:ssl?).returns(false).at_least_once |
271 | get :sslonly | 272 | get :sslonly |
272 | assert_redirected_to :protocol => 'https://' | 273 | assert_redirected_to :protocol => 'https://' |
@@ -292,6 +293,7 @@ class ApplicationControllerTest < Test::Unit::TestCase | @@ -292,6 +293,7 @@ class ApplicationControllerTest < Test::Unit::TestCase | ||
292 | end | 293 | end |
293 | 294 | ||
294 | should 'keep arguments when redirecting to ssl' do | 295 | should 'keep arguments when redirecting to ssl' do |
296 | + Environment.default.update_attribute(:enable_ssl, true) | ||
295 | @request.expects(:ssl?).returns(false).at_least_once | 297 | @request.expects(:ssl?).returns(false).at_least_once |
296 | get :sslonly, :x => '1', :y => '2' | 298 | get :sslonly, :x => '1', :y => '2' |
297 | assert_redirected_to :protocol => 'https://', :x => '1', :y => '2' | 299 | assert_redirected_to :protocol => 'https://', :x => '1', :y => '2' |
@@ -327,13 +329,14 @@ class ApplicationControllerTest < Test::Unit::TestCase | @@ -327,13 +329,14 @@ class ApplicationControllerTest < Test::Unit::TestCase | ||
327 | end | 329 | end |
328 | 330 | ||
329 | should 'add https protocols on redirect_to_ssl' do | 331 | should 'add https protocols on redirect_to_ssl' do |
332 | + Environment.default.update_attribute(:enable_ssl, true) | ||
330 | get :sslonly, :x => '1', :y => '1' | 333 | get :sslonly, :x => '1', :y => '1' |
331 | assert_redirected_to :x => '1', :y => '1', :protocol => 'https://' | 334 | assert_redirected_to :x => '1', :y => '1', :protocol => 'https://' |
332 | end | 335 | end |
333 | 336 | ||
334 | should 'return true in redirect_to_ssl' do | 337 | should 'return true in redirect_to_ssl' do |
335 | env = mock | 338 | env = mock |
336 | - env.expects(:disable_ssl).returns(false) | 339 | + env.expects(:enable_ssl).returns(true) |
337 | @controller.expects(:environment).returns(env) | 340 | @controller.expects(:environment).returns(env) |
338 | @controller.expects(:params).returns({}) | 341 | @controller.expects(:params).returns({}) |
339 | @controller.expects(:redirect_to).with({:protocol => 'https://'}) | 342 | @controller.expects(:redirect_to).with({:protocol => 'https://'}) |
@@ -341,14 +344,14 @@ class ApplicationControllerTest < Test::Unit::TestCase | @@ -341,14 +344,14 @@ class ApplicationControllerTest < Test::Unit::TestCase | ||
341 | end | 344 | end |
342 | should 'return false in redirect_to_ssl when ssl is disabled' do | 345 | should 'return false in redirect_to_ssl when ssl is disabled' do |
343 | env = mock | 346 | env = mock |
344 | - env.expects(:disable_ssl).returns(true) | 347 | + env.expects(:enable_ssl).returns(false) |
345 | @controller.expects(:environment).returns(env) | 348 | @controller.expects(:environment).returns(env) |
346 | assert_equal false, @controller.redirect_to_ssl | 349 | assert_equal false, @controller.redirect_to_ssl |
347 | end | 350 | end |
348 | 351 | ||
349 | should 'not force ssl when ssl is disabled' do | 352 | should 'not force ssl when ssl is disabled' do |
350 | env = Environment.default | 353 | env = Environment.default |
351 | - env.expects(:disable_ssl).returns(true) | 354 | + env.expects(:enable_ssl).returns(false) |
352 | @controller.stubs(:environment).returns(env) | 355 | @controller.stubs(:environment).returns(env) |
353 | @request.expects(:ssl?).returns(false).at_least_once | 356 | @request.expects(:ssl?).returns(false).at_least_once |
354 | get :sslonly | 357 | get :sslonly |
test/integration/login_to_the_application_test.rb
@@ -20,7 +20,7 @@ class LoginToTheApplicationTest < ActionController::IntegrationTest | @@ -20,7 +20,7 @@ class LoginToTheApplicationTest < ActionController::IntegrationTest | ||
20 | end | 20 | end |
21 | 21 | ||
22 | def test_unauthenticated_user_tries_to_access_his_control_panel | 22 | def test_unauthenticated_user_tries_to_access_his_control_panel |
23 | - Environment.any_instance.stubs(:disable_ssl).returns(true) # ignore SSL for this test | 23 | + Environment.any_instance.stubs(:enable_ssl).returns(false) # ignore SSL for this test |
24 | 24 | ||
25 | get '/myprofile/ze' | 25 | get '/myprofile/ze' |
26 | assert_redirected_to '/account/login' | 26 | assert_redirected_to '/account/login' |
test/unit/application_helper_test.rb
@@ -195,19 +195,19 @@ class ApplicationHelperTest < Test::Unit::TestCase | @@ -195,19 +195,19 @@ class ApplicationHelperTest < Test::Unit::TestCase | ||
195 | end | 195 | end |
196 | 196 | ||
197 | should 'use https:// for login_url' do | 197 | should 'use https:// for login_url' do |
198 | - environment = mock | ||
199 | - environment.expects(:disable_ssl).returns(false) | 198 | + environment = Environment.default |
199 | + environment.update_attribute(:enable_ssl, true) | ||
200 | + environment.domains << Domain.new(:name => "test.domain.net", :is_default => true) | ||
200 | stubs(:environment).returns(environment) | 201 | stubs(:environment).returns(environment) |
201 | - request = mock | ||
202 | - request.stubs(:host).returns('myhost.net') | ||
203 | - stubs(:request).returns(request) | ||
204 | - stubs(:url_for).with(has_entries(:protocol => 'https://', :host => 'myhost.net')).returns('LALALA') | 202 | + |
203 | + stubs(:url_for).with(has_entries(:protocol => 'https://', :host => 'test.domain.net')).returns('LALALA') | ||
204 | + | ||
205 | assert_equal 'LALALA', login_url | 205 | assert_equal 'LALALA', login_url |
206 | end | 206 | end |
207 | 207 | ||
208 | should 'not force ssl in login_url when environment has ssl disabled' do | 208 | should 'not force ssl in login_url when environment has ssl disabled' do |
209 | environment = mock | 209 | environment = mock |
210 | - environment.expects(:disable_ssl).returns(true).at_least_once | 210 | + environment.expects(:enable_ssl).returns(false).at_least_once |
211 | stubs(:environment).returns(environment) | 211 | stubs(:environment).returns(environment) |
212 | request = mock | 212 | request = mock |
213 | request.stubs(:host).returns('localhost') | 213 | request.stubs(:host).returns('localhost') |
test/unit/environment_test.rb
@@ -487,14 +487,14 @@ class EnvironmentTest < Test::Unit::TestCase | @@ -487,14 +487,14 @@ class EnvironmentTest < Test::Unit::TestCase | ||
487 | assert_equal template, e.enterprise_template | 487 | assert_equal template, e.enterprise_template |
488 | end | 488 | end |
489 | 489 | ||
490 | - should 'not disable ssl by default' do | 490 | + should 'not enable ssl by default' do |
491 | e = Environment.new | 491 | e = Environment.new |
492 | - assert !e.disable_ssl | 492 | + assert !e.enable_ssl |
493 | end | 493 | end |
494 | 494 | ||
495 | - should 'be able to disable ssl' do | ||
496 | - e = Environment.new(:disable_ssl => true) | ||
497 | - assert_equal true, e.disable_ssl | 495 | + should 'be able to enable ssl' do |
496 | + e = Environment.new(:enable_ssl => true) | ||
497 | + assert_equal true, e.enable_ssl | ||
498 | end | 498 | end |
499 | 499 | ||
500 | should 'have a layout template' do | 500 | should 'have a layout template' do |