[arvados] created: 2.7.1-45-g4afabaf571

[git repository hosting]

        at  4afabaf571ef186cb16408f31f57150f8b6675fa (commit)


commit 4afabaf571ef186cb16408f31f57150f8b6675fa
Author: Tom Clegg <tom at curii.com>
Date:   Mon Apr 1 17:51:43 2024 -0400

    21613: Fix handling of expired token re-validating with new UUID.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 8aeaf2f9cb..a27f1028e6 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -403,8 +403,17 @@ class ApiClientAuthorization < ArvadosModel
         end
       rescue ActiveRecord::RecordNotUnique
         Rails.logger.debug("cached remote token #{token_uuid} already exists, retrying...")
-        # Some other request won the race: retry just once before erroring out
-        if (retries += 1) <= 1
+        # Another request won the race (trying to find_or_create the
+        # same token UUID) ...and/or... there is an expired entry with
+        # the same secret but a different UUID (e.g., the token is an
+        # OIDC access token and [a] our database has an expired cached
+        # row that was not used above, and [b] the remote cluster had
+        # deleted its expired cached row so it assigned a new UUID).
+        #
+        # Delete any conflicting row if any. Retry twice (in case we
+        # hit both of those situations at once), then give up.
+        if (retries += 1) <= 2
+          ApiClientAuthorization.where('api_token=? and uuid<>?', stored_secret, token_uuid).delete_all
           retry
         else
           Rails.logger.warn("cannot find or create cached remote token #{token_uuid}")
diff --git a/services/api/test/integration/remote_user_test.rb b/services/api/test/integration/remote_user_test.rb
index 1a67522f4d..98250a6242 100644
--- a/services/api/test/integration/remote_user_test.rb
+++ b/services/api/test/integration/remote_user_test.rb
@@ -75,7 +75,7 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
         res.status = @stub_token_status
         if res.status == 200
           body = {
-            uuid: api_client_authorizations(:active).uuid.sub('zzzzz', clusterid),
+            uuid: @stub_token_uuid || api_client_authorizations(:active).uuid.sub('zzzzz', clusterid),
             owner_uuid: "#{clusterid}-tpzed-00000000000000z",
             scopes: @stub_token_scopes,
           }
@@ -108,6 +108,7 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
     }
     @stub_token_status = 200
     @stub_token_scopes = ["all"]
+    @stub_token_uuid = nil
     ActionMailer::Base.deliveries = []
   end
 
@@ -241,6 +242,40 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest
     assert_equal 'foo', json_response['username']
   end
 
+  test 'authenticate with remote token with secret part identical to previously cached token' do
+    get '/arvados/v1/users/current',
+      params: {format: 'json'},
+      headers: auth(remote: 'zbbbb')
+    assert_response :success
+    get '/arvados/v1/api_client_authorizations/current',
+      params: {format: 'json'},
+      headers: auth(remote: 'zbbbb')
+    assert_response :success
+
+    # Expire the cached token.
+    @cached_token_uuid = json_response['uuid']
+    act_as_system_user do
+      ApiClientAuthorization.where(uuid: @cached_token_uuid).update_all(expires_at: db_current_time() - 1.day)
+    end
+
+    # Now use the same bare token, but set up the remote cluster to
+    # return a different UUID this time.
+    @stub_token_uuid = 'zbbbb-gj3su-123451234512345'
+    get '/arvados/v1/users/current',
+      params: {format: 'json'},
+      headers: auth(remote: 'zbbbb')
+    assert_response :success
+
+    # Confirm that we actually retrieved the new UUID from the stub
+    # cluster -- otherwise we didn't really test the conflicting-UUID
+    # case.
+    get '/arvados/v1/api_client_authorizations/current',
+      params: {format: 'json'},
+      headers: auth(remote: 'zbbbb')
+    assert_response :success
+    assert_equal @stub_token_uuid, json_response['uuid']
+  end
+
   test 'authenticate with remote token from misbehaving remote cluster' do
     get '/arvados/v1/users/current',
       params: {format: 'json'},

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


hooks/post-receive
--