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

Add support for AWS_SESSION_TOKEN temporary credentials #3

Merged
merged 1 commit into from Dec 10, 2019

Conversation

hayd
Copy link
Contributor

@hayd hayd commented Dec 4, 2019

Fixes #1. As mentioned this is required for AWS Lambda support.

Note: There might be a better way to include this, specifically if env access should not be required we could try to do this once and if there's a AWS_SESSION_TOKEN only then refresh it each request (otherwise do not try/add the header).

cc denoland/deno-lambda#14

@hayd
Copy link
Contributor Author

hayd commented Dec 8, 2019

@chiefbiiko an alternative (without env permissions being required): hayd@3517903

It's not clear how often this token can rotate (whether it needs to be looked up each request).

What do you think?

@chiefbiiko
Copy link
Owner

@hayd thanks for your help! and sorry for the long wait - holidays in 🇬🇭
I would prefer not using Deno specific things and not have any implicit logic like env var lookups inside the module - specifically I would like to just have a sessionToken prop on ClientConfig and the extending HeadersConfig objects. Users will have to pass that sessionToken explicitly when calling createClient.
If for rotating reasons this won’t fit I’m open to doing the env var lookup or similar. What u think?

@hayd
Copy link
Contributor Author

hayd commented Dec 9, 2019

No problem! Hope you had a good Farmers' Day.

I think that's fine... My concern is that I'm not sure what the rules are with regards to lifetimes.

How's about this compromise: an optional sessionToken: async? () => string passed to config/client? Which, if provided, is called prior to each request.

@hayd
Copy link
Contributor Author

hayd commented Dec 9, 2019

^See commit above.

@chiefbiiko
Copy link
Owner

That one is neat. Could u change the naming from securityToken to sessionToken - think it makes the temp nature of that token more obvious.

@hayd
Copy link
Contributor Author

hayd commented Dec 9, 2019

Agreed - done. Sorry for confusion, aws uses these terms seemingly interchangeably!

@chiefbiiko
Copy link
Owner

LGTM - Thanks for your work!

@chiefbiiko chiefbiiko merged commit a12094c into chiefbiiko:master Dec 10, 2019
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.

Support AWS_SESSION_TOKEN
2 participants