21059: Remote user status WIP

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index a1b743cf51825502261d9b86fb106406d061698f..7cc7cc491ca71d088d4fe64e436e54ac6eb375e2 100644 (file)
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -32,7 +32,7 @@ class User < ArvadosModel
 
   before_create :check_auto_admin
   before_create :set_initial_username, :if => Proc.new {
-    username.nil? and email
+    email
   }
   before_create :active_is_not_nil
   after_create :after_ownership_change
@@ -386,6 +386,10 @@ SELECT target_uuid, perm_level
   end
 
   def set_initial_username(requested: false)
+    if new_record? and requested == false and self.username != nil and self.username != ""
+      requested = self.username
+    end
+
     if (!requested.is_a?(String) || requested.empty?) and email
       email_parts = email.partition("@")
       local_parts = email_parts.first.partition("+")
@@ -607,10 +611,57 @@ SELECT target_uuid, perm_level
     remote_user = remote_user.symbolize_keys
     remote_user_prefix = remote_user[:uuid][0..4]
 
+    # interaction between is_invited and is_active
+    #
+    # either can flag can be nil, true or false
+    #
+    # in all cases, we create the user if they don't exist.
+    #
+    # invited nil, active nil: don't call setup or unsetup.
+    #
+    # invited nil, active false: call unsetup
+    #
+    # invited nil, active true: call setup and activate them.
+    #
+    #
+    # invited false, active nil: call unsetup
+    #
+    # invited false, active false: call unsetup
+    #
+    # invited false, active true: call unsetup
+    #
+    #
+    # invited true, active nil: call setup but don't change is_active
+    #
+    # invited true, active false: call setup but don't change is_active
+    #
+    # invited true, active true: call setup and activate them.
+
+    should_setup = (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"])
+
+    should_activate = (remote_user_prefix == Rails.configuration.Login.LoginCluster or
+                       Rails.configuration.Users.NewUsersAreActive or
+                       Rails.configuration.RemoteClusters[remote_user_prefix].andand["ActivateUsers"])
+
+    remote_should_be_unsetup = (remote_user[:is_invited] == nil && remote_user[:is_active] == false) ||
+                               (remote_user[:is_invited] == false)
+
+    remote_should_be_setup = should_setup && (
+      (remote_user[:is_invited] == nil && remote_user[:is_active] == true) ||
+      (remote_user[:is_invited] == false && remote_user[:is_active] == true) ||
+      (remote_user[:is_invited] == true))
+
+    remote_should_be_active = should_activate && remote_user[:is_invited] != false && remote_user[:is_active] == true
+
     begin
       user = User.create_with(email: remote_user[:email],
+                              username: remote_user[:username],
                               first_name: remote_user[:first_name],
                               last_name: remote_user[:last_name],
+                              is_active: remote_should_be_active
       ).find_or_create_by(uuid: remote_user[:uuid])
     rescue ActiveRecord::RecordNotUnique
       retry
@@ -661,30 +712,19 @@ SELECT target_uuid, perm_level
         end
       end
 
-      if user.is_invited && (remote_user[:is_invited] == false || remote_user[:is_active] == false)
+      if remote_should_be_unsetup
         # Remote user is not "invited" or "active" state on their home
         # cluster, so they should be unsetup, which also makes them
         # inactive.
         user.unsetup
       else
-        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"])
-          # Remote user is 'invited' and should be set up
+        if !user.is_invited && remote_should_be_setup
           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"])
+        if !user.is_active && remote_should_be_active
           # remote user is active and invited, we need to activate them
           user.update!(is_active: true)
-        elsif user.is_active && remote_user[:is_active] == false
-          # remote user is not active, we need to de-activate them
-          user.update!(is_active: false)
         end
 
         if remote_user_prefix == Rails.configuration.Login.LoginCluster and

diff --git a/services/api/app/views/admin_notifier/new_inactive_user.text.erb b/services/api/app/views/admin_notifier/new_inactive_user.text.erb
index afcf34da714e591066a7e5f706cbaa23d130db11..4b572128b52500603b3a8e00a53d5e720025d25c 100644 (file)
--- a/services/api/app/views/admin_notifier/new_inactive_user.text.erb
+++ b/services/api/app/views/admin_notifier/new_inactive_user.text.erb
@@ -2,15 +2,14 @@
 
 SPDX-License-Identifier: AGPL-3.0 %>
 
-
 A new user landed on the inactive user page:
 
   <%= @user.full_name %> <<%= @user.email %>>
 
 <% if Rails.configuration.Services.Workbench1.ExternalURL -%>
-Please see workbench for more information:
+Please see Workbench for more information:
 
-  <%= Rails.configuration.Services.Workbench1.ExternalURL %>
+  <%= Rails.configuration.Services.Workbench1.ExternalURL %>user/<%=@user.uuid%>
 
 <% end -%>
 Thanks,

diff --git a/services/api/app/views/admin_notifier/new_user.text.erb b/services/api/app/views/admin_notifier/new_user.text.erb
index 670b84b7c11dd874b1eaaf976cffe3b495633fab..75d2a28ce8d21d142b67739adbea61696da4d6b3 100644 (file)
--- a/services/api/app/views/admin_notifier/new_user.text.erb
+++ b/services/api/app/views/admin_notifier/new_user.text.erb
@@ -3,10 +3,7 @@
 SPDX-License-Identifier: AGPL-3.0 %>
 
 <%
-  add_to_message = ''
-  if Rails.configuration.Users.AutoSetupNewUsers
     add_to_message = @user.is_invited ? ' and setup' : ', but not setup'
-  end
 %>
 A new user has been created<%=add_to_message%>:
 
@@ -15,9 +12,9 @@ A new user has been created<%=add_to_message%>:
 This user is <%= @user.is_active ? '' : 'NOT ' %>active.
 
 <% if Rails.configuration.Services.Workbench1.ExternalURL -%>
-Please see workbench for more information:
+Please see Workbench for more information:
 
-  <%= Rails.configuration.Services.Workbench1.ExternalURL %>
+  <%= Rails.configuration.Services.Workbench1.ExternalURL %>user/<%=@user.uuid%>
 
 <% end -%>
 Thanks,

diff --git a/services/api/app/views/user_notifier/account_is_setup.text.erb b/services/api/app/views/user_notifier/account_is_setup.text.erb
index 352ee7754e1e6855c1b2ffc946dad5c24d0962fe..3f04db8517fb216727a632a3af03e770b3bc4146 100644 (file)
--- a/services/api/app/views/user_notifier/account_is_setup.text.erb
+++ b/services/api/app/views/user_notifier/account_is_setup.text.erb
@@ -2,4 +2,4 @@
 
 SPDX-License-Identifier: AGPL-3.0 %>
 
-<%= ERB.new(Rails.configuration.Users.UserSetupMailText, 0, "-").result(binding) %>
+<%= ERB.new(Rails.configuration.Users.UserSetupMailText, trim_mode: "-").result(binding) %>

diff --git a/services/api/test/integration/remote_user_test.rb b/services/api/test/integration/remote_user_test.rb
index 198f574ef2dda30569f99d5d7939a0533d04a223..df1a2259700e5212ed69c1c71b92f422a7aa6fc9 100644 (file)
--- a/services/api/test/integration/remote_user_test.rb
+++ b/services/api/test/integration/remote_user_test.rb
@@ -100,6 +100,8 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
       uuid: 'zbbbb-tpzed-000000000000001',
       email: 'foo@example.com',
       username: 'barney',
+      first_name: "Barney",
+      last_name: "Foo",
       is_admin: true,
       is_active: true,
       is_invited: true,
@@ -383,9 +385,9 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
     assert_equal 'foo@example.com', json_response['email']
     assert_equal 'barney', json_response['username']
 
-    ActionMailer::Base.deliveries.each do |d|
-      puts "--- delivery #{d.inspect}"
-    end
+    # ActionMailer::Base.deliveries.each do |d|
+    #   puts "--- delivery #{d.inspect} #{d.body}"
+    # end
 
     assert_equal 2, ActionMailer::Base.deliveries.length
     assert_equal "Welcome to Arvados - account enabled", ActionMailer::Base.deliveries[0].subject
@@ -394,10 +396,10 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
 
   [true, false].each do |trusted|
     [true, false].each do |logincluster|
-      [true, false].each do |admin|
-        [true, false].each do |active|
+      [true, false, nil].each do |admin|
+        [true, false, nil].each do |active|
           [true, false].each do |autosetup|
-            [true, false].each do |invited|
+            [true, false, nil].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
@@ -415,9 +417,9 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
                     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 (logincluster && !!admin && (invited != false) && !!active), json_response['is_admin']
+                assert_equal ((invited == true || (invited == nil && !!active)) && (logincluster || trusted || autosetup)), json_response['is_invited']
+                assert_equal ((invited != false) && (logincluster || trusted) && !!active), json_response['is_active']
                 assert_equal 'foo@example.com', json_response['email']
                 assert_equal 'barney', json_response['username']
               end