search

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

raise error on undefined secrets #65

Open borromeotlhs opened 7 years ago

borromeotlhs commented 7 years ago

Change to SignStream class as it uses DataStream objects to create secret and payload streams. This error should then be caught by jwsSign to adequately fail users downstream instead of failing silently.

willm commented 7 years ago

You need to handle the case where SignStream takes a stream. Maybe the check in dataStream could be pushed higher up?

borromeotlhs commented 7 years ago

Yeah, I'm failing the streaming signing of RSA tests specifically because of this. Trying to figure out a way to do just that, it just sucks that I can't run automated build and tests without creating PRs (failing in public is fun, right?)

willm commented 7 years ago

You can, just run npm test

borromeotlhs commented 7 years ago

I don't have a node server at work, so I edit in atom directly and then create PRs. I'm not even using this library, yet, but got down this rabbit hole when investigating using jwts for authentication on an app I'm working on in cloud9, then saw this issue. . . ;)

borromeotlhs commented 7 years ago

There, I added the test reported from the issue that raised this.

I did a naked return whenever a secret is undefined to make an undefined value propagate up through to jws.Sign, at which point, an 'error' should emit.

@willm , does this address your concern without having to change DataStream directly for streams? I'd love a suggestion on how to write a test to catch the emit('error') functions though, for completeness.