From 2523865b224bea03cba0e781655f4a41277c8234 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Tue, 1 Sep 2026 10:50:27 -0700 Subject: [PATCH 01/18] Add test skeleton --- .../spec/aws/refreshing_credentials_spec.rb | 68 ++++ .../spec/aws/refreshing_credentials_test.json | 373 ++++++++++++++++++ 2 files changed, 441 insertions(+) create mode 100644 gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb create mode 100644 gems/aws-sdk-core/spec/aws/refreshing_credentials_test.json diff --git a/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb new file mode 100644 index 00000000000..88246e2340f --- /dev/null +++ b/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require_relative '../spec_helper' + +module Aws + describe RefreshingCredentials do + let(:resolver_class) do + Class.new do + include RefreshingCredentials + + attr_reader :source_calls + + def expect_response(response, lifetime) + @next = [response, lifetime] + end + + def refresh + @source_calls = (@source_calls || 0) + 1 + response, lifetime = @next + case response + when 'freshCredentials' then set_creds(lifetime || 3600) + when 'staleCredentials' then set_creds(-1) + when 'error' then raise 'recoverable' + when 'nonRecoverableError' then raise Aws::Errors::NonRecoverableError + end + end + + # def set_creds + # def seed_state + end + end + + before do + @now = Time.now + allow(Time).to receive(:now) {@now} + end + + context 'test runner' do + tests = JSON.load_file(File.join(File.dirname(__FILE__), 'refreshing_credentials_tests.json')) + + tests.each_with_index do |test, index| + it "case #{index + 1}: #{test['documentation']}" do + resolver = build_resolver(resolver_class, test['given']) + test['steps'].each do |step| + case step['type'] + when 'advanceTime' then @now += step['seconds'] + when 'invalidate' then resolver.invalidate(fake_identity(step['rejectedAccessKeyId'])) + when 'getCredentials' + before = resolver.source_calls || 0 + resolver.expect_response(step['response'], step['lifetimeSeconds']) + exp = step['expected'] + assert_result(resolver, exp['result']) + expect((resolver.source_calls || 0) > before).to eq(exp['sourceContacted']) + expect(resolver.rate_limited?).to eq(exp['rateLimited']) if exp.key?('rateLimited') + if exp.key?('advisoryWindowSeconds') + expect(resolver.advisory_window).to eq(exp['advisoryWindowSeconds']) + end + end + end + end + end + end + + # def build_resolver + # def assert_result + # def fake_identity + end +end diff --git a/gems/aws-sdk-core/spec/aws/refreshing_credentials_test.json b/gems/aws-sdk-core/spec/aws/refreshing_credentials_test.json new file mode 100644 index 00000000000..ed2e4a9d97a --- /dev/null +++ b/gems/aws-sdk-core/spec/aws/refreshing_credentials_test.json @@ -0,0 +1,373 @@ +[ + { + "documentation": "Valid cached credentials: no refresh is attempted and the caller receives the cached credentials.", + "given": { "cachedCredentials": "valid" }, + "steps": [ + { + "type": "getCredentials", + "expected": { "result": "cachedCredentials", "sourceContacted": false, "rateLimited": false } + } + ] + }, + { + "documentation": "Advisory window, refresh succeeds: the caller receives the newly refreshed credentials.", + "given": { "cachedCredentials": "advisory" }, + "steps": [ + { + "type": "getCredentials", + "response": "freshCredentials", + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "Advisory window, refresh fails: the resolver applies the refresh backoff and the caller receives the existing cached credentials.", + "given": { "cachedCredentials": "advisory" }, + "steps": [ + { + "type": "getCredentials", + "response": "error", + "expected": { "result": "cachedCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "Mandatory window, refresh succeeds: the caller receives the newly refreshed credentials.", + "given": { "cachedCredentials": "mandatory" }, + "steps": [ + { + "type": "getCredentials", + "response": "freshCredentials", + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "Mandatory window, refresh fails: the resolver applies the refresh backoff and the caller receives the cached credentials.", + "given": { "cachedCredentials": "mandatory" }, + "steps": [ + { + "type": "getCredentials", + "response": "error", + "expected": { "result": "cachedCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "Expired credentials are refreshed successfully: the caller receives the newly refreshed credentials.", + "given": { "cachedCredentials": "expired" }, + "steps": [ + { + "type": "getCredentials", + "response": "freshCredentials", + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "Expired credentials, refresh fails: the resolver applies the refresh backoff and the caller receives the expired cached credentials rather than raising.", + "given": { "cachedCredentials": "expired" }, + "steps": [ + { + "type": "getCredentials", + "response": "error", + "expected": { "result": "cachedCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "No cached credentials and the initial fetch fails: the SDK raises, since there are no cached credentials to fall back on. The next call retries and succeeds.", + "given": { "cachedCredentials": "none" }, + "steps": [ + { + "type": "getCredentials", + "response": "error", + "expected": { "result": "noCredentialsError", "sourceContacted": true, "rateLimited": false } + }, + { + "type": "getCredentials", + "response": "freshCredentials", + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "Advisory window, source returns stale credentials (Expiration at or before now): treated as a failed refresh. The resolver applies the refresh backoff and returns the existing cached credentials.", + "given": { "cachedCredentials": "advisory" }, + "steps": [ + { + "type": "getCredentials", + "response": "staleCredentials", + "expected": { "result": "cachedCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "Mandatory window, source returns stale credentials: same as the advisory case, treated as a failed refresh.", + "given": { "cachedCredentials": "mandatory" }, + "steps": [ + { + "type": "getCredentials", + "response": "staleCredentials", + "expected": { "result": "cachedCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + + { + "documentation": "A 10-minute credential lifetime selects the 5-minute advisory window (lifetime <= 20 minutes).", + "given": { "cachedCredentials": "none" }, + "steps": [ + { + "type": "getCredentials", + "response": "freshCredentials", + "lifetimeSeconds": 600, + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false, "advisoryWindowSeconds": 300 } + } + ] + }, + { + "documentation": "A 20.5-minute credential lifetime selects the 15-minute advisory window (lifetime > 20 and < 90 minutes).", + "given": { "cachedCredentials": "none" }, + "steps": [ + { + "type": "getCredentials", + "response": "freshCredentials", + "lifetimeSeconds": 1230, + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false, "advisoryWindowSeconds": 900 } + } + ] + }, + { + "documentation": "A 6-hour credential lifetime selects the 60-minute advisory window (lifetime >= 90 minutes).", + "given": { "cachedCredentials": "none" }, + "steps": [ + { + "type": "getCredentials", + "response": "freshCredentials", + "lifetimeSeconds": 21600, + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false, "advisoryWindowSeconds": 3600 } + } + ] + }, + { + "documentation": "After a successful refresh returns credentials with a different lifetime, the SDK recomputes the advisory window. The first credentials have a 6-hour lifetime (60-minute window); after advancing into that window, the refreshed credentials have a 10-minute lifetime (5-minute window).", + "given": { "cachedCredentials": "none" }, + "steps": [ + { + "type": "getCredentials", + "response": "freshCredentials", + "lifetimeSeconds": 21600, + "documentation": "Initial fetch returns 6-hour credentials, selecting the 60-minute advisory window.", + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false, "advisoryWindowSeconds": 3600 } + }, + { + "type": "advanceTime", + "seconds": 18060 + }, + { + "type": "getCredentials", + "response": "freshCredentials", + "lifetimeSeconds": 600, + "documentation": "59 minutes remain until expiration, inside the 60-minute advisory window, so the SDK refreshes. The new 10-minute credentials select the 5-minute advisory window.", + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false, "advisoryWindowSeconds": 300 } + } + ] + }, + { + "documentation": "A customer-configured advisory window overrides the table. Credentials with a 6-hour lifetime would map to 60 minutes, but the configured 30-minute window is used instead.", + "given": { "cachedCredentials": "none", "configuredAdvisoryWindowSeconds": 1800 }, + "steps": [ + { + "type": "getCredentials", + "response": "freshCredentials", + "lifetimeSeconds": 21600, + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false, "advisoryWindowSeconds": 1800 } + } + ] + }, + + { + "documentation": "Advisory window, non-recoverable failure: the SDK raises immediately. No refresh backoff is applied, but the error is cached for up to 5 seconds, so a recovering call succeeds once that cache expires.", + "given": { "cachedCredentials": "advisory" }, + "steps": [ + { + "type": "getCredentials", + "response": "nonRecoverableError", + "documentation": "Non-recoverable failure: the SDK raises and does not apply the refresh backoff.", + "expected": { "result": "nonRecoverableError", "sourceContacted": true, "rateLimited": false } + }, + { + "type": "advanceTime", + "seconds": 6 + }, + { + "type": "getCredentials", + "response": "freshCredentials", + "documentation": "The non-recoverable error cache (max 5 seconds) has expired, so this call contacts the source again and succeeds.", + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "Mandatory window, non-recoverable failure: the SDK raises immediately. No refresh backoff is applied, but the error is cached for up to 5 seconds, so a recovering call succeeds once that cache expires.", + "given": { "cachedCredentials": "mandatory" }, + "steps": [ + { + "type": "getCredentials", + "response": "nonRecoverableError", + "documentation": "Non-recoverable failure: the SDK raises and does not apply the refresh backoff.", + "expected": { "result": "nonRecoverableError", "sourceContacted": true, "rateLimited": false } + }, + { + "type": "advanceTime", + "seconds": 6 + }, + { + "type": "getCredentials", + "response": "freshCredentials", + "documentation": "The non-recoverable error cache (max 5 seconds) has expired, so this call contacts the source again and succeeds.", + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "Non-recoverable error, then an immediate retry with no clock advance: the error is still cached, so the SDK re-raises it without contacting the source. This protects the credential source from an application that swallows the error and retries in a loop.", + "given": { "cachedCredentials": "advisory" }, + "steps": [ + { + "type": "getCredentials", + "response": "nonRecoverableError", + "documentation": "Non-recoverable failure: the SDK raises and caches the error for up to 5 seconds.", + "expected": { "result": "nonRecoverableError", "sourceContacted": true, "rateLimited": false } + }, + { + "type": "getCredentials", + "documentation": "Immediate retry with no clock advance. The cached error is still active, so the SDK re-raises it without contacting the source.", + "expected": { "result": "nonRecoverableError", "sourceContacted": false, "rateLimited": false } + } + ] + }, + + { + "documentation": "Invalidate with an access key ID matching the cached credentials routes the next getCredentials through the mandatory refresh path, and the refresh succeeds.", + "given": { "cachedCredentials": "valid", "accessKeyId": "AKID-1" }, + "steps": [ + { "type": "invalidate", "rejectedAccessKeyId": "AKID-1" }, + { + "type": "getCredentials", + "response": "freshCredentials", + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "Invalidate with a matching access key ID routes the next getCredentials through the mandatory refresh path; the refresh fails and the SDK continues using the cached credentials.", + "given": { "cachedCredentials": "valid", "accessKeyId": "AKID-1" }, + "steps": [ + { "type": "invalidate", "rejectedAccessKeyId": "AKID-1" }, + { + "type": "getCredentials", + "response": "error", + "expected": { "result": "cachedCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "Invalidate during an active backoff: the SDK does not contact the credential source. Once the refresh backoff has elapsed, the next getCredentials attempts a refresh.", + "given": { "cachedCredentials": "expired", "accessKeyId": "AKID-1", "refreshBackoffSeconds": 420 }, + "steps": [ + { + "type": "getCredentials", + "response": "error", + "documentation": "Refresh fails, so the SDK applies the refresh backoff.", + "expected": { "result": "cachedCredentials", "sourceContacted": true, "rateLimited": false } + }, + { + "type": "advanceTime", + "seconds": 60 + }, + { "type": "invalidate", "rejectedAccessKeyId": "AKID-1" }, + { + "type": "getCredentials", + "documentation": "60s elapsed and the refresh backoff has not yet elapsed, so even after invalidation the SDK does not contact the credential source.", + "expected": { "result": "cachedCredentials", "sourceContacted": false, "rateLimited": true } + }, + { + "type": "advanceTime", + "seconds": 425 + }, + { + "type": "getCredentials", + "response": "freshCredentials", + "documentation": "485s elapsed total and the refresh backoff has elapsed, so the SDK contacts the credential source and the refresh succeeds.", + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "Invalidate with a stale access key ID (a concurrent refresh already replaced the credentials): the cache is unchanged and the next getCredentials does not contact the source.", + "given": { "cachedCredentials": "valid", "accessKeyId": "AKID-2" }, + "steps": [ + { "type": "invalidate", "rejectedAccessKeyId": "AKID-1" }, + { + "type": "getCredentials", + "expected": { "result": "cachedCredentials", "sourceContacted": false, "rateLimited": false } + } + ] + }, + + { + "documentation": "After a failed refresh, the SDK does not contact the credential source again until the refresh backoff has elapsed.", + "given": { "cachedCredentials": "expired", "refreshBackoffSeconds": 420 }, + "steps": [ + { + "type": "getCredentials", + "response": "error", + "documentation": "Refresh fails, so the SDK applies the refresh backoff.", + "expected": { "result": "cachedCredentials", "sourceContacted": true, "rateLimited": false } + }, + { + "type": "advanceTime", + "seconds": 300 + }, + { + "type": "getCredentials", + "documentation": "300s elapsed and the refresh backoff has not yet elapsed, so the SDK does not contact the credential source.", + "expected": { "result": "cachedCredentials", "sourceContacted": false, "rateLimited": true } + }, + { + "type": "advanceTime", + "seconds": 425 + }, + { + "type": "getCredentials", + "response": "freshCredentials", + "documentation": "725s elapsed total and the refresh backoff has elapsed, so the SDK contacts the credential source and the refresh succeeds.", + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + }, + { + "documentation": "No cached credentials and the initial fetch fails with a non-recoverable error: the SDK raises the error directly rather than a generic NoCredentialsError. No refresh backoff is applied, but the error is cached for up to 5 seconds, so a recovering call succeeds once that cache expires.", + "given": { "cachedCredentials": "none" }, + "steps": [ + { + "type": "getCredentials", + "response": "nonRecoverableError", + "documentation": "Non-recoverable failure: the SDK raises and does not apply the refresh backoff.", + "expected": { "result": "nonRecoverableError", "sourceContacted": true, "rateLimited": false } + }, + { + "type": "advanceTime", + "seconds": 6 + }, + { + "type": "getCredentials", + "response": "freshCredentials", + "documentation": "The non-recoverable error cache (max 5 seconds) has expired, so this call contacts the source again and succeeds.", + "expected": { "result": "newCredentials", "sourceContacted": true, "rateLimited": false } + } + ] + } +] From 967de606929c3f0899f3b938661c21bbca12d983 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Tue, 1 Sep 2026 14:23:53 -0700 Subject: [PATCH 02/18] Update refreshing credentials --- .../aws-sdk-core/refreshing_credentials.rb | 241 ++++++++++++++---- 1 file changed, 191 insertions(+), 50 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb index 777c32f3786..e80f831d27a 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb @@ -1,17 +1,23 @@ # frozen_string_literal: true module Aws - # Base class used credential classes that can be refreshed. This - # provides basic refresh logic in a thread-safe manner. Classes mixing in - # this module are expected to implement a `#refresh` method that populates - # the following instance variables: + # Base module mixed into refreshable credential classes. Implements the + # credential refresh lifecycle: caching, an advisory and a mandatory + # refresh window, rate-limited backoff on failure, static stability + # (continue using cached credentials when a refresh fails), and + # short-lived caching of non-recoverable errors. # - # * `@credentials` ({Credentials}) - # * `@expiration` (Time) + # Classes mixing in this module must implement `#refresh`, which fetches + # from the source and assigns `@credentials` and `@expiration` on success, + # or raises on failure. It must not partially update those on failure. # + # Before calling `super`, classes may set `@async_refresh` to true to + # refresh in the background during the advisory window, or set + # `@static_stability` to false for caching-only behavior. Classes may + # override `#non_recoverable_error?` to classify provider errors that + # should be raised immediately rather than retried. module RefreshingCredentials - SYNC_EXPIRATION_LENGTH = 300 # 5 minutes - ASYNC_EXPIRATION_LENGTH = 600 # 10 minutes + MANDATORY_REFRESH_WINDOW = 60 # 1 minute CLIENT_EXCLUDE_OPTIONS = Set.new([:before_refresh]).freeze @@ -20,74 +26,209 @@ module RefreshingCredentials # It accepts `self` as the only argument. def initialize(options = {}) @mutex = Mutex.new - @before_refresh = options.delete(:before_refresh) if options.is_a?(Hash) - - @before_refresh.call(self) if @before_refresh - refresh + if options.is_a?(Hash) + @before_refresh = options.delete(:before_refresh) + @configured_advisory_window = options.delete(:advisory_refresh_window) + end + @static_stability = true if @static_stability.nil? + @next_refresh_allowed_at = nil + @cached_error = nil + @cached_error_expires_at = nil + @advisory_window = nil + fetch_initial_credentials end + attr_reader :advisory_window + # @return [Credentials] def credentials - refresh_if_near_expiration! + get_credentials @credentials end - # Refresh credentials. + # Force a synchronous refresh, raising on failure. Does not apply static + # stability or backoff. # @return [void] def refresh! @mutex.synchronize do - @before_refresh.call(self) if @before_refresh - + @before_refresh&.call(self) refresh end end - private + # Mark cached credentials for refresh after a target service rejects them + def invalidate(rejected_credentials) + return unless @mutex.try_lock - def sync_expiration_length - self.class::SYNC_EXPIRATION_LENGTH + begin + if @credentials && @credentials.access_key_id == rejected_credentials.access_key_id + @expiration = Time.now + end + ensure + @mutex.unlock + end end - def async_expiration_length - self.class::ASYNC_EXPIRATION_LENGTH + def rate_limited? + !@next_refresh_allowed_at.nil? && Time.now < @next_refresh_allowed_at end - # Refreshes credentials asynchronously and synchronously. - # If we are near to expiration, block while getting new credentials. - # Otherwise, if we're approaching expiration, use the existing credentials - # but attempt a refresh in the background. - def refresh_if_near_expiration! - # NOTE: This check is an optimization. Rather than acquire the mutex on every #refresh_if_near_expiration - # call, we check before doing so, and then we check within the mutex to avoid a race condition. - # See issue: https://github.com/aws/aws-sdk-ruby/issues/2641 for more info. - if near_expiration?(sync_expiration_length) - @mutex.synchronize do - if near_expiration?(sync_expiration_length) - @before_refresh.call(self) if @before_refresh - refresh - end + private + + def get_credentials + return fetch_initial_credentials if @credentials.nil? + return unless refresh_needed? + return if rate_limited? + + if mandatory_refresh_needed? + attempt_mandatory_refresh + else + attempt_advisory_refresh + end + end + + def fetch_initial_credentials + @mutex.synchronize do + return @credentials unless @credentials.nil? + raise @cached_error if non_recoverable_error_cached? + + error = call_source + if error.nil? + on_refresh_success + return @credentials end - elsif @async_refresh && near_expiration?(async_expiration_length) - unless @mutex.locked? - Thread.new do - @mutex.synchronize do - if near_expiration?(async_expiration_length) - @before_refresh.call(self) if @before_refresh - refresh - end - end - end + + if non_recoverable_error?(error) + cache_non_recoverable_error(error) + raise error end + raise Errors::NoCredentialsError end end - def near_expiration?(expiration_length) - if @expiration - # Are we within expiration? - (Time.now.to_i + expiration_length) > @expiration.to_i + def attempt_advisory_refresh + if @async_refresh + refresh_in_background + @credentials else - true + return @credentials unless @mutex.try_lock + + begin + perform_refresh(mandatory: false, raise_to_caller: true) + ensure + @mutex.unlock + end + end + end + + def attempt_mandatory_refresh + @mutex.synchronize do + perform_refresh(mandatory: true, raise_to_caller: true) + end + end + + def refresh_in_background + return if @mutex.locked? + + Thread.new do + @mutex.synchronize do + perform_refresh(mandatory: false, raise_to_caller: false) + end end end + + def perform_refresh(mandatory:, raise_to_caller:) + return @credentials unless mandatory ? mandatory_refresh_needed? : refresh_needed? + + if non_recoverable_error_cached? + raise @cached_error if raise_to_caller + + return @credentials + end + return @credentials if rate_limited? + + error = call_source + if error.nil? + on_refresh_success + return @credentials + end + + handle_failure(error, raise_to_caller: raise_to_caller) + end + + # Calls the source via #refresh. Returns nil on success, or an error. + def call_source + @before_refresh&.call(self) + refresh + !@expiration.nil? && @expiration <= Time.now ? Errors::StaleCredentialsError.new : nil + rescue StandardError => e + e + end + + def handle_failure(error, raise_to_caller:) + if non_recoverable_error?(error) + cache_non_recoverable_error(error) + raise error if raise_to_caller + + return @credentials + end + + raise error if mandatory_refresh_needed? && !@static_stability + + @next_refresh_allowed_at = Time.now + refresh_backoff + log_refresh_failure(error) + @credentials + end + + def on_refresh_success + @next_refresh_allowed_at = nil + @cached_error = nil + @cached_error_expires_at = nil + @advisory_window = select_advisory_window + end + + def refresh_needed? + within?(@advisory_window || select_advisory_window) + end + + def mandatory_refresh_needed? + within?(MANDATORY_REFRESH_WINDOW) + end + + def within?(seconds) + return false unless @expiration + + Time.now + seconds > @expiration + end + + def select_advisory_window + return @configured_advisory_window if @configured_advisory_window + return 60 * 60 unless @expiration + + lifetime = @expiration - Time.now + return 5 * 60 if lifetime <= 20 * 60 + return 15 * 60 if lifetime < 90 * 60 + + 60 * 60 + end + + def non_recoverable_error_cached? + !@cached_error.nil? && Time.now < @cached_error_expires_at + end + + def refresh_backoff + rand(300..600) + end + + def cache_non_recoverable_error(error) + @cached_error = error + @cached_error_expires_at = Time.now + rand(1..5) + end + + def non_recoverable_error?(_error) + false + end + + def log_refresh_failure(_error); end end end From f9152a238d3e48242aced426cd2ce8854950bde7 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Mon, 7 Sep 2026 10:41:32 -0700 Subject: [PATCH 03/18] Update spec --- gems/aws-sdk-core/lib/aws-sdk-core/errors.rb | 13 ++ .../aws-sdk-core/refreshing_credentials.rb | 13 +- .../spec/aws/refreshing_credentials_spec.rb | 116 +++++++++++++----- ...json => refreshing_credentials_tests.json} | 0 4 files changed, 111 insertions(+), 31 deletions(-) rename gems/aws-sdk-core/spec/aws/{refreshing_credentials_test.json => refreshing_credentials_tests.json} (100%) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/errors.rb b/gems/aws-sdk-core/lib/aws-sdk-core/errors.rb index dc41bb36ee1..884934dee66 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/errors.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/errors.rb @@ -5,6 +5,19 @@ module Errors class NonSupportedRubyVersionError < RuntimeError; end + # Raised when no credentials have been obtained and the initial fetch + # from the credential source failed. + class NoCredentialsError < RuntimeError + def initialize(*args) + super('unable to obtain credentials from the credential source') + end + end + + # Internal: signals a credential source response whose Expiration is at or + # before the current time. Handled within the refresh lifecycle and never + # surfaced to callers. + class StaleCredentialsError < RuntimeError; end + # The base class for all errors returned by an Amazon Web Service. # All ~400 level client errors and ~500 level server errors are raised # as service errors. This indicates it was an error returned from the diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb index e80f831d27a..ef2e737aa2c 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb @@ -156,12 +156,21 @@ def perform_refresh(mandatory:, raise_to_caller:) handle_failure(error, raise_to_caller: raise_to_caller) end - # Calls the source via #refresh. Returns nil on success, or an error. + # Calls the source via #refresh. Returns nil on success, or an error (a + # raised error, or a stale response whose Expiration is at or before now). + # Restores the prior credentials on failure so a failed or stale refresh + # never discards the cached credentials. def call_source + prior = [@credentials, @expiration] @before_refresh&.call(self) refresh - !@expiration.nil? && @expiration <= Time.now ? Errors::StaleCredentialsError.new : nil + if !@expiration.nil? && @expiration <= Time.now + @credentials, @expiration = prior + return Errors::StaleCredentialsError.new + end + nil rescue StandardError => e + @credentials, @expiration = prior e end diff --git a/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb index 88246e2340f..271fb384e00 100644 --- a/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb @@ -3,6 +3,9 @@ require_relative '../spec_helper' module Aws + # test-only error the fake source raises for a non-recoverable response + class RefreshingCredentialsTestError < StandardError; end + describe RefreshingCredentials do let(:resolver_class) do Class.new do @@ -10,6 +13,20 @@ module Aws attr_reader :source_calls + # Bypasses the eager fetch in RefreshingCredentials#initialize and + # seeds the cache state directly. + def initialize(seed = {}) + @mutex = Mutex.new + @static_stability = true + @next_refresh_allowed_at = nil + @cached_error = nil + @cached_error_expires_at = nil + @configured_advisory_window = seed[:configured_advisory_window] + @advisory_window = seed[:advisory_window] + @credentials = seed[:credentials] + @expiration = seed[:expiration] + end + def expect_response(response, lifetime) @next = [response, lifetime] end @@ -20,49 +37,90 @@ def refresh case response when 'freshCredentials' then set_creds(lifetime || 3600) when 'staleCredentials' then set_creds(-1) - when 'error' then raise 'recoverable' - when 'nonRecoverableError' then raise Aws::Errors::NonRecoverableError + when 'error' then raise 'recoverable refresh failure' + when 'nonRecoverableError' then raise RefreshingCredentialsTestError end end - # def set_creds - # def seed_state + def non_recoverable_error?(error) + error.is_a?(RefreshingCredentialsTestError) + end + + private + + def set_creds(lifetime) + @credentials = Credentials.new('FRESH-AKID', 'secret', 'token') + @expiration = Time.now + lifetime + end end end before do @now = Time.now - allow(Time).to receive(:now) {@now} + allow(Time).to receive(:now) { @now } + end + + def build_resolver(klass, given) + @seeded_akid = given['accessKeyId'] || 'CACHED-AKID' + seed = { configured_advisory_window: given['configuredAdvisoryWindowSeconds'] } + unless given['cachedCredentials'] == 'none' + seed[:credentials] = Credentials.new(@seeded_akid, 'secret', 'token') + seed[:advisory_window] = 15 * 60 + ttl = { 'valid' => 1000, 'advisory' => 300, 'mandatory' => 30, 'expired' => -10 } + .fetch(given['cachedCredentials']) + seed[:expiration] = Time.now + ttl + end + resolver = klass.new(seed) + allow(resolver).to receive(:refresh_backoff).and_return(given['refreshBackoffSeconds'] || 300) + resolver end - context 'test runner' do - tests = JSON.load_file(File.join(File.dirname(__FILE__), 'refreshing_credentials_tests.json')) - - tests.each_with_index do |test, index| - it "case #{index + 1}: #{test['documentation']}" do - resolver = build_resolver(resolver_class, test['given']) - test['steps'].each do |step| - case step['type'] - when 'advanceTime' then @now += step['seconds'] - when 'invalidate' then resolver.invalidate(fake_identity(step['rejectedAccessKeyId'])) - when 'getCredentials' - before = resolver.source_calls || 0 - resolver.expect_response(step['response'], step['lifetimeSeconds']) - exp = step['expected'] - assert_result(resolver, exp['result']) - expect((resolver.source_calls || 0) > before).to eq(exp['sourceContacted']) - expect(resolver.rate_limited?).to eq(exp['rateLimited']) if exp.key?('rateLimited') - if exp.key?('advisoryWindowSeconds') - expect(resolver.advisory_window).to eq(exp['advisoryWindowSeconds']) - end + def fake_identity(access_key_id) + double('identity', access_key_id: access_key_id) + end + + def assert_result(resolver, expected) + case expected + when 'newCredentials' + expect(resolver.credentials.access_key_id).to eq('FRESH-AKID') + when 'cachedCredentials' + expect(resolver.credentials.access_key_id).to eq(@seeded_akid) + when 'noCredentialsError' + expect { resolver.credentials }.to raise_error(Errors::NoCredentialsError) + when 'nonRecoverableError' + expect { resolver.credentials }.to raise_error(RefreshingCredentialsTestError) + end + end + + tests = JSON.load_file(File.join(File.dirname(__FILE__), 'refreshing_credentials_tests.json')) + + tests.each_with_index do |test, index| + it "case #{index + 1}: #{test['documentation']}" do + resolver = build_resolver(resolver_class, test['given']) + test['steps'].each do |step| + case step['type'] + when 'advanceTime' + @now += step['seconds'] + when 'invalidate' + resolver.invalidate(fake_identity(step['rejectedAccessKeyId'])) + when 'getCredentials' + source_calls_before = resolver.source_calls || 0 + entering_rate_limited = resolver.rate_limited? + resolver.expect_response(step['response'], step['lifetimeSeconds']) + + expected = step['expected'] + assert_result(resolver, expected['result']) + expect((resolver.source_calls || 0) > source_calls_before) + .to eq(expected['sourceContacted']) + if expected.key?('rateLimited') + expect(entering_rate_limited).to eq(expected['rateLimited']) + end + if expected.key?('advisoryWindowSeconds') + expect(resolver.advisory_window).to eq(expected['advisoryWindowSeconds']) end end end end end - - # def build_resolver - # def assert_result - # def fake_identity end end diff --git a/gems/aws-sdk-core/spec/aws/refreshing_credentials_test.json b/gems/aws-sdk-core/spec/aws/refreshing_credentials_tests.json similarity index 100% rename from gems/aws-sdk-core/spec/aws/refreshing_credentials_test.json rename to gems/aws-sdk-core/spec/aws/refreshing_credentials_tests.json From 8d629df7789f5d2de857e89be3d4b265ce3215cc Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Mon, 14 Sep 2026 10:15:19 -0700 Subject: [PATCH 04/18] Update IMDS and Login credentials --- .../aws-sdk-core/credential_provider_chain.rb | 5 ++ .../instance_profile_credentials.rb | 74 ++++------------- .../lib/aws-sdk-core/login_credentials.rb | 9 ++- .../aws/instance_profile_credentials_spec.rb | 80 ++++++++----------- 4 files changed, 65 insertions(+), 103 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/credential_provider_chain.rb b/gems/aws-sdk-core/lib/aws-sdk-core/credential_provider_chain.rb index 20a61ac2b24..9fa05a11379 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/credential_provider_chain.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/credential_provider_chain.rb @@ -239,6 +239,11 @@ def instance_profile_credentials(options) elsif !(ENV.fetch('AWS_EC2_METADATA_DISABLED', 'false').downcase == 'true') InstanceProfileCredentials.new(options.merge(profile: profile_name)) end + rescue Errors::NoCredentialsError + # The credential source was unreachable on the initial fetch, skip this + # provider so the chain moves on. Non-recoverable errors are not + # NoCredentialsError and still propagate. + nil end def assume_role_with_profile(options, profile_name) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb index d75ffe15649..17fe184a10a 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb @@ -99,7 +99,6 @@ def initialize(options = {}) @async_refresh = false @imds_v1_fallback = false - @no_refresh_until = nil @token = nil @metrics = ['CREDENTIALS_IMDS'] super @@ -181,65 +180,35 @@ def resolve_backoff(backoff) end def refresh - if @no_refresh_until && @no_refresh_until > Time.now - warn_expired_credentials - return - end - - new_creds = - begin - # Retry loading credentials up to 3 times is the instance metadata - # service is responding but is returning invalid JSON documents - # in response to the GET profile credentials call. - retry_errors([Aws::Json::ParseError], max_retries: 3) do - Aws::Json.load(retrieve_credentials.to_s) - end - rescue Aws::Json::ParseError - raise Aws::Errors::MetadataParserError - end - - if @credentials&.set? && empty_credentials?(new_creds) - # credentials are already set, but there was an error getting new credentials - # so don't update the credentials and use stale ones (static stability) - @no_refresh_until = Time.now + rand(300..360) - warn_expired_credentials - else - # credentials are empty or successfully retrieved, update them - update_credentials(new_creds) + # Retry loading credentials up to 3 times if the instance metadata + # service is responding but is returning invalid JSON documents in + # response to the GET profile credentials call. + c = retry_errors([Aws::Json::ParseError], max_retries: 3) do + Aws::Json.load(retrieve_credentials.to_s) end + @credentials = Credentials.new(c['AccessKeyId'], c['SecretAccessKey'], c['Token']) + @expiration = c['Expiration'] ? Time.iso8601(c['Expiration']) : nil + rescue Aws::Json::ParseError + raise Aws::Errors::MetadataParserError end def retrieve_credentials # Retry loading credentials a configurable number of times if # the instance metadata service is not responding. - begin - retry_errors(NETWORK_ERRORS, max_retries: @retries) do - open_connection do |conn| - # attempt to fetch token to start secure flow first - # and rescue to failover - fetch_token(conn) unless @imds_v1_fallback || (@token && !@token.expired?) + retry_errors(NETWORK_ERRORS, max_retries: @retries) do + open_connection do |conn| + # attempt to fetch token to start secure flow first + # and rescue to failover + fetch_token(conn) unless @imds_v1_fallback || (@token && !@token.expired?) - # disable insecure flow if we couldn't get token and imds v1 is disabled - raise TokenRetrivalError if @token.nil? && @disable_imds_v1 + # disable insecure flow if we couldn't get token and imds v1 is disabled + raise TokenRetrivalError if @token.nil? && @disable_imds_v1 - fetch_credentials(conn) - end + fetch_credentials(conn) end - rescue StandardError => e - warn("Error retrieving instance profile credentials: #{e}") - '{}' end end - def update_credentials(creds) - @credentials = Credentials.new(creds['AccessKeyId'], creds['SecretAccessKey'], creds['Token']) - @expiration = creds['Expiration'] ? Time.iso8601(creds['Expiration']) : nil - return unless @expiration && @expiration < Time.now - - @no_refresh_until = Time.now + rand(300..360) - warn_expired_credentials - end - def fetch_token(conn) created_time = Time.now token_value, ttl = http_put(conn) @@ -326,15 +295,6 @@ def retry_errors(error_classes, options = {}, &_block) end end - def warn_expired_credentials - warn('Attempting credential expiration extension due to a credential service availability issue. '\ - 'A refresh of these credentials will be attempted again in 5 minutes.') - end - - def empty_credentials?(creds_hash) - !creds_hash['AccessKeyId'] || creds_hash['AccessKeyId'].empty? - end - # @api private # Token used to fetch IMDS profile and credentials class Token diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/login_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/login_credentials.rb index 60e68dcbebd..1167613ecf5 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/login_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/login_credentials.rb @@ -47,7 +47,8 @@ def refresh # First reload the token from disk to ensure it hasn't been refreshed externally token_json = read_cached_token update_creds(token_json['accessToken']) - return if @credentials && @expiration && !near_expiration?(sync_expiration_length) + # if the reloaded token is fresh use it without contacting Sign-In + return unless refresh_needed? # Using OpenSSL 3.6.0 may result in errors like "certificate verify failed (unable to get certificate CRL)." # A recommended workaround is to use OpenSSL version < 3.6.0 or requiring the openssl gem with a version of at @@ -67,6 +68,12 @@ def refresh 'Login token is invalid and failed to refresh. Please reauthenticate.' end + # A missing, unparseable, or malformed login token requires the user to + # reauthenticate, so it must be raised immediately rather than retried. + def non_recoverable_error?(error) + error.is_a?(Errors::InvalidLoginToken) || error.is_a?(ArgumentError) + end + def read_cached_token cached_token = JSON.load_file(login_cache_file) validate_cached_token(cached_token) diff --git a/gems/aws-sdk-core/spec/aws/instance_profile_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/instance_profile_credentials_spec.rb index 0e644accc75..2c0f37f4544 100644 --- a/gems/aws-sdk-core/spec/aws/instance_profile_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/instance_profile_credentials_spec.rb @@ -165,10 +165,11 @@ module Aws SocketError, Timeout::Error ].each do |error_class| - it "returns no credentials for #{error_class}" do + it "raises NoCredentialsError for #{error_class}" do stub_request(:put, ipv4_endpoint_token_path).to_return(status: 200, body: 'mytoken') stub_request(:get, ipv4_endpoint + path).to_raise(error_class) - expect(InstanceProfileCredentials.new(backoff: 0).set?).to be(false) + expect { InstanceProfileCredentials.new(backoff: 0) } + .to raise_error(Aws::Errors::NoCredentialsError) end end @@ -176,10 +177,11 @@ module Aws 400, 401 ].each do |error_code| - it "returns no credentials for #{error_code} when fetching token" do + it "raises NoCredentialsError for #{error_code} when fetching token" do stub_request(:put, ipv4_endpoint_token_path).to_return(status: error_code) stub_request(:get, ipv4_endpoint + path).to_return(status: 200) - expect(InstanceProfileCredentials.new(backoff: 0).set?).to be(false) + expect { InstanceProfileCredentials.new(backoff: 0) } + .to raise_error(Aws::Errors::NoCredentialsError) end end end @@ -251,6 +253,7 @@ module Aws end it 'has a disable flag which is not case sensitive' do + allow_any_instance_of(InstanceProfileCredentials).to receive(:refresh) ENV['AWS_EC2_METADATA_V1_DISABLED'] = 'TrUe' c = InstanceProfileCredentials.new(backoff: 0) expect(c.disable_imds_v1).to be(true) @@ -258,7 +261,8 @@ module Aws it 'does not attempt to get credentials (insecure)' do stub_request(:put, ipv4_endpoint_token_path).to_return(status: 404) - expect(InstanceProfileCredentials.new(backoff: 0).set?).to be(false) + expect { InstanceProfileCredentials.new(backoff: 0) } + .to raise_error(Aws::Errors::NoCredentialsError) end it 'gets credentials (secure)' do @@ -387,12 +391,13 @@ module Aws expect(c.expiration.to_s).to eq(expiration2.to_s) end - it 'retries invalid JSON exactly 3 times' do + it 'retries invalid JSON exactly 3 times, then raises NoCredentialsError' do stub_request(:get, ipv4_endpoint + path) .with(headers: { 'x-aws-ec2-metadata-token' => 'my-token' }) .to_return(status: 500) .to_return(status: 200, body: "profile-name\n") - stub_request(:get, "#{ipv4_endpoint_creds_path}profile-name") + creds_request = + stub_request(:get, "#{ipv4_endpoint_creds_path}profile-name") .with(headers: { 'x-aws-ec2-metadata-token' => 'my-token' }) .to_return(status: 200, body: '') .to_return(status: 200, body: ' ') @@ -400,13 +405,11 @@ module Aws .to_return(status: 200, body: ' ') expect do InstanceProfileCredentials.new(backoff: 0) - end.to raise_error( - Aws::Errors::MetadataParserError, - 'Failed to parse metadata service response.' - ) + end.to raise_error(Aws::Errors::NoCredentialsError) + assert_requested(creds_request, times: 4) end - it 'retries errors parsing expiration time 3 times' do + it 'raises NoCredentialsError when the expiration time cannot be parsed' do stub_request(:get, ipv4_endpoint + path) .with(headers: { 'x-aws-ec2-metadata-token' => 'my-token' }) .to_return(status: 500) @@ -414,12 +417,9 @@ module Aws stub_request(:get, "#{ipv4_endpoint_creds_path}profile-name") .with(headers: { 'x-aws-ec2-metadata-token' => 'my-token' }) .to_return(status: 200, body: '{ "Expiration": "Expiration" }') - .to_return(status: 200, body: '{ "Expiration": "Expiration" }') - .to_return(status: 200, body: '{ "Expiration": "Expiration" }') - .to_return(status: 200, body: '{ "Expiration": "Expiration" }') expect do InstanceProfileCredentials.new(backoff: 0) - end.to raise_error(ArgumentError) + end.to raise_error(Aws::Errors::NoCredentialsError) end describe 'auto refreshing' do @@ -452,24 +452,20 @@ module Aws expect(c.expiration).to be(nil) end - it 'returns empty credentials on non-200 response from profile endpoint' do + it 'raises NoCredentialsError on non-200 response from profile endpoint' do stub_request(:get, "#{ipv4_endpoint_creds_path}profile-name") .with(headers: { 'x-aws-ec2-metadata-token' => 'my-token' }) .to_return(status: 404, body: 'Not Found') - expect_any_instance_of(InstanceProfileCredentials).to receive(:warn) - .with(/Error retrieving instance profile credentials: HTTP 404: Not Found/) - c = InstanceProfileCredentials.new(backoff: 0, retries: 0) - expect(c.set?).to be(false) + expect { InstanceProfileCredentials.new(backoff: 0, retries: 0) } + .to raise_error(Aws::Errors::NoCredentialsError) end - it 'returns empty credentials on non-200 response from metadata service' do + it 'raises NoCredentialsError on non-200 response from metadata service' do stub_request(:get, ipv4_endpoint + path) .with(headers: { 'x-aws-ec2-metadata-token' => 'my-token' }) .to_return(status: 503, body: 'Service Unavailable') - expect_any_instance_of(InstanceProfileCredentials).to receive(:warn) - .with(/Error retrieving instance profile credentials: HTTP 503: Service Unavailable/) - c = InstanceProfileCredentials.new(backoff: 0, retries: 0) - expect(c.set?).to be(false) + expect { InstanceProfileCredentials.new(backoff: 0, retries: 0) } + .to raise_error(Aws::Errors::NoCredentialsError) end end end @@ -491,6 +487,7 @@ module Aws end it 'defaults to 1' do + allow_any_instance_of(InstanceProfileCredentials).to receive(:refresh) expect(InstanceProfileCredentials.new(backoff: 0).retries).to be(1) end @@ -499,7 +496,9 @@ module Aws expect(Kernel).to receive(:sleep).with(1) expect(Kernel).to receive(:sleep).with(2) expect(Kernel).to receive(:sleep).with(4) - InstanceProfileCredentials.new(backoff: ->(n) { Kernel.sleep(2**n) }, retries: 3) + expect do + InstanceProfileCredentials.new(backoff: ->(n) { Kernel.sleep(2**n) }, retries: 3) + end.to raise_error(Aws::Errors::NoCredentialsError) assert_requested(expected_request, times: 4) end end @@ -544,29 +543,19 @@ module Aws .to_return(status: 200, body: "profile-name\n") end - it 'provides credentials when the first call returns expired credentials' do - expect_any_instance_of(InstanceProfileCredentials).to receive(:warn).at_least(:once) - - expected_request = - stub_request(:get, "#{ipv4_endpoint_creds_path}profile-name") + it 'raises when the first call returns expired credentials' do + # A stale response is treated as a failed refresh. On the initial fetch + # there are no prior credentials to fall back on, so the refresh lifecycle + # raises NoCredentialsError. + stub_request(:get, "#{ipv4_endpoint_creds_path}profile-name") .with(headers: { 'x-aws-ec2-metadata-token' => 'my-token' }) .to_return(status: 200, body: expired_resp) - provider = InstanceProfileCredentials.new(backoff: 0) - creds = provider.credentials - expect(creds.access_key_id).to eq('akid') - assert_requested(expected_request, times: 1) - - # successive requests/credential gets don't result in more calls to imds - provider.credentials - provider.credentials - provider.credentials - - assert_requested(expected_request, times: 1) + expect { InstanceProfileCredentials.new(backoff: 0) } + .to raise_error(Aws::Errors::NoCredentialsError) end - it 'provides credentials after a read timeout during a refresh' do - expect_any_instance_of(InstanceProfileCredentials).to receive(:warn).at_least(:once) + it 'provides cached credentials after a read timeout during a refresh' do expected_request = stub_request(:get, "#{ipv4_endpoint_creds_path}profile-name") .with(headers: { 'x-aws-ec2-metadata-token' => 'my-token' }) @@ -575,6 +564,7 @@ module Aws provider = InstanceProfileCredentials.new(backoff: 0, retries: 0) + # static stability keeps the cached credentials rather than raising creds = provider.credentials expect(creds.access_key_id).to eq('akid-2') From 1978ab0af4e32cf54000a0576a1e74d6111f7035 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Mon, 14 Sep 2026 10:51:00 -0700 Subject: [PATCH 05/18] Update ECS --- .../lib/aws-sdk-core/ecs_credentials.rb | 35 ++++----- .../spec/aws/ecs_credentials_spec.rb | 75 ++++++++----------- 2 files changed, 47 insertions(+), 63 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb index a593e9dbbce..d9e1a793c0a 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb @@ -190,37 +190,32 @@ def backoff(backoff) end def refresh - # Retry loading credentials up to 3 times is the instance metadata - # service is responding but is returning invalid JSON documents - # in response to the GET profile credentials call. - - retry_errors([Aws::Json::ParseError, StandardError], max_retries: 3) do - c = Aws::Json.load(get_credentials.to_s) - @credentials = Credentials.new( - c['AccessKeyId'], - c['SecretAccessKey'], - c['Token'] - ) - @expiration = c['Expiration'] ? Time.iso8601(c['Expiration']) : nil + # Retry loading credentials up to 3 times if the container credential + # service is responding but is returning invalid JSON documents in + # response to the GET credentials call. + c = retry_errors([Aws::Json::ParseError], max_retries: 3) do + Aws::Json.load(retrieve_credentials.to_s) end + @credentials = Credentials.new(c['AccessKeyId'], c['SecretAccessKey'], c['Token']) + @expiration = c['Expiration'] ? Time.iso8601(c['Expiration']) : nil rescue Aws::Json::ParseError raise Aws::Errors::MetadataParserError end - def get_credentials - # Retry loading credentials a configurable number of times if - # the instance metadata service is not responding. + # A missing or malformed authorization token file requires user + # intervention, so it must be raised immediately rather than retried. + def non_recoverable_error?(error) + error.is_a?(TokenFileReadError) || error.is_a?(InvalidTokenError) + end + def retrieve_credentials + # Retry loading credentials a configurable number of times if + # the container credential service is not responding. retry_errors(NETWORK_ERRORS, max_retries: @retries) do open_connection do |conn| http_get(conn, @credential_path) end end - rescue TokenFileReadError, InvalidTokenError - raise - rescue StandardError => e - warn("Error retrieving ECS Credentials: #{e.message}") - '{}' end def fetch_authorization_token diff --git a/gems/aws-sdk-core/spec/aws/ecs_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/ecs_credentials_spec.rb index 0d28aa5a67f..ff18ce95cb7 100644 --- a/gems/aws-sdk-core/spec/aws/ecs_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/ecs_credentials_spec.rb @@ -17,11 +17,10 @@ module Aws SocketError, Timeout::Error ].each do |error_class| - it "returns no credentials for #{error_class}" do + it "raises NoCredentialsError for #{error_class}" do stub_request(:get, "http://169.254.170.2#{path}").to_raise(error_class) - expect_any_instance_of(ECSCredentials).to receive(:warn) - credentials = ECSCredentials.new(credential_path: path, backoff: 0, retries: 0) - expect(credentials.set?).to be(false) + expect { ECSCredentials.new(credential_path: path, backoff: 0, retries: 0) } + .to raise_error(Aws::Errors::NoCredentialsError) end end end @@ -123,28 +122,24 @@ module Aws end.to raise_error(ArgumentError, /without a credential path/) end - it 'returns empty credentials on non-200 response with error details' do + it 'raises NoCredentialsError on non-200 response with error details' do stub_request(:get, "http://169.254.170.2#{path}") .to_return(status: 429, body: 'Rate limit exceeded') - expect_any_instance_of(ECSCredentials).to receive(:warn) - .with(/Error retrieving ECS Credentials: HTTP 429: Rate limit exceeded/) - c = ECSCredentials.new(backoff: 0, retries: 0) - expect(c.set?).to be(false) + expect { ECSCredentials.new(backoff: 0, retries: 0) } + .to raise_error(Aws::Errors::NoCredentialsError) end - it 'returns empty credentials on non-200 response without body' do + it 'raises NoCredentialsError on non-200 response without body' do stub_request(:get, "http://169.254.170.2#{path}") .to_return(status: 500, body: '') - expect_any_instance_of(ECSCredentials).to receive(:warn) - .with(/Error retrieving ECS Credentials: HTTP 500/) - c = ECSCredentials.new(backoff: 0, retries: 0) - expect(c.set?).to be(false) + expect { ECSCredentials.new(backoff: 0, retries: 0) } + .to raise_error(Aws::Errors::NoCredentialsError) end end context 'retries' do it 'defaults to 5' do - stub_request(:get, "http://169.254.170.2#{path}").to_raise(SocketError) + allow_any_instance_of(ECSCredentials).to receive(:refresh) expect(ECSCredentials.new(backoff: 0).retries).to be(5) end @@ -155,10 +150,12 @@ module Aws expect(Kernel).to receive(:sleep).with(1) expect(Kernel).to receive(:sleep).with(2) expect(Kernel).to receive(:sleep).with(4) - ECSCredentials.new( - backoff: ->(n) { Kernel.sleep(2**n) }, - retries: 3 - ) + expect do + ECSCredentials.new( + backoff: ->(n) { Kernel.sleep(2**n) }, + retries: 3 + ) + end.to raise_error(Aws::Errors::NoCredentialsError) assert_requested(expected_request, times: 4) end @@ -185,29 +182,25 @@ module Aws expect(c.expiration.to_s).to eq(expiration2.to_s) end - it 'retries invalid JSON exactly 3 times' do - stub_request(:get, "http://169.254.170.2#{path}") + it 'retries invalid JSON exactly 3 times, then raises NoCredentialsError' do + creds_request = + stub_request(:get, "http://169.254.170.2#{path}") .to_return(status: 200, body: '') .to_return(status: 200, body: ' ') .to_return(status: 200, body: '{') .to_return(status: 200, body: ' ') expect do ECSCredentials.new(backoff: 0, retries: 0) - end.to raise_error( - Aws::Errors::MetadataParserError, - 'Failed to parse metadata service response.' - ) + end.to raise_error(Aws::Errors::NoCredentialsError) + assert_requested(creds_request, times: 4) end - it 'retries errors parsing expiration time 3 times' do + it 'raises NoCredentialsError when the expiration time cannot be parsed' do stub_request(:get, "http://169.254.170.2#{path}") .to_return(status: 200, body: '{ "Expiration": "Expiration" }') - .to_return(status: 200, body: '{ "Expiration": "Expiration" }') - .to_return(status: 200, body: '{ "Expiration": "Expiration" }') - .to_return(status: 200, body: '{ "Expiration": "Expiration" }') expect do ECSCredentials.new(backoff: 0, retries: 0) - end.to raise_error(ArgumentError) + end.to raise_error(Aws::Errors::NoCredentialsError) end end @@ -391,18 +384,11 @@ def setup_response(response) ) end - def handle_expectation(expect) - # hacky, but test cases assume we throw errors - # our credential providers just return nil when not set - case expect['reason'] - when /301 Moved Permanently/, /401 Unauthorized/, - /429 Too Many Requests/, /500 Internal Server Error/ - creds = ECSCredentials.new(backoff: 0, retries: 0) - expect(creds.set?).to be(false) - else - expect { ECSCredentials.new(backoff: 0, retries: 0) } - .to raise_error(RuntimeError) - end + def handle_expectation(_expect) + # A refresh that fails on the initial fetch (no cached credentials to + # fall back on) raises NoCredentialsError. + expect { ECSCredentials.new(backoff: 0, retries: 0) } + .to raise_error(Aws::Errors::NoCredentialsError) end test_cases.each do |test_case| @@ -413,8 +399,11 @@ def handle_expectation(expect) if expect['type'] == 'error' handle_expectation(expect) elsif expect['type'] == 'success' - c = ECSCredentials.new(backoff: 0, retries: 0) credentials = expect['credentials'] + # The fixture's expiration is a fixed timestamp, freeze the clock + # before it so the response is not treated as stale. + allow(Time).to receive(:now).and_return(Time.parse(credentials['expiration']) - 3600) + c = ECSCredentials.new(backoff: 0, retries: 0) expect(c.credentials.access_key_id).to eq(credentials['access_key_id']) expect(c.credentials.secret_access_key).to eq(credentials['secret_access_key']) expect(c.credentials.session_token).to eq(credentials['session_token']) From a952fcdea3f1eee9611cc730268f19b8fab71d27 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Mon, 14 Sep 2026 11:28:04 -0700 Subject: [PATCH 06/18] Add assume role and web id --- .../aws-sdk-core/assume_role_credentials.rb | 18 ++++++++++++++ .../assume_role_web_identity_credentials.rb | 18 ++++++++++++++ .../lib/aws-sdk-core/ecs_credentials.rb | 6 ----- .../spec/aws/assume_role_credentials_spec.rb | 21 +++++++++++++--- ...sume_role_web_identity_credentials_spec.rb | 24 +++++++++++++++---- .../spec/aws/ecs_credentials_spec.rb | 13 +++++++--- 6 files changed, 83 insertions(+), 17 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb index cd52fb74098..6d190b09cde 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb @@ -60,8 +60,26 @@ def initialize(options = {}) # @return [Hash] attr_reader :assume_role_params + # STS error codes that indicate a misconfiguration (bad policy, denied + # access, disabled region, etc). Retrying will not resolve them, so they + # are raised immediately rather than backed off. + # @api private + NON_RECOVERABLE_ERROR_CODES = %w[ + AccessDenied + IDPRejectedClaim + InvalidIdentityToken + MalformedPolicyDocument + PackedPolicyTooLarge + RegionDisabled + ].freeze + private + def non_recoverable_error?(error) + error.is_a?(Aws::Errors::ServiceError) && + NON_RECOVERABLE_ERROR_CODES.include?(error.code) + end + def refresh resp = @client.assume_role(@assume_role_params) creds = resp.credentials diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb index 69088d40dc3..757f791c39c 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb @@ -68,8 +68,26 @@ def initialize(options = {}) # @return [STS::Client] attr_reader :client + # STS error codes that indicate a misconfiguration (bad policy, rejected + # or invalid token, disabled region, etc). Retrying will not resolve them, + # so they are raised immediately rather than backed off. + # @api private + NON_RECOVERABLE_ERROR_CODES = %w[ + AccessDenied + IDPRejectedClaim + InvalidIdentityToken + MalformedPolicyDocument + PackedPolicyTooLarge + RegionDisabled + ].freeze + private + def non_recoverable_error?(error) + error.is_a?(Aws::Errors::ServiceError) && + NON_RECOVERABLE_ERROR_CODES.include?(error.code) + end + def refresh # read from token file everytime it refreshes @assume_role_web_identity_params[:web_identity_token] = _token_from_file(@token_file) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb index d9e1a793c0a..e38b86a1946 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb @@ -202,12 +202,6 @@ def refresh raise Aws::Errors::MetadataParserError end - # A missing or malformed authorization token file requires user - # intervention, so it must be raised immediately rather than retried. - def non_recoverable_error?(error) - error.is_a?(TokenFileReadError) || error.is_a?(InvalidTokenError) - end - def retrieve_credentials # Retry loading credentials a configurable number of times if # the container credential service is not responding. diff --git a/gems/aws-sdk-core/spec/aws/assume_role_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/assume_role_credentials_spec.rb index caadd47f486..e5fffb6c0b7 100644 --- a/gems/aws-sdk-core/spec/aws/assume_role_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/assume_role_credentials_spec.rb @@ -137,8 +137,7 @@ module Aws end it 'refreshes asynchronously' do - # expiration 6 minutes out, within the async exp time window - allow(credentials).to receive(:expiration).and_return(Time.now + (6*60)) + allow(credentials).to receive(:expiration).and_return(Time.now + (2*60)) expect(client).to receive(:assume_role).at_least(2).times expect(Thread).to receive(:new).and_yield c = AssumeRoleCredentials.new( @@ -148,7 +147,7 @@ module Aws end it 'refreshes credentials automatically when they are near expiration' do - allow(credentials).to receive(:expiration).and_return(Time.now) + allow(credentials).to receive(:expiration).and_return(Time.now + 30) expect(client).to receive(:assume_role).exactly(4).times c = AssumeRoleCredentials.new( role_arn: 'arn', @@ -158,6 +157,22 @@ module Aws c.credentials end + it 'raises non-recoverable STS errors immediately instead of backing off' do + error = STS::Errors::AccessDenied.new(nil, 'denied') + allow(client).to receive(:assume_role).and_raise(error) + expect do + AssumeRoleCredentials.new(role_arn: 'arn', role_session_name: 'session') + end.to raise_error(STS::Errors::AccessDenied) + end + + it 'wraps recoverable STS errors as NoCredentialsError on the initial fetch' do + error = STS::Errors::ServiceUnavailable.new(nil, 'try later') + allow(client).to receive(:assume_role).and_raise(error) + expect do + AssumeRoleCredentials.new(role_arn: 'arn', role_session_name: 'session') + end.to raise_error(Aws::Errors::NoCredentialsError) + end + it 'calls before_refresh with self' do before_refresh_called = false before_refresh = proc do |cred_provider| diff --git a/gems/aws-sdk-core/spec/aws/assume_role_web_identity_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/assume_role_web_identity_credentials_spec.rb index 546097f2034..3fa4f04f5b6 100644 --- a/gems/aws-sdk-core/spec/aws/assume_role_web_identity_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/assume_role_web_identity_credentials_spec.rb @@ -89,16 +89,18 @@ module Aws end it 'populates :web_identity_token from file when valid' do + # A missing token file is not considered a non-recoverable error, + # so on the initial fetch it surfaces as NoCredentialsError. expect { AssumeRoleWebIdentityCredentials.new( role_arn: 'arn') - }.to raise_error(Aws::Errors::MissingWebIdentityTokenFile) + }.to raise_error(Aws::Errors::NoCredentialsError) expect { AssumeRoleWebIdentityCredentials.new( role_arn: 'arn', web_identity_token_file: '/not/exist/file/foo', ) - }.to raise_error(Aws::Errors::MissingWebIdentityTokenFile) + }.to raise_error(Aws::Errors::NoCredentialsError) token_file.write('token') token_file.flush @@ -186,9 +188,21 @@ module Aws end end + it 'raises non-recoverable STS errors immediately instead of backing off' do + token_file.write('token') + token_file.flush + error = STS::Errors::InvalidIdentityToken.new(nil, 'bad token') + allow(client).to receive(:assume_role_with_web_identity).and_raise(error) + expect do + AssumeRoleWebIdentityCredentials.new( + role_arn: 'arn', + web_identity_token_file: token_file_path + ) + end.to raise_error(STS::Errors::InvalidIdentityToken) + end + it 'refreshes asynchronously' do - # expiration 6 minutes out, within the async exp time window - allow(credentials).to receive(:expiration).and_return(Time.now + (6*60)) + allow(credentials).to receive(:expiration).and_return(Time.now + (2*60)) expect(client).to receive(:assume_role_with_web_identity).exactly(2).times expect(File).to receive(:read).with(token_file_path).exactly(2).times expect(Thread).to receive(:new).and_yield @@ -201,7 +215,7 @@ module Aws end it 'auto refreshes credentials when near expiration' do - allow(credentials).to receive(:expiration).and_return(Time.now) + allow(credentials).to receive(:expiration).and_return(Time.now + 30) expect(client).to receive(:assume_role_with_web_identity).exactly(4).times expect(File).to receive(:read).with(token_file_path).exactly(4).times diff --git a/gems/aws-sdk-core/spec/aws/ecs_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/ecs_credentials_spec.rb index ff18ce95cb7..058d8561586 100644 --- a/gems/aws-sdk-core/spec/aws/ecs_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/ecs_credentials_spec.rb @@ -211,9 +211,11 @@ module Aws end it 'validates the token for carriage return and newline' do + # A malformed token is not a non-recoverable error, so on the + # initial fetch it surfaces as NoCredentialsError. expect do ECSCredentials.new(backoff: 0, retries: 0) - end.to raise_error(ECSCredentials::InvalidTokenError) + end.to raise_error(Aws::Errors::NoCredentialsError) end end @@ -224,9 +226,11 @@ module Aws end it 'validates the token for carriage return and newline' do + # A malformed token is not a non-recoverable error, so on the + # initial fetch it surfaces as NoCredentialsError. expect do ECSCredentials.new(backoff: 0, retries: 0) - end.to raise_error(ECSCredentials::InvalidTokenError) + end.to raise_error(Aws::Errors::NoCredentialsError) end end end @@ -355,9 +359,12 @@ def setup_request(request) expect = test_case['expect'] if expect['type'] == 'error' + # Host/URI validation fails at construction (ArgumentError). A token + # file read failure happens during the initial fetch and, not being a + # SEP non-recoverable error, surfaces as NoCredentialsError. error = ArgumentError if expect['reason'] =~ /failed to read authorization token/ - error = ECSCredentials::TokenFileReadError + error = Aws::Errors::NoCredentialsError end expect { ECSCredentials.new }.to raise_error(error) elsif expect['type'] == 'success' From 4ce1f7c444fa9cd4586266df42740ef18367a4c9 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Mon, 14 Sep 2026 12:47:21 -0700 Subject: [PATCH 07/18] Add SSO and process --- .../lib/aws-sdk-core/process_credentials.rb | 7 ++----- gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb | 9 +++++++++ gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb | 10 ++++++++++ 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb index 9c60aab0a4c..81fa9bb92ec 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb @@ -36,6 +36,8 @@ def initialize(process) @process = process @credentials = credentials_from_process @async_refresh = false + # The SDK has no visibility into the credential source so static stability must not apply + @static_stability = false @metrics = ['CREDENTIALS_PROCESS'] super end @@ -89,10 +91,5 @@ def _parse_payload_format_v1(creds_json) def refresh @credentials = credentials_from_process end - - def near_expiration?(expiration_length) - # are we within 5 minutes of expiration? - @expiration && (Time.now.to_i + expiration_length) > @expiration.to_i - end end end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb index 0ff11d3e513..fac2ac228f2 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb @@ -124,6 +124,15 @@ def initialize(options = {}) private + # An expired, missing, or malformed cached SSO token (surfaced as + # InvalidSSOCredentials) and an UnauthorizedException from the SSO service + # both require the customer to re-run `aws sso login`, so they are raised + # immediately rather than retried with backoff. + def non_recoverable_error?(error) + error.is_a?(Errors::InvalidSSOCredentials) || + error.is_a?(SSO::Errors::UnauthorizedException) + end + def read_cached_token cached_token = Json.load(File.read(sso_cache_file)) # validation diff --git a/gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb index 5cc05286a6d..f600b0fba8f 100644 --- a/gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb @@ -173,6 +173,7 @@ def mock_token_file(start_url, cached_token) expect(SSO::Client).to receive(:new) .with({region: sso_region, credentials: nil}) .and_return(client) + client.stub_responses(:get_role_credentials, sso_resp) mock_token_file(sso_start_url, cached_token) @@ -184,6 +185,7 @@ def mock_token_file(start_url, cached_token) expect(SSO::Client).to receive(:new) .with({region: sso_region, credentials: nil}) .and_return(client) + client.stub_responses(:get_role_credentials, sso_resp) mock_token_file(sso_start_url, cached_token) @@ -237,6 +239,7 @@ def mock_token_file(start_url, cached_token) it 'sets the client when passed in and does not create a new one' do test_client = client # force construction + test_client.stub_responses(:get_role_credentials, sso_resp) expect(SSO::Client).not_to receive(:new) mock_token_file(sso_start_url, cached_token) @@ -269,6 +272,13 @@ def mock_token_file(start_url, cached_token) expect(sso_creds.expiration).to eq(expiration) end + it 'raises UnauthorizedException immediately instead of backing off' do + client.stub_responses(:get_role_credentials, 'UnauthorizedException') + mock_token_file(sso_start_url, cached_token) + expect { SSOCredentials.new(sso_opts) } + .to raise_error(SSO::Errors::UnauthorizedException) + end + it 'reads a new token from disc for each refresh' do mock_token_file(sso_start_url, cached_token) sso_creds = SSOCredentials.new(sso_opts) From cdcca309fe3ea84929eac6dcf0aa6225f09ef863 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Mon, 14 Sep 2026 13:28:02 -0700 Subject: [PATCH 08/18] Refresh failure logging --- gems/aws-sdk-core/lib/aws-sdk-core/errors.rb | 6 ++++- .../aws-sdk-core/refreshing_credentials.rb | 9 ++++++- .../spec/aws/refreshing_credentials_spec.rb | 26 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/errors.rb b/gems/aws-sdk-core/lib/aws-sdk-core/errors.rb index 884934dee66..3adb9acd06f 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/errors.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/errors.rb @@ -16,7 +16,11 @@ def initialize(*args) # Internal: signals a credential source response whose Expiration is at or # before the current time. Handled within the refresh lifecycle and never # surfaced to callers. - class StaleCredentialsError < RuntimeError; end + class StaleCredentialsError < RuntimeError + def initialize(*args) + super('the credential source returned credentials that are already expired') + end + end # The base class for all errors returned by an Amazon Web Service. # All ~400 level client errors and ~500 level server errors are raised diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb index ef2e737aa2c..af52a00c549 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb @@ -238,6 +238,13 @@ def non_recoverable_error?(_error) false end - def log_refresh_failure(_error); end + def log_refresh_failure(error) + seconds = (@next_refresh_allowed_at - Time.now).round + warn( + "Credential refresh failed: #{error.message}. The SDK will continue " \ + 'using cached credentials. A refresh of these credentials will be ' \ + "attempted again after #{seconds} seconds." + ) + end end end diff --git a/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb index 271fb384e00..2366b1e6943 100644 --- a/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb @@ -72,6 +72,7 @@ def build_resolver(klass, given) end resolver = klass.new(seed) allow(resolver).to receive(:refresh_backoff).and_return(given['refreshBackoffSeconds'] || 300) + allow(resolver).to receive(:warn) resolver end @@ -92,6 +93,31 @@ def assert_result(resolver, expected) end end + describe 'failed refresh messaging' do + it 'logs the source error and next-attempt delay when backing off' do + resolver = build_resolver(resolver_class, 'cachedCredentials' => 'advisory') + allow(resolver).to receive(:refresh_backoff).and_return(300) + resolver.expect_response('error', nil) + + expect(resolver).to receive(:warn).with( + 'Credential refresh failed: recoverable refresh failure. The SDK ' \ + 'will continue using cached credentials. A refresh of these ' \ + 'credentials will be attempted again after 300 seconds.' + ) + + expect(resolver.credentials.access_key_id).to eq(@seeded_akid) + end + + it 'does not log for a non-recoverable error (it is raised instead)' do + resolver = build_resolver(resolver_class, 'cachedCredentials' => 'advisory') + resolver.expect_response('nonRecoverableError', nil) + + expect(resolver).not_to receive(:warn) + + expect { resolver.credentials }.to raise_error(RefreshingCredentialsTestError) + end + end + tests = JSON.load_file(File.join(File.dirname(__FILE__), 'refreshing_credentials_tests.json')) tests.each_with_index do |test, index| From b96e5b0a5ab8a9e70dd69f7509c9f1671c9c40f8 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Mon, 14 Sep 2026 14:06:04 -0700 Subject: [PATCH 09/18] Update invalidate wiring --- .../plugins/retries/error_inspector.rb | 24 +++++-------- .../lib/aws-sdk-core/plugins/retry_errors.rb | 34 +++++++++++++----- .../lib/aws-sdk-core/plugins/sign.rb | 9 +++++ .../plugins/retries/error_inspector_spec.rb | 33 ++++++++--------- .../aws/plugins/retry_errors_legacy_spec.rb | 14 +++++--- .../spec/aws/plugins/retry_errors_spec.rb | 36 +++++++++++++++++++ 6 files changed, 103 insertions(+), 47 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retries/error_inspector.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retries/error_inspector.rb index f799bc7fee2..1b2929516c2 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retries/error_inspector.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retries/error_inspector.rb @@ -6,15 +6,12 @@ module Retries # @api private # This class will be obsolete when APIs contain modeled exceptions class ErrorInspector - EXPIRED_CREDS = Set.new( - [ - 'InvalidClientTokenId', # query services - 'UnrecognizedClientException', # json services - 'InvalidAccessKeyId', # s3 - 'AuthFailure', # ec2 - 'InvalidIdentityToken', # sts - 'ExpiredToken', # route53 - 'ExpiredTokenException' # kinesis + # Target-service authentication failures that indicate the cached + # credentials are no longer valid. + INVALIDATING_AUTH_ERRORS = Set.new( + %w[ + ExpiredToken + InvalidToken ] ) @@ -71,8 +68,8 @@ def initialize(error, http_status_code) @http_status_code = http_status_code end - def expired_credentials? - !!(EXPIRED_CREDS.include?(@name) || @name.match(/expired/i)) + def invalidating_auth_error? + INVALIDATING_AUTH_ERRORS.include?(@name) end def throttling_error? @@ -124,16 +121,11 @@ def retryable?(context) networking? || checksum? || endpoint_discovery?(context) || - (expired_credentials? && refreshable_credentials?(context)) || clock_skew?(context) end private - def refreshable_credentials?(context) - context.config.credentials.respond_to?(:refresh!) - end - def extract_name(error) if error.is_a?(Errors::ServiceError) error.class.code || error.class.name.to_s diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb index 6a0353f3d01..25edd6bfeb5 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb @@ -289,6 +289,11 @@ def call(context) # Estimated skew needs to be updated on every request config.clock_skew.update_estimated_skew(context) + # A target-service authentication failure invalidates the cached + # credentials so the next request refreshes. The rejected request + # itself is not retried. + invalidate_credentials(context, error_inspector) + return response unless retryable?(context, response, error_inspector) return response if context.retries >= config.max_attempts - 1 @@ -410,15 +415,19 @@ def parse_retry_after(context) def retry_request(context, error) context.retries += 1 - context.config.credentials.refresh! if refresh_credentials?(context, error) context.http_request.body.rewind context.http_response.reset call(context) end - def refresh_credentials?(context, error) - error.expired_credentials? && - context.config.credentials.respond_to?(:refresh!) + def invalidate_credentials(context, error_inspector) + return unless error_inspector.invalidating_auth_error? + + provider = context.config.credentials + signed_with = context[:signing_credentials] + return unless provider.respond_to?(:invalidate) && signed_with + + provider.invalidate(signed_with) end def add_retry_headers(context) @@ -465,6 +474,11 @@ def call(context) context.config.endpoint_cache.delete(key) end + # A target-service authentication failure invalidates the cached + # credentials so the next request refreshes. The rejected request + # itself is not retried. + invalidate_credentials(context, error_inspector) + retry_if_possible(response, error_inspector) else response @@ -489,7 +503,6 @@ def retry_if_possible(response, error_inspector) def retry_request(context, error) delay_retry(context) context.retries += 1 - context.config.credentials.refresh! if refresh_credentials?(context, error) context.http_request.body.rewind context.http_response.reset call(context) @@ -505,9 +518,14 @@ def should_retry?(context, error) response_truncatable?(context) end - def refresh_credentials?(context, error) - error.expired_credentials? && - context.config.credentials.respond_to?(:refresh!) + def invalidate_credentials(context, error_inspector) + return unless error_inspector.invalidating_auth_error? + + provider = context.config.credentials + signed_with = context[:signing_credentials] + return unless provider.respond_to?(:invalidate) && signed_with + + provider.invalidate(signed_with) end def retry_limit(context) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb index f08e2ffdb7e..1434b3e5325 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb @@ -147,6 +147,15 @@ def sign(context) # apply signature headers req.headers.update(signature.headers) + # Record the credentials used to sign this request. If the target + # service later rejects it as an authentication failure, the retry + # layer invalidates cached credentials if they match these credentials + # to prevent potentially invalidating valid credentials refreshed + # by a concurrent refresh. + if (provider = @signer.credentials_provider) + context[:signing_credentials] = provider.credentials + end + # add request metadata with signature components for debugging context[:canonical_request] = signature.canonical_request context[:string_to_sign] = signature.string_to_sign diff --git a/gems/aws-sdk-core/spec/aws/plugins/retries/error_inspector_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/retries/error_inspector_spec.rb index 24c011bbc9d..4c96a3306b5 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/retries/error_inspector_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/retries/error_inspector_spec.rb @@ -11,36 +11,33 @@ def inspector(error, http_status_code = 404) Retries::ErrorInspector.new(error, http_status_code) end - describe '#expired_credentials?' do - expired_credentials_errors = [ - RetryErrorsSvc::Errors::UnrecognizedClientException, - RetryErrorsSvc::Errors::InvalidClientTokenId, - RetryErrorsSvc::Errors::InvalidAccessKeyId, - RetryErrorsSvc::Errors::AuthFailure, - RetryErrorsSvc::Errors::InvalidIdentityToken, + describe '#invalidating_auth_error?' do + invalidating_errors = [ RetryErrorsSvc::Errors::ExpiredToken, - RetryErrorsSvc::Errors::ExpiredTokenException + RetryErrorsSvc::Errors::InvalidToken ] - expired_credentials_errors.each do |e| + invalidating_errors.each do |e| it "returns true for #{e.name}" do - expect(inspector(e).expired_credentials?).to be(true) + expect(inspector(e).invalidating_auth_error?).to be(true) end end - it 'returns true for error types that match /expired/' do + it 'returns false for authorization errors like AccessDenied' do expect( - inspector( - RetryErrorsSvc::Errors::SomethingExpiredError - ).expired_credentials? - ).to be(true) + inspector(RetryErrorsSvc::Errors::AccessDenied).invalidating_auth_error? + ).to be(false) + end + + it 'returns false for other credential errors not named by the SEP' do + expect( + inspector(RetryErrorsSvc::Errors::ExpiredTokenException).invalidating_auth_error? + ).to be(false) end it 'returns false for other errors' do expect( - inspector( - RetryErrorsSvc::Errors::SomeRandomError - ).expired_credentials? + inspector(RetryErrorsSvc::Errors::SomeRandomError).invalidating_auth_error? ).to be(false) end end diff --git a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb index 7e8a91b7902..8b29e2fc7a9 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb @@ -210,12 +210,16 @@ module Plugins expect(resp.context.retries).to eq(0) end - it 'retries if creds expire and are refreshable' do - # Note: this adds the refresh! method to credentials - expect(credentials).to receive(:refresh!).exactly(3).times - resp.error = RetryErrorsSvc::Errors::AuthFailure.new(nil, nil) + it 'invalidates the signing credentials and does not retry on an auth failure' do + provider = double('credential_provider', invalidate: nil) + signing_credentials = Credentials.new('akid', 'secret') + config.credentials = provider + resp.context[:signing_credentials] = signing_credentials + + expect(provider).to receive(:invalidate).with(signing_credentials) + resp.error = RetryErrorsSvc::Errors::ExpiredToken.new(nil, nil) handle { |_context| resp } - expect(resp.context.retries).to eq(3) + expect(resp.context.retries).to eq(0) end it 'does not call refresh! when error is expired credentials and clock skew' do diff --git a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb index 9cb05c92602..08d395dd4bb 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb @@ -355,6 +355,42 @@ module Plugins handle_with_retry(test_case_def) end + context 'credential invalidation on authentication failure' do + let(:provider) { double('credential_provider', invalidate: nil) } + let(:signing_credentials) { Credentials.new('akid', 'secret') } + + before(:each) do + config.credentials = provider + resp.context[:signing_credentials] = signing_credentials + end + + it 'invalidates the signing credentials and does not retry on an auth failure' do + expect(provider).to receive(:invalidate).with(signing_credentials) + + resp.context.http_response.status_code = 400 + resp.error = RetryErrorsSvc::Errors::ExpiredToken.new(nil, nil) + handle { |_context| resp } + + expect(resp.context.retries).to eq(0) + end + + it 'does not invalidate for an authorization error such as AccessDenied' do + expect(provider).not_to receive(:invalidate) + + resp.context.http_response.status_code = 400 + resp.error = RetryErrorsSvc::Errors::AccessDenied.new(nil, nil) + handle { |_context| resp } + end + + it 'does not invalidate when the provider does not support it' do + config.credentials = Credentials.new('akid', 'secret') + + resp.context.http_response.status_code = 400 + resp.error = RetryErrorsSvc::Errors::ExpiredToken.new(nil, nil) + expect { handle { |_context| resp } }.not_to raise_error + end + end + context 'DynamoDB base backoff and increased retries' do let(:api) do api = Seahorse::Model::Api.new From e14ac086f51425ffb6cf74b4b19b3d4554bccff0 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Tue, 15 Sep 2026 14:27:41 -0700 Subject: [PATCH 10/18] Add concurrency tests --- .../spec/aws/refreshing_credentials_spec.rb | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb index 2366b1e6943..e2cceaba1cb 100644 --- a/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb @@ -148,5 +148,114 @@ def assert_result(resolver, expected) end end end + + describe 'concurrency' do + let(:gated_resolver_class) do + Class.new do + include RefreshingCredentials + + attr_reader :source_calls, :entered, :release + + def initialize(seed) + @mutex = Mutex.new + @static_stability = true + @next_refresh_allowed_at = nil + @cached_error = nil + @cached_error_expires_at = nil + @advisory_window = seed[:advisory_window] + @credentials = seed[:credentials] + @expiration = seed[:expiration] + @source_calls = 0 + @entered = Queue.new + @release = Queue.new + end + + # Signals that the source has been entered, then blocks until the + # test releases it. Runs under the refresh lock, so @source_calls + # increments are already serialized. + def refresh + @source_calls += 1 + @entered << :in + @release.pop + @credentials = Credentials.new('FRESH-AKID', 'secret', 'token') + @expiration = Time.now + 3600 + end + + def non_recoverable_error?(_error) + false + end + end + end + + def build_gated_resolver(ttl:, advisory_window:) + gated_resolver_class.new( + credentials: Credentials.new('CACHED-AKID', 'secret', 'token'), + expiration: Time.now + ttl, + advisory_window: advisory_window + ) + end + + SWARM_SIZE = 8 + + it 'advisory: one refresh runs while a swarm of callers get cached creds immediately' do + # Inside the advisory window (600s) but outside the mandatory window (60s). + resolver = build_gated_resolver(ttl: 300, advisory_window: 600) + + refresher = Thread.new { resolver.credentials } + resolver.entered.pop # the refresher now holds the lock inside #refresh + + callers = Array.new(SWARM_SIZE) { Thread.new { resolver.credentials.access_key_id } } + results = callers.map(&:value) + + expect(results).to all(eq('CACHED-AKID')) + expect(resolver.source_calls).to eq(1) # only the refresher contacted the source + + resolver.release << :go + refresher.join + + expect(resolver.source_calls).to eq(1) + expect(resolver.credentials.access_key_id).to eq('FRESH-AKID') + end + + it 'mandatory: one refresh runs while a swarm of callers wait and reuse it' do + # Inside the mandatory window (60s). + resolver = build_gated_resolver(ttl: 30, advisory_window: 600) + + refresher = Thread.new { resolver.credentials } + resolver.entered.pop # the refresher now holds the lock inside #refresh + + waiters = Array.new(SWARM_SIZE) { Thread.new { resolver.credentials.access_key_id } } + + # Give waiters time to queue on the lock + sleep 0.1 + expect(resolver.source_calls).to eq(1) + + resolver.release << :go + refresher.join + results = waiters.map(&:value) + + # Exactly one source call was made and every waiter reused that result + expect(resolver.source_calls).to eq(1) + expect(results).to all(eq('FRESH-AKID')) + end + + it 'invalidate does not block or interfere while a refresh holds the lock' do + resolver = build_gated_resolver(ttl: 30, advisory_window: 600) + + refresher = Thread.new { resolver.credentials } + resolver.entered.pop # the refresher now holds the lock inside #refresh + + # invalidate uses try_lock: with the refresh lock held it returns + # immediately without waiting and without mutating state. + rejected = double('identity', access_key_id: 'CACHED-AKID') + expect { resolver.invalidate(rejected) }.not_to raise_error + + resolver.release << :go + refresher.join + + expect(resolver.source_calls).to eq(1) + expect(resolver.credentials.access_key_id).to eq('FRESH-AKID') + end + end end end From e4a01807ac8bb31d422fdfc4920528a45aaf83c2 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Wed, 16 Sep 2026 09:01:39 -0700 Subject: [PATCH 11/18] Add guard for advisory/mandatory configuration --- .../aws-sdk-core/refreshing_credentials.rb | 6 +++++ .../spec/aws/refreshing_credentials_spec.rb | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb index af52a00c549..f4181a40f3d 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb @@ -30,6 +30,12 @@ def initialize(options = {}) @before_refresh = options.delete(:before_refresh) @configured_advisory_window = options.delete(:advisory_refresh_window) end + # mandatory refresh window must not exceed the advisory window + if @configured_advisory_window && @configured_advisory_window < MANDATORY_REFRESH_WINDOW + raise ArgumentError, + "advisory_refresh_window (#{@configured_advisory_window}) must be at least " \ + "the mandatory refresh window (#{MANDATORY_REFRESH_WINDOW} seconds)" + end @static_stability = true if @static_stability.nil? @next_refresh_allowed_at = nil @cached_error = nil diff --git a/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb index e2cceaba1cb..c1cf84c1bfd 100644 --- a/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb @@ -118,6 +118,29 @@ def assert_result(resolver, expected) end end + describe 'advisory window configuration' do + let(:validating_resolver_class) do + Class.new do + include RefreshingCredentials + + def refresh + @credentials = Credentials.new('AKID', 'secret', 'token') + @expiration = Time.now + 3600 + end + end + end + + it 'rejects an advisory window smaller than the mandatory window' do + expect { validating_resolver_class.new(advisory_refresh_window: 30) } + .to raise_error(ArgumentError, /must be at least the mandatory refresh window/) + end + + it 'accepts an advisory window at or above the mandatory window' do + expect { validating_resolver_class.new(advisory_refresh_window: 60) } + .not_to raise_error + end + end + tests = JSON.load_file(File.join(File.dirname(__FILE__), 'refreshing_credentials_tests.json')) tests.each_with_index do |test, index| From 477c05be513a1fa87f1c0c02fde9189431c814e1 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Wed, 16 Sep 2026 09:10:31 -0700 Subject: [PATCH 12/18] Update cognito credentials --- .../customizations/cognito_identity_credentials.rb | 2 -- .../spec/cognito_identity_credentials_spec.rb | 5 +++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/gems/aws-sdk-cognitoidentity/lib/aws-sdk-cognitoidentity/customizations/cognito_identity_credentials.rb b/gems/aws-sdk-cognitoidentity/lib/aws-sdk-cognitoidentity/customizations/cognito_identity_credentials.rb index 42a712ab2db..b55ab7f16fb 100644 --- a/gems/aws-sdk-cognitoidentity/lib/aws-sdk-cognitoidentity/customizations/cognito_identity_credentials.rb +++ b/gems/aws-sdk-cognitoidentity/lib/aws-sdk-cognitoidentity/customizations/cognito_identity_credentials.rb @@ -116,8 +116,6 @@ def identity_id private def refresh - @before_refresh&.call(self) - resp = @client.get_credentials_for_identity( identity_id: identity_id, custom_role_arn: @custom_role_arn, diff --git a/gems/aws-sdk-cognitoidentity/spec/cognito_identity_credentials_spec.rb b/gems/aws-sdk-cognitoidentity/spec/cognito_identity_credentials_spec.rb index 9eebcf7998a..c28ed8a93f1 100644 --- a/gems/aws-sdk-cognitoidentity/spec/cognito_identity_credentials_spec.rb +++ b/gems/aws-sdk-cognitoidentity/spec/cognito_identity_credentials_spec.rb @@ -34,6 +34,11 @@ module CognitoIdentity end let(:resp) { double('client-resp', credentials: cognito_creds) } + # credential lifecycle fetches during construction + before do + allow(client).to receive(:get_credentials_for_identity).and_return(resp) + end + describe '#initialize' do it 'constructs a client with passed arguments when not given' do From d5d69b9a4a0cc9498f27d22d3d8a1b096433c01d Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Wed, 16 Sep 2026 10:57:10 -0700 Subject: [PATCH 13/18] Update S3 express credentials --- gems/aws-sdk-s3/lib/aws-sdk-s3/express_credentials.rb | 8 +++++--- .../aws-sdk-s3/spec/access_grants_credentials_spec.rb | 11 +++++------ gems/aws-sdk-s3/spec/express_credentials_spec.rb | 2 +- gems/aws-sdk-s3/spec/plugins/access_grants_spec.rb | 8 ++++++++ 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/express_credentials.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/express_credentials.rb index 5c9d9758fad..16d34f4f8ee 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/express_credentials.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/express_credentials.rb @@ -9,8 +9,7 @@ class ExpressCredentials include CredentialProvider include RefreshingCredentials - SYNC_EXPIRATION_LENGTH = 60 # 1 minute - ASYNC_EXPIRATION_LENGTH = 120 # 2 minutes + ADVISORY_REFRESH_WINDOW = 120 # 2 minutes def initialize(options = {}) @client = options[:client] @@ -21,7 +20,10 @@ def initialize(options = {}) end end @async_refresh = true - super + # Session credentials are rejected once expired, so static stability + # must not apply. + @static_stability = false + super(options.merge(advisory_refresh_window: ADVISORY_REFRESH_WINDOW)) end # @return [S3::Client] diff --git a/gems/aws-sdk-s3/spec/access_grants_credentials_spec.rb b/gems/aws-sdk-s3/spec/access_grants_credentials_spec.rb index bf986aad8d7..8e5455dace1 100644 --- a/gems/aws-sdk-s3/spec/access_grants_credentials_spec.rb +++ b/gems/aws-sdk-s3/spec/access_grants_credentials_spec.rb @@ -11,9 +11,9 @@ module S3 Aws::S3Control::Client.new(region: 'us-east-1', stub_responses: true) end - let(:in_five_minutes) { Time.now + 60 * 5 } + let(:in_one_hour) { Time.now + 60 * 60 } - let(:expiration) { in_five_minutes } + let(:expiration) { in_one_hour } let(:credentials) do double('credentials', @@ -64,7 +64,7 @@ module S3 expect(c.credentials.access_key_id).to eq('akid') expect(c.credentials.secret_access_key).to eq('secret') expect(c.credentials.session_token).to eq('session') - expect(c.expiration).to eq(in_five_minutes) + expect(c.expiration).to eq(in_one_hour) end it 'provides the matched grant target' do @@ -79,8 +79,7 @@ module S3 end it 'refreshes asynchronously' do - # expiration 9.5 minutes out, within the async exp time window - time = Time.now + 60 * 9.5 + time = Time.now + 60 * 2 allow(credentials).to receive(:expiration).and_return(time) expect(client).to receive(:get_data_access).at_least(2).times expect(Thread).to receive(:new).and_yield @@ -94,7 +93,7 @@ module S3 end it 'refreshes credentials automatically when they are near expiration' do - allow(credentials).to receive(:expiration).and_return(Time.now) + allow(credentials).to receive(:expiration).and_return(Time.now + 30) expect(client).to receive(:get_data_access).exactly(4).times c = AccessGrantsCredentials.new( client: client, diff --git a/gems/aws-sdk-s3/spec/express_credentials_spec.rb b/gems/aws-sdk-s3/spec/express_credentials_spec.rb index 13e928be96d..f1a690a433a 100644 --- a/gems/aws-sdk-s3/spec/express_credentials_spec.rb +++ b/gems/aws-sdk-s3/spec/express_credentials_spec.rb @@ -69,7 +69,7 @@ module S3 end it 'refreshes credentials automatically when they are near expiration' do - allow(credentials).to receive(:expiration).and_return(Time.now) + allow(credentials).to receive(:expiration).and_return(Time.now + 30) expect(client).to receive(:create_session).exactly(4).times c = ExpressCredentials.new( client: client, diff --git a/gems/aws-sdk-s3/spec/plugins/access_grants_spec.rb b/gems/aws-sdk-s3/spec/plugins/access_grants_spec.rb index eb0901e38c1..f24be9ad25a 100644 --- a/gems/aws-sdk-s3/spec/plugins/access_grants_spec.rb +++ b/gems/aws-sdk-s3/spec/plugins/access_grants_spec.rb @@ -68,6 +68,14 @@ module S3 end it 'is skipped for s3 express endpoints' do + # Express endpoint resolves S3 Express credentials which fetch + # session on construction. + client.stub_responses(:create_session, credentials: { + access_key_id: 's3-akid', + secret_access_key: 's3-secret', + session_token: 's3-session', + expiration: Time.now + 60 * 5 + }) expect_any_instance_of(Aws::S3::AccessGrantsCredentialsProvider) .not_to receive(:access_grants_credentials_for) client.head_object(bucket: 'bucket--use1-az2--x-s3', key: 'key') From 24efe0397d0459b3544cda1f09d512a0f23182b0 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Mon, 21 Sep 2026 10:34:08 -0700 Subject: [PATCH 14/18] Add InvalidSSOToken to non recoverable errors --- gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb | 10 ++++++---- gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb | 10 ++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb index fac2ac228f2..1bcf54c504e 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb @@ -124,12 +124,14 @@ def initialize(options = {}) private - # An expired, missing, or malformed cached SSO token (surfaced as - # InvalidSSOCredentials) and an UnauthorizedException from the SSO service - # both require the customer to re-run `aws sso login`, so they are raised - # immediately rather than retried with backoff. + # An expired, missing, or malformed cached SSO token (InvalidSSOCredentials + # for legacy profiles, InvalidSSOToken for sso_session profiles) and an + # UnauthorizedException from the SSO service all require the customer to + # re-run `aws sso login`, so they are raised immediately rather than + # retried with backoff. def non_recoverable_error?(error) error.is_a?(Errors::InvalidSSOCredentials) || + error.is_a?(Errors::InvalidSSOToken) || error.is_a?(SSO::Errors::UnauthorizedException) end diff --git a/gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb index f600b0fba8f..0371c4b50f3 100644 --- a/gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb @@ -139,6 +139,16 @@ def mock_token_file(start_url, cached_token) sso_creds.credentials end + + it 'raises InvalidSSOToken immediately when the token expires on a later refresh' do + sso_creds = SSOCredentials.new(sso_opts) + allow(Time).to receive(:now).and_return(expiration + 60) + allow(token_provider).to receive(:token) + .and_raise(Errors::InvalidSSOToken.new(SSOCredentials::SSO_LOGIN_GUIDANCE)) + + expect { sso_creds.credentials }.to raise_error(Errors::InvalidSSOToken) + expect(sso_creds.rate_limited?).to be(false) + end end describe '#expiration' do From 502c4d6b468193fde796d3ef12c4e8e76c1a1a98 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Thu, 24 Sep 2026 09:20:10 -0700 Subject: [PATCH 15/18] Polish --- .../aws-sdk-core/credential_provider_chain.rb | 8 +- gems/aws-sdk-core/lib/aws-sdk-core/errors.rb | 17 ---- .../lib/aws-sdk-core/plugins/retry_errors.rb | 4 +- .../lib/aws-sdk-core/plugins/sign.rb | 11 +-- .../lib/aws-sdk-core/process_credentials.rb | 2 +- .../aws-sdk-core/refreshing_credentials.rb | 20 +++-- .../spec/aws/assume_role_credentials_spec.rb | 18 +++-- ...sume_role_web_identity_credentials_spec.rb | 30 +++---- .../spec/aws/ecs_credentials_spec.rb | 38 ++++----- .../aws/instance_profile_credentials_spec.rb | 37 +++++---- .../aws/plugins/retry_errors_legacy_spec.rb | 2 +- .../spec/aws/refreshing_credentials_spec.rb | 78 ++++++++++++++----- .../aws/refreshing_credentials_tests.json | 24 ++++++ 13 files changed, 171 insertions(+), 118 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/credential_provider_chain.rb b/gems/aws-sdk-core/lib/aws-sdk-core/credential_provider_chain.rb index 9fa05a11379..223aeebbd46 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/credential_provider_chain.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/credential_provider_chain.rb @@ -239,10 +239,10 @@ def instance_profile_credentials(options) elsif !(ENV.fetch('AWS_EC2_METADATA_DISABLED', 'false').downcase == 'true') InstanceProfileCredentials.new(options.merge(profile: profile_name)) end - rescue Errors::NoCredentialsError - # The credential source was unreachable on the initial fetch, skip this - # provider so the chain moves on. Non-recoverable errors are not - # NoCredentialsError and still propagate. + rescue Errors::MissingCredentialsError + # credential source was unreachable on initial fetch, skip so chain moves on. + # Non-recoverable errors are the source error (not MissingCredentialsError) + # and still propagate. nil end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/errors.rb b/gems/aws-sdk-core/lib/aws-sdk-core/errors.rb index 3adb9acd06f..dc41bb36ee1 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/errors.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/errors.rb @@ -5,23 +5,6 @@ module Errors class NonSupportedRubyVersionError < RuntimeError; end - # Raised when no credentials have been obtained and the initial fetch - # from the credential source failed. - class NoCredentialsError < RuntimeError - def initialize(*args) - super('unable to obtain credentials from the credential source') - end - end - - # Internal: signals a credential source response whose Expiration is at or - # before the current time. Handled within the refresh lifecycle and never - # surfaced to callers. - class StaleCredentialsError < RuntimeError - def initialize(*args) - super('the credential source returned credentials that are already expired') - end - end - # The base class for all errors returned by an Amazon Web Service. # All ~400 level client errors and ~500 level server errors are raised # as service errors. This indicates it was an error returned from the diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb index 25edd6bfeb5..1705c206b6b 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb @@ -290,7 +290,7 @@ def call(context) config.clock_skew.update_estimated_skew(context) # A target-service authentication failure invalidates the cached - # credentials so the next request refreshes. The rejected request + # credentials so the next request refreshes and the rejected request # itself is not retried. invalidate_credentials(context, error_inspector) @@ -475,7 +475,7 @@ def call(context) end # A target-service authentication failure invalidates the cached - # credentials so the next request refreshes. The rejected request + # credentials so the next request refreshes and the rejected request # itself is not retried. invalidate_credentials(context, error_inspector) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb index 1434b3e5325..60344d24f24 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb @@ -147,14 +147,9 @@ def sign(context) # apply signature headers req.headers.update(signature.headers) - # Record the credentials used to sign this request. If the target - # service later rejects it as an authentication failure, the retry - # layer invalidates cached credentials if they match these credentials - # to prevent potentially invalidating valid credentials refreshed - # by a concurrent refresh. - if (provider = @signer.credentials_provider) - context[:signing_credentials] = provider.credentials - end + # Record the signing credentials so the retry layer can invalidate + # them on an auth failure and only if they still match + context[:signing_credentials] = @signer.credentials_provider.credentials # add request metadata with signature components for debugging context[:canonical_request] = signature.canonical_request diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb index 81fa9bb92ec..1e01caeb390 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb @@ -36,7 +36,7 @@ def initialize(process) @process = process @credentials = credentials_from_process @async_refresh = false - # The SDK has no visibility into the credential source so static stability must not apply + # SDK has no visibility into the credential source so static stability must not apply @static_stability = false @metrics = ['CREDENTIALS_PROCESS'] super diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb index f4181a40f3d..bad9eb3d7c4 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb @@ -21,6 +21,15 @@ module RefreshingCredentials CLIENT_EXCLUDE_OPTIONS = Set.new([:before_refresh]).freeze + # @api private + # Signals a credential source response whose Expiration is at or before + # the current time. + class StaleCredentialsError < RuntimeError + def initialize(*_args) + super('the credential source returned credentials that are already expired') + end + end + # @param [Hash] options # @option options [Proc] :before_refresh A Proc called before credentials are refreshed. # It accepts `self` as the only argument. @@ -108,7 +117,7 @@ def fetch_initial_credentials cache_non_recoverable_error(error) raise error end - raise Errors::NoCredentialsError + raise Errors::MissingCredentialsError end end @@ -162,17 +171,16 @@ def perform_refresh(mandatory:, raise_to_caller:) handle_failure(error, raise_to_caller: raise_to_caller) end - # Calls the source via #refresh. Returns nil on success, or an error (a - # raised error, or a stale response whose Expiration is at or before now). - # Restores the prior credentials on failure so a failed or stale refresh - # never discards the cached credentials. + # Calls the source via #refresh. Returns nil on success, or restores + # prior credentials and returns error on failure so a failed or stale + # refresh never discards the cached credentials. def call_source prior = [@credentials, @expiration] @before_refresh&.call(self) refresh if !@expiration.nil? && @expiration <= Time.now @credentials, @expiration = prior - return Errors::StaleCredentialsError.new + return StaleCredentialsError.new end nil rescue StandardError => e diff --git a/gems/aws-sdk-core/spec/aws/assume_role_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/assume_role_credentials_spec.rb index e5fffb6c0b7..99e11f6a13a 100644 --- a/gems/aws-sdk-core/spec/aws/assume_role_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/assume_role_credentials_spec.rb @@ -157,20 +157,22 @@ module Aws c.credentials end - it 'raises non-recoverable STS errors immediately instead of backing off' do - error = STS::Errors::AccessDenied.new(nil, 'denied') - allow(client).to receive(:assume_role).and_raise(error) - expect do - AssumeRoleCredentials.new(role_arn: 'arn', role_session_name: 'session') - end.to raise_error(STS::Errors::AccessDenied) + AssumeRoleCredentials::NON_RECOVERABLE_ERROR_CODES.each do |code| + it "raises non-recoverable STS error #{code} immediately instead of backing off" do + error = STS::Errors.error_class(code).new(nil, 'nope') + allow(client).to receive(:assume_role).and_raise(error) + expect do + AssumeRoleCredentials.new(role_arn: 'arn', role_session_name: 'session') + end.to raise_error(error.class) + end end - it 'wraps recoverable STS errors as NoCredentialsError on the initial fetch' do + it 'wraps recoverable STS errors as MissingCredentialsError on the initial fetch' do error = STS::Errors::ServiceUnavailable.new(nil, 'try later') allow(client).to receive(:assume_role).and_raise(error) expect do AssumeRoleCredentials.new(role_arn: 'arn', role_session_name: 'session') - end.to raise_error(Aws::Errors::NoCredentialsError) + end.to raise_error(Aws::Errors::MissingCredentialsError) end it 'calls before_refresh with self' do diff --git a/gems/aws-sdk-core/spec/aws/assume_role_web_identity_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/assume_role_web_identity_credentials_spec.rb index 3fa4f04f5b6..b93da352804 100644 --- a/gems/aws-sdk-core/spec/aws/assume_role_web_identity_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/assume_role_web_identity_credentials_spec.rb @@ -90,17 +90,17 @@ module Aws it 'populates :web_identity_token from file when valid' do # A missing token file is not considered a non-recoverable error, - # so on the initial fetch it surfaces as NoCredentialsError. + # so on the initial fetch it surfaces as MissingCredentialsError. expect { AssumeRoleWebIdentityCredentials.new( role_arn: 'arn') - }.to raise_error(Aws::Errors::NoCredentialsError) + }.to raise_error(Aws::Errors::MissingCredentialsError) expect { AssumeRoleWebIdentityCredentials.new( role_arn: 'arn', web_identity_token_file: '/not/exist/file/foo', ) - }.to raise_error(Aws::Errors::NoCredentialsError) + }.to raise_error(Aws::Errors::MissingCredentialsError) token_file.write('token') token_file.flush @@ -188,17 +188,19 @@ module Aws end end - it 'raises non-recoverable STS errors immediately instead of backing off' do - token_file.write('token') - token_file.flush - error = STS::Errors::InvalidIdentityToken.new(nil, 'bad token') - allow(client).to receive(:assume_role_with_web_identity).and_raise(error) - expect do - AssumeRoleWebIdentityCredentials.new( - role_arn: 'arn', - web_identity_token_file: token_file_path - ) - end.to raise_error(STS::Errors::InvalidIdentityToken) + AssumeRoleWebIdentityCredentials::NON_RECOVERABLE_ERROR_CODES.each do |code| + it "raises non-recoverable STS error #{code} immediately instead of backing off" do + token_file.write('token') + token_file.flush + error = STS::Errors.error_class(code).new(nil, 'nope') + allow(client).to receive(:assume_role_with_web_identity).and_raise(error) + expect do + AssumeRoleWebIdentityCredentials.new( + role_arn: 'arn', + web_identity_token_file: token_file_path + ) + end.to raise_error(error.class) + end end it 'refreshes asynchronously' do diff --git a/gems/aws-sdk-core/spec/aws/ecs_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/ecs_credentials_spec.rb index 058d8561586..26fff23b01b 100644 --- a/gems/aws-sdk-core/spec/aws/ecs_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/ecs_credentials_spec.rb @@ -17,10 +17,10 @@ module Aws SocketError, Timeout::Error ].each do |error_class| - it "raises NoCredentialsError for #{error_class}" do + it "raises MissingCredentialsError for #{error_class}" do stub_request(:get, "http://169.254.170.2#{path}").to_raise(error_class) expect { ECSCredentials.new(credential_path: path, backoff: 0, retries: 0) } - .to raise_error(Aws::Errors::NoCredentialsError) + .to raise_error(Aws::Errors::MissingCredentialsError) end end end @@ -122,18 +122,18 @@ module Aws end.to raise_error(ArgumentError, /without a credential path/) end - it 'raises NoCredentialsError on non-200 response with error details' do + it 'raises MissingCredentialsError on non-200 response with error details' do stub_request(:get, "http://169.254.170.2#{path}") .to_return(status: 429, body: 'Rate limit exceeded') expect { ECSCredentials.new(backoff: 0, retries: 0) } - .to raise_error(Aws::Errors::NoCredentialsError) + .to raise_error(Aws::Errors::MissingCredentialsError) end - it 'raises NoCredentialsError on non-200 response without body' do + it 'raises MissingCredentialsError on non-200 response without body' do stub_request(:get, "http://169.254.170.2#{path}") .to_return(status: 500, body: '') expect { ECSCredentials.new(backoff: 0, retries: 0) } - .to raise_error(Aws::Errors::NoCredentialsError) + .to raise_error(Aws::Errors::MissingCredentialsError) end end @@ -155,7 +155,7 @@ module Aws backoff: ->(n) { Kernel.sleep(2**n) }, retries: 3 ) - end.to raise_error(Aws::Errors::NoCredentialsError) + end.to raise_error(Aws::Errors::MissingCredentialsError) assert_requested(expected_request, times: 4) end @@ -182,7 +182,7 @@ module Aws expect(c.expiration.to_s).to eq(expiration2.to_s) end - it 'retries invalid JSON exactly 3 times, then raises NoCredentialsError' do + it 'retries invalid JSON exactly 3 times, then raises MissingCredentialsError' do creds_request = stub_request(:get, "http://169.254.170.2#{path}") .to_return(status: 200, body: '') @@ -191,16 +191,16 @@ module Aws .to_return(status: 200, body: ' ') expect do ECSCredentials.new(backoff: 0, retries: 0) - end.to raise_error(Aws::Errors::NoCredentialsError) + end.to raise_error(Aws::Errors::MissingCredentialsError) assert_requested(creds_request, times: 4) end - it 'raises NoCredentialsError when the expiration time cannot be parsed' do + it 'raises MissingCredentialsError when the expiration time cannot be parsed' do stub_request(:get, "http://169.254.170.2#{path}") .to_return(status: 200, body: '{ "Expiration": "Expiration" }') expect do ECSCredentials.new(backoff: 0, retries: 0) - end.to raise_error(Aws::Errors::NoCredentialsError) + end.to raise_error(Aws::Errors::MissingCredentialsError) end end @@ -212,10 +212,10 @@ module Aws it 'validates the token for carriage return and newline' do # A malformed token is not a non-recoverable error, so on the - # initial fetch it surfaces as NoCredentialsError. + # initial fetch it surfaces as MissingCredentialsError. expect do ECSCredentials.new(backoff: 0, retries: 0) - end.to raise_error(Aws::Errors::NoCredentialsError) + end.to raise_error(Aws::Errors::MissingCredentialsError) end end @@ -227,10 +227,10 @@ module Aws it 'validates the token for carriage return and newline' do # A malformed token is not a non-recoverable error, so on the - # initial fetch it surfaces as NoCredentialsError. + # initial fetch it surfaces as MissingCredentialsError. expect do ECSCredentials.new(backoff: 0, retries: 0) - end.to raise_error(Aws::Errors::NoCredentialsError) + end.to raise_error(Aws::Errors::MissingCredentialsError) end end end @@ -361,10 +361,10 @@ def setup_request(request) if expect['type'] == 'error' # Host/URI validation fails at construction (ArgumentError). A token # file read failure happens during the initial fetch and, not being a - # SEP non-recoverable error, surfaces as NoCredentialsError. + # SEP non-recoverable error, surfaces as MissingCredentialsError. error = ArgumentError if expect['reason'] =~ /failed to read authorization token/ - error = Aws::Errors::NoCredentialsError + error = Aws::Errors::MissingCredentialsError end expect { ECSCredentials.new }.to raise_error(error) elsif expect['type'] == 'success' @@ -393,9 +393,9 @@ def setup_response(response) def handle_expectation(_expect) # A refresh that fails on the initial fetch (no cached credentials to - # fall back on) raises NoCredentialsError. + # fall back on) raises MissingCredentialsError. expect { ECSCredentials.new(backoff: 0, retries: 0) } - .to raise_error(Aws::Errors::NoCredentialsError) + .to raise_error(Aws::Errors::MissingCredentialsError) end test_cases.each do |test_case| diff --git a/gems/aws-sdk-core/spec/aws/instance_profile_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/instance_profile_credentials_spec.rb index 2c0f37f4544..e0be4d0c894 100644 --- a/gems/aws-sdk-core/spec/aws/instance_profile_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/instance_profile_credentials_spec.rb @@ -165,11 +165,11 @@ module Aws SocketError, Timeout::Error ].each do |error_class| - it "raises NoCredentialsError for #{error_class}" do + it "raises MissingCredentialsError for #{error_class}" do stub_request(:put, ipv4_endpoint_token_path).to_return(status: 200, body: 'mytoken') stub_request(:get, ipv4_endpoint + path).to_raise(error_class) expect { InstanceProfileCredentials.new(backoff: 0) } - .to raise_error(Aws::Errors::NoCredentialsError) + .to raise_error(Aws::Errors::MissingCredentialsError) end end @@ -177,11 +177,11 @@ module Aws 400, 401 ].each do |error_code| - it "raises NoCredentialsError for #{error_code} when fetching token" do + it "raises MissingCredentialsError for #{error_code} when fetching token" do stub_request(:put, ipv4_endpoint_token_path).to_return(status: error_code) stub_request(:get, ipv4_endpoint + path).to_return(status: 200) expect { InstanceProfileCredentials.new(backoff: 0) } - .to raise_error(Aws::Errors::NoCredentialsError) + .to raise_error(Aws::Errors::MissingCredentialsError) end end end @@ -262,7 +262,7 @@ module Aws it 'does not attempt to get credentials (insecure)' do stub_request(:put, ipv4_endpoint_token_path).to_return(status: 404) expect { InstanceProfileCredentials.new(backoff: 0) } - .to raise_error(Aws::Errors::NoCredentialsError) + .to raise_error(Aws::Errors::MissingCredentialsError) end it 'gets credentials (secure)' do @@ -391,7 +391,7 @@ module Aws expect(c.expiration.to_s).to eq(expiration2.to_s) end - it 'retries invalid JSON exactly 3 times, then raises NoCredentialsError' do + it 'retries invalid JSON exactly 3 times, then raises MissingCredentialsError' do stub_request(:get, ipv4_endpoint + path) .with(headers: { 'x-aws-ec2-metadata-token' => 'my-token' }) .to_return(status: 500) @@ -405,11 +405,11 @@ module Aws .to_return(status: 200, body: ' ') expect do InstanceProfileCredentials.new(backoff: 0) - end.to raise_error(Aws::Errors::NoCredentialsError) + end.to raise_error(Aws::Errors::MissingCredentialsError) assert_requested(creds_request, times: 4) end - it 'raises NoCredentialsError when the expiration time cannot be parsed' do + it 'raises MissingCredentialsError when the expiration time cannot be parsed' do stub_request(:get, ipv4_endpoint + path) .with(headers: { 'x-aws-ec2-metadata-token' => 'my-token' }) .to_return(status: 500) @@ -419,7 +419,7 @@ module Aws .to_return(status: 200, body: '{ "Expiration": "Expiration" }') expect do InstanceProfileCredentials.new(backoff: 0) - end.to raise_error(Aws::Errors::NoCredentialsError) + end.to raise_error(Aws::Errors::MissingCredentialsError) end describe 'auto refreshing' do @@ -452,20 +452,20 @@ module Aws expect(c.expiration).to be(nil) end - it 'raises NoCredentialsError on non-200 response from profile endpoint' do + it 'raises MissingCredentialsError on non-200 response from profile endpoint' do stub_request(:get, "#{ipv4_endpoint_creds_path}profile-name") .with(headers: { 'x-aws-ec2-metadata-token' => 'my-token' }) .to_return(status: 404, body: 'Not Found') expect { InstanceProfileCredentials.new(backoff: 0, retries: 0) } - .to raise_error(Aws::Errors::NoCredentialsError) + .to raise_error(Aws::Errors::MissingCredentialsError) end - it 'raises NoCredentialsError on non-200 response from metadata service' do + it 'raises MissingCredentialsError on non-200 response from metadata service' do stub_request(:get, ipv4_endpoint + path) .with(headers: { 'x-aws-ec2-metadata-token' => 'my-token' }) .to_return(status: 503, body: 'Service Unavailable') expect { InstanceProfileCredentials.new(backoff: 0, retries: 0) } - .to raise_error(Aws::Errors::NoCredentialsError) + .to raise_error(Aws::Errors::MissingCredentialsError) end end end @@ -498,7 +498,7 @@ module Aws expect(Kernel).to receive(:sleep).with(4) expect do InstanceProfileCredentials.new(backoff: ->(n) { Kernel.sleep(2**n) }, retries: 3) - end.to raise_error(Aws::Errors::NoCredentialsError) + end.to raise_error(Aws::Errors::MissingCredentialsError) assert_requested(expected_request, times: 4) end end @@ -546,13 +546,13 @@ module Aws it 'raises when the first call returns expired credentials' do # A stale response is treated as a failed refresh. On the initial fetch # there are no prior credentials to fall back on, so the refresh lifecycle - # raises NoCredentialsError. + # raises MissingCredentialsError. stub_request(:get, "#{ipv4_endpoint_creds_path}profile-name") .with(headers: { 'x-aws-ec2-metadata-token' => 'my-token' }) .to_return(status: 200, body: expired_resp) expect { InstanceProfileCredentials.new(backoff: 0) } - .to raise_error(Aws::Errors::NoCredentialsError) + .to raise_error(Aws::Errors::MissingCredentialsError) end it 'provides cached credentials after a read timeout during a refresh' do @@ -564,7 +564,10 @@ module Aws provider = InstanceProfileCredentials.new(backoff: 0, retries: 0) - # static stability keeps the cached credentials rather than raising + # static stability keeps the cached credentials rather than raising, + # and the failed refresh is logged with the next-attempt delay + expect(provider).to receive(:warn) + .with(/Credential refresh failed:.*continue using cached credentials/) creds = provider.credentials expect(creds.access_key_id).to eq('akid-2') diff --git a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb index 8b29e2fc7a9..51a6df88621 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb @@ -222,7 +222,7 @@ module Plugins expect(resp.context.retries).to eq(0) end - it 'does not call refresh! when error is expired credentials and clock skew' do + it 'retries a clock skew error rather than invalidating credentials' do resp.error = RetryErrorsSvc::Errors::RequestExpired.new(nil, nil) resp.context.http_response.headers['date'] = (Time.now + 10*60).iso8601 handle { |_context| resp } diff --git a/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb index c1cf84c1bfd..ba3710e199b 100644 --- a/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb @@ -3,7 +3,7 @@ require_relative '../spec_helper' module Aws - # test-only error the fake source raises for a non-recoverable response + # test error the fake source raises for a non-recoverable response class RefreshingCredentialsTestError < StandardError; end describe RefreshingCredentials do @@ -14,7 +14,7 @@ class RefreshingCredentialsTestError < StandardError; end attr_reader :source_calls # Bypasses the eager fetch in RefreshingCredentials#initialize and - # seeds the cache state directly. + # sets the cache state directly. def initialize(seed = {}) @mutex = Mutex.new @static_stability = true @@ -87,7 +87,7 @@ def assert_result(resolver, expected) when 'cachedCredentials' expect(resolver.credentials.access_key_id).to eq(@seeded_akid) when 'noCredentialsError' - expect { resolver.credentials }.to raise_error(Errors::NoCredentialsError) + expect { resolver.credentials }.to raise_error(Errors::MissingCredentialsError) when 'nonRecoverableError' expect { resolver.credentials }.to raise_error(RefreshingCredentialsTestError) end @@ -143,8 +143,8 @@ def refresh tests = JSON.load_file(File.join(File.dirname(__FILE__), 'refreshing_credentials_tests.json')) - tests.each_with_index do |test, index| - it "case #{index + 1}: #{test['documentation']}" do + tests.each do |test| + it "#{test['id']}: #{test['documentation']}" do resolver = build_resolver(resolver_class, test['given']) test['steps'].each do |step| case step['type'] @@ -172,6 +172,26 @@ def refresh end end + it 'raises MissingCredentialsError when the initial fetch returns a stale response, then recovers' do + resolver = build_resolver(resolver_class, 'cachedCredentials' => 'none') + + resolver.expect_response('staleCredentials', nil) + expect { resolver.credentials }.to raise_error(Errors::MissingCredentialsError) + + resolver.expect_response('freshCredentials', nil) + expect(resolver.credentials.access_key_id).to eq('FRESH-AKID') + end + + it 'treats invalidate as a no-op when no credentials are cached yet' do + resolver = build_resolver(resolver_class, 'cachedCredentials' => 'none') + + expect { resolver.invalidate(fake_identity('AKID-1')) }.not_to raise_error + + # the next resolution still performs the initial fetch + resolver.expect_response('freshCredentials', nil) + expect(resolver.credentials.access_key_id).to eq('FRESH-AKID') + end + describe 'concurrency' do let(:gated_resolver_class) do Class.new do @@ -182,6 +202,7 @@ def refresh def initialize(seed) @mutex = Mutex.new @static_stability = true + @async_refresh = seed[:async_refresh] @next_refresh_allowed_at = nil @cached_error = nil @cached_error_expires_at = nil @@ -210,28 +231,29 @@ def non_recoverable_error?(_error) end end - def build_gated_resolver(ttl:, advisory_window:) + def build_gated_resolver(ttl:, advisory_window:, async: false) gated_resolver_class.new( credentials: Credentials.new('CACHED-AKID', 'secret', 'token'), expiration: Time.now + ttl, - advisory_window: advisory_window + advisory_window: advisory_window, + async_refresh: async ) end SWARM_SIZE = 8 - it 'advisory: one refresh runs while a swarm of callers get cached creds immediately' do - # Inside the advisory window (600s) but outside the mandatory window (60s). + it 'runs a single advisory refresh while other callers get cached credentials immediately' do + # advisory window (600s), outside the mandatory window (60s) resolver = build_gated_resolver(ttl: 300, advisory_window: 600) refresher = Thread.new { resolver.credentials } - resolver.entered.pop # the refresher now holds the lock inside #refresh + resolver.entered.pop # refresher now holds the lock inside #refresh callers = Array.new(SWARM_SIZE) { Thread.new { resolver.credentials.access_key_id } } results = callers.map(&:value) expect(results).to all(eq('CACHED-AKID')) - expect(resolver.source_calls).to eq(1) # only the refresher contacted the source + expect(resolver.source_calls).to eq(1) resolver.release << :go refresher.join @@ -240,36 +262,50 @@ def build_gated_resolver(ttl:, advisory_window:) expect(resolver.credentials.access_key_id).to eq('FRESH-AKID') end - it 'mandatory: one refresh runs while a swarm of callers wait and reuse it' do - # Inside the mandatory window (60s). + it 'refreshes in the background during the advisory window without blocking callers' do + resolver = build_gated_resolver(ttl: 300, advisory_window: 600, async: true) + + expect(resolver.credentials.access_key_id).to eq('CACHED-AKID') + resolver.entered.pop # background thread now holds the lock inside #refresh + expect(resolver.source_calls).to eq(1) + + # further callers get cached credentials without starting a second refresh + expect(resolver.credentials.access_key_id).to eq('CACHED-AKID') + expect(resolver.source_calls).to eq(1) + + resolver.release << :go + sleep 0.1 # let the background refresh publish new credentials + + expect(resolver.credentials.access_key_id).to eq('FRESH-AKID') + expect(resolver.source_calls).to eq(1) + end + + it 'runs a single mandatory refresh while other callers wait and reuse the result' do + # mandatory window (60s) resolver = build_gated_resolver(ttl: 30, advisory_window: 600) refresher = Thread.new { resolver.credentials } - resolver.entered.pop # the refresher now holds the lock inside #refresh + resolver.entered.pop # refresher now holds the lock inside #refresh waiters = Array.new(SWARM_SIZE) { Thread.new { resolver.credentials.access_key_id } } - # Give waiters time to queue on the lock - sleep 0.1 + sleep 0.1 # let waiters queue on the lock expect(resolver.source_calls).to eq(1) resolver.release << :go refresher.join results = waiters.map(&:value) - # Exactly one source call was made and every waiter reused that result expect(resolver.source_calls).to eq(1) expect(results).to all(eq('FRESH-AKID')) end - it 'invalidate does not block or interfere while a refresh holds the lock' do + it 'does not block or mutate state when invalidate runs while a refresh holds the lock' do resolver = build_gated_resolver(ttl: 30, advisory_window: 600) refresher = Thread.new { resolver.credentials } - resolver.entered.pop # the refresher now holds the lock inside #refresh + resolver.entered.pop # refresher now holds the lock inside #refresh - # invalidate uses try_lock: with the refresh lock held it returns - # immediately without waiting and without mutating state. rejected = double('identity', access_key_id: 'CACHED-AKID') expect { resolver.invalidate(rejected) }.not_to raise_error diff --git a/gems/aws-sdk-core/spec/aws/refreshing_credentials_tests.json b/gems/aws-sdk-core/spec/aws/refreshing_credentials_tests.json index ed2e4a9d97a..9e794394986 100644 --- a/gems/aws-sdk-core/spec/aws/refreshing_credentials_tests.json +++ b/gems/aws-sdk-core/spec/aws/refreshing_credentials_tests.json @@ -1,5 +1,6 @@ [ { + "id": "valid-no-refresh-use-cached", "documentation": "Valid cached credentials: no refresh is attempted and the caller receives the cached credentials.", "given": { "cachedCredentials": "valid" }, "steps": [ @@ -10,6 +11,7 @@ ] }, { + "id": "advisory-refresh-succeeds", "documentation": "Advisory window, refresh succeeds: the caller receives the newly refreshed credentials.", "given": { "cachedCredentials": "advisory" }, "steps": [ @@ -21,6 +23,7 @@ ] }, { + "id": "advisory-refresh-fails-uses-cached", "documentation": "Advisory window, refresh fails: the resolver applies the refresh backoff and the caller receives the existing cached credentials.", "given": { "cachedCredentials": "advisory" }, "steps": [ @@ -32,6 +35,7 @@ ] }, { + "id": "mandatory-refresh-succeeds", "documentation": "Mandatory window, refresh succeeds: the caller receives the newly refreshed credentials.", "given": { "cachedCredentials": "mandatory" }, "steps": [ @@ -43,6 +47,7 @@ ] }, { + "id": "mandatory-refresh-fails-uses-cached", "documentation": "Mandatory window, refresh fails: the resolver applies the refresh backoff and the caller receives the cached credentials.", "given": { "cachedCredentials": "mandatory" }, "steps": [ @@ -54,6 +59,7 @@ ] }, { + "id": "expired-refresh-succeeds", "documentation": "Expired credentials are refreshed successfully: the caller receives the newly refreshed credentials.", "given": { "cachedCredentials": "expired" }, "steps": [ @@ -65,6 +71,7 @@ ] }, { + "id": "expired-refresh-fails-uses-expired", "documentation": "Expired credentials, refresh fails: the resolver applies the refresh backoff and the caller receives the expired cached credentials rather than raising.", "given": { "cachedCredentials": "expired" }, "steps": [ @@ -76,6 +83,7 @@ ] }, { + "id": "no-cache-initial-fetch-fails-then-succeeds", "documentation": "No cached credentials and the initial fetch fails: the SDK raises, since there are no cached credentials to fall back on. The next call retries and succeeds.", "given": { "cachedCredentials": "none" }, "steps": [ @@ -92,6 +100,7 @@ ] }, { + "id": "advisory-stale-response-uses-cached", "documentation": "Advisory window, source returns stale credentials (Expiration at or before now): treated as a failed refresh. The resolver applies the refresh backoff and returns the existing cached credentials.", "given": { "cachedCredentials": "advisory" }, "steps": [ @@ -103,6 +112,7 @@ ] }, { + "id": "mandatory-stale-response-uses-cached", "documentation": "Mandatory window, source returns stale credentials: same as the advisory case, treated as a failed refresh.", "given": { "cachedCredentials": "mandatory" }, "steps": [ @@ -115,6 +125,7 @@ }, { + "id": "advisory-window-tier-5m", "documentation": "A 10-minute credential lifetime selects the 5-minute advisory window (lifetime <= 20 minutes).", "given": { "cachedCredentials": "none" }, "steps": [ @@ -127,6 +138,7 @@ ] }, { + "id": "advisory-window-tier-15m", "documentation": "A 20.5-minute credential lifetime selects the 15-minute advisory window (lifetime > 20 and < 90 minutes).", "given": { "cachedCredentials": "none" }, "steps": [ @@ -139,6 +151,7 @@ ] }, { + "id": "advisory-window-tier-60m", "documentation": "A 6-hour credential lifetime selects the 60-minute advisory window (lifetime >= 90 minutes).", "given": { "cachedCredentials": "none" }, "steps": [ @@ -151,6 +164,7 @@ ] }, { + "id": "advisory-window-recomputed-after-refresh", "documentation": "After a successful refresh returns credentials with a different lifetime, the SDK recomputes the advisory window. The first credentials have a 6-hour lifetime (60-minute window); after advancing into that window, the refreshed credentials have a 10-minute lifetime (5-minute window).", "given": { "cachedCredentials": "none" }, "steps": [ @@ -175,6 +189,7 @@ ] }, { + "id": "configured-advisory-window-overrides-tier", "documentation": "A customer-configured advisory window overrides the table. Credentials with a 6-hour lifetime would map to 60 minutes, but the configured 30-minute window is used instead.", "given": { "cachedCredentials": "none", "configuredAdvisoryWindowSeconds": 1800 }, "steps": [ @@ -188,6 +203,7 @@ }, { + "id": "advisory-non-recoverable-raises-then-recovers", "documentation": "Advisory window, non-recoverable failure: the SDK raises immediately. No refresh backoff is applied, but the error is cached for up to 5 seconds, so a recovering call succeeds once that cache expires.", "given": { "cachedCredentials": "advisory" }, "steps": [ @@ -210,6 +226,7 @@ ] }, { + "id": "mandatory-non-recoverable-raises-then-recovers", "documentation": "Mandatory window, non-recoverable failure: the SDK raises immediately. No refresh backoff is applied, but the error is cached for up to 5 seconds, so a recovering call succeeds once that cache expires.", "given": { "cachedCredentials": "mandatory" }, "steps": [ @@ -232,6 +249,7 @@ ] }, { + "id": "non-recoverable-error-cached-on-retry", "documentation": "Non-recoverable error, then an immediate retry with no clock advance: the error is still cached, so the SDK re-raises it without contacting the source. This protects the credential source from an application that swallows the error and retries in a loop.", "given": { "cachedCredentials": "advisory" }, "steps": [ @@ -250,6 +268,7 @@ }, { + "id": "invalidate-matching-akid-refresh-succeeds", "documentation": "Invalidate with an access key ID matching the cached credentials routes the next getCredentials through the mandatory refresh path, and the refresh succeeds.", "given": { "cachedCredentials": "valid", "accessKeyId": "AKID-1" }, "steps": [ @@ -262,6 +281,7 @@ ] }, { + "id": "invalidate-matching-akid-refresh-fails", "documentation": "Invalidate with a matching access key ID routes the next getCredentials through the mandatory refresh path; the refresh fails and the SDK continues using the cached credentials.", "given": { "cachedCredentials": "valid", "accessKeyId": "AKID-1" }, "steps": [ @@ -274,6 +294,7 @@ ] }, { + "id": "invalidate-during-backoff-waits-for-expiry", "documentation": "Invalidate during an active backoff: the SDK does not contact the credential source. Once the refresh backoff has elapsed, the next getCredentials attempts a refresh.", "given": { "cachedCredentials": "expired", "accessKeyId": "AKID-1", "refreshBackoffSeconds": 420 }, "steps": [ @@ -306,6 +327,7 @@ ] }, { + "id": "invalidate-stale-akid-is-ignored", "documentation": "Invalidate with a stale access key ID (a concurrent refresh already replaced the credentials): the cache is unchanged and the next getCredentials does not contact the source.", "given": { "cachedCredentials": "valid", "accessKeyId": "AKID-2" }, "steps": [ @@ -318,6 +340,7 @@ }, { + "id": "failed-refresh-backoff-blocks-until-elapsed", "documentation": "After a failed refresh, the SDK does not contact the credential source again until the refresh backoff has elapsed.", "given": { "cachedCredentials": "expired", "refreshBackoffSeconds": 420 }, "steps": [ @@ -349,6 +372,7 @@ ] }, { + "id": "no-cache-non-recoverable-raises-then-recovers", "documentation": "No cached credentials and the initial fetch fails with a non-recoverable error: the SDK raises the error directly rather than a generic NoCredentialsError. No refresh backoff is applied, but the error is cached for up to 5 seconds, so a recovering call succeeds once that cache expires.", "given": { "cachedCredentials": "none" }, "steps": [ From b4fdd6c14f9816f7d5fb5c95b60b09427e98af02 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Thu, 24 Sep 2026 10:38:21 -0700 Subject: [PATCH 16/18] Refactor to add ResilientRefreshingCredentials --- gems/aws-sdk-cognitoidentity/CHANGELOG.md | 2 + .../cognito_identity_credentials.rb | 2 +- gems/aws-sdk-core/CHANGELOG.md | 2 + gems/aws-sdk-core/lib/aws-sdk-core.rb | 1 + .../aws-sdk-core/assume_role_credentials.rb | 2 +- .../assume_role_web_identity_credentials.rb | 2 +- .../lib/aws-sdk-core/ecs_credentials.rb | 2 +- .../instance_profile_credentials.rb | 2 +- .../lib/aws-sdk-core/login_credentials.rb | 2 +- .../lib/aws-sdk-core/process_credentials.rb | 7 +- .../aws-sdk-core/refreshing_credentials.rb | 271 ++++-------------- .../resilient_refreshing_credentials.rb | 268 +++++++++++++++++ .../lib/aws-sdk-core/sso_credentials.rb | 2 +- ... resilient_refreshing_credentials_spec.rb} | 12 +- ...silient_refreshing_credentials_tests.json} | 2 +- .../spec/aws/sso_credentials_spec.rb | 4 + .../lib/aws-sdk-s3/express_credentials.rb | 8 +- .../spec/access_grants_credentials_spec.rb | 11 +- .../spec/express_credentials_spec.rb | 2 +- .../spec/plugins/access_grants_spec.rb | 8 - 20 files changed, 356 insertions(+), 256 deletions(-) create mode 100644 gems/aws-sdk-core/lib/aws-sdk-core/resilient_refreshing_credentials.rb rename gems/aws-sdk-core/spec/aws/{refreshing_credentials_spec.rb => resilient_refreshing_credentials_spec.rb} (97%) rename gems/aws-sdk-core/spec/aws/{refreshing_credentials_tests.json => resilient_refreshing_credentials_tests.json} (98%) diff --git a/gems/aws-sdk-cognitoidentity/CHANGELOG.md b/gems/aws-sdk-cognitoidentity/CHANGELOG.md index 776c4c7a682..b4601712b71 100644 --- a/gems/aws-sdk-cognitoidentity/CHANGELOG.md +++ b/gems/aws-sdk-cognitoidentity/CHANGELOG.md @@ -1,6 +1,8 @@ Unreleased Changes ------------------ +* Issue - Remove duplicate `before_refresh` callback during credential refresh. + 1.93.0 (2026-09-11) ------------------ diff --git a/gems/aws-sdk-cognitoidentity/lib/aws-sdk-cognitoidentity/customizations/cognito_identity_credentials.rb b/gems/aws-sdk-cognitoidentity/lib/aws-sdk-cognitoidentity/customizations/cognito_identity_credentials.rb index b55ab7f16fb..3896e038f28 100644 --- a/gems/aws-sdk-cognitoidentity/lib/aws-sdk-cognitoidentity/customizations/cognito_identity_credentials.rb +++ b/gems/aws-sdk-cognitoidentity/lib/aws-sdk-cognitoidentity/customizations/cognito_identity_credentials.rb @@ -45,7 +45,7 @@ module CognitoIdentity # to be refreshed and it has access to the CognitoIdentityCredentials object. class CognitoIdentityCredentials include CredentialProvider - include RefreshingCredentials + include ResilientRefreshingCredentials # @param [Hash] options # @option options [String] :identity_id the Cognito identity_id. Required diff --git a/gems/aws-sdk-core/CHANGELOG.md b/gems/aws-sdk-core/CHANGELOG.md index e9eac7fa8e4..7718a2e8c38 100644 --- a/gems/aws-sdk-core/CHANGELOG.md +++ b/gems/aws-sdk-core/CHANGELOG.md @@ -1,6 +1,8 @@ Unreleased Changes ------------------ +* Feature - Supported AWS credential providers now continue to use cached credentials after refresh failures and retry refresh with backoff. This improves resilience to temporary credential source outages and standardizes credential refresh timing. During an outage, requests may reach the service and return authentication errors instead of failing client-side during credential refresh. See [REFERENCE_PAGE_URL] for details and the full list of providers. + 3.257.0 (2026-09-14) ------------------ diff --git a/gems/aws-sdk-core/lib/aws-sdk-core.rb b/gems/aws-sdk-core/lib/aws-sdk-core.rb index e599a527332..9091dfc0179 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core.rb @@ -17,6 +17,7 @@ module Aws autoload :Credentials, 'aws-sdk-core/credentials' autoload :CredentialProvider, 'aws-sdk-core/credential_provider' autoload :RefreshingCredentials, 'aws-sdk-core/refreshing_credentials' + autoload :ResilientRefreshingCredentials, 'aws-sdk-core/resilient_refreshing_credentials' autoload :AssumeRoleCredentials, 'aws-sdk-core/assume_role_credentials' autoload :AssumeRoleWebIdentityCredentials, 'aws-sdk-core/assume_role_web_identity_credentials' autoload :CredentialProviderChain, 'aws-sdk-core/credential_provider_chain' diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb index 6d190b09cde..201ab418393 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb @@ -20,7 +20,7 @@ module Aws class AssumeRoleCredentials include CredentialProvider - include RefreshingCredentials + include ResilientRefreshingCredentials # @option options [required, String] :role_arn # @option options [required, String] :role_session_name diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb index 757f791c39c..7d71c41c435 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb @@ -24,7 +24,7 @@ module Aws class AssumeRoleWebIdentityCredentials include CredentialProvider - include RefreshingCredentials + include ResilientRefreshingCredentials # @param [Hash] options # @option options [required, String] :role_arn the IAM role diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb index e38b86a1946..168b50f3165 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb @@ -12,7 +12,7 @@ module Aws # ec2 = Aws::EC2::Client.new(credentials: ecs_credentials) class ECSCredentials include CredentialProvider - include RefreshingCredentials + include ResilientRefreshingCredentials # @api private class Non200Response < RuntimeError diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb index 17fe184a10a..21b08322859 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb @@ -23,7 +23,7 @@ module Aws # @see https://docs.aws.amazon.com/sdkref/latest/guide/feature-imds-credentials.html IMDS Credential Provider class InstanceProfileCredentials include CredentialProvider - include RefreshingCredentials + include ResilientRefreshingCredentials # @api private class Non200Response < RuntimeError diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/login_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/login_credentials.rb index 1167613ecf5..71d8f723073 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/login_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/login_credentials.rb @@ -15,7 +15,7 @@ module Aws # be constructed with additional options that were provided. class LoginCredentials include CredentialProvider - include RefreshingCredentials + include ResilientRefreshingCredentials # @option options [required, String] :login_session An opaque string # used to determine the cache file location. This value can be found diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb index 1e01caeb390..9c60aab0a4c 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb @@ -36,8 +36,6 @@ def initialize(process) @process = process @credentials = credentials_from_process @async_refresh = false - # SDK has no visibility into the credential source so static stability must not apply - @static_stability = false @metrics = ['CREDENTIALS_PROCESS'] super end @@ -91,5 +89,10 @@ def _parse_payload_format_v1(creds_json) def refresh @credentials = credentials_from_process end + + def near_expiration?(expiration_length) + # are we within 5 minutes of expiration? + @expiration && (Time.now.to_i + expiration_length) > @expiration.to_i + end end end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb index bad9eb3d7c4..777c32f3786 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb @@ -1,264 +1,93 @@ # frozen_string_literal: true module Aws - # Base module mixed into refreshable credential classes. Implements the - # credential refresh lifecycle: caching, an advisory and a mandatory - # refresh window, rate-limited backoff on failure, static stability - # (continue using cached credentials when a refresh fails), and - # short-lived caching of non-recoverable errors. + # Base class used credential classes that can be refreshed. This + # provides basic refresh logic in a thread-safe manner. Classes mixing in + # this module are expected to implement a `#refresh` method that populates + # the following instance variables: # - # Classes mixing in this module must implement `#refresh`, which fetches - # from the source and assigns `@credentials` and `@expiration` on success, - # or raises on failure. It must not partially update those on failure. + # * `@credentials` ({Credentials}) + # * `@expiration` (Time) # - # Before calling `super`, classes may set `@async_refresh` to true to - # refresh in the background during the advisory window, or set - # `@static_stability` to false for caching-only behavior. Classes may - # override `#non_recoverable_error?` to classify provider errors that - # should be raised immediately rather than retried. module RefreshingCredentials - MANDATORY_REFRESH_WINDOW = 60 # 1 minute + SYNC_EXPIRATION_LENGTH = 300 # 5 minutes + ASYNC_EXPIRATION_LENGTH = 600 # 10 minutes CLIENT_EXCLUDE_OPTIONS = Set.new([:before_refresh]).freeze - # @api private - # Signals a credential source response whose Expiration is at or before - # the current time. - class StaleCredentialsError < RuntimeError - def initialize(*_args) - super('the credential source returned credentials that are already expired') - end - end - # @param [Hash] options # @option options [Proc] :before_refresh A Proc called before credentials are refreshed. # It accepts `self` as the only argument. def initialize(options = {}) @mutex = Mutex.new - if options.is_a?(Hash) - @before_refresh = options.delete(:before_refresh) - @configured_advisory_window = options.delete(:advisory_refresh_window) - end - # mandatory refresh window must not exceed the advisory window - if @configured_advisory_window && @configured_advisory_window < MANDATORY_REFRESH_WINDOW - raise ArgumentError, - "advisory_refresh_window (#{@configured_advisory_window}) must be at least " \ - "the mandatory refresh window (#{MANDATORY_REFRESH_WINDOW} seconds)" - end - @static_stability = true if @static_stability.nil? - @next_refresh_allowed_at = nil - @cached_error = nil - @cached_error_expires_at = nil - @advisory_window = nil - fetch_initial_credentials - end + @before_refresh = options.delete(:before_refresh) if options.is_a?(Hash) - attr_reader :advisory_window + @before_refresh.call(self) if @before_refresh + refresh + end # @return [Credentials] def credentials - get_credentials + refresh_if_near_expiration! @credentials end - # Force a synchronous refresh, raising on failure. Does not apply static - # stability or backoff. + # Refresh credentials. # @return [void] def refresh! @mutex.synchronize do - @before_refresh&.call(self) - refresh - end - end - - # Mark cached credentials for refresh after a target service rejects them - def invalidate(rejected_credentials) - return unless @mutex.try_lock + @before_refresh.call(self) if @before_refresh - begin - if @credentials && @credentials.access_key_id == rejected_credentials.access_key_id - @expiration = Time.now - end - ensure - @mutex.unlock + refresh end end - def rate_limited? - !@next_refresh_allowed_at.nil? && Time.now < @next_refresh_allowed_at - end - private - def get_credentials - return fetch_initial_credentials if @credentials.nil? - return unless refresh_needed? - return if rate_limited? - - if mandatory_refresh_needed? - attempt_mandatory_refresh - else - attempt_advisory_refresh - end + def sync_expiration_length + self.class::SYNC_EXPIRATION_LENGTH end - def fetch_initial_credentials - @mutex.synchronize do - return @credentials unless @credentials.nil? - raise @cached_error if non_recoverable_error_cached? - - error = call_source - if error.nil? - on_refresh_success - return @credentials - end - - if non_recoverable_error?(error) - cache_non_recoverable_error(error) - raise error - end - raise Errors::MissingCredentialsError - end + def async_expiration_length + self.class::ASYNC_EXPIRATION_LENGTH end - def attempt_advisory_refresh - if @async_refresh - refresh_in_background - @credentials - else - return @credentials unless @mutex.try_lock - - begin - perform_refresh(mandatory: false, raise_to_caller: true) - ensure - @mutex.unlock - end - end - end - - def attempt_mandatory_refresh - @mutex.synchronize do - perform_refresh(mandatory: true, raise_to_caller: true) - end - end - - def refresh_in_background - return if @mutex.locked? - - Thread.new do + # Refreshes credentials asynchronously and synchronously. + # If we are near to expiration, block while getting new credentials. + # Otherwise, if we're approaching expiration, use the existing credentials + # but attempt a refresh in the background. + def refresh_if_near_expiration! + # NOTE: This check is an optimization. Rather than acquire the mutex on every #refresh_if_near_expiration + # call, we check before doing so, and then we check within the mutex to avoid a race condition. + # See issue: https://github.com/aws/aws-sdk-ruby/issues/2641 for more info. + if near_expiration?(sync_expiration_length) @mutex.synchronize do - perform_refresh(mandatory: false, raise_to_caller: false) + if near_expiration?(sync_expiration_length) + @before_refresh.call(self) if @before_refresh + refresh + end + end + elsif @async_refresh && near_expiration?(async_expiration_length) + unless @mutex.locked? + Thread.new do + @mutex.synchronize do + if near_expiration?(async_expiration_length) + @before_refresh.call(self) if @before_refresh + refresh + end + end + end end end end - def perform_refresh(mandatory:, raise_to_caller:) - return @credentials unless mandatory ? mandatory_refresh_needed? : refresh_needed? - - if non_recoverable_error_cached? - raise @cached_error if raise_to_caller - - return @credentials - end - return @credentials if rate_limited? - - error = call_source - if error.nil? - on_refresh_success - return @credentials - end - - handle_failure(error, raise_to_caller: raise_to_caller) - end - - # Calls the source via #refresh. Returns nil on success, or restores - # prior credentials and returns error on failure so a failed or stale - # refresh never discards the cached credentials. - def call_source - prior = [@credentials, @expiration] - @before_refresh&.call(self) - refresh - if !@expiration.nil? && @expiration <= Time.now - @credentials, @expiration = prior - return StaleCredentialsError.new - end - nil - rescue StandardError => e - @credentials, @expiration = prior - e - end - - def handle_failure(error, raise_to_caller:) - if non_recoverable_error?(error) - cache_non_recoverable_error(error) - raise error if raise_to_caller - - return @credentials + def near_expiration?(expiration_length) + if @expiration + # Are we within expiration? + (Time.now.to_i + expiration_length) > @expiration.to_i + else + true end - - raise error if mandatory_refresh_needed? && !@static_stability - - @next_refresh_allowed_at = Time.now + refresh_backoff - log_refresh_failure(error) - @credentials - end - - def on_refresh_success - @next_refresh_allowed_at = nil - @cached_error = nil - @cached_error_expires_at = nil - @advisory_window = select_advisory_window - end - - def refresh_needed? - within?(@advisory_window || select_advisory_window) - end - - def mandatory_refresh_needed? - within?(MANDATORY_REFRESH_WINDOW) - end - - def within?(seconds) - return false unless @expiration - - Time.now + seconds > @expiration - end - - def select_advisory_window - return @configured_advisory_window if @configured_advisory_window - return 60 * 60 unless @expiration - - lifetime = @expiration - Time.now - return 5 * 60 if lifetime <= 20 * 60 - return 15 * 60 if lifetime < 90 * 60 - - 60 * 60 - end - - def non_recoverable_error_cached? - !@cached_error.nil? && Time.now < @cached_error_expires_at - end - - def refresh_backoff - rand(300..600) - end - - def cache_non_recoverable_error(error) - @cached_error = error - @cached_error_expires_at = Time.now + rand(1..5) - end - - def non_recoverable_error?(_error) - false - end - - def log_refresh_failure(error) - seconds = (@next_refresh_allowed_at - Time.now).round - warn( - "Credential refresh failed: #{error.message}. The SDK will continue " \ - 'using cached credentials. A refresh of these credentials will be ' \ - "attempted again after #{seconds} seconds." - ) end end end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/resilient_refreshing_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/resilient_refreshing_credentials.rb new file mode 100644 index 00000000000..07602398c65 --- /dev/null +++ b/gems/aws-sdk-core/lib/aws-sdk-core/resilient_refreshing_credentials.rb @@ -0,0 +1,268 @@ +# frozen_string_literal: true + +module Aws + # Mixed into the credential providers that are in scope for the Credential + # Refresh SEP. Implements the statically stable refresh lifecycle: caching, + # an advisory and a mandatory refresh window, rate-limited backoff on + # failure, static stability (continue using cached credentials when a + # refresh fails), and short-lived caching of non-recoverable errors. + # + # Providers whose behavior is out of scope for the SEP (Process, S3 Express, + # and customer-provided providers) use {RefreshingCredentials} instead, which + # preserves the simpler pre-SEP refresh behavior. + # + # Classes mixing in this module must implement `#refresh`, which fetches + # from the source and assigns `@credentials` and `@expiration` on success, + # or raises on failure. It must not partially update those on failure. + # + # Before calling `super`, classes may set `@async_refresh` to true to + # refresh in the background during the advisory window, or set + # `@static_stability` to false for caching-only behavior. Classes may + # override `#non_recoverable_error?` to classify provider errors that + # should be raised immediately rather than retried. + module ResilientRefreshingCredentials + MANDATORY_REFRESH_WINDOW = 60 # 1 minute + + CLIENT_EXCLUDE_OPTIONS = Set.new([:before_refresh]).freeze + + # @api private + # Signals a credential source response whose Expiration is at or before + # the current time. + class StaleCredentialsError < RuntimeError + def initialize(*_args) + super('the credential source returned credentials that are already expired') + end + end + + # @param [Hash] options + # @option options [Proc] :before_refresh A Proc called before credentials are refreshed. + # It accepts `self` as the only argument. + def initialize(options = {}) + @mutex = Mutex.new + if options.is_a?(Hash) + @before_refresh = options.delete(:before_refresh) + @configured_advisory_window = options.delete(:advisory_refresh_window) + end + # mandatory refresh window must not exceed the advisory window + if @configured_advisory_window && @configured_advisory_window < MANDATORY_REFRESH_WINDOW + raise ArgumentError, + "advisory_refresh_window (#{@configured_advisory_window}) must be at least " \ + "the mandatory refresh window (#{MANDATORY_REFRESH_WINDOW} seconds)" + end + @static_stability = true if @static_stability.nil? + @next_refresh_allowed_at = nil + @cached_error = nil + @cached_error_expires_at = nil + @advisory_window = nil + fetch_initial_credentials + end + + attr_reader :advisory_window + + # @return [Credentials] + def credentials + get_credentials + @credentials + end + + # Force a synchronous refresh, raising on failure. Does not apply static + # stability or backoff. + # @return [void] + def refresh! + @mutex.synchronize do + @before_refresh&.call(self) + refresh + end + end + + # Mark cached credentials for refresh after a target service rejects them + def invalidate(rejected_credentials) + return unless @mutex.try_lock + + begin + if @credentials && @credentials.access_key_id == rejected_credentials.access_key_id + @expiration = Time.now + end + ensure + @mutex.unlock + end + end + + def rate_limited? + !@next_refresh_allowed_at.nil? && Time.now < @next_refresh_allowed_at + end + + private + + def get_credentials + return fetch_initial_credentials if @credentials.nil? + return unless refresh_needed? + return if rate_limited? + + if mandatory_refresh_needed? + attempt_mandatory_refresh + else + attempt_advisory_refresh + end + end + + def fetch_initial_credentials + @mutex.synchronize do + return @credentials unless @credentials.nil? + raise @cached_error if non_recoverable_error_cached? + + error = call_source + if error.nil? + on_refresh_success + return @credentials + end + + if non_recoverable_error?(error) + cache_non_recoverable_error(error) + raise error + end + raise Errors::MissingCredentialsError + end + end + + def attempt_advisory_refresh + if @async_refresh + refresh_in_background + @credentials + else + return @credentials unless @mutex.try_lock + + begin + perform_refresh(mandatory: false, raise_to_caller: true) + ensure + @mutex.unlock + end + end + end + + def attempt_mandatory_refresh + @mutex.synchronize do + perform_refresh(mandatory: true, raise_to_caller: true) + end + end + + def refresh_in_background + return if @mutex.locked? + + Thread.new do + @mutex.synchronize do + perform_refresh(mandatory: false, raise_to_caller: false) + end + end + end + + def perform_refresh(mandatory:, raise_to_caller:) + return @credentials unless mandatory ? mandatory_refresh_needed? : refresh_needed? + + if non_recoverable_error_cached? + raise @cached_error if raise_to_caller + + return @credentials + end + return @credentials if rate_limited? + + error = call_source + if error.nil? + on_refresh_success + return @credentials + end + + handle_failure(error, raise_to_caller: raise_to_caller) + end + + # Calls the source via #refresh. Returns nil on success, or restores + # prior credentials and returns error on failure so a failed or stale + # refresh never discards the cached credentials. + def call_source + prior = [@credentials, @expiration] + @before_refresh&.call(self) + refresh + if !@expiration.nil? && @expiration <= Time.now + @credentials, @expiration = prior + return StaleCredentialsError.new + end + nil + rescue StandardError => e + @credentials, @expiration = prior + e + end + + def handle_failure(error, raise_to_caller:) + if non_recoverable_error?(error) + cache_non_recoverable_error(error) + raise error if raise_to_caller + + return @credentials + end + + raise error if mandatory_refresh_needed? && !@static_stability + + @next_refresh_allowed_at = Time.now + refresh_backoff + log_refresh_failure(error) + @credentials + end + + def on_refresh_success + @next_refresh_allowed_at = nil + @cached_error = nil + @cached_error_expires_at = nil + @advisory_window = select_advisory_window + end + + def refresh_needed? + within?(@advisory_window || select_advisory_window) + end + + def mandatory_refresh_needed? + within?(MANDATORY_REFRESH_WINDOW) + end + + def within?(seconds) + return false unless @expiration + + Time.now + seconds > @expiration + end + + def select_advisory_window + return @configured_advisory_window if @configured_advisory_window + return 60 * 60 unless @expiration + + lifetime = @expiration - Time.now + return 5 * 60 if lifetime <= 20 * 60 + return 15 * 60 if lifetime < 90 * 60 + + 60 * 60 + end + + def non_recoverable_error_cached? + !@cached_error.nil? && Time.now < @cached_error_expires_at + end + + def refresh_backoff + rand(300..600) + end + + def cache_non_recoverable_error(error) + @cached_error = error + @cached_error_expires_at = Time.now + rand(1..5) + end + + def non_recoverable_error?(_error) + false + end + + def log_refresh_failure(error) + seconds = (@next_refresh_allowed_at - Time.now).round + warn( + "Credential refresh failed: #{error.message}. The SDK will continue " \ + 'using cached credentials. A refresh of these credentials will be ' \ + "attempted again after #{seconds} seconds." + ) + end + end +end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb index 1bcf54c504e..ae0ce78f576 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb @@ -27,7 +27,7 @@ module Aws class SSOCredentials include CredentialProvider - include RefreshingCredentials + include ResilientRefreshingCredentials # @api private LEGACY_REQUIRED_OPTS = [:sso_start_url, :sso_account_id, :sso_region, :sso_role_name].freeze diff --git a/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/resilient_refreshing_credentials_spec.rb similarity index 97% rename from gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb rename to gems/aws-sdk-core/spec/aws/resilient_refreshing_credentials_spec.rb index ba3710e199b..bdb31f20cbd 100644 --- a/gems/aws-sdk-core/spec/aws/refreshing_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/resilient_refreshing_credentials_spec.rb @@ -6,14 +6,14 @@ module Aws # test error the fake source raises for a non-recoverable response class RefreshingCredentialsTestError < StandardError; end - describe RefreshingCredentials do + describe ResilientRefreshingCredentials do let(:resolver_class) do Class.new do - include RefreshingCredentials + include ResilientRefreshingCredentials attr_reader :source_calls - # Bypasses the eager fetch in RefreshingCredentials#initialize and + # Bypasses the eager fetch in ResilientRefreshingCredentials#initialize and # sets the cache state directly. def initialize(seed = {}) @mutex = Mutex.new @@ -121,7 +121,7 @@ def assert_result(resolver, expected) describe 'advisory window configuration' do let(:validating_resolver_class) do Class.new do - include RefreshingCredentials + include ResilientRefreshingCredentials def refresh @credentials = Credentials.new('AKID', 'secret', 'token') @@ -141,7 +141,7 @@ def refresh end end - tests = JSON.load_file(File.join(File.dirname(__FILE__), 'refreshing_credentials_tests.json')) + tests = JSON.load_file(File.join(File.dirname(__FILE__), 'resilient_refreshing_credentials_tests.json')) tests.each do |test| it "#{test['id']}: #{test['documentation']}" do @@ -195,7 +195,7 @@ def refresh describe 'concurrency' do let(:gated_resolver_class) do Class.new do - include RefreshingCredentials + include ResilientRefreshingCredentials attr_reader :source_calls, :entered, :release diff --git a/gems/aws-sdk-core/spec/aws/refreshing_credentials_tests.json b/gems/aws-sdk-core/spec/aws/resilient_refreshing_credentials_tests.json similarity index 98% rename from gems/aws-sdk-core/spec/aws/refreshing_credentials_tests.json rename to gems/aws-sdk-core/spec/aws/resilient_refreshing_credentials_tests.json index 9e794394986..961ac05649f 100644 --- a/gems/aws-sdk-core/spec/aws/refreshing_credentials_tests.json +++ b/gems/aws-sdk-core/spec/aws/resilient_refreshing_credentials_tests.json @@ -373,7 +373,7 @@ }, { "id": "no-cache-non-recoverable-raises-then-recovers", - "documentation": "No cached credentials and the initial fetch fails with a non-recoverable error: the SDK raises the error directly rather than a generic NoCredentialsError. No refresh backoff is applied, but the error is cached for up to 5 seconds, so a recovering call succeeds once that cache expires.", + "documentation": "No cached credentials and the initial fetch fails with a non-recoverable error: the SDK raises the error directly rather than a generic MissingCredentialsError. No refresh backoff is applied, but the error is cached for up to 5 seconds, so a recovering call succeeds once that cache expires.", "given": { "cachedCredentials": "none" }, "steps": [ { diff --git a/gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb index 0371c4b50f3..2becc0c845f 100644 --- a/gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/sso_credentials_spec.rb @@ -5,6 +5,10 @@ module Aws describe SSOCredentials do + before do + allow_any_instance_of(SSOCredentials).to receive(:warn) + end + let(:client) do SSO::Client.new( region: 'us-west-2', diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/express_credentials.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/express_credentials.rb index 16d34f4f8ee..5c9d9758fad 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/express_credentials.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/express_credentials.rb @@ -9,7 +9,8 @@ class ExpressCredentials include CredentialProvider include RefreshingCredentials - ADVISORY_REFRESH_WINDOW = 120 # 2 minutes + SYNC_EXPIRATION_LENGTH = 60 # 1 minute + ASYNC_EXPIRATION_LENGTH = 120 # 2 minutes def initialize(options = {}) @client = options[:client] @@ -20,10 +21,7 @@ def initialize(options = {}) end end @async_refresh = true - # Session credentials are rejected once expired, so static stability - # must not apply. - @static_stability = false - super(options.merge(advisory_refresh_window: ADVISORY_REFRESH_WINDOW)) + super end # @return [S3::Client] diff --git a/gems/aws-sdk-s3/spec/access_grants_credentials_spec.rb b/gems/aws-sdk-s3/spec/access_grants_credentials_spec.rb index 8e5455dace1..bf986aad8d7 100644 --- a/gems/aws-sdk-s3/spec/access_grants_credentials_spec.rb +++ b/gems/aws-sdk-s3/spec/access_grants_credentials_spec.rb @@ -11,9 +11,9 @@ module S3 Aws::S3Control::Client.new(region: 'us-east-1', stub_responses: true) end - let(:in_one_hour) { Time.now + 60 * 60 } + let(:in_five_minutes) { Time.now + 60 * 5 } - let(:expiration) { in_one_hour } + let(:expiration) { in_five_minutes } let(:credentials) do double('credentials', @@ -64,7 +64,7 @@ module S3 expect(c.credentials.access_key_id).to eq('akid') expect(c.credentials.secret_access_key).to eq('secret') expect(c.credentials.session_token).to eq('session') - expect(c.expiration).to eq(in_one_hour) + expect(c.expiration).to eq(in_five_minutes) end it 'provides the matched grant target' do @@ -79,7 +79,8 @@ module S3 end it 'refreshes asynchronously' do - time = Time.now + 60 * 2 + # expiration 9.5 minutes out, within the async exp time window + time = Time.now + 60 * 9.5 allow(credentials).to receive(:expiration).and_return(time) expect(client).to receive(:get_data_access).at_least(2).times expect(Thread).to receive(:new).and_yield @@ -93,7 +94,7 @@ module S3 end it 'refreshes credentials automatically when they are near expiration' do - allow(credentials).to receive(:expiration).and_return(Time.now + 30) + allow(credentials).to receive(:expiration).and_return(Time.now) expect(client).to receive(:get_data_access).exactly(4).times c = AccessGrantsCredentials.new( client: client, diff --git a/gems/aws-sdk-s3/spec/express_credentials_spec.rb b/gems/aws-sdk-s3/spec/express_credentials_spec.rb index f1a690a433a..13e928be96d 100644 --- a/gems/aws-sdk-s3/spec/express_credentials_spec.rb +++ b/gems/aws-sdk-s3/spec/express_credentials_spec.rb @@ -69,7 +69,7 @@ module S3 end it 'refreshes credentials automatically when they are near expiration' do - allow(credentials).to receive(:expiration).and_return(Time.now + 30) + allow(credentials).to receive(:expiration).and_return(Time.now) expect(client).to receive(:create_session).exactly(4).times c = ExpressCredentials.new( client: client, diff --git a/gems/aws-sdk-s3/spec/plugins/access_grants_spec.rb b/gems/aws-sdk-s3/spec/plugins/access_grants_spec.rb index f24be9ad25a..eb0901e38c1 100644 --- a/gems/aws-sdk-s3/spec/plugins/access_grants_spec.rb +++ b/gems/aws-sdk-s3/spec/plugins/access_grants_spec.rb @@ -68,14 +68,6 @@ module S3 end it 'is skipped for s3 express endpoints' do - # Express endpoint resolves S3 Express credentials which fetch - # session on construction. - client.stub_responses(:create_session, credentials: { - access_key_id: 's3-akid', - secret_access_key: 's3-secret', - session_token: 's3-session', - expiration: Time.now + 60 * 5 - }) expect_any_instance_of(Aws::S3::AccessGrantsCredentialsProvider) .not_to receive(:access_grants_credentials_for) client.head_object(bucket: 'bucket--use1-az2--x-s3', key: 'key') From 7d3fa8a82f7a6cf10db00b2e4caec19d2b361827 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Fri, 25 Sep 2026 10:56:46 -0700 Subject: [PATCH 17/18] Fix some SCP tests --- .../cognito_identity_credentials.rb | 1 - .../aws-sdk-core/assume_role_credentials.rb | 2 +- .../assume_role_web_identity_credentials.rb | 2 +- .../lib/aws-sdk-core/ecs_credentials.rb | 1 - .../instance_profile_credentials.rb | 1 - .../lib/aws-sdk-core/login_credentials.rb | 1 - .../lib/aws-sdk-core/plugins/sign.rb | 13 +++++- .../resilient_refreshing_credentials.rb | 40 +++++++------------ .../lib/aws-sdk-core/sso_credentials.rb | 1 - .../spec/aws/assume_role_credentials_spec.rb | 3 +- ...sume_role_web_identity_credentials_spec.rb | 3 +- .../resilient_refreshing_credentials_spec.rb | 24 +---------- 12 files changed, 33 insertions(+), 59 deletions(-) diff --git a/gems/aws-sdk-cognitoidentity/lib/aws-sdk-cognitoidentity/customizations/cognito_identity_credentials.rb b/gems/aws-sdk-cognitoidentity/lib/aws-sdk-cognitoidentity/customizations/cognito_identity_credentials.rb index 3896e038f28..ac83504fa2c 100644 --- a/gems/aws-sdk-cognitoidentity/lib/aws-sdk-cognitoidentity/customizations/cognito_identity_credentials.rb +++ b/gems/aws-sdk-cognitoidentity/lib/aws-sdk-cognitoidentity/customizations/cognito_identity_credentials.rb @@ -83,7 +83,6 @@ def initialize(options = {}) @identity_id = options.delete(:identity_id) @custom_role_arn = options.delete(:custom_role_arn) @logins = options.delete(:logins) || {} - @async_refresh = false client_opts = {} options.each_pair { |k, v| client_opts[k] = v unless CLIENT_EXCLUDE_OPTIONS.include?(k) } diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb index 201ab418393..974af3d8305 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb @@ -49,7 +49,7 @@ def initialize(options = {}) end end @client = client_opts[:client] || STS::Client.new(client_opts) - @async_refresh = true + warn("[SEP-DEBUG] AssumeRoleCredentials STS client max_attempts=#{@client.config.max_attempts}, retry_mode=#{@client.config.retry_mode}") @metrics = ['CREDENTIALS_STS_ASSUME_ROLE'] super end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb index 7d71c41c435..8802137d8b1 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb @@ -47,7 +47,6 @@ def initialize(options = {}) client_opts = {} @assume_role_web_identity_params = {} @token_file = options.delete(:web_identity_token_file) - @async_refresh = true options.each_pair do |key, value| if self.class.assume_role_web_identity_options.include?(key) @assume_role_web_identity_params[key] = value @@ -61,6 +60,7 @@ def initialize(options = {}) @assume_role_web_identity_params[:role_session_name] = _session_name end @client = client_opts[:client] || STS::Client.new(client_opts.merge(credentials: nil)) + warn("[SEP-DEBUG] AssumeRoleWebIdentityCredentials STS client max_attempts=#{@client.config.max_attempts}, retry_mode=#{@client.config.retry_mode}") @metrics = ['CREDENTIALS_STS_ASSUME_ROLE_WEB_ID'] super end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb index 168b50f3165..056fc9bcd2e 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/ecs_credentials.rb @@ -86,7 +86,6 @@ def initialize(options = {}) @http_read_timeout = options[:http_read_timeout] || 5 @http_debug_output = options[:http_debug_output] @backoff = backoff(options[:backoff]) - @async_refresh = false @metrics = ['CREDENTIALS_HTTP'] super end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb index 21b08322859..5ae7c56b0ee 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/instance_profile_credentials.rb @@ -97,7 +97,6 @@ def initialize(options = {}) @retries = options[:retries] || 1 @token_ttl = options[:token_ttl] || 21_600 - @async_refresh = false @imds_v1_fallback = false @token = nil @metrics = ['CREDENTIALS_IMDS'] diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/login_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/login_credentials.rb index 71d8f723073..c95c7418db4 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/login_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/login_credentials.rb @@ -34,7 +34,6 @@ def initialize(options = {}) @client = Signin::Client.new(client_opts.merge(credentials: nil)) end @metrics = ['CREDENTIALS_LOGIN'] - @async_refresh = true super end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb index 60344d24f24..5d32d0a28df 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb @@ -149,7 +149,7 @@ def sign(context) # Record the signing credentials so the retry layer can invalidate # them on an auth failure and only if they still match - context[:signing_credentials] = @signer.credentials_provider.credentials + context[:signing_credentials] = signing_identity(signature) # add request metadata with signature components for debugging context[:canonical_request] = signature.canonical_request @@ -170,6 +170,17 @@ def credentials private + # Identity of the credentials that signed a request, for invalidation + # matching. Only the access key id is recoverable from the signature + # and it is the only part invalidation compares on. + def signing_identity(signature) + authorization = signature.headers['authorization'] + return unless authorization + + match = authorization.match(%r{Credential=([^/]+)/}) + Credentials.new(match[1], nil, nil) if match + end + def apply_authtype(context, req) # only used for event streaming at input if context[:input_event_emitter] diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/resilient_refreshing_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/resilient_refreshing_credentials.rb index 07602398c65..a3089a39d8f 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/resilient_refreshing_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/resilient_refreshing_credentials.rb @@ -15,11 +15,14 @@ module Aws # from the source and assigns `@credentials` and `@expiration` on success, # or raises on failure. It must not partially update those on failure. # - # Before calling `super`, classes may set `@async_refresh` to true to - # refresh in the background during the advisory window, or set - # `@static_stability` to false for caching-only behavior. Classes may - # override `#non_recoverable_error?` to classify provider errors that - # should be raised immediately rather than retried. + # Advisory refresh is non-blocking in the sense the SEP defines: the caller + # that acquires the refresh lock refreshes inline and adopts the result, + # while concurrent callers get the cached credentials without waiting. + # + # Before calling `super`, classes may set `@static_stability` to false for + # caching-only behavior. Classes may override `#non_recoverable_error?` to + # classify provider errors that should be raised immediately rather than + # retried. module ResilientRefreshingCredentials MANDATORY_REFRESH_WINDOW = 60 # 1 minute @@ -126,17 +129,14 @@ def fetch_initial_credentials end def attempt_advisory_refresh - if @async_refresh - refresh_in_background - @credentials - else - return @credentials unless @mutex.try_lock + # Non-blocking: if another caller holds the lock it is already + # refreshing, so return the cached credentials rather than waiting. + return @credentials unless @mutex.try_lock - begin - perform_refresh(mandatory: false, raise_to_caller: true) - ensure - @mutex.unlock - end + begin + perform_refresh(mandatory: false, raise_to_caller: true) + ensure + @mutex.unlock end end @@ -146,16 +146,6 @@ def attempt_mandatory_refresh end end - def refresh_in_background - return if @mutex.locked? - - Thread.new do - @mutex.synchronize do - perform_refresh(mandatory: false, raise_to_caller: false) - end - end - end - def perform_refresh(mandatory:, raise_to_caller:) return @credentials unless mandatory ? mandatory_refresh_needed? : refresh_needed? diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb index ae0ce78f576..7c6db98ac06 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/sso_credentials.rb @@ -115,7 +115,6 @@ def initialize(options = {}) @metrics = ['CREDENTIALS_SSO_LEGACY'] end - @async_refresh = true super end diff --git a/gems/aws-sdk-core/spec/aws/assume_role_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/assume_role_credentials_spec.rb index 99e11f6a13a..bd729f8e4e3 100644 --- a/gems/aws-sdk-core/spec/aws/assume_role_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/assume_role_credentials_spec.rb @@ -136,10 +136,9 @@ module Aws end end - it 'refreshes asynchronously' do + it 'refreshes inline in the advisory window' do allow(credentials).to receive(:expiration).and_return(Time.now + (2*60)) expect(client).to receive(:assume_role).at_least(2).times - expect(Thread).to receive(:new).and_yield c = AssumeRoleCredentials.new( role_arn: 'arn', role_session_name: 'session') diff --git a/gems/aws-sdk-core/spec/aws/assume_role_web_identity_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/assume_role_web_identity_credentials_spec.rb index b93da352804..df85af6e3a5 100644 --- a/gems/aws-sdk-core/spec/aws/assume_role_web_identity_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/assume_role_web_identity_credentials_spec.rb @@ -203,11 +203,10 @@ module Aws end end - it 'refreshes asynchronously' do + it 'refreshes inline in the advisory window' do allow(credentials).to receive(:expiration).and_return(Time.now + (2*60)) expect(client).to receive(:assume_role_with_web_identity).exactly(2).times expect(File).to receive(:read).with(token_file_path).exactly(2).times - expect(Thread).to receive(:new).and_yield c = AssumeRoleWebIdentityCredentials.new( role_arn: 'arn', diff --git a/gems/aws-sdk-core/spec/aws/resilient_refreshing_credentials_spec.rb b/gems/aws-sdk-core/spec/aws/resilient_refreshing_credentials_spec.rb index bdb31f20cbd..fe0dc52d26c 100644 --- a/gems/aws-sdk-core/spec/aws/resilient_refreshing_credentials_spec.rb +++ b/gems/aws-sdk-core/spec/aws/resilient_refreshing_credentials_spec.rb @@ -202,7 +202,6 @@ def refresh def initialize(seed) @mutex = Mutex.new @static_stability = true - @async_refresh = seed[:async_refresh] @next_refresh_allowed_at = nil @cached_error = nil @cached_error_expires_at = nil @@ -231,12 +230,11 @@ def non_recoverable_error?(_error) end end - def build_gated_resolver(ttl:, advisory_window:, async: false) + def build_gated_resolver(ttl:, advisory_window:) gated_resolver_class.new( credentials: Credentials.new('CACHED-AKID', 'secret', 'token'), expiration: Time.now + ttl, - advisory_window: advisory_window, - async_refresh: async + advisory_window: advisory_window ) end @@ -262,24 +260,6 @@ def build_gated_resolver(ttl:, advisory_window:, async: false) expect(resolver.credentials.access_key_id).to eq('FRESH-AKID') end - it 'refreshes in the background during the advisory window without blocking callers' do - resolver = build_gated_resolver(ttl: 300, advisory_window: 600, async: true) - - expect(resolver.credentials.access_key_id).to eq('CACHED-AKID') - resolver.entered.pop # background thread now holds the lock inside #refresh - expect(resolver.source_calls).to eq(1) - - # further callers get cached credentials without starting a second refresh - expect(resolver.credentials.access_key_id).to eq('CACHED-AKID') - expect(resolver.source_calls).to eq(1) - - resolver.release << :go - sleep 0.1 # let the background refresh publish new credentials - - expect(resolver.credentials.access_key_id).to eq('FRESH-AKID') - expect(resolver.source_calls).to eq(1) - end - it 'runs a single mandatory refresh while other callers wait and reuse the result' do # mandatory window (60s) resolver = build_gated_resolver(ttl: 30, advisory_window: 600) From 6f464d8a5ac9fb0583dbc5ccbf4fc87648fa78e1 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Fri, 25 Sep 2026 11:02:46 -0700 Subject: [PATCH 18/18] Cleanup --- .../lib/aws-sdk-core/assume_role_credentials.rb | 1 - .../assume_role_web_identity_credentials.rb | 1 - .../lib/aws-sdk-core/credential_provider_chain.rb | 4 +--- gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb | 8 +++----- .../aws-sdk-core/resilient_refreshing_credentials.rb | 11 +---------- 5 files changed, 5 insertions(+), 20 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb index 974af3d8305..082ce7acd6a 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_credentials.rb @@ -49,7 +49,6 @@ def initialize(options = {}) end end @client = client_opts[:client] || STS::Client.new(client_opts) - warn("[SEP-DEBUG] AssumeRoleCredentials STS client max_attempts=#{@client.config.max_attempts}, retry_mode=#{@client.config.retry_mode}") @metrics = ['CREDENTIALS_STS_ASSUME_ROLE'] super end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb index 8802137d8b1..2da455a7cc9 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/assume_role_web_identity_credentials.rb @@ -60,7 +60,6 @@ def initialize(options = {}) @assume_role_web_identity_params[:role_session_name] = _session_name end @client = client_opts[:client] || STS::Client.new(client_opts.merge(credentials: nil)) - warn("[SEP-DEBUG] AssumeRoleWebIdentityCredentials STS client max_attempts=#{@client.config.max_attempts}, retry_mode=#{@client.config.retry_mode}") @metrics = ['CREDENTIALS_STS_ASSUME_ROLE_WEB_ID'] super end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/credential_provider_chain.rb b/gems/aws-sdk-core/lib/aws-sdk-core/credential_provider_chain.rb index 223aeebbd46..4e38797a528 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/credential_provider_chain.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/credential_provider_chain.rb @@ -240,9 +240,7 @@ def instance_profile_credentials(options) InstanceProfileCredentials.new(options.merge(profile: profile_name)) end rescue Errors::MissingCredentialsError - # credential source was unreachable on initial fetch, skip so chain moves on. - # Non-recoverable errors are the source error (not MissingCredentialsError) - # and still propagate. + # credential source was unreachable on initial fetch, skip so chain moves on nil end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb index 5d32d0a28df..5bbe8bc360b 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb @@ -149,7 +149,7 @@ def sign(context) # Record the signing credentials so the retry layer can invalidate # them on an auth failure and only if they still match - context[:signing_credentials] = signing_identity(signature) + context[:signing_credentials] = signing_credentials(signature) # add request metadata with signature components for debugging context[:canonical_request] = signature.canonical_request @@ -170,10 +170,8 @@ def credentials private - # Identity of the credentials that signed a request, for invalidation - # matching. Only the access key id is recoverable from the signature - # and it is the only part invalidation compares on. - def signing_identity(signature) + # Credentials that signed a request, used for invalidation matching + def signing_credentials(signature) authorization = signature.headers['authorization'] return unless authorization diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/resilient_refreshing_credentials.rb b/gems/aws-sdk-core/lib/aws-sdk-core/resilient_refreshing_credentials.rb index a3089a39d8f..e627f5fdea2 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/resilient_refreshing_credentials.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/resilient_refreshing_credentials.rb @@ -1,24 +1,15 @@ # frozen_string_literal: true module Aws - # Mixed into the credential providers that are in scope for the Credential - # Refresh SEP. Implements the statically stable refresh lifecycle: caching, + # Implements the statically stable refresh lifecycle: caching, # an advisory and a mandatory refresh window, rate-limited backoff on # failure, static stability (continue using cached credentials when a # refresh fails), and short-lived caching of non-recoverable errors. # - # Providers whose behavior is out of scope for the SEP (Process, S3 Express, - # and customer-provided providers) use {RefreshingCredentials} instead, which - # preserves the simpler pre-SEP refresh behavior. - # # Classes mixing in this module must implement `#refresh`, which fetches # from the source and assigns `@credentials` and `@expiration` on success, # or raises on failure. It must not partially update those on failure. # - # Advisory refresh is non-blocking in the sense the SEP defines: the caller - # that acquires the refresh lock refreshes inline and adopts the result, - # while concurrent callers get the cached credentials without waiting. - # # Before calling `super`, classes may set `@static_stability` to false for # caching-only behavior. Classes may override `#non_recoverable_error?` to # classify provider errors that should be raised immediately rather than