Merge branch '18184-singularity-374'
[arvados.git] / lib / controller / federation_test.go
index 1c859cfc515d142a0289610e402e725e07bfebb1..211c7619809ed6a8855248915facef843da55081 100644 (file)
@@ -6,10 +6,12 @@ package controller
 
 import (
        "bytes"
+       "context"
        "encoding/json"
        "fmt"
        "io"
        "io/ioutil"
+       "net"
        "net/http"
        "net/http/httptest"
        "net/url"
@@ -17,11 +19,11 @@ import (
        "strings"
        "time"
 
-       "git.curoverse.com/arvados.git/sdk/go/arvados"
-       "git.curoverse.com/arvados.git/sdk/go/arvadostest"
-       "git.curoverse.com/arvados.git/sdk/go/ctxlog"
-       "git.curoverse.com/arvados.git/sdk/go/httpserver"
-       "git.curoverse.com/arvados.git/sdk/go/keepclient"
+       "git.arvados.org/arvados.git/sdk/go/arvados"
+       "git.arvados.org/arvados.git/sdk/go/arvadostest"
+       "git.arvados.org/arvados.git/sdk/go/ctxlog"
+       "git.arvados.org/arvados.git/sdk/go/httpserver"
+       "git.arvados.org/arvados.git/sdk/go/keepclient"
        "github.com/sirupsen/logrus"
        check "gopkg.in/check.v1"
 )
@@ -39,7 +41,8 @@ type FederationSuite struct {
        // provided by the integration test environment.
        remoteServer *httpserver.Server
        // remoteMock ("zmock") appends each incoming request to
-       // remoteMockRequests, and returns an empty 200 response.
+       // remoteMockRequests, and returns 200 with an empty JSON
+       // object.
        remoteMock         *httpserver.Server
        remoteMockRequests []http.Request
 }
@@ -57,17 +60,22 @@ func (s *FederationSuite) SetUpTest(c *check.C) {
        cluster := &arvados.Cluster{
                ClusterID:  "zhome",
                PostgreSQL: integrationTestCluster().PostgreSQL,
-               TLS:        arvados.TLS{Insecure: true},
-               API: arvados.API{
-                       MaxItemsPerResponse:     1000,
-                       MaxRequestAmplification: 4,
-               },
        }
+       cluster.TLS.Insecure = true
+       cluster.API.MaxItemsPerResponse = 1000
+       cluster.API.MaxRequestAmplification = 4
+       cluster.API.RequestTimeout = arvados.Duration(5 * time.Minute)
+       cluster.Collections.BlobSigning = true
+       cluster.Collections.BlobSigningKey = arvadostest.BlobSigningKey
+       cluster.Collections.BlobSigningTTL = arvados.Duration(time.Hour * 24 * 14)
        arvadostest.SetServiceURL(&cluster.Services.RailsAPI, "http://localhost:1/")
        arvadostest.SetServiceURL(&cluster.Services.Controller, "http://localhost:/")
        s.testHandler = &Handler{Cluster: cluster}
        s.testServer = newServerFromIntegrationTestEnv(c)
-       s.testServer.Server.Handler = httpserver.AddRequestIDs(httpserver.LogRequests(s.log, s.testHandler))
+       s.testServer.Server.BaseContext = func(net.Listener) context.Context {
+               return ctxlog.Context(context.Background(), s.log)
+       }
+       s.testServer.Server.Handler = httpserver.AddRequestIDs(httpserver.LogRequests(s.testHandler))
 
        cluster.RemoteClusters = map[string]arvados.RemoteCluster{
                "zzzzz": {
@@ -80,6 +88,9 @@ func (s *FederationSuite) SetUpTest(c *check.C) {
                        Proxy:  true,
                        Scheme: "http",
                },
+               "*": {
+                       Scheme: "https",
+               },
        }
 
        c.Assert(s.testServer.Start(), check.IsNil)
@@ -93,6 +104,8 @@ func (s *FederationSuite) remoteMockHandler(w http.ResponseWriter, req *http.Req
        req.Body.Close()
        req.Body = ioutil.NopCloser(b)
        s.remoteMockRequests = append(s.remoteMockRequests, *req)
+       // Repond 200 with a valid JSON object
+       fmt.Fprint(w, "{}")
 }
 
 func (s *FederationSuite) TearDownTest(c *check.C) {
@@ -104,15 +117,15 @@ func (s *FederationSuite) TearDownTest(c *check.C) {
        }
 }
 
-func (s *FederationSuite) testRequest(req *http.Request) *http.Response {
+func (s *FederationSuite) testRequest(req *http.Request) *httptest.ResponseRecorder {
        resp := httptest.NewRecorder()
        s.testServer.Server.Handler.ServeHTTP(resp, req)
-       return resp.Result()
+       return resp
 }
 
 func (s *FederationSuite) TestLocalRequest(c *check.C) {
        req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+strings.Replace(arvadostest.WorkflowWithDefinitionYAMLUUID, "zzzzz-", "zhome-", 1), nil)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        s.checkHandledLocally(c, resp)
 }
 
@@ -127,31 +140,31 @@ func (s *FederationSuite) checkHandledLocally(c *check.C, resp *http.Response) {
 
 func (s *FederationSuite) TestNoAuth(c *check.C) {
        req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+arvadostest.WorkflowWithDefinitionYAMLUUID, nil)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusUnauthorized)
-       s.checkJSONErrorMatches(c, resp, `Not logged in`)
+       s.checkJSONErrorMatches(c, resp, `Not logged in.*`)
 }
 
 func (s *FederationSuite) TestBadAuth(c *check.C) {
        req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+arvadostest.WorkflowWithDefinitionYAMLUUID, nil)
        req.Header.Set("Authorization", "Bearer aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusUnauthorized)
-       s.checkJSONErrorMatches(c, resp, `Not logged in`)
+       s.checkJSONErrorMatches(c, resp, `Not logged in.*`)
 }
 
 func (s *FederationSuite) TestNoAccess(c *check.C) {
        req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+arvadostest.WorkflowWithDefinitionYAMLUUID, nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.SpectatorToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
-       s.checkJSONErrorMatches(c, resp, `.*not found`)
+       s.checkJSONErrorMatches(c, resp, `.*not found.*`)
 }
 
 func (s *FederationSuite) TestGetUnknownRemote(c *check.C) {
        req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+strings.Replace(arvadostest.WorkflowWithDefinitionYAMLUUID, "zzzzz-", "zz404-", 1), nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
        s.checkJSONErrorMatches(c, resp, `.*no proxy available for cluster zz404`)
 }
@@ -163,7 +176,7 @@ func (s *FederationSuite) TestRemoteError(c *check.C) {
 
        req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+arvadostest.WorkflowWithDefinitionYAMLUUID, nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusBadGateway)
        s.checkJSONErrorMatches(c, resp, `.*HTTP response to HTTPS client`)
 }
@@ -171,7 +184,7 @@ func (s *FederationSuite) TestRemoteError(c *check.C) {
 func (s *FederationSuite) TestGetRemoteWorkflow(c *check.C) {
        req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+arvadostest.WorkflowWithDefinitionYAMLUUID, nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        var wf arvados.Workflow
        c.Check(json.NewDecoder(resp.Body).Decode(&wf), check.IsNil)
@@ -182,7 +195,7 @@ func (s *FederationSuite) TestGetRemoteWorkflow(c *check.C) {
 func (s *FederationSuite) TestOptionsMethod(c *check.C) {
        req := httptest.NewRequest("OPTIONS", "/arvados/v1/workflows/"+arvadostest.WorkflowWithDefinitionYAMLUUID, nil)
        req.Header.Set("Origin", "https://example.com")
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        body, err := ioutil.ReadAll(resp.Body)
        c.Check(err, check.IsNil)
@@ -198,7 +211,7 @@ func (s *FederationSuite) TestOptionsMethod(c *check.C) {
 
 func (s *FederationSuite) TestRemoteWithTokenInQuery(c *check.C) {
        req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+strings.Replace(arvadostest.WorkflowWithDefinitionYAMLUUID, "zzzzz-", "zmock-", 1)+"?api_token="+arvadostest.ActiveToken, nil)
-       s.testRequest(req)
+       s.testRequest(req).Result()
        c.Assert(s.remoteMockRequests, check.HasLen, 1)
        pr := s.remoteMockRequests[0]
        // Token is salted and moved from query to Authorization header.
@@ -207,39 +220,58 @@ func (s *FederationSuite) TestRemoteWithTokenInQuery(c *check.C) {
 }
 
 func (s *FederationSuite) TestLocalTokenSalted(c *check.C) {
-       req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+strings.Replace(arvadostest.WorkflowWithDefinitionYAMLUUID, "zzzzz-", "zmock-", 1), nil)
-       req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       s.testRequest(req)
-       c.Assert(s.remoteMockRequests, check.HasLen, 1)
-       pr := s.remoteMockRequests[0]
-       // The salted token here has a "zzzzz-" UUID instead of a
-       // "ztest-" UUID because ztest's local database has the
-       // "zzzzz-" test fixtures. The "secret" part is HMAC(sha1,
-       // arvadostest.ActiveToken, "zmock") = "7fd3...".
-       c.Check(pr.Header.Get("Authorization"), check.Equals, "Bearer v2/zzzzz-gj3su-077z32aux8dg2s1/7fd31b61f39c0e82a4155592163218272cedacdc")
+       defer s.localServiceReturns404(c).Close()
+       for _, path := range []string{
+               // During the transition to the strongly typed
+               // controller implementation (#14287), workflows and
+               // collections test different code paths.
+               "/arvados/v1/workflows/" + strings.Replace(arvadostest.WorkflowWithDefinitionYAMLUUID, "zzzzz-", "zmock-", 1),
+               "/arvados/v1/collections/" + strings.Replace(arvadostest.UserAgreementCollection, "zzzzz-", "zmock-", 1),
+       } {
+               c.Log("testing path ", path)
+               s.remoteMockRequests = nil
+               req := httptest.NewRequest("GET", path, nil)
+               req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+               s.testRequest(req).Result()
+               c.Assert(s.remoteMockRequests, check.HasLen, 1)
+               pr := s.remoteMockRequests[0]
+               // The salted token here has a "zzzzz-" UUID instead of a
+               // "ztest-" UUID because ztest's local database has the
+               // "zzzzz-" test fixtures. The "secret" part is HMAC(sha1,
+               // arvadostest.ActiveToken, "zmock") = "7fd3...".
+               c.Check(pr.Header.Get("Authorization"), check.Equals, "Bearer v2/zzzzz-gj3su-077z32aux8dg2s1/7fd31b61f39c0e82a4155592163218272cedacdc")
+       }
 }
 
 func (s *FederationSuite) TestRemoteTokenNotSalted(c *check.C) {
+       defer s.localServiceReturns404(c).Close()
        // remoteToken can be any v1 token that doesn't appear in
        // ztest's local db.
        remoteToken := "abcdef00000000000000000000000000000000000000000000"
-       req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+strings.Replace(arvadostest.WorkflowWithDefinitionYAMLUUID, "zzzzz-", "zmock-", 1), nil)
-       req.Header.Set("Authorization", "Bearer "+remoteToken)
-       s.testRequest(req)
-       c.Assert(s.remoteMockRequests, check.HasLen, 1)
-       pr := s.remoteMockRequests[0]
-       c.Check(pr.Header.Get("Authorization"), check.Equals, "Bearer "+remoteToken)
+
+       for _, path := range []string{
+               // During the transition to the strongly typed
+               // controller implementation (#14287), workflows and
+               // collections test different code paths.
+               "/arvados/v1/workflows/" + strings.Replace(arvadostest.WorkflowWithDefinitionYAMLUUID, "zzzzz-", "zmock-", 1),
+               "/arvados/v1/collections/" + strings.Replace(arvadostest.UserAgreementCollection, "zzzzz-", "zmock-", 1),
+       } {
+               c.Log("testing path ", path)
+               s.remoteMockRequests = nil
+               req := httptest.NewRequest("GET", path, nil)
+               req.Header.Set("Authorization", "Bearer "+remoteToken)
+               s.testRequest(req).Result()
+               c.Assert(s.remoteMockRequests, check.HasLen, 1)
+               pr := s.remoteMockRequests[0]
+               c.Check(pr.Header.Get("Authorization"), check.Equals, "Bearer "+remoteToken)
+       }
 }
 
 func (s *FederationSuite) TestWorkflowCRUD(c *check.C) {
-       wf := arvados.Workflow{
-               Description: "TestCRUD",
-       }
+       var wf arvados.Workflow
        {
-               body := &strings.Builder{}
-               json.NewEncoder(body).Encode(&wf)
                req := httptest.NewRequest("POST", "/arvados/v1/workflows", strings.NewReader(url.Values{
-                       "workflow": {body.String()},
+                       "workflow": {`{"description": "TestCRUD"}`},
                }.Encode()))
                req.Header.Set("Content-type", "application/x-www-form-urlencoded")
                req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
@@ -270,7 +302,7 @@ func (s *FederationSuite) TestWorkflowCRUD(c *check.C) {
                req := httptest.NewRequest(method, "/arvados/v1/workflows/"+wf.UUID, strings.NewReader(form.Encode()))
                req.Header.Set("Content-type", "application/x-www-form-urlencoded")
                req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-               resp := s.testRequest(req)
+               resp := s.testRequest(req).Result()
                s.checkResponseOK(c, resp)
                err := json.NewDecoder(resp.Body).Decode(&wf)
                c.Check(err, check.IsNil)
@@ -280,7 +312,7 @@ func (s *FederationSuite) TestWorkflowCRUD(c *check.C) {
        {
                req := httptest.NewRequest("DELETE", "/arvados/v1/workflows/"+wf.UUID, nil)
                req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-               resp := s.testRequest(req)
+               resp := s.testRequest(req).Result()
                s.checkResponseOK(c, resp)
                err := json.NewDecoder(resp.Body).Decode(&wf)
                c.Check(err, check.IsNil)
@@ -288,7 +320,7 @@ func (s *FederationSuite) TestWorkflowCRUD(c *check.C) {
        {
                req := httptest.NewRequest("GET", "/arvados/v1/workflows/"+wf.UUID, nil)
                req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-               resp := s.testRequest(req)
+               resp := s.testRequest(req).Result()
                c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
        }
 }
@@ -322,7 +354,21 @@ func (s *FederationSuite) localServiceHandler(c *check.C, h http.Handler) *https
 
 func (s *FederationSuite) localServiceReturns404(c *check.C) *httpserver.Server {
        return s.localServiceHandler(c, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-               w.WriteHeader(404)
+               if req.URL.Path == "/arvados/v1/api_client_authorizations/current" {
+                       if req.Header.Get("Authorization") == "Bearer "+arvadostest.ActiveToken {
+                               json.NewEncoder(w).Encode(arvados.APIClientAuthorization{UUID: arvadostest.ActiveTokenUUID, APIToken: arvadostest.ActiveToken, Scopes: []string{"all"}})
+                       } else {
+                               w.WriteHeader(http.StatusUnauthorized)
+                       }
+               } else if req.URL.Path == "/arvados/v1/users/current" {
+                       if req.Header.Get("Authorization") == "Bearer "+arvadostest.ActiveToken {
+                               json.NewEncoder(w).Encode(arvados.User{UUID: arvadostest.ActiveUserUUID})
+                       } else {
+                               w.WriteHeader(http.StatusUnauthorized)
+                       }
+               } else {
+                       w.WriteHeader(404)
+               }
        }))
 }
 
@@ -334,7 +380,7 @@ func (s *FederationSuite) TestGetLocalCollection(c *check.C) {
 
        req := httptest.NewRequest("GET", "/arvados/v1/collections/"+arvadostest.UserAgreementCollection, nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
 
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        var col arvados.Collection
@@ -351,7 +397,7 @@ func (s *FederationSuite) TestGetLocalCollection(c *check.C) {
        }).Encode()))
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
        req.Header.Set("Content-Type", "application/x-www-form-urlencoded; charset=UTF-8")
-       resp = s.testRequest(req)
+       resp = s.testRequest(req).Result()
 
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        col = arvados.Collection{}
@@ -367,7 +413,7 @@ func (s *FederationSuite) TestGetRemoteCollection(c *check.C) {
 
        req := httptest.NewRequest("GET", "/arvados/v1/collections/"+arvadostest.UserAgreementCollection, nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        var col arvados.Collection
        c.Check(json.NewDecoder(resp.Body).Decode(&col), check.IsNil)
@@ -382,7 +428,7 @@ func (s *FederationSuite) TestGetRemoteCollectionError(c *check.C) {
 
        req := httptest.NewRequest("GET", "/arvados/v1/collections/zzzzz-4zz18-fakefakefakefak", nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
 }
 
@@ -404,7 +450,7 @@ func (s *FederationSuite) TestGetLocalCollectionByPDH(c *check.C) {
 
        req := httptest.NewRequest("GET", "/arvados/v1/collections/"+arvadostest.UserAgreementPDH, nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
 
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        var col arvados.Collection
@@ -420,7 +466,7 @@ func (s *FederationSuite) TestGetRemoteCollectionByPDH(c *check.C) {
 
        req := httptest.NewRequest("GET", "/arvados/v1/collections/"+arvadostest.UserAgreementPDH, nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
 
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
 
@@ -435,10 +481,14 @@ func (s *FederationSuite) TestGetRemoteCollectionByPDH(c *check.C) {
 func (s *FederationSuite) TestGetCollectionByPDHError(c *check.C) {
        defer s.localServiceReturns404(c).Close()
 
+       // zmock's normal response (200 with an empty body) would
+       // change the outcome from 404 to 502
+       delete(s.testHandler.Cluster.RemoteClusters, "zmock")
+
        req := httptest.NewRequest("GET", "/arvados/v1/collections/99999999999999999999999999999999+99", nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
 
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        defer resp.Body.Close()
 
        c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
@@ -447,6 +497,10 @@ func (s *FederationSuite) TestGetCollectionByPDHError(c *check.C) {
 func (s *FederationSuite) TestGetCollectionByPDHErrorBadHash(c *check.C) {
        defer s.localServiceReturns404(c).Close()
 
+       // zmock's normal response (200 with an empty body) would
+       // change the outcome
+       delete(s.testHandler.Cluster.RemoteClusters, "zmock")
+
        srv2 := &httpserver.Server{
                Server: http.Server{
                        Handler: http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
@@ -477,10 +531,10 @@ func (s *FederationSuite) TestGetCollectionByPDHErrorBadHash(c *check.C) {
        req := httptest.NewRequest("GET", "/arvados/v1/collections/99999999999999999999999999999999+99", nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
 
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        defer resp.Body.Close()
 
-       c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
+       c.Check(resp.StatusCode, check.Equals, http.StatusBadGateway)
 }
 
 func (s *FederationSuite) TestSaltedTokenGetCollectionByPDH(c *check.C) {
@@ -488,7 +542,7 @@ func (s *FederationSuite) TestSaltedTokenGetCollectionByPDH(c *check.C) {
 
        req := httptest.NewRequest("GET", "/arvados/v1/collections/"+arvadostest.UserAgreementPDH, nil)
        req.Header.Set("Authorization", "Bearer v2/zzzzz-gj3su-077z32aux8dg2s1/282d7d172b6cfdce364c5ed12ddf7417b2d00065")
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
 
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        var col arvados.Collection
@@ -502,9 +556,13 @@ func (s *FederationSuite) TestSaltedTokenGetCollectionByPDH(c *check.C) {
 func (s *FederationSuite) TestSaltedTokenGetCollectionByPDHError(c *check.C) {
        arvadostest.SetServiceURL(&s.testHandler.Cluster.Services.RailsAPI, "https://"+os.Getenv("ARVADOS_TEST_API_HOST"))
 
+       // zmock's normal response (200 with an empty body) would
+       // change the outcome
+       delete(s.testHandler.Cluster.RemoteClusters, "zmock")
+
        req := httptest.NewRequest("GET", "/arvados/v1/collections/99999999999999999999999999999999+99", nil)
        req.Header.Set("Authorization", "Bearer v2/zzzzz-gj3su-077z32aux8dg2s1/282d7d172b6cfdce364c5ed12ddf7417b2d00065")
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
 
        c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
 }
@@ -513,7 +571,7 @@ func (s *FederationSuite) TestGetRemoteContainerRequest(c *check.C) {
        defer s.localServiceReturns404(c).Close()
        req := httptest.NewRequest("GET", "/arvados/v1/container_requests/"+arvadostest.QueuedContainerRequestUUID, nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        var cr arvados.ContainerRequest
        c.Check(json.NewDecoder(resp.Body).Decode(&cr), check.IsNil)
@@ -528,7 +586,7 @@ func (s *FederationSuite) TestUpdateRemoteContainerRequest(c *check.C) {
                        strings.NewReader(fmt.Sprintf(`{"container_request": {"priority": %d}}`, pri)))
                req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
                req.Header.Set("Content-type", "application/json")
-               resp := s.testRequest(req)
+               resp := s.testRequest(req).Result()
                c.Check(resp.StatusCode, check.Equals, http.StatusOK)
                var cr arvados.ContainerRequest
                c.Check(json.NewDecoder(resp.Body).Decode(&cr), check.IsNil)
@@ -539,6 +597,21 @@ func (s *FederationSuite) TestUpdateRemoteContainerRequest(c *check.C) {
        setPri(1) // Reset fixture so side effect doesn't break other tests.
 }
 
+func (s *FederationSuite) TestCreateContainerRequestBadToken(c *check.C) {
+       defer s.localServiceReturns404(c).Close()
+       // pass cluster_id via query parameter, this allows arvados-controller
+       // to avoid parsing the body
+       req := httptest.NewRequest("POST", "/arvados/v1/container_requests?cluster_id=zzzzz",
+               strings.NewReader(`{"container_request":{}}`))
+       req.Header.Set("Authorization", "Bearer abcdefg")
+       req.Header.Set("Content-type", "application/json")
+       resp := s.testRequest(req).Result()
+       c.Check(resp.StatusCode, check.Equals, http.StatusForbidden)
+       var e map[string][]string
+       c.Check(json.NewDecoder(resp.Body).Decode(&e), check.IsNil)
+       c.Check(e["errors"], check.DeepEquals, []string{"invalid API token"})
+}
+
 func (s *FederationSuite) TestCreateRemoteContainerRequest(c *check.C) {
        defer s.localServiceReturns404(c).Close()
        // pass cluster_id via query parameter, this allows arvados-controller
@@ -556,7 +629,7 @@ func (s *FederationSuite) TestCreateRemoteContainerRequest(c *check.C) {
 `))
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
        req.Header.Set("Content-type", "application/json")
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        var cr arvados.ContainerRequest
        c.Check(json.NewDecoder(resp.Body).Decode(&cr), check.IsNil)
@@ -564,38 +637,93 @@ func (s *FederationSuite) TestCreateRemoteContainerRequest(c *check.C) {
        c.Check(strings.HasPrefix(cr.UUID, "zzzzz-"), check.Equals, true)
 }
 
+// getCRfromMockRequest returns a ContainerRequest with the content of the
+// request sent to the remote mock. This function takes into account the
+// Content-Type and acts accordingly.
+func (s *FederationSuite) getCRfromMockRequest(c *check.C) arvados.ContainerRequest {
+
+       // Body can be a json formated or something like:
+       //  cluster_id=zmock&container_request=%7B%22command%22%3A%5B%22abc%22%5D%2C%22container_image%22%3A%22123%22%2C%22...7D
+       // or:
+       //  "{\"container_request\":{\"command\":[\"abc\"],\"container_image\":\"12...Uncommitted\"}}"
+
+       var cr arvados.ContainerRequest
+       data, err := ioutil.ReadAll(s.remoteMockRequests[0].Body)
+       c.Check(err, check.IsNil)
+
+       if s.remoteMockRequests[0].Header.Get("Content-Type") == "application/json" {
+               // legacy code path sends a JSON request body
+               var answerCR struct {
+                       ContainerRequest arvados.ContainerRequest `json:"container_request"`
+               }
+               c.Check(json.Unmarshal(data, &answerCR), check.IsNil)
+               cr = answerCR.ContainerRequest
+       } else if s.remoteMockRequests[0].Header.Get("Content-Type") == "application/x-www-form-urlencoded" {
+               // new code path sends a form-encoded request body with a JSON-encoded parameter value
+               decodedValue, err := url.ParseQuery(string(data))
+               c.Check(err, check.IsNil)
+               decodedValueCR := decodedValue.Get("container_request")
+               c.Check(json.Unmarshal([]byte(decodedValueCR), &cr), check.IsNil)
+       } else {
+               // mock needs to have Content-Type that we can parse.
+               c.Fail()
+       }
+
+       return cr
+}
+
 func (s *FederationSuite) TestCreateRemoteContainerRequestCheckRuntimeToken(c *check.C) {
        // Send request to zmock and check that outgoing request has
        // runtime_token set with a new random v2 token.
 
        defer s.localServiceReturns404(c).Close()
-       // pass cluster_id via query parameter, this allows arvados-controller
-       // to avoid parsing the body
        req := httptest.NewRequest("POST", "/arvados/v1/container_requests?cluster_id=zmock",
                strings.NewReader(`{
-  "container_request": {
-    "name": "hello world",
-    "state": "Uncommitted",
-    "output_path": "/",
-    "container_image": "123",
-    "command": ["abc"]
-  }
-}
-`))
+         "container_request": {
+           "name": "hello world",
+           "state": "Uncommitted",
+           "output_path": "/",
+           "container_image": "123",
+           "command": ["abc"]
+         }
+       }
+       `))
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
        req.Header.Set("Content-type", "application/json")
 
+       // We replace zhome with zzzzz values (RailsAPI, ClusterID, SystemRootToken)
+       // SystemRoot token is needed because we check the
+       // https://[RailsAPI]/arvados/v1/api_client_authorizations/current
+       // https://[RailsAPI]/arvados/v1/users/current and
+       // https://[RailsAPI]/auth/controller/callback
        arvadostest.SetServiceURL(&s.testHandler.Cluster.Services.RailsAPI, "https://"+os.Getenv("ARVADOS_TEST_API_HOST"))
        s.testHandler.Cluster.ClusterID = "zzzzz"
+       s.testHandler.Cluster.SystemRootToken = arvadostest.SystemRootToken
+       s.testHandler.Cluster.API.MaxTokenLifetime = arvados.Duration(time.Hour)
 
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
-       var cr struct {
-               arvados.ContainerRequest `json:"container_request"`
-       }
-       c.Check(json.NewDecoder(s.remoteMockRequests[0].Body).Decode(&cr), check.IsNil)
-       c.Check(strings.HasPrefix(cr.ContainerRequest.RuntimeToken, "v2/zzzzz-gj3su-"), check.Equals, true)
-       c.Check(cr.ContainerRequest.RuntimeToken, check.Not(check.Equals), arvadostest.ActiveTokenV2)
+
+       cr := s.getCRfromMockRequest(c)
+
+       // Runtime token must match zzzzz cluster
+       c.Check(cr.RuntimeToken, check.Matches, "v2/zzzzz-gj3su-.*")
+
+       // RuntimeToken must be different than the Original Token we originally did the request with.
+       c.Check(cr.RuntimeToken, check.Not(check.Equals), arvadostest.ActiveTokenV2)
+
+       // Runtime token should not have an expiration based on API.MaxTokenLifetime
+       req2 := httptest.NewRequest("GET", "/arvados/v1/api_client_authorizations/current", nil)
+       req2.Header.Set("Authorization", "Bearer "+cr.RuntimeToken)
+       req2.Header.Set("Content-type", "application/json")
+       resp = s.testRequest(req2).Result()
+       c.Check(resp.StatusCode, check.Equals, http.StatusOK)
+       var aca arvados.APIClientAuthorization
+       c.Check(json.NewDecoder(resp.Body).Decode(&aca), check.IsNil)
+       c.Check(aca.ExpiresAt, check.NotNil) // Time.Now()+BlobSigningTTL
+       t, _ := time.Parse(time.RFC3339Nano, aca.ExpiresAt)
+       c.Check(t.After(time.Now().Add(s.testHandler.Cluster.API.MaxTokenLifetime.Duration())), check.Equals, true)
+       c.Check(t.Before(time.Now().Add(s.testHandler.Cluster.Collections.BlobSigningTTL.Duration())), check.Equals, true)
 }
 
 func (s *FederationSuite) TestCreateRemoteContainerRequestCheckSetRuntimeToken(c *check.C) {
@@ -607,54 +735,25 @@ func (s *FederationSuite) TestCreateRemoteContainerRequestCheckSetRuntimeToken(c
        // to avoid parsing the body
        req := httptest.NewRequest("POST", "/arvados/v1/container_requests?cluster_id=zmock",
                strings.NewReader(`{
-  "container_request": {
-    "name": "hello world",
-    "state": "Uncommitted",
-    "output_path": "/",
-    "container_image": "123",
-    "command": ["abc"],
-    "runtime_token": "xyz"
-  }
-}
-`))
+         "container_request": {
+           "name": "hello world",
+           "state": "Uncommitted",
+           "output_path": "/",
+           "container_image": "123",
+           "command": ["abc"],
+           "runtime_token": "xyz"
+         }
+       }
+       `))
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
        req.Header.Set("Content-type", "application/json")
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
-       var cr struct {
-               arvados.ContainerRequest `json:"container_request"`
-       }
-       c.Check(json.NewDecoder(s.remoteMockRequests[0].Body).Decode(&cr), check.IsNil)
-       c.Check(cr.ContainerRequest.RuntimeToken, check.Equals, "xyz")
-}
 
-func (s *FederationSuite) TestCreateRemoteContainerRequestRuntimeTokenFromAuth(c *check.C) {
-       // Send request to zmock and check that outgoing request has
-       // runtime_token set using the Auth token because the user is remote.
+       cr := s.getCRfromMockRequest(c)
 
-       defer s.localServiceReturns404(c).Close()
-       // pass cluster_id via query parameter, this allows arvados-controller
-       // to avoid parsing the body
-       req := httptest.NewRequest("POST", "/arvados/v1/container_requests?cluster_id=zmock",
-               strings.NewReader(`{
-  "container_request": {
-    "name": "hello world",
-    "state": "Uncommitted",
-    "output_path": "/",
-    "container_image": "123",
-    "command": ["abc"]
-  }
-}
-`))
-       req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2+"/zzzzz-dz642-parentcontainer")
-       req.Header.Set("Content-type", "application/json")
-       resp := s.testRequest(req)
-       c.Check(resp.StatusCode, check.Equals, http.StatusOK)
-       var cr struct {
-               arvados.ContainerRequest `json:"container_request"`
-       }
-       c.Check(json.NewDecoder(s.remoteMockRequests[0].Body).Decode(&cr), check.IsNil)
-       c.Check(cr.ContainerRequest.RuntimeToken, check.Equals, arvadostest.ActiveTokenV2)
+       // After mocking around now making sure the runtime_token we sent is still there.
+       c.Check(cr.RuntimeToken, check.Equals, "xyz")
 }
 
 func (s *FederationSuite) TestCreateRemoteContainerRequestError(c *check.C) {
@@ -674,7 +773,7 @@ func (s *FederationSuite) TestCreateRemoteContainerRequestError(c *check.C) {
 `))
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
        req.Header.Set("Content-type", "application/json")
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
 }
 
@@ -683,7 +782,7 @@ func (s *FederationSuite) TestGetRemoteContainer(c *check.C) {
        req := httptest.NewRequest("GET", "/arvados/v1/containers/"+arvadostest.QueuedContainerUUID, nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
        resp := s.testRequest(req)
-       c.Check(resp.StatusCode, check.Equals, http.StatusOK)
+       c.Check(resp.Code, check.Equals, http.StatusOK)
        var cn arvados.Container
        c.Check(json.NewDecoder(resp.Body).Decode(&cn), check.IsNil)
        c.Check(cn.UUID, check.Equals, arvadostest.QueuedContainerUUID)
@@ -694,10 +793,11 @@ func (s *FederationSuite) TestListRemoteContainer(c *check.C) {
        req := httptest.NewRequest("GET", "/arvados/v1/containers?count=none&filters="+
                url.QueryEscape(fmt.Sprintf(`[["uuid", "in", ["%v"]]]`, arvadostest.QueuedContainerUUID)), nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        var cn arvados.ContainerList
        c.Check(json.NewDecoder(resp.Body).Decode(&cn), check.IsNil)
+       c.Assert(cn.Items, check.HasLen, 1)
        c.Check(cn.Items[0].UUID, check.Equals, arvadostest.QueuedContainerUUID)
 }
 
@@ -714,7 +814,7 @@ func (s *FederationSuite) TestListMultiRemoteContainers(c *check.C) {
                url.QueryEscape(`["uuid", "command"]`)),
                nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        var cn arvados.ContainerList
        c.Check(json.NewDecoder(resp.Body).Decode(&cn), check.IsNil)
@@ -737,7 +837,7 @@ func (s *FederationSuite) TestListMultiRemoteContainerError(c *check.C) {
                url.QueryEscape(`["uuid", "command"]`)),
                nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusBadGateway)
        s.checkJSONErrorMatches(c, resp, `error fetching from zhome \(404 Not Found\): EOF`)
 }
@@ -756,14 +856,14 @@ func (s *FederationSuite) TestListMultiRemoteContainersPaged(c *check.C) {
                        w.WriteHeader(200)
                        w.Write([]byte(`{"kind": "arvados#containerList", "items": [{"uuid": "zhome-xvhdp-cr6queuedcontnr", "command": ["efg"]}]}`))
                }
-               callCount += 1
+               callCount++
        })).Close()
        req := httptest.NewRequest("GET", fmt.Sprintf("/arvados/v1/containers?count=none&filters=%s",
                url.QueryEscape(fmt.Sprintf(`[["uuid", "in", ["%v", "zhome-xvhdp-cr5queuedcontnr", "zhome-xvhdp-cr6queuedcontnr"]]]`,
                        arvadostest.QueuedContainerUUID))),
                nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        c.Check(callCount, check.Equals, 2)
        var cn arvados.ContainerList
@@ -792,14 +892,14 @@ func (s *FederationSuite) TestListMultiRemoteContainersMissing(c *check.C) {
                        w.WriteHeader(200)
                        w.Write([]byte(`{"kind": "arvados#containerList", "items": []}`))
                }
-               callCount += 1
+               callCount++
        })).Close()
        req := httptest.NewRequest("GET", fmt.Sprintf("/arvados/v1/containers?count=none&filters=%s",
                url.QueryEscape(fmt.Sprintf(`[["uuid", "in", ["%v", "zhome-xvhdp-cr5queuedcontnr", "zhome-xvhdp-cr6queuedcontnr"]]]`,
                        arvadostest.QueuedContainerUUID))),
                nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        c.Check(callCount, check.Equals, 2)
        var cn arvados.ContainerList
@@ -820,7 +920,7 @@ func (s *FederationSuite) TestListMultiRemoteContainerPageSizeError(c *check.C)
                        arvadostest.QueuedContainerUUID))),
                nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusBadRequest)
        s.checkJSONErrorMatches(c, resp, `Federated multi-object request for 2 objects which is more than max page size 1.`)
 }
@@ -831,7 +931,7 @@ func (s *FederationSuite) TestListMultiRemoteContainerLimitError(c *check.C) {
                        arvadostest.QueuedContainerUUID))),
                nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusBadRequest)
        s.checkJSONErrorMatches(c, resp, `Federated multi-object may not provide 'limit', 'offset' or 'order'.`)
 }
@@ -842,7 +942,7 @@ func (s *FederationSuite) TestListMultiRemoteContainerOffsetError(c *check.C) {
                        arvadostest.QueuedContainerUUID))),
                nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusBadRequest)
        s.checkJSONErrorMatches(c, resp, `Federated multi-object may not provide 'limit', 'offset' or 'order'.`)
 }
@@ -853,7 +953,7 @@ func (s *FederationSuite) TestListMultiRemoteContainerOrderError(c *check.C) {
                        arvadostest.QueuedContainerUUID))),
                nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusBadRequest)
        s.checkJSONErrorMatches(c, resp, `Federated multi-object may not provide 'limit', 'offset' or 'order'.`)
 }
@@ -865,7 +965,7 @@ func (s *FederationSuite) TestListMultiRemoteContainerSelectError(c *check.C) {
                url.QueryEscape(`["command"]`)),
                nil)
        req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       resp := s.testRequest(req)
+       resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusBadRequest)
        s.checkJSONErrorMatches(c, resp, `Federated multi-object request must include 'uuid' in 'select'`)
 }