Add an mfaToken option to the credentials binding

[weaversam8]

Closes #16.

This plugin has support for specifying AWS Credentials with an MFA device serial #, but doesn't actually expose a method of obtaining a session token from a pipeline if your credentials are configured as such.

This PR adds a new mfaToken option to the AmazonWebServicesCredentialsBinding class, which enables an approach like this in a pipeline. (This is a scripted pipeline example, but it should also work for declarative.)

withCredentials([[
    $class: 'AmazonWebServicesCredentialsBinding',
    credentialsId: '${deployCredentialsId}', // odd templating due to JENKINS-58170
    mfaToken: mfaCode
]]) {
    // ...
}

In the above example, deployCredentialsId is a parameter to the pipeline (ideally pointing to AWS credentials attached to a specific user rather than system level credentials, which is why the parameter is interpolated as a template string. See JENKINS-58170 for more info on this.)

The mfaCode variable is a parameter to the pipeline as well. This enables developers invoking a job to provide an MFA code from their authenticator device when starting the job, ensuring MFA still requires a human element.

This is a draft pull request. While I've built this plugin and verified it works with some basic manual testing, there are still a few unanswered questions that I'll need some assistance with:

• I was unable to run the test suite locally (I was developing in VS Code rather than a Java IDE, and it didn't work out of the box) so I'm unsure whether the existing tests (a) pass and (b) cover my additions. Help from a regular contributor here would be appreciated.
• I am unclear whether this cast is appropriate. It "works", but I'm not sure how it covers edge cases elsewhere in the Jenkins Credentials ecosystem. Will provider ever be any type other than AmazonWebServicesCredentials? If so we may need a different solution. If not, why is this variable typed as AWSCredentialsProvider and not something else?

PR submission checklist

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes
• Ensure you have provided tests - that demonstrates feature works or fixes the issue

[rittneje]

@weaversam8 It is also legal to specify roleArn as a parameter in the code, in which case it will assume that role via the original configured credentials, which may themselves be a role assumption (i.e., role chaining). Consequently, the mfaToken parameter you've added is ambiguous - which role assumption is it for? As currently written, the answer varies depending on whether they specified roleArn in code or not.

Personally, I think allowing people to specify the role as part of the actual credentials themselves was a mistake, and so having your new mfaToken parameter always go with the roleArn parameter would make the most sense. (In other words, if mfaToken is provided, then roleArn must also be provided.)

Also, I don't think your code will work as written. The type of provider is AWSCredentialsProvider, which cannot be casted to AmazonWebServicesCredentials. Unfortunately, this plugin is still using the v1 Java SDK, and its STSAssumeRoleSessionCredentialsProvider.Builder does not have any support for MFA tokens. So either you will need to rewrite the assumeRoleProvider implementation, or upgrade to the v2 Java SDK where StsAssumeRoleCredentialsProvider.Builder has such support.

[weaversam8]

Personally, I think allowing people to specify the role as part of the actual credentials themselves was a mistake

I'm not sure I agree. The user running a job may have assume different roles based on their access level within an organization, but those different roles may provide equivalent permissions to perform the actions the job requires. This scenario is especially likely if these AWS credentials are user credentials in Jenkins.

Regardless of whether we agree on that point, unless you think it's also in-scope for this PR to remove the ability to specify a role as part of the credentials, you're right in that we need to clarify this parameter (and potentially create an additional parameter, if the original role assumption requires MFA.)

Suggestion: split the parameter into two params credentialMfaToken and roleMfaToken, the former for roles within the credential itself, and the latter for roles specified with the roleArn param in code.

Also, I don't think your code will work as written.

I've validated that this code works by compiling from source and loading this plugin on a Jenkins instance for testing. The configuration I'm using is:

• AWS credentials for my IAM user stored as a user credential in jenkins
• IAM policy strictly limits actions possible without MFA
• IAM policy strictly limits actions possible without role assumption (which requires MFA)
• Role ARN and MFA serial specified in the credential itself
• Pipeline configured with example code provided in the PR above.

The type of provider is AWSCredentialsProvider, which cannot be casted to AmazonWebServicesCredentials.

AWSCredentialsProvider is a superclass of AmazonWebServicesCredentials. In my testing, it casts just fine, since the underlying object in the provider variable is an instance of AmazonWebServicesCredentials, simply stored within a variable of type AWSCredentialsProvider. As I mentioned in the original PR though, I'm unclear on scenarios when this wouldn't be the case, so definitely looking for guidance there.

[rittneje]

This scenario is especially likely if these AWS credentials are user credentials in Jenkins.

In this situation you can have entirely separate IAM users. The role itself is not really a secret. In fact, specifying it as part of the secret in Jenkins means that refreshing the credentials cannot work, if the closure in question takes significant time. (This is also true if you specify roleArn as a parameter, which is why we only do that when we know the closure will complete fairly quickly.)

unless you think it's also in-scope for this PR to remove the ability to specify a role as part of the credentials

I am not suggesting removing that functionality. From your issue description it already doesn't work properly with MFA tokens, so continuing to not support that wouldn't really be a change.

As I mentioned in the original PR though, I'm unclear on scenarios when this wouldn't be the case, so definitely looking for guidance there.

If you pass roleArn as a parameter in code, it will be of type STSAssumeRoleSessionCredentialsProvider.

aws-credentials-plugin/src/main/java/com/cloudbees/jenkins/plugins/awscredentials/AmazonWebServicesCredentialsBinding.java

Lines 137 to 152 in c748e81

	private AWSSessionCredentialsProvider assumeRoleProvider(AWSCredentialsProvider baseProvider) {
	AWSSecurityTokenService stsClient = AWSCredentialsImpl.buildStsClient(baseProvider);
	String roleSessionName = StringUtils.defaultIfBlank(this.roleSessionName, "Jenkins");
	STSAssumeRoleSessionCredentialsProvider.Builder assumeRoleProviderBuilder =
	new STSAssumeRoleSessionCredentialsProvider.Builder(this.roleArn, roleSessionName)
	.withStsClient(stsClient);
	if (this.roleSessionDurationSeconds > 0) {
	assumeRoleProviderBuilder = assumeRoleProviderBuilder
	.withRoleSessionDurationSeconds(this.roleSessionDurationSeconds);
	}
	return assumeRoleProviderBuilder.build();
	}

[weaversam8]

In this situation you can have entirely separate IAM users. The role itself is not really a secret.

Ah, sorry. To clarify: I'm not arguing that the role itself is a secret and should be treated as such. My point is that some users workflows (involving entirely separate IAM users) may mean that two different users can't assume the same role. Imagine a job required the ListBuckets permission in S3. If a user is configuring their own IAM user, and their IAM user doesn't grant them that permission, but they have the permission to AssumeRole an IAM role that does have that permission, I think we should enable them to configure a user credential that automatically assumes that role without having to modify the job itself.

If we require the role assumption to be provided in the job itself and only in the job itself, in the scenario where two users both want to run a job, if the role they need to adopt is different, you'd have to supply it as an additional job parameter, which is undesirable.

In contrast, if the job knows it needs a particular role (instead of needing a particular set of permissions) then I think that's the situation where you'd want to supply a roleArn. I think both approaches have merits, and solve two related (but slightly different) problems.

I am not suggesting removing that functionality. From your issue description it already doesn't work properly with MFA tokens, so continuing to not support that wouldn't really be a change.

Agreed, it wouldn't be a change, however my primary motivation for submitting this PR was to support adopting the role specified in the credential (which fits my scenario for using this plugin.) You pointed out (rightfully) that I didn't consider roleArn, which I'd like to resolve, but I'd prefer to support both approaches rather than selecting one over the other. Does the suggestion to split the parameter create problems?

If you pass roleArn as a parameter in code, it will be of type STSAssumeRoleSessionCredentialsProvider.

Ah, thank you. I had missed this. I might rewrite the assumeRoleProvider implementation to use the code already in the package to assume a role with an MFA token, from AWSCredentialsImpl.getCredentials(String mfaToken), which returns a type AWSCredentials (which is a subclass of AWSProvider, satisfying the contract of assumeRoleProvider.)

Is there anything that the STSAssumeRoleSessionCredentialsProvider provider offer that AWSCredentials would not? Since we're using the AWSProvider class already, I would guess not, but I wanted to check if you saw any issues with that approach.