Adds a way for a registered user to get a new action email (#14228).

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

app/controllers/account_controller.rb

@@ -75,11 +75,15 @@ class AccountController < ApplicationController
     else
       if request.post?
         user = User.find_by_mail(params[:mail].to_s)
-        # user not found or not active
+        # user not found
-        unless user && user.active?
+        unless user
           flash.now[:error] = l(:notice_account_unknown_email)
           return
         end
+        unless user.active?
+          handle_inactive_user(user, lost_password_path)
+          return
+        end
         # user cannot change its password
         unless user.change_password_allowed?
           flash.now[:error] = l(:notice_can_t_change_password)
@@ -152,6 +156,19 @@ class AccountController < ApplicationController
     redirect_to signin_path
   end
 
+  # Sends a new account activation email
+  def activation_email
+    if session[:registered_user_id] && Setting.self_registration == '1'
+      user_id = session.delete(:registered_user_id).to_i
+      user = User.find_by_id(user_id)
+      if user && user.registered?
+        register_by_email_activation(user)
+        return
+      end
+    end
+    redirect_to(home_url)
+  end
+
   private
 
   def authenticate_user
@@ -163,7 +180,7 @@ class AccountController < ApplicationController
   end
 
   def password_authentication
-    user = User.try_to_login(params[:username], params[:password])
+    user = User.try_to_login(params[:username], params[:password], false)
 
     if user.nil?
       invalid_credentials
@@ -171,7 +188,11 @@ class AccountController < ApplicationController
       onthefly_creation_failed(user, {:login => user.login, :auth_source_id => user.auth_source_id })
     else
       # Valid user
-      successful_authentication(user)
+      if user.active?
+        successful_authentication(user)
+      else
+        handle_inactive_user(user)
+      end
     end
   end
 
@@ -211,7 +232,7 @@ class AccountController < ApplicationController
           if user.active?
             successful_authentication(user)
           else
-            account_pending
+            handle_inactive_user(user)
           end
         end
       end
@@ -291,14 +312,32 @@ class AccountController < ApplicationController
     if user.save
       # Sends an email to the administrators
       Mailer.account_activation_request(user).deliver
-      account_pending
+      account_pending(user)
     else
       yield if block_given?
     end
   end
 
-  def account_pending
+  def handle_inactive_user(user, redirect_path=signin_path)
-    flash[:notice] = l(:notice_account_pending)
+    if user.registered?
-    redirect_to signin_path
+      account_pending(user, redirect_path)
+    else
+      account_locked(user, redirect_path)
+    end
+  end
+
+  def account_pending(user, redirect_path=signin_path)
+    if Setting.self_registration == '1'
+      flash[:error] = l(:notice_account_not_activated_yet, :url => activation_email_path)
+      session[:registered_user_id] = user.id
+    else
+      flash[:error] = l(:notice_account_pending)
+    end
+    redirect_to redirect_path
+  end
+
+  def account_locked(user, redirect_path=signin_path)
+    flash[:error] = l(:notice_account_locked)
+    redirect_to redirect_path
   end
 end

app/models/user.rb

@@ -157,7 +157,7 @@ class User < Principal
   end
 
   # Returns the user that matches provided login and password, or nil
-  def self.try_to_login(login, password)
+  def self.try_to_login(login, password, active_only=true)
     login = login.to_s
     password = password.to_s
 
@@ -166,8 +166,8 @@ class User < Principal
     user = find_by_login(login)
     if user
       # user is already in local database
-      return nil unless user.active?
       return nil unless user.check_password?(password)
+      return nil if !user.active? && active_only 
     else
       # user is not yet registered, try to authenticate with available sources
       attrs = AuthSource.authenticate(login, password)
@@ -181,7 +181,7 @@ class User < Principal
         end
       end
     end
-    user.update_column(:last_login_on, Time.now) if user && !user.new_record?
+    user.update_column(:last_login_on, Time.now) if user && !user.new_record? && user.active?
     user
   rescue => text
     raise text

config/locales/en.yml

@@ -150,6 +150,8 @@ en:
   notice_account_wrong_password: Wrong password
   notice_account_register_done: Account was successfully created. To activate your account, click on the link that was emailed to you.
   notice_account_unknown_email: Unknown user.
+  notice_account_not_activated_yet: You haven't activated your account yet. If you want to receive a new activation email, please <a href="%{url}">click this link</a>.
+  notice_account_locked: Your account is locked.
   notice_can_t_change_password: This account uses an external authentication source. Impossible to change the password.
   notice_account_lost_email_sent: An email with instructions to choose a new password has been sent to you.
   notice_account_activated: Your account has been activated. You can now log in.

config/locales/fr.yml

@@ -167,6 +167,8 @@ fr:
   notice_account_wrong_password: Mot de passe incorrect
   notice_account_register_done: Un message contenant les instructions pour activer votre compte vous a été envoyé.
   notice_account_unknown_email: Aucun compte ne correspond à cette adresse.
+  notice_account_not_activated_yet: Vous n'avez pas encore activé votre compte. Si vous voulez recevoir un nouveau message d'activation, veuillez <a href="%{url}">cliquer sur ce lien</a>.
+  notice_account_locked: Votre compte est verrouillé.
   notice_can_t_change_password: Ce compte utilise une authentification externe. Impossible de changer le mot de passe.
   notice_account_lost_email_sent: Un message contenant les instructions pour choisir un nouveau mot de passe vous a été envoyé.
   notice_account_activated: Votre compte a été activé. Vous pouvez à présent vous connecter.

config/routes.rb

@@ -23,6 +23,7 @@ RedmineApp::Application.routes.draw do
   match 'account/register', :to => 'account#register', :via => [:get, :post], :as => 'register'
   match 'account/lost_password', :to => 'account#lost_password', :via => [:get, :post], :as => 'lost_password'
   match 'account/activate', :to => 'account#activate', :via => :get
+  get 'account/activation_email', :to => 'account#activation_email', :as => 'activation_email'
 
   match '/news/preview', :controller => 'previews', :action => 'news', :as => 'preview_news', :via => [:get, :post, :put]
   match '/issues/preview/new/:project_id', :to => 'previews#issue', :as => 'preview_new_issue', :via => [:get, :post, :put]

test/functional/account_controller_test.rb

@@ -63,6 +63,36 @@ class AccountControllerTest < ActionController::TestCase
     assert_select 'input[name=password][value]', 0
   end
 
+  def test_login_with_locked_account_should_fail
+    User.find(2).update_attribute :status, User::STATUS_LOCKED
+
+    post :login, :username => 'jsmith', :password => 'jsmith'
+    assert_redirected_to '/login'
+    assert_include 'locked', flash[:error]
+    assert_nil @request.session[:user_id]
+  end
+
+  def test_login_as_registered_user_with_manual_activation_should_inform_user
+    User.find(2).update_attribute :status, User::STATUS_REGISTERED
+
+    with_settings :self_registration => '2', :default_language => 'en' do
+      post :login, :username => 'jsmith', :password => 'jsmith'
+      assert_redirected_to '/login'
+      assert_include 'pending administrator approval', flash[:error]
+    end
+  end
+
+  def test_login_as_registered_user_with_email_activation_should_propose_new_activation_email
+    User.find(2).update_attribute :status, User::STATUS_REGISTERED
+
+    with_settings :self_registration => '1', :default_language => 'en' do
+      post :login, :username => 'jsmith', :password => 'jsmith'
+      assert_redirected_to '/login'
+      assert_equal 2, @request.session[:registered_user_id]
+      assert_include 'new activation email', flash[:error]
+    end
+  end
+
   def test_login_should_rescue_auth_source_exception
     source = AuthSource.create!(:name => 'Test')
     User.find(2).update_attribute :auth_source_id, source.id
@@ -217,7 +247,7 @@ class AccountControllerTest < ActionController::TestCase
 
     assert_no_difference 'Token.count' do
       post :lost_password, :mail => 'JSmith@somenet.foo'
-      assert_response :success
+      assert_redirected_to '/account/lost_password'
     end
   end
 
@@ -274,4 +304,16 @@ class AccountControllerTest < ActionController::TestCase
     post :lost_password, :token => "abcdef", :new_password => 'newpass', :new_password_confirmation => 'newpass'
     assert_redirected_to '/'
   end
+
+  def test_activation_email_should_send_an_activation_email
+    User.find(2).update_attribute :status, User::STATUS_REGISTERED
+    @request.session[:registered_user_id] = 2
+
+    with_settings :self_registration => '1' do
+      assert_difference 'ActionMailer::Base.deliveries.size' do
+        get :activation_email
+        assert_redirected_to '/login'
+      end
+    end
+  end
 end

test/integration/account_test.rb

@@ -221,4 +221,49 @@ class AccountTest < ActionController::IntegrationTest
     assert_equal 66, user.auth_source_id
     assert user.hashed_password.blank?
   end
+
+  def test_registered_user_should_be_able_to_get_a_new_activation_email
+    Token.delete_all
+      
+    with_settings :self_registration => '1', :default_language => 'en' do
+      # register a new account
+      assert_difference 'User.count' do
+        assert_difference 'Token.count' do
+          post 'account/register',
+             :user => {:login => "newuser", :language => "en",
+                       :firstname => "New", :lastname => "User", :mail => "newuser@foo.bar",
+                       :password => "newpass123", :password_confirmation => "newpass123"}
+        end
+      end
+      user = User.order('id desc').first
+      assert_equal User::STATUS_REGISTERED, user.status
+      reset!
+
+      # try to use "lost password"
+      assert_no_difference 'ActionMailer::Base.deliveries.size' do
+        post '/account/lost_password', :mail => 'newuser@foo.bar'
+      end
+      assert_redirected_to '/account/lost_password'
+      follow_redirect!
+      assert_response :success
+      assert_select 'div.flash', :text => /new activation email/
+      assert_select 'div.flash a[href=/account/activation_email]'
+
+      # request a new action activation email
+      assert_difference 'ActionMailer::Base.deliveries.size' do
+        get '/account/activation_email'
+      end
+      assert_redirected_to '/login'
+      token = Token.order('id desc').first
+      activation_path = "/account/activate?token=#{token.value}"
+      assert_include activation_path, mail_body(ActionMailer::Base.deliveries.last)
+
+      # activate the account
+      get activation_path
+      assert_redirected_to '/login'
+
+      post '/login', :username => 'newuser', :password => 'newpass123'
+      assert_redirected_to '/my/page'
+    end
+  end
 end