-
Notifications
You must be signed in to change notification settings - Fork 71
Feat: Re implement functional authcontroller and connect use cases #147
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.
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
Feat: Re implement functional authcontroller and connect use cases #147
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis change replaces the stubbed authentication controller with a fully functional implementation, connecting all authentication endpoints to their respective use cases. It introduces new use cases for wallet verification and wallet format validation, updates authentication routes to use the new controller, and removes the stub controller. A minor update is made to a utility import in the certificate module. No changes are made to DTOs or utility helpers. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthController
participant UseCase
participant Repository
Client->>AuthController: POST /api/v1/auth/register (RegisterDto)
AuthController->>UseCase: SendVerificationEmailUseCase.execute(RegisterDto)
UseCase->>Repository: Create user, send email
Repository-->>UseCase: Result
UseCase-->>AuthController: Success/Error
AuthController-->>Client: HTTP 200/400/500
Client->>AuthController: POST /api/v1/auth/verify-wallet (WalletVerificationRequestDto)
AuthController->>UseCase: VerifyWalletUseCase.execute(WalletVerificationRequestDto)
UseCase->>External: Stellar Horizon API
UseCase-->>AuthController: Verification result
AuthController-->>Client: HTTP 200/400/500
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/modules/certificate/infrastructure/utils/pdfGenerator.ts (1)
25-29: Use a resilient asset path resolver instead of a brittle __dirname-based relative pathThe code currently embeds the logo with:
const logoBytes = fs.readFileSync( path.join(__dirname, "../../../../assets/VolunChain.png") );Because your
assets/VolunChain.pngfolder lives at the repo root (not undersrc), this four-level..traverse actually resolves tosrc/assets/...and will almost certainly break once you transpile or bundle intodist. Instead, resolve against the project root (or a configurable base path) and ensure assets are copied into your final output:• File: src/modules/certificate/infrastructure/utils/pdfGenerator.ts
Lines: 25–29Suggested change:
- const logoBytes = fs.readFileSync( - path.join(__dirname, "../../../../assets/VolunChain.png") - ); + // Resolve from project root so path remains correct after build + const logoPath = path.resolve(process.cwd(), "assets", "VolunChain.png"); + const logoBytes = fs.readFileSync(logoPath);Also verify your build/post‐build steps copy the entire
assets/directory intodist/(e.g., via a copy plugin or script) so thatprocess.cwd()continues to point to a folder containingassets/VolunChain.png.
🧹 Nitpick comments (9)
src/modules/certificate/infrastructure/utils/pdfGenerator.ts (4)
30-34: Make QR generation non-fatal (graceful fallback)Even with the import fixed, QR generation may fail (missing asset, library, network, etc.). Don’t fail certificate creation because of QR. Wrap it and proceed without QR when unavailable.
- const qrBuffer = await generateQRCode( - `https://volunchain.org/verify/${uniqueId}` - ); - const qrImage = await pdfDoc.embedPng(qrBuffer); + let qrImage: any; + try { + const qrBuffer = await generateQRCode( + `https://volunchain.org/verify/${uniqueId}` + ); + qrImage = await pdfDoc.embedPng(qrBuffer); + } catch { + qrImage = undefined; // QR is optional; continue without it + }
39-41: Guard against undefined QR image when computing dimensionsAvoid accessing
.scaleon an undefinedqrImage.- const qrDims = qrImage.scale(0.4); + const qrDims = qrImage ? qrImage.scale(0.4) : undefined;
124-130: Draw QR only if it existsPrevent runtime errors and render without QR when generation fails.
- page.drawImage(qrImage, { - x: 842 - qrDims.width - 40, - y: 40, - width: qrDims.width, - height: qrDims.height, - }); + if (qrImage && qrDims) { + page.drawImage(qrImage, { + x: 842 - qrDims.width - 40, + y: 40, + width: qrDims.width, + height: qrDims.height, + }); + }
5-5: Avoid “Invalid Date” in PDFs; handle bad eventDate valuesFormatting an invalid date shows “Invalid Date”. Fall back to the raw input (or another default) when parsing fails.
-import { format } from "date-fns"; +import { format, isValid } from "date-fns";- const eventDateFormatted = format(new Date(eventDate), "EEEE do MMMM yyyy"); + const dateObj = new Date(eventDate); + const eventDateFormatted = isValid(dateObj) + ? format(dateObj, "EEEE do MMMM yyyy") + : eventDate;Also applies to: 93-101
src/modules/auth/use-cases/wallet-format-validation.usecase.ts (1)
12-23: Flatten nested validation errors & avoid double-validation
execute()re-validates a payload that was already validated in the controller and only surfaces root–levelconstraints, so nested DTOs (or future additions) will silently drop their error messages.Consider:
-const messages = errors.flatMap((e) => - Object.values(e.constraints ?? {}) -); +import { ValidationError } from "class-validator"; + +function collectMessages(errs: ValidationError[]): string[] { + return errs.flatMap(e => + e.constraints ? Object.values(e.constraints) : collectMessages(e.children ?? []) + ); +} +const messages = collectMessages(errors);or rely on the controller’s first validation and accept a typed
ValidateWalletFormatDtohere to avoid the extra pass.src/modules/auth/use-cases/verify-wallet.usecase.ts (1)
50-59: Strengthen signature decoding
Buffer.from(signature, "base64")almost never throws; invalid input silently yields an empty or truncated buffer.
Recommend explicit validation:const sig = Buffer.from(signature, "base64"); if (sig.length !== 64) { return { verified: false, walletAddress, error: "Invalid signature length" }; }This avoids passing malformed data into
Keypair.verify.src/routes/authRoutes.ts (1)
16-20: Route duplication may cause confusionBoth
POST /send-verification-emailandPOST /resend-verificationcall the same controller method.
If both are required, add comments clarifying their intended difference; otherwise remove one to keep the public API surface minimal.src/modules/auth/presentation/controllers/Auth.controller.ts (2)
41-50: Do not leak raw class-validator objects to clients
validateOr400returns the entireValidationError[], exposing internal property names and validation metadata.
Return only the human-readable constraint messages or a custom error schema to avoid coupling clients to implementation details.
64-69: Status code mapping looks inverted for registration failuresDuring registration the only expected failure from
SendVerificationEmailUseCaseis usually “User already exists”, not “User not found”.
Map known conditions explicitly and default to500for unexpected ones:const status = message === "User already exists" ? 409 : 500;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
prisma/migrations/20250808020526_init/migration.sql(1 hunks)src/modules/auth/presentation/controllers/Auth.controller.stub.ts(0 hunks)src/modules/auth/presentation/controllers/Auth.controller.ts(1 hunks)src/modules/auth/use-cases/verify-wallet.usecase.ts(1 hunks)src/modules/auth/use-cases/wallet-format-validation.usecase.ts(1 hunks)src/modules/certificate/infrastructure/utils/pdfGenerator.ts(1 hunks)src/routes/authRoutes.ts(2 hunks)
💤 Files with no reviewable changes (1)
- src/modules/auth/presentation/controllers/Auth.controller.stub.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/modules/auth/use-cases/wallet-format-validation.usecase.ts (1)
src/modules/auth/dto/wallet-validation.dto.ts (1)
ValidateWalletFormatDto(3-15)
| -- DropForeignKey | ||
| ALTER TABLE "Certificate" DROP CONSTRAINT "Certificate_volunteerId_fkey"; | ||
|
|
||
| -- DropForeignKey | ||
| ALTER TABLE "CertificateDownloadLog" DROP CONSTRAINT "CertificateDownloadLog_certificateId_fkey"; | ||
|
|
||
| -- DropForeignKey | ||
| ALTER TABLE "NFT" DROP CONSTRAINT "NFT_organizationId_fkey"; | ||
|
|
||
| -- DropForeignKey | ||
| ALTER TABLE "NFT" DROP CONSTRAINT "NFT_userId_fkey"; | ||
|
|
||
| -- DropForeignKey | ||
| ALTER TABLE "Project" DROP CONSTRAINT "Project_organizationId_fkey"; | ||
|
|
||
| -- DropForeignKey | ||
| ALTER TABLE "UserVolunteer" DROP CONSTRAINT "UserVolunteer_userId_fkey"; | ||
|
|
||
| -- DropForeignKey | ||
| ALTER TABLE "UserVolunteer" DROP CONSTRAINT "UserVolunteer_volunteerId_fkey"; | ||
|
|
||
| -- DropForeignKey | ||
| ALTER TABLE "Volunteer" DROP CONSTRAINT "Volunteer_projectId_fkey"; | ||
|
|
||
| -- DropForeignKey | ||
| ALTER TABLE "escrows" DROP CONSTRAINT "escrows_user_id_fkey"; | ||
|
|
||
| -- DropIndex | ||
| DROP INDEX "UserVolunteer_userId_volunteerId_key"; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Referential integrity lost – foreign keys dropped but never re-added
The migration removes every FK on Certificate, NFT, Project, UserVolunteer, Volunteer and escrows yet does not recreate them afterwards.
This leaves the DB without critical constraints, allowing orphaned rows and breaking Prisma’s relation expectations.
Re-add the FKs (with ON DELETE … clauses matching domain rules) in the same migration or a follow-up script before shipping.
🤖 Prompt for AI Agents
In prisma/migrations/20250808020526_init/migration.sql between lines 8 and 37,
the migration drops multiple foreign key constraints but does not re-add them,
which breaks referential integrity. To fix this, add SQL statements after the
DROP CONSTRAINT commands to recreate each foreign key constraint with
appropriate ON DELETE rules that reflect the domain logic, ensuring the database
maintains proper relational integrity and aligns with Prisma's expectations.
| import { Keypair, StrKey, Horizon } from "@stellar/stellar-sdk"; | ||
| import { VerifyWalletDto } from "../dto/wallet-validation.dto"; | ||
| import { horizonConfig } from "../../../config/horizon.config"; | ||
|
|
||
| type WalletVerificationResult = { | ||
| verified: boolean; | ||
| walletAddress: string; | ||
| error?: string; | ||
| }; | ||
|
|
||
| export class VerifyWalletUseCase { | ||
| async execute(input: VerifyWalletDto): Promise<WalletVerificationResult> { | ||
| const { walletAddress, signature, message } = input; | ||
|
|
||
| // Validate public key format | ||
| if (!StrKey.isValidEd25519PublicKey(walletAddress)) { | ||
| return { | ||
| verified: false, | ||
| walletAddress, | ||
| error: "Invalid Stellar public key", | ||
| }; | ||
| } | ||
|
|
||
| // Check that account exists on Horizon network before signature verification | ||
| try { | ||
| const server = new Horizon.Server(horizonConfig.url, { | ||
| allowHttp: horizonConfig.url.startsWith("http://"), | ||
| }); | ||
| await server.accounts().accountId(walletAddress).call(); | ||
| } catch (err: unknown) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Horizon.Server is not part of @stellar/stellar-sdk runtime
@stellar/stellar-sdk exports Server directly; the Horizon namespace only contains types. At runtime new Horizon.Server() will throw TypeError: Cannot read properties of undefined.
-import { Keypair, StrKey, Horizon } from "@stellar/stellar-sdk";
+import { Keypair, StrKey, Server } from "@stellar/stellar-sdk";
-const server = new Horizon.Server(horizonConfig.url, {
+const server = new Server(horizonConfig.url, {
allowHttp: horizonConfig.url.startsWith("http://"),
});Update the import and instantiation to prevent a production crash.
📝 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.
| import { Keypair, StrKey, Horizon } from "@stellar/stellar-sdk"; | |
| import { VerifyWalletDto } from "../dto/wallet-validation.dto"; | |
| import { horizonConfig } from "../../../config/horizon.config"; | |
| type WalletVerificationResult = { | |
| verified: boolean; | |
| walletAddress: string; | |
| error?: string; | |
| }; | |
| export class VerifyWalletUseCase { | |
| async execute(input: VerifyWalletDto): Promise<WalletVerificationResult> { | |
| const { walletAddress, signature, message } = input; | |
| // Validate public key format | |
| if (!StrKey.isValidEd25519PublicKey(walletAddress)) { | |
| return { | |
| verified: false, | |
| walletAddress, | |
| error: "Invalid Stellar public key", | |
| }; | |
| } | |
| // Check that account exists on Horizon network before signature verification | |
| try { | |
| const server = new Horizon.Server(horizonConfig.url, { | |
| allowHttp: horizonConfig.url.startsWith("http://"), | |
| }); | |
| await server.accounts().accountId(walletAddress).call(); | |
| } catch (err: unknown) { | |
| import { Keypair, StrKey, Server } from "@stellar/stellar-sdk"; | |
| import { VerifyWalletDto } from "../dto/wallet-validation.dto"; | |
| import { horizonConfig } from "../../../config/horizon.config"; | |
| type WalletVerificationResult = { | |
| verified: boolean; | |
| walletAddress: string; | |
| error?: string; | |
| }; | |
| export class VerifyWalletUseCase { | |
| async execute(input: VerifyWalletDto): Promise<WalletVerificationResult> { | |
| const { walletAddress, signature, message } = input; | |
| // Validate public key format | |
| if (!StrKey.isValidEd25519PublicKey(walletAddress)) { | |
| return { | |
| verified: false, | |
| walletAddress, | |
| error: "Invalid Stellar public key", | |
| }; | |
| } | |
| // Check that account exists on Horizon network before signature verification | |
| try { | |
| const server = new Server(horizonConfig.url, { | |
| allowHttp: horizonConfig.url.startsWith("http://"), | |
| }); | |
| await server.accounts().accountId(walletAddress).call(); | |
| } catch (err: unknown) { | |
| // …existing error handling… | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/modules/auth/use-cases/verify-wallet.usecase.ts lines 1 to 30, the code
imports Horizon from "@stellar/stellar-sdk" and tries to instantiate the server
with new Horizon.Server(), but Horizon.Server is undefined at runtime causing a
crash. Fix this by importing Server directly from "@stellar/stellar-sdk" and
instantiate it with new Server(...) instead of new Horizon.Server(...).
| import fs from "fs"; | ||
| import path from "path"; | ||
| import { generateQRCode } from "./qrGenerator"; | ||
| // import { generateQRCode } from "./qrGenerator"; //Function not found, commented out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Critical: Commented-out QR import breaks build
generateQRCode is still used (Lines 30–34) but its import was commented out. This will fail TypeScript compilation at minimum and break PDF generation at runtime.
Apply this minimal fix to restore functionality:
-// import { generateQRCode } from "./qrGenerator"; //Function not found, commented out
+import { generateQRCode } from "./qrGenerator";If the module path truly changed or the file is missing, please locate or re-add it. You can verify with:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for qrGenerator module files..."
fd -a -t f 'qrGenerator.(ts|js)'
echo "Searching for generateQRCode definitions..."
rg -n 'export\s+(?:async\s+)?function\s+generateQRCode|export\s+const\s+generateQRCode|module\.exports\s*=\s*{[^}]*generateQRCode'Length of output: 751
We need to inspect the qrGenerator.ts to confirm the exported function and correct import path before updating. Let’s dump its contents:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the qrGenerator file
file_path=$(fd -a -t f 'qrGenerator.ts' | head -n1)
echo "Found qrGenerator at: $file_path"
# Show the first 50 lines to confirm the export
sed -n '1,50p' "$file_path"Length of output: 558
Let’s inspect pdfGenerator.ts to confirm the current imports and where generateQRCode is used:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Top of pdfGenerator.ts ==="
sed -n '1,20p' src/modules/certificate/infrastructure/utils/pdfGenerator.ts
echo
echo "=== Occurrences of generateQRCode ==="
grep -R -n 'generateQRCode' src/modules/certificate/infrastructure/utils/pdfGenerator.tsLength of output: 1004
Restore and correct QRCode import
The generateQRCode function is still invoked around line 30, but its import was commented out and pointed to a non-existent file. Replace it with the correct relative path into src/shared/infrastructure/utils/qrGenerator.ts:
- // import { generateQRCode } from "./qrGenerator"; //Function not found, commented out
+ import { generateQRCode } from "../../../../shared/infrastructure/utils/qrGenerator";Make sure that this path aligns with your tsconfig (e.g. adjust for any baseUrl or path aliases).
📝 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.
| // import { generateQRCode } from "./qrGenerator"; //Function not found, commented out | |
| -import { generateQRCode } from "./qrGenerator"; //Function not found, commented out | |
| +import { generateQRCode } from "../../../../shared/infrastructure/utils/qrGenerator"; |
🤖 Prompt for AI Agents
In src/modules/certificate/infrastructure/utils/pdfGenerator.ts at line 4, the
import for generateQRCode is commented out and points to a non-existent file,
but the function is still used around line 30. Restore the import statement and
update its path to the correct relative location:
src/shared/infrastructure/utils/qrGenerator.ts. Verify and adjust the import
path according to your tsconfig settings, such as baseUrl or path aliases, to
ensure it resolves correctly.
🚀 Volunchain Pull Request
Mark with an
xall the checkboxes that apply (like[x])📌 Type of Change
📝 Changes description
Implemented a function AuthController that doesn't return 501 error codes everytime. Each API endpoint corresponds to its specific usecase. Replaced stub methods with real implementations. Wired up request DTOs to use cases correctly.
Created 2 new usecases: Wallet Format Verification and Wallet Validation (Stellar SDK).
📸 Evidence (A photo is required as evidence)
API implemented.
⏰ Time spent breakdown
4 hours (mostly in the Wallet Validation Use Case)
🌌 Comments
Any comments, code reviews and feedback is appreciated! Anything I missed or something that could be included, don't doubt to contact me!
Thank you for contributing to Volunchain, we are glad that you have chosen us as your project of choice and we hope that you continue to contribute to this great project, so that together we can make our mark at the top!
Summary by CodeRabbit
New Features
Bug Fixes
Chores