[ARVADOS] created: fa92aa4a1db149f2938351ae1220f5d1a08a3b8e
git at public.curoverse.com
git at public.curoverse.com
Thu Mar 19 12:00:06 EDT 2015
at fa92aa4a1db149f2938351ae1220f5d1a08a3b8e (commit)
commit fa92aa4a1db149f2938351ae1220f5d1a08a3b8e
Author: Brett Smith <brett at curoverse.com>
Date: Wed Mar 18 16:00:01 2015 -0400
4253: Use new username to set up repository and VM logins.
The usernames added in 4253 have stricter limits than past usernames
generated to set up a repository and VM login. Use the new generated
username to avoid a weird disconnect between that and these related
objects. Do a little cleanup in the tests, including removing some
test parameters that now seem redundant under the new rules.
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index d4d722f..c395d23 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -22,7 +22,11 @@ class User < ArvadosModel
user.username.nil? and user.email
}
after_create :add_system_group_permission_link
- after_create :auto_setup_new_user
+ after_create :auto_setup_new_user, :if => Proc.new { |user|
+ Rails.configuration.auto_setup_new_users and
+ (user.uuid != system_user_uuid) and
+ (user.uuid != anonymous_user_uuid)
+ }
after_create :send_admin_notifications
after_update :send_profile_created_notification
@@ -459,44 +463,16 @@ class User < ArvadosModel
# Automatically setup new user during creation
def auto_setup_new_user
- return true if !Rails.configuration.auto_setup_new_users
- return true if !self.email
- return true if self.uuid == system_user_uuid
- return true if self.uuid == anonymous_user_uuid
-
- if Rails.configuration.auto_setup_new_users_with_vm_uuid ||
- Rails.configuration.auto_setup_new_users_with_repository
- username = self.email.partition('@')[0] if self.email
- return true if !username
-
- blacklisted_usernames = Rails.configuration.auto_setup_name_blacklist
- if blacklisted_usernames.include?(username)
- return true
- elsif !(/^[a-zA-Z][-._a-zA-Z0-9]{0,30}[a-zA-Z0-9]$/.match(username))
- return true
- else
- return true if !(username = derive_unique_username username)
- end
- end
-
- # setup user
- setup_repo_vm_links(username,
- Rails.configuration.auto_setup_new_users_with_vm_uuid,
- Rails.configuration.default_openid_prefix)
- end
-
- # Find a username that starts with the given string and does not collide
- # with any existing repository name or VM login name
- def derive_unique_username username
- while true
- if Repository.where(name: username).empty?
- login_collisions = Link.where(link_class: 'permission',
- name: 'can_login').select do |perm|
- perm.properties['username'] == username
- end
- return username if login_collisions.empty?
+ setup_repo_vm_links(nil, nil, Rails.configuration.default_openid_prefix)
+ if username
+ create_vm_login_permission_link(Rails.configuration.auto_setup_new_users_with_vm_uuid,
+ username)
+ if Rails.configuration.auto_setup_new_users_with_repository and
+ Repository.where(name: username).first.nil?
+ repo = Repository.create!(name: username)
+ Link.create!(tail_uuid: uuid, head_uuid: repo.uuid,
+ link_class: "permission", name: "can_manage")
end
- username = username + SecureRandom.random_number(10).to_s
end
end
diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index 9ba5646..6eabdfd 100644
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -294,106 +294,60 @@ class UserTest < ActiveSupport::TestCase
test "create new user with notifications" do
set_user_from_auth :admin
- create_user_and_verify_setup_and_notifications true, 'active-notify-address at example.com', 'inactive-notify-address at example.com', nil, false
- create_user_and_verify_setup_and_notifications true, 'active-notify-address at example.com', [], nil, false
- create_user_and_verify_setup_and_notifications true, [], [], nil, false
- create_user_and_verify_setup_and_notifications false, 'active-notify-address at example.com', 'inactive-notify-address at example.com', nil, false
- create_user_and_verify_setup_and_notifications false, [], 'inactive-notify-address at example.com', nil, false
- create_user_and_verify_setup_and_notifications false, [], [], nil, false
+ create_user_and_verify_setup_and_notifications true, 'active-notify-address at example.com', 'inactive-notify-address at example.com', nil, nil
+ create_user_and_verify_setup_and_notifications true, 'active-notify-address at example.com', [], nil, nil
+ create_user_and_verify_setup_and_notifications true, [], [], nil, nil
+ create_user_and_verify_setup_and_notifications false, 'active-notify-address at example.com', 'inactive-notify-address at example.com', nil, nil
+ create_user_and_verify_setup_and_notifications false, [], 'inactive-notify-address at example.com', nil, nil
+ create_user_and_verify_setup_and_notifications false, [], [], nil, nil
end
[
- [false, [], [], 'inactive-none at example.com', false, false, true],
- [false, [], [], 'inactive-vm at example.com', true, false, true],
- [false, [], [], 'inactive-repo at example.com', false, true, true],
- [false, [], [], 'inactive-both at example.com', true, true, true],
-
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'active-none at example.com', false, false, true],
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'active-vm at example.com', true, false, true],
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'active-repo at example.com', false, true, true],
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'active-both at example.com', true, true, true],
-
- [false, [], [], nil, true, true, false],
-
- [false, [], [], 'arvados', true, true, false],
- [false, [], [], 'arvados', true, false, false], # blacklisted username
- [false, [], [], 'arvados', false, false, true], # since we are not creating repo and vm login, this blacklisted name is not a problem
-
- [false, [], [], 'arvados at example.com', false, false, true], # since we are not creating repo and vm login, this blacklisted name is not a problem
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'arvados at example.com', false, false, true], # since we are not creating repo and vm login, this blacklisted name is not a problem
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'root at example.com', true, false, false], # blacklisted name
- [false, 'active-notify at example.com', 'inactive-notify at example.com', 'root at example.com', true, false, false], # blacklisted name
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'roo_t at example.com', false, true, true], # not blacklisted name
-
- [false, [], [], '@example.com', true, false, false], # incorrect format
- [false, [], [], '@example.com', false, true, false],
- [false, [], [], '@example.com', false, false, true], # no repo and vm login, so no issue with email format
-
- [false, [], [], '^^incorrect_format at example.com', true, true, false],
-
- [false, 'active-notify at example.com', 'inactive-notify at example.com', 'auto_setup_repo at example.com', true, true, true], # existing repository name 'auto_setup_repo'
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'auto_setup_repo at example.com', true, false, true], # existing repository name 'auto_setup_repo'
- [false, 'active-notify at example.com', 'inactive-notify at example.com', 'auto_setup_repo at example.com', false, true, true], # existing repository name 'auto_setup_repo'
- [false, 'active-notify at example.com', 'inactive-notify at example.com', 'auto_setup_repo at example.com', false, false, true], # existing repository name 'auto_setup_repo', but we are not creating repo or login link
-
- [false, 'active-notify at example.com', 'inactive-notify at example.com', 'auto_setup_vm_login at example.com', true, true, true], # existing vm login name
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'auto_setup_vm_login at example.com', true, false, true], # existing vm login name
- [false, 'active-notify at example.com', 'inactive-notify at example.com', 'auto_setup_vm_login at example.com', false, true, true], # existing vm login name
- [false, 'active-notify at example.com', 'inactive-notify at example.com', 'auto_setup_vm_login at example.com', false, false, true], # existing vm login name, but we are not creating repo or login link
-
- [true, 'active-notify at example.com', 'inactive-notify at example.com', '*!*@example.com', true, false, false], # username is invalid format
- [false, 'active-notify at example.com', 'inactive-notify at example.com', '*!*@example.com', false, false, true], # since no repo and vm login, username is ok (not validated)
- [true, 'active-notify at example.com', 'inactive-notify at example.com', '*!*@example.com', false, false, true], # since no repo and vm login, username is ok (not validated)
-
- [true, 'active-notify at example.com', 'inactive-notify at example.com', '&4ad at example.com', true, true, false], # username is invalid format
- [true, 'active-notify at example.com', 'inactive-notify at example.com', '&4ad at example.com', false, false, true], # no repo or vm login, so format not checked
- [false, 'active-notify at example.com', 'inactive-notify at example.com', '&4ad at example.com', true, true, false], # username is invalid format
- [false, 'active-notify at example.com', 'inactive-notify at example.com', '&4ad at example.com', false, false, true], # no repo or vm login, so format not checked
-
- [true, 'active-notify at example.com', 'inactive-notify at example.com', '4ad at example.com', true, true, false], # username is invalid format
- [true, 'active-notify at example.com', 'inactive-notify at example.com', '4ad at example.com', false, false, true], # no repo or vm login, so format not checked
- [false, 'active-notify at example.com', 'inactive-notify at example.com', '4ad at example.com', false, false, true], # no repo or vm login, so format not checked
-
- [true, 'active-notify at example.com', 'inactive-notify at example.com', '.foo at example.com', false, false, true], # no repo or vm login, so format not checked
- [true, 'active-notify at example.com', 'inactive-notify at example.com', '.foo at example.com', true, false, false], # invalid format
-
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'bar. at example.com', false, false, true], # no repo or vm login, so format not checked
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'bar. at example.com', true, false, false], # valid format
-
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'ice9 at example.com', false, false, true], # no repo or vm login, so format not checked
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'ice9 at example.com', true, false, true], # valid format
-
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'o_o at example.com', false, false, true], # no repo or vm login, so format not checked
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'o_o at example.com', true, false, true], # valid format
-
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'r00t at example.com', false, false, true], # no repo or vm login, so format not checked
- [true, 'active-notify at example.com', 'inactive-notify at example.com', 'r00t at example.com', true, false, true], # valid format
-
- ].each do |active, new_user_recipients, inactive_recipients, email, auto_setup_vm, auto_setup_repo, ok_to_auto_setup|
+ # Easy inactive user tests.
+ [false, [], [], "inactive-none at example.com", false, false, "inactivenone"],
+ [false, [], [], "inactive-vm at example.com", true, false, "inactivevm"],
+ [false, [], [], "inactive-repo at example.com", false, true, "inactiverepo"],
+ [false, [], [], "inactive-both at example.com", true, true, "inactiveboth"],
+
+ # Easy active user tests.
+ [true, "active-notify at example.com", "inactive-notify at example.com", "active-none at example.com", false, false, "activenone"],
+ [true, "active-notify at example.com", "inactive-notify at example.com", "active-vm at example.com", true, false, "activevm"],
+ [true, "active-notify at example.com", "inactive-notify at example.com", "active-repo at example.com", false, true, "activerepo"],
+ [true, "active-notify at example.com", "inactive-notify at example.com", "active-both at example.com", true, true, "activeboth"],
+
+ # Test users with malformed e-mail addresses.
+ [false, [], [], nil, true, true, nil],
+ [false, [], [], "arvados", true, true, nil],
+ [false, [], [], "@example.com", true, true, nil],
+ [true, "active-notify at example.com", "inactive-notify at example.com", "*!*@example.com", true, false, nil],
+ [true, "active-notify at example.com", "inactive-notify at example.com", "*!*@example.com", false, false, nil],
+
+ # Test users with various username transformations.
+ [false, [], [], "arvados at example.com", false, false, "arvados2"],
+ [true, "active-notify at example.com", "inactive-notify at example.com", "arvados at example.com", false, false, "arvados2"],
+ [true, "active-notify at example.com", "inactive-notify at example.com", "root at example.com", true, false, "root2"],
+ [false, "active-notify at example.com", "inactive-notify at example.com", "root at example.com", true, false, "root2"],
+ [true, "active-notify at example.com", "inactive-notify at example.com", "roo_t at example.com", false, true, "root2"],
+ [false, [], [], "^^incorrect_format at example.com", true, true, "incorrectformat"],
+ [true, "active-notify at example.com", "inactive-notify at example.com", "&4a_d9. at example.com", true, true, "ad9"],
+ [true, "active-notify at example.com", "inactive-notify at example.com", "&4a_d9. at example.com", false, false, "ad9"],
+ [false, "active-notify at example.com", "inactive-notify at example.com", "&4a_d9. at example.com", true, true, "ad9"],
+ [false, "active-notify at example.com", "inactive-notify at example.com", "&4a_d9. at example.com", false, false, "ad9"],
+ ].each do |active, new_user_recipients, inactive_recipients, email, auto_setup_vm, auto_setup_repo, expect_username|
test "create new user with auto setup #{active} #{email} #{auto_setup_vm} #{auto_setup_repo}" do
- auto_setup_new_users = Rails.configuration.auto_setup_new_users
- auto_setup_new_users_with_vm_uuid = Rails.configuration.auto_setup_new_users_with_vm_uuid
- auto_setup_new_users_with_repository = Rails.configuration.auto_setup_new_users_with_repository
+ set_user_from_auth :admin
- begin
- set_user_from_auth :admin
+ Rails.configuration.auto_setup_new_users = true
- Rails.configuration.auto_setup_new_users = true
-
- if auto_setup_vm
- Rails.configuration.auto_setup_new_users_with_vm_uuid = virtual_machines(:testvm)['uuid']
- else
- Rails.configuration.auto_setup_new_users_with_vm_uuid = false
- end
+ if auto_setup_vm
+ Rails.configuration.auto_setup_new_users_with_vm_uuid = virtual_machines(:testvm)['uuid']
+ else
+ Rails.configuration.auto_setup_new_users_with_vm_uuid = false
+ end
- Rails.configuration.auto_setup_new_users_with_repository = auto_setup_repo
+ Rails.configuration.auto_setup_new_users_with_repository = auto_setup_repo
- create_user_and_verify_setup_and_notifications active, new_user_recipients, inactive_recipients, email, ok_to_auto_setup
- ensure
- Rails.configuration.auto_setup_new_users = auto_setup_new_users
- Rails.configuration.auto_setup_new_users_with_vm_uuid = auto_setup_new_users_with_vm_uuid
- Rails.configuration.auto_setup_new_users_with_repository = auto_setup_new_users_with_repository
- end
+ create_user_and_verify_setup_and_notifications active, new_user_recipients, inactive_recipients, email, expect_username
end
end
@@ -620,78 +574,41 @@ class UserTest < ActiveSupport::TestCase
end
end
- def create_user_and_verify_setup_and_notifications (active, new_user_recipients, inactive_recipients, email, ok_to_auto_setup)
+ def create_user_and_verify_setup_and_notifications (active, new_user_recipients, inactive_recipients, email, expect_username)
Rails.configuration.new_user_notification_recipients = new_user_recipients
Rails.configuration.new_inactive_user_notification_recipients = inactive_recipients
- assert_equal new_user_recipients, Rails.configuration.new_user_notification_recipients
- assert_equal inactive_recipients, Rails.configuration.new_inactive_user_notification_recipients
-
ActionMailer::Base.deliveries = []
+ can_setup = (Rails.configuration.auto_setup_new_users and
+ (not expect_username.nil?))
+ prior_repo = Repository.where(name: expect_username).first
+
user = User.new
user.first_name = "first_name_for_newly_created_user"
user.email = email
user.is_active = active
user.save!
+ assert_equal(expect_username, user.username)
# check user setup
- group = Group.where(name: 'All users').select do |g|
- g[:uuid].match /-f+$/
- end.first
-
- if !Rails.configuration.auto_setup_new_users || !ok_to_auto_setup
- # verify that the user is not added to "All groups" by auto_setup
- verify_link_exists false, group[:uuid], user.uuid, 'permission', 'can_read', nil, nil
-
- # check oid login link not created by auto_setup
- verify_link_exists false, user.uuid, user.email, 'permission', 'can_login', nil, nil
- else
- # verify that auto_setup took place
- # verify that the user is added to "All groups"
- verify_link_exists true, group[:uuid], user.uuid, 'permission', 'can_read', nil, nil
-
- # check oid login link
- verify_link_exists true, user.uuid, user.email, 'permission', 'can_login', nil, nil
-
- username = user.email.partition('@')[0] if email
-
- # check repo
- repo_names = []
- if Rails.configuration.auto_setup_new_users_with_repository
- 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]
- repo_names << repo[:name]
- end
- if username == 'auto_setup_repo'
- begin
- repo_names.delete('auto_setup_repo')
- ensure
- assert_equal true, repo_names.any?, 'Repository name for username foo is not unique'
- end
- end
- verify_link_exists true, repo_uuids, user.uuid, 'permission', 'can_manage', nil, nil
- end
-
- # if username is existing vm login name, make sure the username used to generate any repo is unique
- if username == 'auto_setup_vm_login' || username == 'auto_setup_repo'
- if repo_names.any?
- assert repo_names.first.start_with? username
- assert_not_nil /\d$/.match(repo_names.first)
- end
- end
-
- # check vm uuid
- vm_uuid = Rails.configuration.auto_setup_new_users_with_vm_uuid
- if vm_uuid
- verify_link_exists true, vm_uuid, user.uuid, 'permission', 'can_login', 'username', (username == 'auto_setup_repo' ? repo_names.first : username)
- else
- verify_link_exists false, vm_uuid, user.uuid, 'permission', 'can_login', 'username', (username == 'auto_setup_repo' ? repo_names.first : username)
- end
+ verify_link_exists(Rails.configuration.auto_setup_new_users,
+ groups(:all_users).uuid, user.uuid,
+ "permission", "can_read")
+ # Check for OID login link.
+ verify_link_exists(Rails.configuration.auto_setup_new_users,
+ user.uuid, user.email, "permission", "can_login")
+ # Check for repository.
+ if named_repo = (prior_repo or
+ Repository.where(name: expect_username).first)
+ verify_link_exists((can_setup and prior_repo.nil? and
+ Rails.configuration.auto_setup_new_users_with_repository),
+ named_repo.uuid, user.uuid, "permission", "can_manage")
+ end
+ # Check for VM login.
+ if auto_vm_uuid = Rails.configuration.auto_setup_new_users_with_vm_uuid
+ verify_link_exists(can_setup, auto_vm_uuid, user.uuid,
+ "permission", "can_login", "username", expect_username)
end
# check email notifications
@@ -700,7 +617,7 @@ class UserTest < ActiveSupport::TestCase
new_user_email_subject = "#{Rails.configuration.email_subject_prefix}New user created notification"
if Rails.configuration.auto_setup_new_users
- new_user_email_subject = (ok_to_auto_setup || active) ?
+ new_user_email_subject = (expect_username or active) ?
"#{Rails.configuration.email_subject_prefix}New user created and setup notification" :
"#{Rails.configuration.email_subject_prefix}New user created, but not setup notification"
end
@@ -740,7 +657,7 @@ class UserTest < ActiveSupport::TestCase
end
- def verify_link_exists link_exists, head_uuid, tail_uuid, link_class, link_name, property_name, property_value
+ def verify_link_exists link_exists, head_uuid, tail_uuid, link_class, link_name, property_name=nil, property_value=nil
all_links = Link.where(head_uuid: head_uuid,
tail_uuid: tail_uuid,
link_class: link_class,
commit 1a3c91dc982c91416d4f0ed1135ab5bc84f32a7c
Author: Brett Smith <brett at curoverse.com>
Date: Wed Mar 18 15:59:51 2015 -0400
4253: Clean up some user setup methods.
As much as possible, use more methods provided by Rails, rather than
reimplementing them ourselves.
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 3978df7..d4d722f 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -384,80 +384,45 @@ class User < ArvadosModel
return
end
- # Check for an existing repository with the same name we're about to use.
- repo = Repository.where(name: repo_name).first
-
- if repo
- logger.warn "Repository exists for #{repo_name}: #{repo[:uuid]}."
-
- # Look for existing repository access for this repo
- repo_perms = Link.where(tail_uuid: self.uuid,
- head_uuid: repo[:uuid],
- link_class: 'permission',
- name: 'can_manage')
- if repo_perms.any?
- logger.warn "User already has repository access " +
- repo_perms.collect { |p| p[:uuid] }.inspect
- return repo_perms.first
- end
- end
-
- # create repo, if does not already exist
- repo ||= Repository.create(name: repo_name)
+ repo = Repository.where(name: repo_name).first_or_create!
logger.info { "repo uuid: " + repo[:uuid] }
-
- repo_perm = Link.create(tail_uuid: self.uuid,
- head_uuid: repo[:uuid],
- link_class: 'permission',
- name: 'can_manage')
+ repo_perm = Link.where(tail_uuid: uuid, head_uuid: repo.uuid,
+ link_class: "permission",
+ name: "can_manage").first_or_create!
logger.info { "repo permission: " + repo_perm[:uuid] }
return repo_perm
end
# create login permission for the given vm_uuid, if it does not already exist
def create_vm_login_permission_link(vm_uuid, repo_name)
- begin
-
- # vm uuid is optional
- if vm_uuid
- vm = VirtualMachine.where(uuid: vm_uuid).first
+ # vm uuid is optional
+ if vm_uuid
+ vm = VirtualMachine.where(uuid: vm_uuid).first
- if not vm
- logger.warn "Could not find virtual machine for #{vm_uuid.inspect}"
- raise "No vm found for #{vm_uuid}"
- end
- else
- return
+ if not vm
+ logger.warn "Could not find virtual machine for #{vm_uuid.inspect}"
+ raise "No vm found for #{vm_uuid}"
end
+ else
+ return
+ end
- logger.info { "vm uuid: " + vm[:uuid] }
-
- login_perms = Link.where(tail_uuid: self.uuid,
- head_uuid: vm[:uuid],
- link_class: 'permission',
- name: 'can_login')
+ logger.info { "vm uuid: " + vm[:uuid] }
+ login_attrs = {
+ tail_uuid: uuid, head_uuid: vm.uuid,
+ link_class: "permission", name: "can_login",
+ }
- perm_exists = false
- login_perms.each do |perm|
- if perm.properties['username'] == repo_name
- perm_exists = perm
- break
- end
- end
+ login_perm = Link.
+ where(login_attrs).
+ select { |link| link.properties["username"] == repo_name }.
+ first
- if perm_exists
- login_perm = perm_exists
- else
- login_perm = Link.create(tail_uuid: self.uuid,
- head_uuid: vm[:uuid],
- link_class: 'permission',
- name: 'can_login',
- properties: {'username' => repo_name})
- logger.info { "login permission: " + login_perm[:uuid] }
- end
+ login_perm ||= Link.
+ create(login_attrs.merge(properties: {"username" => repo_name}))
- return login_perm
- end
+ logger.info { "login permission: " + login_perm[:uuid] }
+ login_perm
end
# add the user to the 'All users' group
commit ee46c18dc4aa90e84cd022543b754ca25e0f242a
Author: Brett Smith <brett at curoverse.com>
Date: Tue Mar 17 18:10:14 2015 -0400
4253: Add a username attribute to users.
* Add the column, and propagate it based on available VM logins or
e-mail address, if possible.
* Add format validation and tests.
* Set new usernames based on e-mail address, with tests.
diff --git a/doc/api/schema/User.html.textile.liquid b/doc/api/schema/User.html.textile.liquid
index 9a1b056..335b6c1 100644
--- a/doc/api/schema/User.html.textile.liquid
+++ b/doc/api/schema/User.html.textile.liquid
@@ -19,6 +19,7 @@ Each User has, in addition to the usual "attributes of Arvados resources":{{site
table(table table-bordered table-condensed).
|_. Attribute|_. Type|_. Description|_. Example|
|email|string|||
+|username|string|The username used for the user's git repositories and virtual machine logins. Usernames must start with a letter, and contain only alphanumerics. When a new user is created, a default username is set from their e-mail address. Only administrators may change the username.||
|first_name|string|||
|last_name|string|||
|identity_url|string|||
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index a47a458..3978df7 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -8,19 +8,29 @@ class User < ArvadosModel
serialize :prefs, Hash
has_many :api_client_authorizations
+ validates(:username,
+ format: {
+ with: /^[A-Za-z][A-Za-z0-9]*$/,
+ message: "must begin with a letter and contain only alphanumerics",
+ },
+ uniqueness: true,
+ allow_nil: true)
before_update :prevent_privilege_escalation
before_update :prevent_inactive_admin
before_create :check_auto_admin
+ before_create :set_initial_username, :if => Proc.new { |user|
+ user.username.nil? and user.email
+ }
after_create :add_system_group_permission_link
after_create :auto_setup_new_user
after_create :send_admin_notifications
after_update :send_profile_created_notification
-
has_many :authorized_keys, :foreign_key => :authorized_user_uuid, :primary_key => :uuid
api_accessible :user, extend: :common do |t|
t.add :email
+ t.add :username
t.add :full_name
t.add :first_name
t.add :last_name
@@ -222,9 +232,13 @@ class User < ArvadosModel
end
def permission_to_update
- # users must be able to update themselves (even if they are
- # inactive) in order to create sessions
- self == current_user or super
+ if username_changed?
+ current_user.andand.is_admin
+ else
+ # users must be able to update themselves (even if they are
+ # inactive) in order to create sessions
+ self == current_user or super
+ end
end
def permission_to_create
@@ -237,13 +251,62 @@ class User < ArvadosModel
return if self.uuid.end_with?('anonymouspublic')
if (User.where("email = ?",self.email).where(:is_admin => true).count == 0 and
Rails.configuration.auto_admin_user and self.email == Rails.configuration.auto_admin_user) or
- (User.where("uuid not like '%-000000000000000'").where(:is_admin => true).count == 0 and
+ (User.where("uuid not like '%-000000000000000'").where(:is_admin => true).count == 0 and
Rails.configuration.auto_admin_first_user)
self.is_admin = true
self.is_active = true
end
end
+ def find_usable_username_from(basename)
+ # If "basename" is a usable username, return that.
+ # Otherwise, find a unique username "basenameN", where N is the
+ # smallest integer greater than 1, and return that.
+ # Return nil if a unique username can't be found after reasonable
+ # searching.
+ quoted_name = self.class.connection.quote_string(basename)
+ next_username = basename
+ next_suffix = 1
+ while Rails.configuration.auto_setup_name_blacklist.include?(next_username)
+ next_suffix += 1
+ next_username = "%s%i" % [basename, next_suffix]
+ end
+ 0.upto(6).each do |suffix_len|
+ pattern = "%s%s" % [quoted_name, "_" * suffix_len]
+ self.class.
+ where("username like '#{pattern}'").
+ select(:username).
+ order(username: :asc).
+ find_each do |other_user|
+ if other_user.username > next_username
+ break
+ elsif other_user.username == next_username
+ next_suffix += 1
+ next_username = "%s%i" % [basename, next_suffix]
+ end
+ end
+ return next_username if (next_username.size <= pattern.size)
+ end
+ nil
+ end
+
+ def set_initial_username
+ email_parts = email.partition("@")
+ local_parts = email_parts.first.partition("+")
+ if email_parts.any?(&:empty?)
+ return
+ elsif not local_parts.first.empty?
+ base_username = local_parts.first
+ else
+ base_username = email_parts.first
+ end
+ base_username.sub!(/^[^A-Za-z]+/, "")
+ base_username.gsub!(/[^A-Za-z0-9]/, "")
+ unless base_username.empty?
+ self.username = find_usable_username_from(base_username)
+ end
+ end
+
def prevent_privilege_escalation
if current_user.andand.is_admin
return true
diff --git a/services/api/db/migrate/20150317132720_add_username_to_users.rb b/services/api/db/migrate/20150317132720_add_username_to_users.rb
new file mode 100644
index 0000000..6772414
--- /dev/null
+++ b/services/api/db/migrate/20150317132720_add_username_to_users.rb
@@ -0,0 +1,130 @@
+require 'has_uuid'
+require 'kind_and_etag'
+
+class AddUsernameToUsers < ActiveRecord::Migration
+ include CurrentApiClient
+
+ SEARCH_INDEX_COLUMNS =
+ ["uuid", "owner_uuid", "modified_by_client_uuid",
+ "modified_by_user_uuid", "email", "first_name", "last_name",
+ "identity_url", "default_owner_uuid"]
+
+ class ArvadosModel < ActiveRecord::Base
+ self.abstract_class = true
+ extend HasUuid::ClassMethods
+ include CurrentApiClient
+ include KindAndEtag
+ before_create do |record|
+ record.uuid ||= record.class.generate_uuid
+ record.owner_uuid ||= system_user_uuid
+ end
+ serialize :properties, Hash
+
+ def self.to_s
+ # Clean up the name of the stub model class so we generate correct UUIDs.
+ super.rpartition("::").last
+ end
+ end
+
+ class Log < ArvadosModel
+ def self.log_for(thing, age="old")
+ { "#{age}_etag" => thing.etag,
+ "#{age}_attributes" => thing.attributes,
+ }
+ end
+
+ def self.log_create(thing)
+ new_log("create", thing, log_for(thing, "new"))
+ end
+
+ def self.log_update(thing, start_state)
+ new_log("update", thing, start_state.merge(log_for(thing, "new")))
+ end
+
+ def self.log_destroy(thing)
+ new_log("destroy", thing, log_for(thing, "old"))
+ end
+
+ private
+
+ def self.new_log(event_type, thing, properties)
+ create!(event_type: event_type,
+ event_at: Time.now,
+ object_uuid: thing.uuid,
+ object_owner_uuid: thing.owner_uuid,
+ properties: properties)
+ end
+ end
+
+ class Link < ArvadosModel
+ end
+
+ class User < ArvadosModel
+ end
+
+ def sanitize_username(username)
+ username.
+ sub(/^[^A-Za-z]+/, "").
+ gsub(/[^A-Za-z0-9]/, "")
+ end
+
+ def usernames_wishlist(user)
+ usernames = Hash.new(0)
+ usernames[user.email.split("@", 2).first] += 1
+ Link.
+ where(tail_uuid: user.uuid, link_class: "permission", name: "can_login").
+ find_each do |login_perm|
+ username = login_perm.properties["username"]
+ usernames[username] += 2 if (username and not username.empty?)
+ end
+ usernames.keys.
+ sort_by { |n| -usernames[n] }.
+ map { |n| sanitize_username(n) }.
+ reject(&:empty?)
+ end
+
+ def increment_username(username)
+ @username_suffixes[username] += 1
+ "%s%i" % [username, @username_suffixes[username]]
+ end
+
+ def each_wanted_username(user)
+ usernames = usernames_wishlist(user)
+ usernames.each { |n| yield n }
+ base_username = usernames.first || "arvadosuser"
+ loop { yield increment_username(base_username) }
+ end
+
+ def recreate_search_index(columns)
+ remove_index :users, name: "users_search_index"
+ add_index :users, columns, name: "users_search_index"
+ end
+
+ def up
+ @username_suffixes = Hash.new(1)
+ add_column :users, :username, :string, null: true
+ add_index :users, :username, unique: true
+ recreate_search_index(SEARCH_INDEX_COLUMNS + ["username"])
+
+ [Link, Log, User].each { |m| m.reset_column_information }
+ User.where(is_active: true).order(created_at: :asc).find_each do |user|
+ start_log = Log.log_for(user)
+ each_wanted_username(user) do |username|
+ user.username = username
+ begin
+ user.save!
+ break
+ rescue ActiveRecord::RecordNotUnique
+ # We'll try the next username.
+ end
+ end
+ Log.log_update(user, start_log)
+ end
+ end
+
+ def down
+ remove_index :users, :username
+ recreate_search_index(SEARCH_INDEX_COLUMNS)
+ remove_column :users, :username
+ end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 0711f90..40861f4 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -889,7 +889,8 @@ CREATE TABLE users (
prefs text,
updated_at timestamp without time zone NOT NULL,
default_owner_uuid character varying(255),
- is_active boolean DEFAULT false
+ is_active boolean DEFAULT false,
+ username character varying(255)
);
@@ -1965,6 +1966,13 @@ CREATE INDEX index_users_on_owner_uuid ON users USING btree (owner_uuid);
--
+-- Name: index_users_on_username; Type: INDEX; Schema: public; Owner: -; Tablespace:
+--
+
+CREATE UNIQUE INDEX index_users_on_username ON users USING btree (username);
+
+
+--
-- Name: index_users_on_uuid; Type: INDEX; Schema: public; Owner: -; Tablespace:
--
@@ -2122,7 +2130,7 @@ CREATE UNIQUE INDEX unique_schema_migrations ON schema_migrations USING btree (v
-- Name: users_search_index; Type: INDEX; Schema: public; Owner: -; Tablespace:
--
-CREATE INDEX users_search_index ON users USING btree (uuid, owner_uuid, modified_by_client_uuid, modified_by_user_uuid, email, first_name, last_name, identity_url, default_owner_uuid);
+CREATE INDEX users_search_index ON users USING btree (uuid, owner_uuid, modified_by_client_uuid, modified_by_user_uuid, email, first_name, last_name, identity_url, default_owner_uuid, username);
--
@@ -2364,4 +2372,6 @@ INSERT INTO schema_migrations (version) VALUES ('20150216193428');
INSERT INTO schema_migrations (version) VALUES ('20150303210106');
-INSERT INTO schema_migrations (version) VALUES ('20150312151136');
\ No newline at end of file
+INSERT INTO schema_migrations (version) VALUES ('20150312151136');
+
+INSERT INTO schema_migrations (version) VALUES ('20150317132720');
\ No newline at end of file
diff --git a/services/api/test/fixtures/users.yml b/services/api/test/fixtures/users.yml
index c04aa47..46fbd0f 100644
--- a/services/api/test/fixtures/users.yml
+++ b/services/api/test/fixtures/users.yml
@@ -25,6 +25,7 @@ admin:
identity_url: https://admin.openid.local
is_active: true
is_admin: true
+ username: admin
prefs:
profile:
organization: example.com
@@ -39,6 +40,7 @@ miniadmin:
identity_url: https://miniadmin.openid.local
is_active: true
is_admin: false
+ username: miniadmin
prefs:
profile:
organization: example.com
@@ -53,6 +55,7 @@ rominiadmin:
identity_url: https://rominiadmin.openid.local
is_active: true
is_admin: false
+ username: rominiadmin
prefs:
profile:
organization: example.com
@@ -67,6 +70,7 @@ active:
identity_url: https://active-user.openid.local
is_active: true
is_admin: false
+ username: active
prefs:
profile:
organization: example.com
@@ -81,6 +85,7 @@ project_viewer:
identity_url: https://project-viewer.openid.local
is_active: true
is_admin: false
+ username: projectviewer
prefs:
profile:
organization: example.com
@@ -96,6 +101,7 @@ future_project_user:
identity_url: https://future-project-user.openid.local
is_active: true
is_admin: false
+ username: futureprojectviewer
prefs:
profile:
organization: example.com
@@ -110,6 +116,7 @@ subproject_admin:
identity_url: https://subproject-admin.openid.local
is_active: true
is_admin: false
+ username: subprojectadmin
prefs:
profile:
organization: example.com
@@ -124,6 +131,7 @@ spectator:
identity_url: https://spectator.openid.local
is_active: true
is_admin: false
+ username: spectator
prefs:
profile:
organization: example.com
@@ -184,6 +192,7 @@ job_reader:
identity_url: https://spectator.openid.local
is_active: true
is_admin: false
+ username: jobber
prefs:
profile:
organization: example.com
@@ -223,6 +232,7 @@ user_foo_in_sharing_group:
identity_url: https://user_foo_in_sharing_group.openid.local
is_active: true
is_admin: false
+ username: fooinsharing
user_bar_in_sharing_group:
owner_uuid: zzzzz-tpzed-000000000000000
@@ -233,6 +243,7 @@ user_bar_in_sharing_group:
identity_url: https://user_bar_in_sharing_group.openid.local
is_active: true
is_admin: false
+ username: barinsharing
user1_with_load:
owner_uuid: zzzzz-tpzed-000000000000000
@@ -257,6 +268,7 @@ fuse:
identity_url: https://fuse.openid.local
is_active: true
is_admin: false
+ username: FUSE
prefs:
profile:
organization: example.com
diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index 9bcb011..9ba5646 100644
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -9,6 +9,105 @@ class UserTest < ActiveSupport::TestCase
system_user
end
+ %w(a aa a0 aA Aa AA A0).each do |username|
+ test "#{username.inspect} is a valid username" do
+ user = User.new(username: username)
+ assert(user.valid?)
+ end
+ end
+
+ test "username is not required" do
+ user = User.new(username: nil)
+ assert(user.valid?)
+ end
+
+ test "username beginning with numeral is invalid" do
+ user = User.new(username: "0a")
+ refute(user.valid?)
+ end
+
+ "\\.-_/!@#$%^&*()[]{}".each_char do |bad_char|
+ test "username containing #{bad_char.inspect} is invalid" do
+ user = User.new(username: "bad#{bad_char}username")
+ refute(user.valid?)
+ end
+ end
+
+ test "username must be unique" do
+ user = User.new(username: users(:active).username)
+ refute(user.valid?)
+ end
+
+ test "non-admin can't update username" do
+ set_user_from_auth :active_trustedclient
+ user = User.find_by_uuid(users(:active).uuid)
+ user.username = "selfupdate"
+ begin
+ refute(user.save)
+ rescue ArvadosModel::PermissionDeniedError
+ # That works too.
+ end
+ end
+
+ def check_admin_username_change(fixture_name)
+ set_user_from_auth :admin_trustedclient
+ user = User.find_by_uuid(users(fixture_name).uuid)
+ user.username = "newnamefromtest"
+ assert(user.save)
+ end
+
+ test "admin can set username" do
+ check_admin_username_change(:active_no_prefs)
+ end
+
+ test "admin can update username" do
+ check_admin_username_change(:active)
+ end
+
+ test "admin can update own username" do
+ check_admin_username_change(:admin)
+ end
+
+ def check_new_username_setting(email_name, expect_name)
+ set_user_from_auth :admin
+ user = User.create!(email: "#{email_name}@example.org")
+ assert_equal(expect_name, user.username)
+ end
+
+ test "new username set from e-mail" do
+ check_new_username_setting("dakota", "dakota")
+ end
+
+ test "new username set from e-mail with leading digits" do
+ check_new_username_setting("1dakota9", "dakota9")
+ end
+
+ test "new username set from e-mail with punctuation" do
+ check_new_username_setting("dakota.9", "dakota9")
+ end
+
+ test "new username set from e-mail with leading digits and punctuation" do
+ check_new_username_setting("1.dakota.z", "dakotaz")
+ end
+
+ test "new username set from e-mail with extra part" do
+ check_new_username_setting("dakota+arvados", "dakota")
+ end
+
+ test "new username set with deduplication" do
+ name = users(:active).username
+ check_new_username_setting(name, "#{name}2")
+ end
+
+ test "new username set avoiding blacklist" do
+ Rails.configuration.auto_setup_name_blacklist = ["root"]
+ check_new_username_setting("root", "root2")
+ end
+
+ test "no username set when no base available" do
+ check_new_username_setting("_", nil)
+ end
+
[[false, 'foo at example.com', true, nil],
[false, 'bar at example.com', nil, true],
[true, 'foo at example.com', true, nil],
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list