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

[WIP] Modeled authentication resolvers #3517

Open
wants to merge 1 commit into
base: sra-identity-auth
Choose a base branch
from

Conversation

dscpinheiro
Copy link
Contributor

This is a follow-up to #3392 as that branch was out-of-date. I want to make sure the feedback is addressed before generating the resolvers for ~400 services.


// TODO: In the generated version, use a singleton for the scheme resolver (similar to marshallers) to
// prevent creating a new instance for each request.
var authResolver = new AutoScalingAuthSchemeResolver();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there is one instance of AutoScalingAuthSchemeHandler for the the service client you could make AutoScalingAuthSchemeResolver a member variable instead of a singleton.

{
var options = new List<IAuthSchemeOption>();

switch (authParameters.Operation)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't we have to check more then just the operation name for determining the auth? If so then a switch statement might not be a good fit. For example with EventBridge the auth changes if the EndpointId is set. Or is that type of resolution out of scope for the auth resolver.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for services such as S3 and EventBridge we'll need to evaluate the auth options from the endpoint-rule-set files (which is why I was planning to address them separately - their resolvers will still be generated, but their logic will be more complex).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants