[36081] Increase max lengths of user/group names (#8948)

* Unset DB limits for user attributes

* Validate groups in synchronized groups

* Extend limitations and fix specs

app/models/group.rb

@@ -40,7 +40,7 @@ class Group < Principal
   alias_attribute(:groupname, :lastname)
   validates_presence_of :groupname
   validate :uniqueness_of_groupname
-  validates_length_of :groupname, maximum: 30
+  validates_length_of :groupname, maximum: 256
 
   # HACK: We want to have the :preference association on the Principal to allow
   # for eager loading preferences.

app/models/user.rb

@@ -129,9 +129,9 @@ class User < Principal
   # Login must contain letters, numbers, underscores only
   validates_format_of :login, with: /\A[a-z0-9_\-@\.+ ]*\z/i
   validates_length_of :login, maximum: 256
-  validates_length_of :firstname, :lastname, maximum: 30
+  validates_length_of :firstname, :lastname, maximum: 256
   validates_format_of :mail, with: /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i, allow_blank: true
-  validates_length_of :mail, maximum: 60, allow_nil: true
+  validates_length_of :mail, maximum: 256, allow_nil: true
   validates_confirmation_of :password, allow_nil: true
   validates_inclusion_of :mail_notification, in: MAIL_NOTIFICATION_OPTIONS.map(&:first), allow_blank: true
 

db/migrate/20210127134438_alter_user_attributes_max_length.rb

@@ -0,0 +1,13 @@
+class AlterUserAttributesMaxLength < ActiveRecord::Migration[6.0]
+  def up
+    change_column :users, :firstname, :string, limit: nil
+    change_column :users, :lastname, :string, limit: nil
+    change_column :users, :mail, :string, limit: nil
+  end
+
+  def down
+    change_column :users, :firstname, :string, limit: 30
+    change_column :users, :lastname, :string, limit: 30
+    change_column :users, :mail, :string, limit: 60
+  end
+end

modules/ldap_groups/app/models/ldap_groups/synchronized_group.rb

@@ -18,6 +18,7 @@ module LdapGroups
 
     validates_presence_of :dn
     validates_presence_of :group
+    validates_associated :group
     validates_presence_of :auth_source
 
     before_destroy :remove_all_members

modules/ldap_groups/lib/open_project/ldap_groups/synchronize_filter.rb

@@ -77,7 +77,7 @@ module OpenProject::LdapGroups
         Group.where(id: sync.group_id).update_all(lastname: name)
       else
         # Create an OpenProject group
-        sync.group = Group.find_or_initialize_by(groupname: name)
+        sync.group = Group.find_or_create_by!(groupname: name)
       end
     end
 

spec/models/group_spec.rb

@@ -49,6 +49,29 @@ describe Group, type: :model do
     expect(g.save).to eq true
   end
 
+  describe 'with long but allowed attributes' do
+    it 'is valid' do
+      group.groupname = 'a' * 256
+      expect(group).to be_valid
+      expect(group.save).to be_truthy
+    end
+  end
+
+  describe 'with a name too long' do
+    it 'is invalid' do
+      group.groupname = 'a' * 257
+      expect(group).not_to be_valid
+      expect(group.save).to be_falsey
+    end
+  end
+
+  describe 'a user with and overly long firstname (> 256 chars)' do
+    it 'is invalid' do
+      user.firstname = 'a' * 257
+      expect(user).not_to be_valid
+      expect(user.save).to be_falsey
+    end
+  end
 
   describe 'from legacy specs' do
     let!(:roles) { FactoryBot.create_list :role, 2 }

spec/models/user_spec.rb

@@ -98,6 +98,34 @@ describe User, type: :model do
     end
   end
 
+  describe 'with long but allowed attributes' do
+    it 'is valid' do
+      user.firstname = 'a' * 256
+      user.lastname = 'b' * 256
+      user.mail = 'fo' + ('o' * 237) + '@mail.example.com'
+      expect(user).to be_valid
+      expect(user.save).to be_truthy
+    end
+  end
+
+
+  describe 'a user with and overly long firstname (> 256 chars)' do
+    it 'is invalid' do
+      user.firstname = 'a' * 257
+      expect(user).not_to be_valid
+      expect(user.save).to be_falsey
+    end
+  end
+
+  describe 'a user with and overly long lastname (> 256 chars)' do
+    it 'is invalid' do
+      user.lastname = 'a' * 257
+      expect(user).not_to be_valid
+      expect(user.save).to be_falsey
+    end
+  end
+
+
   describe 'login whitespace' do
     before do
       user.login = login

spec_legacy/unit/mail_handler_spec.rb

@@ -386,11 +386,8 @@ describe MailHandler, type: :model do
       ['jsmith@example.net', 'John'] => ['jsmith@example.net', 'John', '-'],
       ['jsmith@example.net', 'John Smith'] => ['jsmith@example.net', 'John', 'Smith'],
       ['jsmith@example.net', 'John Paul Smith'] => ['jsmith@example.net', 'John', 'Paul Smith'],
-      # TODO: implement https://github.com/redmine/redmine/commit/a00f04886fac78e489bb030d20414ebdf10841e3
-      # ['jsmith@example.net', 'AVeryLongFirstnameThatExceedsTheMaximumLength Smith'] => ['jsmith@example.net', 'AVeryLongFirstnameThatExceedsT', 'Smith'],
-      # ['jsmith@example.net', 'John AVeryLongLastnameThatExceedsTheMaximumLength'] => ['jsmith@example.net', 'John', 'AVeryLongLastnameThatExceedsTh']
-      ['jsmith@example.net', 'AVeryLongFirstnameThatExceedsTheMaximumLength Smith'] => ['jsmith@example.net', '-', 'Smith'],
-      ['jsmith@example.net', 'John AVeryLongLastnameThatExceedsTheMaximumLength'] => ['jsmith@example.net', 'John', '-']
+      ['jsmith@example.net', 'AVeryLongFirstnameThatNoLongerExceedsTheMaximumLength Smith'] => ['jsmith@example.net', 'AVeryLongFirstnameThatNoLongerExceedsTheMaximumLength', 'Smith'],
+      ['jsmith@example.net', 'John AVeryLongLastnameThatNoLongerExceedsTheMaximumLength'] => ['jsmith@example.net', 'John', 'AVeryLongLastnameThatNoLongerExceedsTheMaximumLength']
     }
 
     to_test.each do |attrs, expected|