Fixed: When logging in via an autologin cookie the user's last_login_on should be...

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@2524 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb
index 4d96ad8f312df8fe97e8da23bbe1fe6fe40d15a7..9842b9ead799b9184227a255c9c6948b4f2bc827 100644 (file)
--- a/app/controllers/account_controller.rb
+++ b/app/controllers/account_controller.rb
@@ -150,17 +150,8 @@ class AccountController < ApplicationController
     redirect_to :action => 'login'
   end
   
-private
-  def logged_user=(user)
-    if user && user.is_a?(User)
-      User.current = user
-      session[:user_id] = user.id
-    else
-      User.current = User.anonymous
-      session[:user_id] = nil
-    end
-  end
-  
+  private
+
   def password_authentication
     user = User.try_to_login(params[:username], params[:password])
     if user.nil?

diff --git a/app/controllers/application.rb b/app/controllers/application.rb
index 109771133b94eaed652d0c6791b9ba17383bad2e..f86487305429436913c2ce0e1bccadceb4c963a9 100644 (file)
--- a/app/controllers/application.rb
+++ b/app/controllers/application.rb
@@ -46,7 +46,7 @@ class ApplicationController < ActionController::Base
     # Check the settings cache for each request
     Setting.check_cache
     # Find the current user
-    User.current = find_current_user
+    self.logged_user = find_current_user
   end
   
   # Returns the current user or nil if no user is logged in
@@ -56,13 +56,24 @@ class ApplicationController < ActionController::Base
       (User.active.find(session[:user_id]) rescue nil)
     elsif cookies[:autologin] && Setting.autologin?
       # auto-login feature
-      User.find_by_autologin_key(cookies[:autologin])
+      User.try_to_autologin(cookies[:autologin])
     elsif params[:key] && accept_key_auth_actions.include?(params[:action])
       # RSS key authentication
       User.find_by_rss_key(params[:key])
     end
   end
   
+  # Sets the logged in user
+  def logged_user=(user)
+    if user && user.is_a?(User)
+      User.current = user
+      session[:user_id] = user.id
+    else
+      User.current = User.anonymous
+      session[:user_id] = nil
+    end
+  end
+  
   # check if login is globally required to access the application
   def check_if_login_required
     # no check needed if user is already logged in

diff --git a/app/models/user.rb b/app/models/user.rb
index 32cc08b1123bf089803920b1767c38a264877141..f42787f5d0e219714a128157b1ab879ace948f7e 100644 (file)
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -126,6 +126,15 @@ class User < ActiveRecord::Base
   rescue => text
     raise text
   end
+  
+  # Returns the user who matches the given autologin +key+ or nil
+  def self.try_to_autologin(key)
+    token = Token.find_by_action_and_value('autologin', key)
+    if token && (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
+    end
+  end
        
   # Return user's full name for display
   def name(formatter = nil)
@@ -199,11 +208,6 @@ class User < ActiveRecord::Base
     token && token.user.active? ? token.user : nil
   end
   
-  def self.find_by_autologin_key(key)
-    token = Token.find_by_action_and_value('autologin', key)
-    token && (token.created_on > Setting.autologin.to_i.day.ago) && token.user.active? ? token.user : nil
-  end
-  
   # Makes find_by_mail case-insensitive
   def self.find_by_mail(mail)
     find(:first, :conditions => ["LOWER(mail) = ?", mail.to_s.downcase])

diff --git a/test/functional/account_controller_test.rb b/test/functional/account_controller_test.rb
index cf51ba93c8f2277abec34f4716cb9163b2baf3a5..fb23e6bb7baf697e306b0703cb8c1b7939798606 100644 (file)
--- a/test/functional/account_controller_test.rb
+++ b/test/functional/account_controller_test.rb
@@ -160,18 +160,6 @@ class AccountControllerTest < Test::Unit::TestCase
     puts "Skipping openid tests."
   end
   
-  
-  def test_autologin
-    Setting.autologin = "7"
-    Token.delete_all
-    post :login, :username => 'admin', :password => 'admin', :autologin => 1
-    assert_redirected_to 'my/page'
-    token = Token.find :first
-    assert_not_nil token
-    assert_equal User.find_by_login('admin'), token.user
-    assert_equal 'autologin', token.action
-  end
-  
   def test_logout
     @request.session[:user_id] = 2
     get :logout

diff --git a/test/integration/account_test.rb b/test/integration/account_test.rb
index 5c6f1cebd64a6acf959b849a5926380d69fb1f21..a82743333ffa550a3391411a9e70f826088acf19 100644 (file)
--- a/test/integration/account_test.rb
+++ b/test/integration/account_test.rb
@@ -37,6 +37,38 @@ class AccountTest < ActionController::IntegrationTest
     assert_template "my/account"    
   end
   
+  def test_autologin
+    user = User.find(1)
+    Setting.autologin = "7"
+    Token.delete_all
+    
+    # User logs in with 'autologin' checked
+    post '/login', :username => user.login, :password => 'admin', :autologin => 1
+    assert_redirected_to 'my/page'
+    token = Token.find :first
+    assert_not_nil token
+    assert_equal user, token.user
+    assert_equal 'autologin', token.action
+    assert_equal user.id, session[:user_id]
+    assert_equal token.value, cookies['autologin']
+    
+    # Session is cleared
+    reset!
+    User.current = nil
+    # Clears user's last login timestamp
+    user.update_attribute :last_login_on, nil
+    assert_nil user.reload.last_login_on
+    
+    # User comes back with his autologin cookie
+    cookies[:autologin] = token.value
+    get '/my/page'
+    assert_response :success
+    assert_template 'my/page'
+    assert_equal user.id, session[:user_id]
+    assert_not_nil user.reload.last_login_on
+    assert user.last_login_on > 2.second.ago
+  end
+  
   def test_lost_password
     Token.delete_all