Commit 48f51755bc02fae929e8d62d7bf86aee8b2811ec
1 parent
00d46ded
Exists in
master
and in
29 other branches
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.
Showing
4 changed files
with
23 additions
and
2 deletions
Show diff stats
app/controllers/application_controller.rb
@@ -9,11 +9,15 @@ class ApplicationController < ActionController::Base | @@ -9,11 +9,15 @@ class ApplicationController < ActionController::Base | ||
9 | before_filter :allow_cross_domain_access | 9 | before_filter :allow_cross_domain_access |
10 | 10 | ||
11 | before_filter :login_from_cookie | 11 | before_filter :login_from_cookie |
12 | - before_filter :login_required, :if => :private_environment? | 12 | + before_filter :require_login_for_environment, :if => :private_environment? |
13 | 13 | ||
14 | before_filter :verify_members_whitelist, :if => [:private_environment?, :user] | 14 | before_filter :verify_members_whitelist, :if => [:private_environment?, :user] |
15 | before_filter :redirect_to_current_user | 15 | before_filter :redirect_to_current_user |
16 | 16 | ||
17 | + def require_login_for_environment | ||
18 | + login_required | ||
19 | + end | ||
20 | + | ||
17 | def verify_members_whitelist | 21 | def verify_members_whitelist |
18 | render_access_denied unless user.is_admin? || environment.in_whitelist?(user) | 22 | render_access_denied unless user.is_admin? || environment.in_whitelist?(user) |
19 | end | 23 | end |
app/controllers/public/account_controller.rb
@@ -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
@@ -1046,4 +1046,15 @@ class AccountControllerTest < ActionController::TestCase | @@ -1046,4 +1046,15 @@ class AccountControllerTest < ActionController::TestCase | ||
1046 | :national_region_type_id => NationalRegionType::CITY, | 1046 | :national_region_type_id => NationalRegionType::CITY, |
1047 | :parent_national_region_code => parent_region.national_region_code) | 1047 | :parent_national_region_code => parent_region.national_region_code) |
1048 | end | 1048 | end |
1049 | + | ||
1050 | + should 'not lock users out of login if environment is restrict to members' do | ||
1051 | + Environment.default.enable(:restrict_to_members) | ||
1052 | + get :login | ||
1053 | + assert_response :success | ||
1054 | + | ||
1055 | + post :login, :user => {:login => 'johndoe', :password => 'test'} | ||
1056 | + assert session[:user] | ||
1057 | + assert_response :redirect | ||
1058 | + end | ||
1059 | + | ||
1049 | end | 1060 | end |
test/functional/profile_controller_test.rb
@@ -1812,4 +1812,10 @@ class ProfileControllerTest < ActionController::TestCase | @@ -1812,4 +1812,10 @@ class ProfileControllerTest < ActionController::TestCase | ||
1812 | assert @response.body.index("another_user") > @response.body.index("different_user") | 1812 | assert @response.body.index("another_user") > @response.body.index("different_user") |
1813 | end | 1813 | end |
1814 | 1814 | ||
1815 | + should 'redirect to login if environment is restrict to members' do | ||
1816 | + Environment.default.enable(:restrict_to_members) | ||
1817 | + get :index | ||
1818 | + assert_redirected_to :controller => 'account', :action => 'login' | ||
1819 | + end | ||
1820 | + | ||
1815 | end | 1821 | end |