Download as
Browse Code »

Commit 1fe26385aa31f463289a6901201edf36f8e7b455

Authored by Larissa Reis
Committed by Antonio Terceiro
1 parent 1225bf3e
Exists in stable-1.2

Require login for all pages when environment is private 

  This fixes a bug in which some pages (eg. a profile page) were visible
  to unlogged users even if the environment has enabled "show content
  only to members".

  The problem happened because some controllers use `before_filter
  :login_required` so they can apply it to some specific methods,
  effectively overriding the one set in `application_controller`. That
  before filter set in `application_controller` is the one used to make
  the environment private when that feature is enabled, so when a
  controller overrides it, some methods are not required login even when
  the environment is private. So I fixed the problem by using a
  different `before_filter` to take care specifically of private
  environments.

  Now every page requires login when an environment is private, except
  the pages in `account_controller` necessary for login and signup.

(cherry picked from commit 48f51755bc02fae929e8d62d7bf86aee8b2811ec)
Showing 4 changed files with 24 additions and 2 deletions   Show diff stats
app/controllers/application_controller.rb
  Diff comments   View file @1fe2638
@@ -7,10 +7,15 @@ class ApplicationController < ActionController::Base @@ -7,10 +7,15 @@ class ApplicationController < ActionController::Base
7 before_filter :detect_stuff_by_domain 7 before_filter :detect_stuff_by_domain
8 before_filter :init_noosfero_plugins 8 before_filter :init_noosfero_plugins
9 before_filter :allow_cross_domain_access 9 before_filter :allow_cross_domain_access
10 - before_filter :login_required, :if => :private_environment? 10 + before_filter :require_login_for_environment, :if => :private_environment?
  11 +
11 before_filter :verify_members_whitelist, :if => [:private_environment?, :user] 12 before_filter :verify_members_whitelist, :if => [:private_environment?, :user]
12 before_filter :redirect_to_current_user 13 before_filter :redirect_to_current_user
13 14
  15 + def require_login_for_environment
  16 + login_required
  17 + end
  18 +
14 def verify_members_whitelist 19 def verify_members_whitelist
15 render_access_denied unless user.is_admin? || environment.in_whitelist?(user) 20 render_access_denied unless user.is_admin? || environment.in_whitelist?(user)
16 end 21 end
app/controllers/public/account_controller.rb
  Diff comments   View file @1fe2638
@@ -2,7 +2,7 @@ class AccountController < ApplicationController @@ -2,7 +2,7 @@ class AccountController < ApplicationController
2 2
3 no_design_blocks 3 no_design_blocks
4 4
5 - before_filter :login_required, :only => [:activation_question, :accept_terms, :activate_enterprise, :change_password] 5 + before_filter :login_required, :require_login_for_environment, :only => [:activation_question, :accept_terms, :activate_enterprise, :change_password]
6 before_filter :redirect_if_logged_in, :only => [:login, :signup] 6 before_filter :redirect_if_logged_in, :only => [:login, :signup]
7 before_filter :protect_from_bots, :only => :signup 7 before_filter :protect_from_bots, :only => :signup
8 8
test/functional/account_controller_test.rb
  Diff comments   View file @1fe2638
@@ -1029,4 +1029,15 @@ class AccountControllerTest < ActionController::TestCase @@ -1029,4 +1029,15 @@ class AccountControllerTest < ActionController::TestCase
1029 :national_region_type_id => NationalRegionType::CITY, 1029 :national_region_type_id => NationalRegionType::CITY,
1030 :parent_national_region_code => parent_region.national_region_code) 1030 :parent_national_region_code => parent_region.national_region_code)
1031 end 1031 end
  1032 +
  1033 + should 'not lock users out of login if environment is restrict to members' do
  1034 + Environment.default.enable(:restrict_to_members)
  1035 + get :login
  1036 + assert_response :success
  1037 +
  1038 + post :login, :user => {:login => 'johndoe', :password => 'test'}
  1039 + assert session[:user]
  1040 + assert_response :redirect
  1041 + end
  1042 +
1032 end 1043 end
test/functional/profile_controller_test.rb
  Diff comments   View file @1fe2638
@@ -1748,4 +1748,10 @@ class ProfileControllerTest < ActionController::TestCase @@ -1748,4 +1748,10 @@ class ProfileControllerTest < ActionController::TestCase
1748 assert_no_tag :tag => 'td', :descendant => { :tag => 'a', :content => /#{person.enterprises.count}/, :attributes => { :href => /profile\/#{person.identifier}\/enterprises$/ }} 1748 assert_no_tag :tag => 'td', :descendant => { :tag => 'a', :content => /#{person.enterprises.count}/, :attributes => { :href => /profile\/#{person.identifier}\/enterprises$/ }}
1749 end 1749 end
1750 1750
  1751 + should 'redirect to login if environment is restrict to members' do
  1752 + Environment.default.enable(:restrict_to_members)
  1753 + get :index
  1754 + assert_redirected_to :controller => 'account', :action => 'login'
  1755 + end
  1756 +
1751 end 1757 end