Merge branch 'master' into 16678-login-tokens-lifetime-config

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

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index a1b471bd229e7f27b0cbd90bf79919f0d7123992..c392d8638d88d424cb41b8b7b748fda934d5f8d6 100644 (file)
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -698,6 +698,11 @@ 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. 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
       # request will execute this program with the single argument "http-backend"

diff --git a/lib/config/export.go b/lib/config/export.go
index f15a2996197804c24face6be71fc4f97afb558f4..57bea78f056b72676de30177c7d48d5e86964408 100644 (file)
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -170,6 +170,8 @@ var whitelist = map[string]bool{
        "Login.SSO.Enable":                             true,
        "Login.SSO.ProviderAppID":                      false,
        "Login.SSO.ProviderAppSecret":                  false,
+       "Login.RemoteTokenRefresh":                     true,
+       "Login.TokenLifetime":                          false,
        "Mail":                                         true,
        "Mail.EmailFrom":                               false,
        "Mail.IssueReporterEmailFrom":                  false,

diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 8e42eb350516d172cec46c99fc0c163dcaa4fb46..f5004667b23f077750046cd7d0a567832e4e47a8 100644 (file)
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -704,6 +704,11 @@ 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. 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
       # 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 41c20c8db2ee71cf4c4a024e7d1d73b72878a098..86673320da3c41fc394fc8381e0221a47fd2e436 100644 (file)
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -179,6 +179,7 @@ type Cluster struct {
                }
                LoginCluster       string
                RemoteTokenRefresh Duration
+               TokenLifetime      Duration
        }
        Mail struct {
                MailchimpAPIKey                string

diff --git a/services/api/app/controllers/user_sessions_controller.rb b/services/api/app/controllers/user_sessions_controller.rb
index 582b98cf2dc9d9e20b88cf0180b7a9db19fbfd8f..8e3c3ac5e3d8b8656d587e86626f86f57c33b045 100644 (file)
--- 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/app/models/api_client.rb b/services/api/app/models/api_client.rb
index 8ed693f820d5eac0eff9389ac851166e800d6516..c6c48a5b6b13c803d8d54d660a2d8fbd2a265740 100644 (file)
--- 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/config/arvados_config.rb b/services/api/config/arvados_config.rb
index 035a3972f86c318e758318330c7aa63af44ff9c5..4f831160e9351790143505cc16447e6d0507b0e3 100644 (file)
--- 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

diff --git a/services/api/test/functional/user_sessions_controller_test.rb b/services/api/test/functional/user_sessions_controller_test.rb
index fc9475692a5933c2ed01f77e7871f4fd3942d7ec..cd475dea4d1849f6d99374fa3976068767ef1fcb 100644 (file)
--- 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'

diff --git a/services/api/test/unit/api_client_test.rb b/services/api/test/unit/api_client_test.rb
index df082c27fd8c35f7a8d1011bcd3faeba3d4bd4d8..93e4c51abf0e9266a1883ab18bf5634ea45722b0 100644 (file)
--- 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