[ARVADOS] created: 1.3.0-2901-g0371c7348

Git user git at public.arvados.org
Fri Aug 21 15:01:16 UTC 2020


        at  0371c7348509369fe338951b0df86325819622d5 (commit)


commit 0371c7348509369fe338951b0df86325819622d5
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Fri Aug 21 11:59:01 2020 -0300

    16678: Don't trust API clients from trusted URLs when TokenLifetime is set.
    
    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.rb b/services/api/app/models/api_client.rb
index 8ed693f82..c6c48a5b6 100644
--- a/services/api/app/models/api_client.rb
+++ b/services/api/app/models/api_client.rb
@@ -15,13 +15,16 @@ class ApiClient < ArvadosModel
   end
 
   def is_trusted
-    norm(self.url_prefix) == norm(Rails.configuration.Services.Workbench1.ExternalURL) ||
-      norm(self.url_prefix) == norm(Rails.configuration.Services.Workbench2.ExternalURL) ||
-      super
+    (from_trusted_url && Rails.configuration.Login.TokenLifetime == 0) || super
   end
 
   protected
 
+  def from_trusted_url
+    norm(self.url_prefix) == norm(Rails.configuration.Services.Workbench1.ExternalURL) ||
+      norm(self.url_prefix) == norm(Rails.configuration.Services.Workbench2.ExternalURL)
+  end
+
   def norm url
     # normalize URL for comparison
     url = URI(url)
diff --git a/services/api/test/unit/api_client_test.rb b/services/api/test/unit/api_client_test.rb
index df082c27f..93e4c51ab 100644
--- a/services/api/test/unit/api_client_test.rb
+++ b/services/api/test/unit/api_client_test.rb
@@ -7,25 +7,32 @@ require 'test_helper'
 class ApiClientTest < ActiveSupport::TestCase
   include CurrentApiClient
 
-  test "configured workbench is trusted" do
-    Rails.configuration.Services.Workbench1.ExternalURL = URI("http://wb1.example.com")
-    Rails.configuration.Services.Workbench2.ExternalURL = URI("https://wb2.example.com:443")
+  [true, false].each do |token_lifetime_enabled|
+    test "configured workbench is trusted when token lifetime is#{token_lifetime_enabled ? '': ' not'} enabled" do
+      Rails.configuration.Login.TokenLifetime = token_lifetime_enabled ? 8.hours : 0
+      Rails.configuration.Services.Workbench1.ExternalURL = URI("http://wb1.example.com")
+      Rails.configuration.Services.Workbench2.ExternalURL = URI("https://wb2.example.com:443")
 
-    act_as_system_user do
-      [["http://wb0.example.com", false],
-       ["http://wb1.example.com", true],
-       ["http://wb2.example.com", false],
-       ["https://wb2.example.com", true],
-       ["https://wb2.example.com/", true],
-      ].each do |pfx, result|
-        a = ApiClient.create(url_prefix: pfx, is_trusted: false)
-        assert_equal result, a.is_trusted
-      end
+      act_as_system_user do
+        [["http://wb0.example.com", false],
+        ["http://wb1.example.com", true],
+        ["http://wb2.example.com", false],
+        ["https://wb2.example.com", true],
+        ["https://wb2.example.com/", true],
+        ].each do |pfx, result|
+          a = ApiClient.create(url_prefix: pfx, is_trusted: false)
+          if token_lifetime_enabled
+            assert_equal false, a.is_trusted, "API client with url prefix '#{pfx}' shouldn't be trusted"
+          else
+            assert_equal result, a.is_trusted
+          end
+        end
 
-      a = ApiClient.create(url_prefix: "http://example.com", is_trusted: true)
-      a.save!
-      a.reload
-      assert a.is_trusted
+        a = ApiClient.create(url_prefix: "http://example.com", is_trusted: true)
+        a.save!
+        a.reload
+        assert a.is_trusted
+      end
     end
   end
 end

commit 929bf2ca704f43685c58da9fd60dceac037d3593
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Tue Aug 18 16:23:38 2020 -0300

    16678: Sets token expiration at login time. Disabled by default.
    
    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 24e5b71c6..91cd8b435 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -699,8 +699,9 @@ Clusters:
       RemoteTokenRefresh: 5m
 
       # How long a client token created from a login flow will be valid without
-      # asking the user to re-login.
-      TokenLifetime: 12h
+      # asking the user to re-login. Example values: 60m, 8h.
+      # Default value zero means tokens don't have expiration.
+      TokenLifetime: 0s
 
     Git:
       # Path to git or gitolite-shell executable. Each authenticated
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index e35318ff9..a2ff94c38 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -705,8 +705,9 @@ Clusters:
       RemoteTokenRefresh: 5m
 
       # How long a client token created from a login flow will be valid without
-      # asking the user to re-login.
-      TokenLifetime: 12h
+      # asking the user to re-login. Example values: 60m, 8h.
+      # Default value zero means tokens don't have expiration.
+      TokenLifetime: 0s
 
     Git:
       # Path to git or gitolite-shell executable. Each authenticated
diff --git a/services/api/app/controllers/user_sessions_controller.rb b/services/api/app/controllers/user_sessions_controller.rb
index 582b98cf2..8e3c3ac5e 100644
--- a/services/api/app/controllers/user_sessions_controller.rb
+++ b/services/api/app/controllers/user_sessions_controller.rb
@@ -147,10 +147,15 @@ class UserSessionsController < ApplicationController
         find_or_create_by(url_prefix: api_client_url_prefix)
     end
 
+    token_expiration = nil
+    if Rails.configuration.Login.TokenLifetime > 0
+      token_expiration = Time.now + Rails.configuration.Login.TokenLifetime
+    end
     @api_client_auth = ApiClientAuthorization.
       new(user: user,
           api_client: @api_client,
           created_by_ip_address: remote_ip,
+          expires_at: token_expiration,
           scopes: ["all"])
     @api_client_auth.save!
 
diff --git a/services/api/test/functional/user_sessions_controller_test.rb b/services/api/test/functional/user_sessions_controller_test.rb
index fc9475692..cd475dea4 100644
--- a/services/api/test/functional/user_sessions_controller_test.rb
+++ b/services/api/test/functional/user_sessions_controller_test.rb
@@ -14,7 +14,6 @@ class UserSessionsControllerTest < ActionController::TestCase
     assert_nil assigns(:api_client)
   end
 
-
   test "send token when user is already logged in" do
     authorize_with :inactive
     api_client_page = 'http://client.example.com/home'
@@ -26,6 +25,28 @@ class UserSessionsControllerTest < ActionController::TestCase
     assert_not_nil assigns(:api_client)
   end
 
+  test "login creates token without expiration by default" do
+    assert_equal Rails.configuration.Login.TokenLifetime, 0
+    authorize_with :inactive
+    api_client_page = 'http://client.example.com/home'
+    get :login, params: {return_to: api_client_page}
+    assert_not_nil assigns(:api_client)
+    assert_nil assigns(:api_client_auth).expires_at
+  end
+
+  test "login creates token with configured lifetime" do
+    token_lifetime = 1.hour
+    Rails.configuration.Login.TokenLifetime = token_lifetime
+    authorize_with :inactive
+    api_client_page = 'http://client.example.com/home'
+    get :login, params: {return_to: api_client_page}
+    assert_not_nil assigns(:api_client)
+    api_client_auth = assigns(:api_client_auth)
+    assert_in_delta(api_client_auth.expires_at,
+                    api_client_auth.updated_at + token_lifetime,
+                    1.second)
+  end
+
   test "login with remote param returns a salted token" do
     authorize_with :inactive
     api_client_page = 'http://client.example.com/home'

commit 2ea2aedaaadd0fcd204fe891df8b2ab56acf7dee
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Tue Aug 18 12:05:55 2020 -0300

    16678: Adds Login.TokenLifetime 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 a2a34448f..24e5b71c6 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -698,6 +698,10 @@ Clusters:
       # remain valid before it needs to be revalidated.
       RemoteTokenRefresh: 5m
 
+      # How long a client token created from a login flow will be valid without
+      # asking the user to re-login.
+      TokenLifetime: 12h
+
     Git:
       # Path to git or gitolite-shell executable. Each authenticated
       # request will execute this program with the single argument "http-backend"
diff --git a/lib/config/export.go b/lib/config/export.go
index d6b02b750..92389a73d 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -169,6 +169,7 @@ var whitelist = map[string]bool{
 	"Login.SSO.ProviderAppID":                      false,
 	"Login.SSO.ProviderAppSecret":                  false,
 	"Login.RemoteTokenRefresh":                     true,
+	"Login.TokenLifetime":                          false,
 	"Mail":                                         true,
 	"Mail.MailchimpAPIKey":                         false,
 	"Mail.MailchimpListID":                         false,
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index bddb5cedb..e35318ff9 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -704,6 +704,10 @@ Clusters:
       # remain valid before it needs to be revalidated.
       RemoteTokenRefresh: 5m
 
+      # How long a client token created from a login flow will be valid without
+      # asking the user to re-login.
+      TokenLifetime: 12h
+
     Git:
       # Path to git or gitolite-shell executable. Each authenticated
       # request will execute this program with the single argument "http-backend"
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 9cf1ed3cd..399f1103f 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -178,6 +178,7 @@ type Cluster struct {
 		}
 		LoginCluster       string
 		RemoteTokenRefresh Duration
+		TokenLifetime      Duration
 	}
 	Mail struct {
 		MailchimpAPIKey                string
diff --git a/services/api/config/arvados_config.rb b/services/api/config/arvados_config.rb
index 035a3972f..4f831160e 100644
--- a/services/api/config/arvados_config.rb
+++ b/services/api/config/arvados_config.rb
@@ -111,6 +111,7 @@ arvcfg.declare_config "Login.SSO.ProviderAppSecret", String, :sso_app_secret
 arvcfg.declare_config "Login.SSO.ProviderAppID", String, :sso_app_id
 arvcfg.declare_config "Login.LoginCluster", String
 arvcfg.declare_config "Login.RemoteTokenRefresh", ActiveSupport::Duration
+arvcfg.declare_config "Login.TokenLifetime", ActiveSupport::Duration
 arvcfg.declare_config "TLS.Insecure", Boolean, :sso_insecure
 arvcfg.declare_config "Services.SSO.ExternalURL", String, :sso_provider_url
 arvcfg.declare_config "AuditLogs.MaxAge", ActiveSupport::Duration, :max_audit_log_age

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list