[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