diff --git a/app/models/setting.rb b/app/models/setting.rb index b5c8ca2e5..384b6e866 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -19,6 +19,13 @@ class Setting < ActiveRecord::Base + PASSWORD_CHAR_CLASSES = { + 'uppercase' => /[A-Z]/, + 'lowercase' => /[a-z]/, + 'digits' => /[0-9]/, + 'special_chars' => /[[:ascii:]&&[:graph:]&&[:^alnum:]]/ + } + DATE_FORMATS = [ '%Y-%m-%d', '%d/%m/%Y', diff --git a/app/models/user.rb b/app/models/user.rb index 9003b253f..d123f6ed8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -112,6 +112,9 @@ class User < Principal validates_length_of :firstname, :lastname, :maximum => 30 validates_length_of :identity_url, maximum: 255 validates_inclusion_of :mail_notification, :in => MAIL_NOTIFICATION_OPTIONS.collect(&:first), :allow_blank => true + Setting::PASSWORD_CHAR_CLASSES.each do |k, v| + validates_format_of :password, :with => v, :message => :"must_include_#{k}", :allow_blank => true, :if => Proc.new { Setting.password_required_char_classes.include?(k) } + end validate :validate_password_length validate do if password_confirmation && password != password_confirmation @@ -366,10 +369,24 @@ class User < Principal # Generate and set a random password on given length def random_password(length=40) - chars = ("a".."z").to_a + ("A".."Z").to_a + ("0".."9").to_a - chars -= %w(0 O 1 l) + chars_list = [ + ('A'..'Z').to_a, + ('a'..'z').to_a, + ('0'..'9').to_a, + ] + if Setting.password_required_char_classes.include?('special_chars') + chars_list << ("\x20".."\x7e").to_a.select {|c| c =~ Setting::PASSWORD_CHAR_CLASSES['special_chars']} + end + chars_list.each {|v| v.reject! {|c| %(0O1l|'"`*).include?(c)}} + password = +'' - length.times {|i| password << chars[SecureRandom.random_number(chars.size)] } + chars_list.each do |chars| + password << chars[SecureRandom.random_number(chars.size)] + length -= 1 + end + chars = chars_list.flatten + length.times { password << chars[SecureRandom.random_number(chars.size)] } + password = password.split('').shuffle(random: SecureRandom).join self.password = password self.password_confirmation = password self diff --git a/app/views/account/password_recovery.html.erb b/app/views/account/password_recovery.html.erb index 24da8223b..9eeea3119 100644 --- a/app/views/account/password_recovery.html.erb +++ b/app/views/account/password_recovery.html.erb @@ -9,6 +9,9 @@ <%= password_field_tag 'new_password', nil, :size => 25 %> <%= l(:text_caracters_minimum, :count => Setting.password_min_length) %> + <% if Setting.password_required_char_classes.any? %> + <%= l(:text_characters_must_include, :character_classes => Setting.password_required_char_classes.collect{|c| l("label_password_char_class_#{c}")}.join(", ")) %> + <% end %>

diff --git a/app/views/account/register.html.erb b/app/views/account/register.html.erb index ade00adfc..770b94cd7 100644 --- a/app/views/account/register.html.erb +++ b/app/views/account/register.html.erb @@ -8,8 +8,11 @@

<%= f.text_field :login, :size => 25, :required => true %>

<%= f.password_field :password, :size => 25, :required => true %> - <%= l(:text_caracters_minimum, :count => Setting.password_min_length) %>

- + <%= l(:text_caracters_minimum, :count => Setting.password_min_length) %> + <% if Setting.password_required_char_classes.any? %> + <%= l(:text_characters_must_include, :character_classes => Setting.password_required_char_classes.collect{|c| l("label_password_char_class_#{c}")}.join(", ")) %> + <% end %> +

<%= f.password_field :password_confirmation, :size => 25, :required => true %>

<% end %> diff --git a/app/views/my/password.html.erb b/app/views/my/password.html.erb index 7a411e51a..2771ce5fc 100644 --- a/app/views/my/password.html.erb +++ b/app/views/my/password.html.erb @@ -9,7 +9,11 @@

<%= password_field_tag 'new_password', nil, :size => 25 %> -<%= l(:text_caracters_minimum, :count => Setting.password_min_length) %>

+ <%= l(:text_caracters_minimum, :count => Setting.password_min_length) %> + <% if Setting.password_required_char_classes.any? %> + <%= l(:text_characters_must_include, :character_classes => Setting.password_required_char_classes.collect{|c| l("label_password_char_class_#{c}")}.join(", ")) %> + <% end %> +

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

diff --git a/app/views/settings/_authentication.html.erb b/app/views/settings/_authentication.html.erb index 4bf890d3f..9a39497b8 100644 --- a/app/views/settings/_authentication.html.erb +++ b/app/views/settings/_authentication.html.erb @@ -20,6 +20,8 @@

<%= setting_text_field :password_min_length, :size => 6 %>

+

<%= setting_multiselect :password_required_char_classes, Setting::PASSWORD_CHAR_CLASSES.keys.collect {|c| [l("label_password_char_class_#{c}"), c]} , :inline => true %>

+

<%= setting_select :password_max_age, [[l(:label_disabled), 0]] + [7, 30, 60, 90, 180, 365].collect{|days| [l('datetime.distance_in_words.x_days', :count => days), days.to_s]} %>

diff --git a/app/views/users/_form.html.erb b/app/views/users/_form.html.erb index ce5b1f6c7..8d245d707 100644 --- a/app/views/users/_form.html.erb +++ b/app/views/users/_form.html.erb @@ -31,8 +31,13 @@

<%= f.select :auth_source_id, ([[l(:label_internal), ""]] + @auth_sources.collect { |a| [a.name, a.id] }), {}, :onchange => "if (this.value=='') {$('#password_fields').show();} else {$('#password_fields').hide();}" %>

<% end %>
-

<%= f.password_field :password, :required => true, :size => 25 %> - <%= l(:text_caracters_minimum, :count => Setting.password_min_length) %>

+

+ <%= f.password_field :password, :required => true, :size => 25 %> + <%= l(:text_caracters_minimum, :count => Setting.password_min_length) %> + <% if Setting.password_required_char_classes.any? %> + <%= l(:text_characters_must_include, :character_classes => Setting.password_required_char_classes.collect{|c| l("label_password_char_class_#{c}")}.join(", ")) %> + <% end %> +

<%= f.password_field :password_confirmation, :required => true, :size => 25 %>

<%= f.check_box :generate_password %>

<%= f.check_box :must_change_passwd %>

diff --git a/config/locales/en.yml b/config/locales/en.yml index b4f37f0b7..2e8b4a45f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -132,6 +132,10 @@ en: earlier_than_minimum_start_date: "cannot be earlier than %{date} because of preceding issues" not_a_regexp: "is not a valid regular expression" open_issue_with_closed_parent: "An open issue cannot be attached to a closed parent task" + must_include_uppercase: "must include uppercase (A-Z)" + must_include_lowercase: "must include lowercase (a-z)" + must_include_digits: "must include digits (0-9)" + must_include_special_chars: "must include special characters (!, $, %, ...)" actionview_instancetag_blank_option: Please select @@ -437,6 +441,7 @@ en: setting_openid: Allow OpenID login and registration setting_password_max_age: Require password change after setting_password_min_length: Minimum password length + setting_password_required_char_classes : Required character classes for passwords setting_lost_password: Allow password reset via email setting_new_project_user_role_id: Role given to a non-admin user who creates a project setting_default_projects_modules: Default enabled modules for new projects @@ -1061,6 +1066,10 @@ en: label_issue_history_properties: Property changes label_issue_history_notes: Notes label_last_tab_visited: Last visited tab + label_password_char_class_uppercase: Uppercase + label_password_char_class_lowercase: Lowercase + label_password_char_class_digits: Digits + label_password_char_class_special_chars: Special characters button_login: Login button_submit: Submit @@ -1152,6 +1161,7 @@ en: text_project_identifier_info: 'Only lower case letters (a-z), numbers, dashes and underscores are allowed.
Once saved, the identifier cannot be changed.' text_caracters_maximum: "%{count} characters maximum." text_caracters_minimum: "Must be at least %{count} characters long." + text_characters_must_include: "Must include %{character_classes}." text_length_between: "Length between %{min} and %{max} characters." text_tracker_no_workflow: No workflow defined for this tracker text_role_no_workflow: No workflow defined for this role diff --git a/config/settings.yml b/config/settings.yml index 6acfd9769..96b6eca7f 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -36,6 +36,9 @@ lost_password: security_notifications: 1 unsubscribe: default: 1 +password_required_char_classes: + serialized: true + default: [] password_min_length: format: int default: 8 diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index e03528809..519de8b04 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -539,6 +539,18 @@ class UserTest < ActiveSupport::TestCase end end + def test_validate_password_format + Setting::PASSWORD_CHAR_CLASSES.each do |key, regexp| + with_settings :password_required_char_classes => key do + user = User.new(:firstname => "new", :lastname => "user", :login => "random", :mail => "random@somnet.foo") + p = 'PASSWDpasswd01234!@#$%'.gsub(regexp, '') + user.password, user.password_confirmation = p, p + assert !user.save + assert_equal 1, user.errors.count + end + end + end + def test_name_format assert_equal 'John S.', @jsmith.name(:firstname_lastinitial) assert_equal 'Smith, John', @jsmith.name(:lastname_comma_firstname) @@ -1058,6 +1070,14 @@ class UserTest < ActiveSupport::TestCase assert !u.password_confirmation.blank? end + def test_random_password_include_required_characters + with_settings :password_required_char_classes => Setting::PASSWORD_CHAR_CLASSES do + u = User.new(:firstname => "new", :lastname => "user", :login => "random", :mail => "random@somnet.foo") + u.random_password + assert u.valid? + end + end + test "#change_password_allowed? should be allowed if no auth source is set" do user = User.generate! assert user.change_password_allowed?