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

Enable BasicSessionCredentials, given token #49

Merged
merged 1 commit into from
Jan 13, 2014

Conversation

elliot42
Copy link
Contributor

AWS differentiates BasicAWSCredentials, vs. BasicSessionCredentials.
The later requires a session token, while the former does not.
Session credentials are required for features such as IAM, which
allows EC2 instances to retrieve temporary creds and access S3 without
having to hardcode a real user's creds.

This commit adds support for BasicSessionCredentials. Any call
that previously would have accepted a cred map:

{:access-key "...", :secret-key "..."}

will now also accept:

{:access-key "...", :secret-key "...", :token "..."}

and will authorize with BasicSessionCredentials if given a token.

AWS differentiates BasicAWSCredentials, vs. BasicSessionCredentials.
The later requires a session token, while the former does not.
Session credentials are required for features such as IAM, which
allows EC2 instances to retrieve temporary creds and access S3 without
having to hardcode a real user's creds.

This commit adds support for BasicSessionCredentials.  Any call
that previously would have accepted a cred map:

    {:access-key "...", :secret-key "..."}

will now also accept:

    {:access-key "...", :secret-key "...", :token "..."}

and will authorize with BasicSessionCredentials if given a token.
@elliot42
Copy link
Contributor Author

There have been a few attempts at this in the past--I'm not quite sure what happened to them / why they were not acceptable. I've tried to make this the smallest, simplest possible change. Please do let me know feedback if there's some way I can improve this so it's acceptable. Thank you!

@weavejester
Copy link
Owner

I think previous attempts either exposed the underlying Java classes, or possibly got overlooked.

This particular commit looks fine...

@sw1nn
Copy link
Collaborator

sw1nn commented Jan 13, 2014

This seems like the simplest solution to the various PRs, #21 #28 #40, I'm going to merge this and close the other requests.

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.

3 participants