Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: trailing slash issue when finding auth in upmconfig #94

Merged
merged 3 commits into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/cmd-login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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");
Expand Down
3 changes: 2 additions & 1 deletion src/types/pkg-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 5 additions & 11 deletions src/types/registry-url.ts
Original file line number Diff line number Diff line change
@@ -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">;

Expand All @@ -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);
}

/**
Expand All @@ -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
Expand All @@ -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);
}
7 changes: 5 additions & 2 deletions src/types/upm-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
};

/**
Expand Down Expand Up @@ -128,7 +128,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) {
Expand Down
9 changes: 9 additions & 0 deletions src/utils/string-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
10 changes: 5 additions & 5 deletions test/test-cmd-login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/",
Expand All @@ -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/",
Expand All @@ -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?"']);
});
Expand Down
21 changes: 4 additions & 17 deletions test/test-registry-url.ts
Original file line number Diff line number Diff line change
@@ -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();
Expand All @@ -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"));
Expand Down
61 changes: 61 additions & 0 deletions test/test-upm-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -70,5 +74,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();
});
});
});
});