Skip to content

feat(stellar): implement non-custodial refund and dispute flow (#62) - #69

Merged
zeemscript merged 4 commits into
Deen-Bridge:devfrom
Ahbiz:feature/non-custodial-refunds
Jul 23, 2026
Merged

feat(stellar): implement non-custodial refund and dispute flow (#62)#69
zeemscript merged 4 commits into
Deen-Bridge:devfrom
Ahbiz:feature/non-custodial-refunds

Conversation

@Ahbiz

@Ahbiz Ahbiz commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements a non-custodial refund and dispute resolution flow for course and book purchases on DeenBridge (#62). Because payments flow peer-to-peer directly from buyer to creator on Stellar without platform custody, refunds are constructed as reverse Stellar payment transactions signed directly by the educator, verified on-chain, and reconciled with atomic platform access revocation.


Key Features & Architectural Highlights

  1. Non-Custodial Refund Lifecycle:

    • Request: Buyer initiates a refund within the 14-day window.
    • Approval & XDR Generation: Educator approves and generates an unsigned reverse payment transaction XDR (RFND:<tx_hash> memo).
    • Submission & Verification: Educator submits the signed XDR; the platform verifies finality on Horizon.
    • Atomic Access Revocation: Upon confirmed on-chain refund, buyer access to the purchased course/book is revoked atomically via database transactions.
  2. Dispute & Role-Gated Arbitration Flow:

    • Escalation: If an educator rejects a valid refund, the buyer can escalate to disputed status.
    • Arbitration: Role-gated for admin and arbiter users to log resolution notes and off-chain mediation results with clear non-custodial limitations disclaimed.
  3. Data Model Extensions:

    • Refund.js: New model with idempotency partial unique index (enforcing one active refund per transaction) and arbitration schema.
    • Transaction.js: Extended status enum (refunded, disputed) and added refund reference.
    • User.js: Extended roles with admin and arbiter.

API Endpoints Added

Method Endpoint Access Description
POST /api/stellar/payment/transactions/:id/refund-request Buyer Request a refund for a confirmed purchase within window
POST /api/stellar/payment/refunds/:id/build Educator Approve refund & generate unsigned reverse payment XDR
POST /api/stellar/payment/refunds/:id/submit Educator Submit signed reverse XDR, verify Horizon, & revoke access
POST /api/stellar/payment/refunds/:id/reject Educator Reject a pending refund request with reason
POST /api/stellar/payment/refunds/:id/dispute Buyer Escalate a rejected refund request to disputed status
PATCH /api/stellar/payment/refunds/:id/arbitrate Admin/Arbiter Record binding arbitration decision

Automated Test Coverage

Comprehensive unit and integration test suite implemented in test/refund.test.js (11/11 tests passing):

  • Refund request creation, window validation, and idempotency protection
  • Unsigned reverse payment XDR construction with Stellar SDK
  • Horizon on-chain verification & atomic access revocation on User and Course
  • Rejection & dispute escalation lifecycle
  • Role-gated admin/arbiter dispute resolution

Summary by CodeRabbit

  • New Features
    • Added a full non-custodial Stellar refund workflow for eligible confirmed payments: request, educator approval via reverse-payment XDR, submission, confirmation, rejection, dispute escalation, and admin/arbiter resolution with recorded decision details.
    • Added role-based authorization controls for refund and dispute actions.
  • Bug Fixes
    • Enforced refund eligibility rules (including idempotency to prevent duplicate active refunds) and blocked refund submission until approved.
    • After confirmation, updated payment/refund statuses and revoked the buyer’s purchased access.
  • Chores / Tests
    • Reformatted ignore rules and expanded local scratch/temporary markdown ignores.
    • Added comprehensive integration tests covering the full refund lifecycle and permissions.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 806b8d23-f079-4dc4-ae87-cb2ee0e811c3

📥 Commits

Reviewing files that changed from the base of the PR and between 0146810 and ec82ab6.

📒 Files selected for processing (4)
  • src/controllers/stellar/refundController.js
  • src/models/Transaction.js
  • src/routes/stellar/paymentRoutes.js
  • src/services/stellar/stellarService.js
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/routes/stellar/paymentRoutes.js
  • src/models/Transaction.js
  • src/services/stellar/stellarService.js
  • src/controllers/stellar/refundController.js

Walkthrough

The PR adds a non-custodial Stellar refund lifecycle with buyer requests, educator approval and payment submission, access revocation, dispute escalation, arbitration, role authorization, persistence updates, routes, and integration tests.

Changes

Refund and dispute workflow

Layer / File(s) Summary
Refund contracts and authorization
src/models/Refund.js, src/models/Transaction.js, src/models/User.js, src/middlewares/authMiddleware.js
Adds refund lifecycle data and uniqueness constraints, new transaction states, admin/arbitrator roles, and role authorization middleware.
Refund routes and payment preparation
src/routes/stellar/paymentRoutes.js, src/services/stellar/stellarService.js, src/controllers/stellar/refundController.js
Adds authenticated refund/dispute routes, request validation, educator XDR preparation, and reverse-payment transaction construction.
Settlement and dispute lifecycle
src/controllers/stellar/refundController.js
Submits and verifies signed Stellar refunds, revokes course or book access through sequential updates, updates transaction state, and handles rejection, escalation, and arbitration.
Refund integration validation
test/refund.test.js
Covers refund requests, role restrictions, reverse payments, access revocation, dispute escalation, and arbitration with mocked Stellar services.

Repository configuration

Layer / File(s) Summary
Temporary artifact ignore rules
.gitignore
Reorganizes ignore sections and adds specification markdown files and scratch/ to ignored paths.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Buyer
  participant paymentRoutes
  participant refundController
  participant stellarService
  participant Horizon
  participant MongoDB

  Buyer->>paymentRoutes: Request refund
  paymentRoutes->>refundController: Validate buyer and transaction
  refundController->>MongoDB: Create Refund and link Transaction
  refundController-->>Buyer: Return refund request
  paymentRoutes->>refundController: Build reverse-payment XDR
  refundController->>stellarService: Build unsigned Stellar payment
  stellarService->>Horizon: Load source account
  Horizon-->>stellarService: Return account data
  stellarService-->>refundController: Return XDR and network passphrase
  refundController-->>Buyer: Return unsigned XDR
  Buyer->>paymentRoutes: Submit signed XDR
  paymentRoutes->>refundController: Submit refund
  refundController->>Horizon: Submit and verify transaction
  Horizon-->>refundController: Return transaction details
  refundController->>MongoDB: Update refund, transaction, and access records
  refundController-->>Buyer: Return confirmed refund
Loading

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main Stellar refund and dispute workflow added in this PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 14

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.env.example:
- Around line 6-7: Update the JWT_SECRET placeholder in the environment
configuration example to a clearly non-deployable value of at least 32
characters, while preserving the existing guidance comment and variable name.

In `@src/config/validateEnv.js`:
- Around line 27-32: Update the environment validation in validateEnv to remove
the Redis keys from the generic required/configuration list and validate them
only when Redis is enabled. Accept a configured REDIS_URL as a complete setup;
otherwise require both REDIS_HOST and REDIS_PORT, while allowing Redis-disabled
deployments without warnings.

In `@src/controllers/stellar/refundController.js`:
- Around line 276-301: Update the course and book revocation filters in the
refund controller to compare refund.itemId against each subdocument’s courseId
or bookId field, preserving entries only when those fields differ. In
test/refund.test.js lines 294-296, seed purchasedCourses using the { courseId,
purchaseDate } shape and assert against mapped courseId values so the test
validates actual access removal.
- Around line 252-260: Update the verification flow around verifyTransaction in
refundController to validate the refunded payment, not only exists and
successful. Inspect verification.operations.records to confirm an expected
reverse USDC payment to the buyer for refund.amount before allowing the
refund-confirmation and access-revocation path to proceed. Treat a missing or
mismatched operation like failed verification, preserving the existing failed
status and 400 response.
- Around line 86-97: Update the refund creation logic in the refund controller
so fee-split transactions use transaction.platformFee.creatorAmount as the
educator refund amount instead of the gross transaction.amount. Add a separate
refund leg for the platform’s fee share when that amount is refundable, while
preserving the existing behavior for transactions without a fee split.

In `@src/routes/books/bookRoutes.js`:
- Around line 71-75: Update the shared cacheMiddleware used by
bookDetailCacheKey and the cached course, space, user, and search routes so it
stores responses only when the original HTTP status is successful; preserve the
original status and payload on cache misses and ensure cache hits do not convert
non-success responses into 200 results.
- Around line 84-90: Update the review route in src/routes/books/bookRoutes.js
(lines 84-90) to invalidate both ${CACHE_KEYS.BOOK}* and ${CACHE_KEYS.BOOKS}*
after addBookReview succeeds. Update the enrollment and review mutation routes
in src/routes/courses/courseRoutes.js (lines 66-77) to also invalidate
${CACHE_KEYS.COURSES}*. Update the waitlist mutation route in
src/routes/spaceRoutes.js (lines 58-64) to also invalidate
${CACHE_KEYS.SPACES}*. Use the existing invalidateCacheMiddleware configuration
at each site.
- Around line 78-81: Add the existing protect authentication middleware to the
DELETE /:id route before invalidateCacheMiddleware and deleteBook. Update
deleteBook to load the targeted book, verify the authenticated requester is its
author or an authorized admin, and reject unauthorized or unauthenticated
deletion attempts before performing the delete.
- Around line 49-54: Update the recommendation route and fetchRecommendedBooks
flow so cache entries vary by the validated recommendation interests: move the
filter from req.body.interests to validated query parameters and incorporate
that value into the cache key generated from CACHE_KEYS.BOOKS, or remove caching
and expose the endpoint as POST. Ensure different interest sets cannot reuse the
same response.
- Around line 35-90: Add route-level Jest tests for the cache contracts across
src/routes/books/bookRoutes.js lines 35-90, src/routes/courses/courseRoutes.js
lines 34-83, src/routes/searchRoutes.js lines 9-16, src/routes/spaceRoutes.js
lines 28-80, and src/routes/userRoutes.js lines 35-121. Cover cache hit and miss
behavior, key isolation, and invalidation for create, update, delete, review,
bookmark, and follow flows, using the route handlers and cache middleware
defined at each site.

In `@src/routes/courses/courseRoutes.js`:
- Around line 66-71: Update the enrollment flow anchored by enrollInCourse to
invalidate the enrolled user’s cache namespace after a successful enrollment, in
addition to the existing course cache invalidation. Reuse the project’s
established user cache key pattern or CACHE_KEYS symbol so GET /users/:id and
GET /users/:id/stats entries for the affected user are cleared without
invalidating unrelated users.

In `@src/routes/spaceRoutes.js`:
- Around line 66-79: Update the PUT "/update/:id" and DELETE "/:id" routes
around updateSpace and deleteSpace to enforce authorization beyond protect: load
the targeted space, permit the request only when space.host matches req.user._id
or the user has the established admin authorization, and reject unauthorized
mutations before invoking the handlers. Preserve the existing cache invalidation
middleware for authorized requests.

In `@src/routes/userRoutes.js`:
- Around line 45-68: Update the PUT "/update/:id" and DELETE "/:id" routes to
enforce self-or-admin authorization after protect and before updateUser or
deleteUser; compare req.params.id with req.user._id, allowing mismatches only
for admin users, and reject unauthorized requests without invoking the
controllers. Leave the cached GET route unchanged.

In `@test/refund.test.js`:
- Line 26: Remove the hardcoded JWT fallback from the test constant and the
production secret configuration in authMiddleware.js. Require JWT_SECRET during
environment validation so startup fails when it is missing, and have the test
read the environment value directly; document the required variable in
.env.example.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b9c7ca8a-6bf5-4a11-81a6-5f7c3c556134

📥 Commits

Reviewing files that changed from the base of the PR and between 768029d and 403d31a.

📒 Files selected for processing (16)
  • .env.example
  • .gitignore
  • src/config/validateEnv.js
  • src/controllers/stellar/refundController.js
  • src/middlewares/authMiddleware.js
  • src/models/Refund.js
  • src/models/Transaction.js
  • src/models/User.js
  • src/routes/books/bookRoutes.js
  • src/routes/courses/courseRoutes.js
  • src/routes/searchRoutes.js
  • src/routes/spaceRoutes.js
  • src/routes/stellar/paymentRoutes.js
  • src/routes/userRoutes.js
  • src/services/stellar/stellarService.js
  • test/refund.test.js

Comment thread .env.example Outdated
Comment on lines +6 to +7
# JWT secret for authentication (use a strong random string, min 32 characters)
JWT_SECRET=your_jwt_secret_here

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Use a 32+ character JWT placeholder.

your_jwt_secret_here is below the documented minimum and will cause copied production configuration to run with a weak secret warning. Use a clearly non-deployable placeholder that is at least 32 characters long.

As per path instructions, “JWT: set a strong JWT_SECRET (min ~32 chars).”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.env.example around lines 6 - 7, Update the JWT_SECRET placeholder in the
environment configuration example to a clearly non-deployable value of at least
32 characters, while preserving the existing guidance comment and variable name.

Source: Path instructions

Comment thread src/config/validateEnv.js Outdated
Comment on lines +27 to +32
// Redis configuration (optional - app works without Redis)
"REDIS_URL",
"REDIS_HOST",
"REDIS_PORT",
"REDIS_USERNAME",
"REDIS_PASSWORD",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Validate Redis as an alternative configuration set.

A valid REDIS_URL setup still warns about missing host/port credentials, and a valid host/port setup still warns about REDIS_URL. Redis being disabled also emits production warnings. Remove these keys from the generic list and validate Redis only when enabled: accept REDIS_URL, or require both REDIS_HOST and REDIS_PORT.

As per path instructions, “Redis is optional, but if you enable it you should provide complete connection details to avoid production warnings.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/config/validateEnv.js` around lines 27 - 32, Update the environment
validation in validateEnv to remove the Redis keys from the generic
required/configuration list and validate them only when Redis is enabled. Accept
a configured REDIS_URL as a complete setup; otherwise require both REDIS_HOST
and REDIS_PORT, while allowing Redis-disabled deployments without warnings.

Source: Path instructions

Comment on lines +86 to +97
const refund = await Refund.create({
originalTransaction: transaction._id,
buyer: transaction.buyer,
educator: transaction.creator,
itemType: transaction.itemType,
itemId: transaction.itemId,
amount: transaction.amount,
currency: transaction.currency || "USDC",
reason: reason.trim(),
status: "requested",
expiresAt: new Date(Date.now() + windowMs),
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how platformFee is populated at purchase time to gauge how often refunds over-pay.
rg -nP --type=js -C3 'platformFee|creatorAmount|platformAmount' src/controllers

Repository: Deen-Bridge/dnb-backend

Length of output: 2343


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== refundController excerpt ==\n'
sed -n '1,220p' src/controllers/stellar/refundController.js

printf '\n== Transaction model excerpt ==\n'
sed -n '1,220p' src/models/Transaction.js

printf '\n== paymentController relevant excerpts ==\n'
sed -n '160,360p' src/controllers/stellar/paymentController.js

Repository: Deen-Bridge/dnb-backend

Length of output: 16567


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== refundController line map ==\n'
nl -ba src/controllers/stellar/refundController.js | sed -n '1,220p'

printf '\n== search refund/platformFee usages ==\n'
rg -n "platformFee|creatorAmount|platformAmount|refund" src/controllers/stellar src/models src/services

Repository: Deen-Bridge/dnb-backend

Length of output: 234


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== refund-related search ==\n'
rg -n "platformFee|refund|reverse payment|buildReversePaymentTransaction|recordSaleEarnings|refund request" src/controllers src/services src/models

printf '\n== refund service/controller excerpts ==\n'
sed -n '1,260p' src/services/stellar/stellarService.js

Repository: Deen-Bridge/dnb-backend

Length of output: 19675


Split fee-aware refunds
src/controllers/stellar/refundController.js:86-97refund.amount is the gross charge, but fee-split purchases only pay the educator transaction.platformFee.creatorAmount. Building the reverse transfer for the full amount makes the educator repay money they never received; use the creator net amount here, and add a separate refund leg for the platform share if that portion should be returned too.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/controllers/stellar/refundController.js` around lines 86 - 97, Update the
refund creation logic in the refund controller so fee-split transactions use
transaction.platformFee.creatorAmount as the educator refund amount instead of
the gross transaction.amount. Add a separate refund leg for the platform’s fee
share when that amount is refundable, while preserving the existing behavior for
transactions without a fee split.

Comment thread src/controllers/stellar/refundController.js
Comment on lines +276 to +301
if (refund.itemType === "course") {
// Remove course from buyer's purchased list
if (buyer) {
buyer.purchasedCourses = (buyer.purchasedCourses || []).filter(
(cId) => cId.toString() !== refund.itemId.toString()
);
await buyer.save(sessionOpts);
}

// Remove buyer from Course.enrolledUsers
const course = await Course.findById(refund.itemId);
if (course) {
course.enrolledUsers = (course.enrolledUsers || []).filter(
(uId) => uId.toString() !== refund.buyer.toString()
);
await course.save(sessionOpts);
}
} else if (refund.itemType === "book") {
// Remove book from buyer's purchased list
if (buyer) {
buyer.purchasedBooks = (buyer.purchasedBooks || []).filter(
(bId) => bId.toString() !== refund.itemId.toString()
);
await buyer.save(sessionOpts);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win

Access is never actually revoked — purchasedCourses/purchasedBooks are subdocument arrays, not ObjectId arrays. In src/models/User.js these fields are typed as [{ courseId/bookId, purchaseDate }], so each element is a subdocument. The revocation filters call cId.toString() / bId.toString() on those subdocuments (yielding "[object Object]"), which never equals refund.itemId.toString(), so nothing is filtered out and the buyer keeps access after a confirmed refund — breaking this PR's central "atomic access revocation" guarantee. The test doesn't catch it because it seeds with raw ObjectIds and its .map(c => c.toString()) assertion also compares against the wrong shape, so it passes regardless.

  • src/controllers/stellar/refundController.js#L276-L301: filter on the subdocument's id field, e.g. buyer.purchasedCourses.filter((c) => c.courseId?.toString() !== refund.itemId.toString()) and buyer.purchasedBooks.filter((b) => b.bookId?.toString() !== refund.itemId.toString()).
  • test/refund.test.js#L294-L296: seed purchasedCourses with the real { courseId, purchaseDate } shape (at L143) and assert on courseId, e.g. expect(updatedBuyer.purchasedCourses.map((c) => c.courseId.toString())).not.toContain(course._id.toString()), so the test fails until the controller is fixed.
📍 Affects 2 files
  • src/controllers/stellar/refundController.js#L276-L301 (this comment)
  • test/refund.test.js#L294-L296
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/controllers/stellar/refundController.js` around lines 276 - 301, Update
the course and book revocation filters in the refund controller to compare
refund.itemId against each subdocument’s courseId or bookId field, preserving
entries only when those fields differ. In test/refund.test.js lines 294-296,
seed purchasedCourses using the { courseId, purchaseDate } shape and assert
against mapped courseId values so the test validates actual access removal.

Comment thread src/routes/books/bookRoutes.js Outdated
Comment on lines +84 to +90
// review a book - invalidates specific book cache
router.post(
"/:id/reviews",
protect,
invalidateCacheMiddleware([`${CACHE_KEYS.BOOK}*`]),
addBookReview
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Invalidate collection caches when their serialized records change.

These mutations clear only detail entries, but the cached list/by-author/by-host responses include the same changed review, enrollment, rating, or waitlist fields.

  • src/routes/books/bookRoutes.js#L84-L90: also invalidate ${CACHE_KEYS.BOOKS}* after a review succeeds.
  • src/routes/courses/courseRoutes.js#L66-L77: also invalidate ${CACHE_KEYS.COURSES}* after enrollment and review mutations.
  • src/routes/spaceRoutes.js#L58-L64: also invalidate ${CACHE_KEYS.SPACES}* after a waitlist mutation.
📍 Affects 3 files
  • src/routes/books/bookRoutes.js#L84-L90 (this comment)
  • src/routes/courses/courseRoutes.js#L66-L77
  • src/routes/spaceRoutes.js#L58-L64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/routes/books/bookRoutes.js` around lines 84 - 90, Update the review route
in src/routes/books/bookRoutes.js (lines 84-90) to invalidate both
${CACHE_KEYS.BOOK}* and ${CACHE_KEYS.BOOKS}* after addBookReview succeeds.
Update the enrollment and review mutation routes in
src/routes/courses/courseRoutes.js (lines 66-77) to also invalidate
${CACHE_KEYS.COURSES}*. Update the waitlist mutation route in
src/routes/spaceRoutes.js (lines 58-64) to also invalidate
${CACHE_KEYS.SPACES}*. Use the existing invalidateCacheMiddleware configuration
at each site.

Comment thread src/routes/courses/courseRoutes.js Outdated
Comment on lines +66 to +71
router.post(
"/:id/enroll",
protect,
invalidateCacheMiddleware([`${CACHE_KEYS.COURSE}*`]),
enrollInCourse
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Invalidate the enrolled user’s cached data.

Enrollment updates User.purchasedCourses, while GET /users/:id and GET /users/:id/stats are cached for ten minutes. This only clears course detail keys, so the buyer can receive stale profile and enrollment statistics until TTL expiry. Also invalidate the affected user cache namespace after a successful enrollment.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/routes/courses/courseRoutes.js` around lines 66 - 71, Update the
enrollment flow anchored by enrollInCourse to invalidate the enrolled user’s
cache namespace after a successful enrollment, in addition to the existing
course cache invalidation. Reuse the project’s established user cache key
pattern or CACHE_KEYS symbol so GET /users/:id and GET /users/:id/stats entries
for the affected user are cleared without invalidating unrelated users.

Comment thread src/routes/spaceRoutes.js Outdated
Comment on lines +66 to +79
// Update a space - invalidates space caches
router.put(
"/update/:id",
protect,
invalidateCacheMiddleware([`${CACHE_KEYS.SPACES}*`, `${CACHE_KEYS.SPACE}*`]),
updateSpace
);

// Delete a space - invalidates space caches
router.delete(
"/:id",
protect,
invalidateCacheMiddleware([`${CACHE_KEYS.SPACES}*`, `${CACHE_KEYS.SPACE}*`]),
deleteSpace

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Enforce host ownership for space mutations.

protect only establishes identity; updateSpace and deleteSpace mutate by ID without comparing space.host to req.user._id. Any logged-in user can modify or remove another host’s space. Load the space first and allow only its host (or an authorized admin) to proceed.

As per path instructions, “Flag missing auth/ownership checks” for changed endpoints.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/routes/spaceRoutes.js` around lines 66 - 79, Update the PUT "/update/:id"
and DELETE "/:id" routes around updateSpace and deleteSpace to enforce
authorization beyond protect: load the targeted space, permit the request only
when space.host matches req.user._id or the user has the established admin
authorization, and reject unauthorized mutations before invoking the handlers.
Preserve the existing cache invalidation middleware for authorized requests.

Source: Path instructions

Comment thread src/routes/userRoutes.js Outdated
Comment on lines +45 to +68
// Update user profile (with avatar upload) - invalidates user cache
router.put(
"/update/:id",
protect,
upload.single("avatar"),
invalidateCacheMiddleware([`${CACHE_KEYS.USER}*`]),
updateUser
);

// Get personalized recommendations
router.get("/recommendations", protect, getRecommendations);
// Get user by ID - cached for 10 minutes
router.get(
"/:id",
protect,
cacheMiddleware(CACHE_TTL.USERS, userCacheKey),
getUser
);

// Get user statistics
router.get("/:id/stats", protect, getUserStats);
// Delete user - invalidates user cache
router.delete(
"/:id",
protect,
invalidateCacheMiddleware([`${CACHE_KEYS.USER}*`]),
deleteUser
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Restrict profile updates and deletion to the account owner or admin.

Both handlers trust req.params.id; their controllers update/delete by that ID without comparing it to req.user._id. Authentication alone lets any user alter or delete any account. Enforce self-or-admin authorization before calling the controllers.

As per path instructions, “Flag missing auth/ownership checks” for changed endpoints.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/routes/userRoutes.js` around lines 45 - 68, Update the PUT "/update/:id"
and DELETE "/:id" routes to enforce self-or-admin authorization after protect
and before updateUser or deleteUser; compare req.params.id with req.user._id,
allowing mismatches only for admin users, and reject unauthorized requests
without invoking the controllers. Leave the cached GET route unchanged.

Source: Path instructions

Comment thread test/refund.test.js
app.use(express.json());
app.use("/api/stellar/payment", paymentRoutes);

const JWT_SECRET = process.env.JWT_SECRET || "deenbridge-temp-secret-key-2024";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Don't fall back to a hardcoded JWT secret.

Thanks for wiring up process.env.JWT_SECRET here! The concern is the literal fallback: the exact same constant lives in production at src/middlewares/authMiddleware.js:4. If JWT_SECRET is ever unset in a real environment, tokens get signed and verified with a value that's now committed to the repo — anyone can forge a valid token for any role (including the new admin/arbiter gate this PR adds), which is a full auth bypass. Please require JWT_SECRET from the environment (fail fast in env validation) and drop the literal from both files; in the test, read it from process.env and let the run fail loudly if it's missing.

As per path instructions: "Flag hardcoded secrets ... JWT secrets ...; all configuration belongs in environment variables documented in .env.example."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/refund.test.js` at line 26, Remove the hardcoded JWT fallback from the
test constant and the production secret configuration in authMiddleware.js.
Require JWT_SECRET during environment validation so startup fails when it is
missing, and have the test read the environment value directly; document the
required variable in .env.example.

Source: Path instructions

@Ahbiz
Ahbiz force-pushed the feature/non-custodial-refunds branch from 81b20ab to 0146810 Compare July 23, 2026 21:28
@zeemscript
zeemscript merged commit bd0487a into Deen-Bridge:dev Jul 23, 2026
3 checks passed
zeemscript added a commit that referenced this pull request Jul 25, 2026
Merge pull request #69 from Ahbiz/feature/non-custodial-refunds

feat(stellar): implement non-custodial refund and dispute flow (#62)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants