search

aheckmann / node-ses

An Amazon SES api for nodejs with proper error handling.
http://aheckmann.github.com/node-ses
MIT License
201 stars 37 forks source link

Signing broken in `v3.0.0` #61

Closed epegzz closed 4 years ago

epegzz commented 4 years ago

Setting json: true in v3.0.0 brakes signing:

body:
   { Error:
      { Code: 'SignatureDoesNotMatch',
        Message:
         'The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details.',
        Type: 'Sender' },
     RequestId: 'ce056628-f806-48a7-b7a5-f9b47c75d063' } }
markstos commented 4 years ago

@epegzz Did you search for the other issues about SignatureDoesNotMatch before opening this one?

https://github.com/aheckmann/node-ses/issues?q=SignatureDoesNotMatch

I'll presume this issue is a duplicate of one of those. If you've got more information to explain how it's different, I'll re-open this.

epegzz commented 4 years ago

@markstos This issue was introduced when switching from xml to json. I can fix it by removing the line 'json: true'. Therefore it is not related to any of the other issues.

markstos commented 4 years ago

Perhaps the JSON change should be reverted, but on the other hand, AWS definitely supports JSON. Maybe we should switch from usng aws4 to using the aws-sdk for signature handling.

@jamonkko contributed the code to switch to JSON but I don't know if he's around any more. Unless someone else wants to take another approach, I'll plan to revert the switch to JSON.

epegzz commented 4 years ago

My assumption is, that the added Accept='application/json' header is causing the issue with signing. Haven't been able to figure out more yet though.

jamonkko commented 4 years ago

Unfortunately I have moved to other projects and even the work that was using this module with the json format does not exists anymore so I have no need for the json support. So if the json changes broke other use cases please go ahead and revert the change. If there is need for json format in the future I’m pretty sure someone will add a working one with new PR.

MaheshCasiraghi commented 4 years ago

Thank you @epegzz for the effort. Indeed this last version breaks signing. @markstos is there a plan to rollback to a xml based release in the next few days?

epegzz commented 4 years ago

@markstos Yes, let's revert the change from xml to json 👍

markstos commented 4 years ago

I'll do that today.

On Wed, Apr 29, 2020 at 4:09 AM Daniel Schäfer notifications@github.com wrote:

@markstos https://github.com/markstos Yes, let's revert the change from xml to json 👍

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aheckmann/node-ses/issues/61#issuecomment-621053918, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAGJZMT62TJV6I2DDPGISDRO7ODXANCNFSM4MSZFJDQ .

-- Mark Stosberg Director of Systems and Security RideAmigos