search

alexa / alexa-skills-kit-sdk-for-nodejs

The Alexa Skills Kit SDK for Node.js helps you get a skill up and running quickly, letting you focus on skill logic instead of boilerplate code.
Apache License 2.0
3.12k stars 737 forks source link

[Skill Request Verify method] bug in retrieving header fields #604

Closed DavidePezzolla closed 4 years ago

DavidePezzolla commented 4 years ago

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ X] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Other... Please describe:

I am trying to expose an Alexa Skill through an API Gateway, hence I am relying on this library to validate the request I receive. I was not able to perform successfully the validation and after a bit of troubleshooting and code analysis, I found out that in the code of the verify method of the library you lowercase the 'Signature' and the 'SignatureCertChainUrl' string. In this way the code is not able to retrieve those fields inside the headers object and fail. I have temporary fixed the issue by adding the 'signature' and 'signaturecertchainurl' fields in the header, is there a reason for that toLowerCase? I can open a pull request to fix it if needed.

public async verify (requestEnvelope : string, headers : IncomingHttpHeaders) : Promise { // throw error if signature or signatureCertChainUrl are not present const signatureCertChainUrl : string = headers[SIGNATURE_CERT_CHAIN_URL_HEADER.toLowerCase()] as string; const signature : string = headers[SIGNATURE_HEADER.toLowerCase()] as string; if (!signatureCertChainUrl) { throw createAskSdkError( this.constructor.name, 'Missing Certificate for the skill request', ); } if (!signature) { throw createAskSdkError( this.constructor.name, 'Missing Signature for the skill request', ); }

Expected Behavior

The verify method should be able to retrieve the 'Signature' and 'SignatureCertChainUrl' fields from the request.

Current Behavior

The verify method is not able to retrieve the 'Signature' and 'SignatureCertChainUrl' fields from the request, the code is looking for 'signature' and 'signaturecertchainurl' fields (lowercase)

Possible Solution

remove the .toLowerCase method :D

Thank you!

ShenChen93 commented 4 years ago

Hi @DavidePezzolla

Thanks for reporting this issue. I have created a PR to fix it and will do a patch release when it's merged in.

Thanks, Shen

ShenChen93 commented 4 years ago

Hi @DavidePezzolla ,

We just merged in the PR #605 , and released a patch version v2.0.1. You could update the package to the latest version to solve your issues. Close this issue for now and please feel free to follow up if you have any questions.

Thanks, Shen