Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to disable S3 Express session auth #3433

Merged
merged 11 commits into from
Feb 27, 2024
Merged
1 change: 1 addition & 0 deletions aws/rust-runtime/aws-inlineable/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ tracing = "0.1"
aws-smithy-async = { path = "../../../rust-runtime/aws-smithy-async", features = ["test-util"] }
aws-smithy-http = { path = "../../../rust-runtime/aws-smithy-http", features = ["rt-tokio"] }
aws-smithy-runtime-api = { path = "../../../rust-runtime/aws-smithy-runtime-api", features = ["test-util"] }
proptest = "1"
tempfile = "3.6.0"
tokio = { version = "1.23.1", features = ["macros", "rt", "io-util"] }

Expand Down
35 changes: 28 additions & 7 deletions aws/rust-runtime/aws-inlineable/src/s3_express.rs
Original file line number Diff line number Diff line change
Expand Up @@ -690,18 +690,18 @@ pub(crate) mod runtime_plugin {
.is_none()
{
match env.get(env::S3_DISABLE_EXPRESS_SESSION_AUTH) {
Ok(value)
if value.eq_ignore_ascii_case("true")
|| value.eq_ignore_ascii_case("false") =>
{
Ok(value) if valid_environment_variable_value(&value) => {
let value = value
.to_lowercase()
.parse::<bool>()
.expect("just checked to be a bool-valued string");
.expect("just checked to be a valid bool-valued string");
layer.store_or_unset(Some(crate::config::DisableS3ExpressSessionAuth(
value,
)));
}
Ok(value) => panic!(
"Must provide either `true` or `false` for an environment variable `{}`, but got {}",
env::S3_DISABLE_EXPRESS_SESSION_AUTH,
value),
ysaito1001 marked this conversation as resolved.
Show resolved Hide resolved
_ => {
// TODO(aws-sdk-rust#1073): Transfer a value of
// `s3_disable_express_session_auth` from a profile file to `layer`
Expand All @@ -715,6 +715,10 @@ pub(crate) mod runtime_plugin {
}
}

fn valid_environment_variable_value(value: &str) -> bool {
value == "true" || value == "false"
}

impl RuntimePlugin for S3ExpressRuntimePlugin {
fn config(&self) -> Option<FrozenLayer> {
Some(self.config.clone())
Expand All @@ -724,6 +728,8 @@ pub(crate) mod runtime_plugin {
#[cfg(test)]
mod tests {
use super::*;
use proptest::prelude::*;
use proptest::proptest;

#[test]
fn disable_option_set_from_service_client_should_take_the_highest_precedence() {
Expand Down Expand Up @@ -764,8 +770,8 @@ pub(crate) mod runtime_plugin {
);
}

#[should_panic]
#[test]
#[should_panic]
fn disable_option_set_from_profile_file_should_take_the_lowest_precedence() {
// TODO(aws-sdk-rust#1073): Implement a test that mimics only setting
// `s3_disable_express_session_auth` in a profile file
Expand All @@ -785,5 +791,20 @@ pub(crate) mod runtime_plugin {
.load::<crate::config::DisableS3ExpressSessionAuth>()
.is_none());
}

proptest! {
#[test]
#[should_panic]
fn env_value_other_than_case_sensitive_true_or_false_should_result_in_panic(
invalid_env_var_value in any::<String>()
.prop_filter("Use only invalid values for panic test",
|s| !valid_environment_variable_value(&s))
) {
let _ = S3ExpressRuntimePlugin::new_with(
Layer::new("test").freeze(),
Env::from_slice(&[(super::env::S3_DISABLE_EXPRESS_SESSION_AUTH, &invalid_env_var_value)]),
);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ private fun s3ExpressDependencies(runtimeConfig: RuntimeConfig) =
CargoDependency.Hex,
CargoDependency.Hmac,
CargoDependency.Lru,
CargoDependency.Proptest,
CargoDependency.Sha2,
CargoDependency.smithyAsync(runtimeConfig),
CargoDependency.smithyRuntimeApiClient(runtimeConfig),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ class FluentClientGenerator(
/// - Retries or timeouts are enabled without a `sleep_impl` configured.
/// - Identity caching is enabled without a `sleep_impl` and `time_source` configured.
/// - No `behavior_version` is provided.
/// - Base client-level runtime plugins could not be created.
///
/// The panic message for each of these will have instructions on how to resolve them.
##[track_caller]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,8 @@ data class CargoDependency(
val Hound: CargoDependency = CargoDependency("hound", CratesIo("3.4.0"), DependencyScope.Dev)
val PrettyAssertions: CargoDependency =
CargoDependency("pretty_assertions", CratesIo("1.3.0"), DependencyScope.Dev)
val Proptest: CargoDependency =
CargoDependency("proptest", CratesIo("1"), DependencyScope.Dev)
val SerdeJson: CargoDependency = CargoDependency("serde_json", CratesIo("1.0.0"), DependencyScope.Dev)
val Smol: CargoDependency = CargoDependency("smol", CratesIo("1.2.0"), DependencyScope.Dev)
val TempFile: CargoDependency = CargoDependency("tempfile", CratesIo("3.2.0"), DependencyScope.Dev)
Expand Down
Loading