From 8c94da8d89bd36bc7c4c2316207b4f7aba61ace5 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 24 Apr 2014 11:42:53 +0200 Subject: [PATCH 1/5] log last login for omniauth too added tests --- app/controllers/account_controller.rb | 8 +++++--- app/controllers/concerns/omniauth_login.rb | 2 +- app/models/user.rb | 7 +++++-- spec/controllers/concerns/omniauth_login_spec.rb | 13 ++++++++++++- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb index 51fcc7b20f..ce9d0a61c5 100644 --- a/app/controllers/account_controller.rb +++ b/app/controllers/account_controller.rb @@ -215,13 +215,15 @@ class AccountController < ApplicationController end end - def successful_authentication(user) + def successful_authentication(user, log_login = false) # Valid user self.logged_user = user # generate a key and set cookie if autologin if params[:autologin] && Setting.autologin? set_autologin_cookie(user) end + user.log_successful_login if log_login + call_hook(:controller_account_success_authentication_after, {:user => user }) redirect_after_login(user) @@ -239,9 +241,9 @@ class AccountController < ApplicationController cookies[OpenProject::Configuration['autologin_cookie_name']] = cookie_options end - def login_user_if_active(user) + def login_user_if_active(user, log_login = false) if user.active? - successful_authentication(user) + successful_authentication(user, log_login) else account_inactive(user, flash_now: false) redirect_to signin_path diff --git a/app/controllers/concerns/omniauth_login.rb b/app/controllers/concerns/omniauth_login.rb index 7ea71f2460..dffc49d3bd 100644 --- a/app/controllers/concerns/omniauth_login.rb +++ b/app/controllers/concerns/omniauth_login.rb @@ -13,7 +13,7 @@ module OmniauthLogin if user.new_record? create_user_from_omniauth(user, auth_hash) else - login_user_if_active(user) + login_user_if_active(user, log_login = true) end end diff --git a/app/models/user.rb b/app/models/user.rb index 47be7a54f5..b4db9ea8f2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -242,7 +242,7 @@ class User < Principal try_authentication_and_create_user(login, password) end unless prevent_brute_force_attack(user, login).nil? - user.update_attribute(:last_login_on, Time.now) if user && !user.new_record? + user.log_successful_login if user && !user.new_record? return user end nil @@ -289,7 +289,7 @@ class User < Principal if tokens.size == 1 token = tokens.first if (token.created_on > Setting.autologin.to_i.day.ago) && token.user && token.user.active? - token.user.update_attribute(:last_login_on, Time.now) + log_successful_login token.user end end @@ -402,6 +402,9 @@ class User < Principal save end + def log_successful_login + update_attribute(:last_login_on, Time.now) + end def pref preference || build_preference diff --git a/spec/controllers/concerns/omniauth_login_spec.rb b/spec/controllers/concerns/omniauth_login_spec.rb index 364e317cee..9529d7487c 100644 --- a/spec/controllers/concerns/omniauth_login_spec.rb +++ b/spec/controllers/concerns/omniauth_login_spec.rb @@ -196,12 +196,23 @@ describe AccountController do end context 'with an active account' do - it 'should sign in the user after successful external authentication' do + before do user.save! + end + + it 'should sign in the user after successful external authentication' do post :omniauth_login expect(response).to redirect_to my_page_path end + + it 'should log an successful login' do + post_at = Time.now.utc + post :omniauth_login + + user.reload + expect(user.last_login_on.utc.to_i).to be >= post_at.utc.to_i + end end context 'with a registered and not activated accout' do From c1229ae42a5d575fdc75cc714de1fbe03ba837ee Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Thu, 24 Apr 2014 17:36:31 +0200 Subject: [PATCH 2/5] updated copyright --- spec/controllers/concerns/omniauth_login_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/concerns/omniauth_login_spec.rb b/spec/controllers/concerns/omniauth_login_spec.rb index 9529d7487c..0a383e6df8 100644 --- a/spec/controllers/concerns/omniauth_login_spec.rb +++ b/spec/controllers/concerns/omniauth_login_spec.rb @@ -6,8 +6,8 @@ # modify it under the terms of the GNU General Public License version 3. # # OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: -# Copyright (C) 2006-2013 Jean-Philippe Lang -# Copyright (C) 2010-2013 the ChiliProject Team +# Copyright (C) 2006-2014 Jean-Philippe Lang +# Copyright (C) 2010-2014 the ChiliProject Team # # This program is free software; you can redistribute it and/or # modify it under the terms of the GNU General Public License From 21dd6ecf7e1f0021b61d066c2673ec3ea163621d Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 25 Apr 2014 16:37:58 +0200 Subject: [PATCH 3/5] log_successful_login call goes to user instance --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index b4db9ea8f2..e3df8ba8a5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -289,7 +289,7 @@ class User < Principal if tokens.size == 1 token = tokens.first if (token.created_on > Setting.autologin.to_i.day.ago) && token.user && token.user.active? - log_successful_login + token.user.log_successful_login token.user end end From 3551a7ba9fdab51ed4705861fabd9964f07bb281 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Tue, 29 Apr 2014 15:56:47 +0100 Subject: [PATCH 4/5] fixed typo in comment --- spec/controllers/concerns/omniauth_login_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/concerns/omniauth_login_spec.rb b/spec/controllers/concerns/omniauth_login_spec.rb index 0a383e6df8..5a8b2edee8 100644 --- a/spec/controllers/concerns/omniauth_login_spec.rb +++ b/spec/controllers/concerns/omniauth_login_spec.rb @@ -206,7 +206,7 @@ describe AccountController do expect(response).to redirect_to my_page_path end - it 'should log an successful login' do + it 'should log a successful login' do post_at = Time.now.utc post :omniauth_login From e624f84c874ed1fd38bd87971dde9ee380e73a35 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Mon, 5 May 2014 17:50:36 +0100 Subject: [PATCH 5/5] log login directly during omniauth procedure --- app/controllers/account_controller.rb | 7 +++---- app/controllers/concerns/omniauth_login.rb | 3 ++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb index ce9d0a61c5..b3ab7cba7b 100644 --- a/app/controllers/account_controller.rb +++ b/app/controllers/account_controller.rb @@ -215,14 +215,13 @@ class AccountController < ApplicationController end end - def successful_authentication(user, log_login = false) + def successful_authentication(user) # Valid user self.logged_user = user # generate a key and set cookie if autologin if params[:autologin] && Setting.autologin? set_autologin_cookie(user) end - user.log_successful_login if log_login call_hook(:controller_account_success_authentication_after, {:user => user }) @@ -241,9 +240,9 @@ class AccountController < ApplicationController cookies[OpenProject::Configuration['autologin_cookie_name']] = cookie_options end - def login_user_if_active(user, log_login = false) + def login_user_if_active(user) if user.active? - successful_authentication(user, log_login) + successful_authentication(user) else account_inactive(user, flash_now: false) redirect_to signin_path diff --git a/app/controllers/concerns/omniauth_login.rb b/app/controllers/concerns/omniauth_login.rb index dffc49d3bd..778b82a7d2 100644 --- a/app/controllers/concerns/omniauth_login.rb +++ b/app/controllers/concerns/omniauth_login.rb @@ -13,7 +13,8 @@ module OmniauthLogin if user.new_record? create_user_from_omniauth(user, auth_hash) else - login_user_if_active(user, log_login = true) + user.log_successful_login if user.active? + login_user_if_active(user) end end