diff --git a/app/models/access_token.rb b/app/models/access_token.rb index 7cdaeb0cb3..2f158ff18a 100644 --- a/app/models/access_token.rb +++ b/app/models/access_token.rb @@ -145,8 +145,25 @@ def self.find_from_id_or_value(id_or_value) # This can't change or it will create new tokens for everyone OIDC_SYNC_TOKEN = 'OIDC Synchronization Token'.freeze + # nil covers legacy tokens created before expiration was added; can be removed after a release + scope :stale, -> { where(name: OIDC_SYNC_TOKEN, expires_at: [...Time.now.utc, nil]) } + def self.oidc_sync - create_with(scopes: %w[account_management], permission: 'ro').find_or_create_by!(name: OIDC_SYNC_TOKEN) + user_id = scope_attributes["owner_id"] + cache_key = "access_tokens/user:#{user_id}/oidc" + + # Hot path: skip the transaction entirely on cache hit (zero DB queries). + cached = Rails.cache.read(cache_key) + return cached if cached + + transaction do + # Lock to serialize concurrent cache misses (e.g. full resync). + lock.where(name: OIDC_SYNC_TOKEN).load + + Rails.cache.fetch(cache_key, expires_in: 1.hour) do + create!(name: OIDC_SYNC_TOKEN, scopes: %w[account_management], permission: 'ro', expires_at: 1.day.from_now.utc.iso8601).plaintext_value + end + end end def scopes=(values) diff --git a/app/workers/janitor_worker.rb b/app/workers/janitor_worker.rb index a2e09bdc42..3d7502b52d 100644 --- a/app/workers/janitor_worker.rb +++ b/app/workers/janitor_worker.rb @@ -8,7 +8,7 @@ def perform PurgeOldUserSessionsWorker.perform_async PurgeStaleObjectsWorker.perform_later(EventStore::Event.name, DeletedObject.name) - DeleteAllStaleObjectsWorker.perform_later(MessageRecipient.name, Message.name) + DeleteAllStaleObjectsWorker.perform_later(MessageRecipient.name, Message.name, AccessToken.name) true end diff --git a/app/workers/zync_worker.rb b/app/workers/zync_worker.rb index 5ff32e6acb..56ca965a2b 100644 --- a/app/workers/zync_worker.rb +++ b/app/workers/zync_worker.rb @@ -175,7 +175,7 @@ def provider_endpoint(provider) def self.provider_access_token(provider) user = provider.find_impersonation_admin || provider.first_admin! - user.access_tokens.oidc_sync.value + user.access_tokens.oidc_sync end def notification_url diff --git a/test/models/access_token_test.rb b/test/models/access_token_test.rb index 34f3b29c6f..8bc087f359 100644 --- a/test/models/access_token_test.rb +++ b/test/models/access_token_test.rb @@ -312,6 +312,146 @@ def test_find_from_value_rejects_leaked_hash_as_token assert access_token.valid? end + # oidc_sync tests + + class RefreshOidcSyncTest < ActiveSupport::TestCase + setup do + @original_cache = Rails.cache + Rails.cache = ActiveSupport::Cache::MemoryStore.new + @provider = FactoryBot.create(:simple_provider) + @user = FactoryBot.create(:admin, account: @provider) + end + + teardown do + Rails.cache.clear + Rails.cache = @original_cache + end + + test 'returns a plaintext string' do + result = @user.access_tokens.oidc_sync + + assert_instance_of String, result + assert_equal 96, result.length + assert_match(/\A[0-9a-f]+\z/, result) + end + + test 'creates an OIDC sync token in DB whose digest matches the returned plaintext' do + plaintext = @user.access_tokens.oidc_sync + + token = @user.access_tokens.find_by!(name: AccessToken::OIDC_SYNC_TOKEN) + assert_equal AccessToken.compute_digest(plaintext), token.read_attribute(:value) + end + + test 'second call returns same plaintext from cache without rotating' do + first = @user.access_tokens.oidc_sync + second = @user.access_tokens.oidc_sync + + assert_equal first, second + assert_equal 1, @user.access_tokens.where(name: AccessToken::OIDC_SYNC_TOKEN).count + end + + test 'returns a new plaintext after cache expires and does not modify old tokens' do + first = @user.access_tokens.oidc_sync + old_token = @user.access_tokens.find_by!(name: AccessToken::OIDC_SYNC_TOKEN) + old_expires_at = old_token.expires_at + + travel_to(1.hour.from_now + 1.second) do + second = @user.access_tokens.oidc_sync + + assert_not_equal first, second + assert_equal old_expires_at, old_token.reload.expires_at, "Old token should not be modified" + assert_equal 2, @user.access_tokens.where(name: AccessToken::OIDC_SYNC_TOKEN).count + end + end + + test 'new tokens are created with a 1-day expiration' do + @user.access_tokens.oidc_sync + + token = @user.access_tokens.find_by!(name: AccessToken::OIDC_SYNC_TOKEN) + assert token.expires_at.present? + assert token.expires_at > Time.now.utc, "Token should not be expired yet" + assert token.expires_at < Time.now.utc + 1.day + 1.minute, "Token should expire within ~1 day" + end + + test 'expired tokens remain valid for authentication' do + plaintext = @user.access_tokens.oidc_sync + + travel_to(1.hour.from_now + 1.second) do + @user.access_tokens.oidc_sync + + old_token = AccessToken.find_from_value(plaintext) + assert_not_nil old_token, "Old token should still be findable by plaintext" + assert_not old_token.expired?, "Old token should not be expired yet (expires in ~1 day)" + end + end + + test 'works on first call with no existing tokens' do + assert_equal 0, @user.access_tokens.where(name: AccessToken::OIDC_SYNC_TOKEN).count + + result = @user.access_tokens.oidc_sync + + assert_instance_of String, result + assert_equal 1, @user.access_tokens.where(name: AccessToken::OIDC_SYNC_TOKEN).count + end + + test 'tokens from different users are independent' do + other_provider = FactoryBot.create(:simple_provider) + other_user = FactoryBot.create(:admin, account: other_provider) + + plaintext_a = @user.access_tokens.oidc_sync + plaintext_b = other_user.access_tokens.oidc_sync + + assert_not_equal plaintext_a, plaintext_b + assert_equal 1, @user.access_tokens.where(name: AccessToken::OIDC_SYNC_TOKEN).count + assert_equal 1, other_user.access_tokens.where(name: AccessToken::OIDC_SYNC_TOKEN).count + end + + test 'returns cached value without DB queries on cache hit' do + @user.access_tokens.oidc_sync + + assert_number_of_queries(0) { @user.access_tokens.oidc_sync } + end + end + + # stale scope tests + + test 'stale scope returns only expired OIDC sync tokens' do + provider = FactoryBot.create(:simple_provider) + user = FactoryBot.create(:admin, account: provider) + + expired_token = FactoryBot.create(:access_token, owner: user, name: AccessToken::OIDC_SYNC_TOKEN, + scopes: %w[account_management], permission: 'ro') + expired_token.update_columns(expires_at: 1.hour.ago) + + active_token = FactoryBot.create(:access_token, owner: user, name: "#{AccessToken::OIDC_SYNC_TOKEN} 2", + scopes: %w[account_management], permission: 'ro') + active_token.update_columns(expires_at: 1.day.from_now) + + other_token = FactoryBot.create(:access_token, owner: user) + other_token.update_columns(expires_at: 1.hour.ago) + + scope_ids = AccessToken.stale.pluck(:id) + assert_includes scope_ids, expired_token.id + assert_not_includes scope_ids, active_token.id + assert_not_includes scope_ids, other_token.id + end + + test 'stale scope includes OIDC sync tokens with nil expires_at (legacy tokens)' do + provider = FactoryBot.create(:simple_provider) + user = FactoryBot.create(:admin, account: provider) + + legacy_token = FactoryBot.create(:access_token, owner: user, name: AccessToken::OIDC_SYNC_TOKEN, + scopes: %w[account_management], permission: 'ro') + legacy_token.update_columns(expires_at: nil) + + other_legacy_token = FactoryBot.create(:access_token, owner: user) + other_legacy_token.update_columns(expires_at: nil) + + scope_ids = AccessToken.stale.pluck(:id) + assert_includes scope_ids, legacy_token.id + assert_not_includes scope_ids, other_legacy_token.id + end + private def assert_access_token_audit_all_data(access_token, audit) diff --git a/test/workers/janitor_worker_test.rb b/test/workers/janitor_worker_test.rb index a1c10c483b..6908c04ab5 100644 --- a/test/workers/janitor_worker_test.rb +++ b/test/workers/janitor_worker_test.rb @@ -22,6 +22,6 @@ def test_not_enabled def mock_purge_workers(called_times:) PurgeOldUserSessionsWorker.expects(:perform_async).send(called_times) PurgeStaleObjectsWorker.expects(:perform_later).send(called_times).with(EventStore::Event.name, DeletedObject.name) - DeleteAllStaleObjectsWorker.expects(:perform_later).send(called_times).with(MessageRecipient.name, Message.name) + DeleteAllStaleObjectsWorker.expects(:perform_later).send(called_times).with(MessageRecipient.name, Message.name, AccessToken.name) end end