request / oauth-sign Goto Github PK

Would you accept a PR that adds the SHA256 hashing algorithm?

It's not officially part of the specification, but the spec has the following to say regarding signature methods:

OAuth does not mandate a particular signature method, as each
implementation can have its own unique requirements.  Servers are
free to implement and document their own custom methods.

I understand that it's probably a very unique requirement on our part, I'm just curious if you would accept such changes upstream and if it would make it into the request module.

@mikeal @nylen

I just noticed that the oauth-sign tests are migrated to tape in test-oauth The first 6 tests are the same as the first 3 here + the rsa signing for each one, added in this PR request/request#1228

My idea is to move all unit tests specific to signing out of test-oauth and put them here. That way we can concentrate on tests specific to the oauth implementation in request there. The other thing is that we'll have all tests migrated to tape here and that will allow us to enable coverage tool for example.

Let me know what do you think.

Can we get a new version on npm with #7 incorporated? If so, we can then close out request/request#1042.

Clearly this API is not changing. 1.0 please.

Whenever a querystring param contains a tilde, it will be encoded as '%7E' in the base string due to the use of 'escape()'. Since the RFC 3986 spec considers tildes as an unreserved character, it is not encoded in the base string the particular oauth-protected resource I'm trying to hit is comparing against. This will result in the dreaded 'invalid signature' error.

Simply unencoding the tilde before encrypting the base string resolves the issue:

base = base.replace('%7E', '~');

But that reeks of code smell, so I thought I'd instead submit an issue to see what folks thing a better way of handling this might be.

Any usage of this package ? I need to use RSA-SHA1 for creating an Authorization header

It turns out that for debugging purposes between systems, it is really important to have the base string available. So it would be nice to make the code to generate the base string accessible outside the module.

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[ ] Feature request
[x] Documentation issue or request
[ ] Support request
[x] Other... Please describe:

Current behavior

No copyright notice is given in your license text. Since the Apache 2.0 license requires the preservation of the copyright notice at distribution, you are making it hard to impossible to use your library legally in Germany where copyright can not be waivered. I'm not a lawyer, this is just what they tell me ;-)

Expected behavior

A copyright notice is given in your license text. Maybe this link helps: https://www.disclaimergenerator.net/copyright-notices/

Minimal reproduction of the problem with instructions

See https://github.com/request/oauth-sign/blob/master/LICENSE

What is the motivation / use case for changing the behavior?

A big company's compliance department is refusing to allow the use of an application using your library because your copyright notice is missing.

Hi, I found that when the querystring contains nested objects e.g.

filter[email][email protected]

then the base string in hmacsign ends up with

filter%3D%255Bobject%2520Object%255D

rather than

filter%255Bemail%255D%3Dbob%2540bob.com

I don't think this is intentional but just wanted to confirm. Let me know and I'll raise a PR.

What are your thoughts on supporting oauth_body_hash? I'd be happy to write the code for it and submit a pull request. I just want to get some feedback first.

Fails on an example in OAuth 1.0 specification(RFC5849).
It caused by following two:

Handling multiple parameter values with same key.

When node's querystring parse query string and build object, it builds value as an array for same keys. For instance:

> var qs = require('querystring')
> var str = 'a=1&a=2&a=&b=3'
> qs.parse(str)
{ a: [ '1', '2', '' ],
  b: '3' }

Currently, oauth-sign does not handle array value.

Sorting in parameter normalization

The specification requires percent encode before sorting parameters. (http://tools.ietf.org/html/rfc5849#section-3.4.1.3.2)
In some case, the sort order may differ after encode. (e.g. c2 < c@, but c2 > c%40)

And also it requires to sort by value when there is a same keys, but current oauth-sign does not sort by value.

I would love an oauth.rsasign() :)