Merge branch '16387-batch-update-deactivated-user'

fixes #16387

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>

diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 4939b116b0bce22dfe9c3ad1c2bf79a89797b655..3bf64771d70b30d08d6c53312384071bef14a259 100644 (file)
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -309,6 +309,7 @@ func (s *IntegrationSuite) TestListUsers(c *check.C) {
        rootctx1, _, _ := s.rootClients("z1111")
        conn1 := s.conn("z1111")
        conn3 := s.conn("z3333")
+       userctx1, _, _ := s.userClients(rootctx1, c, conn1, "z1111", true)
 
        // Make sure LoginCluster is properly configured
        for cls := range s.testClusters {
@@ -318,7 +319,9 @@ func (s *IntegrationSuite) TestListUsers(c *check.C) {
                        check.Commentf("incorrect LoginCluster config on cluster %q", cls))
        }
        // Make sure z1111 has users with NULL usernames
-       lst, err := conn1.UserList(rootctx1, arvados.ListOptions{Limit: -1})
+       lst, err := conn1.UserList(rootctx1, arvados.ListOptions{
+               Limit: math.MaxInt64, // check that large limit works (see #16263)
+       })
        nullUsername := false
        c.Assert(err, check.IsNil)
        c.Assert(len(lst.Items), check.Not(check.Equals), 0)
@@ -328,27 +331,45 @@ func (s *IntegrationSuite) TestListUsers(c *check.C) {
                }
        }
        c.Assert(nullUsername, check.Equals, true)
+
+       user1, err := conn1.UserGetCurrent(userctx1, arvados.GetOptions{})
+       c.Assert(err, check.IsNil)
+       c.Check(user1.IsActive, check.Equals, true)
+
        // Ask for the user list on z3333 using z1111's system root token
-       _, err = conn3.UserList(rootctx1, arvados.ListOptions{Limit: -1})
-       c.Assert(err, check.IsNil, check.Commentf("getting user list: %q", err))
-}
+       lst, err = conn3.UserList(rootctx1, arvados.ListOptions{Limit: -1})
+       c.Assert(err, check.IsNil)
+       found := false
+       for _, user := range lst.Items {
+               if user.UUID == user1.UUID {
+                       c.Check(user.IsActive, check.Equals, true)
+                       found = true
+                       break
+               }
+       }
+       c.Check(found, check.Equals, true)
 
-// Test for bug #16263
-func (s *IntegrationSuite) TestListUsersWithMaxLimit(c *check.C) {
-       rootctx1, _, _ := s.rootClients("z1111")
-       conn3 := s.conn("z3333")
-       maxLimit := int64(math.MaxInt64)
+       // Deactivate user acct on z1111
+       _, err = conn1.UserUnsetup(rootctx1, arvados.GetOptions{UUID: user1.UUID})
+       c.Assert(err, check.IsNil)
 
-       // Make sure LoginCluster is properly configured
-       for cls := range s.testClusters {
-               c.Check(
-                       s.testClusters[cls].config.Clusters[cls].Login.LoginCluster,
-                       check.Equals, "z1111",
-                       check.Commentf("incorrect LoginCluster config on cluster %q", cls))
+       // Get user list from z3333, check the returned z1111 user is
+       // deactivated
+       lst, err = conn3.UserList(rootctx1, arvados.ListOptions{Limit: -1})
+       c.Assert(err, check.IsNil)
+       found = false
+       for _, user := range lst.Items {
+               if user.UUID == user1.UUID {
+                       c.Check(user.IsActive, check.Equals, false)
+                       found = true
+                       break
+               }
        }
+       c.Check(found, check.Equals, true)
 
-       // Ask for the user list on z3333 using z1111's system root token and
-       // limit: max int64 value.
-       _, err := conn3.UserList(rootctx1, arvados.ListOptions{Limit: maxLimit})
-       c.Assert(err, check.IsNil, check.Commentf("getting user list: %q", err))
+       // Deactivated user can see is_active==false via "get current
+       // user" API
+       user1, err = conn3.UserGetCurrent(userctx1, arvados.GetOptions{})
+       c.Assert(err, check.IsNil)
+       c.Check(user1.IsActive, check.Equals, false)
 }

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index d9ab5556ffc9ac7826abda00bc18e3d4b700269c..867b9a6e6abfdf0ae050a668f4340d1664608586 100644 (file)
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -54,7 +54,11 @@ class Arvados::V1::UsersController < ApplicationController
       @object = current_user
     end
     if not @object.is_active
-      if not (current_user.is_admin or @object.is_invited)
+      if @object.uuid[0..4] == Rails.configuration.Login.LoginCluster &&
+         @object.uuid[0..4] != Rails.configuration.ClusterID
+        logger.warn "Local user #{@object.uuid} called users#activate but only LoginCluster can do that"
+        raise ArgumentError.new "cannot activate user #{@object.uuid} here, only the #{@object.uuid[0..4]} cluster can do that"
+      elsif not (current_user.is_admin or @object.is_invited)
         logger.warn "User #{@object.uuid} called users.activate " +
           "but is not invited"
         raise ArgumentError.new "Cannot activate without being invited."

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index dd447ca51a895fa2297d6860002a52ff7f360037..c3641b64e84f04217145edacab05ac8d84f259a7 100644 (file)
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -238,8 +238,14 @@ class User < ArvadosModel
   end
 
   def must_unsetup_to_deactivate
-    if self.is_active_changed? &&
-       self.is_active_was == true &&
+    if !self.new_record? &&
+       self.uuid[0..4] == Rails.configuration.Login.LoginCluster &&
+       self.uuid[0..4] != Rails.configuration.ClusterID
+      # OK to update our local record to whatever the LoginCluster
+      # reports, because self-activate is not allowed.
+      return
+    elsif self.is_active_changed? &&
+       self.is_active_was &&
        !self.is_active
 
       group = Group.where(name: 'All users').select do |g|