jcberquist / aws-cfml

Lucee/ColdFusion library for interacting with AWS API's
MIT License
72 stars 53 forks source link

Pass token through to credentials in order to pass instance metadata for IAM roles directly #86

Open jlopes1030 opened 4 months ago

jcberquist commented 4 months ago

Hi, thanks for the pull request. I see that you want to be able to pass in a session token directly as part of the AWS credentials. That seems reasonable. Can you explain the addition to the resolveCredentials() method, where you added the initial IAM role check at the top? It duplicates the check further on, so why is it necessary to have it there as well?

jlopes1030 commented 4 months ago

I placed the additional check at the top because it will allow it to trigger only if you manually the token and you will not get down to the other IAMroles check if you pass a aws key and aws secret key it will return the credential on what was the first if statement. Then it will refresh the credentials and then the above mentioned if statement will return the credentials now that you have the proper keys and token.

jcberquist commented 4 months ago

If you want to use the IAM role credentials via the instance metadata, why pass in credentials at all? I am not sure I understand that part.

jlopes1030 commented 4 months ago

It’s mainly because I am calling many different roles and bucket so I use cfhttp to get the metadata then pass that the keys and token through.

jcberquist commented 4 months ago

The way this is structured now, if a token is passed in to resolveCredentials(), then the code you added below will execute:

if(len(token)){
 try {
    variables.iamRole = requestIamRole();
    if ( iamRole.len() ) {
        variables.credentialPath = iamRolePath & iamRole;
        refreshCredentials( credentials );
    }
} catch ( any e ) {
    // pass
}

If an IAM role is available in the instance metadata (assuming this is running on an EC2 instance), then the passed in credentials will be overwritten by the refreshCredentials() call. And if an IAM role is not available in the EC2 instance metadata, then this code has no effect. So I don't think it belongs here.

If you are getting the credentials from elsewhere (e.g. via cfhttp), then I think you just want to use the credentials you obtained, and not do any of the credential resolution logic.

I would think that what you actually need is to change the following:

var credentials = defaultCredentials( awsKey, awsSecretKey );

to

var credentials = defaultCredentials( awsKey, awsSecretKey, token );

so that the token you pass in is added to the credentials struct, and then that struct is immediately returned from the resolveCredentials() struct, since it is not empty, here:

if ( len( credentials.awsKey ) && len( credentials.awsSecretKey ) ) {
    return credentials;
}
jlopes1030 commented 4 months ago

I honestly did not even think to try passing the token into the defaultCredentials from within the resolveCredentials function. I will need to give that a try, it may eliminate the need for that if(len(token)) conditional.