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

Add fallback equality on `noAuth` scheme ID (#4232) 

## Motivation and Context
Implements fallback equality for no auth `AuthSchemeId` so that
`AuthSchemeId::from("no_auth")` (legacy) and
`AuthSchemeId::from("noAuth")` (updated) should be treated as
equivalent.

## Description
In #4203, the internal raw strings of pre-defined `AuthSchemeId` were
updated to better align with the Smithy spec
([discussion](https://github.com/smithy-lang/smithy-rs/pull/4203#discussion_r2198206822)).
Acknowledging that this was a breaking change and that customers should
not rely on these internal representations, we did receive reports of
issues related to this update. After discussion, we proceeded with
implementing fallback equality for no auth scheme ID to allow for a
safer rollout.

## Testing
- Added unit tests for manually implemented traits for `AuthSchemeId`,
`PartialEq`, `Hash`, and `Ord`
- Added an auth scheme preference test to verify the legacy `no_auth` is
supported

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] For changes to the smithy-rs codegen or runtime crates, I have
created a changelog entry Markdown file in the `.changelog` directory,
specifying "client," "server," or both in the `applies_to` key.
- [x] For changes to the AWS SDK, generated SDK code, or SDK runtime
crates, I have created a changelog entry Markdown file in the
`.changelog` directory, specifying "aws-sdk-rust" in the `applies_to`
key.

----

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

.changelog/1753385864.md

0 → 100644
+13 −0
Original line number Diff line number Diff line 
---

applies_to:

- aws-sdk-rust

- client

authors:

- ysaito1001

references:

- smithy-rs#4232

breaking: false

new_feature: false

bug_fix: true

---

Add fallback equality on no auth `AuthSchemeId` for backward compatibility, treating `AuthSchemeId::from("no_auth")` (legacy) and `AuthSchemeId::from("noAuth")` (updated) as equivalent.

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

+28 −0
Original line number Diff line number Diff line
@@ -9,6 +9,7 @@ use aws_smithy_http_client::test_util::capture_request; 
use aws_smithy_http_client::test_util::dvr::ReplayingClient;

use aws_smithy_runtime::client::auth::no_auth::NO_AUTH_SCHEME_ID;

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

use aws_smithy_runtime_api::client::auth::AuthSchemeId;



#[tokio::test]

async fn list_objects() {
@@ -160,3 +161,30 @@ async fn no_auth_should_be_selected_when_no_credentials_is_configured() { 
        auth_scheme_id_str = NO_AUTH_SCHEME_ID.inner(),

    )));

}



#[tracing_test::traced_test]

#[tokio::test]

async fn auth_scheme_preference_specifying_legacy_no_auth_scheme_id_should_be_supported() {

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

    let conf = Config::builder()

        .http_client(http_client)

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

        .with_test_defaults()

        .auth_scheme_preference([AuthSchemeId::from("no_auth")])

        .build();

    let client = Client::from_conf(conf);

    let _ = client

        .get_object()

        .bucket("arn:aws:s3::123456789012:accesspoint/mfzwi23gnjvgw.mrap")

        .key("doesnotmatter")

        .send()

        .await;



    // What should appear in the log is the updated no auth scheme ID, not the legacy one.

    // The legacy no auth scheme ID passed to the auth scheme preference merely reprioritizes

    // ones supported in the runtime, which should contain the updated no auth scheme ID.

    assert!(logs_contain(&format!(

        "resolving identity scheme_id=AuthSchemeId {{ scheme_id: \"{auth_scheme_id_str}\" }}",

        auth_scheme_id_str = NO_AUTH_SCHEME_ID.inner(),

    )));

}

rust-runtime/Cargo.lock

+1 −1
Original line number Diff line number Diff line
@@ -642,7 +642,7 @@ dependencies = [ 


[[package]]

name = "aws-smithy-runtime-api"

version = "1.8.4"

version = "1.8.5"

dependencies = [

 "aws-smithy-async",

 "aws-smithy-types",

rust-runtime/aws-smithy-runtime-api/Cargo.toml

+1 −1
Original line number Diff line number Diff line 
[package]

name = "aws-smithy-runtime-api"

version = "1.8.4"

version = "1.8.5"

authors = ["AWS Rust SDK Team <aws-sdk-rust@amazon.com>", "Zelda Hessler <zhessler@amazon.com>"]

description = "Smithy runtime types."

edition = "2021"

rust-runtime/aws-smithy-runtime-api/src/client/auth.rs

+151 −2
Original line number Diff line number Diff line
@@ -151,11 +151,22 @@ impl AsRef<AuthSchemeId> for AuthSchemeId { 
    }

}



// Normalizes auth scheme IDs for comparison and hashing by treating "no_auth" and "noAuth" as equivalent

// by converting "no_auth" to "noAuth".

// This is for backward compatibility; "no_auth" was incorrectly used in earlier GA versions of the SDK and

// could be used still in some places.

const fn normalize_auth_scheme_id(s: &'static str) -> &'static str {

    match s.as_bytes() {

        b"no_auth" => "noAuth",

        _ => s,

    }

}



impl AuthSchemeId {

    /// Creates a new auth scheme ID.

    pub const fn new(scheme_id: &'static str) -> Self {

        Self {

            scheme_id: Cow::Borrowed(scheme_id),

            scheme_id: Cow::Borrowed(normalize_auth_scheme_id(scheme_id)),

        }

    }
@@ -188,7 +199,19 @@ impl From<&'static str> for AuthSchemeId { 


impl From<Cow<'static, str>> for AuthSchemeId {

    fn from(scheme_id: Cow<'static, str>) -> Self {

        Self { scheme_id }

        let normalized_scheme_id = match &scheme_id {

            Cow::Borrowed(s) => Cow::Borrowed(normalize_auth_scheme_id(s)),

            Cow::Owned(s) => {

                if s == "no_auth" {

                    Cow::Borrowed("noAuth")

                } else {

                    scheme_id

                }

            }

        };

        Self {

            scheme_id: normalized_scheme_id,

        }

    }

}
@@ -454,3 +477,129 @@ where 
        }

    }

}



#[cfg(test)]

mod tests {

    use super::*;



    #[test]

    fn test_auth_scheme_id_new_normalizes_no_auth() {

        // Test that "no_auth" gets normalized to "noAuth"

        let auth_scheme_id = AuthSchemeId::new("no_auth");

        assert_eq!(auth_scheme_id.inner(), "noAuth");

    }



    #[test]

    fn test_auth_scheme_id_new_preserves_no_auth_camel_case() {

        // Test that "noAuth" remains unchanged

        let auth_scheme_id = AuthSchemeId::new("noAuth");

        assert_eq!(auth_scheme_id.inner(), "noAuth");

    }



    #[test]

    fn test_auth_scheme_id_new_preserves_other_schemes() {

        // Test that other auth scheme IDs are not modified

        let test_cases = [

            "sigv4",

            "sigv4a",

            "httpBearerAuth",

            "httpBasicAuth",

            "custom_auth",

            "bearer",

            "basic",

        ];



        for scheme in test_cases {

            let auth_scheme_id = AuthSchemeId::new(scheme);

            assert_eq!(auth_scheme_id.inner(), scheme);

        }

    }



    #[test]

    fn test_auth_scheme_id_equality_after_normalization() {

        // Test that "no_auth" and "noAuth" are considered equal after normalization

        let no_auth_underscore = AuthSchemeId::new("no_auth");

        let no_auth_camel = AuthSchemeId::new("noAuth");



        assert_eq!(no_auth_underscore, no_auth_camel);

        assert_eq!(no_auth_underscore.inner(), no_auth_camel.inner());

    }



    #[test]

    fn test_auth_scheme_id_hash_consistency_after_normalization() {

        use std::collections::HashMap;



        // Test that normalized IDs have consistent hashing behavior

        let mut map = HashMap::new();

        let no_auth_underscore = AuthSchemeId::new("no_auth");

        let no_auth_camel = AuthSchemeId::new("noAuth");



        map.insert(no_auth_underscore.clone(), "value1");

        map.insert(no_auth_camel.clone(), "value2");



        // Should only have one entry since they normalize to the same value

        assert_eq!(map.len(), 1);

        assert_eq!(map.get(&no_auth_underscore), Some(&"value2"));

        assert_eq!(map.get(&no_auth_camel), Some(&"value2"));

    }



    #[test]

    fn test_auth_scheme_id_ordering_after_normalization() {

        // Test that ordering works correctly with normalized values

        let no_auth_underscore = AuthSchemeId::new("no_auth");

        let no_auth_camel = AuthSchemeId::new("noAuth");

        let other_scheme = AuthSchemeId::new("sigv4");



        assert_eq!(

            no_auth_underscore.cmp(&no_auth_camel),

            std::cmp::Ordering::Equal

        );

        assert_eq!(no_auth_underscore.cmp(&other_scheme), "noAuth".cmp("sigv4"));

    }



    #[test]

    fn test_normalize_auth_scheme_id_function() {

        // Test the normalize function directly

        assert_eq!(normalize_auth_scheme_id("no_auth"), "noAuth");

        assert_eq!(normalize_auth_scheme_id("noAuth"), "noAuth");

        assert_eq!(normalize_auth_scheme_id("sigv4"), "sigv4");

        assert_eq!(normalize_auth_scheme_id("custom"), "custom");

    }



    #[test]

    fn test_auth_scheme_id_from_cow_borrowed_normalizes_no_auth() {

        // Test that Cow::Borrowed("no_auth") gets normalized to "noAuth"

        let auth_scheme_id = AuthSchemeId::from(Cow::Borrowed("no_auth"));

        assert_eq!(auth_scheme_id.inner(), "noAuth");

    }



    #[test]

    fn test_auth_scheme_id_from_cow_borrowed_preserves_no_auth_camel_case() {

        // Test that Cow::Borrowed("noAuth") remains unchanged

        let auth_scheme_id = AuthSchemeId::from(Cow::Borrowed("noAuth"));

        assert_eq!(auth_scheme_id.inner(), "noAuth");

    }



    #[test]

    fn test_auth_scheme_id_from_cow_owned_normalizes_no_auth() {

        // Test that Cow::Owned(String::from("no_auth")) gets normalized to "noAuth"

        let auth_scheme_id = AuthSchemeId::from(Cow::Owned(String::from("no_auth")));

        assert_eq!(auth_scheme_id.inner(), "noAuth");

    }



    #[test]

    fn test_auth_scheme_id_from_cow_owned_preserves_no_auth_camel_case() {

        // Test that Cow::Owned(String::from("noAuth")) remains unchanged

        let auth_scheme_id = AuthSchemeId::from(Cow::Owned(String::from("noAuth")));

        assert_eq!(auth_scheme_id.inner(), "noAuth");

    }



    #[test]

    fn test_auth_scheme_id_from_cow_between_borrowed_and_owned_mixing_updated_and_legacy() {

        let borrowed_no_auth = AuthSchemeId::from(Cow::Borrowed("noAuth"));

        let owned_no_auth = AuthSchemeId::from(Cow::Owned(String::from("no_auth")));



        assert_eq!(borrowed_no_auth, owned_no_auth);

        assert_eq!(borrowed_no_auth.inner(), owned_no_auth.inner());

    }

}