From a3840795627cc82a7fe8af47f31987e0f550f44f Mon Sep 17 00:00:00 2001 From: Michael C Date: Fri, 17 Feb 2023 22:28:23 -0500 Subject: [PATCH] more lenient privateIDUsername checks - disallow username = privateID - disallow username = other privateID on username table if length > minLength --- src/routes/setUsername.ts | 12 +--- test/cases/setUsername.ts | 60 ++-------------- test/cases/setUsernamePrivate.ts | 117 +++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 64 deletions(-) create mode 100644 test/cases/setUsernamePrivate.ts diff --git a/src/routes/setUsername.ts b/src/routes/setUsername.ts index e0beb9b..aa67fa1 100644 --- a/src/routes/setUsername.ts +++ b/src/routes/setUsername.ts @@ -33,14 +33,9 @@ export async function setUsername(req: Request, res: Response): Promise { - const userIDHash = await getHashCache(userID); + if (username == userID) return false; + if (username.length <= config.minUserIDLength) return true; // don't check for cross matches <= 30 characters const userNameHash = await getHashCache(username); - if (userIDHash == userNameHash) return false; - const sponsorTimeRow = await db.prepare("get", `SELECT "userID" FROM "sponsorTimes" WHERE "userID" = ? LIMIT 1`, [userNameHash]); const userNameRow = await db.prepare("get", `SELECT "userID" FROM "userNames" WHERE "userID" = ? LIMIT 1`, [userNameHash]); - if ((sponsorTimeRow || userNameRow)?.userID) return false; + if (userNameRow?.userID) return false; return true; } \ No newline at end of file diff --git a/test/cases/setUsername.ts b/test/cases/setUsername.ts index a3da56c..ab379b7 100644 --- a/test/cases/setUsername.ts +++ b/test/cases/setUsername.ts @@ -22,16 +22,6 @@ const user07PrivateUserID = "setUsername_07"; const username07 = "Username 07"; const user08PrivateUserID = "setUsername_08"; -// private = public cases -// user09 - username === privateID -const user09PrivateUserID = "setUsername_09"; -// user 10/11 - user 11 username === user 10 privateID -const user10PrivateUserID = "setUsername_10_collision"; -const username10 = "setUsername_10"; -const user11PrivateUserID = "setUsername_11"; -const user12PrivateUserID = "setUsername_12"; -const username12 = "Username 12"; - async function addUsername(userID: string, userName: string, locked = 0) { await db.prepare("run", 'INSERT INTO "userNames" ("userID", "userName", "locked") VALUES(?, ?, ?)', [userID, userName, locked]); await addLogUserNameChange(userID, userName); @@ -40,7 +30,7 @@ async function addUsername(userID: string, userName: string, locked = 0) { async function getUsernameInfo(userID: string): Promise<{ userName: string, locked: string}> { const row = await db.prepare("get", 'SELECT "userName", "locked" FROM "userNames" WHERE "userID" = ?', [userID]); if (!row) { - return null; + throw new Error("No username found"); } return row; } @@ -98,9 +88,6 @@ describe("setUsername", () => { await addUsername(getHash(user05PrivateUserID), username05, 0); await addUsername(getHash(user06PrivateUserID), username06, 0); await addUsername(getHash(user07PrivateUserID), username07, 1); - await addUsername(getHash(user10PrivateUserID), username10, 0); - // user11 skipped - await addUsername(getHash(user12PrivateUserID), username12, 0); }); it("Should be able to set username that has never been set", (done) => { @@ -249,47 +236,10 @@ describe("setUsername", () => { it("Should delete row if new username is same as publicID", (done) => { const publicID = getHash(user08PrivateUserID); postSetUserName(getHash(user08PrivateUserID), publicID) - .then(async () => { - const usernameInfo = await getUsernameInfo(getHash(user08PrivateUserID)); - assert.strictEqual(usernameInfo, null); - done(); - }) - .catch((err) => done(err)); - }); - - it("Should return error if trying to set username to privateID", (done) => { - const privateID = user09PrivateUserID; - postSetUserName(privateID, privateID) - .then(async (res) => { - assert.strictEqual(res.status, 400); - const usernameInfo = await getUsernameInfo(getHash(privateID)); - assert.strictEqual(usernameInfo, null); - done(); - }) - .catch((err) => done(err)); - }); - - it("Should return error if trying to set username to someone else's privateID", (done) => { - const privateID = user11PrivateUserID; - postSetUserName(privateID, user10PrivateUserID) - .then(async (res) => { - assert.strictEqual(res.status, 400); - const usernameInfo = await getUsernameInfo(getHash(privateID)); // user 10's privateID - assert.strictEqual(usernameInfo, null); - done(); - }) - .catch((err) => done(err)); - }); - - it("Should not return error if trying to set username to someone else's publicID", (done) => { - const privateID = user12PrivateUserID; - const user10PublicID = getHash(user10PrivateUserID); - postSetUserName(privateID, user10PublicID) - .then(async (res) => { - assert.strictEqual(res.status, 200); - const usernameInfo = await getUsernameInfo(getHash(privateID)); // user 10's publicID - assert.strictEqual(usernameInfo.userName, user10PublicID); - done(); + .then(() => { + getUsernameInfo(getHash(user08PrivateUserID)) + .then(usernameinfo => done(`Username should be deleted - ${usernameinfo})`)) + .catch(() => done()); }) .catch((err) => done(err)); }); diff --git a/test/cases/setUsernamePrivate.ts b/test/cases/setUsernamePrivate.ts new file mode 100644 index 0000000..c67e2a5 --- /dev/null +++ b/test/cases/setUsernamePrivate.ts @@ -0,0 +1,117 @@ +import { db } from "../../src/databases/databases"; +import { getHash } from "../../src/utils/getHash"; +import assert from "assert"; +import { client } from "../utils/httpClient"; +import { config } from "../../src/config"; +import sinon from "sinon"; + +const USERID_LIMIT = 30; + +// preexisting username with userid < Limit +const preExisting_underLimit = "preExisting_under"; +// preexisting username with userid > Limit +const preExisting_overLimit = `preExisting_over${"*".repeat(USERID_LIMIT)}`; +// new username to privateID < Limit +const newUser_underLimit = "newUser_under"; +// new username to privateID > Limit +const newUser_overLimit = `newUser_over${"*".repeat(USERID_LIMIT)}`; +// new username to someone else'e privateID +const otherUser = `otherUser${"*".repeat(USERID_LIMIT)}`; + + +const addUsername = async (userID: string, userName: string, locked = 0) => + await db.prepare("run", 'INSERT INTO "userNames" ("userID", "userName", "locked") VALUES(?, ?, ?)', [userID, userName, locked]); + +async function hasSetUsername(userID: string): Promise { + const row = await db.prepare("get", 'SELECT "userName", "locked" FROM "userNames" WHERE "userID" = ?', [userID]); + return Boolean(row); +} + +const endpoint = "/api/setUsername"; +const postSetUserName = (userID: string, username: string) => client({ + method: "POST", + url: endpoint, + params: { + userID, + username, + } +}); + +describe("setUsernamePrivate tests", () => { + // add preexisitng usernames + before(async () => { + await addUsername(getHash(preExisting_underLimit), preExisting_underLimit, 0); + await addUsername(getHash(preExisting_overLimit), preExisting_overLimit, 0); + }); + // stub minUserIDLength + before(() => sinon.stub(config, "minUserIDLength").value(USERID_LIMIT)); + after(() => sinon.restore()); + + it("Existing privateID = username under Limit should retreive successfully", (done) => { + const privateID = preExisting_underLimit; + hasSetUsername(getHash(privateID)) + .then((usernameInfo) => { + assert.ok(usernameInfo); + done(); + }); + }); + + it("Existing privateID = username over Limit should retreive successfully", (done) => { + const privateID = preExisting_overLimit; + hasSetUsername(getHash(privateID)) + .then((usernameInfo) => { + assert.ok(usernameInfo); + done(); + }); + }); + + it("Should return error if trying to set userID = username under limit", (done) => { + const privateID = newUser_underLimit; + postSetUserName(privateID, privateID) + .then(async (res) => { + assert.strictEqual(res.status, 400); + const usernameInfo = await hasSetUsername(getHash(privateID)); + assert.ok(!usernameInfo); + done(); + }) + .catch((err) => done(err)); + }); + + it("Should return error if trying to set username = other privateID over limit", (done) => { + const privateID = newUser_overLimit; + postSetUserName(privateID, privateID) + .then(async (res) => { + assert.strictEqual(res.status, 400); + const usernameInfo = await hasSetUsername(getHash(privateID)); + assert.ok(!usernameInfo); + done(); + }) + .catch((err) => done(err)); + }); + + it("Should return error if trying to set username = other privateID over limit", (done) => { + const privateID = otherUser; + const otherUserPrivate = preExisting_overLimit; + postSetUserName(privateID, otherUserPrivate) + .then(async (res) => { + assert.strictEqual(res.status, 400); + const usernameInfo = await hasSetUsername(getHash(privateID)); + assert.ok(!usernameInfo); + done(); + }) + .catch((err) => done(err)); + }); + + it("Should not return error if trying to set username = other privateID under limit", (done) => { + const privateID = otherUser; + const otherUserPrivate = preExisting_underLimit; + postSetUserName(privateID, otherUserPrivate) + .then(async (res) => { + assert.strictEqual(res.status, 200); + const usernameInfo = await hasSetUsername(getHash(privateID)); + assert.ok(usernameInfo); + done(); + }) + .catch((err) => done(err)); + }); +});