[ARVADOS] updated: febb804129f39963722e530a394f36367220d4b5
git at public.curoverse.com
git at public.curoverse.com
Wed Mar 25 11:51:31 EDT 2015
Summary of changes:
doc/api/schema/User.html.textile.liquid | 1 +
services/api/app/models/user.rb | 212 +++++++-------
.../20150317132720_add_username_to_users.rb | 127 ++++++++
services/api/db/structure.sql | 16 +-
services/api/test/fixtures/users.yml | 12 +
services/api/test/unit/user_test.rb | 324 +++++++++++----------
6 files changed, 431 insertions(+), 261 deletions(-)
create mode 100644 services/api/db/migrate/20150317132720_add_username_to_users.rb
via febb804129f39963722e530a394f36367220d4b5 (commit)
via 9cb7b08709aec196d7cd2170ec60134f2253d46b (commit)
via 9aa1156989884e2e5d07dba055c1a16aac25b1c3 (commit)
via ff8721dd5854279fab712f4a4ff8aed2256c1b87 (commit)
from 8505cb4000d7e1c776cbc7ea630d9413616f49c3 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit febb804129f39963722e530a394f36367220d4b5
Merge: 8505cb4 9cb7b08
Author: Brett Smith <brett at curoverse.com>
Date: Wed Mar 25 11:51:08 2015 -0400
Merge branch '4253-user-usernames-wip'
Refs #4253. Closes #5512.
commit 9cb7b08709aec196d7cd2170ec60134f2253d46b
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 9aa1156989884e2e5d07dba055c1a16aac25b1c3
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 ff8721dd5854279fab712f4a4ff8aed2256c1b87
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..de2fc96
--- /dev/null
+++ b/services/api/db/migrate/20150317132720_add_username_to_users.rb
@@ -0,0 +1,127 @@
+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.validates(:username, uniqueness: true, allow_nil: true)
+ 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
+ break if user.valid?
+ end
+ user.save!
+ 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