Exclude querystring in params passed to `url.format` in `authify`

[indexzero]

@izs @othiym23 We've been seeing a lot of:

npm info retry will retry, error on last attempt: Error: This request requires auth credentials. Run `npm login` and repeat the request.

So after running in to it again tonight for seemingly no reason, I sat down to figure it out. Starting with the proper "nerf darts" (lol @jennschiffer on that name btw) in my .npmrc:

$ cat ~/.npmrc
# other valid config
//demo.registry.nodejitsu.com/:_password={REDACTED}
//demo.registry.nodejitsu.com/:username=indexzero
//demo.registry.nodejitsu.com/:email={REDACTED}
//demo.registry.nodejitsu.com/:always-auth=true

when trying to do an npm unpublish somepkg1042 --force (and a cache miss I believe):

$ npm unpublish somepkg1042 --force --loglevel=silly
npm info it worked if it ends with ok
npm verb cli [ 'node',
npm verb cli   '/Users/charlie/.local/bin/npm',
npm verb cli   'unpublish',
npm verb cli   'somepkg1042',
npm verb cli   '--force',
npm verb cli   '--loglevel=silly' ]
npm info using npm@2.1.4
npm info using node@v0.10.32
npm WARN using --force I sure hope you know what you are doing.
npm sill unpublish args[0] somepkg1042
npm sill unpublish thing { raw: 'somepkg1042',
npm sill unpublish   scope: null,
npm sill unpublish   name: 'somepkg1042',
npm sill unpublish   rawSpec: '',
npm sill unpublish   spec: '*',
npm sill unpublish   type: 'range' }
npm sill ls normalized somepkg1042
npm sill ls normalized somepkg1042
npm verb gentlyRm vacuuming /Users/charlie/.npm/somepkg1042
npm verb mapToRegistry name somepkg1042
npm verb mapToRegistry uri https://demo.registry.nodejitsu.com/somepkg1042
npm verb request on initialization, where is /somepkg1042
npm verb request after pass 1, where is /somepkg1042?write=true
npm verb request url raw /somepkg1042?write=true
npm verb request resolving registry [ 'https://demo.registry.nodejitsu.com/',
npm verb request   './somepkg1042?write=true' ]
npm verb request after pass 2, where is https://demo.registry.nodejitsu.com/somepkg1042?write=true
npm verb request always-auth set; sending authorization
npm info attempt registry request try #1 at 03:31:12
npm info retry will retry, error on last attempt: Error: This request requires auth credentials. Run `npm login` and repeat the request.

I got bounced back for auth credentials. But why!? I threw in some console.dir around authify to try to get more information because as I saw above the nerf darts are clearly valid:

function authify (authed, parsed, headers) {
  console.dir(arguments)
  var c = this.conf.getCredentialsByURI(url.format(parsed))
  console.dir(c);
  // all of the original code..

This is when the real problem was revealed:

{ '0': true,
  '1': 
   { protocol: 'https:',
     slashes: true,
     auth: null,
     host: 'demo.registry.nodejitsu.com',
     port: null,
     hostname: 'demo.registry.nodejitsu.com',
     hash: null,
     search: '?write=true',
     query: 'write=true',
     pathname: '/somepkg1042',
     path: '/somepkg1042?write=true',
     href: 'https://demo.registry.nodejitsu.com/somepkg1042?write=true' },
  '2': { 'accept-encoding': 'gzip' } }
{ scope: '//demo.registry.nodejitsu.com/?write=true',
  token: undefined,
  password: undefined,
  username: undefined,
  email: '{REDACTED}',
  auth: undefined,
  alwaysAuth: false }

As you can see: the parsed URL object which is reformatted before being passed to getCredentialsByURI includes any querystring parameters like ?write=true which are not in the "nerf darts" stored in the .npmrc files.

This pull-request fixes this by passing in a more minimal object to url.format to exclude the querystrings given to getCredentialsByURI. This seems preferable to deleting the appropriate keys in parsed since the object is used elsewhere in the caller functions. It also adds unit tests for authify which are easy to extend for more edge cases like this (scopes, etc).

[jcrugzz]

@othiym23 any word on this? I'd love to actually use the new client you've been putting work into!

[isaacs]

This is a legit bug. I asked @othiym23 yesterday and he said that he was in the middle of some surgery in npm-registry-client, so it'll get addressed soon.

[indexzero]

Thanks @isaacs. Spoke a bit about it with @terinjokes and the fix in #74 and https://github.com/npm/npm/pull/6514 seems to be the better solution. Going to close this in favor of those.