auth0 / node-jws

JSON Web Signatures
http://self-issued.info/docs/draft-ietf-jose-json-web-signature.html
MIT License
709 stars 108 forks source link

Stream interface should catch verification errors and emit as 'error' #37

Closed cbaron closed 8 years ago

cbaron commented 8 years ago

Let's say something like "%s" gets passed as a signature to createVerify.

Then,

buffer.js:139 throw new TypeError('must start with number, buffer, array or string');

How can I catch that error ?

omsmith commented 8 years ago

Are you sure that's what is causing the error?

That case is caused by passing an Object into a buffer, which would only make much sense for the secret/key. Which is a known issue to fix in the "stream" workflow of jws.

Either way, try/catch would be the way to handle it, as it's being thrown synchronously in the constructor

omsmith commented 8 years ago

Closing this in favour of #39

cbaron commented 8 years ago

Hm, I do not believe try/catch is handling it. Please advise. I'm using node 5.1:

var jws = require('jws')
try { 
   jws.createVerify( { 
       algorithm: "HS256", 
       key: 'someKey', 
       signature :'%s'
    } ).on( 'done', ( verified, obj ) => { if( !verified ) { console.log( 'not verified' ) } } )
} catch(e) { console.log('caught') }
omsmith commented 8 years ago

What is the stack trace you're getting?

cbaron commented 8 years ago
TypeError: must start with number, buffer, array or string
    at fromObject (buffer.js:139:11)
    at Buffer (buffer.js:58:10)
    at Object.verify (/Users/cbaron/freelancR/patchwork/website/node_modules/jwa/index.js:42:24)
    at jwsVerify (/Users/cbaron/freelancR/patchwork/website/node_modules/jws/lib/verify-stream.js:54:15)
    at VerifyStream.verify (/Users/cbaron/freelancR/patchwork/website/node_modules/jws/lib/verify-stream.js:101:17)
    at VerifyStream.<anonymous> (/Users/cbaron/freelancR/patchwork/website/node_modules/jws/lib/verify-stream.js:91:12)
    at DataStream.g (events.js:260:16)
    at emitNone (events.js:67:13)
    at DataStream.emit (events.js:166:7)
    at DataStream.<anonymous> (/Users/cbaron/freelancR/patchwork/website/node_modules/jws/lib/data-stream.js:20:12)
omsmith commented 8 years ago

Great, thanks

I can fix that this evening

cbaron commented 8 years ago

Thank you. :)

On Thu, Jan 14, 2016 at 11:54 AM, Owen Smith notifications@github.com wrote:

Great, thanks

I can fix that this evening

— Reply to this email directly or view it on GitHub https://github.com/brianloveswords/node-jws/issues/37#issuecomment-171701026 .

Chris Baron

omsmith commented 8 years ago

@cbaron Can you give it a go with the latest master?

cbaron commented 8 years ago

I will, not tonight, but soon. I appreciate the fix -- the library has proved quite useful for my new web applications. Perhaps we will get to work together in the future.

cbaron commented 8 years ago

Hm, the error is still not being caught. Also, could you please increment the version number to make it easier for me, or anyone else to pull updates ?

omsmith commented 8 years ago

Wanted you to give it a go before publishing.

Should be able to catch it with .on('error', function (e) {});

cbaron commented 8 years ago

Ahhh, I see. One moment.

On Sun, Jan 17, 2016 at 12:29 PM, Owen Smith notifications@github.com wrote:

Wanted you to give it a go before publishing.

Should be able to catch it with .on('error', function (e) {});

— Reply to this email directly or view it on GitHub https://github.com/brianloveswords/node-jws/issues/37#issuecomment-172354991 .

Chris Baron

cbaron commented 8 years ago

Yes, great success. Thanks again. :shipit:

omsmith commented 8 years ago

@cbaron 3.1.1