[ARVADOS] created: 2.1.0-452-g65219ea55

Git user git at public.arvados.org
Thu Feb 11 23:40:02 UTC 2021


        at  65219ea552b17e3501f933e0b5a40506a5837709 (commit)


commit 65219ea552b17e3501f933e0b5a40506a5837709
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Tue Feb 9 19:27:23 2021 -0300

    16736: Adds tests to confirm expires_at gets properly set on runtime tokens.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/lib/controller/federation_test.go b/lib/controller/federation_test.go
index a92fc7105..e3b2291bc 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 8feee77ff..e6d945a00 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 61e01da64..556b889fa 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 35e2b7ed1..375ab5a7b 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

commit 71fd7a7a8bc1d00ef0ed3a9e6e5240c13b7967a0
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Mon Feb 8 19:01:41 2021 -0300

    16736: Simplifies conditionals.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 03e1b38fd..ee63c4d93 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -390,9 +390,7 @@ class ApiClientAuthorization < ArvadosModel
   def clamp_token_expiration
     if !current_user.andand.is_admin && Rails.configuration.API.MaxTokenLifetime > 0
       max_token_expiration = db_current_time + Rails.configuration.API.MaxTokenLifetime
-      if self.new_record? && (self.expires_at.nil? || self.expires_at > max_token_expiration)
-        self.expires_at = max_token_expiration
-      elsif !self.new_record? && self.expires_at_changed? && (self.expires_at.nil? || self.expires_at > max_token_expiration)
+      if (self.new_record? || self.expires_at_changed?) && (self.expires_at.nil? || self.expires_at > max_token_expiration)
         self.expires_at = max_token_expiration
       end
     end

commit 46a5e3958ce98fd1bca4f035dab83ea4c39e6666
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Mon Feb 8 18:16:52 2021 -0300

    16736: Replaces Time.now with db_current_time on token expiration handling code.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/app/controllers/user_sessions_controller.rb b/services/api/app/controllers/user_sessions_controller.rb
index da0523d2b..8e9a26b7a 100644
--- a/services/api/app/controllers/user_sessions_controller.rb
+++ b/services/api/app/controllers/user_sessions_controller.rb
@@ -158,9 +158,9 @@ class UserSessionsController < ApplicationController
     end
     if Rails.configuration.Login.TokenLifetime > 0
       if token_expiration == nil
-        token_expiration = Time.now + Rails.configuration.Login.TokenLifetime
+        token_expiration = db_current_time + Rails.configuration.Login.TokenLifetime
       else
-        token_expiration = [token_expiration, Time.now + Rails.configuration.Login.TokenLifetime].min
+        token_expiration = [token_expiration, db_current_time + Rails.configuration.Login.TokenLifetime].min
       end
     end
 
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 4218645d5..03e1b38fd 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -7,6 +7,7 @@ class ApiClientAuthorization < ArvadosModel
   include KindAndEtag
   include CommonApiTemplate
   extend CurrentApiClient
+  extend DbCurrentTime
 
   belongs_to :api_client
   belongs_to :user
@@ -356,7 +357,7 @@ class ApiClientAuthorization < ArvadosModel
       auth.update_attributes!(user: user,
                               api_token: stored_secret,
                               api_client_id: 0,
-                              expires_at: Time.now + Rails.configuration.Login.RemoteTokenRefresh)
+                              expires_at: db_current_time + Rails.configuration.Login.RemoteTokenRefresh)
       Rails.logger.debug "cached remote token #{token_uuid} with secret #{stored_secret} in local db"
       auth.api_token = secret
       return auth
@@ -388,7 +389,7 @@ class ApiClientAuthorization < ArvadosModel
 
   def clamp_token_expiration
     if !current_user.andand.is_admin && Rails.configuration.API.MaxTokenLifetime > 0
-      max_token_expiration = Time.now + Rails.configuration.API.MaxTokenLifetime
+      max_token_expiration = db_current_time + Rails.configuration.API.MaxTokenLifetime
       if self.new_record? && (self.expires_at.nil? || self.expires_at > max_token_expiration)
         self.expires_at = max_token_expiration
       elsif !self.new_record? && self.expires_at_changed? && (self.expires_at.nil? || self.expires_at > max_token_expiration)
diff --git a/services/api/test/integration/api_client_authorizations_api_test.rb b/services/api/test/integration/api_client_authorizations_api_test.rb
index 3a7b20131..ce79fc557 100644
--- a/services/api/test/integration/api_client_authorizations_api_test.rb
+++ b/services/api/test/integration/api_client_authorizations_api_test.rb
@@ -5,6 +5,8 @@
 require 'test_helper'
 
 class ApiClientAuthorizationsApiTest < ActionDispatch::IntegrationTest
+  include DbCurrentTime
+  extend DbCurrentTime
   fixtures :all
 
   test "create system auth" do
@@ -74,12 +76,12 @@ class ApiClientAuthorizationsApiTest < ActionDispatch::IntegrationTest
     assert_response 403
   end
 
-  [nil, Time.now + 2.hours].each do |desired_expiration|
+  [nil, db_current_time + 2.hours].each do |desired_expiration|
     test "expires_at gets clamped on non-admins when API.MaxTokenLifetime is set and desired expires_at #{desired_expiration.nil? ? 'is not set' : 'exceeds the limit'}" do
       Rails.configuration.API.MaxTokenLifetime = 1.hour
 
       # Test token creation
-      start_t = Time.now
+      start_t = db_current_time
       post "/arvados/v1/api_client_authorizations",
         params: {
           :format => :json,
@@ -89,7 +91,7 @@ class ApiClientAuthorizationsApiTest < ActionDispatch::IntegrationTest
           }
         },
         headers: {'HTTP_AUTHORIZATION' => "OAuth2 #{api_client_authorizations(:active_trustedclient).api_token}"}
-      end_t = Time.now
+      end_t = db_current_time
       assert_response 200
       expiration_t = json_response['expires_at'].to_time
       assert_operator expiration_t.to_f, :>, (start_t + Rails.configuration.API.MaxTokenLifetime).to_f
@@ -102,7 +104,7 @@ class ApiClientAuthorizationsApiTest < ActionDispatch::IntegrationTest
       # Test token update
       previous_expiration = expiration_t
       token_uuid = json_response["uuid"]
-      start_t = Time.now
+      start_t = db_current_time
       put "/arvados/v1/api_client_authorizations/#{token_uuid}",
         params: {
           :api_client_authorization => {
@@ -110,7 +112,7 @@ class ApiClientAuthorizationsApiTest < ActionDispatch::IntegrationTest
           }
         },
         headers: {'HTTP_AUTHORIZATION' => "OAuth2 #{api_client_authorizations(:active_trustedclient).api_token}"}
-      end_t = Time.now
+      end_t = db_current_time
       assert_response 200
       expiration_t = json_response['expires_at'].to_time
       assert_operator previous_expiration.to_f, :<, expiration_t.to_f
@@ -146,7 +148,7 @@ class ApiClientAuthorizationsApiTest < ActionDispatch::IntegrationTest
       previous_expiration = json_response['expires_at']
       token_uuid = json_response['uuid']
       if previous_expiration.nil?
-        desired_updated_expiration = Time.now + Rails.configuration.API.MaxTokenLifetime + 1.hour
+        desired_updated_expiration = db_current_time + Rails.configuration.API.MaxTokenLifetime + 1.hour
       else
         desired_updated_expiration = nil
       end

commit 1dc205aca4d9df2880083022b33216209d414052
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu Feb 4 14:46:30 2021 -0300

    16736: Updates arvados-login-sync to support expiring tokens.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/login-sync/bin/arvados-login-sync b/services/login-sync/bin/arvados-login-sync
index a9bff0546..d8836f19b 100755
--- a/services/login-sync/bin/arvados-login-sync
+++ b/services/login-sync/bin/arvados-login-sync
@@ -9,6 +9,7 @@ require 'arvados'
 require 'etc'
 require 'fileutils'
 require 'yaml'
+require 'optparse'
 
 req_envs = %w(ARVADOS_API_HOST ARVADOS_API_TOKEN ARVADOS_VIRTUAL_MACHINE_UUID)
 req_envs.each do |k|
@@ -17,16 +18,20 @@ req_envs.each do |k|
   end
 end
 
-exclusive_mode = ARGV.index("--exclusive")
+options = {}
+OptionParser.new do |parser|
+  parser.on('--exclusive', 'Manage SSH keys file exclusively.')
+  parser.on('--rotate-tokens', 'Always create new user tokens. Usually needed with --token-lifetime.')
+  parser.on('--skip-missing-users', "Don't try to create any local accounts.")
+  parser.on('--token-lifetime SECONDS', 'Create user tokens that expire after SECONDS.', Integer)
+end.parse!(into: options)
+
 exclusive_banner = "#######################################################################################
 #  THIS FILE IS MANAGED BY #{$0} -- CHANGES WILL BE OVERWRITTEN  #
 #######################################################################################\n\n"
 start_banner = "### BEGIN Arvados-managed keys -- changes between markers will be overwritten\n"
 end_banner = "### END Arvados-managed keys -- changes between markers will be overwritten\n"
 
-# Don't try to create any local accounts
-skip_missing_users = ARGV.index("--skip-missing-users")
-
 keys = ''
 
 begin
@@ -64,7 +69,7 @@ begin
       begin
         pwnam[l[:username]] = Etc.getpwnam(l[:username])
       rescue
-        if skip_missing_users
+        if options[:"skip-missing-users"]
           STDERR.puts "Account #{l[:username]} not found. Skipping"
           true
         end
@@ -165,7 +170,7 @@ begin
       oldkeys = ""
     end
 
-    if exclusive_mode
+    if options[:exclusive]
       newkeys = exclusive_banner + newkeys
     elsif oldkeys.start_with?(exclusive_banner)
       newkeys = start_banner + newkeys + end_banner
@@ -192,8 +197,12 @@ begin
     tokenfile = File.join(configarvados, "settings.conf")
 
     begin
-      if !File.exist?(tokenfile)
-        user_token = logincluster_arv.api_client_authorization.create(api_client_authorization: {owner_uuid: l[:user_uuid], api_client_id: 0})
+      if !File.exist?(tokenfile) || options[:"rotate-tokens"]
+        aca_params = {owner_uuid: l[:user_uuid], api_client_id: 0}
+        if options[:"token-lifetime"] && options[:"token-lifetime"] > 0
+          aca_params.merge!(expires_at: (Time.now + options[:"token-lifetime"]))
+        end
+        user_token = logincluster_arv.api_client_authorization.create(api_client_authorization: aca_params)
         f = File.new(tokenfile, 'w')
         f.write("ARVADOS_API_HOST=#{ENV['ARVADOS_API_HOST']}\n")
         f.write("ARVADOS_API_TOKEN=v2/#{user_token[:uuid]}/#{user_token[:api_token]}\n")

commit 4aa26076d2c7ed1f2a84ee0c5e9c63ab30ace530
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu Jan 28 18:30:30 2021 -0300

    16736: Fixes typo.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 2c0429edc..3e853e1dc 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -161,7 +161,7 @@ Clusters:
       # Limits for how long a client token created by regular users can be valid,
       # and also is used as a default expiration policy when no expiration date is
       # specified.
-      # Default value zero menans token expirations don't get clamped and no
+      # Default value zero means token expirations don't get clamped and no
       # default expiration is set.
       MaxTokenLifetime: 0s
 
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 6135e2a3c..a2d83f77f 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -167,7 +167,7 @@ Clusters:
       # Limits for how long a client token created by regular users can be valid,
       # and also is used as a default expiration policy when no expiration date is
       # specified.
-      # Default value zero menans token expirations don't get clamped and no
+      # Default value zero means token expirations don't get clamped and no
       # default expiration is set.
       MaxTokenLifetime: 0s
 

commit e8d73e8066b61f7704dc0f6cf200953cdf9a5e60
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu Jan 28 17:44:59 2021 -0300

    16736: Limit token's expires_at depending on the cluster config and user type.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb b/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
index 59e359232..99446688d 100644
--- a/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
+++ b/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
@@ -17,6 +17,7 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController
       scopes: {type: 'array', required: false}
     }
   end
+
   def create_system_auth
     @object = ApiClientAuthorization.
       new(user_id: system_user.id,
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 9290e01a1..4218645d5 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -13,6 +13,8 @@ class ApiClientAuthorization < ArvadosModel
   after_initialize :assign_random_api_token
   serialize :scopes, Array
 
+  before_validation :clamp_token_expiration
+
   api_accessible :user, extend: :common do |t|
     t.add :owner_uuid
     t.add :user_id
@@ -384,6 +386,17 @@ class ApiClientAuthorization < ArvadosModel
 
   protected
 
+  def clamp_token_expiration
+    if !current_user.andand.is_admin && Rails.configuration.API.MaxTokenLifetime > 0
+      max_token_expiration = Time.now + Rails.configuration.API.MaxTokenLifetime
+      if self.new_record? && (self.expires_at.nil? || self.expires_at > max_token_expiration)
+        self.expires_at = max_token_expiration
+      elsif !self.new_record? && self.expires_at_changed? && (self.expires_at.nil? || self.expires_at > max_token_expiration)
+        self.expires_at = max_token_expiration
+      end
+    end
+  end
+
   def permission_to_create
     current_user.andand.is_admin or (current_user.andand.id == self.user_id)
   end
diff --git a/services/api/config/arvados_config.rb b/services/api/config/arvados_config.rb
index 5327713f6..8f4395dad 100644
--- a/services/api/config/arvados_config.rb
+++ b/services/api/config/arvados_config.rb
@@ -92,6 +92,7 @@ arvcfg.declare_config "API.DisabledAPIs", Hash, :disable_api_methods, ->(cfg, k,
 arvcfg.declare_config "API.MaxRequestSize", Integer, :max_request_size
 arvcfg.declare_config "API.MaxIndexDatabaseRead", Integer, :max_index_database_read
 arvcfg.declare_config "API.MaxItemsPerResponse", Integer, :max_items_per_response
+arvcfg.declare_config "API.MaxTokenLifetime", ActiveSupport::Duration
 arvcfg.declare_config "API.AsyncPermissionsUpdateInterval", ActiveSupport::Duration, :async_permissions_update_interval
 arvcfg.declare_config "Users.AutoSetupNewUsers", Boolean, :auto_setup_new_users
 arvcfg.declare_config "Users.AutoSetupNewUsersWithVmUUID", String, :auto_setup_new_users_with_vm_uuid
diff --git a/services/api/test/integration/api_client_authorizations_api_test.rb b/services/api/test/integration/api_client_authorizations_api_test.rb
index 296ab8a2f..3a7b20131 100644
--- a/services/api/test/integration/api_client_authorizations_api_test.rb
+++ b/services/api/test/integration/api_client_authorizations_api_test.rb
@@ -74,4 +74,95 @@ class ApiClientAuthorizationsApiTest < ActionDispatch::IntegrationTest
     assert_response 403
   end
 
+  [nil, Time.now + 2.hours].each do |desired_expiration|
+    test "expires_at gets clamped on non-admins when API.MaxTokenLifetime is set and desired expires_at #{desired_expiration.nil? ? 'is not set' : 'exceeds the limit'}" do
+      Rails.configuration.API.MaxTokenLifetime = 1.hour
+
+      # Test token creation
+      start_t = Time.now
+      post "/arvados/v1/api_client_authorizations",
+        params: {
+          :format => :json,
+          :api_client_authorization => {
+            :owner_uuid => users(:active).uuid,
+            :expires_at => desired_expiration,
+          }
+        },
+        headers: {'HTTP_AUTHORIZATION' => "OAuth2 #{api_client_authorizations(:active_trustedclient).api_token}"}
+      end_t = Time.now
+      assert_response 200
+      expiration_t = json_response['expires_at'].to_time
+      assert_operator expiration_t.to_f, :>, (start_t + Rails.configuration.API.MaxTokenLifetime).to_f
+      if !desired_expiration.nil?
+        assert_operator expiration_t.to_f, :<, desired_expiration.to_f
+      else
+        assert_operator expiration_t.to_f, :<, (end_t + Rails.configuration.API.MaxTokenLifetime).to_f
+      end
+
+      # Test token update
+      previous_expiration = expiration_t
+      token_uuid = json_response["uuid"]
+      start_t = Time.now
+      put "/arvados/v1/api_client_authorizations/#{token_uuid}",
+        params: {
+          :api_client_authorization => {
+            :expires_at => desired_expiration
+          }
+        },
+        headers: {'HTTP_AUTHORIZATION' => "OAuth2 #{api_client_authorizations(:active_trustedclient).api_token}"}
+      end_t = Time.now
+      assert_response 200
+      expiration_t = json_response['expires_at'].to_time
+      assert_operator previous_expiration.to_f, :<, expiration_t.to_f
+      assert_operator expiration_t.to_f, :>, (start_t + Rails.configuration.API.MaxTokenLifetime).to_f
+      if !desired_expiration.nil?
+        assert_operator expiration_t.to_f, :<, desired_expiration.to_f
+      else
+        assert_operator expiration_t.to_f, :<, (end_t + Rails.configuration.API.MaxTokenLifetime).to_f
+      end
+    end
+
+    test "expires_at can be set to #{desired_expiration.nil? ? 'nil' : 'exceed the limit'} by admins when API.MaxTokenLifetime is set" do
+      Rails.configuration.API.MaxTokenLifetime = 1.hour
+
+      # Test token creation
+      post "/arvados/v1/api_client_authorizations",
+        params: {
+          :format => :json,
+          :api_client_authorization => {
+            :owner_uuid => users(:admin).uuid,
+            :expires_at => desired_expiration,
+          }
+        },
+        headers: {'HTTP_AUTHORIZATION' => "OAuth2 #{api_client_authorizations(:admin_trustedclient).api_token}"}
+      assert_response 200
+      if desired_expiration.nil?
+        assert json_response['expires_at'].nil?
+      else
+        assert_equal json_response['expires_at'].to_time.to_i, desired_expiration.to_i
+      end
+
+      # Test token update (reverse the above behavior)
+      previous_expiration = json_response['expires_at']
+      token_uuid = json_response['uuid']
+      if previous_expiration.nil?
+        desired_updated_expiration = Time.now + Rails.configuration.API.MaxTokenLifetime + 1.hour
+      else
+        desired_updated_expiration = nil
+      end
+      put "/arvados/v1/api_client_authorizations/#{token_uuid}",
+        params: {
+          :api_client_authorization => {
+            :expires_at => desired_updated_expiration,
+          }
+        },
+        headers: {'HTTP_AUTHORIZATION' => "OAuth2 #{api_client_authorizations(:admin_trustedclient).api_token}"}
+      assert_response 200
+      if desired_updated_expiration.nil?
+        assert json_response['expires_at'].nil?
+      else
+        assert_equal json_response['expires_at'].to_time.to_i, desired_updated_expiration.to_i
+      end
+    end
+  end
 end
diff --git a/services/api/test/integration/user_sessions_test.rb b/services/api/test/integration/user_sessions_test.rb
index fcc0ce4e5..6e951499a 100644
--- a/services/api/test/integration/user_sessions_test.rb
+++ b/services/api/test/integration/user_sessions_test.rb
@@ -53,19 +53,19 @@ class UserSessionsApiTest < ActionDispatch::IntegrationTest
   test 'existing user login' do
     mock_auth_with(identity_url: "https://active-user.openid.local")
     u = assigns(:user)
-    assert_equal 'zzzzz-tpzed-xurymjxw79nv3jz', u.uuid
+    assert_equal users(:active).uuid, u.uuid
   end
 
   test 'user redirect_to_user_uuid' do
     mock_auth_with(identity_url: "https://redirects-to-active-user.openid.local")
     u = assigns(:user)
-    assert_equal 'zzzzz-tpzed-xurymjxw79nv3jz', u.uuid
+    assert_equal users(:active).uuid, u.uuid
   end
 
   test 'user double redirect_to_user_uuid' do
     mock_auth_with(identity_url: "https://double-redirects-to-active-user.openid.local")
     u = assigns(:user)
-    assert_equal 'zzzzz-tpzed-xurymjxw79nv3jz', u.uuid
+    assert_equal users(:active).uuid, u.uuid
   end
 
   test 'create new user during omniauth callback' do

commit 1118101f84c013e4a9f8d33d1f2f9c072c6ff4aa
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Fri Jan 15 12:34:57 2021 -0300

    16736: Adds API.MaxTokenLifetime config knob.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 68e518732..2c0429edc 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -158,6 +158,13 @@ Clusters:
         dbname: ""
         SAMPLE: ""
     API:
+      # Limits for how long a client token created by regular users can be valid,
+      # and also is used as a default expiration policy when no expiration date is
+      # specified.
+      # Default value zero menans token expirations don't get clamped and no
+      # default expiration is set.
+      MaxTokenLifetime: 0s
+
       # Maximum size (in bytes) allowed for a single API request.  This
       # limit is published in the discovery document for use by clients.
       # Note: You must separately configure the upstream web server or
diff --git a/lib/config/export.go b/lib/config/export.go
index 3d0e27c72..b6531c59d 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -69,6 +69,7 @@ var whitelist = map[string]bool{
 	"API.MaxKeepBlobBuffers":                              false,
 	"API.MaxRequestAmplification":                         false,
 	"API.MaxRequestSize":                                  true,
+	"API.MaxTokenLifetime":                                false,
 	"API.RequestTimeout":                                  true,
 	"API.SendTimeout":                                     true,
 	"API.WebsocketClientEventQueue":                       false,
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 8ef787771..6135e2a3c 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -164,6 +164,13 @@ Clusters:
         dbname: ""
         SAMPLE: ""
     API:
+      # Limits for how long a client token created by regular users can be valid,
+      # and also is used as a default expiration policy when no expiration date is
+      # specified.
+      # Default value zero menans token expirations don't get clamped and no
+      # default expiration is set.
+      MaxTokenLifetime: 0s
+
       # Maximum size (in bytes) allowed for a single API request.  This
       # limit is published in the discovery document for use by clients.
       # Note: You must separately configure the upstream web server or
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 4a56c9302..4ccb1ef5d 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -86,6 +86,7 @@ type Cluster struct {
 		MaxKeepBlobBuffers             int
 		MaxRequestAmplification        int
 		MaxRequestSize                 int
+		MaxTokenLifetime               Duration
 		RequestTimeout                 Duration
 		SendTimeout                    Duration
 		WebsocketClientEventQueue      int

commit 9af58889fa57b1d81065aa3f202dac00767afb3a
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Thu Sep 10 11:11:33 2020 -0300

    16736: Enhances tests about login issued tokens.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/services/api/test/functional/user_sessions_controller_test.rb b/services/api/test/functional/user_sessions_controller_test.rb
index 129464cf1..1f9196893 100644
--- a/services/api/test/functional/user_sessions_controller_test.rb
+++ b/services/api/test/functional/user_sessions_controller_test.rb
@@ -30,6 +30,7 @@ class UserSessionsControllerTest < ActionController::TestCase
     authorize_with :inactive
     api_client_page = 'http://client.example.com/home'
     get :login, params: {return_to: api_client_page}
+    assert_response :redirect
     assert_not_nil assigns(:api_client)
     assert_nil assigns(:api_client_auth).expires_at
   end
@@ -40,6 +41,7 @@ class UserSessionsControllerTest < ActionController::TestCase
     authorize_with :inactive
     api_client_page = 'http://client.example.com/home'
     get :login, params: {return_to: api_client_page}
+    assert_response :redirect
     assert_not_nil assigns(:api_client)
     api_client_auth = assigns(:api_client_auth)
     assert_in_delta(api_client_auth.expires_at,

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list