Skip to content

Prepare Stellar Binding #197

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
iNeocero wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Prepare Stellar Binding #197

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
43 changes: 13 additions & 30 deletions src/modules/nft/presentation/controllers/NFTController.stub.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,20 @@
import { Request, Response } from "express";
import { Request, Response } from 'express';
Copy link

@coderabbitai coderabbitai bot Sep 3, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix duplicate Express import (Biome noRedeclare).

You’re importing Request/Response twice; Biome flags this and CI will fail.

Apply:

-import { Request, Response } from 'express';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { Request, Response } from 'express';
🧰 Tools
🪛 Biome (2.1.2)

[error] 2-2: Shouldn't redeclare 'Request'. Consider to delete it or rename it.

'Request' is defined here:

(lint/suspicious/noRedeclare)

[error] 2-2: Shouldn't redeclare 'Response'. Consider to delete it or rename it.

'Response' is defined here:

(lint/suspicious/noRedeclare)

🤖 Prompt for AI Agents 
In src/modules/nft/presentation/controllers/NFTController.stub.ts around line 2,
there is a duplicate import of Request and Response from 'express' causing a
Biome noRedeclare error; remove the redundant import (or consolidate any
separate express imports into a single import statement that includes Request
and Response) so only one import from 'express' remains in the file, then save
and run the linter to confirm the noRedeclare issue is resolved.

import { prepareClaimTx } from '../../use-cases/prepareClaimTx.use-case';
import { submitSignedClaim } from '../../use-cases/submitSignedClaim.use-case';

/**
* Stub controller for NFT functionality
* This replaces the original controller that referenced deleted services
* TODO: Implement proper NFT controller using new modular architecture
*/
class NFTController {
async createNFT(req: Request, res: Response) {
res.status(501).json({
message: "NFT service temporarily disabled during migration",
error: "Service migration in progress"
});
export class NFTController {
static async prepareClaim(req: Request, res: Response) {
// TODO(security): Validar que el usuario autenticado puede reclamar el NFT.
Copy link

@coderabbitai coderabbitai bot Sep 3, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Implement authorization now; don’t trust claimantPublicKey from the client.

Enforce that the authenticated user is allowed to claim this NFT (e.g., tie claimantPublicKey to the session/user and verify nftId is in a claimable state). Return 403 when not authorized.

🏁 Script executed:

#!/bin/bash
# Check routing and auth gating for these endpoints
rg -nP -C2 "(router|app)\\.(post|use)\\(.*nft/claim/(prepare|submit)" -g '!**/dist/**'
rg -nP -C2 "NFTController\\.(prepareClaim|submitClaim)|new\\s+NFTController\\(\\)"
rg -nP -C2 "(auth|passport|jwt|ensure.*Auth|verify.*Token|requireAuth)"

Length of output: 101504

🏁 Script executed:

#!/bin/bash
sed -n '1,200p' src/routes/nftRoutes.ts

Length of output: 805

🏁 Script executed:

#!/bin/bash
sed -n '1,200p' src/modules/nft/presentation/controllers/NFTController.stub.ts

Length of output: 897

Enforce authorization on NFT claim flows

  • In src/routes/nftRoutes.ts, add authenticated routes for preparing and submitting claims, e.g.:

    import { authMiddleware } from "../middleware/authMiddleware";

router.post("/nfts/claim/prepare", authMiddleware, NFTController.prepareClaim);
router.post("/nfts/claim/submit",  authMiddleware, NFTController.submitClaim);

  • In src/modules/nft/presentation/controllers/NFTController.stub.ts, replace the TODO with logic that reads req.user.id (populated by authMiddleware), verifies that claimantPublicKey belongs to that user and that nftId is in a claimable state, and returns 403 on any authorization failure.

🤖 Prompt for AI Agents 
In src/modules/nft/presentation/controllers/NFTController.stub.ts around line 8,
replace the TODO with authorization logic that reads req.user.id (set by
authMiddleware), fetches/validates that the claimantPublicKey belongs to that
user (e.g. via userService or keysRepository), checks that the nftId is in a
claimable state (via nftService/status check), and if either check fails
immediately respond with res.status(403). If both checks pass, continue the
existing flow; surface any service errors as appropriate but do not proceed on
authorization failures.

const { nftId, claimantPublicKey } = req.body;
const result = await prepareClaimTx({ nftId, claimantPublicKey });
res.status(200).json(result);
}
Comment on lines +6 to 12
Copy link

@coderabbitai coderabbitai bot Sep 3, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add authZ, input validation, and error handling; wire NextFunction and try/catch.

Current handlers trust req.body and can throw unhandled rejections. Validate inputs, surface 400s, and forward unexpected errors to the error middleware.

Apply within these ranges:

-  static async prepareClaim(req: Request, res: Response) {
-    // TODO(security): Validar que el usuario autenticado puede reclamar el NFT.
-    const { nftId, claimantPublicKey } = req.body;
-    const result = await prepareClaimTx({ nftId, claimantPublicKey });
-    res.status(200).json(result);
-  }
+  static async prepareClaim(req: Request, res: Response, next: NextFunction) {
+    try {
+      // TODO(security): Validate that the authenticated user can claim the NFT.
+      const { nftId, claimantPublicKey } = req.body ?? {};
+      if (typeof nftId !== 'string' || typeof claimantPublicKey !== 'string' || !nftId || !claimantPublicKey) {
+        return res.status(400).json({
+          error: 'Invalid payload',
+          details: { nftId: 'required string', claimantPublicKey: 'required string' },
+        });
+      }
+      const result = await prepareClaimTx({ nftId, claimantPublicKey });
+      return res.status(200).json(result);
+    } catch (err) {
+      return next(err);
+    }
+  }
-  static async submitClaim(req: Request, res: Response) {
-    const { nftId, signedXdr } = req.body;
-    const result = await submitSignedClaim({ nftId, signedXdr });
-    res.status(202).json(result);
-  }
+  static async submitClaim(req: Request, res: Response, next: NextFunction) {
+    try {
+      const { nftId, signedXdr } = req.body ?? {};
+      if (typeof nftId !== 'string' || typeof signedXdr !== 'string' || !nftId || !signedXdr) {
+        return res.status(400).json({
+          error: 'Invalid payload',
+          details: { nftId: 'required string', signedXdr: 'required base64 XDR string' },
+        });
+      }
+      const result = await submitSignedClaim({ nftId, signedXdr });
+      return res.status(202).json(result);
+    } catch (err) {
+      return next(err);
+    }
+  }

Also update the import (outside the changed ranges):

-import { Request, Response } from "express";
+import { Request, Response, NextFunction } from "express";

Also applies to: 14-17

🤖 Prompt for AI Agents 
In src/modules/nft/presentation/controllers/NFTController.stub.ts around lines 6
to 12 (and also apply the same pattern to lines 14 to 17), the controller
currently trusts req.body and can produce unhandled rejections; update the
method signature to accept (req: Request, res: Response, next: NextFunction),
wrap the handler body in a try/catch, perform explicit input validation for
nftId and claimantPublicKey (return res.status(400).json({ error: '...' }) for
missing/invalid values), perform an authorization check to ensure req.user (or
an injected authZ helper) is allowed to claim the NFT and return 403 if not
allowed, call prepareClaimTx inside the try and on success
res.status(200).json(result), and on unexpected errors call next(err) to forward
to error middleware; also update the imports at the top to include NextFunction
(and any validator/authZ helpers you use).

💡 Verification agent

🧩 Analysis chain

Static-only class vs default instance export will break callers.

You export an instance (export default new NFTController()), but static methods aren’t available on instances, so legacy call sites like defaultExport.prepareClaim will be undefined at runtime.

Safer bridge fix (keeps static methods and a callable default):

-export default new NFTController();
+export default {
+  prepareClaim: NFTController.prepareClaim,
+  submitClaim: NFTController.submitClaim,
+};

Alternatively, remove static and keep an instance API, or export the class as default and update all routes to call NFTController.prepareClaim.

🏁 Script executed:

#!/bin/bash
# Find usages that import the default and call controller methods
rg -nP -C2 "import\\s+\\w+\\s+from\\s+['\"]\\.?\\.?/.*NFTController" -g '!**/dist/**'
rg -nP -C2 "\\w+\\.prepareClaim\\(|\\w+\\.submitClaim\\(" -g '!**/dist/**'

Length of output: 837

Export mismatch: static methods vs default instance
In src/modules/nft/presentation/controllers/NFTController.stub.ts you declare prepareClaim/submitClaim as static but do export default new NFTController(), so callers doing

import NFTController from ".../NFTController.stub";
NFTController.prepareClaim(...)

will get undefined at runtime. Fix by exporting the class instead of an instance:

-export default new NFTController();
+export default NFTController;

Alternatively, convert these to instance methods (remove static) or explicitly export a bridge object:

export default {
  prepareClaim: NFTController.prepareClaim,
  submitClaim:  NFTController.submitClaim,
};
🤖 Prompt for AI Agents 
In src/modules/nft/presentation/controllers/NFTController.stub.ts around lines
6-12 there is a mismatch: methods are declared static but the module exports an
instance, causing callers like `NFTController.prepareClaim(...)` to be
undefined; fix by either (A) exporting the class itself (replace the exported
instance with `export default NFTController`) so static calls work, or (B)
convert the static methods to instance methods (remove `static`) and keep
exporting an instance, or (C) keep static methods and export a bridge object
mapping those methods to the class (e.g., `export default { prepareClaim:
NFTController.prepareClaim, submitClaim: NFTController.submitClaim }`); also
update any imports/usage to match the chosen approach.


async getNFTById(req: Request, res: Response) {
res.status(501).json({
message: "NFT service temporarily disabled during migration",
error: "Service migration in progress"
});
}

async getNFTsByUserId(req: Request, res: Response) {
res.status(501).json({
message: "NFT service temporarily disabled during migration",
error: "Service migration in progress"
});
}

async deleteNFT(req: Request, res: Response) {
res.status(501).json({
message: "NFT service temporarily disabled during migration",
error: "Service migration in progress"
});
static async submitClaim(req: Request, res: Response) {
const { nftId, signedXdr } = req.body;
const result = await submitSignedClaim({ nftId, signedXdr });
res.status(202).json(result);
}
}

Expand Down