Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Adds RSA-SHA1 Support #43

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants

jeffv commented May 10, 2011

We needed RSA-SHA1 signatures for api.xero.com.
So we added them.

Nice, thank you. Any chance of a test, if so I'll pull it straight in ?

Owner

jeffv replied May 11, 2011

I'll see what I can do when time permits.
For some reason the library I built over top of my hack started failing to sign its POST requests (and only its post requests) so I have to dig into that first.

Odd. Did you get to the bottom of this ?

Owner

jeffv replied May 26, 2011

Yes. It turned out that if you send it as a querystring instead of an object the string used for the signature doesn't get the POST params added to it like it should. If you use an object everything is fine.

Can we get this added in? Xero still has the same requirement in some places.

As-is this pull request does not work. New line 169 refers to this._privateKey, which isn't defined anywhere.

I did some googling and this appears to be the way most people are self-patching this library to support rsa-sha1. For instance if you want to use oauth with JIRA and follow their examples:

https://bitbucket.org/rmanalan/atlassian-oauth-examples/src/807992a74230/nodejs/app.js?at=default

You are directed to a fork of node-oauth:

https://github.com/sladey/node-oauth

... that does something similarly. However one issue w/ the sladey fork is that it mucks about w/ the parameter count, which would break existing deploys of node-oauth who upgraded.

I even have my own branch:

https://github.com/wraithgar/node-oauth

... in which I repurposed the consumerSecret parameter to be interpreted as the private key if the signatureMethod was in fact rsa-sha1. I think this is probably the safest approach, as it doesn't mess w/ the parameter count or order so existing deploys of node-oauth wouldn't break.

In the case of tests, unfortunately it's stated as 'outside the scope' of the core 1.0 examples:

http://oauth.net/core/1.0/#sig_base_example

So I'm not 100% on how to construct a test for it. Otherwise I'd have already submitted a pull request from my fork. I think this is an important addition, since rsa-sha1 signing is the method used by most 'enterprise' implementations of oauth, being the more secure method. Can we please revisit this, either in this pull request or in a new issue? What do you need in order to get this into node-oauth?

@knechtandreas knechtandreas referenced this pull request Dec 20, 2012

Merged

Add RSA-SHA1 support #121

I've created a new pull request that adds a test to https://github.com/wraithgar/node-oauth 's work:
#121

This pull request can probably be closed if 121 goes ahead.

Owner

ciaranj commented Jan 4, 2014

Closing, as merging in #121

@ciaranj ciaranj closed this Jan 4, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment