Merge pull request #1163 from opf/fix/7137_no_last_connection

Fix #7137: last login not logged (omniauth)

app/controllers/account_controller.rb

@@ -222,6 +222,7 @@ class AccountController < ApplicationController
     if params[:autologin] && Setting.autologin?
       set_autologin_cookie(user)
     end
+
     call_hook(:controller_account_success_authentication_after, {:user => user })
 
     redirect_after_login(user)

app/controllers/concerns/omniauth_login.rb

@@ -13,6 +13,7 @@ module OmniauthLogin
     if user.new_record?
       create_user_from_omniauth(user, auth_hash)
     else
+      user.log_successful_login if user.active?
       login_user_if_active(user)
     end
   end

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)
+        token.user.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

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
@@ -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 a 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