botman / driver-amazon-alexa

BotMan Amazon Alexa Driver
MIT License
14 stars 11 forks source link

Driver does not verify Alexa request #11

Open rosswintle opened 4 years ago

rosswintle commented 4 years ago

Not sure how active this repo is. I'd love to be able to work with someone to get this fixed and may even PR it if I can.

I've been building an Alexa skill with Botman and this driver.

The skill is failing Amazon's automated tests because it wasn't checking the request signature.

To publish a Skill it looks like you have to:

I've had a quick look through the source for this driver and I can't see any code that does any of this.

Can I ask:

1) Does anyone think that the driver should be doing this? Am I just missing something? Or is the code missing something? 2) If the code is missing something, can someone suggest where I should add these checks? I'm thinking it probably goes in AmazonAlexaDriver->matchesRequest() does that seem right? Or does BotMan have some other place I should put this kind of "middleware" check?

Thanks for any help.

rosswintle commented 4 years ago

Been looking into this. Just adding some notes from my diving in.

The BotMan package makes use of https://github.com/MiniCodeMonkey/amazon-alexa-php which seems to do the timestamp verification at least (though it may be broken due to Amazon changes).

There are issues raised on that package for the verification problems I've stated, and even fixes proposed.

So the easiest thing here is probably to just use one of the updated forks of MiniCodeMonkey's package.

rosswintle commented 4 years ago

I started looking into using Froodley's fork of amazon-alexa-php which has had a lot of rework.

I have the basics of text-based responses working in a fork of the botman driver but I think some driver re-work is needed to make it compatible with the new alexa package

rosswintle commented 4 years ago

OK. Froodley's fork didn't work. I think there was a Symfony Validator versioning issue. It turns out there's loads of forks of MiniCodeMonkey's package. I managed to find this one and patch it in to my fork of the Botman Alexa Driver

This now passes Amazon's verification. Though I note that:

  1. I've probably not used the package properly. I'm using the library's validation on lines 49-52 of AmazonAlexaDriver.php, but throwing away the Request object that I create.
  2. I've added an AMAZON_APP_ID environment variable that needs to be set.
  3. There was an error when package discovery was happening (on composer update/install) - it was trying to instantiate the Alexa Request object without any data being passed. So I added code to check for an HTTP request being present. I'm sure there's a better way to handle this.
  4. I've not added any tests - it would be good to test that the driver's validation passes and fails when it should.
envatic commented 4 years ago

You did most of the heavy lifting already, Im grateful, will take a look to see how to fix this.

dottxado commented 3 years ago

Hello! I've just proposed a pull request about the validation :) #15