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

Includes session token if provided to client initialization. #57

Conversation

RickCSong
Copy link
Contributor

If you're using Temporary Credentials with AWS STS, you must include a
session token.

Reference:
http://docs.aws.amazon.com/AWSSdkDocsJava/latest/DeveloperGuide/prog-services-sts.html

@ghedamat
Copy link
Contributor

ghedamat commented Jun 4, 2016

@RickCSong thanks for this

I think we should add this option to the index.js file and update the tests accordingly

(somewhere around here could be a good place)
https://github.com/ember-cli-deploy/ember-cli-deploy-s3/blob/master/index.js#L41

we should also update the README.md info as part of the same PR
https://github.com/ember-cli-deploy/ember-cli-deploy-s3#configuration-options

let me know if you need more direction!

thanks!

@ghedamat ghedamat self-assigned this Jun 4, 2016
If you're using Temporary Credentials with AWS STS, you must include a
session token.

Reference:
http://docs.aws.amazon.com/AWSSdkDocsJava/latest/DeveloperGuide/prog-services-sts.html
@RickCSong RickCSong force-pushed the ricksong/add-session-token-to-initialization branch from dc438c5 to 667ebef Compare June 8, 2016 06:01
@RickCSong
Copy link
Contributor Author

Hey @ghedamat! I've updated the README. However, I'm uncertain whether we should be updating anything in index.js? This seems like it belongs wherever we are passing/setting the Access Key ID / Secret Access Key (which currently occurs in lib/s3.js) Furthermore, if you pass in your own S3 client, things appear to work perfectly :).

I'm not sure if there are any tests around the Access Key ID stuff / Secret Access Key (but I've added the option already to the tests).

Would be happy to make any other changes! We've already tested this / successfully deployed to production on our end.

@ghedamat
Copy link
Contributor

ghedamat commented Jun 8, 2016

@RickCSong you are correct, we already don't mention explicitly the other S3 options so I guess this is fine :)

thanks for your work!

@ghedamat ghedamat merged commit 880c67b into ember-cli-deploy:master Jun 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants