fix: avoid spilling new database account fields into login response #610
@ -7,11 +7,11 @@ 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 { ILoginResponse } from "@/src/types/loginTypes";
|
||||
import { IDatabaseAccountJson, ILoginResponse } from "@/src/types/loginTypes";
|
||||
import { DTLS, groups, HUB, platformCDNs } from "@/static/fixed_responses/login_static";
|
||||
import { logger } from "@/src/utils/logger";
|
||||
|
||||
const loginController: RequestHandler = async (request, response) => {
|
||||
export const loginController: RequestHandler = async (request, response) => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-argument
|
||||
const body = JSON.parse(request.body); // parse octet stream of json data to json object
|
||||
const loginRequest = toLoginRequest(body);
|
||||
@ -39,21 +39,7 @@ const loginController: RequestHandler = async (request, response) => {
|
||||
Nonce: nonce
|
||||
});
|
||||
logger.debug("created new account");
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
const { email, password, LastLoginDay, ...databaseAccount } = newAccount;
|
||||
const newLoginResponse: ILoginResponse = {
|
||||
...databaseAccount,
|
||||
Groups: groups,
|
||||
platformCDNs: platformCDNs,
|
||||
NRS: [config.myAddress],
|
||||
DTLS: DTLS,
|
||||
IRC: config.myIrcAddresses ?? [config.myAddress],
|
||||
HUB: HUB,
|
||||
BuildLabel: buildLabel,
|
||||
MatchmakingBuildId: buildConfig.matchmakingBuildId
|
||||
};
|
||||
|
||||
response.json(newLoginResponse);
|
||||
response.json(createLoginResponse(newAccount, buildLabel));
|
||||
return;
|
||||
} catch (error: unknown) {
|
||||
if (error instanceof Error) {
|
||||
@ -76,9 +62,22 @@ const loginController: RequestHandler = async (request, response) => {
|
||||
}
|
||||
await account.save();
|
||||
|
||||
const { email, password, LastLoginDay, ...databaseAccount } = account.toJSON();
|
||||
const newLoginResponse: ILoginResponse = {
|
||||
...databaseAccount,
|
||||
response.json(createLoginResponse(account.toJSON(), buildLabel));
|
||||
};
|
||||
|
||||
const createLoginResponse = (account: IDatabaseAccountJson, buildLabel: string): ILoginResponse => {
|
||||
return {
|
||||
id: account.id,
|
||||
DisplayName: account.DisplayName,
|
||||
|
||||
CountryCode: account.CountryCode,
|
||||
ClientType: account.ClientType,
|
||||
CrossPlatformAllowed: account.CrossPlatformAllowed,
|
||||
ForceLogoutVersion: account.ForceLogoutVersion,
|
||||
AmazonAuthToken: account.AmazonAuthToken,
|
||||
AmazonRefreshToken: account.AmazonRefreshToken,
|
||||
ConsentNeeded: account.ConsentNeeded,
|
||||
TrackedSettings: account.TrackedSettings,
|
||||
Nonce: account.Nonce,
|
||||
Groups: groups,
|
||||
platformCDNs: platformCDNs,
|
||||
NRS: [config.myAddress],
|
||||
@ -88,8 +87,4 @@ const loginController: RequestHandler = async (request, response) => {
|
||||
BuildLabel: buildLabel,
|
||||
MatchmakingBuildId: buildConfig.matchmakingBuildId
|
||||
};
|
||||
|
||||
response.json(newLoginResponse);
|
||||
};
|
||||
|
||||
export { loginController };
|
||||
|
@ -1,4 +1,4 @@
|
||||
import { IDatabaseAccountDocument } from "@/src/types/loginTypes";
|
||||
import { IDatabaseAccountJson } from "@/src/types/loginTypes";
|
||||
import { model, Schema, SchemaOptions } from "mongoose";
|
||||
|
||||
const opts = {
|
||||
@ -20,7 +20,7 @@ const opts = {
|
||||
// }
|
||||
// }
|
||||
|
||||
const databaseAccountSchema = new Schema<IDatabaseAccountDocument>(
|
||||
const databaseAccountSchema = new Schema<IDatabaseAccountJson>(
|
||||
{
|
||||
email: { type: String, required: true, unique: true },
|
||||
password: { type: String, required: true },
|
||||
@ -48,4 +48,4 @@ databaseAccountSchema.set("toJSON", {
|
||||
virtuals: true
|
||||
});
|
||||
|
||||
export const Account = model<IDatabaseAccountDocument>("Account", databaseAccountSchema);
|
||||
export const Account = model<IDatabaseAccountJson>("Account", databaseAccountSchema);
|
||||
|
@ -1,6 +1,6 @@
|
||||
import { Account } from "@/src/models/loginModel";
|
||||
import { createInventory } from "@/src/services/inventoryService";
|
||||
import { IDatabaseAccount } from "@/src/types/loginTypes";
|
||||
import { IDatabaseAccount, IDatabaseAccountJson } from "@/src/types/loginTypes";
|
||||
import { createShip } from "./shipService";
|
||||
import { Types } from "mongoose";
|
||||
import { Loadout } from "@/src/models/inventoryModels/loadoutModel";
|
||||
@ -12,7 +12,7 @@ export const isCorrectPassword = (requestPassword: string, databasePassword: str
|
||||
return requestPassword === databasePassword;
|
||||
};
|
||||
|
||||
export const createAccount = async (accountData: IDatabaseAccount) => {
|
||||
export const createAccount = async (accountData: IDatabaseAccount): Promise<IDatabaseAccountJson> => {
|
||||
const account = new Account(accountData);
|
||||
try {
|
||||
await account.save();
|
||||
|
@ -1,4 +1,18 @@
|
||||
export interface ILoginResponse extends Omit<IDatabaseAccountDocument, "email" | "password"> {
|
||||
export interface IAccountAndLoginResponseCommons {
|
||||
DisplayName: string;
|
||||
CountryCode: string;
|
||||
ClientType: string;
|
||||
CrossPlatformAllowed: boolean;
|
||||
ForceLogoutVersion: number;
|
||||
AmazonAuthToken?: string;
|
||||
AmazonRefreshToken?: string;
|
||||
ConsentNeeded: boolean;
|
||||
TrackedSettings: string[];
|
||||
Nonce: number;
|
||||
}
|
||||
|
||||
![]() ⚠️ Potential issue Security concern: Sensitive fields in common interface may leak into login response The Consider splitting this into two separate interfaces:
_:warning: Potential issue_
**Security concern: Sensitive fields in common interface may leak into login response**
The `IAccountAndLoginResponseCommons` interface includes potentially sensitive fields like `AmazonAuthToken` and `AmazonRefreshToken`. By making these common between account and login response, we risk exposing sensitive data in the login response, which contradicts the PR's objective of preventing field spillage.
Consider splitting this into two separate interfaces:
```typescript
interface IAccountFields {
DisplayName: string;
CountryCode: string;
ClientType: string;
CrossPlatformAllowed: boolean;
ForceLogoutVersion: number;
AmazonAuthToken?: string;
AmazonRefreshToken?: string;
ConsentNeeded: boolean;
TrackedSettings: string[];
Nonce: number;
}
interface ILoginResponseFields {
DisplayName: string;
CountryCode: string;
ClientType: string;
CrossPlatformAllowed: boolean;
ForceLogoutVersion: number;
ConsentNeeded: boolean;
TrackedSettings: string[];
Nonce: number;
}
```
<!-- This is an auto-generated comment by CodeRabbit -->
|
||||
export interface ILoginResponse extends IAccountAndLoginResponseCommons {
|
||||
id: string;
|
||||
Groups: IGroup[];
|
||||
BuildLabel: string;
|
||||
MatchmakingBuildId: string;
|
||||
@ -10,7 +24,7 @@ export interface ILoginResponse extends Omit<IDatabaseAccountDocument, "email" |
|
||||
}
|
||||
|
||||
// Includes virtual ID
|
||||
export interface IDatabaseAccountDocument extends IDatabaseAccount {
|
||||
export interface IDatabaseAccountJson extends IDatabaseAccount {
|
||||
id: string;
|
||||
}
|
||||
|
||||
@ -19,19 +33,9 @@ export interface IGroup {
|
||||
experimentGroup: string;
|
||||
}
|
||||
|
||||
export interface IDatabaseAccount {
|
||||
export interface IDatabaseAccount extends IAccountAndLoginResponseCommons {
|
||||
email: string;
|
||||
password: string;
|
||||
DisplayName: string;
|
||||
CountryCode: string;
|
||||
ClientType: string;
|
||||
CrossPlatformAllowed: boolean;
|
||||
ForceLogoutVersion: number;
|
||||
AmazonAuthToken?: string;
|
||||
AmazonRefreshToken?: string;
|
||||
ConsentNeeded: boolean;
|
||||
TrackedSettings: string[];
|
||||
Nonce: number;
|
||||
LastLoginDay?: number;
|
||||
}
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user
⚠️ Potential issue
Inconsistent usage of toJSON() method
There's an inconsistency in how the account object is passed to
createLoginResponse
:newAccount
directlyaccount.toJSON()
This could lead to type mismatches or inconsistent behavior.
Suggestion to standardize the approach:
Also applies to: 65-65