[ARVADOS] updated: 2.1.0-2202-g6456207aa

Git user git at public.arvados.org
Mon Apr 4 19:48:30 UTC 2022


Summary of changes:
 apps/workbench/config/arvados_config.rb            |  4 ++
 doc/admin/upgrading.html.textile.liquid            |  2 +-
 lib/config/load.go                                 |  4 +-
 lib/controller/federation/conn.go                  | 19 ++++----
 lib/controller/integration_test.go                 | 50 ++++++++++++++++++++++
 .../api/app/models/api_client_authorization.rb     | 13 ++++--
 6 files changed, 78 insertions(+), 14 deletions(-)

       via  6456207aa5606c51642ebc2dd0b50b68eb7defc8 (commit)
       via  5d5eef02dce7a30dc03af97d59a85f668340d410 (commit)
       via  3b5716f4b309223e809a870a9d21f44ad3e31100 (commit)
       via  65b56acd2e854aa7313f4d976803c6f65cab5a51 (commit)
       via  3ef19722e686169ddd97de9b8a39595fecb64a66 (commit)
       via  0fc359fb99b0c3fe661e091d103edaa76eefa633 (commit)
       via  b328d1d5f11a0cdc9f6d6e12984af9fe30608358 (commit)
      from  ebc4600a2cced48b2b9580da4e6b0c8c6a23b78a (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 6456207aa5606c51642ebc2dd0b50b68eb7defc8
Author: Ward Vandewege <ward at curii.com>
Date:   Mon Apr 4 15:01:18 2022 -0400

    18887: Fix salted_secret check. Add test.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 442c9a6df..b71c4afb5 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -418,6 +418,15 @@ func (s *IntegrationSuite) TestForwardAnonymousTokenToLoginCluster(c *check.C) {
 	)
 	// The local z3333 anonymous token must be allowed to be forwarded to the login cluster
 	c.Check(err, check.IsNil)
+
+	userac1.AuthToken = "v2/z1111-gj3su-asdfasdfasdfasd/this-token-does-not-validate-so-anonymous-token-will-be-used-instead"
+	err = userac1.RequestAndDecode(&userList, "GET", "/arvados/v1/users", nil,
+		map[string]interface{}{
+			"reader_tokens": []string{anon3Auth.TokenV2()},
+			"where":         where,
+		},
+	)
+	c.Check(err, check.IsNil)
 }
 
 // Get a token from the login cluster (z1111), use it to submit a
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 726061a4a..52922d32b 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -131,7 +131,7 @@ class ApiClientAuthorization < ArvadosModel
     end
 
     # Usually, the secret is salted
-    salted_secret = OpenSSL::HMAC.hexdigest('sha1', secret, remote)
+    salted_secret = OpenSSL::HMAC.hexdigest('sha1', Rails.configuration.Users.AnonymousUserToken, remote)
 
     # The anonymous token could be specified as a full v2 token in the config,
     # but the config loader strips it down to the secret part.

commit 5d5eef02dce7a30dc03af97d59a85f668340d410
Author: Ward Vandewege <ward at curii.com>
Date:   Mon Apr 4 11:00:25 2022 -0400

    18887: address review comments.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 2eabf4464..1b8ec9e64 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -75,9 +75,11 @@ func saltedTokenProvider(cluster *arvados.Cluster, local backend, remoteID strin
 				// If we did this, the login cluster would call back to us and then
 				// reject our response because the user UUID prefix (i.e., the
 				// LoginCluster prefix) won't match the token UUID prefix (i.e., our
-				// prefix). The anonymous token is OK to forward, because it gets
-				// mapped to the local anonymous token automatically on the login
-				// cluster.
+				// prefix). The anonymous token is OK to forward, because (unlike other
+				// local tokens for real users) the validation callback will return the
+				// locally issued anonymous user ID instead of a login-cluster user ID.
+				// That anonymous user ID gets mapped to the local anonymous user
+				// automatically on the login cluster.
 				return nil, httpErrorf(http.StatusUnauthorized, "cannot use a locally issued token to forward a request to our login cluster (%s)", remoteID)
 			}
 			salted, err := auth.SaltToken(token, remoteID)
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 3ef4d0e33..726061a4a 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -130,23 +130,13 @@ class ApiClientAuthorization < ArvadosModel
       secret = token
     end
 
-    # the anonymous token could be specified as a full v2 token in the config
-    case Rails.configuration.Users.AnonymousUserToken[0..2]
-    when 'v2/'
-      _, anon_token_uuid, anon_secret, anon_optional = Rails.configuration.Users.AnonymousUserToken.split('/')
-      unless anon_token_uuid.andand.length == 27 && anon_secret.andand.length.andand > 0
-        # invalid v2 token
-        return nil
-      end
-    else
-      # v1 token
-      anon_secret = Rails.configuration.Users.AnonymousUserToken
-    end
-
-    salted_secret = OpenSSL::HMAC.hexdigest('sha1', anon_secret, remote)
+    # Usually, the secret is salted
+    salted_secret = OpenSSL::HMAC.hexdigest('sha1', secret, remote)
 
+    # The anonymous token could be specified as a full v2 token in the config,
+    # but the config loader strips it down to the secret part.
     # The anonymous token content and minimum length is verified in lib/config
-    if secret.length >= 0 && (secret == anon_secret || secret == salted_secret)
+    if secret.length >= 0 && (secret == Rails.configuration.Users.AnonymousUserToken || secret == salted_secret)
       return ApiClientAuthorization.new(user: User.find_by_uuid(anonymous_user_uuid),
                                         uuid: Rails.configuration.ClusterID+"-gj3su-anonymouspublic",
                                         api_token: secret,

commit 3b5716f4b309223e809a870a9d21f44ad3e31100
Author: Ward Vandewege <ward at curii.com>
Date:   Fri Apr 1 14:09:36 2022 -0400

    18887: self.check_anonymous_user_token can now handle a full V2 token in
           the config file. It can also verify a salted anonymous token.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 993a49e5b..3ef4d0e33 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -116,7 +116,7 @@ class ApiClientAuthorization < ArvadosModel
     clnt
   end
 
-  def self.check_anonymous_user_token token
+  def self.check_anonymous_user_token(token:, remote:)
     case token[0..2]
     when 'v2/'
       _, token_uuid, secret, optional = token.split('/')
@@ -130,11 +130,26 @@ class ApiClientAuthorization < ArvadosModel
       secret = token
     end
 
+    # the anonymous token could be specified as a full v2 token in the config
+    case Rails.configuration.Users.AnonymousUserToken[0..2]
+    when 'v2/'
+      _, anon_token_uuid, anon_secret, anon_optional = Rails.configuration.Users.AnonymousUserToken.split('/')
+      unless anon_token_uuid.andand.length == 27 && anon_secret.andand.length.andand > 0
+        # invalid v2 token
+        return nil
+      end
+    else
+      # v1 token
+      anon_secret = Rails.configuration.Users.AnonymousUserToken
+    end
+
+    salted_secret = OpenSSL::HMAC.hexdigest('sha1', anon_secret, remote)
+
     # The anonymous token content and minimum length is verified in lib/config
-    if secret.length >= 0 && secret == Rails.configuration.Users.AnonymousUserToken
+    if secret.length >= 0 && (secret == anon_secret || secret == salted_secret)
       return ApiClientAuthorization.new(user: User.find_by_uuid(anonymous_user_uuid),
                                         uuid: Rails.configuration.ClusterID+"-gj3su-anonymouspublic",
-                                        api_token: token,
+                                        api_token: secret,
                                         api_client: anonymous_user_token_api_client,
                                         scopes: ['GET /'])
     else
@@ -157,7 +172,7 @@ class ApiClientAuthorization < ArvadosModel
     return nil if token.nil? or token.empty?
     remote ||= Rails.configuration.ClusterID
 
-    auth = self.check_anonymous_user_token(token)
+    auth = self.check_anonymous_user_token(token: token, remote: remote)
     if !auth.nil?
       return auth
     end

commit 65b56acd2e854aa7313f4d976803c6f65cab5a51
Author: Ward Vandewege <ward at curii.com>
Date:   Thu Mar 31 15:50:07 2022 -0400

    18887: add an integration test.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 9f5d12598..442c9a6df 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -379,6 +379,47 @@ func (s *IntegrationSuite) TestGetCollectionAsAnonymous(c *check.C) {
 	c.Check(coll.PortableDataHash, check.Equals, pdh)
 }
 
+// z3333 should forward the locally-issued anonymous user token to its login
+// cluster z1111. That is no problem because the login cluster controller will
+// map any anonymous user token to its local anonymous user.
+//
+// This needs to work because wb1 has a tendency to slap the local anonymous
+// user token on every request as a reader_token, which gets folded into the
+// request token list controller.
+//
+// Use a z1111 user token and the anonymous token from z3333 passed in as a
+// reader_token to do a request on z3333, asking for the z1111 anonymous user
+// object. The request will be forwarded to the z1111 cluster. The presence of
+// the z3333 anonymous user token should not prohibit the request from being
+// forwarded.
+func (s *IntegrationSuite) TestForwardAnonymousTokenToLoginCluster(c *check.C) {
+	conn1 := s.testClusters["z1111"].Conn()
+	s.testClusters["z3333"].Conn()
+
+	rootctx1, _, _ := s.testClusters["z1111"].RootClients()
+	_, anonac3, _ := s.testClusters["z3333"].AnonymousClients()
+
+	// Make a user connection to z3333 (using a z1111 user, because that's the login cluster)
+	_, userac1, _, _ := s.testClusters["z3333"].UserClients(rootctx1, c, conn1, "user at example.com", true)
+
+	// Get the anonymous user token for z3333
+	var anon3Auth arvados.APIClientAuthorization
+	err := anonac3.RequestAndDecode(&anon3Auth, "GET", "/arvados/v1/api_client_authorizations/current", nil, nil)
+	c.Check(err, check.IsNil)
+
+	var userList arvados.UserList
+	where := make(map[string]string)
+	where["uuid"] = "z1111-tpzed-anonymouspublic"
+	err = userac1.RequestAndDecode(&userList, "GET", "/arvados/v1/users", nil,
+		map[string]interface{}{
+			"reader_tokens": []string{anon3Auth.TokenV2()},
+			"where":         where,
+		},
+	)
+	// The local z3333 anonymous token must be allowed to be forwarded to the login cluster
+	c.Check(err, check.IsNil)
+}
+
 // Get a token from the login cluster (z1111), use it to submit a
 // container request on z2222.
 func (s *IntegrationSuite) TestCreateContainerRequestWithFedToken(c *check.C) {

commit 3ef19722e686169ddd97de9b8a39595fecb64a66
Author: Ward Vandewege <ward at curii.com>
Date:   Mon Mar 28 16:18:16 2022 -0400

    18887: undo the warning introduced in 18676 if a v2 anonymous token is
           supplied in the config file, using a v2 anonymous token is now
           fine. Also add a config loader check for the minimum secret
           length if a v2 token is acceptable.
    
    refs #18887, #18676
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/doc/admin/upgrading.html.textile.liquid b/doc/admin/upgrading.html.textile.liquid
index 1ed3b694c..97f6ce2f8 100644
--- a/doc/admin/upgrading.html.textile.liquid
+++ b/doc/admin/upgrading.html.textile.liquid
@@ -46,7 +46,7 @@ The minimum supported Ruby version is now 2.6.  If you are running Arvados on De
 
 h3. Anonymous token changes
 
-The anonymous token configured in @Users.AnonymousUserToken@ must now be 32 characters or longer. This was already the suggestion in the documentation, now it is enforced. The @script/get_anonymous_user_token.rb@ script that was needed to register the anonymous user token in the database has been removed. Registration of the anonymous token is no longer necessary. If the anonymous token in @config.yml@ is specified as a full V2 token, that will now generate a warning - it should be updated to list just the secret (i.e. the part after the last forward slash).
+The anonymous token configured in @Users.AnonymousUserToken@ must now be 32 characters or longer. This was already the suggestion in the documentation, now it is enforced. The @script/get_anonymous_user_token.rb@ script that was needed to register the anonymous user token in the database has been removed. Registration of the anonymous token is no longer necessary.
 
 h3. Preemptible instance support changes
 
diff --git a/lib/config/load.go b/lib/config/load.go
index de43b9d2e..5afb51c5a 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -369,10 +369,12 @@ func (ldr *Loader) checkToken(label, token string, mandatory bool, acceptV2 bool
 		if !strings.HasPrefix(token, "v2/") {
 			return fmt.Errorf("%s: unacceptable characters in token (only a-z, A-Z, 0-9 are acceptable)", label)
 		}
-		ldr.Logger.Warnf("%s: token is a full V2 token, should just be a secret (remove everything up to and including the last forward slash)", label)
 		if !acceptableTokenRe.MatchString(tmp[2]) {
 			return fmt.Errorf("%s: unacceptable characters in V2 token secret (only a-z, A-Z, 0-9 are acceptable)", label)
 		}
+		if len(tmp[2]) < acceptableTokenLength {
+			ldr.Logger.Warnf("%s: secret is too short (should be at least %d characters)", label, acceptableTokenLength)
+		}
 	} else if len(token) < acceptableTokenLength {
 		if ldr.Logger != nil {
 			ldr.Logger.Warnf("%s: token is too short (should be at least %d characters)", label, acceptableTokenLength)

commit 0fc359fb99b0c3fe661e091d103edaa76eefa633
Author: Ward Vandewege <ward at curii.com>
Date:   Fri Mar 25 09:11:53 2022 -0400

    18887: it is OK for controller to forward the local anymous token,
           because all anonymous tokens get mapped to the local anonymous
           token on every cluster.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index d3819f626..2eabf4464 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -69,14 +69,15 @@ func saltedTokenProvider(cluster *arvados.Cluster, local backend, remoteID strin
 			return nil, errors.New("no token provided")
 		}
 		for _, token := range incoming.Tokens {
-			if strings.HasPrefix(token, "v2/"+cluster.ClusterID+"-") && remoteID == cluster.Login.LoginCluster {
-				// If we did this, the login cluster
-				// would call back to us and then
-				// reject our response because the
-				// user UUID prefix (i.e., the
-				// LoginCluster prefix) won't match
-				// the token UUID prefix (i.e., our
-				// prefix).
+			if strings.HasPrefix(token, "v2/"+cluster.ClusterID+"-") &&
+				!strings.HasPrefix(token, "v2/"+cluster.ClusterID+"-gj3su-anonymouspublic/") &&
+				remoteID == cluster.Login.LoginCluster {
+				// If we did this, the login cluster would call back to us and then
+				// reject our response because the user UUID prefix (i.e., the
+				// LoginCluster prefix) won't match the token UUID prefix (i.e., our
+				// prefix). The anonymous token is OK to forward, because it gets
+				// mapped to the local anonymous token automatically on the login
+				// cluster.
 				return nil, httpErrorf(http.StatusUnauthorized, "cannot use a locally issued token to forward a request to our login cluster (%s)", remoteID)
 			}
 			salted, err := auth.SaltToken(token, remoteID)

commit b328d1d5f11a0cdc9f6d6e12984af9fe30608358
Author: Ward Vandewege <ward at curii.com>
Date:   Fri Mar 25 08:19:10 2022 -0400

    18887: when wb1 sends the anonymous user token, it makes sure to always
           send a v2 token.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/apps/workbench/config/arvados_config.rb b/apps/workbench/config/arvados_config.rb
index c5cc544b9..7cc46d298 100644
--- a/apps/workbench/config/arvados_config.rb
+++ b/apps/workbench/config/arvados_config.rb
@@ -199,4 +199,8 @@ ArvadosWorkbench::Application.configure do
     ConfigValidators.validate_wb2_url_config()
     ConfigValidators.validate_download_config()
   end
+  if Rails.configuration.Users.AnonymousUserToken and
+     !Rails.configuration.Users.AnonymousUserToken.starts_with?("v2/")
+    Rails.configuration.Users.AnonymousUserToken = "v2/#{clusterID}-gj3su-anonymouspublic/#{Rails.configuration.Users.AnonymousUserToken}"
+  end
 end

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list