Skip to content
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

aws temporary credentials doesn't use the default credentials provider chain #1064

Closed
ghost opened this issue Jul 18, 2016 · 16 comments
Closed
Assignees
Labels
feature-request A feature should be added or improved.

Comments

@ghost
Copy link

ghost commented Jul 18, 2016

We would like to be able to use temporary credentials with master credentials pulling from the default credentials provider chain. Currently as far as we can the only way to do this is to explicitly set the global AWS.config.credentials field with a credential provider on startup asynchronously which opens the risk of the credentials not being set when the temporary credentials are called.

Ideally we would like to be able to set the credentials explicitly when creating the temporary credentials and have it fallback to using the default credentials provider if the global credentials are not set.

@chrisradek chrisradek self-assigned this Jul 18, 2016
@chrisradek
Copy link
Contributor

@nickwhiggins
Just to make sure I understand, you want to use TemporaryCredentials with the master credentials defined by the default credentials provider? When would you expect it to use the default credentials provider as a fallback? I'm unsure what you mean by 'global credentials' in this context.

@ghost
Copy link
Author

ghost commented Jul 18, 2016

@chrisradek
Yes, we want to use the TemporaryCredentials with the master credentials provided by the default credentials provider. The aim was not to have the credentials provider as a fallback but as the means to generate the temporary credentials. (e.g. using ec2metadata credentials to assume a different role). By global credentials I meant AWS.config.credentials which appear to be the only credentials that the temporary credentials pulls from for its master creds.

@chrisradek
Copy link
Contributor

@nickwhiggins
I think the below code may accomplish what you want. It instantiates a new CredentialProviderChain with no credential provicers in the constructor, so it uses the default providers. Calling resolve will attempt to go down the chain until it finds credentials (in my case, this was EC2MetadataCredentials). In the callback provided to resolve, it then updates the 'global' config master credentials to what was resolved in the chain. Then, it creates TemporaryCredentials. When you call an operation after the TemporaryCredentials are set, those are what will be used.

var AWS = require('aws-sdk');
// Uses default providers if none are passed to the constructor
var credentialProviderChain = new AWS.CredentialProviderChain();
credentialProviderChain.resolve(function(err, creds) {
  if (err) {
    console.log('No master credentials available');
  } else {
    console.log('Master credentials available');
    // Set master credentials
    AWS.config.update({
      credentials: creds
    });
    // create temporary credentials
    AWS.config.update({
      credentials:  new AWS.TemporaryCredentials(/* params */)
    });
  }
});

We could potentially update the TemporaryCredentials to use the default provider chain if no global credentials are found. We may need to make that an opt-in feature since making changes to default behavior of credential providers is almost certain to break someone. Will the above example work for your use-case? One down-side I see with the above approach is that it runs asynchronously, so you'd need to execute your code after the above has finished running.

@ash2k
Copy link

ash2k commented Jul 27, 2016

It works for us but ideally we think it should:

  1. Look for explicit credentials parameter passed into constructor;
  2. Look for global credentials in AWS.config.credentials;
  3. Use AWS.config.credentialProviderChain as the last resort.

Adding 1 and 3 should not break current behaviour.

@mifi
Copy link

mifi commented Aug 1, 2016

Yes, this is really confusing behaviour imo. It works with some of the default credential providers, but not all. For me it worked when testing locally (which used SharedIniFileCredentials), but when deployed on an EC2 instance my program just crashed with a "TypeError: Cannot read property 'masterCredentials' of null".

If not changed, at least it should be documented in http://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/TemporaryCredentials.html that you have to do it like in @chrisradek's code sample if you need async provider support.

@chrisradek
Copy link
Contributor

@ash2k

  1. I like the idea of being able to specify an explicit credentials parameter for TemporaryCredentials to make use of.
  2. It currently takes its master credentials from AWS.config.credentials, so point 2 isn't an issue. The point @mifi makes is relevant here, because if the credentials in AWS.config.credentials haven't been resolved yet and resolve asynchronously, you could wind up getting an error like he saw. That's something we should be able to account for.
  3. This is probably safe to do, but we'll want to tackle this one very carefully since it would be new behavior and history has shown us that users will use credential providers in unanticipated ways. The safest thing would still be to stick this behavior behind a flag but we might not have to do that if we can be confident this won't impact existing applications.

@mifi
I think we'll need to address the issue in your comment first, and then look into implementing points 1 and 3 from above.

@addisonj
Copy link

Any update this on when this might be implemented?

It is pretty surprising behavior, since with most other places where credentialProvider are used (say in service clients) the consumer takes care of resolving so the API appears "sync" to the end user. I have also done this same pattern of creating a service client with a specific TemporaryCredentials in other language aws-sdks, where it works without having to worry about the masterCredentials, so its doubly surprising :)

@tjunnone
Copy link

tjunnone commented Feb 27, 2017

Ran into this problem today, the EC2 instance has access to STS AssumeRole via a role itself and AWS.TemporaryCredentials fails with the "Cannot read property 'masterCredentials' of null" error.

For others that have this problem, the SDK now (as of 2.7.28) has a second parameter to the TemporaryCredentials constructor that takes the master credentials, so you can fix it yourself. Use the CredentialProviderChain as per above, or even EC2MetadataCredentials explicitly to acquire some.

BUT, it's not enough. The credentials provided for the role via the metadata service are temporary themselves, so now you need to manage the refreshing of the master credentials yourself if you run a server, or get them on every request to AssumeRole. Pain in the behind and error prone. Everything else in the SDK works out-of-the-box with roles, so it's quite inconsistent from a developer point of view. +1 to just using the default CredentialProviderChain here, so that it simply works the same way whether using an ini file or an EC2 role transparently.

@DerekOverlock
Copy link

Any updates on this? Ran into this exact issue today, would be great if TemporaryCredentials was handling the default provider credential chain in a consistent manner w.r.t the other SDK services.

@jthomerson
Copy link

Another "me too". I need to specify a profile that gets used to load the credentials that the temporary credentials provider can use to assume a role. For example:

var params = {
   Credentials: new AWS.SharedIniFileCredentials({ profile: argv.profile }),
   RoleArn: argv['role-arn'],
   SerialNumber: argv['mfa-serial'],
   TokenCode: argv['mfa-token'],
};
AWS.config.credentials = new AWS.TemporaryCredentials(params);

Then all my connections to AWS resources should use the temporary credentials provided by the assumed role, except for the calls to STS itself, which would use the credentials provided by the specified profile in my ~/.aws/credentials file (from the SharedIniFileCredentials provider).

@jthomerson
Copy link

Okay, an update: I actually was able to do this fairly easily. There was a problem - which I'll explain below. First, what I'm doing now (note that this is in a script intended to be run from the CLI).

// set my master credentials to ones that read use the profile in the ~/.aws/credentials file
AWS.config.credentials = new AWS.SharedIniFileCredentials({ profile: argv.profile });

var params = {
   RoleArn: argv['role-arn'],
   SerialNumber: argv['mfa-serial'],
   TokenCode: argv['mfa-token'],
};

// now override the master credentials with temporary credentials that will use
// STS.assumeRole. This credentials object will use the one set above as its
// credentials for calling STS.assumeRole. It does this automatically.
AWS.config.credentials = new AWS.TemporaryCredentials(params);

startupPromise = Q.ninvoke(AWS.config.credentials, 'refresh');
}

startupPromise
   .then(function() {
      return runYourScript();
   })
   .done();

The problem seems to be that TemporaryCredentials refreshes its credentials lazily and asynchronously, with no lock. So what was happening was this: when the script started running, several different processes were being made simultaneously. The first one might succeed, but then others would fail because they were all also refreshing, using the one-time code that I had passed in.

By calling refresh and making sure it completes before I start the script, I ensure that only one operation is using the one-time code, and the credentials are loaded from STS correctly before the script even starts.

@jeskew
Copy link
Contributor

jeskew commented Aug 4, 2017

@jthomerson Would you mind copying that comment into a new issue? I don't believe it's directly related to the OP, but it is something we can likely address.

@jthomerson
Copy link

@jeskew reported as #1664. Thanks!

@davidrissato
Copy link

I had a similar problem and I was getting the error message TypeError: Cannot read property 'masterCredentials' of null while I was trying to use sts.assumeRole from an EC2 that was using instance profiles.

I could get rid of that strange error message doing this way:

const AWS = require('aws-sdk');
const Promise = require('bluebird');

const getCrossAccountTemporaryCredentials = Promise.coroutine(function* (accountNumber, roleName, roleSessionName) {
    const masterCredentials = yield AWS.config.credentialProvider.resolvePromise();
    const sts = new AWS.STS();
    const assumeRoleParams = {
        DurationSeconds: 3600,
        RoleArn: `arn:aws:iam::${accountNumber}:role/${roleName}`,
        RoleSessionName: roleSessionName
    };
    const assumeRoleRequestData = yield sts.assumeRole(assumeRoleParams).promise();
    return sts.credentialsFrom(assumeRoleRequestData, masterCredentials);
});

@jstewmon
Copy link
Contributor

This should be resolved in the next release by #2175 using ChainableTemporaryCredentials.

@srchase
Copy link
Contributor

srchase commented Dec 25, 2018

Closing out this issue.

@srchase srchase closed this as completed Dec 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests