3153: tests with duplicate username

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 3c0332d9bac6b265596f2a8f7008e62d0f33d2fa..b15d441ce1d377e8660ca8f8fb0c4b34c410aa61 100644 (file)
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -12,8 +12,8 @@ class User < ArvadosModel
   before_update :prevent_inactive_admin
   before_create :check_auto_admin
   after_create :add_system_group_permission_link
-  after_create :send_admin_notifications
   after_create :auto_setup_new_user
+  after_create :send_admin_notifications
   after_update :send_profile_created_notification
 
 
@@ -423,28 +423,27 @@ class User < ArvadosModel
 
   # Automatically setup new user during creation
   def auto_setup_new_user
-    username = self.email.partition('@')[0] if self.email
-
     blacklisted_usernames = Rails.configuration.auto_setup_name_blacklist.split(', ')
 
+    username = self.email.partition('@')[0] if self.email
+
     if !Rails.configuration.auto_setup_new_users ||
        !(/^[_.A-Za-z0-9][-\@_.A-Za-z0-9]*\$?$/.match(self.email)) ||
        blacklisted_usernames.include?(username)
       return true
     else
-      # Derive repo name and username using the string before @ in user's email
-      # If a repo or vm login link with this prefix exists, generate unique string by appending a random number
       username = derive_unique_username username
-
       # setup user
-      setup_repo_vm_links(username, Rails.configuration.auto_setup_new_users_with_vm_uuid, Rails.configuration.default_openid_prefix)
+      setup_repo_vm_links(username, Rails.configuration.auto_setup_new_users_with_vm_uuid,
+                          Rails.configuration.default_openid_prefix)
     end
   end
 
-  # Derive repo name and username using the string before @ in user's email
-  # If a repo or vm login link with this prefix exists, generate unique string by appending a random number
+  # Derive repo name and vm username using the string before @ in user's email
+  # If a repo or vm login link with this username exists,
+  # generate unique string by appending a random number
   def derive_unique_username username
-      # no need to verify if vm login link or repo exists, if they both are not being created
+      # If repo and vm login link are not being created, no need to generate a unique username
       vm_uuid = Rails.configuration.auto_setup_new_users_with_vm_uuid
       if !vm_uuid && !Rails.configuration.auto_setup_new_users_with_repository
         return username

diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index 4125757b9d63968bbb3bc0ed2ed4aa0a48051ef5..4b71e483d1b8d87ee33365016f57c0e3487e74e0 100644 (file)
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -707,3 +707,17 @@ empty_collection_name_in_active_user_home_project:
   head_uuid: d41d8cd98f00b204e9800998ecf8427e+0
   properties: {}
   updated_at: 2014-08-06 22:11:51.242010312 Z
+
+xyz_can_login_to_vm_username_can_login_to_test_vm:
+  uuid: zzzzz-o0j2j-i3n6m98766tmoi4
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-08-06 22:11:51.242392533 Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  modified_at: 2014-08-06 22:11:51.242150425 Z
+  tail_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  link_class: permission
+  name: can_login
+  head_uuid: zzzzz-2x53u-382brsig8rp3064
+  properties: {username: 'xyz_can_login_to_vm'}
+  updated_at: 2014-08-06 22:11:51.242010312 Z

diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index c0a8cd87d1990b195333323f2dab77a2001417d9..bd450e4a5ebf457aeda8340d91cb7f333e32fe55 100644 (file)
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -153,10 +153,14 @@ class UserTest < ActiveSupport::TestCase
     [false, [], [], '@example.com', true, true, false],
     [false, [], [], '^^incorrect_format@example.com', true, true, false],
 
-#    [false, 'active-notify@example.com', 'inactive-notify@example.com', 'repeat_username@example.com', true, true, true],
-#    [false, 'active-notify@example.com', 'inactive-notify@example.com', 'repeat_username@example.com', true, false, true],
-#    [false, 'active-notify@example.com', 'inactive-notify@example.com', 'with existing repo name', true, false, true],
-#    [false, 'active-notify@example.com', 'inactive-notify@example.com', 'with existing vm login name', true, false, true],
+    [false, 'active-notify@example.com', 'inactive-notify@example.com', 'foo@example.com', true, true, true],  # existing repository name 'foo'
+    [false, 'active-notify@example.com', 'inactive-notify@example.com', 'foo@example.com', true, false, true],  # existing repository name 'foo'
+    [false, 'active-notify@example.com', 'inactive-notify@example.com', 'foo@example.com', false, true, true],  # existing repository name 'foo'
+    [false, 'active-notify@example.com', 'inactive-notify@example.com', 'foo@example.com', false, false, true],  # existing repository name 'foo', but we are not creating repo or login link
+    [false, 'active-notify@example.com', 'inactive-notify@example.com', 'xyz_can_login_to_vm@example.com', true, true, true], # existing vm login name
+    [false, 'active-notify@example.com', 'inactive-notify@example.com', 'xyz_can_login_to_vm@example.com', true, false, true], # existing vm login name
+    [false, 'active-notify@example.com', 'inactive-notify@example.com', 'xyz_can_login_to_vm@example.com', false, true, true], # existing vm login name
+    [false, 'active-notify@example.com', 'inactive-notify@example.com', 'xyz_can_login_to_vm@example.com', false, false, true], # existing vm login name, but we are not creating repo or login link
   ].each do |active, active_recipients, inactive_recipients, email, auto_setup_vm, auto_setup_repo, valid_email_format|
     test "create new user with auto setup #{email} #{auto_setup_vm} #{auto_setup_repo}" do
       auto_setup_new_users = Rails.configuration.auto_setup_new_users
@@ -458,11 +462,14 @@ class UserTest < ActiveSupport::TestCase
 
       # check repo
       if Rails.configuration.auto_setup_new_users_with_repository
-        repo = Repository.where(name: username).first
-        assert_not_nil repo, 'repository not found'
-        verify_link_exists true, repo[:uuid], user.uuid, 'permission', 'can_manage', nil, nil
-      else
-        verify_link_exists false, nil, user.uuid, 'permission', 'can_manage', nil, nil
+        repos = Repository.where('name like ?', "%#{username}%")
+        assert_not_nil repos, 'repository not found'
+        assert_equal true, repos.any?, 'repository not found'
+        repo_uuids = []
+        repos.each do |repo|
+          repo_uuids << repo[:uuid]
+        end
+        verify_link_exists true, repo_uuids, user.uuid, 'permission', 'can_manage', nil, nil
       end
     end
 
@@ -509,9 +516,11 @@ class UserTest < ActiveSupport::TestCase
                            tail_uuid: tail_uuid,
                            link_class: link_class,
                            name: link_name)
-    assert_equal link_exists, all_links.any?, "Link not found"
+    assert_equal link_exists, all_links.any?, "Link #{'not' if link_exists} found #{property_value}"
     if link_exists && property_name && property_value
-      assert_equal property_value, all_links.first.properties[property_name], 'Property not found in link'
+      all_links.each do |link|
+        assert_equal true, all_links.first.properties[property_name].start_with?(property_value), 'Property not found in link'
+      end
     end
   end