[ARVADOS] created: 1.3.0-3293-g2958a94fa

Git user git at public.arvados.org
Fri Oct 9 17:01:39 UTC 2020


        at  2958a94fafbab941f8d6eb76bb2785b5c2868d3d (commit)


commit 2958a94fafbab941f8d6eb76bb2785b5c2868d3d
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Fri Oct 9 13:00:05 2020 -0400

    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 <peter.amstutz at curii.com>

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index c5eac83c6..f4d42edf6 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 518fe5693..37ad31feb 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 8ec90f7e5..57fe4f055 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 d3180e173..432388452 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 at 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 at 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 at 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 at 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 at 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 at 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

commit 33cd4c0daf9bb5134d14fc34389e19697fa2ea3c
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Fri Oct 9 12:26:04 2020 -0400

    16989: Add test to sync is_invited
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/services/api/test/integration/remote_user_test.rb b/services/api/test/integration/remote_user_test.rb
index 9ad93c2ee..d3180e173 100644
--- a/services/api/test/integration/remote_user_test.rb
+++ b/services/api/test/integration/remote_user_test.rb
@@ -348,7 +348,30 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
     assert_equal 'barney', json_response['username']
   end
 
-    test 'get active user from Login cluster when AutoSetupNewUsers is set' do
+  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 at 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 at example.com', json_response['email']
+    assert_equal 'barney', json_response['username']
+  end
+
+  test 'get active user from Login cluster when AutoSetupNewUsers is set' do
     Rails.configuration.Login.LoginCluster = 'zbbbb'
     Rails.configuration.Users.AutoSetupNewUsers = true
     @stub_content = {

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list