simple_oauth
simple_oauth copied to clipboard
laserlemon
Unionizing attribute keys to add easy support for implementations that require supplemental arguments
Attribute keys are now the union of the keys we must have, and the keys we provided, so that extra keys in the oauth payload are not silently omitted.
In particular, the Twitter API expects a custom Authorization header in at least some of the calls.
This is somewhat similar to Pull Req #6, but they achieve slightly different ends. One favors convention over configuration for a few more of the knobs, the other (this pull) is a little more configuration over convention.
@azanar Thank you! :clap: One thing to consider. The ATTRIBUTE_KEYS constant was used to constrain which options are eligible for inclusion in the header. Now that we're effectively saying that we'll put any/all options into the header, I don't think that ATTRIBUTE_KEYS is required any longer.
But I do fear that as it is, this will also include secret values in the header such as the oauth_consumer_secret.
The pull request also needs tests and unfortunately, makes the current tests fail hard. :-1:
If those concerns are addressed and tested, I think this is a valuable change. Thoughts, @sferik?
@laserlemon So I did have a reason for keeping the ATTRIBUTE_KEYS constant around. :-) The idea is that an OAuth header needs a bare minimum of a certain set of attributes to be valid. The constant, afaik, is a representation of all of those required keys. But maybe there's a better way.
As far as the tests failing, yeah, I'll go fix those.
For now, I'm going to merge #6, since I believe it is a net improvement over the existing code.
@azanar, when you get around to reworking this pull request to add tests and make the existing tests pass, we'll take another look at it.