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@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 59e359232e834fbeb1f12a9c6daec6c52168debd..99446688db338f54b0d0ba353c66e2dcefdcd26c 100644 (file)
--- 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 9290e01a1a7a5b4284580615585d963a5201c386..4218645d5daf3014369f693ea7faa68fed9546dd 100644 (file)
--- 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 5327713f699e58771ef4060e4a71a1554ab4b87d..8f4395dada2c226149ae221db7cc10ba70249f86 100644 (file)
--- 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 296ab8a2ff4169167d8c85bffcdab34f67f078e5..3a7b201312b341e158814b082492dcbedbeee2c3 100644 (file)
--- 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 fcc0ce4e5266b5b032d997535a72cf86d3382cbf..6e951499adfc173d7653376500a49e1f5a49a8e3 100644 (file)
--- 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