[ARVADOS] created: 2.1.0-1713-gb7fb5c459
Git user
git at public.arvados.org
Fri Dec 10 14:46:12 UTC 2021
at b7fb5c4593dcc679f5343f0f55b3774a7bcfe499 (commit)
commit b7fb5c4593dcc679f5343f0f55b3774a7bcfe499
Author: Tom Clegg <tom at curii.com>
Date: Fri Dec 10 09:46:00 2021 -0500
18277: Add RoleGroupsVisibleToAll config, default true.
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 517c2dd0c..411e2ea0f 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-11-10)
"previous: Upgrading from 2.3.0":#v2_3_0
+h3. Role groups are visible to all users by default
+
+The permission model has changed such that all role groups are visible to all active users. This enables users to share objects with groups they don't belong to. To preserve the previous behavior, where role groups are only visible to members and admins, add @RoleGroupsVisibleToAll: false@ to the @Users@ section of your configuration file.
+
h3. Default LSF arguments have changed
If you use LSF and your configuration specifies @Containers.LSF.BsubArgumentsList@, you should update it to include the new arguments (@"-R", "select[mem>=%MMB]", ...@, see "configuration reference":{{site.baseurl}}/admin/config.html). Otherwise, containers that are too big to run on any LSF host will remain in the LSF queue instead of being cancelled.
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index a84dc5d31..7f8598225 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -319,6 +319,14 @@ Clusters:
Thanks,
Your Arvados administrator.
+ # If RoleGroupsVisibleToAll is true, all role groups are visible
+ # to all active users.
+ #
+ # If false, users must be granted permission to role groups in
+ # order to see them. This is more appropriate for a multi-tenant
+ # cluster.
+ RoleGroupsVisibleToAll: true
+
AuditLogs:
# Time to keep audit logs, in seconds. (An audit log is a row added
# to the "logs" table in the PostgreSQL database each time an
diff --git a/lib/config/export.go b/lib/config/export.go
index 4c4e341f5..cfc4cb627 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -234,6 +234,7 @@ var whitelist = map[string]bool{
"Users.UserNotifierEmailBcc": false,
"Users.UserProfileNotificationAddress": false,
"Users.UserSetupMailText": false,
+ "Users.RoleGroupsVisibleToAll": false,
"Volumes": true,
"Volumes.*": true,
"Volumes.*.*": false,
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 567ac30a9..485273e1b 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -325,6 +325,14 @@ Clusters:
Thanks,
Your Arvados administrator.
+ # If RoleGroupsVisibleToAll is true, all role groups are visible
+ # to all active users.
+ #
+ # If false, users must be granted permission to role groups in
+ # order to see them. This is more appropriate for a multi-tenant
+ # cluster.
+ RoleGroupsVisibleToAll: true
+
AuditLogs:
# Time to keep audit logs, in seconds. (An audit log is a row added
# to the "logs" table in the PostgreSQL database each time an
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 474ce33b0..6d5156e76 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -241,6 +241,7 @@ type Cluster struct {
UserProfileNotificationAddress string
PreferDomainForUsername string
UserSetupMailText string
+ RoleGroupsVisibleToAll bool
}
StorageClasses map[string]StorageClassConfig
Volumes map[string]Volume
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index af226cde8..54b1394ff 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -384,6 +384,14 @@ class ArvadosModel < ApplicationRecord
direct_check = " OR " + direct_check
end
+ if Rails.configuration.Users.RoleGroupsVisibleToAll &&
+ sql_table == "groups" &&
+ users_list.select { |u| u.is_active }.any?
+ # All role groups are readable (but we still need the other
+ # direct_check clauses to handle non-role groups).
+ direct_check += " OR #{sql_table}.group_class = 'role'"
+ end
+
links_cond = ""
if sql_table == "links"
# 1) Match permission links incoming or outgoing on the
diff --git a/services/api/config/arvados_config.rb b/services/api/config/arvados_config.rb
index 49865039c..c5db6e6b9 100644
--- a/services/api/config/arvados_config.rb
+++ b/services/api/config/arvados_config.rb
@@ -100,6 +100,7 @@ arvcfg.declare_config "Users.UserNotifierEmailFrom", String, :user_notifier_emai
arvcfg.declare_config "Users.UserNotifierEmailBcc", Hash
arvcfg.declare_config "Users.NewUserNotificationRecipients", Hash, :new_user_notification_recipients, ->(cfg, k, v) { arrayToHash cfg, "Users.NewUserNotificationRecipients", v }
arvcfg.declare_config "Users.NewInactiveUserNotificationRecipients", Hash, :new_inactive_user_notification_recipients, method(:arrayToHash)
+arvcfg.declare_config "Users.RoleGroupsVisibleToAll", Boolean
arvcfg.declare_config "Login.LoginCluster", String
arvcfg.declare_config "Login.TrustedClients", Hash
arvcfg.declare_config "Login.RemoteTokenRefresh", ActiveSupport::Duration
diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb
index 4dbccc5eb..0819c2306 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -6,12 +6,19 @@ require 'test_helper'
class Arvados::V1::GroupsControllerTest < ActionController::TestCase
- test "attempt to delete group without read or write access" do
+ test "attempt to delete group that cannot be seen" do
+ Rails.configuration.Users.RoleGroupsVisibleToAll = false
authorize_with :active
post :destroy, params: {id: groups(:empty_lonely_group).uuid}
assert_response 404
end
+ test "attempt to delete group without read or write access" do
+ authorize_with :active
+ post :destroy, params: {id: groups(:empty_lonely_group).uuid}
+ assert_response 403
+ end
+
test "attempt to delete group without write access" do
authorize_with :active
post :destroy, params: {id: groups(:all_users).uuid}
@@ -553,6 +560,30 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
end
end
+ [
+ [false, :inactive, :private_role, false],
+ [false, :spectator, :private_role, false],
+ [false, :admin, :private_role, true],
+ [true, :inactive, :private_role, false],
+ [true, :spectator, :private_role, true],
+ [true, :admin, :private_role, true],
+ # project (non-role) groups are invisible even when RoleGroupsVisibleToAll is true
+ [true, :inactive, :private, false],
+ [true, :spectator, :private, false],
+ [true, :admin, :private, true],
+ ].each do |visibleToAll, userFixture, groupFixture, visible|
+ test "with RoleGroupsVisibleToAll=#{visibleToAll}, #{groupFixture} group is #{visible ? '' : 'in'}visible to #{userFixture} user" do
+ Rails.configuration.Users.RoleGroupsVisibleToAll = visibleToAll
+ authorize_with userFixture
+ get :show, params: {id: groups(groupFixture).uuid, format: :json}
+ if visible
+ assert_response :success
+ else
+ assert_response 404
+ end
+ end
+ end
+
### trashed project tests ###
#
diff --git a/services/api/test/integration/remote_user_test.rb b/services/api/test/integration/remote_user_test.rb
index 568d5088d..179d30f3c 100644
--- a/services/api/test/integration/remote_user_test.rb
+++ b/services/api/test/integration/remote_user_test.rb
@@ -304,6 +304,7 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
end
test "list readable groups with salted token" do
+ Rails.configuration.Users.RoleGroupsVisibleToAll = false
salted_token = salt_token(fixture: :active, remote: 'zbbbb')
get '/arvados/v1/groups',
params: {
diff --git a/services/api/test/unit/permission_test.rb b/services/api/test/unit/permission_test.rb
index 128d0ebaa..647139d9e 100644
--- a/services/api/test/unit/permission_test.rb
+++ b/services/api/test/unit/permission_test.rb
@@ -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.RoleGroupsVisibleToAll = false
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"
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list