16817: Add Users.ActivatedUsersAreVisibleToOthers config.
authorTom Clegg <tom@curii.com>
Fri, 29 Oct 2021 14:08:48 +0000 (10:08 -0400)
committerTom Clegg <tom@curii.com>
Fri, 29 Oct 2021 14:08:48 +0000 (10:08 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

doc/admin/upgrading.html.textile.liquid
lib/config/config.default.yml
lib/config/export.go
lib/config/generated_config.go
sdk/go/arvados/config.go
services/api/app/models/user.rb
services/api/test/functional/arvados/v1/users_controller_test.rb
services/api/test/unit/permission_test.rb
services/api/test/unit/user_test.rb

index 0aea90bd0ddab04164ee6a668e26c1851ab6e934..a05902125320470f148307b203dbf708d2b3828e 100644 (file)
@@ -39,6 +39,10 @@ h2(#main). development main (as of 2021-10-27)
 
 "previous: Upgrading from 2.3.0":#v2_3_0
 
+h3. Users are visible to other users by default
+
+When a new user is set up (either via @AutoSetupNewUsers@ config or via Workbench admin interface) the user immediately becomes visible to other users. To revert to the previous behavior, where the administrator must add two users to the same group using the Workbench admin interface in order for the users to see each other, change the new @Users.ActivatedUsersAreVisibleToOthers@ config to @false@.
+
 h3. Dedicated keepstore process for each container
 
 When Arvados runs a container via @arvados-dispatch-cloud@, the @crunch-run@ supervisor process now brings up its own keepstore server to handle I/O for mounted collections, outputs, and logs. With the default configuration, the keepstore process allocates one 64 MiB block buffer per VCPU requested by the container. For most workloads this will increase throughput, reduce total network traffic, and make it possible to run more containers at once without provisioning additional keepstore nodes to handle the I/O load.
index 97ded6bf6863739e23ed53d365be8db1014a5b35..12415a83a05d04a310a2cdea6b5bd2f325906682 100644 (file)
@@ -259,6 +259,16 @@ Clusters:
       # user agreements.  Should only be enabled for development.
       NewUsersAreActive: false
 
+      # Newly activated users (whether set up by an admin or via
+      # AutoSetupNewUsers) immediately become visible to other active
+      # users.
+      #
+      # On a multi-tenant cluster, where the intent is for users to be
+      # invisible to one another unless they have been added to the
+      # same group(s) via Workbench admin interface, change this to
+      # false.
+      ActivatedUsersAreVisibleToOthers: true
+
       # The e-mail address of the user you would like to become marked as an admin
       # user on their first login.
       AutoAdminUserWithEmail: ""
index e36d6e76cae40d17c54217e643bf82341e298b87..73ef2d0cfc29b53d39221ababf388116bd7fa538 100644 (file)
@@ -215,6 +215,7 @@ var whitelist = map[string]bool{
        "SystemRootToken":                                     false,
        "TLS":                                                 false,
        "Users":                                               true,
+       "Users.ActivatedUsersAreVisibleToOthers":              false,
        "Users.AdminNotifierEmailFrom":                        false,
        "Users.AnonymousUserToken":                            true,
        "Users.AutoAdminFirstUser":                            false,
index f7849d6142cda6e4353a076a90a561295f7fa034..3d7d90f40c74a248ee9a1cb261dd536ddad4d63c 100644 (file)
@@ -265,6 +265,16 @@ Clusters:
       # user agreements.  Should only be enabled for development.
       NewUsersAreActive: false
 
+      # Newly activated users (whether set up by an admin or via
+      # AutoSetupNewUsers) immediately become visible to other active
+      # users.
+      #
+      # On a multi-tenant cluster, where the intent is for users to be
+      # invisible to one another unless they have been added to the
+      # same group(s) via Workbench admin interface, change this to
+      # false.
+      ActivatedUsersAreVisibleToOthers: true
+
       # The e-mail address of the user you would like to become marked as an admin
       # user on their first login.
       AutoAdminUserWithEmail: ""
index e736f79fd7f2bcee24e449596b4950bd279881bc..e7e60aed78aff7d34ef8483cce1795b15a99dcc5 100644 (file)
@@ -220,6 +220,7 @@ type Cluster struct {
                Insecure    bool
        }
        Users struct {
+               ActivatedUsersAreVisibleToOthers      bool
                AnonymousUserToken                    string
                AdminNotifierEmailFrom                string
                AutoAdminFirstUser                    bool
index 366c03e309ca6f54c0ef5bd2ad9f3aa331c592ea..febb8ea51611eb5a21549b8133770fe059a2ca5c 100644 (file)
@@ -234,8 +234,9 @@ SELECT target_uuid, perm_level
                               name: 'can_read').empty?
 
     # Add can_read link from this user to "all users" which makes this
-    # user "invited"
-    group_perm = create_user_group_link
+    # user "invited", and (depending on config) a link in the opposite
+    # direction which makes this user visible to other users.
+    group_perms = add_to_all_users_group
 
     # Add git repo
     repo_perm = if (!repo_name.nil? || Rails.configuration.Users.AutoSetupNewUsersWithRepository) and !username.nil?
@@ -267,7 +268,7 @@ SELECT target_uuid, perm_level
 
     forget_cached_group_perms
 
-    return [repo_perm, vm_login_perm, group_perm, self].compact
+    return [repo_perm, vm_login_perm, *group_perms, self].compact
   end
 
   # delete user signatures, login, repo, and vm perms, and mark as inactive
@@ -728,16 +729,26 @@ SELECT target_uuid, perm_level
     login_perm
   end
 
-  # add the user to the 'All users' group
-  def create_user_group_link
-    return (Link.where(tail_uuid: self.uuid,
+  def add_to_all_users_group
+    resp = [Link.where(tail_uuid: self.uuid,
                        head_uuid: all_users_group_uuid,
                        link_class: 'permission',
-                       name: 'can_read').first or
+                       name: 'can_read').first ||
             Link.create(tail_uuid: self.uuid,
                         head_uuid: all_users_group_uuid,
                         link_class: 'permission',
-                        name: 'can_read'))
+                        name: 'can_read')]
+    if Rails.configuration.Users.ActivatedUsersAreVisibleToOthers
+      resp += [Link.where(tail_uuid: all_users_group_uuid,
+                          head_uuid: self.uuid,
+                          link_class: 'permission',
+                          name: 'can_read').first ||
+               Link.create(tail_uuid: all_users_group_uuid,
+                           head_uuid: self.uuid,
+                           link_class: 'permission',
+                           name: 'can_read')]
+    end
+    return resp
   end
 
   # Give the special "System group" permission to manage this user and
index c807a7d6cb5ec6f3f4d92ceb18fb519629a3d1fb..ae7b21dec83e556f8321ee7290e5680824be5881 100644 (file)
@@ -13,6 +13,7 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     @initial_link_count = Link.count
     @vm_uuid = virtual_machines(:testvm).uuid
     ActionMailer::Base.deliveries = []
+    Rails.configuration.Users.ActivatedUsersAreVisibleToOthers = false
   end
 
   test "activate a user after signing UA" do
index 123031b35feb90b0fc874b0461fff896ca531702..128d0ebaa6f6f255ce54add5e8aac9b39ecca208 100644 (file)
@@ -218,6 +218,7 @@ class PermissionTest < ActiveSupport::TestCase
   end
 
   test "manager user gets permission to minions' articles via can_manage link" do
+    Rails.configuration.Users.ActivatedUsersAreVisibleToOthers = false
     manager = create :active_user, first_name: "Manage", last_name: "Er"
     minion = create :active_user, first_name: "Min", last_name: "Ion"
     minions_specimen = act_as_user minion do
@@ -314,6 +315,7 @@ class PermissionTest < ActiveSupport::TestCase
   end
 
   test "users with bidirectional read permission in group can see each other, but cannot see each other's private articles" do
+    Rails.configuration.Users.ActivatedUsersAreVisibleToOthers = false
     a = create :active_user, first_name: "A"
     b = create :active_user, first_name: "B"
     other = create :active_user, first_name: "OTHER"
index c00164c0a36235254248ece8fef06fed3f59e4be..7368d893745658fd20de42de6d2410f421a1098b 100644 (file)
@@ -447,30 +447,40 @@ class UserTest < ActiveSupport::TestCase
     assert_not_allowed { User.new.save }
   end
 
-  test "setup new user" do
-    set_user_from_auth :admin
+  [true, false].each do |visible|
+    test "setup new user with ActivatedUsersAreVisibleToOthers=#{visible}" do
+      Rails.configuration.Users.ActivatedUsersAreVisibleToOthers = visible
+      set_user_from_auth :admin
 
-    email = 'foo@example.com'
+      email = 'foo@example.com'
 
-    user = User.create ({uuid: 'zzzzz-tpzed-abcdefghijklmno', email: email})
+      user = User.create ({uuid: 'zzzzz-tpzed-abcdefghijklmno', email: email})
 
-    vm = VirtualMachine.create
+      vm = VirtualMachine.create
 
-    response = user.setup(repo_name: 'foo/testrepo',
-                          vm_uuid: vm.uuid)
+      response = user.setup(repo_name: 'foo/testrepo',
+                            vm_uuid: vm.uuid)
 
-    resp_user = find_obj_in_resp response, 'User'
-    verify_user resp_user, email
+      resp_user = find_obj_in_resp response, 'User'
+      verify_user resp_user, email
 
-    group_perm = find_obj_in_resp response, 'Link', 'arvados#group'
-    verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil
+      group_perm = find_obj_in_resp response, 'Link', 'arvados#group'
+      verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil
 
-    repo_perm = find_obj_in_resp response, 'Link', 'arvados#repository'
-    verify_link repo_perm, 'permission', 'can_manage', resp_user[:uuid], nil
+      group_perm2 = find_obj_in_resp response, 'Link', 'arvados#user'
+      if visible
+        verify_link group_perm2, 'permission', 'can_read', groups(:all_users).uuid, nil
+      else
+        assert_nil group_perm2
+      end
 
-    vm_perm = find_obj_in_resp response, 'Link', 'arvados#virtualMachine'
-    verify_link vm_perm, 'permission', 'can_login', resp_user[:uuid], vm.uuid
-    assert_equal("foo", vm_perm.properties["username"])
+      repo_perm = find_obj_in_resp response, 'Link', 'arvados#repository'
+      verify_link repo_perm, 'permission', 'can_manage', resp_user[:uuid], nil
+
+      vm_perm = find_obj_in_resp response, 'Link', 'arvados#virtualMachine'
+      verify_link vm_perm, 'permission', 'can_login', resp_user[:uuid], vm.uuid
+      assert_equal("foo", vm_perm.properties["username"])
+    end
   end
 
   test "setup new user with junk in database" do
@@ -514,6 +524,9 @@ class UserTest < ActiveSupport::TestCase
     group_perm = find_obj_in_resp response, 'Link', 'arvados#group'
     verify_link group_perm, 'permission', 'can_read', resp_user[:uuid], nil
 
+    group_perm2 = find_obj_in_resp response, 'Link', 'arvados#user'
+    verify_link group_perm2, 'permission', 'can_read', groups(:all_users).uuid, nil
+
     # invoke setup again with repo_name
     response = user.setup(repo_name: 'foo/testrepo')
     resp_user = find_obj_in_resp response, 'User', nil
@@ -560,7 +573,7 @@ class UserTest < ActiveSupport::TestCase
           break
         end
       else  # looking for a link
-        if ArvadosModel::resource_class_for_uuid(x['head_uuid']).kind == head_kind
+        if ArvadosModel::resource_class_for_uuid(x['head_uuid']).andand.kind == head_kind
           return_obj = x
           break
         end