[ARVADOS] updated: 2.1.0-847-gb42a11ea4

Git user git at public.arvados.org
Fri May 28 19:27:39 UTC 2021


Summary of changes:
 cmd/arvados-client/container_gateway.go            | 11 +++-
 doc/api/requests.html.textile.liquid               |  2 +-
 doc/api/tokens.html.textile.liquid                 |  3 +-
 doc/install/salt-multi-host.html.textile.liquid    | 16 ++++--
 doc/install/salt-single-host.html.textile.liquid   | 14 +++--
 lib/config/config.default.yml                      | 19 ++++++-
 lib/config/export.go                               |  4 +-
 lib/config/generated_config.go                     | 26 +++++++--
 lib/controller/auth_test.go                        |  2 +
 lib/controller/integration_test.go                 |  2 +
 lib/controller/localdb/login.go                    | 20 +++----
 lib/controller/localdb/login_oidc.go               | 63 ++++++++++++++++++----
 lib/controller/localdb/login_oidc_test.go          | 54 +++++++++++++++++--
 lib/install/deps.go                                |  1 +
 sdk/go/arvados/config.go                           |  3 ++
 sdk/go/arvadostest/oidc_provider.go                | 15 ++++--
 sdk/ruby/lib/arvados/google_api_client.rb          |  3 +-
 services/api/app/models/api_client.rb              |  2 +-
 .../api_client_authorizations_api_test.rb          | 14 ++---
 services/api/test/unit/api_client_test.rb          |  1 +
 services/keep-web/cache.go                         |  2 +-
 services/keep-web/handler_test.go                  | 56 +++++++++++++++++++
 tools/compute-images/build.sh                      |  3 ++
 tools/salt-install/provision.sh                    | 14 +++--
 24 files changed, 290 insertions(+), 60 deletions(-)

  discards  acc875d0675672aab58f6bde9ad7bab6841a62b6 (commit)
       via  b42a11ea4126c176edb290abb61721000c59f0d2 (commit)
       via  dfdddd21774a4937154e9af7769dcd30d96d9418 (commit)
       via  7aeb2366c284475c34764abc2dbed1367ef3bbc3 (commit)
       via  de8e379d5b7e4e0c130f9f94585f283095e325fa (commit)
       via  6c2913e20e2be88537bc5a78e61cb113c7a44750 (commit)
       via  b1d9ac8b550f03e39f2fad2a470b54f05b04d981 (commit)
       via  bfcade85c7aa6f88d25a002c271b3120d68f1120 (commit)
       via  116c96b2c709ed47490ddf05a66000fa3974359b (commit)
       via  07a1f9567e872fcc1df6ba23fa1f75d84848a15e (commit)
       via  ff1bf4d21d09879c2a6f37e997e911fd461e6f5b (commit)
       via  ba56503f90d099a215fb7375f5cb1cc1ac667e2c (commit)
       via  5e5fc4b5d73b70f416c6d08099a096619bb39f45 (commit)
       via  2ff5dd54e5daf4dfed3dfd07d161681fc87fe8ff (commit)
       via  7dd7506b850130fe2ad1a9ca09d31207ba949a23 (commit)
       via  e0b63c68db6c398aeb7a5820ac0ff5553d33bb40 (commit)
       via  89e9f940678b8f60166d3c2f7dd9be856bbc5557 (commit)
       via  8daccc2ab3f2178745d12bc54ec9a8d06d88864a (commit)
       via  e9ab3d864a8deb2810d7a49b0529a2aef0c0b41c (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (acc875d0675672aab58f6bde9ad7bab6841a62b6)
            \
             N -- N -- N (b42a11ea4126c176edb290abb61721000c59f0d2)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit b42a11ea4126c176edb290abb61721000c59f0d2
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Fri May 28 15:26:03 2021 -0400

    17722: Update tests for adjusted MaxTokenLifetime behavior
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 69fd11768..655e973c2 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -776,7 +776,7 @@ Clusters:
       # If false, tokens issued through login are not allowed to
       # viewing/creating other tokens.  New tokens can only be created
       # by going through login again.
-      TrustLoginTokens: true
+      IssueTrustedTokens: true
 
       # When the token is returned to a client, the token itself may
       # be restricted from viewing/creating other tokens based on whether
diff --git a/lib/config/export.go b/lib/config/export.go
index 8f98258bf..d1c71ed2d 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -180,7 +180,7 @@ var whitelist = map[string]bool{
 	"Login.Test.Enable":                                   true,
 	"Login.Test.Users":                                    false,
 	"Login.TokenLifetime":                                 false,
-	"Login.TrustLoginTokens":                              false,
+	"Login.IssueTrustedTokens":                            false,
 	"Login.TrustedClients":                                false,
 	"Mail":                                                true,
 	"Mail.EmailFrom":                                      false,
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index d9c9af027..0ae85461b 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -779,12 +779,13 @@ Clusters:
 
       # If true (default) tokens issued through login are allowed to create
       # new tokens.
-      # If false, tokens issued through login are not allowed to create new tokens,
-      # new tokens can only be created by going through login again.
-      TrustLoginTokens: true
+      # If false, tokens issued through login are not allowed to
+      # viewing/creating other tokens.  New tokens can only be created
+      # by going through login again.
+      IssueTrustedTokens: true
 
       # When the token is returned to a client, the token itself may
-      # be restricted from manipulating other tokens based on whether
+      # be restricted from viewing/creating other tokens based on whether
       # the client is "trusted" or not.  The local Workbench1 and
       # Workbench2 are trusted by default, but if this is a
       # LoginCluster, you probably want to include the other Workbench
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 65e2ff538..8149b9396 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -189,6 +189,7 @@ type Cluster struct {
 		RemoteTokenRefresh Duration
 		TokenLifetime      Duration
 		TrustedClients     map[string]struct{}
+		IssueTrustedTokens bool
 	}
 	Mail struct {
 		MailchimpAPIKey                string
diff --git a/services/api/app/models/api_client.rb b/services/api/app/models/api_client.rb
index 6ff29c83e..c914051a3 100644
--- a/services/api/app/models/api_client.rb
+++ b/services/api/app/models/api_client.rb
@@ -15,7 +15,7 @@ class ApiClient < ArvadosModel
   end
 
   def is_trusted
-    (from_trusted_url && Rails.configuration.Login.TrustLoginTokens) || super
+    (from_trusted_url && Rails.configuration.Login.IssueTrustedTokens) || super
   end
 
   protected
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 14e3bb361..a366664e0 100644
--- a/services/api/test/integration/api_client_authorizations_api_test.rb
+++ b/services/api/test/integration/api_client_authorizations_api_test.rb
@@ -139,7 +139,7 @@ class ApiClientAuthorizationsApiTest < ActionDispatch::IntegrationTest
         headers: {'HTTP_AUTHORIZATION' => "OAuth2 #{api_client_authorizations(:admin_trustedclient).api_token}"}
       assert_response 200
       if desired_expiration.nil?
-        assert json_response['expires_at'].nil?
+        assert_equal json_response['expires_at'].to_time.to_i, (db_current_time + Rails.configuration.API.MaxTokenLifetime).to_i
       else
         assert_equal json_response['expires_at'].to_time.to_i, desired_expiration.to_i
       end
@@ -148,22 +148,22 @@ class ApiClientAuthorizationsApiTest < ActionDispatch::IntegrationTest
       previous_expiration = json_response['expires_at']
       token_uuid = json_response['uuid']
       if previous_expiration.nil?
-        desired_updated_expiration = db_current_time + Rails.configuration.API.MaxTokenLifetime + 1.hour
+        submitted_updated_expiration = db_current_time + Rails.configuration.API.MaxTokenLifetime + 1.hour
       else
-        desired_updated_expiration = nil
+        submitted_updated_expiration = nil
       end
       put "/arvados/v1/api_client_authorizations/#{token_uuid}",
         params: {
           :api_client_authorization => {
-            :expires_at => desired_updated_expiration,
+            :expires_at => submitted_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?
+      if submitted_updated_expiration.nil?
+        assert_equal json_response['expires_at'].to_time.to_i, (db_current_time + Rails.configuration.API.MaxTokenLifetime).to_i
       else
-        assert_equal json_response['expires_at'].to_time.to_i, desired_updated_expiration.to_i
+        assert_equal json_response['expires_at'].to_time.to_i, submitted_updated_expiration.to_i
       end
     end
   end
diff --git a/services/api/test/unit/api_client_test.rb b/services/api/test/unit/api_client_test.rb
index bf47cd175..a0eacfd13 100644
--- a/services/api/test/unit/api_client_test.rb
+++ b/services/api/test/unit/api_client_test.rb
@@ -10,6 +10,7 @@ class ApiClientTest < ActiveSupport::TestCase
   [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.Login.IssueTrustedTokens = !token_lifetime_enabled;
       Rails.configuration.Services.Workbench1.ExternalURL = URI("http://wb1.example.com")
       Rails.configuration.Services.Workbench2.ExternalURL = URI("https://wb2.example.com:443")
       Rails.configuration.Login.TrustedClients = ActiveSupport::OrderedOptions.new

commit dfdddd21774a4937154e9af7769dcd30d96d9418
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Tue May 25 16:45:19 2021 -0400

    17722: Adjust token expiry behavior
    
    * Admins tokens have API.MaxTokenLifetime by default, but admins can create
      token which are not restricted by API.MaxTokenLifetime.
    
    * Login.TokenLifetime does no longer implies untrusted tokens
    
    * Added Login.TrustLoginToken to determine whether tokens issued by login are
      considered "trusted"
    
    * Updated documentation page
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/doc/admin/token-expiration-policy.html.textile.liquid b/doc/admin/token-expiration-policy.html.textile.liquid
index 9d84bf666..c71d86c47 100644
--- a/doc/admin/token-expiration-policy.html.textile.liquid
+++ b/doc/admin/token-expiration-policy.html.textile.liquid
@@ -56,7 +56,18 @@ Clusters:
 
 This is independent of @Workbench.IdleTimeout at .  Even if Workbench auto-logout is disabled, this option will ensure that the user is always required to log in again after the configured amount of time.
 
-When this configuration is active (has a nonzero value), the Workbench client will also be "untrusted" by default.  This means tokens issued to Workbench cannot be used to list other tokens issued to the user, and cannot be used to grant new tokens.  This stops an attacker from leveraging a leaked token to aquire other tokens, but also interferes with some Workbench features that create new tokens on behalf of the user.
+h2. Untrusted login tokens
+
+<pre>
+Clusters:
+  zzzzz:
+    ...
+    Login:
+      TrustLoginTokens: false
+    ...
+</pre>
+
+When `TrustLoginTokens` is `false`, tokens issued through login will be "untrusted" by default.  Untrusted tokens cannot be used to list other tokens issued to the user, and cannot be used to grant new tokens.  This stops an attacker from leveraging a leaked token to aquire other tokens, but also interferes with some Workbench features that create new tokens on behalf of the user.
 
 The default value @Login.TokenLifetime@ is zero, meaning login tokens do not expire (unless @API.MaxTokenLifetime@ is set).
 
@@ -73,25 +84,25 @@ Clusters:
     ...
 </pre>
 
-Tokens created without an explicit expiration time, or that exceed maximum lifetime, will be clamped to @API.MaxTokenLifetime at .
+Tokens created without an explicit expiration time, or that exceed maximum lifetime, will be set to @API.MaxTokenLifetime at .
 
 Similar to @Login.TokenLifetime@, this option ensures that the user is always required to log in again after the configured amount of time.
 
-Unlike @Login.TokenLifetime@, this applies to all API operations that manipulate tokens, regardless of whether the token was created by logging in, or by using the API.  Also unlike @Login.TokenLifetime@, this setting does not imply any additional restrictions on token capabilities (it does not interfere with Workbench features that create new tokens on behalf of the user).  If @Login.TokenLifetime@ is greater than @API.MaxTokenLifetime@, MaxTokenLifetime takes precedence.
+Unlike @Login.TokenLifetime@, this applies to all API operations that manipulate tokens, regardless of whether the token was created by logging in, or by using the API.  If @Login.TokenLifetime@ is greater than @API.MaxTokenLifetime@, MaxTokenLifetime takes precedence.
 
-Admin users are permitted to create tokens with expiration times further in the future than MaxTokenLifetime, or with no expiration time at all.
+Admin users are permitted to create tokens with expiration times further in the future than @MaxTokenLifetime at .
 
 The default value @MaxTokenLifetime@ is zero, which means there is no maximum token lifetime.
 
 h2. Choosing a policy
 
- at Workbench.IdleTimeout@ only affects browser behavior.  It is strongly recommended that automatic browser logout be used together with one or both token lifetime options, which are enforced on API side.
+ at Workbench.IdleTimeout@ only affects browser behavior.  It is strongly recommended that automatic browser logout be used together with @Login.TokenLifetime@, which is enforced on API side.
 
- at Login.TokenLifetime@ is more restrictive.  A token obtained by logging into Workbench cannot be "refreshed" to gain access for an indefinite period.  However, it interferes with some Workbench features, as well as ease of use in other contexts, such as the Arvados command line.  This option is recommended only if most users will only ever interact with the system through Workbench or WebShell.  For users or service accounts that need to tokens with fewer restrictions, the admin can "create a token at the command line":user-management-cli.html#create-token .
+ at TrustLoginTokens: true@ (default value) is less restrictive.  Be aware that an unrestricted token can be "refreshed" to gain access for an indefinite period.  This means, during the window that the token is valid, the user is permitted to create a new token, which will have a new expiration further in the future (of course, once the token has expired, this is no longer possible).  Unrestricted tokens are required for some Workbench features, as well as ease of use in other contexts, such as the Arvados command line.  This option is recommended if many users will interact with the system through the command line.
 
- at API.MaxTokenLifetime@ is less restrictive.  Be aware that an unrestricted token can be "refreshed" to gain access for an indefinite period.  This means, during the window that the token is valid, the user is permitted to create a new token, which will have a new expiration further in the future (of course, once the token has expired, this is no longer possible).  Unrestricted tokens are required for some Workbench features, as well as ease of use in other contexts, such as the Arvados command line.  This option is recommended if many users will interact with the system through the command line.
+ at TrustLoginTokens: false@ is more restrictive.  A token obtained by logging into Workbench cannot be "refreshed" to gain access for an indefinite period.  However, it interferes with some Workbench features, as well as ease of use in other contexts, such as the Arvados command line.  This option is recommended only if most users will only ever interact with the system through Workbench or WebShell.  For users or service accounts that need to tokens with fewer restrictions, the admin can "create a token at the command line":user-management-cli.html#create-token using the @SystemRootToken at .
 
-In every case, admin users may always create tokens with no expiration date.
+In every case, admin users may always create tokens with expiration dates far in the future.
 
 These policies do not apply to tokens created by the API server for the purposes of authorizing a container to run, as those tokens are automatically expired when the container is finished.
 
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 8ad2cb53f..69fd11768 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -771,8 +771,15 @@ Clusters:
       # Default value zero means tokens don't have expiration.
       TokenLifetime: 0s
 
+      # If true (default) tokens issued through login are allowed to create
+      # new tokens.
+      # If false, tokens issued through login are not allowed to
+      # viewing/creating other tokens.  New tokens can only be created
+      # by going through login again.
+      TrustLoginTokens: true
+
       # When the token is returned to a client, the token itself may
-      # be restricted from manipulating other tokens based on whether
+      # be restricted from viewing/creating other tokens based on whether
       # the client is "trusted" or not.  The local Workbench1 and
       # Workbench2 are trusted by default, but if this is a
       # LoginCluster, you probably want to include the other Workbench
diff --git a/lib/config/export.go b/lib/config/export.go
index 890d4ce47..8f98258bf 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -180,6 +180,7 @@ var whitelist = map[string]bool{
 	"Login.Test.Enable":                                   true,
 	"Login.Test.Users":                                    false,
 	"Login.TokenLifetime":                                 false,
+	"Login.TrustLoginTokens":                              false,
 	"Login.TrustedClients":                                false,
 	"Mail":                                                true,
 	"Mail.EmailFrom":                                      false,
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 9e59f8c92..d9c9af027 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -777,6 +777,12 @@ Clusters:
       # Default value zero means tokens don't have expiration.
       TokenLifetime: 0s
 
+      # If true (default) tokens issued through login are allowed to create
+      # new tokens.
+      # If false, tokens issued through login are not allowed to create new tokens,
+      # new tokens can only be created by going through login again.
+      TrustLoginTokens: true
+
       # When the token is returned to a client, the token itself may
       # be restricted from manipulating other tokens based on whether
       # the client is "trusted" or not.  The local Workbench1 and
diff --git a/services/api/app/models/api_client.rb b/services/api/app/models/api_client.rb
index 015b61dc4..6ff29c83e 100644
--- a/services/api/app/models/api_client.rb
+++ b/services/api/app/models/api_client.rb
@@ -15,7 +15,7 @@ class ApiClient < ArvadosModel
   end
 
   def is_trusted
-    (from_trusted_url && Rails.configuration.Login.TokenLifetime == 0) || super
+    (from_trusted_url && Rails.configuration.Login.TrustLoginTokens) || super
   end
 
   protected
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 7e7140369..52f2cee06 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -406,9 +406,9 @@ class ApiClientAuthorization < ArvadosModel
   protected
 
   def clamp_token_expiration
-    if !current_user.andand.is_admin && Rails.configuration.API.MaxTokenLifetime > 0
+    if Rails.configuration.API.MaxTokenLifetime > 0
       max_token_expiration = db_current_time + Rails.configuration.API.MaxTokenLifetime
-      if (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 && !current_user.andand.is_admin))
         self.expires_at = max_token_expiration
       end
     end

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list