From: Peter Amstutz <peter.amstutz@curii.com>
Date: Tue, 29 Sep 2020 14:55:16 +0000 (-0400)
Subject: 16914: Make sure remote uninvited user is not invited here
X-Git-Tag: 2.1.0~31^2
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/b096358dfbd438e89d77b6d4899817b82aca3ca3

16914: Make sure remote uninvited user is not invited here

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>
---

diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 60cdfbfd42..43fe1cc014 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -517,32 +517,7 @@ func (conn *Conn) UserGet(ctx context.Context, options arvados.GetOptions) (arva
 }
 
 func (conn *Conn) UserGetCurrent(ctx context.Context, options arvados.GetOptions) (arvados.User, error) {
-	c, ok := auth.FromContext(ctx)
-	if !ok || len(c.Tokens) == 0 {
-		return arvados.User{}, httpErrorf(http.StatusUnauthorized, "Must supply a token")
-	}
-
-	tok := c.Tokens[0]
-	if !strings.HasPrefix(tok, "v2/") || len(tok) < 30 {
-		return conn.localOrLoginCluster().UserGetCurrent(ctx, options)
-	}
-
-	// Contact the cluster that issued the token to find out what
-	// user it belongs to.
-	remote := tok[3:8]
-	resp, err := conn.chooseBackend(remote).UserGetCurrent(ctx, options)
-	if err != nil {
-		return resp, err
-	}
-
-	// If it is a remote cluster that owns the user, update the local user record.
-	if remote != conn.cluster.ClusterID && remote == resp.UUID[:5] {
-		err = conn.batchUpdateUsers(ctx, arvados.ListOptions{}, []arvados.User{resp})
-		if err != nil {
-			return arvados.User{}, err
-		}
-	}
-	return resp, nil
+	return conn.local.UserGetCurrent(ctx, options)
 }
 
 func (conn *Conn) UserGetSystem(ctx context.Context, options arvados.GetOptions) (arvados.User, error) {
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index ab6fd8000c..76b722694a 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -268,12 +268,16 @@ class ApiClientAuthorization < ArvadosModel
       end
 
       act_as_system_user do
-        if user.is_active && !remote_user['is_active']
+        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.
           user.unsetup
         end
 
-        user.save!
-
         # We will accept this token (and avoid reloading the user
         # record) for 'RemoteTokenRefresh' (default 5 minutes).
         # Possible todo:
diff --git a/services/api/test/integration/remote_user_test.rb b/services/api/test/integration/remote_user_test.rb
index 8ad09894a1..71f90d4938 100644
--- a/services/api/test/integration/remote_user_test.rb
+++ b/services/api/test/integration/remote_user_test.rb
@@ -84,6 +84,7 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
       username: 'barney',
       is_admin: true,
       is_active: true,
+      is_invited: true,
     }
   end
 
@@ -153,6 +154,7 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
 
     # revoke original token
     @stub_content[:is_active] = false
+    @stub_content[:is_invited] = false
 
     # simulate cache expiry
     ApiClientAuthorization.where(
@@ -323,6 +325,29 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
     assert_equal 'barney', json_response['username']
   end
 
+  test 'get inactive 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@example.com',
+      username: 'barney',
+      is_admin: false,
+      is_active: false,
+      is_invited: false,
+    }
+    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@example.com', json_response['email']
+    assert_equal 'barney', json_response['username']
+  end
+
   test 'pre-activate remote user' do
     @stub_content = {
       uuid: 'zbbbb-tpzed-000000000001234',
@@ -330,6 +355,7 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
       username: 'barney',
       is_admin: true,
       is_active: true,
+      is_invited: true,
     }
 
     post '/arvados/v1/users',
@@ -364,6 +390,7 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
       username: 'barney',
       is_admin: true,
       is_active: true,
+      is_invited: true,
     }
 
     get '/arvados/v1/users/current',