fix: trailing slash issue when finding auth in upmconfig (#94) (close #…

…29)

* feat: enforce registry-urls have no trailing slash

Registry urls now never have a trailing slash. This has the effect that they will also always be saved to upmconfig without trailing slash.

* feat: search npm auth with trailing slash

Search npm-auth for registry using both the regular registry url as well as with a trailing slash.

* refactor: remove unused import

src/cmd-login.ts

@@ -11,7 +11,7 @@ import {
 import { parseEnv } from "./utils/env";
 import { encodeBasicAuth } from "./types/upm-config";
 import { Base64 } from "./types/base64";
-import { RegistryUrl, removeTrailingSlash } from "./types/registry-url";
+import { RegistryUrl } from "./types/registry-url";
 import {
   promptEmail,
   promptPassword,
@@ -204,8 +204,6 @@ const writeUnityToken = async function (
   // Read config file
   const config = (await loadUpmConfig(configDir)) || {};
   if (!config.npmAuth) config.npmAuth = {};
-  // Remove ending slash of registry
-  registry = removeTrailingSlash(registry);
 
   if (basicAuth) {
     if (_auth === null) throw new Error("Auth is null");

src/types/pkg-manifest.ts

@@ -2,8 +2,9 @@ import { DomainName } from "./domain-name";
 import { SemanticVersion } from "./semantic-version";
 import { PackageUrl } from "./package-url";
 import { ScopedRegistry } from "./scoped-registry";
-import { RegistryUrl, removeTrailingSlash } from "./registry-url";
+import { RegistryUrl } from "./registry-url";
 import path from "path";
+import { removeTrailingSlash } from "../utils/string-utils";
 
 /**
  * The content of the package-manifest (manifest.json) of a Unity project

src/types/registry-url.ts

@@ -1,8 +1,10 @@
 import { Brand } from "ts-brand";
 import assert from "assert";
+import { removeTrailingSlash } from "../utils/string-utils";
 
 /**
  * A string of a http-based registry-url
+ * Registry-urls may not have trailing slashes
  */
 export type RegistryUrl = Brand<string, "RegistryUrl">;
 
@@ -11,7 +13,7 @@ export type RegistryUrl = Brand<string, "RegistryUrl">;
  * @param s The string
  */
 export function isRegistryUrl(s: string): s is RegistryUrl {
-  return /http(s?):\/\//.test(s);
+  return /http(s?):\/\/.*[^/]$/.test(s);
 }
 
 /**
@@ -24,15 +26,6 @@ export function registryUrl(s: string): RegistryUrl {
   return s;
 }
 
-/**
- * Removes trailing slash from a registry-url
- * @param registry The url
- */
-export function removeTrailingSlash(registry: RegistryUrl): RegistryUrl {
-  if (registry.endsWith("/")) return registry.slice(0, -1) as RegistryUrl;
-  return registry;
-}
-
 /**
  * Attempts to coerce a string into a registry-url, by
  * - Prepending http if it is missing
@@ -42,5 +35,6 @@ export function removeTrailingSlash(registry: RegistryUrl): RegistryUrl {
  */
 export function coerceRegistryUrl(s: string): RegistryUrl {
   if (!s.toLowerCase().startsWith("http")) s = "http://" + s;
-  return removeTrailingSlash(registryUrl(s));
+  s = removeTrailingSlash(s);
+  return registryUrl(s);
 }

src/types/upm-config.ts

@@ -51,7 +51,7 @@ export type UPMConfig = {
   /**
    * Authentication information organized by the registry they should be used on
    */
-  npmAuth?: Record<RegistryUrl, UpmAuth>;
+  npmAuth?: Record<string, UpmAuth>;
 };
 
 /**
@@ -131,7 +131,10 @@ export function tryGetAuthForRegistry(
   upmConfig: UPMConfig,
   registry: RegistryUrl
 ): NpmAuth | null {
-  const upmAuth = upmConfig.npmAuth![registry];
+  const upmAuth =
+    upmConfig.npmAuth?.[registry] ||
+    // As a backup search for the registry with trailing slash
+    upmConfig.npmAuth?.[registry + "/"];
   if (upmAuth === undefined) return null;
   const npmAuth = tryToNpmAuth(upmAuth);
   if (npmAuth === null) {

src/utils/string-utils.ts

@@ -6,3 +6,12 @@ export function trySplitAtFirstOccurrenceOf(
   if (elements.length === 1) return [s, undefined];
   return [elements[0]!, elements.slice(1).join(split)];
 }
+
+/**
+ * Removes trailing slash from a string
+ * @param s The string
+ */
+export function removeTrailingSlash<T extends string>(s: T): T {
+  if (s.endsWith("/")) return s.slice(0, -1) as T;
+  return s;
+}

test/test-cmd-login.ts

@@ -9,14 +9,14 @@ describe("cmd-login.ts", function () {
     it("should append token to empty content", async function () {
       generateNpmrcLines(
         "",
-        registryUrl("http://registry.npmjs.org/"),
+        registryUrl("http://registry.npmjs.org"),
         "123-456-789"
       ).should.deepEqual(["//registry.npmjs.org/:_authToken=123-456-789"]);
     });
     it("should append token to exist contents", async function () {
       generateNpmrcLines(
         "registry=https://registry.npmjs.org/",
-        registryUrl(" registry(http://registry.npmjs.org/"),
+        registryUrl(" registry(http://registry.npmjs.org"),
         "123-456-789"
       ).should.deepEqual([
         "registry=https://registry.npmjs.org/",
@@ -26,7 +26,7 @@ describe("cmd-login.ts", function () {
     it("should replace token to exist contents", async function () {
       generateNpmrcLines(
         "registry=https://registry.npmjs.org/\n//127.0.0.1:4873/:_authToken=blar-blar-blar\n//registry.npmjs.org/:_authToken=blar-blar-blar",
-        registryUrl("http://registry.npmjs.org/"),
+        registryUrl("http://registry.npmjs.org"),
         "123-456-789"
       ).should.deepEqual([
         "registry=https://registry.npmjs.org/",
@@ -44,12 +44,12 @@ describe("cmd-login.ts", function () {
     it("should quote token if necessary", async function () {
       generateNpmrcLines(
         "",
-        registryUrl("http://registry.npmjs.org/"),
+        registryUrl("http://registry.npmjs.org"),
         "=123-456-789="
       ).should.deepEqual(['//registry.npmjs.org/:_authToken="=123-456-789="']);
       generateNpmrcLines(
         "",
-        registryUrl("http://registry.npmjs.org/"),
+        registryUrl("http://registry.npmjs.org"),
         "?123-456-789?"
       ).should.deepEqual(['//registry.npmjs.org/:_authToken="?123-456-789?"']);
     });

test/test-registry-url.ts

@@ -1,15 +1,10 @@
 import { describe } from "mocha";
-import {
-  coerceRegistryUrl,
-  isRegistryUrl,
-  registryUrl,
-  removeTrailingSlash,
-} from "../src/types/registry-url";
+import { coerceRegistryUrl, isRegistryUrl } from "../src/types/registry-url";
 import should from "should";
 
 describe("registry-url", function () {
   describe("validation", function () {
-    ["http://registry.npmjs.org/", "https://registry.npmjs.org/"].forEach(
+    ["http://registry.npmjs.org", "https://registry.npmjs.org"].forEach(
       (input) =>
         it(`"${input}" should be registry-url`, function () {
           should(isRegistryUrl(input)).be.ok();
@@ -18,22 +13,14 @@ describe("registry-url", function () {
     [
       // Missing protocol
       "registry.npmjs.org/",
+      // Trailing slash
+      "http://registry.npmjs.org/",
     ].forEach((input) =>
       it(`"${input}" should not be registry-url`, function () {
         should(isRegistryUrl(input)).not.be.ok();
       })
     );
   });
-  describe("remove trailing slash", function () {
-    it("should remove trailing slash if it is exists", () =>
-      should(removeTrailingSlash(registryUrl("http://test.com/"))).be.equal(
-        "http://test.com"
-      ));
-    it("should do nothing if there is no trailing slash", () =>
-      should(removeTrailingSlash(registryUrl("http://test.com"))).be.equal(
-        "http://test.com"
-      ));
-  });
   describe("coerce", function () {
     it("should coerce urls without protocol", () =>
       should(coerceRegistryUrl("test.com")).be.equal("http://test.com"));

test/test-upm-config.ts

@@ -5,10 +5,14 @@ import {
   isBasicAuth,
   isTokenAuth,
   shouldAlwaysAuth,
+  tryGetAuthForRegistry,
   UpmAuth,
+  UPMConfig,
 } from "../src/types/upm-config";
 import should from "should";
 import { Base64 } from "../src/types/base64";
+import { registryUrl, RegistryUrl } from "../src/types/registry-url";
+import { NpmAuth } from "another-npm-registry-client";
 
 describe("upm-config", function () {
   describe("auth", function () {
@@ -75,5 +79,62 @@ describe("upm-config", function () {
         should(shouldAlwaysAuth(auth)).be.false();
       });
     });
+    describe("get auth for registry", function () {
+      it("should find auth for url without trailing slash", function () {
+        const url = registryUrl("http://registry.npmjs.com");
+        const expected: NpmAuth = {
+          alwaysAuth: false,
+          token: "This is not a valid token",
+        };
+        const config: UPMConfig = {
+          npmAuth: {
+            [url]: {
+              alwaysAuth: expected.alwaysAuth,
+              email: "real@email.com",
+              token: expected.token,
+            },
+          },
+        };
+
+        const actual = tryGetAuthForRegistry(config, url);
+        should(actual).be.deepEqual(expected);
+      });
+      it("should find auth for url with trailing slash", function () {
+        const url = "http://registry.npmjs.com/" as RegistryUrl;
+        const expected: NpmAuth = {
+          alwaysAuth: false,
+          token: "This is not a valid token",
+        };
+        const config: UPMConfig = {
+          npmAuth: {
+            [url]: {
+              alwaysAuth: expected.alwaysAuth,
+              email: "real@email.com",
+              token: expected.token,
+            },
+          },
+        };
+
+        const actual = tryGetAuthForRegistry(config, url);
+        should(actual).be.deepEqual(expected);
+      });
+      it("should not find auth for url that does not exist", function () {
+        const config: UPMConfig = {
+          npmAuth: {
+            ["http://registryA.com"]: {
+              alwaysAuth: false,
+              email: "real@email.com",
+              token: "This is not a valid token",
+            },
+          },
+        };
+
+        const actual = tryGetAuthForRegistry(
+          config,
+          registryUrl("http://registryB.com")
+        );
+        should(actual).be.null();
+      });
+    });
   });
 });