[ARVADOS] updated: 1.3.0-3244-g2a1a14393
Git user
git at public.arvados.org
Wed Sep 30 21:52:25 UTC 2020
Summary of changes:
lib/controller/federation/conn.go | 2 +-
lib/controller/federation/user_test.go | 54 +++++++++++++++++++
.../api/app/models/api_client_authorization.rb | 10 ++--
services/api/app/models/user.rb | 3 +-
services/api/test/integration/remote_user_test.rb | 62 ++++++++++++++++++++++
5 files changed, 123 insertions(+), 8 deletions(-)
via 2a1a143938bfbfa9713c8f368898a8dda1ac685f (commit)
from 5cefb12be01e9df6997f88696048ecb5d77d305e (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 2a1a143938bfbfa9713c8f368898a8dda1ac685f
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed Sep 30 17:13:59 2020 -0400
UserGet() passes "select" option along to batch update refs #16914
Fixes issue of user being deactivated because something called "get
user" and selected fields didn't include is_active.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 43fe1cc01..f07c3b631 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -508,7 +508,7 @@ func (conn *Conn) UserGet(ctx context.Context, options arvados.GetOptions) (arva
return arvados.User{}, httpErrorf(http.StatusBadGateway, "Had requested %v but response was for %v", options.UUID, resp.UUID)
}
if options.UUID[:5] != conn.cluster.ClusterID {
- err = conn.batchUpdateUsers(ctx, arvados.ListOptions{}, []arvados.User{resp})
+ err = conn.batchUpdateUsers(ctx, arvados.ListOptions{Select: options.Select}, []arvados.User{resp})
if err != nil {
return arvados.User{}, err
}
diff --git a/lib/controller/federation/user_test.go b/lib/controller/federation/user_test.go
index 09aa5086d..2812c1f41 100644
--- a/lib/controller/federation/user_test.go
+++ b/lib/controller/federation/user_test.go
@@ -117,6 +117,60 @@ func (s *UserSuite) TestLoginClusterUserList(c *check.C) {
}
}
+func (s *UserSuite) TestLoginClusterUserGet(c *check.C) {
+ s.cluster.ClusterID = "local"
+ s.cluster.Login.LoginCluster = "zzzzz"
+ s.fed = New(s.cluster)
+ s.addDirectRemote(c, "zzzzz", rpc.NewConn("zzzzz", &url.URL{Scheme: "https", Host: os.Getenv("ARVADOS_API_HOST")}, true, rpc.PassthroughTokenProvider))
+
+ opts := arvados.GetOptions{UUID: "zzzzz-tpzed-xurymjxw79nv3jz", Select: []string{"uuid", "email"}}
+
+ stub := &arvadostest.APIStub{Error: errors.New("local cluster failure")}
+ s.fed.local = stub
+ s.fed.UserGet(s.ctx, opts)
+
+ calls := stub.Calls(stub.UserBatchUpdate)
+ if c.Check(calls, check.HasLen, 1) {
+ c.Logf("... stub.UserUpdate called with options: %#v", calls[0].Options)
+ shouldUpdate := map[string]bool{
+ "uuid": false,
+ "email": true,
+ "first_name": true,
+ "last_name": true,
+ "is_admin": true,
+ "is_active": true,
+ "prefs": true,
+ // can't safely update locally
+ "owner_uuid": false,
+ "identity_url": false,
+ // virtual attrs
+ "full_name": false,
+ "is_invited": false,
+ }
+ if opts.Select != nil {
+ // Only the selected
+ // fields (minus uuid)
+ // should be updated.
+ for k := range shouldUpdate {
+ shouldUpdate[k] = false
+ }
+ for _, k := range opts.Select {
+ if k != "uuid" {
+ shouldUpdate[k] = true
+ }
+ }
+ }
+ var uuid string
+ for uuid = range calls[0].Options.(arvados.UserBatchUpdateOptions).Updates {
+ }
+ for k, shouldFind := range shouldUpdate {
+ _, found := calls[0].Options.(arvados.UserBatchUpdateOptions).Updates[uuid][k]
+ c.Check(found, check.Equals, shouldFind, check.Commentf("offending attr: %s", k))
+ }
+ }
+
+}
+
func (s *UserSuite) TestLoginClusterUserListBypassFederation(c *check.C) {
s.cluster.ClusterID = "local"
s.cluster.Login.LoginCluster = "zzzzz"
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 76b722694..518fe5693 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -268,14 +268,12 @@ class ApiClientAuthorization < ArvadosModel
end
act_as_system_user do
- user.save!
-
if (user.is_active && !remote_user['is_active']) or (user.is_invited && !remote_user['is_invited'])
- # If the user is newly created and AutoSetupNewUsers is
- # true, they will auto-setup in an after_create hook.
- # Synchronize the user's "active/invited" state state after the record
- # has been saved.
+ # Synchronize the user's "active/invited" state state. This
+ # also saves the record.
user.unsetup
+ else
+ user.save!
end
# We will accept this token (and avoid reloading the user
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 778ad7d0b..8ec90f7e5 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -38,7 +38,8 @@ class User < ArvadosModel
after_create :auto_setup_new_user, :if => Proc.new {
Rails.configuration.Users.AutoSetupNewUsers and
(uuid != system_user_uuid) and
- (uuid != anonymous_user_uuid)
+ (uuid != anonymous_user_uuid) and
+ (uuid[0..4] == Rails.configuration.ClusterID)
}
after_create :send_admin_notifications
diff --git a/services/api/test/integration/remote_user_test.rb b/services/api/test/integration/remote_user_test.rb
index 71f90d493..9ad93c2ee 100644
--- a/services/api/test/integration/remote_user_test.rb
+++ b/services/api/test/integration/remote_user_test.rb
@@ -348,6 +348,68 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
assert_equal 'barney', json_response['username']
end
+ test 'get active user from Login cluster when AutoSetupNewUsers is set' do
+ Rails.configuration.Login.LoginCluster = 'zbbbb'
+ Rails.configuration.Users.AutoSetupNewUsers = true
+ @stub_content = {
+ uuid: 'zbbbb-tpzed-000000000000001',
+ email: 'foo at example.com',
+ username: 'barney',
+ is_admin: false,
+ is_active: true,
+ is_invited: true,
+ }
+ get '/arvados/v1/users/current',
+ params: {format: 'json'},
+ headers: auth(remote: 'zbbbb')
+ assert_response :success
+ assert_equal 'zbbbb-tpzed-000000000000001', json_response['uuid']
+ assert_equal false, json_response['is_admin']
+ assert_equal true, json_response['is_active']
+ assert_equal true, json_response['is_invited']
+ assert_equal 'foo at example.com', json_response['email']
+ assert_equal 'barney', json_response['username']
+
+ @stub_content = {
+ uuid: 'zbbbb-tpzed-000000000000001',
+ email: 'foo at example.com',
+ username: 'barney',
+ is_admin: false,
+ is_active: false,
+ is_invited: false,
+ }
+
+ # Use cached value. User will still be active because we haven't
+ # re-queried the upstream cluster.
+ get '/arvados/v1/users/current',
+ params: {format: 'json'},
+ headers: auth(remote: 'zbbbb')
+ assert_response :success
+ assert_equal 'zbbbb-tpzed-000000000000001', json_response['uuid']
+ assert_equal false, json_response['is_admin']
+ assert_equal true, json_response['is_active']
+ assert_equal true, json_response['is_invited']
+ assert_equal 'foo at example.com', json_response['email']
+ assert_equal 'barney', json_response['username']
+
+ # Delete cached value. User should be inactive now.
+ act_as_system_user do
+ ApiClientAuthorization.delete_all
+ end
+
+ get '/arvados/v1/users/current',
+ params: {format: 'json'},
+ headers: auth(remote: 'zbbbb')
+ assert_response :success
+ assert_equal 'zbbbb-tpzed-000000000000001', json_response['uuid']
+ assert_equal false, json_response['is_admin']
+ assert_equal false, json_response['is_active']
+ assert_equal false, json_response['is_invited']
+ assert_equal 'foo at example.com', json_response['email']
+ assert_equal 'barney', json_response['username']
+
+ end
+
test 'pre-activate remote user' do
@stub_content = {
uuid: 'zbbbb-tpzed-000000000001234',
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list