Smithy Identity auth refactor for Rest XML S3 client #3262

sbera87 · 2025-01-20T19:25:56Z

Issue #, if available:

Description of changes:

Check all that applies:

Did a review by yourself.
Added proper tests to cover this PR. (If tests are not applicable, explain.)
Checked if this PR is a breaking (APIs have been changed) change.
Checked if this PR will not introduce cross-platform inconsistent behavior.
Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

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

# This is the 1st commit message: s3 code gen updates # This is the commit message #2: fixes # This is the commit message #3: updates # This is the commit message #4: updates # This is the commit message #5: compiling version # This is the commit message #6: updates # This is the commit message #7: test changes # This is the commit message #8: updates # This is the commit message #9: temp changes in s3client # This is the commit message #10: for debug # This is the commit message #11: Update API model # This is the commit message #12: corrects the dual-stack endpoint configuration for cognitoidp Added DeleteContactFlowVersion API and the CAMPAIGN flow type AWS IoT SiteWise now supports ingestion and querying of Null (all data types) and NaN (double type) values of bad or uncertain data quality. New partial error handling prevents data loss during ingestion. Enabled by default for new customers; existing customers can opt-in. Documentation-only update to address doc errors Documentation-only update: clarified the description of the shareDecaySeconds parameter of the FairsharePolicy data type, clarified the description of the priority parameter of the JobQueueDetail data type. Added `DigitGroupingStyle` in ThousandsSeparator to allow grouping by `LAKH`( Indian Grouping system ) currency. Support LAKH and `CRORE` currency types in Column Formatting. This release adds support for the topic attribute FifoThroughputScope for SNS FIFO topics. For details, see the documentation history in the Amazon Simple Notification Service Developer Guide. Increasing entryPoint in SparkSubmit to accept longer script paths. New limit is 4kb. # This is the commit message #13: upgrade smithy version (#3263) # This is the commit message #14: Refactor and simplify logic in sdksCommon cmake script to not rely on c2j model (#3251) # This is the commit message #15: Update API model # This is the commit message #16: Adds multi-turn input support for an Agent node in an Amazon Bedrock Flow Docs Update for timeout changes Rename WorkSpaces Web to WorkSpaces Secure Browser AWS Elemental MediaLive adds a new feature, ID3 segment tagging, in CMAF Ingest output groups. It allows customers to insert ID3 tags into every output segment, controlled by a newly added channel schedule action Id3SegmentTagging. # This is the commit message #17: Fix serialization for query xml # This is the commit message #18: Update API model # This is the commit message #19: Added "future" allocation type for future dated capacity reservation # This is the commit message #20: add stable order for smoke tests + restoring client codegen (#3265) # This is the commit message #21: add protocol test models (clients+tests) to the repo

sbera87 · 2025-02-03T03:33:15Z

...ain/java/com/amazonaws/util/awsclientgenerator/domainmodels/codegeneration/ServiceModel.java

@@ -90,4 +90,5 @@ public Operation getOperationForRequestShapeName(String requestShapeName) {
    ClientContextParams clientContextParams;
    boolean useSmithyClient;
    List<String> authSchemes;
+    String rawEndpointRules;


This is needed to parse at the service model level since otherwise only shortened rules get stored

sbera87 · 2025-02-03T03:34:09Z

...r/src/main/java/com/amazonaws/util/awsclientgenerator/generators/cpp/CppClientGenerator.java

@@ -836,4 +842,105 @@ private void addRequestlessRequestObjectS(final ServiceModel serviceModel) {
                    requestlessOperations.add(operation.getName());
                });
    }
+
+    //dfs 


Since this is the base class , added here so that it can be extended to any client code generator which needs multi auth

sbera87 · 2025-02-03T03:34:49Z

...r/src/main/java/com/amazonaws/util/awsclientgenerator/generators/cpp/CppClientGenerator.java

+        serviceModel.setAuthSchemes(authschemes);
+    }
+    //auth schemes can be named differently in endpoints/operations, this is a mapping
+    private static final Map<String, String> AuthSchemeNameMapping = ImmutableMap.of(


mapping is needed to simplify auth scheme lookup

sbera87 · 2025-02-03T03:40:45Z

src/aws-cpp-sdk-core/source/smithy/client/AwsSmithyClientBase.cpp

+    m_httpClient->EnableRequestProcessing();
+}
+
+StreamOutcome AwsSmithyClientBase::MakeRequestWithUnparsedResponse(Aws::AmazonWebServiceRequest const * const request,


Different outcome handling as legacy but reusing smithy functions

sbera87 · 2025-02-03T03:49:41Z

tests/aws-cpp-sdk-core-tests/smithy/client/SmithyClientTest.cpp

@@ -305,7 +305,7 @@ TEST_F(SmithyClientTest, testSigV4a) {

    EXPECT_EQ(res2.IsSuccess(), true);

-    EXPECT_TRUE(res2.GetResult()->GetSigningAccessKey().empty());
+    EXPECT_TRUE(!res2.GetResult()->GetSigningAccessKey().empty());


previously, this was missing support which legacy had. Now added and tested

sbera87 · 2025-02-03T03:50:20Z

tests/aws-cpp-sdk-s3-integration-tests/BucketAndObjectOperationTest.cpp

@@ -150,13 +150,37 @@ namespace
        long CalculateDelayBeforeNextRetry(const AWSError<CoreErrors>&, long) const override { return 0; }
    };

+    class S3TestClient : public S3Client


This wrapper client is used instead because of use of protected methods in smithy client. Else will need to be exposed.

sbera87 · 2025-02-04T16:41:08Z

src/aws-cpp-sdk-core/source/smithy/client/AwsSmithyClientBase.cpp

        {
            AWS_LOGSTREAM_DEBUG(AWS_SMITHY_CLIENT_LOG, "Request returned error. Attempting to generate appropriate error codes from response");
            assert(m_errorMarshaller);
            auto error = m_errorMarshaller->BuildAWSError(httpResponse);
            return HttpResponseOutcome(std::move(error));
        }
+        else if (hasEmbeddedError()) {


Follows legacy design. Without this face error marshalling test errors. Embedded error is handled differently that other errors. This invokes client overriden implementation which changes the underlying iostream ptrs as a result of which one fetching the response body more than once sets the fail bit BuildAWSError causes incorrect parsing of the error

sbera87 · 2025-02-04T16:46:32Z

...esources/com/amazonaws/util/awsclientgenerator/velocity/cpp/smithy/SmithyClientSourceInit.vm

 #end
        })
 {}

 /* Legacy constructors due deprecation */
+#if($serviceModel.metadata.serviceId == "S3")


s3 has specific signature of legacy, hence need this

generated/src/aws-cpp-sdk-s3/include/aws/s3/S3Client.h

sbera87 · 2025-02-04T16:49:11Z

src/aws-cpp-sdk-core/include/smithy/identity/auth/AuthSchemeOption.h

-        PropertyBag virtual signerProperties() const { return PropertyBag{}; };
+        PropertyBag virtual identityProperties() const { return m_identityProperties; };
+        PropertyBag virtual signerProperties() const { return m_signerProperties; };
+        void putIdentityProperty( const Aws::String& key,const Aws::Crt::Variant<Aws::String, bool>& value) {


Need this now as the resolution interfaces accepts properties which will need all the parameters that was used previously but now simply respective property bag for identity or signer

sbiscigl

high level feedback:

there are some backwards incompatible changes that we cant do
we shouldnt care about signer in identity provider, we should remove all references to signer in identity provider.
theres now s3-crt in here, we likely want to do that at the same time.
s3 components should be scoped to the s3 library and shouldnt leak into core

sbiscigl · 2025-02-04T18:41:15Z

generated/src/aws-cpp-sdk-s3/include/aws/s3/S3ClientConfiguration.h

@@ -58,10 +57,6 @@ namespace Aws
            bool useArnRegion = false;
            Client::AWSAuthV4Signer::PayloadSigningPolicy payloadSigningPolicy = Client::AWSAuthV4Signer::PayloadSigningPolicy::RequestDependent;
            bool disableS3ExpressAuth = false;
-            using IdentityProviderSupplier = std::function<std::shared_ptr<S3ExpressIdentityProvider> (const S3Client &)>;


you cant remove this because it is part of the public interface i.e.

auto main() -> int { SDKOptions options{}; InitAPI(options); { S3ClientConfiguration configuration{}; configuration.identityProviderSupplier = [](const S3Client &client) -> std::shared_ptr<S3ExpressIdentityProvider> { //... } S3Client client{configuration}; } ShutdownAPI(options); return 0; }

that is valid code and our customers have been using that to override the identity provider.

sbiscigl · 2025-02-04T18:42:33Z

generated/src/aws-cpp-sdk-s3/include/aws/s3/S3ExpressIdentity.h

@@ -1,71 +0,0 @@
-/**


what if someone is including this header in their project right now? that would break them, i dont think you can simply remove these. i.e.

#include <aws/core/Aws.h> #include <aws/s3/S3ExpressIdentity.h> using namespace Aws; using namespace Aws::S3; auto main() -> int { SDKOptions options{}; InitAPI(options); { S3ExpressIdentity indentity{}; } ShutdownAPI(options); return 0; }

This is already resolved now

sbiscigl · 2025-02-04T19:03:01Z

generated/src/aws-cpp-sdk-s3/source/S3ExpressIdentityResolver.cpp

+            return ResolveIdentityFutureOutcome();
+        }
+        auto creds = m_credsProvider->GetAWSCredentials();
+        return ResolveIdentityFutureOutcome(Aws::MakeUnique<AwsCredentialIdentity>("DefaultAwsCredentialIdentityResolver",  AwsCredentialIdentity{creds.GetAWSAccessKeyId(), creds.GetAWSSecretKey(), creds.GetSessionToken(), creds.GetExpiration()}));


nit: why are you using DefaultAwsCredentialIdentityResolver instead of S3_EXPRESS_IDENTITY_PROVIDER

COpy paste error. Will fix

sbiscigl · 2025-02-04T20:18:01Z

generated/src/aws-cpp-sdk-s3/source/S3ExpressIdentityResolver.cpp

+    }
+
+    //if signer name is not s3 express as set in signer properties, get from credential provider
+    auto signerName = params->parameterMap.find(smithy::AUTH_SCHEME_PROPERTY);


i think search for the signer here isnt the intention of the SRA the S3ExpressIdentityResolver should not be interacting with signer at all. the s3 express identity provider should only be responsible for providing the identity for s3 express, i.e. from calling the connect API. before going into this rather long winded explanation you actually did get the general flow down, but i think we drifted from the goal.

at a very very very very high level the call flow is supposed to follow

struct Client { public: http_response MakeRequest(http_request& request) { const auto schemes = auth_scheme_resolver_->resolveAuthScheme(); const auto option = SelectFirstKnownAuthOption(schemes, auth_schemes_); auth_scheme_variant authScheme = auth_schemes_[option.schemeId]; auto signOutcome = sign_request(request, authOption); auto http_response = http_client.send_request(signOutcome.http_request); return http_response; } private: signing_outcome sign_request(http_request& request, const auth_option& option) { //...visit the variant } std::map<std::string, AuthOption> auth_schemes_; AuthSchemeResolverBase<>* auth_scheme_resolver_; };

so in this case the two different variants of Identity that are returned are either AwsIdentity or S3 Express Identity. the signer does not need to know which one it is getting, it will just visit variants. the identity lives by itself.

at a very high level the calls are supposed to look like this

so translating this to our use. we dont care about signer in idenitity provider, we just provide identity.

i think this boils down to the idea of AuthSchemeOption and the "Choose Auth Scheme" where we declare a auth scheme as such

class AuthScheme { public: virtual std::shared_ptr<IdentityResolverBase<IdentityT>> identityResolver() = 0; virtual std::shared_ptr<AwsSignerBase<IdentityT>> signer() = 0; }; }

ideally we should have something like

class S3ExpressIdentityProvider: public AuthScheme { public: std::shared_ptr<IdentityResolverBase<IdentityT>> identityResolver() override { // call s3 express identity provider } std::shared_ptr<AwsSignerBase<IdentityT>> signer() override { // call s3 express signer } }; };

which you did add

the when we "Choose Auth Scheme", i.e. here in the the smithy client we choose auth sceme option you correctly map it to different auth scheme options.

so why do we care about the signer here, the smithy client chooses the auth scheme, the identity provider just returns the identity.

ok, made the change in the next commit

sbiscigl · 2025-02-04T20:19:16Z

src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClient.h

@@ -98,9 +99,9 @@ namespace client
            return *this;
        }

-        AwsSmithyClientT (AwsSmithyClientT&&) = default;
+        AwsSmithyClientT (AwsSmithyClientT&& other) = default;


nit why do we need to add "other" unused param for default?

artifact of latest add and remove. Will restore

doesnt look like you did

This should be done now

sbiscigl · 2025-02-04T22:15:33Z

src/aws-cpp-sdk-core/include/smithy/identity/signer/built-in/S3ExpressSigner.h

+
+    //Ensuring S3 Express Signer can use Sigv4 or Sigv4a signing algorithm
+    template <typename BASECLASS>
+    class S3ExpressSigner : public std::enable_if<IsValidS3ExpressSigner<BASECLASS>::value, BASECLASS>::type


this should likely exist in the S3 library, since only the S3ExpressAuthScheme uses it, and its created in the constructor there, should live there, and only reference as we visit the auth scheme in the smithy client.

Sure, since s3 name in it , I will relocate this back to where it was

sbiscigl · 2025-02-04T22:35:16Z

src/aws-cpp-sdk-core/include/smithy/identity/auth/built-in/S3ExpressSigV4AuthScheme.h

+namespace smithy {
+    constexpr char SIGV4_EXPRESS[] = "sigv4-s3express";
+
+    class S3ExpressSigV4AuthScheme : public AuthScheme<AwsCredentialIdentityBase>


this should likely be in s3 and not in core. nothing in core directly calls this, it is all populated from constructor calls.

All auth schemes are in core already. Putting this in S3 is fine since s3 name in it

src/aws-cpp-sdk-core/include/smithy/identity/auth/built-in/SigV4MultiAuthResolver.h

sbiscigl · 2025-02-04T22:38:35Z

src/aws-cpp-sdk-core/source/auth/signer/AWSAuthV4Signer.cpp

@@ -209,7 +210,8 @@ bool AWSAuthV4Signer::SignRequestWithCreds(Aws::Http::HttpRequest& request, cons
            0 /* expirationTimeInSeconds doesn't matter for HttpRequestViaHeaders */, Aws::Crt::Auth::SignatureType::HttpRequestViaHeaders);
    }

-    if (!credentials.GetSessionToken().empty())
+    //additional check is needed for s3 header since this is invoked by s3 signer
+    if (!credentials.GetSessionToken().empty() && !request.HasHeader(S3_EXPRESS_HEADER))


Sigv4 signer should be unaware of the s3 express, when we choose a auth scheme, we choose the signer then, and if we do use s3 express signer, we just use it. It is the sigv4 signer with an extra header, so it should be the same as before

src/aws-cpp-sdk-core/source/smithy/client/AwsSmithyClientBase.cpp

sbera87 · 2025-02-10T15:15:33Z

src/aws-cpp-sdk-core/include/smithy/identity/signer/built-in/SignerProperties.h

@@ -0,0 +1,11 @@
+/**
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0.


override properties used for signing. This is updated from endpoints

sbera87 · 2025-02-10T15:44:43Z

tests/aws-cpp-sdk-s3-integration-tests/S3ExpressTest.cpp

+        {
+          if(auth.first == S3::S3ExpressSigV4AuthSchemeOption::s3ExpressSigV4AuthSchemeOption.schemeId)
+          {
+            auth.second.get<S3::S3ExpressSigV4AuthScheme>().signer() = 


legacy signer overriden here for demonstrating usage of backwards compatible interface

sbiscigl · 2025-02-10T15:55:58Z

generated/src/aws-cpp-sdk-s3/include/aws/s3/S3ExpressSigner.h

+    template <>
+    struct ExtractIdentity<smithy::AwsSigV4Signer> {
+      auto operator()(const smithy::AwsCredentialIdentityBase& identity) const -> smithy::AwsCredentialIdentity {
+        return smithy::AwsCredentialIdentity{identity.accessKeyId(), identity.secretAccessKey(), {},{}};


why arent we passing identity.sessionToken() and identity.expiration(), we need to be. furthermore if we do pass those things, why have a template specialization at all?

If we pass expiration and session token it adds other headers which breaks functionality. This is because the logic :

aws-sdk-cpp/src/aws-cpp-sdk-core/source/auth/signer/AWSAuthV4Signer.cpp

Line 214 in 4012ba9

request.SetAwsSessionToken(credentials.GetSessionToken());

This was needed to remove the check for presence of S3_EXPRESS_HEADER in previous commit.
The same exists in the legacy callflow where the session token is not passed to add session token header for S3 express callflow because session token is already set for s3 express in the signer

And when this session token is overriden we get
Error details: HTTP response code: 403
Exception name: AccessDenied
Error message: Access Denied
causing all write operations to fail

so the reason you are getting 403 is because in your auth scheme, you are using a SigV4 signer and not the S3Express signer. your auth scheme should return a s3 express identity provider, and call the s3 express signer. So the question becomes why are we signing the s3 express request with the sigv4 signer and not the s3 express signer

OK debugged through this and i think the missing part is in the specification

Note: S3 Express SessionToken is different from STS SessionToken and X-Amz-Security-Token MUST NOT be set for S3 Express requests when S3 Express Session Credentials are used.

so session token is NOT used, but expiry is, so please set the expiry

Also with the templates and ExtractIdentity I dont think there is a need for this level of template indirection in the sigv4 signer itself. would prefer you to just create an extra identity object in sign that drops these values, or a different signer class altogether.

Thanks for finding the wording in the sep. resolved in next commit.

sbiscigl · 2025-02-10T16:04:08Z

generated/src/aws-cpp-sdk-s3/source/S3Client.cpp

-        }());
-      return GetBucketAnalyticsConfigurationOutcome(MakeRequest(request, endpointResolutionOutcome.GetResult(), Aws::Http::HttpMethod::HTTP_GET));
+       [&]() -> std::shared_ptr<Http::ServiceSpecificParameters> {
+       Aws::Map<Aws::String, Aws::String> params;


we dont need service specific parameters anymore, please remove them, they are legacy and they cause us pain, now that we resolve endpoints in the smithy client we dont need them in generated code anymore

as discussed, this will have to be a separate PR as this PR ensures same functionality as before but in smithy call flow

sbiscigl · 2025-02-10T16:06:51Z

generated/src/aws-cpp-sdk-s3/source/S3Client.cpp


 S3Client& S3Client::operator=(const S3Client &rhs) {
    if (&rhs == this) {
      return *this;
    }
-    BASECLASS::operator=(rhs);
-    m_signerProvider = Aws::MakeShared<Aws::Auth::S3ExpressSignerProvider>(ALLOCATION_TAG,


the reason we had copy constructors and move constructors one S3 was because the S3 express identity provider has a cache, and we dont want copied clients to share the cache with the parent. in this it would appear that they do. we cant do that.

Fair point. We never had a test case to validate this. Will fix this in next commit and add a test case

sbiscigl · 2025-02-10T16:08:19Z

generated/src/aws-cpp-sdk-s3/source/S3ExpressSigner.cpp

+
+//for backwards compatibility
+bool S3ExpressSigner::SignRequest(Aws::Http::HttpRequest& ) const {
+    return false;


The signer should be signing the request? not returning false? wont this break anyone who was using the signer directly?

This is resolved now with old interface as is

sbiscigl · 2025-02-10T16:09:08Z

src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClient.h

@@ -98,9 +99,9 @@ namespace client
            return *this;
        }

-        AwsSmithyClientT (AwsSmithyClientT&&) = default;
+        AwsSmithyClientT (AwsSmithyClientT&& other) = default;


doesnt look like you did

src/aws-cpp-sdk-core/source/smithy/client/AwsSmithyClientBase.cpp

sbiscigl · 2025-02-10T16:24:20Z

generated/src/aws-cpp-sdk-s3/include/aws/s3/S3ExpressSigner.h

+    };
+
+
+    class AWS_S3_API S3ExpressSigner : public Aws::Client::AWSAuthSigner, public S3ExpressSignerBase<smithy::AwsSigV4Signer>{


previously S3ExpressSigner was defined as

class AWS_S3_API S3ExpressSigner : public Aws::Client::AWSAuthV4Signer

which means S3ExpressSigneris a Aws::Client::AWSAuthV4Signer. is this public interface change not a breaking change?

addressed in next commit by retaining old interface and adding separate new interface

sbiscigl · 2025-02-10T16:27:02Z

generated/src/aws-cpp-sdk-s3/include/aws/s3/S3ExpressIdentityProvider.h

-            const S3Client &m_s3Client;
-            mutable std::mutex m_bucketNameMapMutex;
-            Aws::Map<Aws::String, std::shared_ptr<std::mutex>> m_bucketNameMutex;
+        class S3ExpressIdentityProvider : public smithy::IdentityResolverBase<smithy::AwsCredentialIdentityBase> {


the breaks is a public inheritance no? i.e. before we were

class S3ExpressIdentityProvider: public smithy::IdentityResolverBase<S3ExpressIdentity>

now it is

class S3ExpressIdentityProvider : public smithy::IdentityResolverBase<smithy::AwsCredentialIdentityBase>

dont we need to implement both public contracts here for backwards compatibility?

This method ensures backwards compatibility
GetS3ExpressIdentity(const std::shared_ptr<Aws::Http::ServiceSpecificParameters> &serviceSpecificParameters);

consider the following code, this would work before your refactor and fail after

#include <aws/core/Aws.h> #include <aws/s3/S3Client.h> #include <aws/s3/S3ClientConfiguration.h> #include <aws/s3/S3ExpressIdentity.h> #include <aws/s3/S3ExpressIdentityProvider.h> using namespace Aws; using namespace Aws::Http; using namespace Aws::Utils; using namespace Aws::S3; using namespace Aws::S3::Model; class MyIdentityProvider : public S3ExpressIdentityProvider { public: explicit MyIdentityProvider(const S3Client& client):S3ExpressIdentityProvider(client) {} ~MyIdentityProvider() override = default; S3ExpressIdentity GetS3ExpressIdentity( const std::shared_ptr<ServiceSpecificParameters>& serviceSpecificParameters) override { return S3ExpressIdentity{"access_key", "secret_key", "sessions_token", DateTime::Now()}; } ResolveIdentityFutureOutcome getIdentity( const IdentityProperties& identityProperties, const AdditionalParameters& additionalParameters) override { return Aws::MakeUnique<S3ExpressIdentity>("log", "access_key", "secret_key", "sessions_token", DateTime::Now()); } }; auto main() -> int { SDKOptions options; InitAPI(options); { S3ClientConfiguration configuration{}; configuration.identityProviderSupplier = [](const S3Client &client) -> std::shared_ptr<S3ExpressIdentityProvider> { AWS_UNREFERENCED_PARAM(client); return Aws::MakeShared<MyIdentityProvider>("log", client); }; } ShutdownAPI(options); }

I will implement both versions of identity provider in next commit to facilitate this

sbiscigl · 2025-02-10T16:33:34Z

generated/src/aws-cpp-sdk-s3/source/S3ExpressSignerProvider.cpp

@@ -21,12 +21,12 @@ Aws::Auth::S3ExpressSignerProvider::S3ExpressSignerProvider(
        region,
        signingPolicy,
        urlEscapePath) {
-    m_signers.emplace_back(Aws::MakeShared<Aws::S3::S3ExpressSigner>(CLASS_TAG,
+    m_signers.emplace_back(std::static_pointer_cast<Aws::Client::AWSAuthSigner>(Aws::MakeShared<Aws::S3::S3ExpressSigner>(CLASS_TAG,


why are we casting S3ExpressSigner to a Aws::Client::AWSAuthSigner when it already has the inheritance of
class AWS_S3_API S3ExpressSigner : public Aws::Client::AWSAuthSigner, public S3ExpressSignerBase<smithy::AwsSigV4Signer>, so that static cast should be unneeded

I will just maintain old and new interfaces in next commit. The above is because just wanted to make old interface use new implementation and inherit from abstract base class. This will be changed.

sbiscigl · 2025-02-10T16:41:49Z

...r/src/main/java/com/amazonaws/util/awsclientgenerator/generators/cpp/CppClientGenerator.java

+        findNestedField(jsonElement, "authSchemes", authSchemes);
+        // extract authschemes
+        authSchemes.stream()
+        .filter(entry -> entry.isJsonArray()) // check if the element is a JsonArray


please indent streams correctly, stream is a new line, so you indent it

sure, this shd be done now

SergeyRyabinin · 2025-02-10T21:32:21Z

src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClient.h

+
+            //resolve endpoint first time to fetch auth schemes
+            Aws::Endpoint::EndpointParameters epParams = ctx.m_pRequest ? ctx.m_pRequest->GetEndpointContextParams() : Aws::Endpoint::EndpointParameters();
+            auto epResolutionOutcome = this->ResolveEndpoint(epParams, [](Aws::Endpoint::AWSEndpoint&){});


This endpoint resolution call should be called within

m_authSchemeResolver->resolveAuthScheme(identityParams);

which type is statically selected during code-generation / compilation time and will contain:

endpoint call on multi-auth services;

static auth scheme option return on single-auth services.

The (1) is to be generated for S3 with resolveAuthScheme method overwritten.

ok, I have implemented the change in resolveAuthScheme in subsequent commit in such a way that helps s3 mult auth and any other mult auth client in later prs

sbiscigl

Fix nit and ship

sbiscigl · 2025-02-11T18:30:41Z

src/aws-cpp-sdk-core/include/smithy/identity/auth/built-in/SigV4MultiAuthResolver.h

+#include <smithy/identity/auth/built-in/SigV4AuthSchemeOption.h>
+#include <smithy/identity/auth/built-in/SigV4aAuthSchemeOption.h>
+
+#include "SigV4AuthScheme.h"


no quote imports

sbera87 added 13 commits January 10, 2025 09:27

s3 support

f0bf448

more changes for s3

1b5d497

codegen changes

e6638d3

codegen changes 2

b8676c7

first version s3 client

1aec690

fix s3 legacy constructors

39e841f

s3 express signer

ffa61d3

s3 express related changes

fa017e4

s3 identity resolver changes

30cba54

s3 resolver + constructor updates

df0f1f8

s3 client operations updates

c2ae2e3

merged from main

3a8f644

pending merge conflict

d421218

sbera87 marked this pull request as draft January 23, 2025 17:41

sbera87 changed the title ~~Smithy Identity auth refactor for Rest XML clients [Work in Progress]~~ Smithy Identity auth refactor for Rest XML client S3 [Work in Progress] Jan 30, 2025

sbera87 force-pushed the s3_smithy branch from 31ef089 to 3770e9b Compare January 31, 2025 04:20

sbera87 added 3 commits January 30, 2025 23:23

Codegen updates

98e45f4

multi auth support + sigv4a fixes

64d8d51

sbera87 force-pushed the s3_smithy branch from 3770e9b to 64d8d51 Compare January 31, 2025 04:26

sbera87 added 4 commits January 31, 2025 13:24

sigv4a working

35474c3

merge changes from main

6032dc0

merge from main

5af3458

merge from main

733c1e8

sbera87 commented Feb 3, 2025

View reviewed changes

sbera87 commented Feb 4, 2025

View reviewed changes

generated/src/aws-cpp-sdk-s3/include/aws/s3/S3Client.h Show resolved Hide resolved

sbera87 commented Feb 4, 2025

View reviewed changes

remove reference member for proper move constructor functionality

bc8503a

sbiscigl reviewed Feb 5, 2025

View reviewed changes

structural changes for PR comments

27a5e8b

sbera87 force-pushed the s3_smithy branch from ec7adce to 27a5e8b Compare February 10, 2025 15:05

sbera87 commented Feb 10, 2025

View reviewed changes

cleanup

473c27c

sbera87 requested a review from sbiscigl February 10, 2025 15:31

relocate files

abd92a8

sbera87 commented Feb 10, 2025

View reviewed changes

sbiscigl reviewed Feb 10, 2025

View reviewed changes

Backward compatibility for S3 express endpoint provider

40732db

SergeyRyabinin reviewed Feb 10, 2025

View reviewed changes

sbera87 added 9 commits February 10, 2025 20:08

resolve authscheme structural change

20f79cc

S3 express signer support for old and new interfaces

1e5146c

s3 copy constructor fix

3f154ac

copy constructor fix + test

0940beb

include header for test

9e5302f

fix for linker errors

a08a906

s3 express resolve endpoint updates

b6145a0

address identity supplier backward compatibility

cc4b2df

updates in s3express signer

290750f

sbera87 requested review from SergeyRyabinin and sbiscigl February 11, 2025 17:47

sbiscigl approved these changes Feb 11, 2025

View reviewed changes

sbera87 added 2 commits February 11, 2025 14:42

fixes for pipeline

72ca83b

dry run fixes

2eeab3f

		};


		class AWS_S3_API S3ExpressSigner : public Aws::Client::AWSAuthSigner, public S3ExpressSignerBase<smithy::AwsSigV4Signer>{

Smithy Identity auth refactor for Rest XML S3 client #3262

Are you sure you want to change the base?

Smithy Identity auth refactor for Rest XML S3 client #3262

Conversation

sbera87 commented Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbiscigl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbera87 Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbiscigl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbera87 commented Jan 20, 2025 •

edited

Loading

sbera87 Feb 10, 2025 •

edited

Loading