Skip to content

Commit

Permalink
feat(auth) load/send based on URI, not registry
Browse files Browse the repository at this point in the history
BREAKING CHANGE: removes the alwaysAuth config

This refactors the handling of auth in the new context where npm/cli
will _only_ ever send authorization in such a way that any
authorization-related options are scoped to a given sanitized registry
host and path.

This fixes a troubling situation where a user has intentionally logged
into two separate registries, one which serves package distribution
tarballs, and the other which has packuments pointing to it.

For example, a request to `https://registry.internal/private-thing`
might return this packument:

```json
{
  "name": "private-thing",
  "dist-tags": {
    "latest": "1.0.0"
  },
  "versions": {
    "1.0.0": {
      "name": "private-thing",
      "version": "1.0.0",
      "dist": {
        "tarball": "https://tarballs.internal/private-thing-1.0.0.tgz"
      }
    }
  }
}
```

If the user has properly logged into both `https://registry.internal`
and `https://tarballs.internal` (or at least placed an auth or bearer
token in their `.npmrc` for each), then this should work properly when
they run:

```
npm install private-thing --registry=https://registry.internal
```

However, previously the auth for `https://tarballs.internal` would never
be loaded, because it was being loaded based solely on the _registry_
URI, regardless of the actual URI being fetched, and then only _sent_
when the hostname matches, and so the request for the tarball would
fail.

BREAKING CHANGES:

* The `alwaysAuth` config is no longer relevant.  If we have
  authentication information for a given URI, we send it when making the
  request to that URI.  This was previously being done based on the
  presence of a `scope` parameter or a scoped `spec` option.  However,
  that is not reliable, as users can depend directly on tarball URLs,
  and other parts of the system may have reason to make requests to the
  registry in question without a specific dependency spec in mind (for
  example, to fetch a packument for the purpose of calculating
  metavulnerabilities during an `npm audit` operation.)

* A top level `_auth`, `_authToken`, `username`, `_password`, or
  `password` option is no longer respected if not scoped to a given
  registry URL.  This functionality was removed from npm/cli as of
  version 7, due to the risk of sending credentials to an incorrect host
  by mistake.  The npm cli will automatically scope these top-level
  configs to the default registry option and save them back to the
  `.npmrc` file the first time they are encountered, so users generally
  see no disruption, and this change only affects non-npm users of
  npm-registry-fetch.

* Add 'name' to HttpError classes, canonicalize tap usage

* Suggest fix for case broken by removing alwaysAuth

Since we no longer send auth to different registry hosts when alwaysAuth
is set, attempt to get the scoped registry auth when an un-authenticated
request fails.  If that scoped registry auth exists, then direct the
user to a web page that tells them how to fix the problem.
  • Loading branch information
isaacs authored Apr 22, 2021
1 parent d77b165 commit cc11cc1
Show file tree
Hide file tree
Showing 9 changed files with 481 additions and 280 deletions.
129 changes: 84 additions & 45 deletions auth.js
Original file line number Diff line number Diff line change
@@ -1,55 +1,94 @@
'use strict'
const npa = require('npm-package-arg')

const defaultOpts = require('./default-opts.js')
const url = require('url')
// Find the longest registry key that is used for some kind of auth
// in the options.
const regKeyFromURI = (uri, opts) => {
const parsed = new URL(uri)
// try to find a config key indicating we have auth for this registry
// can be one of :_authToken, :_auth, or :_password and :username
// We walk up the "path" until we're left with just //<host>[:<port>],
// stopping when we reach '//'.
let regKey = `//${parsed.host}${parsed.pathname}`
while (regKey.length > '//'.length) {
// got some auth for this URI
if (hasAuth(regKey, opts))
return regKey

module.exports = getAuth
function getAuth (registry, opts_ = {}) {
if (!registry)
throw new Error('registry is required')
const opts = opts_.forceAuth ? opts_.forceAuth : { ...defaultOpts, ...opts_ }
const AUTH = {}
const regKey = registry && registryKey(registry)
const doKey = (key, alias) => addKey(opts, AUTH, regKey, key, alias)
doKey('token')
doKey('_authToken', 'token')
doKey('username')
doKey('password')
doKey('_password', 'password')
doKey('email')
doKey('_auth')
doKey('otp')
doKey('always-auth', 'alwaysAuth')
if (AUTH.password)
AUTH.password = Buffer.from(AUTH.password, 'base64').toString('utf8')

if (AUTH._auth && !(AUTH.username && AUTH.password)) {
let auth = Buffer.from(AUTH._auth, 'base64').toString()
auth = auth.split(':')
AUTH.username = auth.shift()
AUTH.password = auth.join(':')
// can be either //host/some/path/:_auth or //host/some/path:_auth
// walk up by removing EITHER what's after the slash OR the slash itself
regKey = regKey.replace(/([^/]+|\/)$/, '')
}
AUTH.alwaysAuth = AUTH.alwaysAuth === 'false' ? false : !!AUTH.alwaysAuth
return AUTH
}

function addKey (opts, obj, scope, key, objKey) {
if (opts[key])
obj[objKey || key] = opts[key]
const hasAuth = (regKey, opts) => (
opts[`${regKey}:_authToken`] ||
opts[`${regKey}:_auth`] ||
opts[`${regKey}:username`] && opts[`${regKey}:_password`]
)

if (scope && opts[`${scope}:${key}`])
obj[objKey || key] = opts[`${scope}:${key}`]
}
const getAuth = (uri, opts = {}) => {
const { forceAuth } = opts
if (!uri)
throw new Error('URI is required')
const regKey = regKeyFromURI(uri, forceAuth || opts)

// we are only allowed to use what's in forceAuth if specified
if (forceAuth && !regKey) {
return new Auth({
scopeAuthKey: null,
token: forceAuth._authToken,
username: forceAuth.username,
password: forceAuth._password || forceAuth.password,
auth: forceAuth._auth || forceAuth.auth,
})
}

// no auth for this URI
if (!regKey && opts.spec) {
// If making a tarball request to a different base URI than the
// registry where we logged in, but the same auth SHOULD be sent
// to that artifact host, then we track where it was coming in from,
// and warn the user if we get a 4xx error on it.
const { spec } = opts
const { scope: specScope, subSpec } = npa(spec)
const subSpecScope = subSpec && subSpec.scope
const scope = subSpec ? subSpecScope : specScope
const scopeReg = scope && opts[`${scope}:registry`]
const scopeAuthKey = scopeReg && regKeyFromURI(scopeReg, opts)
return new Auth({ scopeAuthKey })
}

// Called a nerf dart in the main codebase. Used as a "safe"
// key when fetching registry info from config.
function registryKey (registry) {
const parsed = new url.URL(registry)
const formatted = url.format({
protocol: parsed.protocol,
host: parsed.host,
pathname: parsed.pathname,
slashes: true,
const {
[`${regKey}:_authToken`]: token,
[`${regKey}:username`]: username,
[`${regKey}:_password`]: password,
[`${regKey}:_auth`]: auth,
} = opts

return new Auth({
scopeAuthKey: null,
token,
auth,
username,
password,
})
return url.format(new url.URL('.', formatted)).replace(/^[^:]+:/, '')
}

class Auth {
constructor ({ token, auth, username, password, scopeAuthKey }) {
this.scopeAuthKey = scopeAuthKey
this.token = null
this.auth = null
if (token)
this.token = token
else if (auth)
this.auth = auth
else if (username && password) {
const p = Buffer.from(password, 'base64').toString('utf8')
this.auth = Buffer.from(`${username}:${p}`, 'utf8').toString('base64')
}
}
}

module.exports = getAuth
17 changes: 14 additions & 3 deletions check-response.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,25 @@ const LRU = require('lru-cache')
const { Response } = require('minipass-fetch')
const defaultOpts = require('./default-opts.js')

module.exports = checkResponse
function checkResponse (method, res, registry, startTime, opts_ = {}) {
const opts = { ...defaultOpts, ...opts_ }
const checkResponse = async ({ method, uri, res, registry, startTime, auth, opts }) => {
opts = { ...defaultOpts, ...opts }
if (res.headers.has('npm-notice') && !res.headers.has('x-local-cache'))
opts.log.notice('', res.headers.get('npm-notice'))

checkWarnings(res, registry, opts)
if (res.status >= 400) {
logRequest(method, res, startTime, opts)
if (auth && auth.scopeAuthKey && !auth.token && !auth.auth) {
// we didn't have auth for THIS request, but we do have auth for
// requests to the registry indicated by the spec's scope value.
// Warn the user.
opts.log.warn('registry', `No auth for URI, but auth present for scoped registry.
URI: ${uri}
Scoped Registry Key: ${auth.scopeAuthKey}
More info here: https://github.com/npm/cli/wiki/No-auth-for-URI,-but-auth-present-for-scoped-registry`)
}
return checkErrors(method, res, startTime, opts)
} else {
res.body.on('end', () => logRequest(method, res, startTime, opts))
Expand All @@ -24,6 +34,7 @@ function checkResponse (method, res, registry, startTime, opts_ = {}) {
return res
}
}
module.exports = checkResponse

function logRequest (method, res, startTime, opts) {
const elapsedTime = Date.now() - startTime
Expand Down
1 change: 1 addition & 0 deletions errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ function packageName (href) {
class HttpErrorBase extends Error {
constructor (method, res, body, spec) {
super()
this.name = this.constructor.name
this.headers = res.headers.raw()
this.statusCode = res.status
this.code = `E${res.status}`
Expand Down
62 changes: 32 additions & 30 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,26 +27,32 @@ function regFetch (uri, /* istanbul ignore next */ opts_ = {}) {
...defaultOpts,
...opts_,
}
const registry = opts.registry = (
(opts.spec && pickRegistry(opts.spec, opts)) ||
opts.registry ||
/* istanbul ignore next */
'https://registry.npmjs.org/'
)

if (!urlIsValid(uri)) {

// if we did not get a fully qualified URI, then we look at the registry
// config or relevant scope to resolve it.
const uriValid = urlIsValid(uri)
let registry = opts.registry || defaultOpts.registry
if (!uriValid) {
registry = opts.registry = (
(opts.spec && pickRegistry(opts.spec, opts)) ||
opts.registry ||
registry
)
uri = `${
registry.trim().replace(/\/?$/g, '')
}/${
uri.trim().replace(/^\//, '')
}`
// asserts that this is now valid
new url.URL(uri)
}

const method = opts.method || 'GET'

// through that takes into account the scope, the prefix of `uri`, etc
const startTime = Date.now()
const headers = getHeaders(registry, uri, opts)
const auth = getAuth(uri, opts)
const headers = getHeaders(uri, auth, opts)
let body = opts.body
const bodyIsStream = Minipass.isStream(body)
const bodyIsPromise = body &&
Expand Down Expand Up @@ -117,9 +123,15 @@ function regFetch (uri, /* istanbul ignore next */ opts_ = {}) {
},
strictSSL: opts.strictSSL,
timeout: opts.timeout || 30 * 1000,
}).then(res => checkResponse(
method, res, registry, startTime, opts
))
}).then(res => checkResponse({
method,
uri,
res,
registry,
startTime,
auth,
opts,
}))

return Promise.resolve(body).then(doFetch)
}
Expand Down Expand Up @@ -151,7 +163,7 @@ function pickRegistry (spec, opts = {}) {
registry = opts[opts.scope.replace(/^@?/, '@') + ':registry']

if (!registry)
registry = opts.registry || 'https://registry.npmjs.org/'
registry = opts.registry || defaultOpts.registry

return registry
}
Expand All @@ -163,7 +175,7 @@ function getCacheMode (opts) {
: 'default'
}

function getHeaders (registry, uri, opts) {
function getHeaders (uri, auth, opts) {
const headers = Object.assign({
'npm-in-ci': !!opts.isFromCI,
'user-agent': opts.userAgent,
Expand All @@ -178,25 +190,15 @@ function getHeaders (registry, uri, opts) {
if (opts.npmCommand)
headers['npm-command'] = opts.npmCommand

const auth = getAuth(registry, opts)
// If a tarball is hosted on a different place than the manifest, only send
// credentials on `alwaysAuth`
const shouldAuth = (
auth.alwaysAuth ||
new url.URL(uri).host === new url.URL(registry).host
)
if (shouldAuth && auth.token)
if (auth.token)
headers.authorization = `Bearer ${auth.token}`
else if (shouldAuth && auth.username && auth.password) {
const encoded = Buffer.from(
`${auth.username}:${auth.password}`, 'utf8'
).toString('base64')
headers.authorization = `Basic ${encoded}`
} else if (shouldAuth && auth._auth)
headers.authorization = `Basic ${auth._auth}`

if (shouldAuth && auth.otp)
headers['npm-otp'] = auth.otp
else if (auth.auth)
headers.authorization = `Basic ${auth.auth}`

if (opts.otp)
headers['npm-otp'] = opts.otp

return headers
}
Loading

0 comments on commit cc11cc1

Please sign in to comment.