-
Notifications
You must be signed in to change notification settings - Fork 2.9k
NIFI-14956 - AWS Credentials Provider - Add support for STS AssumeRoleWithWebIdentity #10294
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
Conversation
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.
Thanks for proposing the additional feature for AWS authentication @pvillard31.
On initial review of the AssumeRoleWithWebIdentity along with the instructions provided, there appears to be a mismatch between the intended use case for this strategy and how it would be used. As described in the documentation, and as the name implies, the identity comes from an authenticated web or mobile application user. The setup steps describe provisioning a token, including a refresh token, with a process that normally involves authentication to a web application. Since NiFi does not make use of an authenticated user's credentials directly to drive authentication to other services, the WebIdentity strategy does not seem to align. Additionally, using a user's credentials for service-based authentication is not a good strategy in general.
If there are other use cases, they are worth considering, but I'm not initially inclined to move forward in this particular direction.
|
Thanks for the feedback @exceptionfactory. As I noted in the PR description, what you're pointing out is a pure limitation of using Cognito but I wanted to provide an example that is easy to reproduce for a reviewer willing to try the change in AWS. I would not expect users to actually use Cognito in this context.
I also tested this change with NiFi running on GCP:
As you can see this is way more involved for a reviewer and I did have to build a custom NAR for the token provider implementation in GCP to get the token. For reference: The last link gives an example where STS AssumeRoleWithWebIdentity is used with a service account in the context of EKS with the below role: {
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Principal": {
"Federated": "arn:aws:iam::$account_id:oidc-provider/$oidc_provider"
},
"Action": "sts:AssumeRoleWithWebIdentity",
"Condition": {
"StringEquals": {
"$oidc_provider:aud": "sts.amazonaws.com",
"$oidc_provider:sub": "system:serviceaccount:$namespace:$service_account"
}
}
}
]
}Let me know what you think. |
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.
Thanks for the helpful and substantive background on WebIdentity @pvillard31. The use case for platforms other than web application is clear now, and I see why the Cognito example was useful for testing purposes, if not applicable for general use cases.
I provided some feedback on the implementation details. Most of it seems clear, although I think some adjustment of property descriptors would help make it clear when this particular type of credentials provider is being configured.
...fi/processors/aws/credentials/provider/factory/strategies/AssumeRoleCredentialsStrategy.java
Outdated
Show resolved
Hide resolved
...i/processors/aws/credentials/provider/factory/strategies/WebIdentityCredentialsStrategy.java
Outdated
Show resolved
Hide resolved
...i/processors/aws/credentials/provider/factory/strategies/WebIdentityCredentialsStrategy.java
Outdated
Show resolved
Hide resolved
...i/processors/aws/credentials/provider/factory/strategies/WebIdentityCredentialsStrategy.java
Outdated
Show resolved
Hide resolved
...i/processors/aws/credentials/provider/factory/strategies/WebIdentityCredentialsStrategy.java
Outdated
Show resolved
Hide resolved
...i/processors/aws/credentials/provider/factory/strategies/WebIdentityCredentialsStrategy.java
Outdated
Show resolved
Hide resolved
...ifi/processors/aws/credentials/provider/service/AWSCredentialsProviderControllerService.java
Show resolved
Hide resolved
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.
@pvillard31 Following the merge of changes to remove AWS SDK 1, this pull request can be updated to remove SDK 1 references, then it looks close to completion.
7490ef9 to
501c032
Compare
|
Thanks @exceptionfactory, rebased against main, hopefully I didn't mess it up :) |
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.
Thanks @pvillard31, the latest version looks good! +1 merging
Summary
NIFI-14956 - AWS Credentials Provider - Add support for STS AssumeRoleWithWebIdentity
Given the EOL of the SDK v1, I only implemented a version for the SDK v2
Steps to setup an AWS only test environment for testing this new feature.
(note the commands are using
--profile perso, change base on your needs)Setup variables
Create Cognito User Pool
Create Hosted UI Domain
Create Confidential App Client (with Auth Code Flow)
From the output, capture the below:
And:
Create and confirm a user
Get a Refresh Token for the user
Go to AWS IAM and Create Role
AmazonSQSFullAccessCapture the Role ARN for the role you just created. Example:
Configure NiFi
StandardOauth2AccessTokenProvider$TOKEN_ENDPOINT$REFRESH_TOKEN$CONF_CLIENT_ID$CONF_CLIENT_SECRETAWS Credentials Provider (Web Identity)
$ROLE_ARNnifi-oidc-test$REGIONUse an AWS SDK v2 processor (e.g.,
PutSQS) with this credentials provider and testImportant notes
aud; usingrefresh_token(or code) yields anid_tokenwithaudthat matches the app client ID.Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000Pull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation