16914: Make sure remote uninvited user is not invited here
authorPeter Amstutz <peter.amstutz@curii.com>
Tue, 29 Sep 2020 14:55:16 +0000 (10:55 -0400)
committerPeter Amstutz <peter.amstutz@curii.com>
Tue, 29 Sep 2020 14:55:16 +0000 (10:55 -0400)
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>

lib/controller/federation/conn.go
services/api/app/models/api_client_authorization.rb
services/api/test/integration/remote_user_test.rb

index 60cdfbfd42360bc8f92ac291b6bc554d4765047e..43fe1cc0140b34d24444dbb883eeb29da6d4055d 100644 (file)
@@ -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) {
index ab6fd8000c1f4a966be7e01450402719f7182d5a..76b722694aa763baeb79a2ce621c620383c139a2 100644 (file)
@@ -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:
index 8ad09894a16df4bc6281ba95db6c8b6b25be589c..71f90d493846d637f2d296ef9583befcf1e3ba22 100644 (file)
@@ -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',