Added Request.sign() option to omit oauth_body_hash

[bensonk]

I'm using oauth2 to consume Twitter's streaming API. To make the library compatible with their API, the oauth_body_hash header must be omitted. To allow that, without breaking any existing functionality, I added a parameter defaulted to True called omit_body_hash. With this small change, my code to consume twitter's streaming API works.

[bensonk]

The folks at twitter would like me to point out that oauth_body_hash is a nonstandard addition to the protocol, and that when you're connecting to twitter it shouldn't ever be included. While the oauth2 library works with their REST API currently, it's possible they will be more strict in the future and it will cease to work. To maintain compatibility with twitter in the long run, it would be best to make omit_body_hash true by default.

[ghost]

I'd love to see this merged as soon as possible. Not having a way of controlling the body_hash results in new versions of oauth2 immediately breaking when used against Oauth providers that do not support this extension. I

[bensonk]

Sadly, I've heard simplegeo is defunct, so I'm not sure it will ever get merged. I guess somebody else needs to take up the torch and maintain this.

[groodt]

I just ran into the same issue. I've hacked it to work by setting is_form_encoded=True when I create my Request. Clearly a massive hack. I think this OAuth library needs a bit of an overhaul. There are a lot of open issues, which is a pity since it appears to be the best Python OAuth lib out there.

[bensonk]

I'd love to see somebody take it and run with it, but I don't know who would do it. I don't think I have time to maintain it full time, but I'll keep patching it as I see bugs that I know how to fix.

[domino14]

merge this

[domino14]

please

[joestump]

@rickhanlonii this seems like something that's relevant to your work in #138 as well?

[joestump]

@bensonk thanks for the PR! @jaitaiwan and @rickhanlonii are probably going to chime in on this as well. We need tests on this before we merge, but feel free to wait on those for now.

[rickhanlonii]

It looks like section 4.1.1 specifies when to include the oauth_body_hash:

4.1.1. When to Include the Body Hash

Not all requests should contain the oauth_body_hash parameter.

• OAuth Consumers SHOULD NOT include an oauth_body_hash parameter when making Request Token or Access Token OAuth requests.
• OAuth Consumers MUST NOT include an oauth_body_hash parameter on requests with form-encoded request bodies.
• OAuth Consumers SHOULD include the oauth_body_hash parameter on all other requests.

So I think this change in general is necessary according to 4.1.1.

The only question I have here is whether or not the default value should be True. It seems to me that it would be straightforward to pass omit_body_hash=True for only calls to Request Token or Access Token OAuth and default to False for all other requests.

In other words: I think the default value should match the requirements of "all other requests" rather than the two special defined cases. I understand from the above discussion that the requirements of "all other requests" to the twitter API would require omit_body_hash=True, but supporting a particular third party API should not be a motivating factor in the design at hand.

[rickhanlonii]

Also, I think it is more clear for the parameter to be defined positively as in include_body_hash=True, rather than negatively as in omit_body_hash=False since the latter requires parsing a double negative to understand that the body hash will be included.

[joestump]

@rickhanlonii I concur all around. @bensonk if you could add tests I'm :+1: after @jaitaiwan has reviewed.

[jaitaiwan]

@bensonk @joestump adding this to my long list of things to review.

[jaitaiwan]

LGTM after tests are added. @bensonk are you able to provide the tests for this?