From 4bd874ab46c38792f9e834d28a17ed5eabde9d61 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 9 Jun 2013 10:01:56 +0000 Subject: [PATCH] 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 | 57 ++++++++++++++++++---- app/models/user.rb | 6 +-- config/locales/en.yml | 2 + config/locales/fr.yml | 2 + config/routes.rb | 1 + test/functional/account_controller_test.rb | 44 ++++++++++++++++- test/integration/account_test.rb | 45 +++++++++++++++++ 7 files changed, 144 insertions(+), 13 deletions(-) diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb index 5b8122b6e..ae96c85b3 100644 --- a/app/controllers/account_controller.rb +++ b/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 - unless user && user.active? + # user not found + 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 - flash[:notice] = l(:notice_account_pending) - redirect_to signin_path + def handle_inactive_user(user, redirect_path=signin_path) + if user.registered? + 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 diff --git a/app/models/user.rb b/app/models/user.rb index 738554d85..8ad717042 100644 --- a/app/models/user.rb +++ b/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 diff --git a/config/locales/en.yml b/config/locales/en.yml index 04b74b02b..9e280ecd8 100644 --- a/config/locales/en.yml +++ b/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 click this link. + 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. diff --git a/config/locales/fr.yml b/config/locales/fr.yml index db83c74f8..dd0f7a844 100644 --- a/config/locales/fr.yml +++ b/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 cliquer sur ce lien. + 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. diff --git a/config/routes.rb b/config/routes.rb index 2bf784b23..040aba23b 100644 --- a/config/routes.rb +++ b/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] diff --git a/test/functional/account_controller_test.rb b/test/functional/account_controller_test.rb index 0f9ae8eed..b54a306d5 100644 --- a/test/functional/account_controller_test.rb +++ b/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 diff --git a/test/integration/account_test.rb b/test/integration/account_test.rb index 874fbf6a4..4136d4408 100644 --- a/test/integration/account_test.rb +++ b/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 -- 2.39.5