search

krakenjs / passport-saml-encrypted

A strategy for Passport authentication that supports encrypted SAML responses
MIT License
14 stars 26 forks source link

Please add Try-Catch over xml-cypto methods #14

Closed mayhs19 closed 8 years ago

mayhs19 commented 8 years ago

Please add try-catch blocks wherever functions from xml-crypto modules are used . This is because xml-crypto in itself doesnt seem to handle exceptions well.

Sample Stack trace due to this :

at Object.findAttr (/apps/snap/snapnodeweb/node_modules/xml-crypto/lib/utils.js:20:18) at SignedXml.loadReference (/apps/snap/snapnodeweb/node_modules/xml-crypto/lib/signed-xml.js:320:29) at SignedXml.loadSignature (/apps/snap/snapnodeweb/node_modules/xml-crypto/lib/signed-xml.js:280:10) at SAML.validateSignature (/apps/snap/snapnodeweb/node_modules/passport-saml-encrypted/lib/saml.js:236:7) at SAML.validateResponse (/apps/snap/snapnodeweb/node_modules/passport-saml-encrypted/lib/saml.js:258:33) at Strategy.authenticate (/apps/snap/snapnodeweb/node_modules/passport-saml-encrypted/lib/strategy.js:31:16) at attempt (/apps/snap/snapnodeweb/node_modules/passport/lib/middleware/authenticate.js:343:16) at authenticate (/apps/snap/snapnodeweb/node_modules/passport/lib/middleware/authenticate.js:344:7) at Layer.handle [as handle_request] (/apps/snap/snapnodeweb/node_modules/express/lib/router/layer.js:95:5) at next (/apps/snap/snapnodeweb/node_modules/express/lib/router/route.js:131:13) at Route.dispatch (/apps/snap/snapnodeweb/node_modules/express/lib/router/route.js:112:3) at Layer.handle [as handle_request] (/apps/snap/snapnodeweb/node_modules/express/lib/router/layer.js:95:5) at /apps/snap/snapnodeweb/node_modules/express/lib/router/index.js:277:22 at Function.process_params (/apps/snap/snapnodeweb/node_modules/express/lib/router/index.js:330:12) at next (/apps/snap/snapnodeweb/node_modules/express/lib/router/index.js:271:10) at ssoAuthenticate (/apps/snap/snapnodeweb/node_modules/sso-paypal/lib/middleware.js:15:13)

lmarkus commented 8 years ago

You are probably right, unfortunately I'm completely tapped out for time. PR's welcome :)

jonfairbanks commented 8 years ago

Hi Lenny, I have opened https://github.com/lmarkus/passport-saml-encrypted/pull/17 to add try/catch blocks when using xml-crypto.

Please feel free to let me know if additional changes are needed.

Thanks for reviewing!

lmarkus commented 8 years ago

Solved via #17 Thanks @jonfairbanks

Publishing an update shortly...

lmarkus commented 8 years ago

Published v0.1.15