[ARVADOS] updated: 1.3.0-1534-g5713754c5
Git user
git at public.curoverse.com
Thu Aug 22 15:31:07 UTC 2019
Summary of changes:
services/api/app/models/user.rb | 90 ++++++++++-------------
services/api/test/integration/remote_user_test.rb | 39 ++++++++--
services/api/test/integration/users_test.rb | 28 +++++++
services/api/test/unit/user_test.rb | 43 +++++++++++
4 files changed, 142 insertions(+), 58 deletions(-)
via 5713754c574254f9e3650ac80bf8fdca235898f6 (commit)
via b19eea5a71ff3fa6259df25bc0726bd1e152d89b (commit)
from fb7a094681db65daa2f28b50ca734dc27e9d6db7 (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 5713754c574254f9e3650ac80bf8fdca235898f6
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date: Thu Aug 22 11:30:46 2019 -0400
15558: Add tests for user lookup by alternate email
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 5f94db0bb..db9eddc06 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -327,8 +327,26 @@ class User < ArvadosModel
end
end
+ def redirects_to
+ user = self
+ redirects = 0
+ while (uuid = user.redirect_to_user_uuid)
+ user = User.unscoped.find_by_uuid(uuid)
+ if !user
+ raise Exception.new("user uuid #{user.uuid} redirects to nonexistent uuid #{uuid}")
+ end
+ redirects += 1
+ if redirects > 15
+ raise "Starting from #{self.uuid} redirect_to_user_uuid exceeded maximum number of redirects"
+ end
+ end
+ user
+ end
+
def self.register info
- # login info should have fields:
+ # login info expected fields, all can be optional but at minimum
+ # must supply either 'identity_url' or 'email'
+ #
# email
# first_name
# last_name
@@ -336,6 +354,8 @@ class User < ArvadosModel
# alternate_emails
# identity_url
+ info = info.with_indifferent_access
+
primary_user = nil
# local database
@@ -345,41 +365,13 @@ class User < ArvadosModel
# Only local users can create sessions, hence uuid_like_pattern
# here.
user = User.unscoped.where('identity_url = ? and uuid like ?',
- info['identity_url'],
+ identity_url,
User.uuid_like_pattern).first
- if user
- while (uuid = user.redirect_to_user_uuid)
- user = User.unscoped.find_by_uuid(uuid)
- if !user
- raise Exception.new("user uuid #{user.uuid} redirects to nonexistent uuid #{uuid}")
- end
- end
- primary_user = user
- end
-
- # Don't think this is necessary if admin can just create a user
- # record with the desired email.
- #
- # if not user
- # # Check for permission to log in to an existing User record with
- # # a different identity_url
- # Link.where("link_class = ? and name = ? and tail_uuid = ? and head_uuid like ?",
- # 'permission',
- # 'can_login',
- # info['email'],
- # User.uuid_like_pattern).each do |link|
- # if prefix = link.properties['identity_url_prefix']
- # if prefix == info['identity_url'][0..prefix.size-1]
- # user = User.find_by_uuid(link.head_uuid)
- # break if user
- # end
- # end
- # end
- # end
+ primary_user = user.redirects_to if user
end
if !primary_user
- # identity url is unset or didn't find anything.
+ # identity url is unset or didn't find matching record.
emails = [info['email']] + (info['alternate_emails'] || [])
emails.select! {|em| !em.nil? && !em.empty?}
emails.each do |em|
@@ -389,17 +381,10 @@ class User < ArvadosModel
user = User.unscoped.where('email = ? and uuid like ?',
em,
User.uuid_like_pattern).first
- next if !user
-
- while (uuid = user.redirect_to_user_uuid)
- user = User.unscoped.find_by_uuid(uuid)
- if !user
- raise Exception.new("user uuid #{user.uuid} redirects to nonexistent uuid #{uuid}")
- end
+ if user
+ primary_user = user.redirects_to
+ break
end
-
- primary_user = user
- break
end
end
@@ -412,10 +397,14 @@ class User < ArvadosModel
primary_user.set_initial_username(requested: info['username']) if info['username']
end
- primary_user.email = info['email']
- primary_user.identity_url = info['identity_url']
- primary_user.first_name = info['first_name']
- primary_user.last_name = info['last_name']
+ primary_user.email = info['email'] if info['email']
+ primary_user.identity_url = info['identity_url'] if identity_url
+ primary_user.first_name = info['first_name'] if info['first_name']
+ primary_user.last_name = info['last_name'] if info['last_name']
+
+ if (!primary_user.email or primary_user.identity_url.empty?) and (!primary_user.identity_url or primary_user.identity_url.empty?)
+ raise "Must have supply at least one of 'email' or 'identity_url' to User.register"
+ end
act_as_system_user do
primary_user.save!
diff --git a/services/api/test/integration/remote_user_test.rb b/services/api/test/integration/remote_user_test.rb
index 259d7f554..f68a271ed 100644
--- a/services/api/test/integration/remote_user_test.rb
+++ b/services/api/test/integration/remote_user_test.rb
@@ -67,10 +67,6 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
res.status = @stub_status
res.body = @stub_content.is_a?(String) ? @stub_content : @stub_content.to_json
end
- srv.mount_proc '/arvados/v1/users/register' do |req, res|
- res.status = @stub_status
- res.body = @stub_content.is_a?(String) ? @stub_content : @stub_content.to_json
- end
Thread.new do
srv.start
end
@@ -273,10 +269,18 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
end
test 'pre-activate remote user' do
+ @stub_content = {
+ uuid: 'zbbbb-tpzed-000000000001234',
+ email: 'foo at example.com',
+ username: 'barney',
+ is_admin: true,
+ is_active: true,
+ }
+
post '/arvados/v1/users',
params: {
"user" => {
- "uuid" => "zbbbb-tpzed-000000000000000",
+ "uuid" => "zbbbb-tpzed-000000000001234",
"email" => 'foo at example.com',
"username" => 'barney',
"is_active" => true
@@ -289,13 +293,34 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
params: {format: 'json'},
headers: auth(remote: 'zbbbb')
assert_response :success
- assert_equal 'zbbbb-tpzed-000000000000000', json_response['uuid']
- assert_equal nil, json_response['is_admin']
+ assert_equal 'zbbbb-tpzed-000000000001234', json_response['uuid']
+ assert_nil json_response['is_admin']
assert_equal true, json_response['is_active']
assert_equal 'foo at example.com', json_response['email']
assert_equal 'barney', json_response['username']
end
+
+ test 'remote user inactive without pre-activation' do
+ @stub_content = {
+ uuid: 'zbbbb-tpzed-000000000001234',
+ email: 'foo at example.com',
+ username: 'barney',
+ is_admin: true,
+ is_active: true,
+ }
+
+ get '/arvados/v1/users/current',
+ params: {format: 'json'},
+ headers: auth(remote: 'zbbbb')
+ assert_response :success
+ assert_equal 'zbbbb-tpzed-000000000001234', json_response['uuid']
+ assert_nil json_response['is_admin']
+ assert_equal false, json_response['is_active']
+ assert_equal 'foo at example.com', json_response['email']
+ assert_equal 'barney', json_response['username']
+ end
+
test "validate unsalted v2 token for remote cluster zbbbb" do
auth = api_client_authorizations(:active)
token = "v2/#{auth.uuid}/#{auth.api_token}"
diff --git a/services/api/test/integration/users_test.rb b/services/api/test/integration/users_test.rb
index 5886fb2d0..6b7415407 100644
--- a/services/api/test/integration/users_test.rb
+++ b/services/api/test/integration/users_test.rb
@@ -275,4 +275,32 @@ class UsersTest < ActionDispatch::IntegrationTest
assert_response(:success)
assert_equal(project_uuid, json_response['owner_uuid'])
end
+
+ test 'pre-activate user' do
+ post '/arvados/v1/users',
+ params: {
+ "user" => {
+ "email" => 'foo at example.com',
+ "is_active" => true,
+ "username" => "barney"
+ }
+ },
+ headers: {'HTTP_AUTHORIZATION' => "OAuth2 #{api_token(:admin)}"}
+ assert_response :success
+ rp = json_response
+ assert_not_nil rp["uuid"]
+ assert_not_nil rp["is_active"]
+ assert_nil rp["is_admin"]
+
+ get "/arvados/v1/users/#{rp['uuid']}",
+ params: {format: 'json'},
+ headers: auth(:admin)
+ assert_response :success
+ assert_equal rp["uuid"], json_response['uuid']
+ assert_nil json_response['is_admin']
+ assert_equal true, json_response['is_active']
+ assert_equal 'foo at example.com', json_response['email']
+ assert_equal 'barney', json_response['username']
+ end
+
end
diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index 6d2157b14..21252eeaf 100644
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -800,4 +800,47 @@ class UserTest < ActiveSupport::TestCase
end
end
end
+
+ test "lookup user by email" do
+ u = User.register({"email" => "active-user at arvados.local", "identity_url" => "different-identity-url"})
+ active = User.find_by_uuid(users(:active).uuid)
+ assert_equal active.uuid, u.uuid
+ assert_equal "different-identity-url", active.identity_url
+ assert_equal "active-user at arvados.local", active.email
+ end
+
+ test "lookup user by alternate email" do
+ u = User.register({"email" => "user at parent-company.com",
+ "alternate_emails" => ["active-user at arvados.local"],
+ "identity_url" => "different-identity-url"})
+ active = User.find_by_uuid(users(:active).uuid)
+ assert_equal active.uuid, u.uuid
+ assert_equal "different-identity-url", active.identity_url
+ assert_equal "user at parent-company.com", active.email
+ end
+
+ test "register new user" do
+ u = User.register({"email" => "never-before-seen-user at arvados.local",
+ "identity_url" => "different-identity-url",
+ "first_name" => "Robert",
+ "last_name" => "Baratheon",
+ "username" => "bobby"})
+ nbs = User.find_by_uuid(u.uuid)
+ assert_equal nbs.uuid, u.uuid
+ assert_equal "different-identity-url", nbs.identity_url
+ assert_equal "never-before-seen-user at arvados.local", nbs.email
+ assert_equal false, nbs.is_admin
+ assert_equal false , nbs.is_active
+ assert_equal "bobby", nbs.username
+ assert_equal "Robert", nbs.first_name
+ assert_equal "Baratheon", nbs.last_name
+ end
+
+ test "fail lookup without identifiers" do
+ assert_raises do
+ User.register({"first_name" => "Robert", "last_name" => "Baratheon"})
+ end
+ end
+
+
end
commit b19eea5a71ff3fa6259df25bc0726bd1e152d89b
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date: Tue Aug 20 16:05:00 2019 -0400
15558: Tweak logic
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 6f2f92b93..5f94db0bb 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -378,10 +378,10 @@ class User < ArvadosModel
# end
end
- if !primary_user and info['email'] and !info['email'].empty?
+ if !primary_user
# identity url is unset or didn't find anything.
emails = [info['email']] + (info['alternate_emails'] || [])
- emails.select! {|em| !em.empty?}
+ emails.select! {|em| !em.nil? && !em.empty?}
emails.each do |em|
# Go through each email address, try to find a user record
# corresponding to one of the addresses supplied.
@@ -421,8 +421,7 @@ class User < ArvadosModel
primary_user.save!
end
- return primary_user
-
+ primary_user
end
protected
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list