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

Fix signing other http methods than GET #8

Merged
merged 3 commits into from
Dec 14, 2020
Merged

Fix signing other http methods than GET #8

merged 3 commits into from
Dec 14, 2020

Conversation

bruno-xo7
Copy link
Contributor

No description provided.

@SamVerschueren
Copy link
Owner

Hi @bruno-xo7

Thanks for creating this issue. It doesn't look like adding the method to the request before signing changes something to the signature. The tests still succeed. So is it possible that the issue you hit was more about the query params not present?

@bruno-xo7
Copy link
Contributor Author

Hello @SamVerschueren ,

The HTTP method seems to be important for the AWS signature V4 process, as stated in the official documentation: https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html

Your test succeed because you get the default GET method if you don't assign the method to the request:

@SamVerschueren
Copy link
Owner

Ah yes, you are right, thanks for clarifying 🙏 .

Would it be able to add an extra test for a POST method for instance?

@bruno-xo7
Copy link
Contributor Author

bruno-xo7 commented Dec 14, 2020

Would it be able to add an extra test for a POST method for instance?

Done. And also one for PATCH method

Could you please publish a new version?

@SamVerschueren SamVerschueren changed the title fix sign of the call with search parameters and with a different method than 'GET' Fix signing other http methods than GET Dec 14, 2020
@SamVerschueren SamVerschueren merged commit cc43799 into SamVerschueren:master Dec 14, 2020
@SamVerschueren
Copy link
Owner

Released! Thanks for following up on this one!

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.

2 participants