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", "myAddress": "localhost",
"httpPort": 80, "httpPort": 80,
"httpsPort": 443, "httpsPort": 443,
"administratorNames": [],
"autoCreateAccount": true, "autoCreateAccount": true,
"skipStoryModeChoice": true, "skipStoryModeChoice": true,
"skipTutorial": true, "skipTutorial": true,

View File

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

View File

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

View File

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

View File

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

View File

@ -1,9 +1,15 @@
import { RequestHandler } from "express"; import { RequestHandler } from "express";
import { updateConfig } from "@/src/services/configService"; import { updateConfig } from "@/src/services/configService";
import { getAccountForRequest, isAdministrator } from "@/src/services/loginService";
const updateConfigDataController: RequestHandler = async (req, res) => { const updateConfigDataController: RequestHandler = async (req, res) => {
await updateConfig(String(req.body)); const account = await getAccountForRequest(req);
res.end(); 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 }; export { updateConfigDataController };

View File

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

View File

@ -14,6 +14,13 @@ fs.watchFile(configPath, () => {
amnesia = false; amnesia = false;
} else { } else {
logger.info("Detected a change to config.json, reloading its contents."); 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"))); Object.assign(config, JSON.parse(fs.readFileSync(configPath, "utf-8")));
} }
}); });
@ -25,6 +32,7 @@ interface IConfig {
httpPort?: number; httpPort?: number;
httpsPort?: number; httpsPort?: number;
myIrcAddresses?: string[]; myIrcAddresses?: string[];
administratorNames?: string[];
autoCreateAccount?: boolean; autoCreateAccount?: boolean;
skipStoryModeChoice?: boolean; skipStoryModeChoice?: boolean;
skipTutorial?: boolean; skipTutorial?: boolean;

View File

@ -2,16 +2,21 @@ import { Account } from "@/src/models/loginModel";
import { createInventory } from "@/src/services/inventoryService"; import { createInventory } from "@/src/services/inventoryService";
import { IDatabaseAccount, IDatabaseAccountJson } from "@/src/types/loginTypes"; import { IDatabaseAccount, IDatabaseAccountJson } from "@/src/types/loginTypes";
import { createShip } from "./shipService"; import { createShip } from "./shipService";
import { Types } from "mongoose"; import { Document, Types } from "mongoose";
import { Loadout } from "@/src/models/inventoryModels/loadoutModel"; import { Loadout } from "@/src/models/inventoryModels/loadoutModel";
import { PersonalRooms } from "@/src/models/personalRoomsModel"; import { PersonalRooms } from "@/src/models/personalRoomsModel";
import new_personal_rooms from "@/static/fixed_responses/personalRooms.json"; import new_personal_rooms from "@/static/fixed_responses/personalRooms.json";
import { Request } from "express"; import { Request } from "express";
import { config } from "@/src/services/configService";
export const isCorrectPassword = (requestPassword: string, databasePassword: string): boolean => { export const isCorrectPassword = (requestPassword: string, databasePassword: string): boolean => {
return requestPassword === databasePassword; 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> => { export const createAccount = async (accountData: IDatabaseAccount): Promise<IDatabaseAccountJson> => {
const account = new Account(accountData); const account = new Account(accountData);
try { try {
@ -44,20 +49,21 @@ export const createPersonalRooms = async (accountId: Types.ObjectId, shipId: Typ
await personalRooms.save(); 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) { if (!req.query.accountId) {
throw new Error("Request is missing accountId parameter"); throw new Error("Request is missing accountId parameter");
} }
if (!req.query.nonce || parseInt(req.query.nonce as string) === 0) { if (!req.query.nonce || parseInt(req.query.nonce as string) === 0) {
throw new Error("Request is missing nonce parameter"); throw new Error("Request is missing nonce parameter");
} }
const account = await Account.findOne( const account = await Account.findOne({
{ _id: req.query.accountId,
_id: req.query.accountId, Nonce: req.query.nonce
Nonce: req.query.nonce });
},
"_id"
);
if (!account) { if (!account) {
throw new Error("Invalid accountId-nonce pair"); 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> => { export const getAccountIdForRequest = async (req: Request): Promise<string> => {
return (await getAccountForRequest(req))._id.toString(); 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="col-lg-4">
<div class="card mb-4"> <div class="card mb-4">
<h5 class="card-header">Server</h5> <h5 class="card-header">Server</h5>
<form class="card-body" onsubmit="doChangeSettings();return false;"> <div class="card-body">
<div class="form-check"> <div id="server-settings-no-perms" class="d-none">
<input class="form-check-input" type="checkbox" id="skipStoryModeChoice" /> <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>
<label class="form-check-label" for="skipStoryModeChoice">Skip Story Mode Choice</label>
</div> </div>
<div class="form-check"> <form id="server-settings" class="d-none" onsubmit="doChangeSettings();return false;">
<input class="form-check-input" type="checkbox" id="skipTutorial" /> <div class="form-check">
<label class="form-check-label" for="skipTutorial">Skip Tutorial</label> <input class="form-check-input" type="checkbox" id="skipStoryModeChoice" />
</div> <label class="form-check-label" for="skipStoryModeChoice">Skip Story Mode Choice</label>
<div class="form-check"> </div>
<input class="form-check-input" type="checkbox" id="skipAllDialogue" /> <div class="form-check">
<label class="form-check-label" for="skipAllDialogue">Skip All Dialogue</label> <input class="form-check-input" type="checkbox" id="skipTutorial" />
</div> <label class="form-check-label" for="skipTutorial">Skip Tutorial</label>
<div class="form-check"> </div>
<input class="form-check-input" type="checkbox" id="unlockAllScans" /> <div class="form-check">
<label class="form-check-label" for="unlockAllScans">Unlock All Scans</label> <input class="form-check-input" type="checkbox" id="skipAllDialogue" />
</div> <label class="form-check-label" for="skipAllDialogue">Skip All Dialogue</label>
<div class="form-check"> </div>
<input class="form-check-input" type="checkbox" id="unlockAllMissions" /> <div class="form-check">
<label class="form-check-label" for="unlockAllMissions">Unlock All Missions</label> <input class="form-check-input" type="checkbox" id="unlockAllScans" />
</div> <label class="form-check-label" for="unlockAllScans">Unlock All Scans</label>
<div class="form-check"> </div>
<input class="form-check-input" type="checkbox" id="unlockAllQuests" /> <div class="form-check">
<label class="form-check-label" for="unlockAllQuests">Unlock All Quests</label> <input class="form-check-input" type="checkbox" id="unlockAllMissions" />
</div> <label class="form-check-label" for="unlockAllMissions">Unlock All Missions</label>
<div class="form-check"> </div>
<input class="form-check-input" type="checkbox" id="completeAllQuests" /> <div class="form-check">
<label class="form-check-label" for="completeAllQuests">Complete All Quests</label> <input class="form-check-input" type="checkbox" id="unlockAllQuests" />
</div> <label class="form-check-label" for="unlockAllQuests">Unlock All Quests</label>
<div class="form-check"> </div>
<input class="form-check-input" type="checkbox" id="infiniteCredits" /> <div class="form-check">
<label class="form-check-label" for="infiniteCredits">Infinite Credits</label> <input class="form-check-input" type="checkbox" id="completeAllQuests" />
</div> <label class="form-check-label" for="completeAllQuests">Complete All Quests</label>
<div class="form-check"> </div>
<input class="form-check-input" type="checkbox" id="infinitePlatinum" /> <div class="form-check">
<label class="form-check-label" for="infinitePlatinum">Infinite Platinum</label> <input class="form-check-input" type="checkbox" id="infiniteCredits" />
</div> <label class="form-check-label" for="infiniteCredits">Infinite Credits</label>
<div class="form-check"> </div>
<input class="form-check-input" type="checkbox" id="unlockAllShipFeatures" /> <div class="form-check">
<label class="form-check-label" for="unlockAllShipFeatures">Unlock All Ship Features</label> <input class="form-check-input" type="checkbox" id="infinitePlatinum" />
</div> <label class="form-check-label" for="infinitePlatinum">Infinite Platinum</label>
<div class="form-check"> </div>
<input class="form-check-input" type="checkbox" id="unlockAllShipDecorations" /> <div class="form-check">
<label class="form-check-label" for="unlockAllShipDecorations">Unlock All Ship Decorations</label> <input class="form-check-input" type="checkbox" id="unlockAllShipFeatures" />
</div> <label class="form-check-label" for="unlockAllShipFeatures">Unlock All Ship Features</label>
<div class="form-check"> </div>
<input class="form-check-input" type="checkbox" id="unlockAllFlavourItems" /> <div class="form-check">
<label class="form-check-label" for="unlockAllFlavourItems"> <input class="form-check-input" type="checkbox" id="unlockAllShipDecorations" />
Unlock All <abbr title="Animation Sets, Glyphs, Plattes, etc.">Flavor Items</abbr> <label class="form-check-label" for="unlockAllShipDecorations">Unlock All Ship Decorations</label>
</label> </div>
</div> <div class="form-check">
<div class="form-check"> <input class="form-check-input" type="checkbox" id="unlockAllFlavourItems" />
<input class="form-check-input" type="checkbox" id="unlockAllSkins" /> <label class="form-check-label" for="unlockAllFlavourItems">
<label class="form-check-label" for="unlockAllSkins">Unlock All Skins</label> Unlock All <abbr title="Animation Sets, Glyphs, Plattes, etc.">Flavor Items</abbr>
</div> </label>
<div class="form-check"> </div>
<input class="form-check-input" type="checkbox" id="universalPolarityEverywhere" /> <div class="form-check">
<label class="form-check-label" for="universalPolarityEverywhere"> <input class="form-check-input" type="checkbox" id="unlockAllSkins" />
Universal Polarity Everywhere <label class="form-check-label" for="unlockAllSkins">Unlock All Skins</label>
</label> </div>
</div> <div class="form-check">
<div class="form-group mt-2"> <input class="form-check-input" type="checkbox" id="universalPolarityEverywhere" />
<label class="form-label" for="spoofMasteryRank"> <label class="form-check-label" for="universalPolarityEverywhere">
Spoofed Mastery Rank (-1 to disable) Universal Polarity Everywhere
</label> </label>
<input class="form-control" id="spoofMasteryRank" type="number" min="-1" /> </div>
</div> <div class="form-group mt-2">
<button class="btn btn-primary mt-3" type="submit">Save Settings</button> <label class="form-label" for="spoofMasteryRank">
</form> 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> </div>
<div class="col-lg-4"> <div class="col-lg-4">

View File

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