Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: Adds support for user defined subject token suppliers in AWSCredentials and IdentityPoolCredentials #1336
feat: Adds support for user defined subject token suppliers in AWSCredentials and IdentityPoolCredentials #1336
Changes from 31 commits
b21127a
e6457d9
448a8ec
59eb856
f2ab1a2
0495b7f
8d12e07
a8b2f92
616fb13
b2552eb
d32e19c
6726160
2fc4f99
e5a9c59
bfb83fa
6e7a975
164ac25
a257e55
f09adfa
97946b3
eb08391
5ae2645
61f6ae5
e43d708
bd4604f
08bde82
4a22e08
f40743e
f480b84
188b803
e69108e
4f3e253
c22a4e9
dd659c4
9cd8d96
8963d68
f4cadd2
20c2f7d
abd462b
9835df7
d06eb36
d59fb92
4371463
5b21010
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
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.
I understand it was technically not final before, could you please remind why? Does it change multiple times? if once - it can be final and can make the rest of the logic simpler
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.
The issue with having impersonated credentials final is that the ImpersonatedCredentials object is built by passing the current credential as the SourceCredential. If we have it final, the Impersonated Credential needs to get created in the parent constructor before the child constructor runs, which means the credential object that gets passed as the source credential is not fully instantiated. As a series of steps, the exact problem is:
Since the source credential passed to the impersonated credential isn't actually correct here we have 1 of two options if we want any child class specific builder setters
1: Set the impersonatedCredentialOverride in the child constructor and just use that, ignoring the "final" impersonatedCredentials. This is what we do for executableCredentials, but the issue is that its kind of a weird pattern that we just have the unused and incorrect impersonatedCredentials object attached to the credential, and that it means we can't do any input validation on the child credential.
2: Make impersonatedCredentials not final, and just set it when we use it the first time when retrieving an access token. This lets us get rid of the override, and makes it so the impersonatedCredential is never built with the incorrect source credential.