Fix missing credentials when fetching tarballs (#622)

* Add support for absolute paths in npmHttpUtils.get

* Use npmHttpUtils.get in NpmHttpFetcher

* Add regression test

* Add additional regression test

* Add yet another regression test

Author: rtsao

packages/acceptance-tests/pkg-tests-core/sources/utils/tests.ts

@@ -233,7 +233,7 @@ export const startPackageServer = (): Promise<string> => {
                 [version as string]: Object.assign({}, packageVersionEntry!.packageJson, {
                   dist: {
                     shasum: await getPackageArchiveHash(name, version),
-                    tarball: localName === `unconventional-tarball`
+                    tarball: (localName === `unconventional-tarball` || localName === `private-unconventional-tarball`)
                       ? (await getPackageHttpArchivePath(name, version)).replace(`/-/`, `/tralala/`)
                       : await getPackageHttpArchivePath(name, version),
                   },
@@ -377,7 +377,7 @@ export const startPackageServer = (): Promise<string> => {
     } else if (match = url.match(/^\/(?:(@[^\/]+)\/)?([^@\/][^\/]*)\/(-|tralala)\/\2-(.*)\.tgz$/)) {
       const [, scope, localName, split, version] = match;
 
-      if (localName === `unconventional-tarball` && split === `-`)
+      if ((localName === `unconventional-tarball` || localName === `private-unconventional-tarball`) && split === `-`)
         return null;
 
       return {

packages/acceptance-tests/pkg-tests-fixtures/packages/@private__unconventional-tarball-1.0.0/index.js

@@ -0,0 +1,10 @@
+/* @flow */
+
+module.exports = require(`./package.json`);
+
+for (const key of [`dependencies`, `devDependencies`, `peerDependencies`]) {
+  for (const dep of Object.keys(module.exports[key] || {})) {
+    // $FlowFixMe The whole point of this file is to be dynamic
+    module.exports[key][dep] = require(dep);
+  }
+}

packages/acceptance-tests/pkg-tests-fixtures/packages/@private__unconventional-tarball-1.0.0/package.json

@@ -0,0 +1,4 @@
+{
+    "name": "@private/unconventional-tarball",
+    "version": "1.0.0"
+}

packages/acceptance-tests/pkg-tests-fixtures/packages/private-unconventional-tarball-1.0.0/index.js

@@ -0,0 +1,10 @@
+/* @flow */
+
+module.exports = require(`./package.json`);
+
+for (const key of [`dependencies`, `devDependencies`, `peerDependencies`]) {
+  for (const dep of Object.keys(module.exports[key] || {})) {
+    // $FlowFixMe The whole point of this file is to be dynamic
+    module.exports[key][dep] = require(dep);
+  }
+}

packages/acceptance-tests/pkg-tests-fixtures/packages/private-unconventional-tarball-1.0.0/package.json

@@ -0,0 +1,4 @@
+{
+    "name": "private-unconventional-tarball",
+    "version": "1.0.0"
+}

packages/acceptance-tests/pkg-tests-specs/sources/auth.test.js

@@ -155,6 +155,63 @@ describe(`Auth tests`, () => {
     ),
   );
 
+  test(
+    `it should install unconventional scoped packages which require authentication if an authentication token is configured`,
+    makeTemporaryEnv(
+      {
+        dependencies: {[`@private/unconventional-tarball`]: `1.0.0`},
+      },
+      async ({path, run, source}) => {
+        await writeFile(`${path}/.yarnrc.yml`, `npmAuthToken: "${AUTH_TOKEN}"\n`);
+
+        await run(`install`);
+
+        await expect(source(`require('@private/unconventional-tarball')`)).resolves.toMatchObject({
+          name: `@private/unconventional-tarball`,
+          version: `1.0.0`,
+        });
+      },
+    ),
+  );
+
+  test(
+    `it should install unconventional scoped packages which require authentication if npmAlwaysAuth is set to true and an authentication ident is present`,
+    makeTemporaryEnv(
+      {
+        dependencies: {[`@private/unconventional-tarball`]: `1.0.0`},
+      },
+      async ({path, run, source}) => {
+        await writeFile(`${path}/.yarnrc.yml`, `npmAuthIdent: "${AUTH_IDENT}"\nnpmAlwaysAuth: true\n`);
+
+        await run(`install`);
+
+        await expect(source(`require('@private/unconventional-tarball')`)).resolves.toMatchObject({
+          name: `@private/unconventional-tarball`,
+          version: `1.0.0`,
+        });
+      },
+    ),
+  );
+
+  test(
+    `it should install unconventional unscoped packages which require authentication if npmAlwaysAuth is set to true and an authentication ident is present`,
+    makeTemporaryEnv(
+      {
+        dependencies: {[`private-unconventional-tarball`]: `1.0.0`},
+      },
+      async ({path, run, source}) => {
+        await writeFile(`${path}/.yarnrc.yml`, `npmAuthIdent: "${AUTH_IDENT}"\nnpmAlwaysAuth: true\n`);
+
+        await run(`install`);
+
+        await expect(source(`require('private-unconventional-tarball')`)).resolves.toMatchObject({
+          name: `private-unconventional-tarball`,
+          version: `1.0.0`,
+        });
+      },
+    ),
+  );
+
   test(
     `it should fail when an invalid authenticaation token is used`,
     makeTemporaryEnv(

packages/plugin-npm/sources/NpmHttpFetcher.ts

@@ -1,10 +1,11 @@
 import {Fetcher, FetchOptions, MinimalFetchOptions} from '@yarnpkg/core';
 import {Locator, MessageName}                       from '@yarnpkg/core';
-import {httpUtils, structUtils, tgzUtils}           from '@yarnpkg/core';
+import {structUtils, tgzUtils}                      from '@yarnpkg/core';
 import semver                                       from 'semver';
 import {URL}                                        from 'url';
 
 import {PROTOCOL}                                   from './constants';
+import * as npmHttpUtils                            from './npmHttpUtils';
 
 export class NpmHttpFetcher implements Fetcher {
   supports(locator: Locator, opts: MinimalFetchOptions) {
@@ -50,8 +51,9 @@ export class NpmHttpFetcher implements Fetcher {
     if (archiveUrl === null)
       throw new Error(`Assertion failed: The archiveUrl querystring parameter should have been available`);
 
-    const sourceBuffer = await httpUtils.get(archiveUrl, {
+    const sourceBuffer = await npmHttpUtils.get(archiveUrl, {
       configuration: opts.project.configuration,
+      ident: locator,
     });
 
     return await tgzUtils.convertToZip(sourceBuffer, {

packages/plugin-npm/sources/npmHttpUtils.ts

@@ -47,7 +47,14 @@ export async function get(path: string, {configuration, headers, ident, authType
   if (auth)
     headers = {...headers, authorization: auth};
 
-  return await httpUtils.get(registry + path, {configuration, headers, ...rest});
+  let url;
+  try {
+    url = new URL(path);
+  } catch (e) {
+    url = new URL(registry + path);
+  }
+
+  return await httpUtils.get(url.href, {configuration, headers, ...rest});
 }
 
 export async function put(path: string, body: httpUtils.Body, {configuration, headers, ident, authType = AuthType.ALWAYS_AUTH, registry, ...rest}: Options) {