Skip to content

Commit

Permalink
fix(publish): follow configs for registry auth
Browse files Browse the repository at this point in the history
The "do you have auth configured" only takes into consideration a hard-coded
"registry" config, which means that if you don't have auth configured for
the npm registry, but you do for the one you have tied to a scope elsewhere
in your npmrc, we would erroneously tell you that to add a token.

This uses the same method that the rest of the publish chain uses to determine
which registry it would be publishing to, then sees if you have auth for THAT registry.

Because that other function does a hard fallback to the npm registry, there is no more
need for the exception we throw if you do not have one configured.  Also, the npm cli
already defaults that config item, so you can't even set it to a falsey value if you
wanted
  • Loading branch information
wraithgar authored and ruyadorno committed Feb 5, 2021
1 parent d443939 commit df596bf
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 75 deletions.
41 changes: 18 additions & 23 deletions lib/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const libpub = require('libnpmpublish').publish
const runScript = require('@npmcli/run-script')
const pacote = require('pacote')
const npa = require('npm-package-arg')
const npmFetch = require('npm-registry-fetch')

const npm = require('./npm.js')
const output = require('./utils/output.js')
Expand Down Expand Up @@ -71,27 +72,12 @@ const publish_ = async (arg, opts) => {
// you can publish name@version, ./foo.tgz, etc.
// even though the default is the 'file:.' cwd.
const spec = npa(arg)
const manifest = await getManifest(spec, opts)

let manifest = await getManifest(spec, opts)

if (manifest.publishConfig)
Object.assign(opts, publishConfigToOpts(manifest.publishConfig))

const { registry } = opts
if (!registry) {
throw Object.assign(new Error('No registry specified.'), {
code: 'ENOREGISTRY',
})
}

if (!dryRun) {
const creds = npm.config.getCredentialsByURI(registry)
if (!creds.token && !creds.username) {
throw Object.assign(new Error('This command requires you to be logged in.'), {
code: 'ENEEDAUTH',
})
}
}

// only run scripts for directory type publishes
if (spec.type === 'directory') {
await runScript({
Expand All @@ -105,18 +91,27 @@ const publish_ = async (arg, opts) => {
const tarballData = await pack(spec, opts)
const pkgContents = await getContents(manifest, tarballData)

// The purpose of re-reading the manifest is in case it changed,
// so that we send the latest and greatest thing to the registry
// note that publishConfig might have changed as well!
manifest = await getManifest(spec, opts)
if (manifest.publishConfig)
Object.assign(opts, publishConfigToOpts(manifest.publishConfig))

// note that logTar calls npmlog.notice(), so if we ARE in silent mode,
// this will do nothing, but we still want it in the debuglog if it fails.
if (!json)
logTar(pkgContents, { log, unicode })

if (!dryRun) {
// The purpose of re-reading the manifest is in case it changed,
// so that we send the latest and greatest thing to the registry
// note that publishConfig might have changed as well!
const manifest = await getManifest(spec, opts)
if (manifest.publishConfig)
Object.assign(opts, publishConfigToOpts(manifest.publishConfig))
const resolved = npa.resolve(manifest.name, manifest.version)
const registry = npmFetch.pickRegistry(resolved, opts)
const creds = npm.config.getCredentialsByURI(registry)
if (!creds.token && !creds.username) {
throw Object.assign(new Error('This command requires you to be logged in.'), {
code: 'ENEEDAUTH',
})
}
await otplease(opts, opts => libpub(manifest, tarballData, opts))
}

Expand Down
Loading

0 comments on commit df596bf

Please sign in to comment.