From df535c6346c04afc2d08de2ff057894d1f9b66d5 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 25 Jul 2016 12:07:31 +1000 Subject: [PATCH] FEATURE: refresh session cookie at most once an hour This feature ensures session cookie lifespan is extended when user is online. Also decreases session timeout from 90 to 60 days. Ensures all users (including logged on ones) get expiring sessions. --- app/controllers/application_controller.rb | 5 ++++ app/models/user.rb | 2 +- config/locales/server.en.yml | 2 +- config/site_settings.yml | 2 +- ...0725015749_rename_auth_token_created_at.rb | 5 ++++ lib/auth/current_user_provider.rb | 4 ++++ lib/auth/default_current_user_provider.rb | 14 +++++++++-- lib/current_user.rb | 4 ++++ .../default_current_user_provider_spec.rb | 24 +++++++++++++++++++ 9 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20160725015749_rename_auth_token_created_at.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6c17642f39..7ca4330a51 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -44,6 +44,7 @@ class ApplicationController < ActionController::Base before_filter :redirect_to_login_if_required before_filter :check_xhr after_filter :add_readonly_header + after_filter :perform_refresh_session layout :set_layout @@ -59,6 +60,10 @@ class ApplicationController < ActionController::Base response.headers['Discourse-Readonly'] = 'true' if Discourse.readonly_mode? end + def perform_refresh_session + refresh_session(current_user) + end + def slow_platform? request.user_agent =~ /Android/ end diff --git a/app/models/user.rb b/app/models/user.rb index bc0a613daf..af641a50df 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1048,7 +1048,7 @@ end # trust_level_locked :boolean default(FALSE), not null # staged :boolean default(FALSE), not null # first_seen_at :datetime -# auth_token_created_at :datetime +# auth_token_updated_at :datetime # # Indexes # diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index b51efc38b5..7792d56ff6 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -909,7 +909,7 @@ en: post_undo_action_window_mins: "Number of minutes users are allowed to undo recent actions on a post (like, flag, etc)." must_approve_users: "Staff must approve all new user accounts before they are allowed to access the site. WARNING: enabling this for a live site will revoke access for existing non-staff users!" pending_users_reminder_delay: "Notify moderators if new users have been waiting for approval for longer than this many hours. Set to -1 to disable notifications." - maximum_session_age: "User will remain logged in for n hours." + maximum_session_age: "User will remain logged in for n hours since last visit" ga_tracking_code: "OBSOLETE: Google analytics (ga.js) tracking code code, eg: UA-12345678-9; see http://google.com/analytics" ga_domain_name: "OBSOLETE: Google analytics (ga.js) domain name, eg: mysite.com; see http://google.com/analytics" ga_universal_tracking_code: "Google Universal Analytics (analytics.js) tracking code code, eg: UA-12345678-9; see http://google.com/analytics" diff --git a/config/site_settings.yml b/config/site_settings.yml index b42174fd96..d52898e1a5 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -305,7 +305,7 @@ login: min: -1 default: 8 maximum_session_age: - default: 2160 + default: 1440 min: 1 max: 175200 diff --git a/db/migrate/20160725015749_rename_auth_token_created_at.rb b/db/migrate/20160725015749_rename_auth_token_created_at.rb new file mode 100644 index 0000000000..a34ea6d7a2 --- /dev/null +++ b/db/migrate/20160725015749_rename_auth_token_created_at.rb @@ -0,0 +1,5 @@ +class RenameAuthTokenCreatedAt < ActiveRecord::Migration + def change + rename_column :users, :auth_token_created_at, :auth_token_updated_at + end +end diff --git a/lib/auth/current_user_provider.rb b/lib/auth/current_user_provider.rb index d5bff65753..4de839d7d7 100644 --- a/lib/auth/current_user_provider.rb +++ b/lib/auth/current_user_provider.rb @@ -16,6 +16,10 @@ class Auth::CurrentUserProvider raise NotImplementedError end + # optional interface to be called to refresh cookies etc if needed + def refresh_session(user,session,cookies) + end + # api has special rights return true if api was detected def is_api? raise NotImplementedError diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb index 107562e913..9562314b83 100644 --- a/lib/auth/default_current_user_provider.rb +++ b/lib/auth/default_current_user_provider.rb @@ -36,7 +36,10 @@ class Auth::DefaultCurrentUserProvider current_user = nil if auth_token && auth_token.length == 32 - current_user = User.where(auth_token: auth_token).where('auth_token_created_at IS NULL OR auth_token_created_at > ?', SiteSetting.maximum_session_age.hours.ago).first + current_user = User.where(auth_token: auth_token) + .where('auth_token_updated_at IS NULL OR auth_token_updated_at > ?', + SiteSetting.maximum_session_age.hours.ago) + .first end if current_user && (current_user.suspended? || !current_user.active) @@ -61,9 +64,16 @@ class Auth::DefaultCurrentUserProvider @env[CURRENT_USER_KEY] = current_user end + def refresh_session(user, session, cookies) + if user && (!user.auth_token_updated_at || user.auth_token_updated_at <= 1.hour.ago) + user.update_column(:auth_token_updated_at, Time.zone.now) + cookies[TOKEN_COOKIE] = { value: user.auth_token, httponly: true, expires: SiteSetting.maximum_session_age.hours.from_now } + end + end + def log_on_user(user, session, cookies) user.auth_token = SecureRandom.hex(16) - user.auth_token_created_at = Time.zone.now + user.auth_token_updated_at = Time.zone.now user.save! cookies[TOKEN_COOKIE] = { value: user.auth_token, httponly: true, expires: SiteSetting.maximum_session_age.hours.from_now } make_developer_admin(user) diff --git a/lib/current_user.rb b/lib/current_user.rb index dc81ca43ca..5c5ec6b250 100644 --- a/lib/current_user.rb +++ b/lib/current_user.rb @@ -30,6 +30,10 @@ module CurrentUser current_user_provider.current_user end + def refresh_session(user) + current_user_provider.refresh_session(user,session,cookies) + end + private def current_user_provider diff --git a/spec/components/auth/default_current_user_provider_spec.rb b/spec/components/auth/default_current_user_provider_spec.rb index 7cf6976073..45e29c05ce 100644 --- a/spec/components/auth/default_current_user_provider_spec.rb +++ b/spec/components/auth/default_current_user_provider_spec.rb @@ -72,5 +72,29 @@ describe Auth::DefaultCurrentUserProvider do expect(provider("/topic/anything/goes", method: "POST").should_update_last_seen?).to eq(true) expect(provider("/topic/anything/goes", method: "GET").should_update_last_seen?).to eq(true) end + + it "correctly renews session once an hour" do + SiteSetting.maximum_session_age = 3 + user = Fabricate(:user) + provider('/').log_on_user(user, {}, {}) + + freeze_time 2.hours.from_now + cookies = {} + provider("/", "HTTP_COOKIE" => "_t=#{user.auth_token}").refresh_session(user, {}, cookies) + + expect(user.auth_token_updated_at - Time.now).to eq(0) + + end + + it "correctly expires session" do + SiteSetting.maximum_session_age = 2 + user = Fabricate(:user) + provider('/').log_on_user(user, {}, {}) + + expect(provider("/", "HTTP_COOKIE" => "_t=#{user.auth_token}").current_user.id).to eq(user.id) + + freeze_time 3.hours.from_now + expect(provider("/", "HTTP_COOKIE" => "_t=#{user.auth_token}").current_user).to eq(nil) + end end