Merge branch '16343-create-cr-with-logincluster-token'

fixes #16343

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

diff --git a/lib/controller/fed_containers.go b/lib/controller/fed_containers.go
index a923f757f2eb61afc29d27ee18bfbd42a27a6c1c..c62cea1168eb29c212ad5eefdd7a9d58dc609f8c 100644 (file)
--- a/lib/controller/fed_containers.go
+++ b/lib/controller/fed_containers.go
@@ -42,13 +42,11 @@ func remoteContainerRequestCreate(
                return true
        }
 
-       if *clusterId == "" {
-               *clusterId = h.handler.Cluster.ClusterID
-       }
-
-       if strings.HasPrefix(currentUser.Authorization.UUID, h.handler.Cluster.ClusterID) &&
-               *clusterId == h.handler.Cluster.ClusterID {
-               // local user submitting container request to local cluster
+       if *clusterId == "" || *clusterId == h.handler.Cluster.ClusterID {
+               // Submitting container request to local cluster. No
+               // need to set a runtime_token (rails api will create
+               // one when the container runs) or do a remote cluster
+               // request.
                return false
        }
 

diff --git a/lib/controller/federation.go b/lib/controller/federation.go
index 674183dcc1d8b9f2d96e5f9b5bded909dad23f26..c0d127284ce35a21d5618814dd2792984809f5f2 100644 (file)
--- a/lib/controller/federation.go
+++ b/lib/controller/federation.go
@@ -19,6 +19,7 @@ import (
 
        "git.arvados.org/arvados.git/sdk/go/arvados"
        "git.arvados.org/arvados.git/sdk/go/auth"
+       "git.arvados.org/arvados.git/sdk/go/ctxlog"
        "github.com/jmcvetta/randutil"
 )
 
@@ -153,6 +154,7 @@ func (h *Handler) validateAPItoken(req *http.Request, token string) (*CurrentUse
        user := CurrentUser{Authorization: arvados.APIClientAuthorization{APIToken: token}}
        db, err := h.db(req)
        if err != nil {
+               ctxlog.FromContext(req.Context()).WithError(err).Debugf("validateAPItoken(%s): database error", token)
                return nil, false, err
        }
 
@@ -166,18 +168,23 @@ func (h *Handler) validateAPItoken(req *http.Request, token string) (*CurrentUse
        var scopes string
        err = db.QueryRowContext(req.Context(), `SELECT api_client_authorizations.uuid, api_client_authorizations.scopes, users.uuid FROM api_client_authorizations JOIN users on api_client_authorizations.user_id=users.id WHERE api_token=$1 AND (expires_at IS NULL OR expires_at > current_timestamp) LIMIT 1`, token).Scan(&user.Authorization.UUID, &scopes, &user.UUID)
        if err == sql.ErrNoRows {
+               ctxlog.FromContext(req.Context()).Debugf("validateAPItoken(%s): not found in database", token)
                return nil, false, nil
        } else if err != nil {
+               ctxlog.FromContext(req.Context()).WithError(err).Debugf("validateAPItoken(%s): database error", token)
                return nil, false, err
        }
        if uuid != "" && user.Authorization.UUID != uuid {
                // secret part matches, but UUID doesn't -- somewhat surprising
+               ctxlog.FromContext(req.Context()).Debugf("validateAPItoken(%s): secret part found, but with different UUID: %s", token, user.Authorization.UUID)
                return nil, false, nil
        }
        err = json.Unmarshal([]byte(scopes), &user.Authorization.Scopes)
        if err != nil {
+               ctxlog.FromContext(req.Context()).WithError(err).Debugf("validateAPItoken(%s): error parsing scopes from db", token)
                return nil, false, err
        }
+       ctxlog.FromContext(req.Context()).Debugf("validateAPItoken(%s): ok", token)
        return &user, true, nil
 }
 

diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index aad2c4775ad20c0884cca173326e64fa97f98bc7..4939b116b0bce22dfe9c3ad1c2bf79a89797b655 100644 (file)
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -7,9 +7,11 @@ package controller
 import (
        "bytes"
        "context"
+       "encoding/json"
        "io"
        "math"
        "net"
+       "net/http"
        "net/url"
        "os"
        "path/filepath"
@@ -84,19 +86,26 @@ func (s *IntegrationSuite) SetUpSuite(c *check.C) {
         Insecure: true
         Proxy: true
         ActivateUsers: true
-      z2222:
+`
+               if id != "z2222" {
+                       yaml += `      z2222:
         Host: ` + hostport["z2222"] + `
         Scheme: https
         Insecure: true
         Proxy: true
         ActivateUsers: true
-      z3333:
+`
+               }
+               if id != "z3333" {
+                       yaml += `      z3333:
         Host: ` + hostport["z3333"] + `
         Scheme: https
         Insecure: true
         Proxy: true
         ActivateUsers: true
 `
+               }
+
                loader := config.NewLoader(bytes.NewBufferString(yaml), ctxlog.TestLogger(c))
                loader.Path = "-"
                loader.SkipLegacy = true
@@ -225,6 +234,76 @@ func (s *IntegrationSuite) TestGetCollectionByPDH(c *check.C) {
        c.Check(coll.PortableDataHash, check.Equals, pdh)
 }
 
+// Get a token from the login cluster (z1111), use it to submit a
+// container request on z2222.
+func (s *IntegrationSuite) TestCreateContainerRequestWithFedToken(c *check.C) {
+       conn1 := s.conn("z1111")
+       rootctx1, _, _ := s.rootClients("z1111")
+       _, ac1, _ := s.userClients(rootctx1, c, conn1, "z1111", true)
+
+       // Use ac2 to get the discovery doc with a blank token, so the
+       // SDK doesn't magically pass the z1111 token to z2222 before
+       // we're ready to start our test.
+       _, ac2, _ := s.clientsWithToken("z2222", "")
+       var dd map[string]interface{}
+       err := ac2.RequestAndDecode(&dd, "GET", "discovery/v1/apis/arvados/v1/rest", nil, nil)
+       c.Assert(err, check.IsNil)
+
+       var (
+               body bytes.Buffer
+               req  *http.Request
+               resp *http.Response
+               u    arvados.User
+               cr   arvados.ContainerRequest
+       )
+       json.NewEncoder(&body).Encode(map[string]interface{}{
+               "container_request": map[string]interface{}{
+                       "command":         []string{"echo"},
+                       "container_image": "d41d8cd98f00b204e9800998ecf8427e+0",
+                       "cwd":             "/",
+                       "output_path":     "/",
+               },
+       })
+       ac2.AuthToken = ac1.AuthToken
+
+       c.Log("...post CR with good (but not yet cached) token")
+       cr = arvados.ContainerRequest{}
+       req, err = http.NewRequest("POST", "https://"+ac2.APIHost+"/arvados/v1/container_requests", bytes.NewReader(body.Bytes()))
+       c.Assert(err, check.IsNil)
+       req.Header.Set("Content-Type", "application/json")
+       err = ac2.DoAndDecode(&cr, req)
+       c.Logf("err == %#v", err)
+
+       c.Log("...get user with good token")
+       u = arvados.User{}
+       req, err = http.NewRequest("GET", "https://"+ac2.APIHost+"/arvados/v1/users/current", nil)
+       c.Assert(err, check.IsNil)
+       err = ac2.DoAndDecode(&u, req)
+       c.Check(err, check.IsNil)
+       c.Check(u.UUID, check.Matches, "z1111-tpzed-.*")
+
+       c.Log("...post CR with good cached token")
+       cr = arvados.ContainerRequest{}
+       req, err = http.NewRequest("POST", "https://"+ac2.APIHost+"/arvados/v1/container_requests", bytes.NewReader(body.Bytes()))
+       c.Assert(err, check.IsNil)
+       req.Header.Set("Content-Type", "application/json")
+       err = ac2.DoAndDecode(&cr, req)
+       c.Check(err, check.IsNil)
+       c.Check(cr.UUID, check.Matches, "z2222-.*")
+
+       c.Log("...post with good cached token ('OAuth2 ...')")
+       cr = arvados.ContainerRequest{}
+       req, err = http.NewRequest("POST", "https://"+ac2.APIHost+"/arvados/v1/container_requests", bytes.NewReader(body.Bytes()))
+       c.Assert(err, check.IsNil)
+       req.Header.Set("Content-Type", "application/json")
+       req.Header.Set("Authorization", "OAuth2 "+ac2.AuthToken)
+       resp, err = arvados.InsecureHTTPClient.Do(req)
+       if c.Check(err, check.IsNil) {
+               err = json.NewDecoder(resp.Body).Decode(&cr)
+               c.Check(cr.UUID, check.Matches, "z2222-.*")
+       }
+}
+
 // Test for bug #16263
 func (s *IntegrationSuite) TestListUsers(c *check.C) {
        rootctx1, _, _ := s.rootClients("z1111")

diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 5386cb119a0c9cadbcc2cc0d8edfc5cadd8a1e76..6057c4d2698c8e1bb3d131d7dfcd9d0a8c85ea0d 100644 (file)
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -164,6 +164,9 @@ class ApiClientAuthorization < ArvadosModel
          (secret == auth.api_token ||
           secret == OpenSSL::HMAC.hexdigest('sha1', auth.api_token, remote))
         # found it
+        if token_uuid[0..4] != Rails.configuration.ClusterID
+          Rails.logger.debug "found cached remote token #{token_uuid} with secret #{secret} in local db"
+        end
         return auth
       end
 
@@ -274,6 +277,7 @@ class ApiClientAuthorization < ArvadosModel
                                 api_token: secret,
                                 api_client_id: 0,
                                 expires_at: Time.now + Rails.configuration.Login.RemoteTokenRefresh)
+        Rails.logger.debug "cached remote token #{token_uuid} with secret #{secret} in local db"
       end
       return auth
     else