-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Multi auth request level resolution #3632
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
base: main
Are you sure you want to change the base?
Conversation
| #include <aws/core/auth/signer/AWSAuthEventStreamV4Signer.h> | ||
| #include <aws/core/auth/signer/AWSNullSigner.h> | ||
|
|
||
| #include <smithy/identity/auth/built-in/NoAuthScheme.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: why are we adding headers here if they arent used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated so these are only included during code gen if needed
|
|
||
| std::shared_ptr<AwsCredentialIdentityResolverT> identityResolver() override | ||
| { | ||
| return nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not really a fan of using nullptr to denote NoAuthIdenitityResolver. could we just instead actually implement a NoAuthIdenitityResolver that just returns empty credentials. This would functionally work the same way as it does today in non-smithy clients.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, done
|
|
||
| #if($serviceModel.useSmithyClient && $operation.auth) | ||
| Aws::Vector<smithy::AuthSchemeOption> ${typeInfo.className}::GetRequestSpecificSupportedAuth() const | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super nit: spacing
Issue #, if available:
Description of changes:
Added support for request level auth resolution while maintaining backwards compatibility for non-smithy clients.
Check all that applies:
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.