[ARVADOS] created: 2.1.0-1559-gd3f8ffaaa

Git user git at public.arvados.org
Thu Oct 28 21:10:50 UTC 2021


        at  d3f8ffaaa1085bf53e1696fcd7f491ed92515f19 (commit)


commit d3f8ffaaa1085bf53e1696fcd7f491ed92515f19
Author: Tom Clegg <tom at curii.com>
Date:   Thu Oct 28 17:10:31 2021 -0400

    16817: Add Users.ActivatedUsersAreVisibleToOthers config.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/doc/admin/upgrading.html.textile.liquid b/doc/admin/upgrading.html.textile.liquid
index 0aea90bd0..a05902125 100644
--- a/doc/admin/upgrading.html.textile.liquid
+++ b/doc/admin/upgrading.html.textile.liquid
@@ -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 at .
+
 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.
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 97ded6bf6..12415a83a 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -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: ""
diff --git a/lib/config/export.go b/lib/config/export.go
index e36d6e76c..73ef2d0cf 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -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,
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index f7849d614..3d7d90f40 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -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: ""
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index e736f79fd..e7e60aed7 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -220,6 +220,7 @@ type Cluster struct {
 		Insecure    bool
 	}
 	Users struct {
+		ActivatedUsersAreVisibleToOthers      bool
 		AnonymousUserToken                    string
 		AdminNotifierEmailFrom                string
 		AutoAdminFirstUser                    bool
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 366c03e30..febb8ea51 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -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
diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index c00164c0a..7368d8937 100644
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -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 at example.com'
+      email = 'foo at 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

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list