[ARVADOS] created: 1.3.0-3239-gb096358df

Git user git at public.arvados.org
Tue Sep 29 14:55:45 UTC 2020


        at  b096358dfbd438e89d77b6d4899817b82aca3ca3 (commit)


commit b096358dfbd438e89d77b6d4899817b82aca3ca3
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Tue Sep 29 10:55:16 2020 -0400

    16914: Make sure remote uninvited user is not invited here
    
    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 60cdfbfd4..43fe1cc01 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 ab6fd8000..76b722694 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 8ad09894a..71f90d493 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 at 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 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',
@@ -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',

commit a1305701c34508c80f638e2c7666b255019ba9a4
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Mon Sep 28 17:00:33 2020 -0400

    16914: "Get current user" is forwarded to LoginCluster
    
    Results from "get user" and "current user" are now cached, same as
    "user list".
    
    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 61cac9bba..60cdfbfd4 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -500,11 +500,49 @@ func (conn *Conn) UserUnsetup(ctx context.Context, options arvados.GetOptions) (
 }
 
 func (conn *Conn) UserGet(ctx context.Context, options arvados.GetOptions) (arvados.User, error) {
-	return conn.chooseBackend(options.UUID).UserGet(ctx, options)
+	resp, err := conn.chooseBackend(options.UUID).UserGet(ctx, options)
+	if err != nil {
+		return resp, err
+	}
+	if options.UUID != resp.UUID {
+		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})
+		if err != nil {
+			return arvados.User{}, err
+		}
+	}
+	return resp, nil
 }
 
 func (conn *Conn) UserGetCurrent(ctx context.Context, options arvados.GetOptions) (arvados.User, error) {
-	return conn.chooseBackend(options.UUID).UserGetCurrent(ctx, options)
+	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
 }
 
 func (conn *Conn) UserGetSystem(ctx context.Context, options arvados.GetOptions) (arvados.User, error) {

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list