auth0 / node-samlp

SAML Protocol support for node (only IdP for now)
MIT License
136 stars 117 forks source link

Dependency querystring is deprecated. URLSearchParams API should be used instead #137

Open aelliott1485 opened 1 year ago

aelliott1485 commented 1 year ago

Description

Dependency querystring is deprecated

Reproduction

While installing the latest version a warning about the querystring dependency being deprecated is displayed:

% npm install samlp
npm WARN deprecated querystring@0.2.1: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.

added 38 packages, and audited 39 packages in 2s

2 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

Environment

aelliott1485 commented 1 year ago

I forked the repo and was planning to make the changes to go into a PR. Before doing that I ran the test script from package.json and it showed two failures:

  2 failing

  1) samlp
       response signing
         signResponse=true and signAssertion=true
           when invalid signing key is used
             should return an error:
     Uncaught AssertionError: expected 'error:1E08010C:DECODER routines::unsu…' to match /error:\w+:PEM routines:\w+:no start line/
      at /Users/samonela/code/node-samlp/test/samlp.tests.js:691:38
      at Request._callback (test/samlp.tests.js:652:9)
      at self.callback (node_modules/request/request.js:185:22)
      at Request.emit (node:events:514:28)
      at Request.<anonymous> (node_modules/request/request.js:1154:10)
      at Request.emit (node:events:514:28)
      at IncomingMessage.<anonymous> (node_modules/request/request.js:1076:12)
      at Object.onceWrapper (node:events:628:28)
      at IncomingMessage.emit (node:events:526:35)
      at endReadableNT (node:internal/streams/readable:1408:12)
      at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

  2) samlp
       response signing
         signResponse=true and signAssertion=false
           when invalid signing key is used
             should return an error:
     Uncaught AssertionError: expected 'error:1E08010C:DECODER routines::unsu…' to match /error:\w+:PEM routines:\w+:no start line/
      at /Users/samonela/code/node-samlp/test/samlp.tests.js:741:38
      at Request._callback (test/samlp.tests.js:652:9)
      at self.callback (node_modules/request/request.js:185:22)
      at Request.emit (node:events:514:28)
      at Request.<anonymous> (node_modules/request/request.js:1154:10)
      at Request.emit (node:events:514:28)
      at IncomingMessage.<anonymous> (node_modules/request/request.js:1076:12)
      at Object.onceWrapper (node:events:628:28)
      at IncomingMessage.emit (node:events:526:35)
      at endReadableNT (node:internal/streams/readable:1408:12)
      at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

I checked on response.body when it doesn't match as expected - both times it appears to be

 error:1E08010C:DECODER routines::unsupported

Is there a setup step that needs to be completed first?

EDIT I noticed the .github/workflows/ci.yml file targets node-version: [12.x, 14.x, 15.x, 16.x] so I was able to setup a dockerfile:

# specify the node base image with your desired version node:<version>
FROM node:16

Then with a mounting -v of the local directory to /code I can run cd code && npm run cover. The tests pass before and after I made these changes to remove the dependency querystring. I'll attempt to add another test for more coverage.