From a74d5b45869e9fc916ed798082c5e48f5fc5fb62 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Wed, 14 Aug 2024 09:15:07 -0400 Subject: [PATCH] 21910: Trust system root token even if IssueTrustedTokens == false. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- ...oken-expiration-policy.html.textile.liquid | 2 +- .../api_client_authorizations_controller.rb | 3 ++- ...i_client_authorizations_controller_test.rb | 3 +++ .../api_client_authorizations_api_test.rb | 27 ++++++++++--------- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/doc/admin/token-expiration-policy.html.textile.liquid b/doc/admin/token-expiration-policy.html.textile.liquid index 88c569333d..ece09b0b38 100644 --- a/doc/admin/token-expiration-policy.html.textile.liquid +++ b/doc/admin/token-expiration-policy.html.textile.liquid @@ -100,7 +100,7 @@ h2. Choosing a policy @IssueTrustedTokens: 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. -@IssueTrustedTokens: 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@. +@IssueTrustedTokens: 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 issue tokens with fewer restrictions, the admin can "create a token at the command line":user-management-cli.html#create-token using the @SystemRootToken@. In every case, admin users may always create tokens with expiration dates far in the future. 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 3d9c52ae0a..9fe7aab035 100644 --- a/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb +++ b/services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb @@ -144,11 +144,12 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController end def check_issue_trusted_tokens + return true if current_api_client_authorization.andand.api_token == Rails.configuration.SystemRootToken return forbidden if !Rails.configuration.Login.IssueTrustedTokens end def forbidden - send_error('Forbidden: this API client cannot manipulate other clients\' access tokens.', + send_error('Action prohibited by IssueTrustedTokens configuration.', status: 403) end end diff --git a/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb b/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb index 6535738348..5312b61994 100644 --- a/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb +++ b/services/api/test/functional/arvados/v1/api_client_authorizations_controller_test.rb @@ -84,6 +84,9 @@ class Arvados::V1::ApiClientAuthorizationsControllerTest < ActionController::Tes # cannot look up other tokens for other users [:admin_trustedclient, :active, 404, 200, 0], [:active_trustedclient, :admin, 404, 200, 0], + # system root token is always trusted + [:system_user, :active, 200, 200, 1], + [:system_user, :admin, 200, 200, 1], ].each do |auth_token, target_token, expect_get_response, expect_list_response, expect_list_items| test "using '#{auth_token}', get '#{target_token}' by uuid" do authorize_with auth_token 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 b477c66fe0..f6bf232de8 100644 --- a/services/api/test/integration/api_client_authorizations_api_test.rb +++ b/services/api/test/integration/api_client_authorizations_api_test.rb @@ -16,15 +16,16 @@ class ApiClientAuthorizationsApiTest < ActionDispatch::IntegrationTest assert_response :success end - [:admin_trustedclient, :SystemRootToken].each do |tk| - test "create token for different user using #{tk}" do - if tk == :SystemRootToken - token = "xyzzy-SystemRootToken" - Rails.configuration.SystemRootToken = token - else - token = api_client_authorizations(tk).api_token - end - + [ + [true, :active, 403], + [true, :admin, 200], + [true, :system_user, 200], + [false, :active, 403], + [false, :admin, 403], + [false, :system_user, 200], + ].each do |issue_trusted_tokens, tk, expect_response| + test "create token for different user using #{tk} with IssueTrustedTokens=#{issue_trusted_tokens}" do + Rails.configuration.Login.IssueTrustedTokens = issue_trusted_tokens post "/arvados/v1/api_client_authorizations", params: { :format => :json, @@ -32,12 +33,14 @@ class ApiClientAuthorizationsApiTest < ActionDispatch::IntegrationTest :owner_uuid => users(:spectator).uuid } }, - headers: {'HTTP_AUTHORIZATION' => "OAuth2 #{token}"} - assert_response :success + headers: {'HTTP_AUTHORIZATION' => "Bearer #{api_client_authorizations(tk).api_token}"} + + assert_response expect_response + return if expect_response >= 300 get "/arvados/v1/users/current", params: {:format => :json}, - headers: {'HTTP_AUTHORIZATION' => "OAuth2 #{json_response['api_token']}"} + headers: {'HTTP_AUTHORIZATION' => "Bearer #{json_response['api_token']}"} @json_response = nil assert_equal json_response['uuid'], users(:spectator).uuid end -- 2.30.2