From: Ward Vandewege Date: Mon, 4 Apr 2022 19:33:28 +0000 (-0400) Subject: 18887: Merge branch 'main' into 18887-wb1-sends-v2-anonymous-token X-Git-Tag: 2.5.0~228^2 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/a017e1f5ac7ad49a29e302275d0f0f83754941ea?hp=eb8307817db46d208c5c3517dbeafbc5b02a6f5f 18887: Merge branch 'main' into 18887-wb1-sends-v2-anonymous-token Arvados-DCO-1.1-Signed-off-by: Ward Vandewege --- diff --git a/apps/workbench/config/arvados_config.rb b/apps/workbench/config/arvados_config.rb index c5cc544b9b..7cc46d2983 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 diff --git a/doc/admin/upgrading.html.textile.liquid b/doc/admin/upgrading.html.textile.liquid index 1ed3b694ce..97f6ce2f89 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 de43b9d2e2..5afb51c5ad 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) diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go index d3819f6262..1b8ec9e64a 100644 --- a/lib/controller/federation/conn.go +++ b/lib/controller/federation/conn.go @@ -69,14 +69,17 @@ 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 (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/lib/controller/integration_test.go b/lib/controller/integration_test.go index 9f5d12598b..b71c4afb55 100644 --- a/lib/controller/integration_test.go +++ b/lib/controller/integration_test.go @@ -379,6 +379,56 @@ 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@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) + + 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 // container request on z2222. func (s *IntegrationSuite) TestCreateContainerRequestWithFedToken(c *check.C) { diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb index 993a49e5b7..52922d32b1 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,16 @@ class ApiClientAuthorization < ArvadosModel secret = token end + # Usually, the secret is salted + 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. # 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 == 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: token, + api_token: secret, api_client: anonymous_user_token_api_client, scopes: ['GET /']) else @@ -157,7 +162,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