feat: add administrators, require administrator perms to change server config in webui #628

Merged
Sainan merged 6 commits from permissions into main 2024-12-23 13:44:01 -08:00
11 changed files with 174 additions and 112 deletions

View File

@ -8,6 +8,7 @@
"myAddress": "localhost",
"httpPort": 80,
"httpsPort": 443,
"administratorNames": [],
"autoCreateAccount": true,
"skipStoryModeChoice": true,
"skipTutorial": true,

View File

@ -6,7 +6,7 @@ import buildConfig from "@/static/data/buildConfig.json";
import { toLoginRequest } from "@/src/helpers/loginHelpers";
import { Account } from "@/src/models/loginModel";
import { createAccount, isCorrectPassword } from "@/src/services/loginService";
import { createAccount, isCorrectPassword, isNameTaken } from "@/src/services/loginService";
import { IDatabaseAccountJson, ILoginResponse } from "@/src/types/loginTypes";
import { DTLS, groups, HUB, platformCDNs } from "@/static/fixed_responses/login_static";
import { logger } from "@/src/utils/logger";
@ -26,10 +26,19 @@ export const loginController: RequestHandler = async (request, response) => {
if (!account && config.autoCreateAccount && loginRequest.ClientType != "webui") {
try {
const nameFromEmail = loginRequest.email.substring(0, loginRequest.email.indexOf("@"));
let name = nameFromEmail;
if (await isNameTaken(name)) {
let suffix = 0;
do {
++suffix;
name = nameFromEmail + suffix;
} while (await isNameTaken(name));
}
const newAccount = await createAccount({
email: loginRequest.email,
password: loginRequest.password,
DisplayName: loginRequest.email.substring(0, loginRequest.email.indexOf("@")),
DisplayName: name,
CountryCode: loginRequest.lang.toUpperCase(),
ClientType: loginRequest.ClientType,
CrossPlatformAllowed: true,

View File

@ -1,14 +1,16 @@
import { toCreateAccount, toDatabaseAccount } from "@/src/helpers/customHelpers/customHelpers";
import { createAccount } from "@/src/services/loginService";
import { createAccount, isNameTaken } from "@/src/services/loginService";
import { RequestHandler } from "express";
const createAccountController: RequestHandler = async (req, res) => {
const createAccountData = toCreateAccount(req.body);
const databaseAccount = toDatabaseAccount(createAccountData);
const account = await createAccount(databaseAccount);
res.json(account);
if (await isNameTaken(createAccountData.DisplayName)) {
res.status(409).json("Name already in use");
} else {
const databaseAccount = toDatabaseAccount(createAccountData);
const account = await createAccount(databaseAccount);
res.json(account);
}
};
export { createAccountController };

View File

@ -1,8 +1,14 @@
import { RequestHandler } from "express";
import { config } from "@/src/services/configService";
import { getAccountForRequest, isAdministrator } from "@/src/services/loginService";
const getConfigDataController: RequestHandler = (_req, res) => {
res.json(config);
const getConfigDataController: RequestHandler = async (req, res) => {
const account = await getAccountForRequest(req);
if (isAdministrator(account)) {
res.json(config);
} else {
res.status(401).end();
}
};
export { getConfigDataController };

View File

@ -1,12 +1,16 @@
import { RequestHandler } from "express";
import { getAccountForRequest } from "@/src/services/loginService";
import { getAccountForRequest, isNameTaken } from "@/src/services/loginService";
export const renameAccountController: RequestHandler = async (req, res) => {
const account = await getAccountForRequest(req);
if (typeof req.query.newname == "string") {
account.DisplayName = req.query.newname;
await account.save();
res.end();
if (await isNameTaken(req.query.newname)) {
res.status(409).json("Name already in use");
} else {
account.DisplayName = req.query.newname;
await account.save();
res.end();
}
} else {
res.status(400).end();
}

View File

@ -1,9 +1,15 @@
import { RequestHandler } from "express";
import { updateConfig } from "@/src/services/configService";
import { getAccountForRequest, isAdministrator } from "@/src/services/loginService";
const updateConfigDataController: RequestHandler = async (req, res) => {
await updateConfig(String(req.body));
res.end();
const account = await getAccountForRequest(req);
if (isAdministrator(account)) {
await updateConfig(String(req.body));
res.end();
} else {
res.status(401).end();
}
};
coderabbitai[bot] commented 2024-12-23 10:40:43 -08:00 (Migrated from github.com)
Review

🛠️ Refactor suggestion

Consider error handling for config updates.

While the admin check is performed, consider handling potential errors if updateConfig throws an exception. Logging errors or sending a 500 status can help in diagnosing failed updates.

_:hammer_and_wrench: Refactor suggestion_ **Consider error handling for config updates.** While the admin check is performed, consider handling potential errors if updateConfig throws an exception. Logging errors or sending a 500 status can help in diagnosing failed updates. <!-- This is an auto-generated comment by CodeRabbit -->
export { updateConfigDataController };

View File

@ -24,7 +24,7 @@ const databaseAccountSchema = new Schema<IDatabaseAccountJson>(
{
email: { type: String, required: true, unique: true },
password: { type: String, required: true },
DisplayName: { type: String, required: true },
DisplayName: { type: String, required: true, unique: true },
CountryCode: { type: String, required: true },
ClientType: { type: String },
CrossPlatformAllowed: { type: Boolean, required: true },

View File

@ -14,6 +14,13 @@ fs.watchFile(configPath, () => {
amnesia = false;
} else {
logger.info("Detected a change to config.json, reloading its contents.");
// Set all values to undefined now so if the new config.json omits some fields that were previously present, it's correct in-memory.
for (const key of Object.keys(config)) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
(config as any)[key] = undefined;
}
Object.assign(config, JSON.parse(fs.readFileSync(configPath, "utf-8")));
}
});
@ -25,6 +32,7 @@ interface IConfig {
httpPort?: number;
httpsPort?: number;
myIrcAddresses?: string[];
administratorNames?: string[];
autoCreateAccount?: boolean;
skipStoryModeChoice?: boolean;
skipTutorial?: boolean;

View File

@ -2,16 +2,21 @@ import { Account } from "@/src/models/loginModel";
import { createInventory } from "@/src/services/inventoryService";
import { IDatabaseAccount, IDatabaseAccountJson } from "@/src/types/loginTypes";
import { createShip } from "./shipService";
import { Types } from "mongoose";
import { Document, Types } from "mongoose";
import { Loadout } from "@/src/models/inventoryModels/loadoutModel";
import { PersonalRooms } from "@/src/models/personalRoomsModel";
import new_personal_rooms from "@/static/fixed_responses/personalRooms.json";
import { Request } from "express";
import { config } from "@/src/services/configService";
export const isCorrectPassword = (requestPassword: string, databasePassword: string): boolean => {
return requestPassword === databasePassword;
};
export const isNameTaken = async (name: string): Promise<boolean> => {
return !!(await Account.findOne({ DisplayName: name }));
};
coderabbitai[bot] commented 2024-12-23 10:40:43 -08:00 (Migrated from github.com)
Review

💡 Codebase verification

Based on the gathered information, I can now provide the final response:

Add unique constraint for DisplayName to prevent race conditions
The DisplayName field in the Account schema lacks a unique constraint, while the current implementation relies only on the isNameTaken check. This can lead to race conditions where two concurrent requests might pass the check but conflict during save. Add unique: true to the DisplayName field in the schema to enforce uniqueness at the database level.

  • In src/models/loginModel.ts, update the schema definition:
DisplayName: { type: String, required: true, unique: true },
🔗 Analysis chain

Ensure concurrency safety.
Though the database query for checking if a name is taken (with “findOne”) is correct, be aware of potential race conditions when two requests attempt to create the same name concurrently. You may want to handle such cases or at least confirm the system’s acceptable collision behavior.


Let me gather more information about the codebase to understand how the isNameTaken function is used and if there are any existing concurrency controls.


Let me search specifically for the Account schema definition to check if there are any database-level constraints.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for usages of isNameTaken function
rg "isNameTaken" -B 2 -A 2

# Look for any unique/index constraints on DisplayName in schema definitions
rg "DisplayName.*unique" -B 2 -A 2

# Search for mongoose schema definitions related to Account
ast-grep --pattern 'new Schema({
  $$$
})'

Length of output: 3589


Script:

#!/bin/bash
# Search for Account model/schema definition
rg "Account.*Schema" -B 5 -A 10

# Look for any index definitions related to DisplayName
rg "index.*DisplayName" -B 2 -A 2

# Search for the Account model file content
cat src/models/loginModel.ts

Length of output: 3387

_:bulb: Codebase verification_ Based on the gathered information, I can now provide the final response: **Add unique constraint for DisplayName to prevent race conditions** The `DisplayName` field in the Account schema lacks a unique constraint, while the current implementation relies only on the `isNameTaken` check. This can lead to race conditions where two concurrent requests might pass the check but conflict during save. Add `unique: true` to the DisplayName field in the schema to enforce uniqueness at the database level. - In `src/models/loginModel.ts`, update the schema definition: ```typescript DisplayName: { type: String, required: true, unique: true }, ``` <details> <summary>🔗 Analysis chain</summary> **Ensure concurrency safety.** Though the database query for checking if a name is taken (with “findOne”) is correct, be aware of potential race conditions when two requests attempt to create the same name concurrently. You may want to handle such cases or at least confirm the system’s acceptable collision behavior. --- Let me gather more information about the codebase to understand how the `isNameTaken` function is used and if there are any existing concurrency controls. --- Let me search specifically for the Account schema definition to check if there are any database-level constraints. </details> <details> <summary>🏁 Scripts executed</summary> The following scripts were executed for the analysis: Error: No shell scripts found --- Script: ```shell #!/bin/bash # Search for usages of isNameTaken function rg "isNameTaken" -B 2 -A 2 # Look for any unique/index constraints on DisplayName in schema definitions rg "DisplayName.*unique" -B 2 -A 2 # Search for mongoose schema definitions related to Account ast-grep --pattern 'new Schema({ $$$ })' ``` Length of output: 3589 --- Script: ```shell #!/bin/bash # Search for Account model/schema definition rg "Account.*Schema" -B 5 -A 10 # Look for any index definitions related to DisplayName rg "index.*DisplayName" -B 2 -A 2 # Search for the Account model file content cat src/models/loginModel.ts ``` Length of output: 3387 </details> <!-- This is an auto-generated comment by CodeRabbit -->
export const createAccount = async (accountData: IDatabaseAccount): Promise<IDatabaseAccountJson> => {
const account = new Account(accountData);
try {
@ -44,20 +49,21 @@ export const createPersonalRooms = async (accountId: Types.ObjectId, shipId: Typ
await personalRooms.save();
};
export const getAccountForRequest = async (req: Request) => {
// eslint-disable-next-line @typescript-eslint/ban-types
type TAccountDocument = Document<unknown, {}, IDatabaseAccountJson> &
IDatabaseAccountJson & { _id: Types.ObjectId; __v: number };
export const getAccountForRequest = async (req: Request): Promise<TAccountDocument> => {
if (!req.query.accountId) {
throw new Error("Request is missing accountId parameter");
}
if (!req.query.nonce || parseInt(req.query.nonce as string) === 0) {
throw new Error("Request is missing nonce parameter");
}
const account = await Account.findOne(
{
_id: req.query.accountId,
Nonce: req.query.nonce
},
"_id"
);
const account = await Account.findOne({
_id: req.query.accountId,
Nonce: req.query.nonce
});
if (!account) {
throw new Error("Invalid accountId-nonce pair");
}
@ -67,3 +73,7 @@ export const getAccountForRequest = async (req: Request) => {
export const getAccountIdForRequest = async (req: Request): Promise<string> => {
return (await getAccountForRequest(req))._id.toString();
};
export const isAdministrator = (account: TAccountDocument): boolean => {
return !!config.administratorNames?.find(x => x == account.DisplayName);
};

View File

@ -198,75 +198,80 @@
<div class="col-lg-4">
<div class="card mb-4">
<h5 class="card-header">Server</h5>
<form class="card-body" onsubmit="doChangeSettings();return false;">
<div class="form-check">
<input class="form-check-input" type="checkbox" id="skipStoryModeChoice" />
<label class="form-check-label" for="skipStoryModeChoice">Skip Story Mode Choice</label>
<div class="card-body">
<div id="server-settings-no-perms" class="d-none">
<p>You must be an administrator to use this feature. To become an administrator, add <code>"<span class="displayname"></span>"</code> to <code>administratorNames</code> in the config.json.</p>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="skipTutorial" />
<label class="form-check-label" for="skipTutorial">Skip Tutorial</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="skipAllDialogue" />
<label class="form-check-label" for="skipAllDialogue">Skip All Dialogue</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllScans" />
<label class="form-check-label" for="unlockAllScans">Unlock All Scans</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllMissions" />
<label class="form-check-label" for="unlockAllMissions">Unlock All Missions</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllQuests" />
<label class="form-check-label" for="unlockAllQuests">Unlock All Quests</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="completeAllQuests" />
<label class="form-check-label" for="completeAllQuests">Complete All Quests</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="infiniteCredits" />
<label class="form-check-label" for="infiniteCredits">Infinite Credits</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="infinitePlatinum" />
<label class="form-check-label" for="infinitePlatinum">Infinite Platinum</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllShipFeatures" />
<label class="form-check-label" for="unlockAllShipFeatures">Unlock All Ship Features</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllShipDecorations" />
<label class="form-check-label" for="unlockAllShipDecorations">Unlock All Ship Decorations</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllFlavourItems" />
<label class="form-check-label" for="unlockAllFlavourItems">
Unlock All <abbr title="Animation Sets, Glyphs, Plattes, etc.">Flavor Items</abbr>
</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllSkins" />
<label class="form-check-label" for="unlockAllSkins">Unlock All Skins</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="universalPolarityEverywhere" />
<label class="form-check-label" for="universalPolarityEverywhere">
Universal Polarity Everywhere
</label>
</div>
<div class="form-group mt-2">
<label class="form-label" for="spoofMasteryRank">
Spoofed Mastery Rank (-1 to disable)
</label>
<input class="form-control" id="spoofMasteryRank" type="number" min="-1" />
</div>
<button class="btn btn-primary mt-3" type="submit">Save Settings</button>
</form>
<form id="server-settings" class="d-none" onsubmit="doChangeSettings();return false;">
<div class="form-check">
<input class="form-check-input" type="checkbox" id="skipStoryModeChoice" />
<label class="form-check-label" for="skipStoryModeChoice">Skip Story Mode Choice</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="skipTutorial" />
<label class="form-check-label" for="skipTutorial">Skip Tutorial</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="skipAllDialogue" />
<label class="form-check-label" for="skipAllDialogue">Skip All Dialogue</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllScans" />
<label class="form-check-label" for="unlockAllScans">Unlock All Scans</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllMissions" />
<label class="form-check-label" for="unlockAllMissions">Unlock All Missions</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllQuests" />
<label class="form-check-label" for="unlockAllQuests">Unlock All Quests</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="completeAllQuests" />
<label class="form-check-label" for="completeAllQuests">Complete All Quests</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="infiniteCredits" />
<label class="form-check-label" for="infiniteCredits">Infinite Credits</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="infinitePlatinum" />
<label class="form-check-label" for="infinitePlatinum">Infinite Platinum</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllShipFeatures" />
<label class="form-check-label" for="unlockAllShipFeatures">Unlock All Ship Features</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllShipDecorations" />
<label class="form-check-label" for="unlockAllShipDecorations">Unlock All Ship Decorations</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllFlavourItems" />
<label class="form-check-label" for="unlockAllFlavourItems">
Unlock All <abbr title="Animation Sets, Glyphs, Plattes, etc.">Flavor Items</abbr>
</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="unlockAllSkins" />
<label class="form-check-label" for="unlockAllSkins">Unlock All Skins</label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" id="universalPolarityEverywhere" />
<label class="form-check-label" for="universalPolarityEverywhere">
Universal Polarity Everywhere
</label>
</div>
<div class="form-group mt-2">
<label class="form-label" for="spoofMasteryRank">
Spoofed Mastery Rank (-1 to disable)
</label>
<input class="form-control" id="spoofMasteryRank" type="number" min="-1" max="65535" />
</div>
<button class="btn btn-primary mt-3" type="submit">Save Settings</button>
</form>
</div>
</div>
</div>
<div class="col-lg-4">

View File

@ -792,7 +792,7 @@ const uiConfigs = [
];
function doChangeSettings() {
fetch("/custom/config")
fetch("/custom/config?" + window.authz)
.then(response => response.json())
.then(json => {
for (const i of uiConfigs) {
@ -810,7 +810,7 @@ function doChangeSettings() {
}
}
$.post({
url: "/custom/config",
url: "/custom/config?" + window.authz,
contentType: "text/plain",
data: JSON.stringify(json, null, 2)
});
@ -820,23 +820,34 @@ function doChangeSettings() {
// Cheats route
single.getRoute("/webui/cheats").on("beforeload", function () {
fetch("/custom/config")
.then(response => response.json())
.then(json =>
Object.entries(json).forEach(entry => {
const [key, value] = entry;
var x = document.getElementById(`${key}`);
if (x != null) {
if (x.type == "checkbox") {
if (value === true) {
x.setAttribute("checked", "checked");
}
} else if (x.type == "number") {
x.setAttribute("value", `${value}`);
}
let interval;
interval = setInterval(() => {
if (window.authz) {
clearInterval(interval);
fetch("/custom/config?" + window.authz).then(res => {
if (res.status == 200) {
$("#server-settings").removeClass("d-none");
res.json().then(json =>
Object.entries(json).forEach(entry => {
const [key, value] = entry;
var x = document.getElementById(`${key}`);
if (x != null) {
if (x.type == "checkbox") {
if (value === true) {
x.setAttribute("checked", "checked");
}
} else if (x.type == "number") {
x.setAttribute("value", `${value}`);
}
}
})
);
} else {
$("#server-settings-no-perms").removeClass("d-none");
}
})
);
});
}
}, 10);
fetch("http://localhost:61558/ping", { mode: "no-cors" })
.then(() => {