From 2958a94fafbab941f8d6eb76bb2785b5c2868d3d Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Fri, 9 Oct 2020 13:00:05 -0400 Subject: [PATCH] 16989: Test all combinations of remote user status Rework remote user setup/activate/unsetup. Sending welcome email moved to model setup method. Add tests. Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- .../arvados/v1/users_controller.rb | 12 +-- .../app/models/api_client_authorization.rb | 59 ++++++++------ services/api/app/models/user.rb | 22 +++++- .../api/test/integration/remote_user_test.rb | 78 ++++++++----------- 4 files changed, 91 insertions(+), 80 deletions(-) diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb index c5eac83c62..f4d42edf6c 100644 --- a/services/api/app/controllers/arvados/v1/users_controller.rb +++ b/services/api/app/controllers/arvados/v1/users_controller.rb @@ -132,16 +132,8 @@ class Arvados::V1::UsersController < ApplicationController end @response = @object.setup(repo_name: full_repo_name, - vm_uuid: params[:vm_uuid]) - - # setup succeeded. send email to user - if params[:send_notification_email] && !Rails.configuration.Users.UserSetupMailText.empty? - begin - UserNotifier.account_is_setup(@object).deliver_now - rescue => e - logger.warn "Failed to send email to #{@object.email}: #{e}" - end - end + vm_uuid: params[:vm_uuid], + send_notification_email: params[:send_notification_email]) send_json kind: "arvados#HashList", items: @response.as_api_response(nil) end diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb index 518fe56937..37ad31feb0 100644 --- a/services/api/app/models/api_client_authorization.rb +++ b/services/api/app/models/api_client_authorization.rb @@ -244,36 +244,47 @@ class ApiClientAuthorization < ArvadosModel end # Sync user record. - if remote_user_prefix == Rails.configuration.Login.LoginCluster - # Remote cluster controls our user database, set is_active if - # remote is active. If remote is not active, user will be - # unsetup (see below). - user.is_active = true if remote_user['is_active'] - user.is_admin = remote_user['is_admin'] - else - if Rails.configuration.Users.NewUsersAreActive || - Rails.configuration.RemoteClusters[remote_user_prefix].andand["ActivateUsers"] - # Default policy is to activate users - user.is_active = true if remote_user['is_active'] + act_as_system_user do + %w[first_name last_name email prefs].each do |attr| + user.send(attr+'=', remote_user[attr]) end - end - %w[first_name last_name email prefs].each do |attr| - user.send(attr+'=', remote_user[attr]) - end + if remote_user['uuid'][-22..-1] == '-tpzed-000000000000000' + user.first_name = "root" + user.last_name = "from cluster #{remote_user_prefix}" + end - if remote_user['uuid'][-22..-1] == '-tpzed-000000000000000' - user.first_name = "root" - user.last_name = "from cluster #{remote_user_prefix}" - end + user.save! - act_as_system_user do - if (user.is_active && !remote_user['is_active']) or (user.is_invited && !remote_user['is_invited']) - # Synchronize the user's "active/invited" state state. This - # also saves the record. + if user.is_invited && !remote_user['is_invited'] + # Remote user is not "invited" state, they should be unsetup, which + # also makes them inactive. user.unsetup else - user.save! + if !user.is_invited && remote_user['is_invited'] and + (remote_user_prefix == Rails.configuration.Login.LoginCluster or + Rails.configuration.Users.AutoSetupNewUsers or + Rails.configuration.Users.NewUsersAreActive or + Rails.configuration.RemoteClusters[remote_user_prefix].andand["ActivateUsers"]) + user.setup + end + + if !user.is_active && remote_user['is_active'] && user.is_invited and + (remote_user_prefix == Rails.configuration.Login.LoginCluster or + Rails.configuration.Users.NewUsersAreActive or + Rails.configuration.RemoteClusters[remote_user_prefix].andand["ActivateUsers"]) + user.update_attributes!(is_active: true) + elsif user.is_active && !remote_user['is_active'] + user.update_attributes!(is_active: false) + end + + if remote_user_prefix == Rails.configuration.Login.LoginCluster and + user.is_active and + user.is_admin != remote_user['is_admin'] + # Remote cluster controls our user database, including the + # admin flag. + user.update_attributes!(is_admin: remote_user['is_admin']) + end end # We will accept this token (and avoid reloading the user diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb index 8ec90f7e53..57fe4f055d 100644 --- a/services/api/app/models/user.rb +++ b/services/api/app/models/user.rb @@ -213,10 +213,27 @@ SELECT target_uuid, perm_level end # create links - def setup(repo_name: nil, vm_uuid: nil) + def setup(repo_name: nil, vm_uuid: nil, send_notification_email: nil) + newly_invited = Link.where(tail_uuid: self.uuid, + head_uuid: all_users_group_uuid, + link_class: 'permission', + name: 'can_read').empty? + + group_perm = create_user_group_link repo_perm = create_user_repo_link repo_name vm_login_perm = create_vm_login_permission_link(vm_uuid, username) if vm_uuid - group_perm = create_user_group_link + + if send_notification_email.nil? + send_notification_email = Rails.configuration.Mail.SendUserSetupNotificationEmail + end + + if newly_invited and send_notification_email and !Rails.configuration.Users.UserSetupMailText.empty? + begin + UserNotifier.account_is_setup(self).deliver_now + rescue => e + logger.warn "Failed to send email to #{self.email}: #{e}" + end + end return [repo_perm, vm_login_perm, group_perm, self].compact end @@ -255,6 +272,7 @@ SELECT target_uuid, perm_level self.prefs = {} # mark the user as inactive + self.is_admin = false # can't be admin and inactive self.is_active = false self.save! end diff --git a/services/api/test/integration/remote_user_test.rb b/services/api/test/integration/remote_user_test.rb index d3180e173f..4323884529 100644 --- a/services/api/test/integration/remote_user_test.rb +++ b/services/api/test/integration/remote_user_test.rb @@ -325,50 +325,40 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest assert_equal 'barney', json_response['username'] end - test 'get inactive user from Login cluster when AutoSetupNewUsers is set' do - Rails.configuration.Login.LoginCluster = 'zbbbb' - Rails.configuration.Users.AutoSetupNewUsers = true - @stub_content = { - uuid: 'zbbbb-tpzed-000000000000001', - email: 'foo@example.com', - username: 'barney', - is_admin: false, - is_active: false, - is_invited: false, - } - get '/arvados/v1/users/current', - params: {format: 'json'}, - headers: auth(remote: 'zbbbb') - assert_response :success - assert_equal 'zbbbb-tpzed-000000000000001', json_response['uuid'] - assert_equal false, json_response['is_admin'] - assert_equal false, json_response['is_active'] - assert_equal false, json_response['is_invited'] - assert_equal 'foo@example.com', json_response['email'] - assert_equal 'barney', json_response['username'] - end - - test 'get invited by inactive user from Login cluster when AutoSetupNewUsers is set' do - Rails.configuration.Login.LoginCluster = 'zbbbb' - Rails.configuration.Users.AutoSetupNewUsers = true - @stub_content = { - uuid: 'zbbbb-tpzed-000000000000001', - email: 'foo@example.com', - username: 'barney', - is_admin: false, - is_active: false, - is_invited: true, - } - get '/arvados/v1/users/current', - params: {format: 'json'}, - headers: auth(remote: 'zbbbb') - assert_response :success - assert_equal 'zbbbb-tpzed-000000000000001', json_response['uuid'] - assert_equal false, json_response['is_admin'] - assert_equal false, json_response['is_active'] - assert_equal true, json_response['is_invited'] - assert_equal 'foo@example.com', json_response['email'] - assert_equal 'barney', json_response['username'] + [true, false].each do |trusted| + [true, false].each do |logincluster| + [true, false].each do |admin| + [true, false].each do |active| + [true, false].each do |autosetup| + [true, false].each do |invited| + test "get invited=#{invited}, active=#{active}, admin=#{admin} user from #{if logincluster then "Login" else "peer" end} cluster when AutoSetupNewUsers=#{autosetup} ActivateUsers=#{trusted}" do + Rails.configuration.Login.LoginCluster = 'zbbbb' if logincluster + Rails.configuration.RemoteClusters['zbbbb'].ActivateUsers = trusted + Rails.configuration.Users.AutoSetupNewUsers = autosetup + @stub_content = { + uuid: 'zbbbb-tpzed-000000000000001', + email: 'foo@example.com', + username: 'barney', + is_admin: admin, + is_active: active, + is_invited: invited, + } + get '/arvados/v1/users/current', + params: {format: 'json'}, + headers: auth(remote: 'zbbbb') + assert_response :success + assert_equal 'zbbbb-tpzed-000000000000001', json_response['uuid'] + assert_equal (logincluster && admin && invited && active), json_response['is_admin'] + assert_equal (invited and (logincluster || trusted || autosetup)), json_response['is_invited'] + assert_equal (invited and (logincluster || trusted) and active), json_response['is_active'] + assert_equal 'foo@example.com', json_response['email'] + assert_equal 'barney', json_response['username'] + end + end + end + end + end + end end test 'get active user from Login cluster when AutoSetupNewUsers is set' do -- 2.30.2