fix(Aws): Remove SystemCredentials source check during AWS role assume #22859
Conversation
onyxraven
force-pushed
the
assume-role-issue
branch
from
6eef6c3 to
eca2c8a
Compare
|
|
||
| type Resolvers = 'environment' | 'podIdentity' | 'containerMetadata' | 'instanceMetadata'; | ||
| type RetrunData = { | ||
| type ReturnData = { |
There was a problem hiding this comment.
While stepping through understanding, noticed this misspelling. This type is not exported and only used below.
| ); | ||
| }); | ||
|
|
||
| it('should successfully assume role using system credentials by instanceMetadata', async () => { |
There was a problem hiding this comment.
Ultimately, this could (should?) iterate through each source type and ensure it behaves as intended.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
n8n-assistant
bot
added
community
|
Hey @onyxraven, Thank you for your contribution. We appreciate the time and effort you’ve taken to submit this pull request. Before we can proceed, please ensure the following: Regarding new nodes: If your node integrates with an AI service that you own or represent, please email [email protected] and we will be happy to discuss the best approach. About review timelines: Thank you again for contributing to n8n. |
1 participant
Summary
This removes the 'source === "environment"' check done during the role assume for AWS SystemCredentials.
In every case of the SystemCredentials usage for AWS, it is supposed to assume the role required during credential setup (with the given externalId as well). The previous conditional would only do that assume if it was via environment. This left out the (recently added) pod, container, and instance sources, and those would return the system credentials instead (unintended, I think, and a potential security risk). These sources should act the same way.
I added a test using one of the other sources, and red-green tested. I've also tested this "in practice" along with my other PR (#22316) in our environment.
Related Linear tickets, Github issues, and Community forum posts
Fixes #21961
(https://linear.app/n8n/issue/GHC-5527)
Review / Merge checklist
release/backport(if the PR is an urgent fix that needs to be backported)