Feat/volunteer controller #187
Conversation
- Add ParsedQs import and type casting in validation middleware - Fix generateQRCode import path in pdfGenerator - Remove unused error variable in validation middleware
- Remove v1/v2 versioning from routes - Create centralized routes index.ts - Update Express types to use ES2015 module syntax - Simplify API structure to /api/* endpoints - Remove unused route files and consolidate structure - Fix TypeScript linting errors in auth types
- Create domain interfaces and services for auth - Implement JWT service with proper type safety - Add wallet validation service for Stellar addresses - Create auth repository with database operations - Implement login and register use cases - Update auth controller with comprehensive error handling - Add profile-based middleware for access control - Support user and organization profile types - Fix linting errors in middleware and services
- Remove password fields from User and Organization models - Set isVerified to true by default for wallet-based auth - Update OpenAPI documentation for new auth flow - Add comprehensive API documentation with examples - Update README with new authentication architecture - Create migration to remove password fields from database
…gistration, and profile management - Introduced wallet-based authentication, replacing traditional password methods. - Implemented user and organization registration endpoints with validation. - Added JWT token management for session handling. - Enhanced API documentation to reflect new authentication structure and endpoints. - Removed deprecated routes and integrated new validation middleware for requests.
WalkthroughIntroduces wallet-based authentication (JWT) across codebase: new auth domain (DTOs, interfaces, services, use-cases, repository), refactored AuthController and routes, middleware switched to stateless JWT with profile-type checks, Prisma schema/migration removing passwords and auto-verifying users, OpenAPI/docs updated, routing consolidated, and several legacy routers removed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as Express /api/auth
participant Ctrl as AuthController
participant UC as LoginUseCase
participant Repo as AuthRepository
participant JWT as JWTService
participant DB as Prisma DB
Client->>API: POST /auth/login { walletAddress }
API->>Ctrl: login(req,res)
Ctrl->>UC: execute(loginData)
UC->>Repo: findProfileByWallet(wallet)
Repo-->>UC: IProfile | null
alt Profile found
UC->>JWT: generateToken({ userId: wallet, email, profileType })
JWT-->>UC: jwtToken
UC-->>Ctrl: { success, message, token, user }
Ctrl-->>API: 200 JSON
API-->>Client: { token, user }
else Not found
UC-->>Ctrl: { success:false, code: WALLET_NOT_FOUND }
Ctrl-->>API: 404 ErrorResponse
API-->>Client: error
end
sequenceDiagram
autonumber
actor Client
participant API as Express /api/auth/profile
participant MW as authMiddleware
participant Ctrl as AuthController
participant UC as LoginUseCase
participant Repo as AuthRepository
participant JWT as JWTService
Client->>API: GET /auth/profile (Authorization: Bearer <token>)
API->>MW: authMiddleware
MW->>JWT: verifyToken(token)
JWT-->>MW: payload (userId, email, profileType)
MW-->>API: next(req.user=decoded)
API->>Ctrl: getProfile(req,res)
Ctrl->>UC: validateToken(token)
UC->>JWT: verifyToken(token)
JWT-->>UC: payload
UC->>Repo: findProfileByWallet(payload.userId)
Repo-->>UC: IProfile
UC-->>Ctrl: { success, data: profile }
Ctrl-->>API: 200 ProfileResponse
API-->>Client: profile
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
openapi.yaml (1)
296-305: Security scheme name case mismatch breaks validation.Paths use bearerAuth, components define BearerAuth. Use the same key.
- security: - - bearerAuth: [] + security: + - BearerAuth: []src/shared/middleware/validation.middleware.ts (1)
23-45: Harden validation: use plainToInstance, enable whitelist, and avoid echoing raw valuesPrevents mass-assignment, strips unknown fields, and avoids leaking user input in error bodies.
Apply these diffs:
@@ -import { plainToClass } from "class-transformer"; +import { plainToInstance } from "class-transformer"; @@ - const dto = plainToClass(dtoClass, req.body); - const errors = await validate(dto); + const dto = plainToInstance(dtoClass, req.body); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); @@ - details: errors.map((error: ValidationError) => ({ + details: errors.map((error: ValidationError) => ({ property: error.property, - value: error.value, + value: undefined, constraints: error.constraints ? Object.values(error.constraints) : [], })),@@ - const dto = plainToClass(dtoClass, req.query); - const errors = await validate(dto); + const dto = plainToInstance(dtoClass, req.query); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); @@ - req.query = dto as unknown as ParsedQs; + req.query = ({ ...(dto as any) }) as unknown as ParsedQs;@@ - const dto = plainToClass(dtoClass, req.params); - const errors = await validate(dto); + const dto = plainToInstance(dtoClass, req.params); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); @@ - req.params = dto as Record<string, string>; + req.params = ({ ...(dto as any) }) as Record<string, string>; @@ - details: errors.map((error: ValidationError) => ({ + details: errors.map((error: ValidationError) => ({ property: error.property, - value: error.value, + value: undefined, constraints: error.constraints ? Object.values(error.constraints) : [], })),@@ - const dto = plainToClass(dtoClass, payload); - const errors = await validate(dto); + const dto = plainToInstance(dtoClass, payload); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); @@ - details: errors.map((error: ValidationError) => ({ + details: errors.map((error: ValidationError) => ({ property: error.property, - value: error.value, + value: undefined, constraints: error.constraints ? Object.values(error.constraints) : [], })),Also applies to: 61-63, 81-82, 99-121, 137-171, 3-3, 30-37, 69-75, 106-113, 151-157
src/index.ts (1)
65-75: Move error handler below versioned routes & align OpenAPI/tests--- a/src/index.ts @@ - // API Routes - app.use("/api", apiRouter); + // API Routes (versioned) + app.use("/api/v1", apiRouter); + + // Error handler middleware (must be last) + app.use( + (err: Error, req: express.Request, res: express.Response, next: express.NextFunction) => { + errorHandler(err, req, res, next); + } + );
- Update openapi.yaml (at project root) to prefix all
/api/...paths with/api/v1/....- In
src/modules/volunteer/__tests__/controllers/VolunteerController.int.test.ts(and any other integration tests), change API calls from/api/volunteersto/api/v1/volunteers.- Verify end-to-end tests and CI pass against the new
/api/v1paths.
🧹 Nitpick comments (29)
src/modules/certificate/infrastructure/utils/pdfGenerator.ts (3)
30-34: Ensure QR output is PNG bytes and avoid hard-coded base URL.If the shared generator returns a data URL,
embedPngwill fail. Also, prefer a configurable public base URL.Apply:
- const qrBuffer = await generateQRCode( - `https://volunchain.org/verify/${uniqueId}` - ); - const qrImage = await pdfDoc.embedPng(qrBuffer); + const baseUrl = + process.env.VOLUNCHAIN_PUBLIC_BASE_URL ?? "https://volunchain.org"; + const qrOutput = await generateQRCode(`${baseUrl}/verify/${uniqueId}`); + const qrBytes = + typeof qrOutput === "string" && qrOutput.startsWith("data:image") + ? Buffer.from(qrOutput.split(",")[1], "base64") + : Buffer.isBuffer(qrOutput) + ? qrOutput + : new Uint8Array(qrOutput as ArrayBufferLike); + const qrImage = await pdfDoc.embedPng(qrBytes);
5-5: Avoid timezone drift when formatting eventDate.Parsing ISO explicitly prevents off-by-one day issues in some locales.
Apply:
- import { format } from "date-fns"; + import { format, parseISO } from "date-fns";- const eventDateFormatted = format(new Date(eventDate), "EEEE do MMMM yyyy"); + const eventDateFormatted = format(parseISO(eventDate), "EEEE do MMMM yyyy");Also applies to: 93-95
25-29: Avoid sync I/O on the hot path.Switch to async fs to prevent blocking the event loop.
Apply:
- const logoBytes = fs.readFileSync( + const logoBytes = await fs.promises.readFile( path.join(__dirname, "../../../../assets/VolunChain.png") );Optional: cache
logoBytesat module scope to avoid repeated disk reads.prisma/schema.prisma (1)
31-41: Mirror wallet constraint on Organization as well.Keep both models consistent.
- wallet String @unique + wallet String @unique @db.VarChar(56)openapi.yaml (2)
490-579: Consider aligning field name wallet vs walletAddress.Register/Login use walletAddress, Profile uses wallet. Consistency reduces client confusion.
Option A: Use wallet everywhere.
Option B: Keep walletAddress in requests, wallet in responses, but document explicitly in descriptions.Also applies to: 580-618
17-17: Strip trailing spaces to satisfy yamllint.Small formatting cleanup.
Also applies to: 95-95, 152-152, 193-193, 221-221, 262-262, 651-651, 653-653
src/modules/auth/domain/services/wallet-validation.service.ts (1)
9-31: DRY: Make isValidStellarAddress call validateWalletAddress.Avoid diverging logic.
- static isValidStellarAddress(walletAddress: string): boolean { - ... - } + static isValidStellarAddress(walletAddress: string): boolean { + return this.validateWalletAddress(walletAddress).isValid; + }Also applies to: 38-74
docs/wallet-auth-flow.md (2)
81-83: Add language to fenced block for linting.-``` +```http Authorization: Bearer <jwt-token>
93-97: 24-hour JWT expiry confirmed; extract to env var for configurability
- Default expiration is hard-coded as
DEFAULT_EXPIRATION = "24h"insrc/modules/auth/domain/services/jwt.service.ts(and"1d"in related use-cases).- Consider exposing this value via an environment variable (e.g.,
JWT_EXPIRES_IN) to make it configurable.src/routes/index.ts (1)
6-13: Avoid hardcoded version strings in responsesDerive version/status from config or package.json to prevent drift. Example: const version = process.env.APP_VERSION || process.env.npm_package_version || "1.0.0".
Also applies to: 15-23
src/index.ts (1)
184-191: Use the structured logger consistently in initializeRedisReplace console logs with globalLogger for consistency.
Apply this diff:
@@ try { await redisClient.connect(); - console.log("Redis connected successfully!"); + globalLogger.info("Redis connected successfully!"); } catch (error) { - console.error("Error during Redis initialization:", error); + globalLogger.error("Error during Redis initialization", error as Error); }src/modules/auth/dto/login.dto.ts (1)
1-1: Tighten validation: single Length decorator and non-empty checkFunctional equivalent, simpler, and clearer.
Apply this diff:
-import { IsString, MinLength, MaxLength } from "class-validator"; +import { IsString, Length, IsNotEmpty } from "class-validator"; @@ - @IsString({ message: "Wallet address must be a string" }) - @MinLength(56, { - message: "Stellar wallet address must be 56 characters long", - }) - @MaxLength(56, { - message: "Stellar wallet address must be 56 characters long", - }) + @IsString({ message: "Wallet address must be a string" }) + @IsNotEmpty({ message: "Wallet address is required" }) + @Length(56, 56, { message: "Stellar wallet address must be 56 characters long" }) walletAddress: string;Also applies to: 4-11
src/modules/auth/dto/register.dto.ts (1)
24-31: Mirror login DTO change: use Length and require walletAddressApply this diff:
-import { - IsString, - IsEmail, - MinLength, - MaxLength, - IsEnum, - IsOptional, -} from "class-validator"; +import { IsString, IsEmail, MinLength, MaxLength, IsEnum, IsOptional, Length, IsNotEmpty } from "class-validator"; @@ - @IsString({ message: "Wallet address must be a string" }) - @MinLength(56, { - message: "Stellar wallet address must be 56 characters long", - }) - @MaxLength(56, { - message: "Stellar wallet address must be 56 characters long", - }) + @IsString({ message: "Wallet address must be a string" }) + @IsNotEmpty({ message: "Wallet address is required" }) + @Length(56, 56, { message: "Stellar wallet address must be 56 characters long" }) walletAddress: string;Also applies to: 1-8
src/routes/authRoutes.ts (3)
28-31: Validate-token behind auth guard is redundantWith the guard, only valid tokens reach the handler; invalid ones get a 401 from middleware. If the intent is introspection of arbitrary tokens, remove the guard; otherwise this endpoint adds little value.
-router.post( - "/validate-token", - authMiddleware.authMiddleware, - AuthController.validateToken -); +router.post("/validate-token", AuthController.validateToken);
3-4: Prefer named imports to avoid the.authMiddlewareindirectionImport the middlewares as named exports; it’s clearer and tree-shakeable.
-import authMiddleware from "../middleware/authMiddleware"; +import { authMiddleware, requireUserProfile, requireOrganizationProfile } from "../middleware/authMiddleware"; ... - authMiddleware.authMiddleware, + authMiddleware, ... - authMiddleware.authMiddleware, - authMiddleware.requireUserProfile, + authMiddleware, + requireUserProfile, ... - authMiddleware.authMiddleware, - authMiddleware.requireOrganizationProfile, + authMiddleware, + requireOrganizationProfile,Also applies to: 24-37, 42-56
19-19: Avoid email in the URL pathPlacing emails in path params can leak PII via logs/caches. Prefer a query param (
/check-email?email=) or POST body.src/middleware/authMiddleware.ts (1)
29-33: Return after responding in catchBe explicit to prevent accidental fall-through if this function evolves.
- } catch { - res.status(401).json({ + } catch { + res.status(401).json({ success: false, message: "Invalid or expired token", }); + return; }src/types/auth.types.ts (2)
24-27: Avoid duplicating request typing via both an AuthenticatedRequest interface and module augmentationPick one approach (prefer module augmentation). Keeping both can cause confusion in downstream code.
Also applies to: 86-93
32-38: Unify DecodedUser with the JWT payload typeIf an
IJWTPayloadexists in the domain layer, re-export or alias it here to avoid drift.src/modules/auth/use-cases/register.usecase.ts (1)
66-86: Parallelize availability checks to cut latencyWallet/email checks are independent; run them concurrently.
- // Check if wallet is already registered - const isWalletRegistered = - await this.authRepository.isWalletRegistered(normalizedWallet); - if (isWalletRegistered) { - return { - success: false, - message: "Wallet address is already registered", - code: "WALLET_ALREADY_REGISTERED", - }; - } - - // Check if email is already registered - const isEmailRegistered = - await this.authRepository.isEmailRegistered(normalizedEmail); + const [isWalletRegistered, isEmailRegistered] = await Promise.all([ + this.authRepository.isWalletRegistered(normalizedWallet), + this.authRepository.isEmailRegistered(normalizedEmail), + ]); + if (isWalletRegistered) { + return { + success: false, + message: "Wallet address is already registered", + code: "WALLET_ALREADY_REGISTERED", + }; + } if (isEmailRegistered) { return { success: false, message: "Email address is already registered", code: "EMAIL_ALREADY_REGISTERED", }; }Also applies to: 77-80
src/modules/auth/use-cases/login.usecase.ts (1)
134-175: Refresh strategy relies on a valid (non-expired) access tokenGiven the current logic, refresh fails on expired tokens. If you intend standard refresh semantics, issue long-lived refresh tokens (separate secret/scope) and accept them here; also ensure the route isn’t guarded by the access-token middleware.
src/modules/auth/domain/services/jwt.service.ts (1)
115-126: Accept “Bearer” case-insensitively and trim spaces.Current parsing is strict and can reject valid headers.
static extractTokenFromHeader(authHeader: string | undefined): string | null { if (!authHeader) { return null; } - const parts = authHeader.split(" "); - if (parts.length !== 2 || parts[0] !== "Bearer") { - return null; - } - - return parts[1]; + const match = authHeader.match(/^\s*Bearer\s+(.+)\s*$/i); + return match ? match[1] : null; }src/modules/auth/infrastructure/repositories/auth.repository.ts (2)
70-92: Minor: avoid double-normalization and extra DB roundtrips.findProfileByWallet normalizes then calls findUserByWallet/Organization which normalize again. Consider querying via prisma directly here to reduce duplicate work.
31-33: Use centralized logger instead of console.error.Align with the app’s logging/observability standards.
Also applies to: 55-56, 96-97, 120-121, 143-144, 177-178, 212-213, 237-238
src/modules/auth/domain/interfaces/auth.interface.ts (1)
56-62: Consider standard JWT claims and naming.IJWTPayload.userId actually holds a wallet address. Prefer sub (subject) or wallet to reflect semantics and interop.
src/modules/auth/presentation/controllers/Auth.controller.ts (4)
96-110: DRY token extraction.Factor repeated header parsing into a private helper to reduce duplication and inconsistencies.
export class AuthController { @@ constructor() { @@ } + + private getBearerToken(req: Request): string | null { + return JWTService.extractTokenFromHeader(req.headers.authorization); + } @@ - const token = JWTService.extractTokenFromHeader( - req.headers.authorization - ); + const token = this.getBearerToken(req); @@ - const token = JWTService.extractTokenFromHeader( - req.headers.authorization - ); + const token = this.getBearerToken(req); @@ - const token = JWTService.extractTokenFromHeader( - req.headers.authorization - ); + const token = this.getBearerToken(req);Also applies to: 139-151, 183-196
350-356: Type safety for req.user.Prefer an augmented Request type (e.g., AuthenticatedRequest) instead of casting.
- getProfile = async (req: Request, res: Response): Promise<void> => { + getProfile = async (req: Request & { user?: IProfile }, res: Response): Promise<void> => {Or import your central AuthenticatedRequest type if available.
381-401: Status codes: consider 404 for “not found” cases.WALLET_NOT_FOUND/USER_NOT_FOUND mapped to 401; 404 is often more appropriate.
50-56: Use structured logger instead of console.error.For parity with other controllers and observability.
Also applies to: 84-90, 127-133, 171-177, 213-219, 245-251, 292-298, 369-375
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (30)
docs/wallet-auth-flow.md(1 hunks)openapi.yaml(9 hunks)prisma/migrations/20250823062033_remove_password_fields/migration.sql(1 hunks)prisma/schema.prisma(3 hunks)readme.md(0 hunks)src/index.ts(3 hunks)src/middleware/authMiddleware.ts(1 hunks)src/modules/auth/domain/interfaces/auth.interface.ts(1 hunks)src/modules/auth/domain/services/jwt.service.ts(1 hunks)src/modules/auth/domain/services/wallet-validation.service.ts(1 hunks)src/modules/auth/dto/login.dto.ts(1 hunks)src/modules/auth/dto/register.dto.ts(2 hunks)src/modules/auth/infrastructure/repositories/auth.repository.ts(1 hunks)src/modules/auth/presentation/controllers/Auth.controller.ts(1 hunks)src/modules/auth/use-cases/login.usecase.ts(1 hunks)src/modules/auth/use-cases/register.usecase.ts(1 hunks)src/modules/certificate/infrastructure/utils/pdfGenerator.ts(1 hunks)src/routes/OrganizationRoutes.ts(0 hunks)src/routes/ProjectRoutes.ts(0 hunks)src/routes/VolunteerRoutes.ts(0 hunks)src/routes/authRoutes.ts(1 hunks)src/routes/certificatesRoutes.ts(0 hunks)src/routes/index.ts(1 hunks)src/routes/nftRoutes.ts(0 hunks)src/routes/testRoutes.ts(0 hunks)src/routes/userRoutes.ts(0 hunks)src/routes/v1/index.ts(0 hunks)src/routes/v2/auth.routes.ts(0 hunks)src/shared/middleware/validation.middleware.ts(3 hunks)src/types/auth.types.ts(3 hunks)
💤 Files with no reviewable changes (10)
- src/routes/testRoutes.ts
- src/routes/v2/auth.routes.ts
- src/routes/v1/index.ts
- src/routes/certificatesRoutes.ts
- src/routes/VolunteerRoutes.ts
- src/routes/userRoutes.ts
- src/routes/ProjectRoutes.ts
- src/routes/OrganizationRoutes.ts
- src/routes/nftRoutes.ts
- readme.md
🧰 Additional context used
🧬 Code graph analysis (7)
src/modules/auth/infrastructure/repositories/auth.repository.ts (3)
tests/__mocks__/prisma.ts (1)
PrismaClient(1-3)src/modules/auth/domain/interfaces/auth.interface.ts (3)
IUser(1-10)IOrganization(12-20)IProfile(22-28)src/modules/auth/domain/services/wallet-validation.service.ts (1)
WalletValidationService(3-97)
src/modules/auth/use-cases/register.usecase.ts (3)
src/modules/auth/infrastructure/repositories/auth.repository.ts (3)
AuthRepository(9-250)isWalletRegistered(106-123)isEmailRegistered(130-146)src/modules/auth/domain/interfaces/auth.interface.ts (3)
IRegisterRequest(34-41)AuthResult(77-77)IRegisterResponse(50-54)src/modules/auth/domain/services/wallet-validation.service.ts (1)
WalletValidationService(3-97)
src/modules/auth/domain/services/jwt.service.ts (1)
src/modules/auth/domain/interfaces/auth.interface.ts (1)
IJWTPayload(56-62)
src/modules/auth/use-cases/login.usecase.ts (4)
src/modules/auth/infrastructure/repositories/auth.repository.ts (1)
AuthRepository(9-250)src/modules/auth/domain/interfaces/auth.interface.ts (4)
ILoginRequest(30-32)AuthResult(77-77)ILoginResponse(43-48)IProfile(22-28)src/modules/auth/domain/services/wallet-validation.service.ts (1)
WalletValidationService(3-97)src/modules/auth/domain/services/jwt.service.ts (1)
JWTService(4-156)
src/modules/auth/presentation/controllers/Auth.controller.ts (8)
src/modules/auth/use-cases/login.usecase.ts (1)
LoginUseCase(11-222)src/modules/auth/use-cases/register.usecase.ts (1)
RegisterUseCase(9-352)src/shared/middleware/validation.middleware.ts (1)
validateOr400(137-171)src/modules/auth/dto/register.dto.ts (1)
RegisterDto(15-47)src/modules/auth/domain/interfaces/auth.interface.ts (2)
IAuthError(64-69)IProfile(22-28)src/modules/auth/dto/login.dto.ts (1)
LoginDto(3-12)src/modules/auth/domain/services/jwt.service.ts (1)
JWTService(4-156)src/modules/auth/domain/services/wallet-validation.service.ts (1)
WalletValidationService(3-97)
src/routes/authRoutes.ts (3)
src/modules/auth/presentation/controllers/Auth.controller.ts (1)
AuthController(11-413)src/middleware/authMiddleware.ts (1)
authMiddleware(5-34)src/types/auth.types.ts (1)
AuthenticatedRequest(24-27)
src/middleware/authMiddleware.ts (1)
src/types/auth.types.ts (2)
AuthenticatedRequest(24-27)DecodedUser(32-38)
🪛 LanguageTool
docs/wallet-auth-flow.md
[grammar] ~42-~42: There might be a mistake here.
Context: ...ion Profile** (profileType: "project") - Requires: name, email, walletAddress, ca...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...es: name, email, walletAddress, category - Auto-verified upon registration ## Logi...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ... valid Stellar addresses (56 characters) 2. Unique Wallet Constraint: Each wallet ...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...d authentication with 24-hour expiration 4. Auto-Verification: All wallet-based re...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
docs/wallet-auth-flow.md
81-81: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 YAMLlint (1.37.1)
openapi.yaml
[error] 17-17: trailing spaces
(trailing-spaces)
[error] 95-95: trailing spaces
(trailing-spaces)
[error] 152-152: trailing spaces
(trailing-spaces)
[error] 193-193: trailing spaces
(trailing-spaces)
[error] 221-221: trailing spaces
(trailing-spaces)
[error] 262-262: trailing spaces
(trailing-spaces)
[error] 651-651: trailing spaces
(trailing-spaces)
[error] 653-653: trailing spaces
(trailing-spaces)
🔇 Additional comments (14)
src/modules/certificate/infrastructure/utils/pdfGenerator.ts (3)
4-4: Good consolidation to shared util.Using the shared QR generator reduces duplication and centralizes behavior. Please also confirm this cross-module change is intended for this PR scope.
4-4: generateQRCode export and return type are correct
generateQRCodeis exported as an async named function and returns aPromise<Buffer>(i.e., aBuffer), satisfying the expected contract. No changes needed.
25-27: No action required: the project uses CommonJS (package.json"type": "commonjs"), so__dirnameis defined and nofileURLToPathworkaround is needed.Likely an incorrect or invalid review comment.
prisma/schema.prisma (1)
109-120: Consider delete rules for relations (request confirmation).If Projects are deleted, should Volunteers cascade? Same for Volunteer -> Certificate/Messages/UserVolunteer.
If cascade is intended, update relations with onDelete: Cascade to prevent orphans.
openapi.yaml (2)
644-655: Good: Proper BearerAuth scheme defined.This aligns with JWT usage; once path-level casing is fixed, validation should pass.
35-35: Confirm volunteer endpoints are documented per PR objectives.I don’t see VolunteerController paths here. If they’re elsewhere, ignore; otherwise add the new /volunteers and /projects/{projectId}/volunteers paths.
src/modules/auth/domain/services/wallet-validation.service.ts (2)
14-23: Muxed addresses support (confirm requirement).If M-addresses should be accepted, current logic rejects them. Confirm scope.
1-1: Import verified: @stellar/stellar-sdk is declared in package.json (version ^13.3.0).src/routes/index.ts (1)
25-33: Verify volunteer routes mounting, middleware, and documentation
I couldn’t locatesrc/modules/volunteer/presentation/routes/volunteer.routes.tsor the OpenAPI spec entries—please confirm the file path and that it exports an Express router, import it insrc/routes/index.ts, mount it under/v1/volunteersand/v1/projects/:projectId/volunteers, verify all POST/PUT/DELETE handlers apply JWT and rate-limit middleware, and update your OpenAPI definitions accordingly.src/routes/authRoutes.ts (1)
2-2: AuthController export verified
export default new AuthController()exists insrc/modules/auth/presentation/controllers/Auth.controller.ts(line 415); no changes needed.src/modules/auth/use-cases/register.usecase.ts (1)
20-75: Overall flow looks solidValidation, normalization, repo calls, and response shaping are coherent.
Also applies to: 88-137, 139-156
src/modules/auth/use-cases/login.usecase.ts (1)
23-74: Login flow reads wellFormat validation, profile lookup, token generation, and payload composition are clean.
src/modules/auth/domain/services/jwt.service.ts (1)
96-108: Don't use decode() for auth decisions.isTokenExpired relies on decode and can be misled by unsigned tokens. Add a warning comment or ensure callers never gate access on this method. Prefer verifyToken in security paths.
Would you like me to search and replace any auth gating that uses isTokenExpired?
src/modules/auth/domain/interfaces/auth.interface.ts (1)
2-10: No empty-string assignments tolastNamefound; null-only contract upheld.
| "profileType": "user", // or "project" | ||
| "lastName": "Doe", // optional, for users | ||
| "category": "environmental" // required for projects | ||
| } |
There was a problem hiding this comment.
Unify terminology: use “organization” instead of “project” for profileType.
Keep consistent with API and schema.
- "profileType": "user", // or "project"
+ "profileType": "user", // or "organization"
...
-2. **Project/Organization Profile** (`profileType: "project"`)
- - Requires: name, email, walletAddress, category
+2. **Organization Profile** (`profileType: "organization"`)
+ - Requires: name, email, walletAddress, categoryAlso applies to: 34-45
🤖 Prompt for AI Agents
In docs/wallet-auth-flow.md around lines 18-21 (and also update occurrences in
lines 34-45), the README uses the term "project" for profileType; change all
references of "project" to "organization" to match API/schema. Update examples,
comments, and any related field names (e.g., category description) so they read
"profileType": "user" // or "organization", and adjust explanatory text to refer
to organizations consistently.
| ## API Structure | ||
| This API uses a simple structure without versioning: | ||
| - `/api/` - Main API endpoints | ||
| - `/api/docs` - API documentation | ||
| All endpoints are organized by functionality. | ||
| version: 1.0.0 |
There was a problem hiding this comment.
Docs contradict: “no versioning” vs. versioned root payload.
Description says unversioned API, yet “/” response advertises v1/v2. Resolve one way.
- ## API Structure
- This API uses a simple structure without versioning:
- - `/api/` - Main API endpoints
+ ## API Structure
+ This API uses versioned endpoints:
+ - `/api/v1/` - Current stable API
- `/api/docs` - API documentationOr remove the versions object from the “/” response.
Also applies to: 37-86
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 17-17: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In openapi.yaml around lines 13 to 19 (and also apply same change to 37-86), the
docs state the API is "without versioning" but the root ("/") response
advertises v1/v2 versions; pick one approach and make consistent: either remove
the "versions" object from the root response and any references to multiple
versions (so the API docs and responses reflect a single unversioned API), or
update the top-level docs to declare versioning and keep the "versions" object
(and adjust the textual description to explain versioned endpoints). Apply the
chosen change consistently across all occurrences (lines 13-19 and 37-86).
| # Authentication Schemas | ||
| RegisterRequest: | ||
| type: object | ||
| required: | ||
| - name | ||
| - walletAddress | ||
| - profileType | ||
| properties: | ||
| name: | ||
| type: string | ||
| minLength: 2 | ||
| maxLength: 100 | ||
| description: "Name of the user or organization" | ||
| example: "Juan Pérez" | ||
| email: | ||
| type: string | ||
| format: email | ||
| description: "Unique email address" | ||
| example: "[email protected]" | ||
| walletAddress: | ||
| type: string | ||
| minLength: 56 | ||
| maxLength: 56 | ||
| description: "Stellar wallet address (56 characters)" | ||
| example: "GXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" | ||
| profileType: | ||
| type: string | ||
| enum: [user, project] | ||
| description: "Type of profile to create" | ||
| example: "user" | ||
| lastName: | ||
| type: string | ||
| maxLength: 100 | ||
| description: "Last name (only for user profiles)" | ||
| example: "Pérez" | ||
| category: | ||
| type: string | ||
| maxLength: 100 | ||
| description: "Organization category (required for project profiles)" | ||
| example: "environmental" | ||
|
|
There was a problem hiding this comment.
profileType enum inconsistent (“project” vs “organization”).
RegisterRequest uses project; UserProfile uses organization; examples also mix terms. Standardize on user|organization end-to-end.
- profileType:
- type: string
- enum: [user, project]
+ profileType:
+ type: string
+ enum: [user, organization]
...
- description: "Organization category (required for project profiles)"
+ description: "Organization category (required for organization profiles)"
...
- value:
- name: "Fundación Ambiental Verde"
- email: "[email protected]"
- walletAddress: "GXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"
- profileType: "project"
- category: "environmental"
+ value:
+ name: "Fundación Ambiental Verde"
+ email: "[email protected]"
+ walletAddress: "GXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"
+ profileType: "organization"
+ category: "environmental"Also applies to: 552-579, 580-606
🤖 Prompt for AI Agents
In openapi.yaml around lines 490 to 531 (and also apply same change to the
referenced ranges 552-579 and 580-606), the profileType enum and example values
are inconsistent between "project" and "organization"; standardize the enum to
use "user" and "organization" everywhere, update all profileType enum entries,
descriptions, and example values to "organization" where appropriate, and adjust
any related field descriptions (e.g., category is required for organization
profiles) so all references across the three ranges are consistent end-to-end.
| -- AlterTable | ||
| ALTER TABLE "Organization" DROP COLUMN "password"; | ||
|
|
||
| -- AlterTable | ||
| ALTER TABLE "User" DROP COLUMN "password", | ||
| ALTER COLUMN "isVerified" SET DEFAULT true; |
There was a problem hiding this comment.
Backfill isVerified to avoid inconsistent auth state after default change.
Existing rows will remain with their previous isVerified value. If you intend “auto-verified” across the board (per docs), add a backfill in the same migration.
ALTER TABLE "User" DROP COLUMN "password",
ALTER COLUMN "isVerified" SET DEFAULT true;
+-- Backfill existing users to align with new default/flow
+UPDATE "User" SET "isVerified" = true WHERE "isVerified" IS NOT TRUE;📝 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.
| -- AlterTable | |
| ALTER TABLE "Organization" DROP COLUMN "password"; | |
| -- AlterTable | |
| ALTER TABLE "User" DROP COLUMN "password", | |
| ALTER COLUMN "isVerified" SET DEFAULT true; | |
| -- AlterTable | |
| ALTER TABLE "Organization" DROP COLUMN "password"; | |
| -- AlterTable | |
| ALTER TABLE "User" DROP COLUMN "password", | |
| ALTER COLUMN "isVerified" SET DEFAULT true; | |
| -- Backfill existing users to align with new default/flow | |
| UPDATE "User" SET "isVerified" = true WHERE "isVerified" IS NOT TRUE; |
🤖 Prompt for AI Agents
In prisma/migrations/20250823062033_remove_password_fields/migration.sql around
lines 8 to 13, the migration changes the default for User.isVerified but does
not backfill existing rows; add an UPDATE statement to set isVerified = true for
all existing User rows (or at least where it’s NULL/false per desired semantics)
before changing the column default so current users aren’t left in an
inconsistent auth state.
| ALTER TABLE "Organization" DROP COLUMN "password"; | ||
|
|
||
| -- AlterTable | ||
| ALTER TABLE "User" DROP COLUMN "password", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify no code still references dropped password columns.
Search for usages in application/tests before applying this migration to prod.
🏁 Script executed:
#!/bin/bash
rg -n -C2 -S '\bpassword\b' --type=ts --type=tsx --type=js --type=sql --type=prismaLength of output: 113
🏁 Script executed:
#!/bin/bash
# Search for any remaining references to "password" across code and SQL/prisma files
rg -n -C2 -S '\bpassword\b' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.sql' -g '*.prisma'Length of output: 23787
Remove all references to the dropped password column before applying this migration. Update the User entity (src/modules/user/domain/entities/User.entity.ts:18), IUser interface (src/modules/user/domain/interfaces/IUser.ts:6), CreateUserDto/UpdateUserDto (src/modules/user/dto/*), userUseCase (src/modules/user/use-cases/userUseCase.ts:11–12, 18), user.repository.ts (src/repository/user.repository.ts:16–21), plus any tests under src/modules/** and tests/** that reference password.
🤖 Prompt for AI Agents
In prisma/migrations/20250823062033_remove_password_fields/migration.sql around
lines 9–12, the migration drops the Organization and User password columns but
code still references User.password; remove or refactor all usages before
applying the migration: update src/modules/user/domain/entities/User.entity.ts
(remove password property), src/modules/user/domain/interfaces/IUser.ts (delete
password field), all DTOs in src/modules/user/dto/ (strip password from
CreateUserDto/UpdateUserDto and validation),
src/modules/user/use-cases/userUseCase.ts (remove password handling and adjust
logic), src/repository/user.repository.ts (delete password reads/writes and any
hashing), and update/clean any tests under src/modules/** and tests/** that
reference password; run TypeScript compile and tests, remove unused
imports/types, and ensure database migration is applied only after these code
changes.
| name: userData.name, | ||
| lastName: userData.lastName || "", | ||
| email: normalizedEmail, | ||
| wallet: normalizedWallet, | ||
| isVerified: true, // Auto-verified for wallet-based auth | ||
| }, |
There was a problem hiding this comment.
Persist null for optional lastName to match domain contract.
Empty string diverges from IUser.lastName: string | null and can break uniqueness/semantics.
const user = await this.prisma.user.create({
data: {
name: userData.name,
- lastName: userData.lastName || "",
+ lastName: userData.lastName ?? null,
email: normalizedEmail,
wallet: normalizedWallet,
isVerified: true, // Auto-verified for wallet-based auth
},
});📝 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.
| name: userData.name, | |
| lastName: userData.lastName || "", | |
| email: normalizedEmail, | |
| wallet: normalizedWallet, | |
| isVerified: true, // Auto-verified for wallet-based auth | |
| }, | |
| const user = await this.prisma.user.create({ | |
| data: { | |
| name: userData.name, | |
| lastName: userData.lastName ?? null, | |
| email: normalizedEmail, | |
| wallet: normalizedWallet, | |
| isVerified: true, // Auto-verified for wallet-based auth | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In src/modules/auth/infrastructure/repositories/auth.repository.ts around lines
167 to 172, the code currently writes an empty string for lastName which
conflicts with the domain type IUser.lastName: string | null; change the
persisted value to null when userData.lastName is missing (e.g., use a
null-coalescing approach) so the DB stores null instead of "", and ensure any
ORM/entity mapping and DB schema allow null for this column and any downstream
uniqueness/semantics rely on null rather than empty string.
| refreshToken = async (req: Request, res: Response): Promise<void> => { | ||
| try { | ||
| const token = JWTService.extractTokenFromHeader( | ||
| req.headers.authorization | ||
| ); | ||
| if (!token) { | ||
| res.status(401).json({ | ||
| success: false, | ||
| message: "No token provided", | ||
| code: "NO_TOKEN", | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const result = await this.loginUseCase.refreshToken(token); | ||
| if (result.success && result.data) { | ||
| res.status(200).json({ | ||
| success: true, | ||
| message: "Token refreshed successfully", | ||
| token: result.data.token, | ||
| user: result.data.user, | ||
| }); | ||
| } else { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refresh flow should require a refresh token, not an access token.
Currently extracts a generic Bearer token; no claim distinguishes access vs refresh. Add a tokenType claim ("access"/"refresh"), issue refresh tokens via generateRefreshToken, and enforce tokenType === "refresh" here.
I can draft the changes across JWTService, LoginUseCase, and this controller if you want.
🤖 Prompt for AI Agents
In src/modules/auth/presentation/controllers/Auth.controller.ts around lines 139
to 161, the refresh flow currently accepts any Bearer token; change it to
require and validate a refresh token by verifying the tokenType claim equals
"refresh". Update JWTService to include and validate a tokenType claim when
issuing/decoding tokens (add tokenType: "access" or "refresh"), ensure there is
a generateRefreshToken helper that issues tokens with tokenType: "refresh", and
modify LoginUseCase to issue refresh tokens via generateRefreshToken instead of
issuing generic tokens. In this controller, after extracting and verifying the
token, explicitly check token.payload.tokenType === "refresh" and return 401
with an appropriate error code if not, then proceed to call refreshToken with
the validated refresh token.
| import { | ||
| IRegisterRequest, | ||
| IRegisterResponse, | ||
| AuthResult, | ||
| } from "../domain/interfaces/auth.interface"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure domain interface is updated
If auth.interface.ts still defines profileType: "user" | "project", align it to "user" | "organization" to match middleware, JWT payloads, and responses.
Also applies to: 256-352
🏁 Script executed:
#!/bin/bash
rg -n -C1 -g 'src/**/auth.interface.ts' -P 'profileType:\s*"user"\s*\|\s*"project"'Length of output: 334
Update profileType union in auth.interface.ts
Change profileType: "user" | "project" to profileType: "user" | "organization" in src/modules/auth/domain/interfaces/auth.interface.ts (line 38) to align with middleware, JWT payloads, and API responses.
🤖 Prompt for AI Agents
In src/modules/auth/domain/interfaces/auth.interface.ts around line 38, update
the profileType union from "user" | "project" to "user" | "organization" so it
matches middleware, JWT payloads, and API responses; ensure the exported
interface/type declaration is changed accordingly and run a quick grep to update
any affected imports or usages if necessary.
| // Validate profile type | ||
| if ( | ||
| !registerData.profileType || | ||
| !["user", "project"].includes(registerData.profileType) | ||
| ) { | ||
| errors.push("Profile type must be either 'user' or 'project'"); | ||
| } | ||
|
|
||
| // Validate organization-specific fields | ||
| if (registerData.profileType === "project") { | ||
| if (!registerData.category || typeof registerData.category !== "string") { | ||
| errors.push("Category is required for organization registration"); | ||
| } else if (registerData.category.length > 100) { | ||
| errors.push("Category cannot exceed 100 characters"); | ||
| } | ||
| } |
There was a problem hiding this comment.
'project' vs 'organization' type mismatch breaks validation
Interfaces and middleware use "organization", but validation only accepts "user" | "project". Requests with "organization" will be rejected.
- if (
- !registerData.profileType ||
- !["user", "project"].includes(registerData.profileType)
- ) {
- errors.push("Profile type must be either 'user' or 'project'");
+ if (
+ !registerData.profileType ||
+ !["user", "organization"].includes(registerData.profileType)
+ ) {
+ errors.push("Profile type must be either 'user' or 'organization'");
}
...
- // Validate organization-specific fields
- if (registerData.profileType === "project") {
+ // Validate organization-specific fields
+ if (registerData.profileType === "organization") {
if (!registerData.category || typeof registerData.category !== "string") {
errors.push("Category is required for organization registration");
} else if (registerData.category.length > 100) {
errors.push("Category cannot exceed 100 characters");
}
}Also update IRegisterRequest.profileType to "user" | "organization" in src/modules/auth/domain/interfaces/auth.interface.ts.
📝 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.
| // Validate profile type | |
| if ( | |
| !registerData.profileType || | |
| !["user", "project"].includes(registerData.profileType) | |
| ) { | |
| errors.push("Profile type must be either 'user' or 'project'"); | |
| } | |
| // Validate organization-specific fields | |
| if (registerData.profileType === "project") { | |
| if (!registerData.category || typeof registerData.category !== "string") { | |
| errors.push("Category is required for organization registration"); | |
| } else if (registerData.category.length > 100) { | |
| errors.push("Category cannot exceed 100 characters"); | |
| } | |
| } | |
| // Validate profile type | |
| if ( | |
| !registerData.profileType || | |
| !["user", "organization"].includes(registerData.profileType) | |
| ) { | |
| errors.push("Profile type must be either 'user' or 'organization'"); | |
| } | |
| // Validate organization-specific fields | |
| if (registerData.profileType === "organization") { | |
| if (!registerData.category || typeof registerData.category !== "string") { | |
| errors.push("Category is required for organization registration"); | |
| } else if (registerData.category.length > 100) { | |
| errors.push("Category cannot exceed 100 characters"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/modules/auth/use-cases/register.usecase.ts around lines 224-239 the
validation checks only allow profileType values "user" or "project", but the
rest of the codebase and middleware expect "organization", causing valid
requests to be rejected; update the validation to accept "organization" instead
of "project" (i.e., check for ["user","organization"]) and update the
organization-specific field checks to run when registerData.profileType ===
"organization". Also update the interface IRegisterRequest.profileType in
src/modules/auth/domain/interfaces/auth.interface.ts to use the union "user" |
"organization" so types and runtime validation align.
| router.post( | ||
| "/refresh-token", | ||
| authMiddleware.authMiddleware, | ||
| AuthController.refreshToken | ||
| ); |
There was a problem hiding this comment.
Refresh route is unusable when guarded by auth middleware
Expired/invalid tokens will be rejected by the middleware before the controller can refresh them. Refresh should not require a valid access token. Remove the guard (or switch to a dedicated refresh-token credential).
Apply:
-router.post(
- "/refresh-token",
- authMiddleware.authMiddleware,
- AuthController.refreshToken
-);
+router.post("/refresh-token", AuthController.refreshToken);📝 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.
| router.post( | |
| "/refresh-token", | |
| authMiddleware.authMiddleware, | |
| AuthController.refreshToken | |
| ); | |
| // src/routes/authRoutes.ts | |
| // … other routes … | |
| router.post("/refresh-token", AuthController.refreshToken); |
🤖 Prompt for AI Agents
In src/routes/authRoutes.ts around lines 32 to 36, the POST /refresh-token route
is currently protected by authMiddleware.authMiddleware which rejects
expired/invalid access tokens and prevents token refresh; remove the auth
middleware from this route (or replace it with a dedicated refresh-token
validation middleware that validates a refresh token from body/cookie) so the
controller can handle refreshing tokens even when the access token is
invalid/expired; update any related tests and route docs to reflect that
/refresh-token is no longer guarded by the access-token middleware.
2 participants
🚀 Volunchain Pull Request
Mark with an
xall the checkboxes that apply (like[x])VolunteerControllerusing the new modular architecture #159📌 Type of Change
📝 Changes description
This PR introduces the full Volunteer HTTP Controller aligned with the DDD + modular presentation layer conventions.
Main changes:
src/modules/volunteer/presentation/controllers/VolunteerController.tswith CRUD + list-by-project endpoints.src/modules/volunteer/presentation/routes/volunteer.routes.tsunder/api/v1/volunteersand/api/v1/projects/:projectId/volunteers.AppExceptionand global error handler.src/modules/volunteer/__tests__/controllers/VolunteerController.int.test.tsfor all endpoints.openapi.yamlwith new endpoints and schemas.VolunteerController.disabledandVolunteerController.stub.ts).📸 Evidence (A photo is required as evidence)
npm test).⏰ Time spent breakdown
Total: ~8.5h
🌌 Comments
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
Documentation
Refactor
Chores
Bug Fixes