Skip to content
This repository has been archived by the owner on Nov 3, 2022. It is now read-only.

Commit

Permalink
feat: throw errors for invalid auth configuration (#92)
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: unscoped auth configuration is no longer automatically scoped to a registry. the `validate` method is no longer called automatically. the `_auth` configuration key is no longer split into `username` and `_password`. errors will be thrown by `validate()` if problems are found.
  • Loading branch information
@nlf
nlf authored Oct 6, 2022
1 parent 5e345f0 commit 344ccd3
Show file tree
Hide file tree
Showing 4 changed files with 192 additions and 117 deletions.
13 changes: 13 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ process.on('log', (level, ...args) => {
// returns a promise that fails if config loading fails, and
// resolves when the config object is ready for action
conf.load().then(() => {
conf.validate()
console.log('loaded ok! some-key = ' + conf.get('some-key'))
}).catch(er => {
console.error('error loading configs!', er)
Expand Down Expand Up @@ -210,6 +211,10 @@ Delete the configuration key from the specified level in the config stack.
Verify that all known configuration options are set to valid values, and
log a warning if they are invalid.

Invalid auth options will cause this method to throw an error with a `code`
property of `ERR_INVALID_AUTH`, and a `problems` property listing the specific
concerns with the current configuration.

If `where` is not set, then all config objects are validated.

Returns `true` if all configs are valid.
Expand All @@ -218,6 +223,14 @@ Note that it's usually enough (and more efficient) to just check
`config.valid`, since each data object is marked for re-evaluation on every
`config.set()` operation.

### `config.repair(problems)`

Accept an optional array of problems (as thrown by `config.validate()`) and
perform the necessary steps to resolve them. If no problems are provided,
this method will call `config.validate()` internally to retrieve them.

Note that you must `await config.save('user')` in order to persist the changes.

### `config.isDefault(key)`

Returns `true` if the value is coming directly from the
Expand Down
22 changes: 22 additions & 0 deletions lib/errors.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict'

class ErrInvalidAuth extends Error {
constructor (problems) {
let message = 'Invalid auth configuration found: '
message += problems.map((problem) => {
if (problem.action === 'delete') {
return `\`${problem.key}\` is not allowed in ${problem.where} config`
} else if (problem.action === 'rename') {
return `\`${problem.from}\` must be renamed to \`${problem.to}\` in ${problem.where} config`
}
}).join(', ')
message += '\nPlease run `npm config fix` to repair your configuration.`'
super(message)
this.code = 'ERR_INVALID_AUTH'
this.problems = problems
}
}

module.exports = {
ErrInvalidAuth,
}
198 changes: 106 additions & 92 deletions lib/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ const parseField = require('./parse-field.js')
const typeDescription = require('./type-description.js')
const setEnvs = require('./set-envs.js')

const {
ErrInvalidAuth,
} = require('./errors.js')

// types that can be saved back to
const confFileTypes = new Set([
'global',
Expand Down Expand Up @@ -280,26 +284,10 @@ class Config {
await this.loadGlobalConfig()
process.emit('timeEnd', 'config:load:global')

// warn if anything is not valid
process.emit('time', 'config:load:validate')
this.validate()
process.emit('timeEnd', 'config:load:validate')

// set this before calling setEnvs, so that we don't have to share
// symbols, as that module also does a bunch of get operations
this[_loaded] = true

process.emit('time', 'config:load:credentials')
const reg = this.get('registry')
const creds = this.getCredentialsByURI(reg)
// ignore this error because a failed set will strip out anything that
// might be a security hazard, which was the intention.
try {
this.setCredentialsByURI(reg, creds)
// eslint-disable-next-line no-empty
} catch (_) {}
process.emit('timeEnd', 'config:load:credentials')

// set proper globalPrefix now that everything is loaded
this.globalPrefix = this.get('prefix')

Expand Down Expand Up @@ -399,15 +387,58 @@ class Config {
validate (where) {
if (!where) {
let valid = true
for (const [entryWhere] of this.data.entries()) {
const authProblems = []

for (const entryWhere of this.data.keys()) {
// no need to validate our defaults, we know they're fine
// cli was already validated when parsed the first time
if (entryWhere === 'default' || entryWhere === 'builtin' || entryWhere === 'cli') {
continue
}
const ret = this.validate(entryWhere)
valid = valid && ret

if (['global', 'user', 'project'].includes(entryWhere)) {
// after validating everything else, we look for old auth configs we no longer support
// if these keys are found, we build up a list of them and the appropriate action and
// attach it as context on the thrown error

// first, keys that should be removed
for (const key of ['_authtoken', '-authtoken']) {
if (this.get(key, entryWhere)) {
authProblems.push({ action: 'delete', key, where: entryWhere })
}
}

// NOTE we pull registry without restricting to the current 'where' because we want to
// suggest scoping things to the registry they would be applied to, which is the default
// regardless of where it was defined
const nerfedReg = nerfDart(this.get('registry'))
// keys that should be nerfed but currently are not
for (const key of ['_auth', '_authToken', 'username', '_password']) {
if (this.get(key, entryWhere)) {
// username and _password must both exist in the same file to be recognized correctly
if (key === 'username' && !this.get('_password', entryWhere)) {
authProblems.push({ action: 'delete', key, where: entryWhere })
} else if (key === '_password' && !this.get('username', entryWhere)) {
authProblems.push({ action: 'delete', key, where: entryWhere })
} else {
authProblems.push({
action: 'rename',
from: key,
to: `${nerfedReg}:${key}`,
where: entryWhere,
})
}
}
}
}
}

if (authProblems.length) {
throw new ErrInvalidAuth(authProblems)
}

return valid
} else {
const obj = this.data.get(where)
Expand All @@ -423,6 +454,40 @@ class Config {
}
}

// fixes problems identified by validate(), accepts the 'problems' property from a thrown
// ErrInvalidAuth to avoid having to check everything again
repair (problems) {
if (!problems) {
try {
this.validate()
} catch (err) {
// coverage skipped here because we don't need to test re-throwing an error
// istanbul ignore next
if (err.code !== 'ERR_INVALID_AUTH') {
throw err
}

problems = err.problems
} finally {
if (!problems) {
problems = []
}
}
}

for (const problem of problems) {
// coverage disabled for else branch because it doesn't do anything and shouldn't
// istanbul ignore else
if (problem.action === 'delete') {
this.delete(problem.key, problem.where)
} else if (problem.action === 'rename') {
const old = this.get(problem.from, problem.where)
this.set(problem.to, old, problem.where)
this.delete(problem.from, problem.where)
}
}
}

// Returns true if the value is coming directly from the source defined
// in default definitions, if the current value for the key config is
// coming from any other different source, returns false
Expand Down Expand Up @@ -644,21 +709,19 @@ class Config {
if (!confFileTypes.has(where)) {
throw new Error('invalid config location param: ' + where)
}

const conf = this.data.get(where)
conf[_raw] = { ...conf.data }
conf[_loadError] = null

// upgrade auth configs to more secure variants before saving
if (where === 'user') {
const reg = this.get('registry')
const creds = this.getCredentialsByURI(reg)
// we ignore this error because the failed set already removed
// anything that might be a security hazard, and it won't be
// saved back to the .npmrc file, so we're good.
try {
this.setCredentialsByURI(reg, creds)
// eslint-disable-next-line no-empty
} catch (_) {}
// if email is nerfed, then we want to de-nerf it
const nerfed = nerfDart(this.get('registry'))
const email = this.get(`${nerfed}:email`, 'user')
if (email) {
this.delete(`${nerfed}:email`, 'user')
this.set('email', email, 'user')
}
}

const iniData = ini.stringify(conf.data).trim() + '\n'
Expand Down Expand Up @@ -686,14 +749,17 @@ class Config {
const nerfed = nerfDart(uri)
const def = nerfDart(this.get('registry'))
if (def === nerfed) {
// do not delete email, that shouldn't be nerfed any more.
// just delete the nerfed copy, if one exists.
this.delete(`-authtoken`, 'user')
this.delete(`_authToken`, 'user')
this.delete(`_authtoken`, 'user')
this.delete(`_auth`, 'user')
this.delete(`_password`, 'user')
this.delete(`username`, 'user')
// de-nerf email if it's nerfed to the default registry
const email = this.get(`${nerfed}:email`, 'user')
if (email) {
this.set('email', email, 'user')
}
}
this.delete(`${nerfed}:_authToken`, 'user')
this.delete(`${nerfed}:_auth`, 'user')
Expand All @@ -706,28 +772,9 @@ class Config {

setCredentialsByURI (uri, { token, username, password, email, certfile, keyfile }) {
const nerfed = nerfDart(uri)
const def = nerfDart(this.get('registry'))

if (def === nerfed) {
// remove old style auth info not limited to a single registry
this.delete('_password', 'user')
this.delete('username', 'user')
this.delete('_auth', 'user')
this.delete('_authtoken', 'user')
this.delete('-authtoken', 'user')
this.delete('_authToken', 'user')
}

// email used to be nerfed always. if we're using the default
// registry, de-nerf it.
if (nerfed === def) {
email = email ||
this.get('email', 'user') ||
this.get(`${nerfed}:email`, 'user')
if (email) {
this.set('email', email, 'user')
}
}
// email is either provided, a top level key, or nothing
email = email || this.get('email', 'user')

// field that hasn't been used as documented for a LONG time,
// and as of npm 7.10.0, isn't used at all. We just always
Expand Down Expand Up @@ -765,15 +812,17 @@ class Config {
// this has to be a bit more complicated to support legacy data of all forms
getCredentialsByURI (uri) {
const nerfed = nerfDart(uri)
const def = nerfDart(this.get('registry'))
const creds = {}

const deprecatedAuthWarning = [
'`_auth`, `_authToken`, `username` and `_password` must be scoped to a registry.',
'see `npm help npmrc` for more information.',
].join(' ')

// email is handled differently, it used to always be nerfed and now it never should be
// if it's set nerfed to the default registry, then we copy it to the unnerfed key
// TODO: evaluate removing 'email' from the credentials object returned here
const email = this.get(`${nerfed}:email`) || this.get('email')
if (email) {
if (nerfed === def) {
this.set('email', email, 'user')
}
creds.email = email
}

Expand All @@ -785,13 +834,8 @@ class Config {
// cert/key may be used in conjunction with other credentials, thus no `return`
}

const defaultToken = nerfDart(this.get('registry')) && this.get('_authToken')
const tokenReg = this.get(`${nerfed}:_authToken`) || defaultToken

const tokenReg = this.get(`${nerfed}:_authToken`)
if (tokenReg) {
if (tokenReg === defaultToken) {
log.warn('config', deprecatedAuthWarning)
}
creds.token = tokenReg
return creds
}
Expand All @@ -816,37 +860,7 @@ class Config {
return creds
}

// at this point, we can only use the values if the URI is the
// default registry.
const defaultNerf = nerfDart(this.get('registry'))
if (nerfed !== defaultNerf) {
return creds
}

const userDef = this.get('username')
const passDef = this.get('_password')
if (userDef && passDef) {
log.warn('config', deprecatedAuthWarning)
creds.username = userDef
creds.password = Buffer.from(passDef, 'base64').toString('utf8')
const auth = `${creds.username}:${creds.password}`
creds.auth = Buffer.from(auth, 'utf8').toString('base64')
return creds
}

// Handle the old-style _auth=<base64> style for the default
// registry, if set.
const auth = this.get('_auth')
if (!auth) {
return creds
}

log.warn('config', deprecatedAuthWarning)
const authDecode = Buffer.from(auth, 'base64').toString('utf8')
const authSplit = authDecode.split(':')
creds.username = authSplit.shift()
creds.password = authSplit.join(':')
creds.auth = auth
// at this point, nothing else is usable so just return what we do have
return creds
}

Expand Down
Loading

0 comments on commit 344ccd3

Please sign in to comment.