germanzero / oauth-php

Automatically exported from code.google.com/p/oauth-php
MIT License
0 stars 0 forks source link

POST data included in signature when content type is "multipart/form-data" #141

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Create a post with the content type/encoding "multipart/form-data" with at 
least one posted variable
2. Echo the signatureBaseString in the OAuthRequest Library

What is the expected output?
You will see the variable included the signatureBaseString

What do you see instead?
Variables passed via "multipart/form-data", should not be included in the 
signatureBaseString.

Reference to the specification: Section 9.1.1. Normalize Request Parameters of 
OAuth 1.0a http://oauth.net/core/1.0a/#anchor13

Currently in the code there is a TODO marker for correct handling of the 
situation of "multipart/form-data", which does fetch the POST data.

Attached is a minor patch which no longer retrieves the POST data with a 
Comment so that it is clear that this behaviour is intentional.

Original issue reported on code.google.com by mat...@gmail.com on 25 Nov 2013 at 3:02

Attachments:

GoogleCodeExporter commented 8 years ago
I see that that behavior was changed at some point, in the code there is a 
comment:

>  Get the body of a POST with multipart/form-data by Edison tsai on 16:52 
2010/09/16

The original code was using a separate signature for the request body, per 
discussion with Eran et al.
See also https://groups.google.com/forum/#!topic/oauth/rNA2R0ZJQCo
And http://www.marcworrell.com/article-3130-en.html

Maybe you can check the reasons behind Edison Tsai's commit?

Original comment by ma...@pobox.com on 25 Nov 2013 at 3:36

GoogleCodeExporter commented 8 years ago
Thanks for finding the previous discussions: 
https://groups.google.com/forum/#!topic/oauth/rNA2R0ZJQCo
And http://www.marcworrell.com/article-3130-en.html

However they seem to be about allowing extended signing of body content above 
and beyond oauth standard signing. They were implemented in a separate class 
OAuthRequestVerifier.php in the verifyExtended method. These discussions were 
had in April 2008. The check-in from Edison Tsai seems to be unrelated to the 
extended verification.

The check-in looks to be a helpful change from what was there, which just throw 
an exception. There doesn't seem to be a lot of information about the Issue 81 
or commit 165. However it does not conform to the specs.

Would you be happy with some sort of flag that would make the behaviour act as 
it does currently and then apply the change?

Original comment by mat...@gmail.com on 25 Nov 2013 at 10:29

GoogleCodeExporter commented 8 years ago
Sounds good to me. Maybe the best default behavior is to stick with the specs, 
and an option to switch to Tsai's additions.

Original comment by ma...@pobox.com on 26 Nov 2013 at 8:44

GoogleCodeExporter commented 8 years ago
Finally found time to create the updated patch.

The flag by default is in-line with the spec.

The backwards compatibility maybe enabled by passing to the OAuthServer options 
parameter:
array (
'sign_body_of_multipart_request' => true
)

Looking at the code this was the cleanest way I could see that fits with the 
coding style/conventions.

Hope this is acceptable

Original comment by mat...@gmail.com on 3 Dec 2013 at 3:54

Attachments: