diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d12807aa3a8c..0d1cf36145bc 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -141,6 +141,7 @@ def logged_user=(user) if user && user.is_a?(User) User.current = user session[:user_id] = user.id + session[:updated_at] = Time.now else User.current = User.anonymous end @@ -616,27 +617,32 @@ def disable_api ActiveSupport.run_load_hooks(:application_controller, self) def check_session_lifetime - session_ttl_value = Setting.session_ttl.to_i - - if Setting.session_ttl_enabled? && session_ttl_value >= 5 - if session[:updated_at] && User.current.logged? && ((session[:updated_at] + (session_ttl_value * 60)) < Time.now) - self.logged_user = nil - if request.get? - url = url_for(params) - else - url = url_for(:controller => params[:controller], :action => params[:action], :id => params[:id], :project_id => params[:project_id]) - end - - flash[:warning] = I18n.t('notice_forced_logout', :ttl_time => Setting.session_ttl) - redirect_to(:controller => "account", :action => "login", :back_url => url) + if session_expired? + self.logged_user = nil + if request.get? + url = url_for(params) else - session[:updated_at] = Time.now + url = url_for(:controller => params[:controller], :action => params[:action], + :id => params[:id], :project_id => params[:project_id]) end + flash[:warning] = I18n.t('notice_forced_logout', :ttl_time => Setting.session_ttl) + redirect_to(:controller => "account", :action => "login", :back_url => url) end + session[:updated_at] = Time.now end private + def session_expired? + current_user.logged? && + (session_ttl_enabled? && (session[:updated_at].nil? || + (session[:updated_at] + Setting.session_ttl.to_i.minutes) < Time.now)) + end + + def session_ttl_enabled? + Setting.session_ttl_enabled? && Setting.session_ttl.to_i >= 5 + end + def permitted_params @permitted_params ||= PermittedParams.new(params, current_user) end diff --git a/doc/CHANGELOG.md b/doc/CHANGELOG.md index 6f8011515f3c..ab11ceda9ac4 100644 --- a/doc/CHANGELOG.md +++ b/doc/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +* `#1314` Always set last activity timestamp and check session expiry if ttl-setting is enabled * `#1414` Remove start & due date requirement from planning elements ## 3.0.0pre8 diff --git a/features/session/user_session.feature b/features/session/user_session.feature new file mode 100644 index 000000000000..c02d661803c7 --- /dev/null +++ b/features/session/user_session.feature @@ -0,0 +1,49 @@ +#-- copyright +# OpenProject is a project management system. +# +# Copyright (C) 2012-2013 the OpenProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# See doc/COPYRIGHT.rdoc for more details. +#++ + +Feature: User session + Background: + Given there is 1 project with the following: + | name | Awesome Project | + | identifier | awesome-project | + And project "Awesome Project" uses the following modules: + | calendar | + And there is a role "member" + And the role "member" may have the following rights: + | view_calendar | + And there is 1 user with the following: + | login | bob | + And the user "bob" is a "member" in the project "Awesome Project" + + Scenario: A user can log in + When I login as "bob" + Then I should be logged in as "bob" + + Scenario: A user can log out + Given I am logged in as "bob" + When I logout + Then I should be logged out + + Scenario: A user is logged out when their session is expired + Given the "session_ttl_enabled" setting is set to true + And the "session_ttl" setting is set to 5 + When I login as "bob" + And I wait for "10" minutes + And I go to the home page + Then I should be logged out + + Scenario: A user is logged in as long as their session is valid + Given the "session_ttl_enabled" setting is set to true + And the "session_ttl" setting is set to 5 + When I login as "bob" + And I wait for "4" minutes + And I go to the home page + Then I should be logged in as "bob" diff --git a/features/step_definitions/general_steps.rb b/features/step_definitions/general_steps.rb index d0b456569f2f..2ad019dee5d3 100644 --- a/features/step_definitions/general_steps.rb +++ b/features/step_definitions/general_steps.rb @@ -366,6 +366,10 @@ login(username, password) end +When "I logout" do + visit "/logout" +end + Then /^I should be logged in as "([^\"]*)"?$/ do |username| user = User.find_by_login(username) || User.anonymous page.should have_xpath("//div[contains(., 'Logged in as #{username}')] | //a[contains(.,'#{user.name}')]") @@ -373,6 +377,10 @@ User.current = user end +Then "I should be logged out" do + page.should have_css("a.login") +end + When /^I satisfy the "(.+)" plugin to (.+)$/ do |plugin_name, action| if plugin_loaded?(plugin_name) action_name = action.gsub("\"", "") @@ -409,6 +417,10 @@ end end +When(/^I wait for "(.*?)" minutes$/) do |number_of_minutes| + page.set_rack_session(updated_at: Time.now - number_of_minutes.to_i.minutes) +end + def get_project(project_name = nil) if project_name.blank? project = @project diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 57a95490a538..664cf0f2f9ef 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -241,6 +241,14 @@ end end + shared_examples_for 'index action with enabled session lifetime and inactivity exceeded' do + it "logs out the user and redirects with a warning that he has been locked out" do + response.redirect_url.should == (signin_url + "?back_url=" + CGI::escape(@controller.url_for(:controller => "users", :action => "index"))) + User.current.should_not == admin + flash[:warning].should == I18n.t(:notice_forced_logout, :ttl_time => Setting.session_ttl) + end + end + context "disabled" do before do Setting.stub!(:session_ttl_enabled?).and_return(false) @@ -272,11 +280,16 @@ session[:updated_at] = Time.now - 3.hours get :index end - it "logs out the user and redirects with a warning that he has been locked out" do - response.redirect_url.should == (signin_url + "?back_url=" + CGI::escape(@controller.url_for(:controller => "users", :action => "index"))) - User.current.should_not == admin - flash[:warning].should == I18n.t(:notice_forced_logout, :ttl_time => Setting.session_ttl) + it_should_behave_like 'index action with enabled session lifetime and inactivity exceeded' + end + + context "without last activity time in the session" do + before do + Setting.stub!(:session_ttl).and_return("60") + session[:updated_at] = nil + get :index end + it_should_behave_like 'index action with enabled session lifetime and inactivity exceeded' end context "with ttl = 0" do