16736: Adds tests to confirm expires_at gets properly set on runtime tokens.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas@di-pentima.com.ar>

diff --git a/lib/controller/federation_test.go b/lib/controller/federation_test.go
index a92fc71053cea1f5ff3adfa4c5db0fca5c84576b..e3b2291bcef4481ff37159c2d3c8b79b744b03e6 100644 (file)
--- a/lib/controller/federation_test.go
+++ b/lib/controller/federation_test.go
@@ -695,6 +695,8 @@ func (s *FederationSuite) TestCreateRemoteContainerRequestCheckRuntimeToken(c *c
        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)
+       s.testHandler.Cluster.Collections.BlobSigningTTL = arvados.Duration(336 * time.Hour) // For some reason, this was set to 0h
 
        resp := s.testRequest(req).Result()
        c.Check(resp.StatusCode, check.Equals, http.StatusOK)
@@ -703,8 +705,22 @@ func (s *FederationSuite) TestCreateRemoteContainerRequestCheckRuntimeToken(c *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) {

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 8feee77ff23553eaba0429125c1b06f3f5688d50..e6d945a005c79dd3e3a30549bc43b4768aaed021 100644 (file)
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -605,7 +605,8 @@ class Container < ArvadosModel
         self.runtime_auth_scopes = ["all"]
       end
 
-      # generate a new token
+      # Generate a new token. This runs with admin credentials as it's done by a
+      # dispatcher user, so expires_at isn't enforced by API.MaxTokenLifetime.
       self.auth = ApiClientAuthorization.
                     create!(user_id: User.find_by_uuid(self.runtime_user_uuid).id,
                             api_client_id: 0,

diff --git a/services/api/test/integration/container_dispatch_test.rb b/services/api/test/integration/container_dispatch_test.rb
index 61e01da64c08bea1921bce6598ccd980ac2a37cb..556b889fad1c4b063ef8b2bcdc9cbd7e6905f48e 100644 (file)
--- a/services/api/test/integration/container_dispatch_test.rb
+++ b/services/api/test/integration/container_dispatch_test.rb
@@ -11,7 +11,6 @@ class ContainerDispatchTest < ActionDispatch::IntegrationTest
     get("/arvados/v1/api_client_authorizations/current",
         headers: authheaders)
     assert_response 200
-    #assert_not_empty json_response['uuid']
 
     system_auth_uuid = json_response['uuid']
     post("/arvados/v1/containers/#{containers(:queued).uuid}/lock",

diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 35e2b7ed1d0501d02162e22cd08e3556107b2799..375ab5a7bbb9d22a10144a6ae02e1b72a8f3da8c 100644 (file)
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -750,6 +750,17 @@ class ContainerTest < ActiveSupport::TestCase
     check_no_change_from_cancelled c
   end
 
+  test "Container locked with non-expiring token" do
+    Rails.configuration.API.TokenMaxLifetime = 1.hour
+    set_user_from_auth :active
+    c, _ = minimal_new
+    set_user_from_auth :dispatch1
+    assert c.lock, show_errors(c)
+    refute c.auth.nil?
+    assert c.auth.expires_at.nil?
+    assert c.auth.user_id == User.find_by_uuid(users(:active).uuid).id
+  end
+
   test "Container locked cancel with log" do
     set_user_from_auth :active
     c, _ = minimal_new