Skip to content

Revert "fix: Enforce authenticated user ID in savings controller" #62

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Junman140 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Revert "fix: Enforce authenticated user ID in savings controller" #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 0 additions & 19 deletions .eslintrc.json
View file
Open in desktop

This file was deleted.

85 changes: 33 additions & 52 deletions src/controllers/investmentController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,19 @@
/**
* Investment withdrawal: request flow (retail 24h + messaging; business calendar or 1% forced removal).
*/
import { Response, NextFunction } from "express";
import { z } from "zod";
import { prisma } from "../config/database";
import { AuthRequest } from "../middleware/auth";
import { Decimal } from "@prisma/client/runtime/library";
import { AppError } from "../middleware/errorHandler";
import {
isBusinessWithdrawalAllowedDate,
INVESTMENT_FORCED_REMOVAL_FEE_PERCENT,
} from "../config/investment";
import { getRabbitMQChannel } from "../config/rabbitmq";
import { QUEUES } from "../config/rabbitmq";
import { Response, NextFunction } from 'express';
import { z } from 'zod';
import { prisma } from '../config/database';
import { AuthRequest } from '../middleware/auth';
import { Decimal } from '@prisma/client/runtime/library';
import { AppError } from '../middleware/errorHandler';
import { isBusinessWithdrawalAllowedDate, INVESTMENT_FORCED_REMOVAL_FEE_PERCENT } from '../config/investment';
import { getRabbitMQChannel } from '../config/rabbitmq';
import { QUEUES } from '../config/rabbitmq';

const requestSchema = z.object({
amount_acbu: z
.string()
.min(1)
.refine(
(s) => !Number.isNaN(Number(s)) && Number(s) > 0,
"must be positive",
),
audience: z.enum(["retail", "business"]),
amount_acbu: z.string().min(1).refine((s) => !Number.isNaN(Number(s)) && Number(s) > 0, 'must be positive'),
audience: z.enum(['retail', 'business']),
forced_removal: z.boolean().optional(),
});

Expand All @@ -34,41 +25,38 @@ const TWENTY_FOUR_HOURS_MS = 24 * 60 * 60 * 1000;
export async function postInvestmentWithdrawRequest(
req: AuthRequest,
res: Response,
next: NextFunction,
next: NextFunction
): Promise<void> {
try {
const userId = req.apiKey?.userId ?? null;
const organizationId = req.apiKey?.organizationId ?? null;
if (!userId && !organizationId) {
throw new AppError("User or organization context required", 401);
throw new AppError('User or organization context required', 401);
}
const parsed = requestSchema.safeParse(req.body);
if (!parsed.success) {
res
.status(400)
.json({ error: "Invalid request", details: parsed.error.flatten() });
res.status(400).json({ error: 'Invalid request', details: parsed.error.flatten() });
return;
}
const { amount_acbu, audience, forced_removal } = parsed.data;
const amountNum = Number(amount_acbu);

if (audience === "business") {
if (audience === 'business') {
const onAllowedDate = isBusinessWithdrawalAllowedDate();
if (!onAllowedDate && !forced_removal) {
res.status(403).json({
error: "Withdrawal only on allowed dates",
code: "INVESTMENT_BUSINESS_CALENDAR",
message:
"Business investment withdrawals are only allowed on specific dates. Use forced_removal: true to withdraw with 1% fee (funds in 24h).",
allowed_days: process.env.INVESTMENT_BUSINESS_ALLOWED_DAYS || "1,15",
error: 'Withdrawal only on allowed dates',
code: 'INVESTMENT_BUSINESS_CALENDAR',
message: 'Business investment withdrawals are only allowed on specific dates. Use forced_removal: true to withdraw with 1% fee (funds in 24h).',
allowed_days: process.env.INVESTMENT_BUSINESS_ALLOWED_DAYS || '1,15',
});
return;
}
}

const availableAt = new Date(Date.now() + TWENTY_FOUR_HOURS_MS);
let feePercent: Decimal | null = null;
if (audience === "business" && forced_removal) {
if (audience === 'business' && forced_removal) {
feePercent = new Decimal(INVESTMENT_FORCED_REMOVAL_FEE_PERCENT);
}

Expand All @@ -78,26 +66,22 @@ export async function postInvestmentWithdrawRequest(
organizationId: organizationId ?? undefined,
audience,
amountAcbu: new Decimal(amountNum),
status: "requested",
forcedRemoval: audience === "business" && forced_removal === true,
status: 'requested',
forcedRemoval: audience === 'business' && (forced_removal === true),
feePercent,
availableAt,
},
});

res.status(202).json({
request_id: record.id,
status: "requested",
status: 'requested',
amount_acbu: amount_acbu,
available_at: availableAt.toISOString(),
fee_percent: feePercent?.toNumber() ?? null,
message:
audience === "retail"
? "Funds will be available in 24 hours. You will receive a notification when ready."
: "Funds will be available in 24 hours." +
(feePercent
? ` A ${feePercent}% fee applies (forced removal).`
: ""),
message: audience === 'retail'
? 'Funds will be available in 24 hours. You will receive a notification when ready.'
: 'Funds will be available in 24 hours.' + (feePercent ? ` A ${feePercent}% fee applies (forced removal).` : ''),
});
} catch (e) {
next(e);
Expand All @@ -110,18 +94,18 @@ export async function postInvestmentWithdrawRequest(
export async function getInvestmentWithdrawRequests(
req: AuthRequest,
res: Response,
next: NextFunction,
next: NextFunction
): Promise<void> {
try {
const userId = req.apiKey?.userId ?? null;
const organizationId = req.apiKey?.organizationId ?? null;
const list = await prisma.investmentWithdrawalRequest.findMany({
where: userId ? { userId } : { organizationId },
orderBy: { createdAt: "desc" },
orderBy: { createdAt: 'desc' },
take: 50,
});
res.status(200).json({
requests: list.map((r: any) => ({
requests: list.map((r) => ({
id: r.id,
amount_acbu: r.amountAcbu.toString(),
status: r.status,
Expand All @@ -139,22 +123,19 @@ export async function getInvestmentWithdrawRequests(
/**
* Publish investment_withdrawal_ready to NOTIFICATIONS queue (called by job).
*/
export async function publishInvestmentWithdrawalReady(
userId: string | null,
amountAcbu: number,
): Promise<void> {
export async function publishInvestmentWithdrawalReady(userId: string | null, amountAcbu: number): Promise<void> {
const ch = getRabbitMQChannel();
await ch.assertQueue(QUEUES.NOTIFICATIONS, { durable: true });
ch.sendToQueue(
QUEUES.NOTIFICATIONS,
Buffer.from(
JSON.stringify({
type: "investment_withdrawal_ready",
type: 'investment_withdrawal_ready',
userId,
amountAcbu,
timestamp: new Date().toISOString(),
}),
})
),
{ persistent: true },
{ persistent: true }
);
}
84 changes: 32 additions & 52 deletions src/controllers/savingsController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,29 @@
import { Request, Response, NextFunction } from "express";
import { acbuSavingsVaultService } from "../services/contracts";
import { contractAddresses } from "../config/contracts";
import type { AuthRequest } from "../middleware/auth";
import { AppError } from "../middleware/errorHandler";
import { Request, Response, NextFunction } from 'express';
import { acbuSavingsVaultService } from '../services/contracts';
import { contractAddresses } from '../config/contracts';
import type { AuthRequest } from '../middleware/auth';
import { AppError } from '../middleware/errorHandler';
import {
isSavingsLockDate,
getNextSavingsWithdrawalDate,
getApyForTerm,
} from "../config/savings";
} from '../config/savings';

export async function postSavingsDeposit(
req: Request,
res: Response,
next: NextFunction,
next: NextFunction
): Promise<void> {
try {
const authReq = req as AuthRequest;
const userId = authReq.apiKey?.userId;

if (!userId) {
throw new AppError("Authenticated user ID required for savings", 401);
}

const { amount, term_seconds } = authReq.body || {};
if (!amount || term_seconds == null) {
throw new AppError("amount and term_seconds required", 400);
const { user, amount, term_seconds } = (req as AuthRequest).body || {};
if (!user || !amount || term_seconds == null) {
throw new AppError('user, amount, and term_seconds required', 400);
}
if (!contractAddresses.savingsVault) {
throw new AppError("Savings vault contract not configured", 503);
throw new AppError('Savings vault contract not configured', 503);
}
const result = await acbuSavingsVaultService.deposit({
user: userId,
user,
Comment on lines +18 to +26
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Don't trust user from the request on savings endpoints.

These handlers now take req.body.user / req.query.user and forward it straight into acbuSavingsVaultService. The savings routes only enforce API key + scope, and the contract service passes user directly to the contract, so any authenticated caller can target another Stellar address by swapping that field. Resolve the caller's Stellar address from the authenticated principal, or verify the supplied address belongs to that principal before invoking the contract.

Also applies to: 55-63, 79-89

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/savingsController.ts` around lines 18 - 26, The handlers in
savingsController.ts are trusting req.body.user / req.query.user and passing it
to acbuSavingsVaultService.deposit (and similar calls) which allows one caller
to act as another; instead, resolve the caller's Stellar address from the
authenticated principal (AuthRequest) or verify the supplied address belongs to
that principal before calling acbuSavingsVaultService methods (e.g.,
acbuSavingsVaultService.deposit), and throw an AppError(403) if the address does
not match or cannot be resolved; update all savings-related handlers in this
file to use the principal-derived address (or ownership check) instead of
blindly forwarding the request-supplied user.

amount: String(amount),
termSeconds: Number(term_seconds),
});
Expand All @@ -46,36 +39,28 @@ export async function postSavingsDeposit(
export async function postSavingsWithdraw(
req: Request,
res: Response,
next: NextFunction,
next: NextFunction
): Promise<void> {
try {
if (isSavingsLockDate()) {
const nextDate = getNextSavingsWithdrawalDate();
res.status(403).json({
error: "Savings locked",
code: "SAVINGS_LOCK_DATE",
message:
"Savings withdrawals are not allowed on this date. Next available withdrawal date below.",
error: 'Savings locked',
code: 'SAVINGS_LOCK_DATE',
message: 'Savings withdrawals are not allowed on this date. Next available withdrawal date below.',
next_available_withdrawal_date: nextDate.toISOString().slice(0, 10),
});
return;
}
const authReq = req as AuthRequest;
const userId = authReq.apiKey?.userId;

if (!userId) {
throw new AppError("Authenticated user ID required for savings", 401);
}

const { term_seconds, amount } = authReq.body || {};
if (term_seconds == null || !amount) {
throw new AppError("term_seconds and amount required", 400);
const { user, term_seconds, amount } = (req as AuthRequest).body || {};
if (!user || term_seconds == null || !amount) {
throw new AppError('user, term_seconds, and amount required', 400);
}
if (!contractAddresses.savingsVault) {
throw new AppError("Savings vault contract not configured", 503);
throw new AppError('Savings vault contract not configured', 503);
}
const txHash = await acbuSavingsVaultService.withdraw({
user: userId,
user,
termSeconds: Number(term_seconds),
amount: String(amount),
});
Expand All @@ -88,35 +73,30 @@ export async function postSavingsWithdraw(
export async function getSavingsPositions(
req: Request,
res: Response,
next: NextFunction,
next: NextFunction
): Promise<void> {
try {
const authReq = req as AuthRequest;
const userId = authReq.apiKey?.userId;

if (!userId) {
throw new AppError("Authenticated user ID required for savings", 401);
const user = (req as AuthRequest).query?.user as string;
const termSeconds = (req as AuthRequest).query?.term_seconds as string;
if (!user) {
throw new AppError('query user required', 400);
}

const termSeconds = authReq.query?.term_seconds as string;
if (!contractAddresses.savingsVault) {
throw new AppError("Savings vault contract not configured", 503);
throw new AppError('Savings vault contract not configured', 503);
}
const balance = await acbuSavingsVaultService.getBalance(
userId,
termSeconds != null ? Number(termSeconds) : 0,
user,
termSeconds != null ? Number(termSeconds) : 0
);
const termSec = termSeconds != null ? Number(termSeconds) : 0;
const apy = getApyForTerm(termSec);
const nextDate = getNextSavingsWithdrawalDate();
res.status(200).json({
user: userId,
user,
term_seconds: termSec || null,
balance,
apy_percent: apy,
next_available_withdrawal_date: isSavingsLockDate()
? nextDate.toISOString().slice(0, 10)
: null,
next_available_withdrawal_date: isSavingsLockDate() ? nextDate.toISOString().slice(0, 10) : null,
});
} catch (e) {
next(e);
Expand All @@ -127,7 +107,7 @@ export async function getSavingsPositions(
export async function getNextWithdrawalDate(
_req: Request,
res: Response,
next: NextFunction,
next: NextFunction
): Promise<void> {
try {
const nextDate = getNextSavingsWithdrawalDate();
Expand Down
Loading
Loading