mswjs / interceptors

Low-level network interception library.
https://npm.im/@mswjs/interceptors
MIT License
561 stars 126 forks source link

Support username:password in NodeClientRequest #438

Closed mikicho closed 1 year ago

mikicho commented 1 year ago

The request fails when we send auth option to http.request:

http.request({
  hostname: 'example.test',
  auth: 'username:password',
  path: '/',
})

logs

20:33:47:990 [http:request GET http://username:password@example.test/] constructing ClientRequest using options: {"url":"http://username:password@example.test/","requestOptions":{"hostname":"example.test","auth":"username:password","path":"/","protocol":"http:","method":"GET","agent":{"_events":{},"_eventsCount":2,"defaultPort":80,"protocol":"http:","options":{"noDelay":true,"path":null},"requests":{},"sockets":{"example.test:80:":[{"connecting":true,"_hadError":false,"_parent":null,"_host":"example.test","_closeAfterHandlingError":false,"_readableState":{"objectMode":false,"highWaterMark":16384,"buffer":{"head":null,"tail":null,"length":0},"length":0,"pipes":[],"flowing":null,"ended":false,"endEmitted":false,"reading":false,"constructed":true,"sync":true,"needReadable":false,"emittedReadable":false,"readableListening":false,"resumeScheduled":false,"errorEmitted":false,"emitClose":false,"autoDestroy":true,"destroyed":false,"errored":null,"closed":false,"closeEmitted":false,"defaultEncoding":"utf8","awaitDrainWriters":null,"multiAwaitDrain":false,"readingMore":false,"dataEmitted":false,"decoder":null,"encoding":null},"_events":{},"_eventsCount":6,"_writableState":{"objectMode":false,"highWaterMark":16384,"finalCalled":false,"needDrain":false,"ending":false,"ended":false,"finished":false,"destroyed":false,"decodeStrings":false,"defaultEncoding":"utf8","length":0,"writing":false,"corked":0,"sync":true,"bufferProcessing":false,"writecb":null,"writelen":0,"afterWriteTickInfo":null,"buffered":[],"bufferedIndex":0,"allBuffers":true,"allNoop":true,"pendingcb":0,"constructed":true,"prefinished":false,"errorEmitted":false,"emitClose":false,"autoDestroy":true,"errored":null,"closed":false,"closeEmitted":false},"allowHalfOpen":false,"_sockname":null,"_pendingData":null,"_pendingEncoding":"","server":null,"_server":null}]},"freeSockets":{},"keepAliveMsecs":1000,"keepAlive":false,"maxSockets":null,"maxFreeSockets":256,"scheduling":"lifo","maxTotalSockets":null,"totalSocketCount":1},"_defaultAgent":{"_events":{},"_eventsCount":2,"defaultPort":80,"protocol":"http:","options":{"keepAlive":true,"scheduling":"lifo","timeout":5000,"noDelay":true,"path":null},"requests":{},"sockets":{},"freeSockets":{},"keepAliveMsecs":1000,"keepAlive":true,"maxSockets":null,"maxFreeSockets":256,"scheduling":"lifo","maxTotalSockets":null,"totalSocketCount":0}}}

I think we should add some logic into the createRequest function https://github.com/mswjs/interceptors/blob/main/src/interceptors/ClientRequest/NodeClientRequest.ts#L141

I think I can open a PR if you think I'm correct and we should fix the createRequest function

kettanaito commented 1 year ago

Hi, @mikicho. Thanks for proposing this.

Can you please tell me more about the expected behavior here? Do you wish to see the username/password represented as the Basic authentication header on the constructed Request instance? I do believe we grab the values from the options and forward them onto the ClientRequest instance as of now.

The request fails when we send auth option to http.request:

You must be referring to some third-party request module. auth isn't a supported property on the http.ClientRequest constructor. Instead, Node.js uses username and password properties:

http.request({
  method: 'POST',
  host: 'localhost',
  port: 12345,
  username: 'john',
  password: 'supersecret123'
})
mikicho commented 1 year ago

Can you please tell me more about the expected behavior here?

I think this is a bug:

const { ClientRequestInterceptor } = require('@mswjs/interceptors/ClientRequest')
const http = require('http')

const interceptor = new ClientRequestInterceptor({
  name: 'my-interceptor',
})
interceptor.apply();
interceptor.on('request', async ({request, a}) => {
  await new Promise(r => setTimeout(r, 700));
  throw new Error('error')
});

const req = http.request(
  {
    host: 'example.test',
    path: '/wrong-path',
    auth: 'user:pass'
  },
)
req.end() // TypeError: Request cannot be constructed from a URL that includes credentials: http://user:pass@example.test/wrong-path

Do you wish to see the username/password represented...

Probably yes.

You must be referring to some third-party request module...

It's one of the http.request options: https://nodejs.org/api/http.html#httprequesturl-options-callback

kettanaito commented 1 year ago

Oh, I've overlooked that one 🤦 Thanks for pointing out. I wonder why this errors.

mikicho commented 1 year ago

The error is from the createRequest function: image

kettanaito commented 1 year ago

I see. So we need to omit the credentials from the URL itself and instead represent them in the Authorization request header. I think that's doable! I think this implementation can consist of multiple steps.

  1. Do not propagate username and password onto the NodeClientRequest.url. From the source of it, we don't do that, but it doesn't hurt to check.
  2. In createRequest, check request.username and request.password, and translate them into the Authorization header.

If you have time, I'd be thankful if you opened a pull request and we iterated on this together. Starting with some integration test would also be a good point.

mikicho commented 1 year ago

on it

kettanaito commented 1 year ago

Released: v0.25.4 🎉

This has been released in v0.25.4!

Make sure to always update to the latest version (npm i @mswjs/interceptors@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.