From cfac63da247a50480416991245ffc5cbcaa5027a Mon Sep 17 00:00:00 2001 From: Antonio Terceiro Date: Sat, 1 Aug 2009 11:43:32 -0300 Subject: [PATCH] ActionItem1165: fixing SSL support --- app/controllers/application.rb | 18 ++++++++++++------ app/helpers/application_helper.rb | 8 ++++++-- app/models/environment.rb | 8 ++++---- app/views/layouts/application.rhtml | 2 +- test/functional/application_controller_test.rb | 9 ++++++--- test/integration/login_to_the_application_test.rb | 2 +- test/unit/application_helper_test.rb | 14 +++++++------- test/unit/environment_test.rb | 10 +++++----- 8 files changed, 42 insertions(+), 29 deletions(-) diff --git a/app/controllers/application.rb b/app/controllers/application.rb index ca79db1..cc3b22d 100644 --- a/app/controllers/application.rb +++ b/app/controllers/application.rb @@ -50,18 +50,24 @@ class ApplicationController < ActionController::Base redirect_to_ssl end def redirect_to_ssl - return false if environment.disable_ssl - redirect_to(params.merge(:protocol => 'https://')) - true + if environment.enable_ssl + redirect_to(params.merge(:protocol => 'https://')) + true + else + false + end end def self.refuse_ssl(*options) before_filter :avoid_ssl, *options end def avoid_ssl - return true if (!request.ssl? || ENV['RAILS_ENV'] == 'development') - redirect_to(params.merge(:protocol => 'http://')) - false + if (!request.ssl? || ENV['RAILS_ENV'] == 'development') + true + else + redirect_to(params.merge(:protocol => 'http://')) + false + end end before_init_gettext :maybe_save_locale diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 1df5a31..22deef5 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -792,12 +792,16 @@ module ApplicationHelper def login_url options = Noosfero.url_options.merge({ :controller => 'account', :action => 'login' }) - if !environment.disable_ssl && (ENV['RAILS_ENV'] != 'development') - options.merge!(:protocol => 'https://', :host => request.host) + if environment.enable_ssl && (ENV['RAILS_ENV'] != 'development') + options.merge!(:protocol => 'https://', :host => ssl_hostname) end url_for(options) end + def ssl_hostname + environment.default_hostname + end + def base_url environment.top_url(request.ssl?) end diff --git a/app/models/environment.rb b/app/models/environment.rb index 048d193..b1a957a 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -513,12 +513,12 @@ class Environment < ActiveRecord::Base result end - def disable_ssl - settings[:disable_ssl] + def enable_ssl + settings[:enable_ssl] end - def disable_ssl=(value) - settings[:disable_ssl] = value + def enable_ssl=(value) + settings[:enable_ssl] = value end def to_s diff --git a/app/views/layouts/application.rhtml b/app/views/layouts/application.rhtml index 23ac645..1f4fd5c 100644 --- a/app/views/layouts/application.rhtml +++ b/app/views/layouts/application.rhtml @@ -87,7 +87,7 @@ :id=>"menu_link_to_envhome", :title=>@environment.name %> <% unless environment.enabled?(:disable_categories) %> - <% cache(environment.name.id.to_s + '_categories_menu') do %> + <% cache(environment.id.to_s + '_categories_menu') do %> <%= render :file => 'shared/categories_menu' %> <% end %> <% end %> diff --git a/test/functional/application_controller_test.rb b/test/functional/application_controller_test.rb index 4ef9df8..c8d976d 100644 --- a/test/functional/application_controller_test.rb +++ b/test/functional/application_controller_test.rb @@ -267,6 +267,7 @@ class ApplicationControllerTest < Test::Unit::TestCase end should 'require ssl when told to' do + Environment.default.update_attribute(:enable_ssl, true) @request.expects(:ssl?).returns(false).at_least_once get :sslonly assert_redirected_to :protocol => 'https://' @@ -292,6 +293,7 @@ class ApplicationControllerTest < Test::Unit::TestCase end should 'keep arguments when redirecting to ssl' do + Environment.default.update_attribute(:enable_ssl, true) @request.expects(:ssl?).returns(false).at_least_once get :sslonly, :x => '1', :y => '2' assert_redirected_to :protocol => 'https://', :x => '1', :y => '2' @@ -327,13 +329,14 @@ class ApplicationControllerTest < Test::Unit::TestCase end should 'add https protocols on redirect_to_ssl' do + Environment.default.update_attribute(:enable_ssl, true) get :sslonly, :x => '1', :y => '1' assert_redirected_to :x => '1', :y => '1', :protocol => 'https://' end should 'return true in redirect_to_ssl' do env = mock - env.expects(:disable_ssl).returns(false) + env.expects(:enable_ssl).returns(true) @controller.expects(:environment).returns(env) @controller.expects(:params).returns({}) @controller.expects(:redirect_to).with({:protocol => 'https://'}) @@ -341,14 +344,14 @@ class ApplicationControllerTest < Test::Unit::TestCase end should 'return false in redirect_to_ssl when ssl is disabled' do env = mock - env.expects(:disable_ssl).returns(true) + env.expects(:enable_ssl).returns(false) @controller.expects(:environment).returns(env) assert_equal false, @controller.redirect_to_ssl end should 'not force ssl when ssl is disabled' do env = Environment.default - env.expects(:disable_ssl).returns(true) + env.expects(:enable_ssl).returns(false) @controller.stubs(:environment).returns(env) @request.expects(:ssl?).returns(false).at_least_once get :sslonly diff --git a/test/integration/login_to_the_application_test.rb b/test/integration/login_to_the_application_test.rb index bca0a84..b1bc04f 100644 --- a/test/integration/login_to_the_application_test.rb +++ b/test/integration/login_to_the_application_test.rb @@ -20,7 +20,7 @@ class LoginToTheApplicationTest < ActionController::IntegrationTest end def test_unauthenticated_user_tries_to_access_his_control_panel - Environment.any_instance.stubs(:disable_ssl).returns(true) # ignore SSL for this test + Environment.any_instance.stubs(:enable_ssl).returns(false) # ignore SSL for this test get '/myprofile/ze' assert_redirected_to '/account/login' diff --git a/test/unit/application_helper_test.rb b/test/unit/application_helper_test.rb index ad344c0..0a7aba2 100644 --- a/test/unit/application_helper_test.rb +++ b/test/unit/application_helper_test.rb @@ -195,19 +195,19 @@ class ApplicationHelperTest < Test::Unit::TestCase end should 'use https:// for login_url' do - environment = mock - environment.expects(:disable_ssl).returns(false) + environment = Environment.default + environment.update_attribute(:enable_ssl, true) + environment.domains << Domain.new(:name => "test.domain.net", :is_default => true) stubs(:environment).returns(environment) - request = mock - request.stubs(:host).returns('myhost.net') - stubs(:request).returns(request) - stubs(:url_for).with(has_entries(:protocol => 'https://', :host => 'myhost.net')).returns('LALALA') + + stubs(:url_for).with(has_entries(:protocol => 'https://', :host => 'test.domain.net')).returns('LALALA') + assert_equal 'LALALA', login_url end should 'not force ssl in login_url when environment has ssl disabled' do environment = mock - environment.expects(:disable_ssl).returns(true).at_least_once + environment.expects(:enable_ssl).returns(false).at_least_once stubs(:environment).returns(environment) request = mock request.stubs(:host).returns('localhost') diff --git a/test/unit/environment_test.rb b/test/unit/environment_test.rb index 8d50f0d..53fde0f 100644 --- a/test/unit/environment_test.rb +++ b/test/unit/environment_test.rb @@ -487,14 +487,14 @@ class EnvironmentTest < Test::Unit::TestCase assert_equal template, e.enterprise_template end - should 'not disable ssl by default' do + should 'not enable ssl by default' do e = Environment.new - assert !e.disable_ssl + assert !e.enable_ssl end - should 'be able to disable ssl' do - e = Environment.new(:disable_ssl => true) - assert_equal true, e.disable_ssl + should 'be able to enable ssl' do + e = Environment.new(:enable_ssl => true) + assert_equal true, e.enable_ssl end should 'have a layout template' do -- libgit2 0.21.2