diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb index a9b8a1b8..1fe99000 100644 --- a/app/controllers/account_controller.rb +++ b/app/controllers/account_controller.rb @@ -44,7 +44,16 @@ class AccountController < ApplicationController else # Authenticate user user = User.try_to_login(params[:username], params[:password]) - if user + if user.nil? + # Invalid credentials + flash.now[:error] = l(:notice_account_invalid_creditentials) + elsif user.new_record? + # Onthefly creation failed, display the registration form to fill/fix attributes + @user = user + session[:auth_source_registration] = {:login => user.login, :auth_source_id => user.auth_source_id } + render :action => 'register' + else + # Valid user self.logged_user = user # generate a key and set cookie if autologin if params[:autologin] && Setting.autologin? @@ -52,12 +61,8 @@ class AccountController < ApplicationController cookies[:autologin] = { :value => token.value, :expires => 1.year.from_now } end redirect_back_or_default :controller => 'my', :action => 'page' - else - flash.now[:error] = l(:notice_account_invalid_creditentials) end end - rescue User::OnTheFlyCreationFailure - flash.now[:error] = 'Redmine could not retrieve the required information from the LDAP to create your account. Please, contact your Redmine administrator.' end # Log out current user and redirect to welcome page @@ -107,39 +112,52 @@ class AccountController < ApplicationController # User self-registration def register - redirect_to(home_url) && return unless Setting.self_registration? + redirect_to(home_url) && return unless Setting.self_registration? || session[:auth_source_registration] if request.get? + session[:auth_source_registration] = nil @user = User.new(:language => Setting.default_language) else @user = User.new(params[:user]) @user.admin = false - @user.login = params[:user][:login] @user.status = User::STATUS_REGISTERED - @user.password, @user.password_confirmation = params[:password], params[:password_confirmation] - case Setting.self_registration - when '1' - # Email activation - token = Token.new(:user => @user, :action => "register") - if @user.save and token.save - Mailer.deliver_register(token) - flash[:notice] = l(:notice_account_register_done) - redirect_to :action => 'login' - end - when '3' - # Automatic activation + if session[:auth_source_registration] @user.status = User::STATUS_ACTIVE + @user.login = session[:auth_source_registration][:login] + @user.auth_source_id = session[:auth_source_registration][:auth_source_id] if @user.save + session[:auth_source_registration] = nil self.logged_user = @user flash[:notice] = l(:notice_account_activated) redirect_to :controller => 'my', :action => 'account' end else - # Manual activation by the administrator - if @user.save - # Sends an email to the administrators - Mailer.deliver_account_activation_request(@user) - flash[:notice] = l(:notice_account_pending) - redirect_to :action => 'login' + @user.login = params[:user][:login] + @user.password, @user.password_confirmation = params[:password], params[:password_confirmation] + case Setting.self_registration + when '1' + # Email activation + token = Token.new(:user => @user, :action => "register") + if @user.save and token.save + Mailer.deliver_register(token) + flash[:notice] = l(:notice_account_register_done) + redirect_to :action => 'login' + end + when '3' + # Automatic activation + @user.status = User::STATUS_ACTIVE + if @user.save + self.logged_user = @user + flash[:notice] = l(:notice_account_activated) + redirect_to :controller => 'my', :action => 'account' + end + else + # Manual activation by the administrator + if @user.save + # Sends an email to the administrators + Mailer.deliver_account_activation_request(@user) + flash[:notice] = l(:notice_account_pending) + redirect_to :action => 'login' + end end end end diff --git a/app/models/auth_source.rb b/app/models/auth_source.rb index 47c121a1..a0a2cdc5 100644 --- a/app/models/auth_source.rb +++ b/app/models/auth_source.rb @@ -20,10 +20,7 @@ class AuthSource < ActiveRecord::Base validates_presence_of :name validates_uniqueness_of :name - validates_length_of :name, :host, :maximum => 60 - validates_length_of :account_password, :maximum => 60, :allow_nil => true - validates_length_of :account, :base_dn, :maximum => 255 - validates_length_of :attr_login, :attr_firstname, :attr_lastname, :attr_mail, :maximum => 30 + validates_length_of :name, :maximum => 60 def authenticate(login, password) end diff --git a/app/models/auth_source_ldap.rb b/app/models/auth_source_ldap.rb index a438bd3c..655ffd6d 100644 --- a/app/models/auth_source_ldap.rb +++ b/app/models/auth_source_ldap.rb @@ -20,7 +20,10 @@ require 'iconv' class AuthSourceLdap < AuthSource validates_presence_of :host, :port, :attr_login - validates_presence_of :attr_firstname, :attr_lastname, :attr_mail, :if => Proc.new { |a| a.onthefly_register? } + validates_length_of :name, :host, :account_password, :maximum => 60, :allow_nil => true + validates_length_of :account, :base_dn, :maximum => 255, :allow_nil => true + validates_length_of :attr_login, :attr_firstname, :attr_lastname, :attr_mail, :maximum => 30, :allow_nil => true + validates_numericality_of :port, :only_integer => true def after_initialize self.port = 389 if self.port == 0 diff --git a/app/models/user.rb b/app/models/user.rb index 55fe3ac0..5a839721 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -103,19 +103,16 @@ class User < ActiveRecord::Base # user is not yet registered, try to authenticate with available sources attrs = AuthSource.authenticate(login, password) if attrs - onthefly = new(*attrs) - onthefly.login = login - onthefly.language = Setting.default_language - if onthefly.save - user = find(:first, :conditions => ["login=?", login]) + user = new(*attrs) + user.login = login + user.language = Setting.default_language + if user.save + user.reload logger.info("User '#{user.login}' created from the LDAP") if logger - else - logger.error("User '#{onthefly.login}' found in LDAP but could not be created (#{onthefly.errors.full_messages.join(', ')})") if logger - raise OnTheFlyCreationFailure.new end end end - user.update_attribute(:last_login_on, Time.now) if user + user.update_attribute(:last_login_on, Time.now) if user && !user.new_record? user rescue => text raise text diff --git a/app/views/account/register.rhtml b/app/views/account/register.rhtml index 4e2b5adf..755a7ad4 100644 --- a/app/views/account/register.rhtml +++ b/app/views/account/register.rhtml @@ -5,8 +5,9 @@
+<% if @user.auth_source_id.nil? %>

-<%= text_field 'user', 'login', :size => 25 %>

+<%= text_field 'user', 'login', :size => 25 %>

<%= password_field_tag 'password', nil, :size => 25 %>
@@ -14,6 +15,7 @@

<%= password_field_tag 'password_confirmation', nil, :size => 25 %>

+<% end %>

<%= text_field 'user', 'firstname' %>

diff --git a/app/views/auth_sources/_form.rhtml b/app/views/auth_sources/_form.rhtml index 3d148c11..9ffffafc 100644 --- a/app/views/auth_sources/_form.rhtml +++ b/app/views/auth_sources/_form.rhtml @@ -22,14 +22,12 @@

<%= text_field 'auth_source', 'base_dn', :size => 60 %>

-
-

<%= check_box 'auth_source', 'onthefly_register' %>

+
-

-

<%=l(:label_attribute_plural)%> +
<%=l(:label_attribute_plural)%>

<%= text_field 'auth_source', 'attr_login', :size => 20 %>

@@ -42,7 +40,5 @@

<%= text_field 'auth_source', 'attr_mail', :size => 20 %>

-

- diff --git a/test/integration/account_test.rb b/test/integration/account_test.rb index a01a3ba0..c349200d 100644 --- a/test/integration/account_test.rb +++ b/test/integration/account_test.rb @@ -17,6 +17,12 @@ require "#{File.dirname(__FILE__)}/../test_helper" +begin + require 'mocha' +rescue + # Won't run some tests +end + class AccountTest < ActionController::IntegrationTest fixtures :users @@ -102,4 +108,46 @@ class AccountTest < ActionController::IntegrationTest assert_redirected_to 'account/login' log_user('newuser', 'newpass') end + + if Object.const_defined?(:Mocha) + + def test_onthefly_registration + # disable registration + Setting.self_registration = '0' + AuthSource.expects(:authenticate).returns([:login => 'foo', :firstname => 'Foo', :lastname => 'Smith', :mail => 'foo@bar.com', :auth_source_id => 66]) + + post 'account/login', :username => 'foo', :password => 'bar' + assert_redirected_to 'my/page' + + user = User.find_by_login('foo') + assert user.is_a?(User) + assert_equal 66, user.auth_source_id + assert user.hashed_password.blank? + end + + def test_onthefly_registration_with_invalid_attributes + # disable registration + Setting.self_registration = '0' + AuthSource.expects(:authenticate).returns([:login => 'foo', :lastname => 'Smith', :auth_source_id => 66]) + + post 'account/login', :username => 'foo', :password => 'bar' + assert_response :success + assert_template 'account/register' + assert_tag :input, :attributes => { :name => 'user[firstname]', :value => '' } + assert_tag :input, :attributes => { :name => 'user[lastname]', :value => 'Smith' } + assert_no_tag :input, :attributes => { :name => 'user[login]' } + assert_no_tag :input, :attributes => { :name => 'user[password]' } + + post 'account/register', :user => {:firstname => 'Foo', :lastname => 'Smith', :mail => 'foo@bar.com'} + assert_redirected_to 'my/account' + + user = User.find_by_login('foo') + assert user.is_a?(User) + assert_equal 66, user.auth_source_id + assert user.hashed_password.blank? + end + + else + puts 'Mocha is missing. Skipping tests.' + end end