OpenWF/SpaceNinjaServer
12
6
Fork 14
Code Issues 41 Pull Requests 1 Actions Packages Projects Releases Activity

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
Conversation 2 Commits 6 Files Changed 11 +31 -12
4 changed files with 31 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit e3bac09e21 - Show all commits

13
src/controllers/api/loginController.ts
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,

14
src/controllers/custom/createAccountController.ts
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 };

12
src/controllers/custom/renameAccountController.ts
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();
} }

4
src/services/loginService.ts
View File

@ -12,6 +12,10 @@ export const isCorrectPassword = (requestPassword: string, databasePassword: str
return requestPassword === databasePassword; return requestPassword === databasePassword;
}; };
export const isNameTaken = async (name: string): Promise<boolean> => {
return !!(await Account.findOne({ DisplayName: name }));
};
export const createAccount = async (accountData: IDatabaseAccount): Promise<IDatabaseAccountJson> => { export const createAccount = async (accountData: IDatabaseAccount): Promise<IDatabaseAccountJson> => {
coderabbitai[bot] commented 2024-12-23 10:40:43 -08:00 (Migrated from github.com)
Review
Copy Link

💡 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 -->
const account = new Account(accountData); const account = new Account(accountData);
try { try {