Cli: [BUG] Default ports in scoped registry URLs break credentials

[jftanner]

Current Behavior:

If you configure a scoped repository where the registry URL contains the default port (443 for HTTPS`), then any credentials for that repository won't be loaded.

That I _think_ is happening is that the NPM cli is parsing the registry url and stripping off the unnecessary port. Then, when it tries look up the matching configuration parameters, it doesn't match the keys in the key-value pairs. (Since those keys contain the port.)

Expected Behavior:

I would expect NPM to find credentials/settings with the matching URL, even if that URL (unnecessarily) contains the default port.

Steps To Reproduce:

Unfortunately, this is impossible to produce without a private package, since the failure only occurs when required credentials are missing. I don't have any private packages in npmjs.org, so I can only reproduce this in a private registry.

(The registry details here have been altered for security reasons.)

1. Configure a scope to a private registry: (In this case, Artifactory)
   @example:registry=https://artifactory.example.com:443/artifactory/api/npm/npm-default-remote/ //artifactory.example.com:443/artifactory/api/npm/npm-default-remote/:_password=[REDACTED] //artifactory.example.com:443/artifactory/api/npm/npm-default-remote/:username=[REDACTED] //artifactory.example.com:443/artifactory/api/npm/npm-default-remote/:email=[REDACTED] //artifactory.example.com:443/artifactory/api/npm/npm-default-remote/:always-auth=true
2. Run any command against that scrope: npm view @example/some-package
3. See the resulting error:
   
4. Remove the port from the repository configuration lines:
   @example:registry=https://artifactory.example.com:443/artifactory/api/npm/npm-default-remote/ //artifactory.example.com:443/artifactory/api/npm/npm-default-remote/:_password=[REDACTED] //artifactory.example.com/artifactory/api/npm/npm-default-remote/:username=[REDACTED] //artifactory.example.com/artifactory/api/npm/npm-default-remote/:[email protected] //artifactory.example.com/artifactory/api/npm/npm-default-remote/:always-auth=true
5. Try the command again, and see success.

Note that I _left_ the :443 portion in @example:registry=https://artifactory.example.com:443, this demonstrates my hypothesis about the root cause of the bug.

Environment:

• OS: Ubuntu 18.04 on WSL2
• Node: 15.0.1
• npm: 7.0.3

This issue did not occur prior to NPM v7, and has been reproduced on several other machines with varying environments.

[jftanner]

This isn't a particularly severe bug, and it has a simple workaround: remove the unnecessary port from the URL.

However, it's an unexpected behavior and has the possibility to break automated systems which are creating these configurations dynamically. (Which is how I found it.)

[jftanner]

I've narrowed it down to a function in npm-registry-fetch: https://github.com/npm/npm-registry-fetch/blob/latest/auth.js#L44-L55

I'm gonna take a look at the code history over there to understand how/why this changed, and if it can be fixed.
If so, I'll open a PR against that repository.

Edit: Looks breaking change was in this commit. So, now to see if I can adjust it safely.
The new URL parser behaves differently from the old (deprecated) one. Relevantly:

[ljharb]

That commit was included in npm 6, though.

[jftanner]

It's possible that we had an old enough version of NPM 6 to have not run into the issue. I'm not entirely sure.
But I _am_ sure that's the code change that causes the bug.

That said, it's a _good_ code change. And, unfortunately, I don't see any good way altering the behavior without reverting to the old, deprecated API. Since that's obviously not acceptable, and the workaround is "don't put unnecessary ports in your .npmrc configs", I think it's probably safe to close this issue.

Hopefully it's a rare enough edge case that it doesn't cause problems, and maybe this issue will help the few folks that encounter it.

Anyone disagree?

[ljharb]

oh actually it was in v6.0.0 of npm-registry-fetch, and v4 was what was in npm 6, my bad.

[ljharb]

It seems to me like if stripping the default ports in the implementation is best, then doing so in the config also would solve the issue?

[jftanner]

It seems to me like if stripping the default ports in the implementation is best, then doing so in the config also would solve the issue?

Oh, that's an idea! I haven't look at that code yet, but it's worth investigating.