jro_apps/redmine Commit - r1661:eb1d969237a9

Improved on-the-fly account creation. If some attributes are missing (eg. not present in the LDAP) or are invalid, the registration form is displayed so that the user is able to fill or fix these attributes....

Jean-Philippe Lang -

r1661:eb1d969237a9

parent child

Context file:

r1661:eb1d969237a9

Collapse all files

app/controllers/account_controller.rb +43 -25

@@ -44,7 +44,16 class AccountController < ApplicationController
44	else	44	else
45	# Authenticate user	45	# Authenticate user
46	user = User.try_to_login(params[:username], params[:password])	46	user = User.try_to_login(params[:username], params[:password])
47	if user	47	if user.nil?
		48	# Invalid credentials
		49	flash.now[:error] = l(:notice_account_invalid_creditentials)
		50	elsif user.new_record?
		51	# Onthefly creation failed, display the registration form to fill/fix attributes
		52	@user = user
		53	session[:auth_source_registration] = {:login => user.login, :auth_source_id => user.auth_source_id }
		54	render :action => 'register'
		55	else
		56	# Valid user
48	self.logged_user = user	57	self.logged_user = user
49	# generate a key and set cookie if autologin	58	# generate a key and set cookie if autologin
50	if params[:autologin] && Setting.autologin?	59	if params[:autologin] && Setting.autologin?
@@ -52,12 +61,8 class AccountController < ApplicationController
52	cookies[:autologin] = { :value => token.value, :expires => 1.year.from_now }	61	cookies[:autologin] = { :value => token.value, :expires => 1.year.from_now }
53	end	62	end
54	redirect_back_or_default :controller => 'my', :action => 'page'	63	redirect_back_or_default :controller => 'my', :action => 'page'
55	else
56	flash.now[:error] = l(:notice_account_invalid_creditentials)
57	end	64	end
58	end	65	end
59	rescue User::OnTheFlyCreationFailure
60	flash.now[:error] = 'Redmine could not retrieve the required information from the LDAP to create your account. Please, contact your Redmine administrator.'
61	end	66	end
62		67
63	# Log out current user and redirect to welcome page	68	# Log out current user and redirect to welcome page
@@ -107,39 +112,52 class AccountController < ApplicationController
107		112
108	# User self-registration	113	# User self-registration
109	def register	114	def register
110	redirect_to(home_url) && return unless Setting.self_registration?	115	redirect_to(home_url) && return unless Setting.self_registration? \|\| session[:auth_source_registration]
111	if request.get?	116	if request.get?
		117	session[:auth_source_registration] = nil
112	@user = User.new(:language => Setting.default_language)	118	@user = User.new(:language => Setting.default_language)
113	else	119	else
114	@user = User.new(params[:user])	120	@user = User.new(params[:user])
115	@user.admin = false	121	@user.admin = false
116	@user.login = params[:user][:login]
117	@user.status = User::STATUS_REGISTERED	122	@user.status = User::STATUS_REGISTERED
118	@user.password, @user.password_confirmation = params[:password], params[:password_confirmation]	123	if session[:auth_source_registration]
119	case Setting.self_registration
120	when '1'
121	# Email activation
122	token = Token.new(:user => @user, :action => "register")
123	if @user.save and token.save
124	Mailer.deliver_register(token)
125	flash[:notice] = l(:notice_account_register_done)
126	redirect_to :action => 'login'
127	end
128	when '3'
129	# Automatic activation
130	@user.status = User::STATUS_ACTIVE	124	@user.status = User::STATUS_ACTIVE
		125	@user.login = session[:auth_source_registration][:login]
		126	@user.auth_source_id = session[:auth_source_registration][:auth_source_id]
131	if @user.save	127	if @user.save
		128	session[:auth_source_registration] = nil
132	self.logged_user = @user	129	self.logged_user = @user
133	flash[:notice] = l(:notice_account_activated)	130	flash[:notice] = l(:notice_account_activated)
134	redirect_to :controller => 'my', :action => 'account'	131	redirect_to :controller => 'my', :action => 'account'
135	end	132	end
136	else	133	else
137	# Manual activation by the administrator	134	@user.login = params[:user][:login]
138	if @user.save	135	@user.password, @user.password_confirmation = params[:password], params[:password_confirmation]
139	# Sends an email to the administrators	136	case Setting.self_registration
140	Mailer.deliver_account_activation_request(@user)	137	when '1'
141	flash[:notice] = l(:notice_account_pending)	138	# Email activation
142	redirect_to :action => 'login'	139	token = Token.new(:user => @user, :action => "register")
		140	if @user.save and token.save
		141	Mailer.deliver_register(token)
		142	flash[:notice] = l(:notice_account_register_done)
		143	redirect_to :action => 'login'
		144	end
		145	when '3'
		146	# Automatic activation
		147	@user.status = User::STATUS_ACTIVE
		148	if @user.save
		149	self.logged_user = @user
		150	flash[:notice] = l(:notice_account_activated)
		151	redirect_to :controller => 'my', :action => 'account'
		152	end
		153	else
		154	# Manual activation by the administrator
		155	if @user.save
		156	# Sends an email to the administrators
		157	Mailer.deliver_account_activation_request(@user)
		158	flash[:notice] = l(:notice_account_pending)
		159	redirect_to :action => 'login'
		160	end
143	end	161	end
144	end	162	end
145	end	163	end

app/models/auth_source.rb +1 -4

               validates_presence_of :name
               validates_uniqueness_of :name
-              validates_length_of :name, :host, :maximum => 60
+              validates_length_of :name, :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
               def authenticate(login, password)
               end

app/models/auth_source_ldap.rb +4 -1

             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

app/models/user.rb +6 -9

                   # user is not yet registered, try to authenticate with available sources
                   attrs = AuthSource.authenticate(login, password)
                   if attrs
-                    onthefly = new(*attrs)
+                    user = new(*attrs)
-                    onthefly.login = login
+                    user.login = login
-                    onthefly.language = Setting.default_language
+                    user.language = Setting.default_language
-                    if onthefly.save
+                    if user.save
-                      user = find(:first, :conditions => ["login=?", login])
+                      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

app/views/account/register.rhtml +3 -1

             <div class="box">
             <!--[form:user]-->
+            <% if @user.auth_source_id.nil? %>
             <p><label for="user_login"><%=l(:field_login)%> <span class="required">*</span></label>
-            <%= text_field 'user', 'login', :size => 25  %></p>
+            <%= text_field 'user', 'login', :size => 25 %></p>
             <p><label for="password"><%=l(:field_password)%> <span class="required">*</span></label>
             <%= password_field_tag 'password', nil, :size => 25  %><br />
             <p><label for="password_confirmation"><%=l(:field_password_confirmation)%> <span class="required">*</span></label>
             <%= password_field_tag 'password_confirmation', nil, :size => 25  %></p>
+            <% end %>
             <p><label for="user_firstname"><%=l(:field_firstname)%> <span class="required">*</span></label>
             <%= text_field 'user', 'firstname'  %></p>

app/views/auth_sources/_form.rhtml +2 -6

             <p><label for="auth_source_base_dn"><%=l(:field_base_dn)%> <span class="required">*</span></label>
             <%= text_field 'auth_source', 'base_dn', :size => 60 %></p>
-            </div>
-            <div class="box">
             <p><label for="auth_source_onthefly_register"><%=l(:field_onthefly)%></label>
             <%= check_box 'auth_source', 'onthefly_register' %></p>
+            </div>
-            <p>
+            <fieldset class="box"><legend><%=l(:label_attribute_plural)%></legend>
-            <fieldset><legend><%=l(:label_attribute_plural)%></legend>
             <p><label for="auth_source_attr_login"><%=l(:field_login)%> <span class="required">*</span></label>
             <%= text_field 'auth_source', 'attr_login', :size => 20  %></p>
             <p><label for="auth_source_attr_mail"><%=l(:field_mail)%></label>
             <%= text_field 'auth_source', 'attr_mail', :size => 20  %></p>
             </fieldset>
-            </p>
-            </div>
             <!--[eoform:auth_source]-->

test/integration/account_test.rb +48 0

@@ -17,6 +17,12
17		17
18	require "#{File.dirname(__FILE__)}/../test_helper"	18	require "#{File.dirname(__FILE__)}/../test_helper"
19		19
		20	begin
		21	require 'mocha'
		22	rescue
		23	# Won't run some tests
		24	end
		25
20	class AccountTest < ActionController::IntegrationTest	26	class AccountTest < ActionController::IntegrationTest
21	fixtures :users	27	fixtures :users
22		28
@@ -102,4 +108,46 class AccountTest < ActionController::IntegrationTest
102	assert_redirected_to 'account/login'	108	assert_redirected_to 'account/login'
103	log_user('newuser', 'newpass')	109	log_user('newuser', 'newpass')
104	end	110	end
		111
		112	if Object.const_defined?(:Mocha)
		113
		114	def test_onthefly_registration
		115	# disable registration
		116	Setting.self_registration = '0'
		117	AuthSource.expects(:authenticate).returns([:login => 'foo', :firstname => 'Foo', :lastname => 'Smith', :mail => 'foo@bar.com', :auth_source_id => 66])
		118
		119	post 'account/login', :username => 'foo', :password => 'bar'
		120	assert_redirected_to 'my/page'
		121
		122	user = User.find_by_login('foo')
		123	assert user.is_a?(User)
		124	assert_equal 66, user.auth_source_id
		125	assert user.hashed_password.blank?
		126	end
		127
		128	def test_onthefly_registration_with_invalid_attributes
		129	# disable registration
		130	Setting.self_registration = '0'
		131	AuthSource.expects(:authenticate).returns([:login => 'foo', :lastname => 'Smith', :auth_source_id => 66])
		132
		133	post 'account/login', :username => 'foo', :password => 'bar'
		134	assert_response :success
		135	assert_template 'account/register'
		136	assert_tag :input, :attributes => { :name => 'user[firstname]', :value => '' }
		137	assert_tag :input, :attributes => { :name => 'user[lastname]', :value => 'Smith' }
		138	assert_no_tag :input, :attributes => { :name => 'user[login]' }
		139	assert_no_tag :input, :attributes => { :name => 'user[password]' }
		140
		141	post 'account/register', :user => {:firstname => 'Foo', :lastname => 'Smith', :mail => 'foo@bar.com'}
		142	assert_redirected_to 'my/account'
		143
		144	user = User.find_by_login('foo')
		145	assert user.is_a?(User)
		146	assert_equal 66, user.auth_source_id
		147	assert user.hashed_password.blank?
		148	end
		149
		150	else
		151	puts 'Mocha is missing. Skipping tests.'
		152	end
105	end	153	end

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages