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

feat(aws): allow service and region to be specified in aws options. #2741

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chasingmaxwell
Copy link

PR Checklist:

  • I have run npm test locally and all tests are passing.
  • I have added/updated tests for any new behavior.
  • If this is a significant change, an issue has already been created where the problem / solution was discussed: N/A (small change)

PR Description

This PR picks up where #2188 and #2339 left off to allow aws signing for API Gateway requests which require service and region as part of the signature. aws4 supports adding both service and region so this is a simple change.

Many thanks to @NewGyu who originally wrote this code which I've simply applied to the latest master!

@chasingmaxwell
Copy link
Author

apologies. I did run tests locally, but there appears to be a problem. I'm looking into it.

@chasingmaxwell
Copy link
Author

There we go!

Copy link

@elliotttf elliotttf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me overall 😄

@@ -1364,6 +1364,8 @@ Request.prototype.aws = function (opts, now) {
host: self.uri.host,
path: self.uri.path,
method: self.method,
service: opts.service,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it expected that custom domains would only be available for version 4 signing?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not positive what service and region might be used for other than API Gateway. API Gateway only supports version 4 though.

@cmbrehm
Copy link

cmbrehm commented Jan 23, 2018

👍 for merging this, working around it currently.

@johnz
Copy link

johnz commented Mar 14, 2018

When can we get this merged? we need 'region' to sign AWS API Gateway url with custom DNS

@crsepulv
Copy link

When this will be merged? I need to sign4 request to API Gateway, and the service param is needed to create the signature.

@andrhamm
Copy link

Please merge

@ffoysal
Copy link

ffoysal commented Apr 23, 2019

we need this feature. Please merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants