Support npmrc for private registries and auth #839

devongovett · 2016-10-12T08:45:39Z

Summary

This implements support for private registries configured in .npmrc files. It extracts the registry to use and the auth token info. Supports bearer tokens and basic auth, scoped registries, and the always-auth option. Should fix #606.

My flow experience is minimal, so if I messed something up or did something silly please let me know. 😀

Test plan

I tested with several npm configs against real live Sonatype Nexus, and Artifactory instances. Here are some cases:

# Default registry bearer token from npm
//registry.npmjs.org/:_authToken=TOKEN

# Default registry, basic auth token, always-auth turned on
registry=https://my-registry.com/
_auth=TOKEN
[email protected]
always-auth=true

# Scoped registry
@coralui:registry=https://my-registry.com/
//my-registry.com/:_password=PASSWORD
//my-registry.com/:username=USERNAME
//my-registry.com/:[email protected]
//my-registry.com/:always-auth=true

jbt · 2016-10-12T10:13:29Z

I haven't had the chance to check this thoroughly but as one of the people who spotted the bearer token leak in npm I thought I'd chime in early to make sure something similar doesn't creep in here.

From my super-quick testing on this PR it looks like if always-auth=true is present, the bearer token will get sent on every request with the token for the default registry (if present), rather than just those requests to the registry itself.

For example, if my .npmrc looks like this:

always-auth=true
//registry.npmjs.org/:_authToken=foo

... and I install an arbitrary tarball from somewhere other than registry.npmjs.org (e.g. yarn add http://localhost/foo.tgz), then Bearer foo gets sent to that other server. Which is Not Good.

This is just based on my quick 2 mins of testing so feel free to tell me I'm talking rubbish if I've missed something.

cpojer · 2016-10-12T10:17:05Z

Wow this is awesome. Nice work @devongovett and good to see you again. I haven't heard from you since the old MooTools/jQuery days!

zetoke · 2016-10-12T10:29:27Z

Should fix #606.

I think, this PR is also should fix #521
Or not?

thefourtheye · 2016-10-12T10:29:43Z

src/fetchers/tarball-fetcher.js


-    return this.config.requestManager.request({
-      url: ref,
+    return registry.request(ref, {


Possible NPE?

jbt · 2016-10-12T10:36:10Z

On reflection, I've just realised this may actually be the intended behaviour of always-auth, judging by tests in npm. If so, do please just ignore most of what I said above.

p0wl · 2016-10-12T11:59:58Z

this fixes #521 when you set always-auth to true via npm config set always-auth true \o/

awesome!

mrtnbroder · 2016-10-12T12:41:53Z

Great work!

However, still having some issues with this, where the requestUrl is sometimes mixed up like this:
https://registry.yarnpkg.com/https://registry.npmjs.org/

Also, one major problem I still see is that the saved yarn.lock saves the full resolved url with credentials in it, which is a no-no.

e.g:

# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1
"@ourpackagescope/PACKAGENAME@VERSION":
  version ...
  resolved "https://NPM_USERNAME:NPM_PASSWORD@OUR_PRIVATE_REGISTRY.com/@ourpackagescope/PACKAGENAME/-/@ourpackagescope/PACKAGENAME.....''
  dependencies:...

donaldpipowitch · 2016-10-12T13:15:54Z

Awesome PR. Does this pick up strict-ssl=false settings, too?

devongovett · 2016-10-12T16:34:56Z

@jbt yes, that is the desired behavior of always-auth. I tested with a registry that requires auth for the tarballs, for example.

@zetoke yes, looks like it should fix #521.

@mrtnbroder does it work if you add a trailing slash to the registry url in your config? If so, I guess we could add the slash if it's not there...

@donaldpipowitch no.

devongovett · 2016-10-12T16:55:20Z

Fixed an issue caused by a later PR #712, which changed from url.resolve to url.format. We need resolve in the case where the pathname is really a full url (when requesting tarballs). So only add the trailing / if it is not there already. Should fix your issue @mrtnbroder.

@zetoke Also added a check to ensure that we only add authorization if the request is going to the registry (not random tarballs).

also · 2016-10-12T18:51:16Z

This finally works with my ~/.npmrc, which looks like

registry = https://npm.example.com/nexus/content/groups/npm-all/
always-auth = true
_auth = basic-auth-string

🎉

sohara · 2016-10-12T21:19:54Z

In our case this allows yarn to resolve the packages in our private registry but when it actually goes to fetch them we get errors like:
error https://registry.npmjs.org/@condenast/x-package/-/x-package-2.2.0.tgz: invalid tar file

devongovett · 2016-10-12T21:25:39Z

@sohara can you provide your .npmrc file?

sohara · 2016-10-12T21:32:23Z

Sure

@condenast:registry=https://registry.npmjs.org/
//registry.npmjs.org/:_authToken=very-secret-code
always-auth=true
progress=false
python=python2.7

jgoz · 2016-10-12T22:05:44Z

In our case this allows yarn to resolve the packages in our private registry but when it actually goes to fetch them we get errors like:
error https://registry.npmjs.org/@condenast/x-package/-/x-package-2.2.0.tgz: invalid tar file

I'm getting the same error with our sinopia proxy/registry:

error https://path-to-sinopia/my-package/-/my-package-1.2.3.tgz: invalid tar file

.npmrc:

registry=https://path-to-sinopia/
always-auth=true
_auth=some-secret-key

devongovett · 2016-10-12T22:13:28Z

@jgoz @sohara can you verify that the tar file exists and actually is valid? Can you debug? Try putting a log here to see what the requestUrl and headers.authorization are.

jgoz · 2016-10-12T22:50:27Z

@devongovett

I did some debugging and I think I know how to fix it on my end, but there may be an issue when reading .npmrc files.

The issue is that I have always-auth=true in my local .npmrc (in the repository) and always-auth=false in my user .npmrc. This would be fine if the local file was loaded last, but the files are being loaded in this order:

User
Local
Local
User

So the final value for always-auth is false, which skips adding the necessary header. I can remove this config value from my user-level .npmrc, but the config files should only be read a single time (and in global-user-local order).

jgoz · 2016-10-12T23:19:38Z

@devongovett As a follow-up, I think the issue I'm seeing (can't speak for @sohara) is not related to this PR and should be fixed separately. I'll log an issue.

Edit: #949

devongovett · 2016-10-13T00:19:19Z

@jgoz yes, that sounds like a separate issue.

sohara · 2016-10-13T02:10:27Z

@devongovett I've added some debug logging as you've suggest and and I'm getting the following on stdout (logging first the header followed by the request url):

yarn install v0.15.7
info No lockfile found.
[1/4] Resolving packages...
headers.authorization  Bearer blah-secret-code-xxx-xxxxx
requestUrl  https://registry.npmjs.org/@condenast%2fcn-brands
... # other stuff
[2/4] Fetching packages...
headers.authorization  undefined
requestUrl  https://registry.npmjs.org/@condenast/cn-brands/-/cn-brands-4.2.2.tgz

I'm not sure where tarballs would be stored on the system in order to verify they are ok? I did not find any in ~/.yarn-cache. Would they be in a tpm dir or something?

nigelzor · 2016-10-13T18:08:55Z

src/registries/npm-registry.js


    const headers = {};
-    if (this.token) {
-      headers.authorization = `Bearer ${this.token}`;
+    if (this.token || (this.getOption('always-auth') && requestUrl.startsWith(registry))) {


Looks like there should also be a call to this.getScopedOption(registry.replace(/^https?:/, ''), 'always-auth') here.

You're right. Added in 35d617e.

zetoke · 2016-10-13T19:35:22Z

@cpojer What is blocking thing for merge this PR to upstream?
(I highlighted you, because you are the team member and participant of this thread 😄 )

cpojer · 2016-10-14T02:18:45Z

@zetoke @kittens and I are currently catching up on the repo (while also holding the fort internally at FB :D ). The post launch craziness made everything a bit unstable. Please give us some time until everything is settled down – if you'd like to help get the tests in a good state again, please send us a PR.

zetoke · 2016-10-14T10:41:54Z

@cpojer of course! Thanks a lot for your work!
It is the next step for javascript ecosystem and tooling!

bcoe · 2016-10-14T18:34:52Z

@devongovett love this approach! @kittens, @bestander what would we need to do to move this forward? Perhaps we can come to a consensus around this RFC? and get this out the door 💯

sebmck · 2016-10-15T16:53:04Z

Can you please rebase this against master, I'll give it a final review then we can get this in 😄

kribblo · 2016-10-16T22:18:14Z

Any chance this is getting published to NPM registry soonish?

bestander · 2016-10-16T22:33:23Z

we'll cut the branch Monday-Tuesday

On 16 October 2016 at 23:18, Kristoffer Lundén [email protected]
wrote:

Any chance this is getting published to NPM registry soonish?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#839 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACBdWJc6gq7YUxzgFI5_Ysb845f5yZNOks5q0qKxgaJpZM4KUghv
.

SEAPUNK · 2016-10-18T15:26:08Z

I installed yarn from the master branch just now, but it doesn't seem to be working for me. It seems to fetch the metadata correctly, but I get the invalid tar file error:

$ yarn install
yarn install v0.15.1
info No lockfile found.
[1/4] Resolving packages...
warning [email protected]: Use the `minico` package instead: It's the same thing!
[2/4] Fetching packages...
error An unexpected error occured, please open a bug report with the information provided in "/home/ivan/code/xbpf/packages/manager-frontend/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

yarn-error.log: https://gist.github.com/SEAPUNK/84025e425c858dfefb6eaed7b97207b0

My .npmrc looks like this:

registry=https://registry.npmjs.com/
//registry.npmjs.com/:_authToken=the-long-token

motss · 2016-10-18T17:34:29Z

@SEAPUNK Is registry="https://registry.npmjs.com//" with double slashes at the end going to work?

devongovett · 2016-10-18T17:37:10Z

@SEAPUNK remove the first line from your npmrc and it should work. Currently it replaces the https://registry.npmjs.com/ with https://registry.yarnpkg.com/ here, which confuses the check here.

SEAPUNK · 2016-10-18T17:37:58Z

@motss That just throws an early error:

Trace:
  TypeError: Cannot use 'in' operator to search for '^6.16.0' in undefined
      at /usr/lib/node_modules/yarn/lib/resolvers/registries/npm-resolver.js:62:24
      at next (native)
      at step (/usr/lib/node_modules/yarn/node_modules/babel-runtime/helpers/asyncToGenerator.js:17:30)
      at /usr/lib/node_modules/yarn/node_modules/babel-runtime/helpers/asyncToGenerator.js:35:14
      at Promise.F (/usr/lib/node_modules/yarn/node_modules/core-js/library/modules/_export.js:35:28)
      at /usr/lib/node_modules/yarn/node_modules/babel-runtime/helpers/asyncToGenerator.js:14:12
      at Function.findVersionInRegistryResponse (/usr/lib/node_modules/yarn/lib/resolvers/registries/npm-resolver.js:72:7)
      at /usr/lib/node_modules/yarn/lib/resolvers/registries/npm-resolver.js:89:34
      at next (native)
      at step (/usr/lib/node_modules/yarn/node_modules/babel-runtime/helpers/asyncToGenerator.js:17:30)

@devongovett

Trace:
  Error: https://registry.yarnpkg.com/@xbpf%2fsuppliers: Not found
      at Request.params.callback [as _callback] (/usr/lib/node_modules/yarn/lib/util/request-manager.js:294:18)
      at Request.self.callback (/usr/lib/node_modules/yarn/node_modules/request/request.js:187:22)
      at emitTwo (events.js:106:13)
      at Request.emit (events.js:191:7)
      at Request.<anonymous> (/usr/lib/node_modules/yarn/node_modules/request/request.js:1048:10)
      at emitOne (events.js:96:13)
      at Request.emit (events.js:188:7)
      at IncomingMessage.<anonymous> (/usr/lib/node_modules/yarn/node_modules/request/request.js:969:12)
      at emitNone (events.js:91:20)
      at IncomingMessage.emit (events.js:185:7)

SEAPUNK · 2016-10-18T17:40:42Z

@devongovett Wait, now it's giving me that error with the original npmrc. One sec, let me test a few things...

zetoke · 2016-10-18T17:41:01Z

@SEAPUNK I had the same trouble. But with fix from @devongovett it's working!

SEAPUNK · 2016-10-18T17:46:08Z

Yeah, that fix didn't work for me. @xbpf/suppliers is a private package.

SEAPUNK · 2016-10-18T17:47:26Z

I installed yarn 0.16.0 just now, same problems.

SEAPUNK · 2016-10-18T17:52:15Z

Not to mention that removing that first line from my npmrc breaks npm's install process.

devongovett · 2016-10-18T17:54:02Z

@SEAPUNK it's a bug. I will look at it when I have a chance.

SEAPUNK · 2016-10-18T17:54:21Z

Alright, thanks.

kribblo · 2016-10-18T19:22:22Z

Also getting the invalid tar file error with 0.16.0:

The .npmrc contains something like this, working with npm:

@mycompany:registry=https://npm.mycompany.com/
//npm.mycompany.com/:_authToken="COMPANYTOKEN"

And the private registry replies with "an invalid tar file" in the form of a JSON response:

{
    "error": "unregistered users are not allowed to access package @mycompany/company-package"
}

This pull request was supposed to add support for scoped registrys (which we use heavily), so is it the _authToken that's missing maybe (it's not in the examples)?

jameshartig · 2016-10-18T20:03:43Z

I'm having the same issue as @kribblo. When doing a tcpdump on the sinopia box I see that it sends the authorization token with the GET call to the package metadata (@levenlabs/js-lib in this case):

GET /@levenlabs%2fjs-lib HTTP/1.1
Connection: close
User-Agent: yarn/0.16.0 npm/? node/v4.4.7 linux x64
authorization: Bearer $token
accept-encoding: gzip, deflate
accept: application/json

But then when it fetches the tar it doesn't:

GET /@levenlabs%2fjs-lib/-/js-lib-0.3.24.tgz HTTP/1.1
Connection: close
User-Agent: yarn/0.16.0 npm/? node/v4.4.7 linux x64
accept-encoding: gzip, deflate

rexxars · 2016-10-18T21:16:11Z

Experiencing some weird issues.

My npmrc looks something like this:

@somescope:registry=https://npm.somescope.no
//npm.somescope.no/:_authToken=someSecretToken
//registry.npmjs.org/:_authToken=otherSecretToken

I went into request-manager.js and added a console.log of outgoing requests. Upon running yarn with modules both hosted on the private registry and the public, I'm seeing it use the public token instead of the private one:

{ url: 'https://npm.somescope.no/@somescope%2fsomemodule',
  headers: 
   { authorization: 'Bearer otherSecretToken' },

rexxars · 2016-10-18T21:31:16Z

It seems to indeed be related to a "missing" trailing slash, here.

If I add a trailing slash to the first entry in the array above, the resolving succeeds, though the tarball download fails (no token is sent).

It's a bit worrying that it seems to fall back to sending a private registry token to the public registry, however.

vn-jelli · 2016-10-19T00:53:57Z

Hi! How do you get to install version 0.16.0?
When I do brew install yarn, I only get version 0.15.1

Gpx · 2016-10-19T13:42:24Z

@vinngn1 brew update && brew upgrade yarn

vn-jelli · 2016-10-19T17:23:54Z

Able to install private package out of the box after the upgrade. Thank @Gpx for the upgrade instruction and @devongovett and others for the fix.

KidkArolis · 2016-11-04T14:06:38Z

I didn't read all of this thread, but I was wondering - howcome nobody ran into the issue where auth header is not getting sent when downloading tarballs: #1687.

kribblo · 2016-11-04T14:09:33Z

@KidkArolis #839 (comment) just a few comments above yours

KidkArolis · 2016-11-04T14:13:56Z

@kribblo yeah, thanks :) I take it the issue hasn't been resolved yet. I have a way to reproduce it: https://github.com/KidkArolis/yarn-scopes-issue. And a potential fix: #1666.

thefourtheye reviewed Oct 12, 2016

View reviewed changes

ashleygwilliams mentioned this pull request Oct 12, 2016

docs: RFC proposal to center talk about authentication around yarnpkg/rfcs#5

Closed

devongovett mentioned this pull request Oct 12, 2016

Handling of multiple npm servers #547

Open

sergeysova approved these changes Oct 13, 2016

View reviewed changes

also mentioned this pull request Oct 13, 2016

Cannot find packages in private npm registry #678

Closed

nigelzor reviewed Oct 13, 2016

View reviewed changes

palmerj3 mentioned this pull request Oct 15, 2016

Support private registries (non-auth) and fix git resolver #791

Closed

sebmck mentioned this pull request Oct 15, 2016

Use auth tokens for NPM registry requests #1086

Closed

devongovett mentioned this pull request Oct 17, 2016

Support for private packages #521

Closed

chicoxyzzy mentioned this pull request Oct 17, 2016

Please add support for private npm repos #1173

Closed

SEAPUNK mentioned this pull request Nov 16, 2016

Fix request authentication for scoped private registry tarballs #1666

Merged

jonaskello mentioned this pull request Nov 27, 2016

Yarn authentication not working verdaccio/verdaccio#74

Closed

pklinef mentioned this pull request Jan 20, 2017

DDF-2721: Replaced NPM with Yarn codice/ddf#1572

Merged

4 tasks

Support npmrc for private registries and auth #839

Support npmrc for private registries and auth #839

Uh oh!

Conversation

devongovett commented Oct 12, 2016

Uh oh!

jbt commented Oct 12, 2016

Uh oh!

cpojer commented Oct 12, 2016

Uh oh!

zetoke commented Oct 12, 2016

Uh oh!

thefourtheye Oct 12, 2016

Choose a reason for hiding this comment

Uh oh!

jbt commented Oct 12, 2016

Uh oh!

p0wl commented Oct 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrtnbroder commented Oct 12, 2016

Uh oh!

donaldpipowitch commented Oct 12, 2016

Uh oh!

devongovett commented Oct 12, 2016

Uh oh!

devongovett commented Oct 12, 2016

Uh oh!

also commented Oct 12, 2016

Uh oh!

sohara commented Oct 12, 2016

Uh oh!

devongovett commented Oct 12, 2016

Uh oh!

sohara commented Oct 12, 2016

Uh oh!

jgoz commented Oct 12, 2016

Uh oh!

devongovett commented Oct 12, 2016

Uh oh!

jgoz commented Oct 12, 2016

Uh oh!

jgoz commented Oct 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devongovett commented Oct 13, 2016

Uh oh!

sohara commented Oct 13, 2016

Uh oh!

nigelzor Oct 13, 2016

Choose a reason for hiding this comment

Uh oh!

devongovett Oct 13, 2016

Choose a reason for hiding this comment

Uh oh!

zetoke commented Oct 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cpojer commented Oct 14, 2016

Uh oh!

zetoke commented Oct 14, 2016

Uh oh!

bcoe commented Oct 14, 2016

Uh oh!

sebmck commented Oct 15, 2016

Uh oh!

kribblo commented Oct 16, 2016

Uh oh!

bestander commented Oct 16, 2016

Uh oh!

SEAPUNK commented Oct 18, 2016

Uh oh!

motss commented Oct 18, 2016

Uh oh!

devongovett commented Oct 18, 2016

Uh oh!

SEAPUNK commented Oct 18, 2016

Uh oh!

SEAPUNK commented Oct 18, 2016

Uh oh!

p0wl commented Oct 12, 2016 •

edited

Loading

jgoz commented Oct 12, 2016 •

edited

Loading

zetoke commented Oct 13, 2016 •

edited

Loading

SEAPUNK commented Oct 18, 2016 •

edited

Loading

kribblo commented Oct 18, 2016 •

edited

Loading

jameshartig commented Oct 18, 2016 •

edited

Loading