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

Support uri parameter #28

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

Support uri parameter #28

wants to merge 2 commits into from

Conversation

pahud
Copy link

@pahud pahud commented Jul 9, 2016

  1. initial support for AWS API Gateway
  2. Custom doamin name support with API Gateway
  3. when request.uri is provided, npm modules like request and request-promise are also supported
  4. examples provided

… domain name

2. when request.uri is provided, generate all required parameters and support request module as well
@Dayjo
Copy link

Dayjo commented Aug 5, 2016

Would be great to merge in if this is finished!

@mhart
Copy link
Owner

mhart commented Aug 5, 2016

@Dayjo the title was misleading – API Gateway has been supported by aws4 since it launched – it just expects you to pass in a host and path as per the Node.js http client. This PR is just a convenience method for parsing those from a URI.

uri is not recognised by the Node.js http client, which is why it's not included at the moment – but I guess it makes sense to add this as a convenience with some tests (possibly url as well)

In any case, you should be able to use API Gateway without needing this PR in

@mhart mhart changed the title add API Gateway support Support uri parameter Aug 5, 2016
@Dayjo
Copy link

Dayjo commented Aug 8, 2016

Hi @mhart thanks,

I kind of hoped / assumed the reason my signature is 100% always erroring was because of this haha. Damn.

Back to the drawing board.

@mhart
Copy link
Owner

mhart commented Aug 8, 2016

@Dayjo what does your code look like?

@Dayjo
Copy link

Dayjo commented Aug 8, 2016

Well it's been butchered several times to try and get it working. I'm trying to do signed requests across AJAX for a JS app. I was trying to manually just use the headers created by aws4 with my ajax request, but to no avail. for instance;

var d = new Date();
d = new Date(d.getUTCFullYear(), d.getUTCMonth(), d.getUTCDate(), d.getUTCHours(), d.getUTCMinutes(), d.getUTCSeconds());

CONFIG.dateStamp = d.yyyymmdd();
CONFIG.dateTimeStamp = d.yyyymmddhis();

var test = aws4.sign({
   uri: url,
   service: 'execute-api',
   region: CONFIG.REGION
},
{
    accessKeyId:     CONFIG.ACCESS_KEY,
    secretAccessKey: CONFIG.SECRET_KEY
});

//'AWS4-HMAC-SHA256 Credential=' + CONFIG.ACCESS_KEY + '/' + CONFIG.dateStamp + '/' + CONFIG.REGION + '/execute-api/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date;x-amz-security-token, Signature=' + CONFIG.SIGNED_KEY,
var headers = {
    'Authorization': test.headers['Authorization'],
    'X-Amz-Date': CONFIG.dateTimeStamp,
    'X-Amz-Security-Token': CONFIG.SECURITY_TOKEN
};

// Make the request
$.ajax({
    type: type,
    url: url,
    headers: headers
});


// ----- Date functions for date/time 'stamps'
Date.prototype.yyyymmdd = function() {
      var mm = this.getMonth() + 1; // getMonth() is zero-based
      var dd = this.getDate();

      return [this.getFullYear(), !mm[1] && '0', mm, !dd[1] && '0', dd].join(''); // padding
    };
    Date.prototype.yyyymmddhis = function() {
        var d = this.yyyymmdd();
        var time = this.toTimeString();
        var ts = time.substr(0,8).replace(/:/g,"");
      return d + 'T' + ts + 'Z';
    };

Any suggestions, or if I'm doing something utterly stupid it'd be great to know haha.

@mhart
Copy link
Owner

mhart commented Aug 8, 2016

@Dayjo - yeah, I see a bunch of things.

Firstly, the uri is not supported (already covered this obvs) – use host and path instead.

Secondly, you shouldn't modify the headers after the signing – you should just pull the headers out exactly as they are in the signed request. I'm not sure what all the date stuff is (aws4 handles this for you)

So try something like this:

var opts = aws4.sign({
  host: 'my-execute-api-host.com',
  path: '/some/path',
  service: 'execute-api',
  region: CONFIG.REGION,
}, {
  accessKeyId: CONFIG.ACCESS_KEY,
  secretAccessKey: CONFIG.SECRET_KEY,
  sessionToken: CONFIG.SECURITY_TOKEN,
})

return $.ajax({
  url: 'https://' + opts.host + opts.path,
  headers: opts.headers,
})

(I'm assuming you're doing GETs – if you're doing POSTs, then you should pass a body in to the object that aws4 signs)

@mhart
Copy link
Owner

mhart commented Aug 8, 2016

(btw, unless you're specifically wanting to include jQuery for some other reason, I wouldn't use it just for making AJAX requests – use something like xhr or got instead)

@Dayjo
Copy link

Dayjo commented Aug 8, 2016

Haha yeah, unfortunately the rest of the app is using it anyway, thanks for the info, will give it a shot!

@Dayjo
Copy link

Dayjo commented Aug 9, 2016

Edit: See comment below for fix.

@mhart unfortunately I'm now getting a javascript error from within the aws4 module :(.

I get Uncaught TypeError: h.escape is not a function(…) - h being minified form of querystring.

image

I believe this is because I may incorrectly configured the 'browser' version. Do you think you could give me a little run down of the process for building this for the browser? I've read https://github.com/mhart/aws4/tree/master/browser but still not 100% sure what I actually need to do. I'm already using browsify to load in the aws4 module, but not sure what the build process is for or if it's necessary.

If you like I can start up a new issue with this info in?

@Dayjo
Copy link

Dayjo commented Aug 9, 2016

So I managed to fix my above js errors by using; https://github.com/mathiasvr/querystring though i've not yet been able to get aliasify working to actually alias it, had to fork and created a browser branch to pull in that version instead (as per https://github.com/pallant/aws4/tree/browser)

And now it appears to be working ok, need to do more tests but my requests seem to be signed ok :)

Thanks a bunch @mhart - might be good to add the query string fix to the browser readme file :)

@mhart
Copy link
Owner

mhart commented Aug 9, 2016

@Dayjo I'm really not sure what you mean – the example in the browser README works perfectly for me.

I do:

npm install
npm run build
open index.html

And it all works fine.

Can you explain what problems you ran into with it?

@Dayjo
Copy link

Dayjo commented Aug 9, 2016

@mhart I think I'm just unfamiliar with build scripts and such in npm 😄 - we mostly use gulp - this did work for me, thanks!

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