diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 44498f6ffb..54cbd7844f 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -22,3 +22,9 @@ message = "Fix server SDK bug with directly constrained list/map shapes in opera references = ["smithy-rs#2761"] meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "server"} author = "david-perez" + +[[aws-sdk-rust]] +message = "`SsoCredentialsProvider`, `AssumeRoleProvider`, and `WebIdentityTokenCredentialsProvider` now use `NoCredentialsCache` internally when fetching credentials using an STS client. This avoids double-caching when these providers are wrapped by `LazyCredentialsCache` when a service client is created." +references = ["smithy-rs#2720"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "ysaito1001" diff --git a/aws/rust-runtime/aws-config/src/sso.rs b/aws/rust-runtime/aws-config/src/sso.rs index 7c693bdf2e..6adc586ab4 100644 --- a/aws/rust-runtime/aws-config/src/sso.rs +++ b/aws/rust-runtime/aws-config/src/sso.rs @@ -14,6 +14,7 @@ use crate::fs_util::{home_dir, Os}; use crate::json_credentials::{json_parse_loop, InvalidJsonCredentials}; use crate::provider_config::ProviderConfig; +use aws_credential_types::cache::CredentialsCache; use aws_credential_types::provider::{self, error::CredentialsError, future, ProvideCredentials}; use aws_credential_types::Credentials; use aws_sdk_sso::middleware::DefaultMiddleware as SsoMiddleware; @@ -211,6 +212,7 @@ async fn load_sso_credentials( .map_err(CredentialsError::provider_error)?; let config = aws_sdk_sso::Config::builder() .region(sso_config.region.clone()) + .credentials_cache(CredentialsCache::no_caching()) .build(); let operation = GetRoleCredentialsInput::builder() .role_name(&sso_config.role_name) diff --git a/aws/rust-runtime/aws-config/src/sts/assume_role.rs b/aws/rust-runtime/aws-config/src/sts/assume_role.rs index 35f2b3fa27..66ad8cd89d 100644 --- a/aws/rust-runtime/aws-config/src/sts/assume_role.rs +++ b/aws/rust-runtime/aws-config/src/sts/assume_role.rs @@ -179,7 +179,13 @@ impl AssumeRoleProviderBuilder { self } + #[deprecated( + note = "This should not be necessary as the default, no caching, is usually what you want." + )] /// Set the [`CredentialsCache`] for credentials retrieved from STS. + /// + /// By default, an [`AssumeRoleProvider`] internally uses `NoCredentialsCache` because the + /// provider itself will be wrapped by `LazyCredentialsCache` when a service client is created. pub fn credentials_cache(mut self, cache: CredentialsCache) -> Self { self.credentials_cache = Some(cache); self @@ -198,11 +204,9 @@ impl AssumeRoleProviderBuilder { pub fn build(self, provider: impl ProvideCredentials + 'static) -> AssumeRoleProvider { let conf = self.conf.unwrap_or_default(); - let credentials_cache = self.credentials_cache.unwrap_or_else(|| { - let mut builder = CredentialsCache::lazy_builder().time_source(conf.time_source()); - builder.set_sleep(conf.sleep()); - builder.into_credentials_cache() - }); + let credentials_cache = self + .credentials_cache + .unwrap_or_else(CredentialsCache::no_caching); let config = aws_sdk_sts::Config::builder() .credentials_cache(credentials_cache) @@ -333,39 +337,71 @@ mod test { } #[tokio::test] - async fn provider_caches_credentials() { + async fn provider_does_not_cache_credentials_by_default() { let conn = TestConnection::new(vec![ (http::Request::new(SdkBody::from("request body")), http::Response::builder().status(200).body(SdkBody::from( - "\n \n \n AROAR42TAWARILN3MNKUT:assume-role-from-profile-1632246085998\n arn:aws:sts::130633740322:assumed-role/imds-chained-role-test/assume-role-from-profile-1632246085998\n \n \n ASIARCORRECT\n secretkeycorrect\n tokencorrect\n 2009-02-13T23:31:30Z\n \n \n \n d9d47248-fd55-4686-ad7c-0fb7cd1cddd7\n \n\n" + "\n \n \n AROAR42TAWARILN3MNKUT:assume-role-from-profile-1632246085998\n arn:aws:sts::130633740322:assumed-role/assume-provider-test/assume-role-from-profile-1632246085998\n \n \n ASIARCORRECT\n secretkeycorrect\n tokencorrect\n 2009-02-13T23:31:30Z\n \n \n \n d9d47248-fd55-4686-ad7c-0fb7cd1cddd7\n \n\n" )).unwrap()), (http::Request::new(SdkBody::from("request body")), http::Response::builder().status(200).body(SdkBody::from( - "\n \n \n AROAR42TAWARILN3MNKUT:assume-role-from-profile-1632246085998\n arn:aws:sts::130633740322:assumed-role/imds-chained-role-test/assume-role-from-profile-1632246085998\n \n \n ASIARCORRECT\n secretkeycorrect\n tokencorrect\n 2009-02-13T23:31:30Z\n \n \n \n d9d47248-fd55-4686-ad7c-0fb7cd1cddd7\n \n\n" + "\n \n \n AROAR42TAWARILN3MNKUT:assume-role-from-profile-1632246085998\n arn:aws:sts::130633740322:assumed-role/assume-provider-test/assume-role-from-profile-1632246085998\n \n \n ASIARCORRECT\n TESTSECRET\n tokencorrect\n 2009-02-13T23:33:30Z\n \n \n \n c2e971c2-702d-4124-9b1f-1670febbea18\n \n\n" )).unwrap()), ]); + + let mut testing_time_source = TestingTimeSource::new( + UNIX_EPOCH + Duration::from_secs(1234567890 - 120), // 1234567890 since UNIX_EPOCH is 2009-02-13T23:31:30Z + ); + let provider_conf = ProviderConfig::empty() .with_sleep(TokioSleep::new()) - .with_time_source(TimeSource::testing(&TestingTimeSource::new( - UNIX_EPOCH + Duration::from_secs(1234567890 - 120), - ))) + .with_time_source(TimeSource::testing(&testing_time_source)) .with_http_connector(DynConnector::new(conn)); + let credentials_list = std::sync::Arc::new(std::sync::Mutex::new(vec![ + Credentials::new( + "test", + "test", + None, + Some(UNIX_EPOCH + Duration::from_secs(1234567890 + 1)), + "test", + ), + Credentials::new( + "test", + "test", + None, + Some(UNIX_EPOCH + Duration::from_secs(1234567890 + 120)), + "test", + ), + ])); + let credentials_list_cloned = credentials_list.clone(); let provider = AssumeRoleProvider::builder("myrole") .configure(&provider_conf) .region(Region::new("us-east-1")) - .build(provide_credentials_fn(|| async { - Ok(Credentials::for_tests()) + .build(provide_credentials_fn(move || { + let list = credentials_list.clone(); + async move { + let next = list.lock().unwrap().remove(0); + Ok(next) + } })); + + tokio::time::pause(); + let creds_first = provider .provide_credentials() .await .expect("should return valid credentials"); - // The effect of caching is implicitly enabled by a `LazyCredentialsCache` - // baked in the configuration for STS stored in `provider.inner.conf`. + + // After time has been advanced by 120 seconds, the first credentials _could_ still be valid + // if `LazyCredentialsCache` were used, but the provider uses `NoCredentialsCache` by default + // so the first credentials will not be used. + testing_time_source.advance(Duration::from_secs(120)); + let creds_second = provider .provide_credentials() .await - .expect("cached credentials should be returned"); - assert_eq!(creds_first, creds_second); + .expect("should return the second credentials"); + assert_ne!(creds_first, creds_second); + assert!(credentials_list_cloned.lock().unwrap().is_empty()); } } diff --git a/aws/rust-runtime/aws-config/src/web_identity_token.rs b/aws/rust-runtime/aws-config/src/web_identity_token.rs index dd13524b06..31819a1952 100644 --- a/aws/rust-runtime/aws-config/src/web_identity_token.rs +++ b/aws/rust-runtime/aws-config/src/web_identity_token.rs @@ -63,6 +63,7 @@ use crate::provider_config::ProviderConfig; use crate::sts; +use aws_credential_types::cache::CredentialsCache; use aws_credential_types::provider::{self, error::CredentialsError, future, ProvideCredentials}; use aws_sdk_sts::config::Region; use aws_sdk_sts::middleware::DefaultMiddleware; @@ -235,6 +236,7 @@ async fn load_credentials( })?; let conf = aws_sdk_sts::Config::builder() .region(region.clone()) + .credentials_cache(CredentialsCache::no_caching()) .build(); let operation = AssumeRoleWithWebIdentityInput::builder() diff --git a/aws/rust-runtime/aws-credential-types/src/cache.rs b/aws/rust-runtime/aws-credential-types/src/cache.rs index 23e90c48fa..4d4810efab 100644 --- a/aws/rust-runtime/aws-credential-types/src/cache.rs +++ b/aws/rust-runtime/aws-credential-types/src/cache.rs @@ -7,9 +7,11 @@ mod expiring_cache; mod lazy_caching; +mod no_caching; pub use expiring_cache::ExpiringCache; pub use lazy_caching::Builder as LazyBuilder; +use no_caching::NoCredentialsCache; use crate::provider::{future, SharedCredentialsProvider}; use std::sync::Arc; @@ -63,6 +65,7 @@ impl ProvideCachedCredentials for SharedCredentialsCache { #[derive(Clone, Debug)] pub(crate) enum Inner { Lazy(lazy_caching::Builder), + NoCaching, } /// `CredentialsCache` allows for configuring and creating a credentials cache. @@ -104,10 +107,18 @@ impl CredentialsCache { lazy_caching::Builder::new() } + /// Creates a [`CredentialsCache`] that offers no caching ability. + pub fn no_caching() -> Self { + Self { + inner: Inner::NoCaching, + } + } + /// Creates a [`SharedCredentialsCache`] wrapping a concrete caching implementation. pub fn create_cache(self, provider: SharedCredentialsProvider) -> SharedCredentialsCache { match self.inner { Inner::Lazy(builder) => SharedCredentialsCache::new(builder.build(provider)), + Inner::NoCaching => SharedCredentialsCache::new(NoCredentialsCache::new(provider)), } } } diff --git a/aws/rust-runtime/aws-credential-types/src/cache/no_caching.rs b/aws/rust-runtime/aws-credential-types/src/cache/no_caching.rs new file mode 100644 index 0000000000..5827f5cea1 --- /dev/null +++ b/aws/rust-runtime/aws-credential-types/src/cache/no_caching.rs @@ -0,0 +1,83 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +//! Credentials cache that offers no caching ability + +use crate::cache::ProvideCachedCredentials; +use crate::provider::SharedCredentialsProvider; +use crate::provider::{future, ProvideCredentials}; +use tracing::debug; + +#[derive(Debug)] +pub(crate) struct NoCredentialsCache { + provider: SharedCredentialsProvider, +} + +impl NoCredentialsCache { + pub(crate) fn new(provider: SharedCredentialsProvider) -> Self { + Self { provider } + } +} + +impl ProvideCachedCredentials for NoCredentialsCache { + fn provide_cached_credentials<'a>(&'a self) -> future::ProvideCredentials<'_> + where + Self: 'a, + { + debug!("Delegating `provide_cached_credentials` to `provide_credentials` on the provider"); + self.provider.provide_credentials() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::credential_fn::provide_credentials_fn; + use crate::Credentials; + use std::sync::{Arc, Mutex}; + use std::time::{Duration, SystemTime}; + + fn test_provider(load_list: Vec) -> NoCredentialsCache { + let load_list = Arc::new(Mutex::new(load_list)); + NoCredentialsCache::new(SharedCredentialsProvider::new(provide_credentials_fn( + move || { + let list = load_list.clone(); + async move { + let next = list.lock().unwrap().remove(0); + next + } + }, + ))) + } + + fn epoch_secs(secs: u64) -> SystemTime { + SystemTime::UNIX_EPOCH + Duration::from_secs(secs) + } + + fn credentials(expired_secs: u64) -> Credentials { + Credentials::new("test", "test", None, Some(epoch_secs(expired_secs)), "test") + } + + async fn expect_creds(expired_secs: u64, provider: &NoCredentialsCache) { + let creds = provider + .provide_cached_credentials() + .await + .expect("expected credentials"); + assert_eq!(Some(epoch_secs(expired_secs)), creds.expiry()); + } + + #[tokio::test] + async fn no_caching() { + let credentials_cache = test_provider(vec![ + Ok(credentials(1000)), + Ok(credentials(2000)), + Ok(credentials(3000)), + ]); + + expect_creds(1000, &credentials_cache).await; + expect_creds(2000, &credentials_cache).await; + expect_creds(3000, &credentials_cache).await; + } +}