Unverified Commit 3e81645f authored by ysaito1001's avatar ysaito1001 Committed by GitHub
Browse files

Fix `sigv4-s3express` position in `auth_scheme_options` (#3498) 

## Motivation and Context
Fixes an S3-related bug introduced when s3 Express was added

## Description
This PR fixes a bug where s3 Express auth flow was incorrectly used when
no auth schemes were available for an endpoint. This use case was
observed in test settings and manifested itself as a confusing behavior.

To address this issue, the fix ensures that an auth scheme for S3
express will be registered after that for sigv4 has been registered.

## Testing
Added an integration test
`s3_express_auth_flow_should_not_be_reached_with_no_auth_schemes`

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent f341aa22

CHANGELOG.next.toml

+6 −0
Original line number Diff line number Diff line
@@ -38,3 +38,9 @@ Users may also ignore endpoint URLs sourced from the env and profile files: 
references = ["smithy-rs#3488"]

meta = { "breaking" = false, "tada" = true, "bug" = false }

authors = ["Velfi"]



[[aws-sdk-rust]]

message = "Fix a bug where a `sigv4-s3express` auth scheme was incorrectly positioned at the front of `auth_scheme_options` and was used when no auth schemes were available for an endpoint."

references = ["smithy-rs#3498"]

meta = { "breaking" = false, "bug" = true, "tada" = false }

author = "ysaito1001"

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4AuthDecorator.kt

+6 −2
Original line number Diff line number Diff line
@@ -61,7 +61,7 @@ class SigV4AuthDecorator : ConditionalDecorator( 
    delegateTo =

        object : ClientCodegenDecorator {

            override val name: String get() = "SigV4AuthDecorator"

            override val order: Byte = 0

            override val order: Byte = ORDER



            private val sigv4a = "sigv4a"
@@ -121,7 +121,11 @@ class SigV4AuthDecorator : ConditionalDecorator( 
                }

            }

        },

)

) {

    companion object {

        const val ORDER: Byte = 0

    }

}



private class SigV4SigningConfig(

    runtimeConfig: RuntimeConfig,

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/s3/S3ExpressDecorator.kt

+4 −1
Original line number Diff line number Diff line
@@ -33,10 +33,13 @@ import software.amazon.smithy.rust.codegen.core.util.getTrait 
import software.amazon.smithy.rustsdk.AwsCargoDependency

import software.amazon.smithy.rustsdk.AwsRuntimeType

import software.amazon.smithy.rustsdk.InlineAwsDependency

import software.amazon.smithy.rustsdk.SigV4AuthDecorator



class S3ExpressDecorator : ClientCodegenDecorator {

    override val name: String = "S3ExpressDecorator"

    override val order: Byte = 0



    // This decorator must decorate after SigV4AuthDecorator so that sigv4 appears before sigv4-s3express within auth_scheme_options

    override val order: Byte = (SigV4AuthDecorator.ORDER - 1).toByte()



    private fun sigv4S3Express(runtimeConfig: RuntimeConfig) =

        writable {

aws/sdk/integration-tests/s3/tests/express.rs

+37 −0
Original line number Diff line number Diff line
@@ -5,7 +5,9 @@ 


use std::time::{Duration, SystemTime};



use aws_config::timeout::TimeoutConfig;

use aws_config::Region;

use aws_sdk_s3::config::endpoint::{EndpointFuture, Params, ResolveEndpoint};

use aws_sdk_s3::config::{Builder, Credentials};

use aws_sdk_s3::presigning::PresigningConfig;

use aws_sdk_s3::primitives::SdkBody;
@@ -16,6 +18,7 @@ use aws_smithy_runtime::client::http::test_util::{ 
    capture_request, ReplayEvent, StaticReplayClient,

};

use aws_smithy_runtime::test_util::capture_test_logs::capture_test_logs;

use aws_smithy_types::endpoint::Endpoint;

use http::Uri;



async fn test_client<F>(update_builder: F) -> Client
@@ -410,3 +413,37 @@ async fn support_customer_overriding_express_credentials_provider() { 
    assert_eq!(expected_session_token, actual_session_token);

    assert!(req.headers().get("x-amz-s3session-token").is_none());

}



#[tokio::test]

async fn s3_express_auth_flow_should_not_be_reached_with_no_auth_schemes() {

    #[derive(Debug)]

    struct TestResolver {

        url: String,

    }

    impl ResolveEndpoint for TestResolver {

        fn resolve_endpoint(&self, _params: &Params) -> EndpointFuture<'_> {

            EndpointFuture::ready(Ok(Endpoint::builder().url(self.url.clone()).build()))

        }

    }



    let (http_client, request) = capture_request(None);

    let conf = Config::builder()

        .http_client(http_client)

        .region(Region::new("us-west-2"))

        .endpoint_resolver(TestResolver {

            url: "http://127.0.0.1".to_owned(),

        })

        .with_test_defaults()

        .timeout_config(

            TimeoutConfig::builder()

                .operation_attempt_timeout(Duration::from_secs(1))

                .build(),

        )

        .build();

    let client = Client::from_conf(conf);



    // Note that we pass a regular bucket; when the bug was present, it still went through s3 Express auth flow.

    let _ = client.list_objects_v2().bucket("test-bucket").send().await;

    // If s3 Express auth flow were exercised, no request would be received, most likely due to `TimeoutError`.

    let _ = request.expect_request();

}

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/customize/CoreCodegenDecorator.kt

+4 −1
Original line number Diff line number Diff line
@@ -30,7 +30,10 @@ interface CoreCodegenDecorator<CodegenContext, CodegenSettings> { 
    val name: String



    /**

     * Enable a deterministic ordering to be applied, with the lowest numbered integrations being applied first

     * Enable a deterministic ordering to be applied

     *

     * Lowest numbered integrations are applied last since they are applied in reverse order of their positions

     * in the list of decorators.

     */

    val order: Byte