[#57052] Add optimistic locking to oauth_client_tokens (#16376)

https://community.openproject.org/work_packages/57052

It prevents a race condition update of access/refresh tokens.
ActiveRecord::StaleObjectError is raised then and udpate is
prevented.

app/services/oauth_clients/connection_manager.rb

@@ -33,7 +33,6 @@
 
 module OAuthClients
   class ConnectionManager
-    # Nextcloud API endpoint to check if Bearer token is valid
     TOKEN_IS_FRESH_DURATION = 10.seconds.freeze
 
     def initialize(user:, configuration:)

db/migrate/20240808133947_add_lock_version_to_oauth_client_tokens.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal:true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2024 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+class AddLockVersionToOAuthClientTokens < ActiveRecord::Migration[7.1]
+  def change
+    add_column :oauth_client_tokens, :lock_version, :integer, default: 0, null: false
+  end
+end

modules/storages/app/common/storages/peripherals/storage_interaction/authentication_strategies/oauth_user_token.rb

@@ -39,6 +39,7 @@ def self.strategy
 
           def initialize(user)
             @user = user
+            @retried_after_stale_object_update = false
           end
 
           # rubocop:disable Metrics/AbcSize
@@ -56,6 +57,12 @@ def call(storage:, http_options: {}, &)
 
               refresh_and_retry(httpx_oauth_config, http_options, token.result, &)
             end
+          rescue ActiveRecord::StaleObjectError => e
+            raise e if @retried_after_stale_object_update
+
+            @retried_after_stale_object_update = true
+            Rails.logger.error("#{e.inspect} happend for User ##{@user.id} #{@user.name}")
+            retry
           end
 
           # rubocop:enable Metrics/AbcSize

modules/storages/spec/common/storages/peripherals/storage_interaction/authentication_spec.rb

@@ -131,6 +131,8 @@
       end
 
       context "with invalid oauth refresh token", vcr: "auth/nextcloud/user_token_refresh_token_invalid" do
+        before { storage }
+
         it "must return unauthorized" do
           result = described_class[auth_strategy].call(storage:, http_options:) { |http| make_request(http) }
           expect(result).to be_failure
@@ -140,6 +142,23 @@
           expect(error.data.source)
             .to be(Storages::Peripherals::StorageInteraction::AuthenticationStrategies::OAuthUserToken)
         end
+
+        it "logs, retries once, raises exception if race condition happens" do
+          token = OAuthClientToken.last
+          strategy = described_class[auth_strategy]
+
+          allow(Rails.logger).to receive(:error)
+          allow(strategy).to receive(:current_token).and_return(ServiceResult.success(result: token))
+          allow(token).to receive(:destroy).and_raise(ActiveRecord::StaleObjectError).twice
+
+          expect do
+            strategy.call(storage:, http_options:) { |http| make_request(http) }
+          end.to raise_error(ActiveRecord::StaleObjectError)
+
+          expect(Rails.logger)
+            .to have_received(:error)
+            .with("#<ActiveRecord::StaleObjectError: Stale object error.> happend for User ##{user.id} #{user.name}").once
+        end
       end
 
       context "with invalid oauth access token", vcr: "auth/nextcloud/user_token_access_token_invalid" do

spec/support/vcr.rb

@@ -68,7 +68,11 @@ def stub_request_with_timeout(method, path_matcher)
     end
   end
 
-  config.default_cassette_options = { record: ENV.fetch("VCR_RECORD_MODE", :once).to_sym, drop_unused_requests: true }
+  config.default_cassette_options = {
+    record: ENV.fetch("VCR_RECORD_MODE", :once).to_sym,
+    allow_playback_repeats: true,
+    drop_unused_requests: true
+  }
 end
 
 VCR.turn_off!