From 65219ea552b17e3501f933e0b5a40506a5837709 Mon Sep 17 00:00:00 2001 From: Lucas Di Pentima Date: Tue, 9 Feb 2021 19:27:23 -0300 Subject: [PATCH] 16736: Adds tests to confirm expires_at gets properly set on runtime tokens. Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima --- lib/controller/federation_test.go | 16 ++++++++++++++++ services/api/app/models/container.rb | 3 ++- .../test/integration/container_dispatch_test.rb | 1 - services/api/test/unit/container_test.rb | 11 +++++++++++ 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/lib/controller/federation_test.go b/lib/controller/federation_test.go index a92fc71053..e3b2291bce 100644 --- 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 8feee77ff2..e6d945a005 100644 --- 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 61e01da64c..556b889fad 100644 --- 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 35e2b7ed1d..375ab5a7bb 100644 --- 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 -- 2.30.2