From: Tom Clegg <tom@tomclegg.ca>
Date: Wed, 23 Sep 2020 21:37:34 +0000 (-0400)
Subject: 16669: Fix pass-through of cached remote token.
X-Git-Tag: 2.2.0~253^2~1
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/27ab60e21b3cf9c908716ae74e63aba8e4cb6349

16669: Fix pass-through of cached remote token.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>
---

diff --git a/lib/controller/federation.go b/lib/controller/federation.go
index aceaba8087..cab5e4c4ca 100644
--- a/lib/controller/federation.go
+++ b/lib/controller/federation.go
@@ -263,17 +263,20 @@ func (h *Handler) saltAuthToken(req *http.Request, remote string) (updatedReq *h
 		return updatedReq, nil
 	}
 
+	ctxlog.FromContext(req.Context()).Infof("saltAuthToken: cluster %s token %s remote %s", h.Cluster.ClusterID, creds.Tokens[0], remote)
 	token, err := auth.SaltToken(creds.Tokens[0], remote)
 
 	if err == auth.ErrObsoleteToken {
-		// If the token exists in our own database, salt it
-		// for the remote. Otherwise, assume it was issued by
-		// the remote, and pass it through unmodified.
+		// If the token exists in our own database for our own
+		// user, salt it for the remote. Otherwise, assume it
+		// was issued by the remote, and pass it through
+		// unmodified.
 		currentUser, ok, err := h.validateAPItoken(req, creds.Tokens[0])
 		if err != nil {
 			return nil, err
-		} else if !ok {
-			// Not ours; pass through unmodified.
+		} else if !ok || strings.HasPrefix(currentUser.UUID, remote) {
+			// Unknown, or cached + belongs to remote;
+			// pass through unmodified.
 			token = creds.Tokens[0]
 		} else {
 			// Found; make V2 version and salt it.
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index f07c3b6316..986faa7b05 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -79,6 +79,14 @@ func saltedTokenProvider(local backend, remoteID string) rpc.TokenProvider {
 				} else if err != nil {
 					return nil, err
 				}
+				if strings.HasPrefix(aca.UUID, remoteID) {
+					// We have it cached here, but
+					// the token belongs to the
+					// remote target itself, so
+					// pass it through unmodified.
+					tokens = append(tokens, token)
+					continue
+				}
 				salted, err := auth.SaltToken(aca.TokenV2(), remoteID)
 				if err != nil {
 					return nil, err
diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go
index 8909e0a72e..2da7ca5ccf 100644
--- a/lib/controller/localdb/login_oidc.go
+++ b/lib/controller/localdb/login_oidc.go
@@ -380,7 +380,7 @@ func (ta *oidcTokenAuthorizer) WrapCalls(origFunc api.RoutableFunc) api.Routable
 // if so, ensures that an api_client_authorizations row exists so that
 // RailsAPI will accept it as an Arvados token.
 func (ta *oidcTokenAuthorizer) registerToken(ctx context.Context, tok string) error {
-	if strings.HasPrefix(tok, "v2/") {
+	if tok == ta.ctrl.Cluster.SystemRootToken || strings.HasPrefix(tok, "v2/") {
 		return nil
 	}
 	if cached, hit := ta.cache.Get(tok); !hit {
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 6a34ed9552..74a4c1efa5 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -206,7 +206,7 @@ class ApiClientAuthorization < ArvadosModel
         # below. If so, we'll stuff the database with hmac instead of
         # the real OIDC token.
         upstream_cluster_id = Rails.configuration.Login.LoginCluster
-        token_uuid = generate_uuid
+        token_uuid = upstream_cluster_id + generate_uuid[5..27]
         secret = hmac
       else
         return nil