Develop #171
Develop #171
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 across docs, OpenAPI, and backend. Removes password fields/models, adds JWT services, wallet validation, and class-based auth controller with use-cases. Consolidates routes under /api, eliminates versioned routers and several feature routers, updates Prisma schema/migration, and adjusts middleware/types accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as API (/api/auth)
participant Controller as AuthController
participant UseCase as RegisterUseCase
participant Repo as AuthRepository
Client->>API: POST /register {name,email,walletAddress,profileType,...}
API->>Controller: register()
Controller->>UseCase: execute(registerData)
UseCase->>UseCase: validate wallet/email/requireds
UseCase->>Repo: isWalletRegistered / isEmailRegistered
alt profileType == "user"
UseCase->>Repo: createUser(...)
else profileType == "project"
UseCase->>Repo: createOrganization(...)
end
UseCase-->>Controller: { success, message, data: profile }
Controller-->>API: 201 { success, message, profile }
API-->>Client: Response
sequenceDiagram
autonumber
actor Client
participant API as API (/api/auth)
participant Controller as AuthController
participant Login as LoginUseCase
participant Repo as AuthRepository
participant MW as authMiddleware
Client->>API: POST /login { walletAddress }
API->>Controller: login()
Controller->>Login: execute(walletAddress)
Login->>Repo: findProfileByWallet()
alt found
Login->>Login: JWTService.generateToken(payload)
Login-->>Controller: { success, data: { token, user } }
Controller-->>Client: 200 { token, user }
else not found
Login-->>Controller: { success: false, code: WALLET_NOT_FOUND }
Controller-->>Client: 404 ErrorResponse
end
Client->>API: GET /user-only (Authorization: Bearer <jwt>)
API->>MW: authMiddleware()
MW->>MW: verify token -> req.user
API->>MW: requireUserProfile()
alt allowed
API-->>Client: 200 { success: true }
else forbidden
API-->>Client: 403 ErrorResponse
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 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: 25
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
prisma/schema.prisma (2)
51-65: NFT requires both userId and organizationId — likely wrong cardinality.As modeled, every NFT must belong to both a user and an organization. If intent is “owned by user XOR organization”, make relations optional and enforce XOR in app/migration.
model NFT { id String @id @default(uuid()) createdAt DateTime @default(now()) updatedAt DateTime @updatedAt - userId String // Foreign key for User - organizationId String // Foreign key for Organization + userId String? // Foreign key for User (optional) + organizationId String? // Foreign key for Organization (optional) description String - user User @relation(fields: [userId], references: [id], name: "UserNFTs") - organization Organization @relation(fields: [organizationId], references: [id], name: "OrganizationNFTs") + user User? @relation(fields: [userId], references: [id], name: "UserNFTs") + organization Organization? @relation(fields: [organizationId], references: [id], name: "OrganizationNFTs") @@index([userId]) // Index for quick lookup @@index([organizationId]) }Follow up: add an app-level guard or raw SQL CHECK to enforce exactly one owner.
Also applies to: 60-62
152-158: Photo lacks a Prisma relation to User.Without a relation, Prisma Client can’t navigate Photo→User or enforce integrity (even in relationMode=prisma).
model Photo { id String @id @default(uuid()) url String - userId String + userId String + user User @relation(fields: [userId], references: [id]) uploadedAt DateTime @default(now()) metadata Json? + @@index([userId]) }src/index.ts (2)
66-75: Error handler is registered before routes; it won't catch route errors. Move it after app.use("/api", ...).Express only invokes error handlers that appear later in the middleware chain.
-// Error handler middleware -app.use( - ( - err: Error, - req: express.Request, - res: express.Response, - next: express.NextFunction - ) => { - errorHandler(err, req, res, next); - } -); +// ... keep routes above ... +app.use("/api", 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); +});Also applies to: 139-141
184-191: initializeRedis swallows errors and uses console; server starts even if Redis fails.Because the catch in initializeRedis does not rethrow, the outer .catch never runs and the server still starts.
const initializeRedis = async () => { try { - await redisClient.connect(); - console.log("Redis connected successfully!"); + await redisClient.connect(); + globalLogger.info("Redis connected successfully!"); } catch (error) { - console.error("Error during Redis initialization:", error); + globalLogger.error("Error during Redis initialization", error); + throw error; // propagate to abort startup } };Also remove the now-redundant “Redis connected successfully!” log in the .then() block, or keep it and retain the one here, not both.
Also applies to: 148-176
openapi.yaml (1)
301-312: Security scheme name mismatch (bearerAuth vs BearerAuth).You defined
components.securitySchemes.BearerAuthbut usebearerAuthhere.- security: - - bearerAuth: [] + security: + - BearerAuth: []
🧹 Nitpick comments (42)
src/modules/certificate/infrastructure/utils/pdfGenerator.ts (4)
30-33: Don’t hard-code the verify URL; make it configurableUse an env-backed base URL with a safe fallback and trailing-slash guard.
- const qrBuffer = await generateQRCode( - `https://volunchain.org/verify/${uniqueId}` - ); + const baseUrl = process.env.APP_PUBLIC_BASE_URL ?? "https://volunchain.org"; + const qrBuffer = await generateQRCode( + `${baseUrl.replace(/\/$/, "")}/verify/${uniqueId}` + );
5-5: Safer date parsing to avoid locale/engine variance
new Date(eventDate)is platform-dependent. PreferparseISOwith validity check.- import { format } from "date-fns"; + import { format, parseISO, isValid } from "date-fns";- const eventDateFormatted = format(new Date(eventDate), "EEEE do MMMM yyyy"); + const parsedDate = parseISO(eventDate); + const dateObj = isValid(parsedDate) ? parsedDate : new Date(eventDate); + const eventDateFormatted = format(dateObj, "EEEE do MMMM yyyy");Also applies to: 93-100
2-2: Avoid blocking I/O in request pathSwap
readFileSyncforfs/promises.readFileto keep the event loop responsive during certificate generation.-import fs from "fs"; +import { readFile } from "fs/promises";- const logoBytes = fs.readFileSync( - path.join(__dirname, "../../../../assets/VolunChain.png") - ); + const logoBytes = await readFile( + path.join(__dirname, "../../../../assets/VolunChain.png") + );If the project uses ESM, ensure
__dirnameis defined viafileURLToPath+dirname.Also applies to: 25-29
102-113: Prevent text overflow for long custom messagesCap the number of rendered lines so content doesn’t collide with the signature/QR area.
- const lines = customMessage.match(/.{1,80}(\s|$)/g) || [customMessage]; - lines.forEach((line, i) => { + const lines = customMessage.match(/.{1,80}(\s|$)/g) || [customMessage]; + const maxLines = 8; // prevents overflow into footer + lines.slice(0, maxLines).forEach((line, i) => { const textWidth = normalFont.widthOfTextAtSize(line.trim(), 18); page.drawText(line.trim(), { x: (842 - textWidth) / 2, y: 210 - i * 20, size: 18, font: normalFont, color: rgb(0.15, 0.15, 0.15), }); });src/shared/middleware/validation.middleware.ts (2)
4-4: ParsedQs import becomes unnecessary with the safer approach.If you adopt res.locals.queryDto, remove this import.
-import { ParsedQs } from "qs";
129-171: Consolidate validation logic and harden validator options.Reuse across middlewares and enable whitelist/forbidNonWhitelisted; also prefer plainToInstance.
-export async function validateOr400<T extends object>( +export async function validateOr400<T extends object>( dtoClass: new () => T, payload: unknown, res: Response ): Promise<T | undefined> { try { - const dto = plainToClass(dtoClass, payload); - const errors = await validate(dto); + const dto = plainToClass(dtoClass, payload); + const errors = await validate(dto, { + whitelist: true, + forbidNonWhitelisted: true, + skipMissingProperties: false, + }); @@ - return dto; + return dto; } catch {Follow up: Mirror these options in validateDto and validateQueryDto for consistency.
src/modules/auth/dto/login.dto.ts (1)
1-11: Enforce Stellar address charset in addition to length.Add a regex to ensure base32 (A–Z, 2–7) and update imports.
-import { IsString, MinLength, MaxLength } from "class-validator"; +import { IsString, MinLength, MaxLength, Matches } from "class-validator"; @@ @MaxLength(56, { message: "Stellar wallet address must be 56 characters long", }) + @Matches(/^[A-Z2-7]{56}$/, { + message: "Stellar wallet address must be 56 base32 characters (A–Z, 2–7)", + }) walletAddress: string;src/modules/auth/dto/register.dto.ts (1)
38-46: Optional: add conditional validations tied to profileType.E.g., require lastName for user, require category for organization.
// Example: @ValidateIf((o) => o.profileType === ProfileType.USER) @IsString({ message: "Last name must be a string" }) @MaxLength(100, { message: "Last name cannot exceed 100 characters" }) lastName?: string; @ValidateIf((o) => o.profileType === ProfileType.ORGANIZATION) @IsString({ message: "Category must be a string" }) @MaxLength(100, { message: "Category cannot exceed 100 characters" }) category?: string;prisma/schema.prisma (2)
67-81: Index organizationId and decide on delete behavior.Common queries filter by organization; also specify referential action explicitly.
model Project { @@ - organization Organization @relation(fields: [organizationId], references: [id]) // Add this relation + organization Organization @relation(fields: [organizationId], references: [id], onDelete: Cascade) volunteers Volunteer[] + @@index([organizationId]) }
160-169: Avoid hard-coded external URL as default s3Key.Defaults tied to third-party URLs are brittle; make it required or default to a controlled CDN placeholder.
- s3Key String @default("https://onlydust-app-images.s3.eu-west-1.amazonaws.com/8ee3b7d84fe0672850e4c81890361b73.png") + s3Key Stringprisma/migrations/20250823062033_remove_password_fields/migration.sql (1)
12-13: Confirm data/backfill semantics for isVerified; consider drift-safe drops.
- If the intent (per docs) is “auto-verified” for wallet-based auth, existing rows remain unchanged by only setting a default. If you need them verified, add a one-time backfill or clarify docs that only new records default to verified.
- Dropping columns without IF EXISTS can fail in drifted envs. Optional: make the drops resilient.
Example adjustments:
-ALTER TABLE "Organization" DROP COLUMN "password"; +ALTER TABLE "Organization" DROP COLUMN IF EXISTS "password"; -ALTER TABLE "User" DROP COLUMN "password", +ALTER TABLE "User" DROP COLUMN IF EXISTS "password", ALTER COLUMN "isVerified" SET DEFAULT true; + +-- Optional backfill if business-approved: +-- UPDATE "User" SET "isVerified" = true WHERE "isVerified" IS NOT TRUE;Also applies to: 9-9
src/routes/index.ts (2)
6-13: Avoid two different health schemas; remove /api/health or align it with root /health.You already expose a rich /health at the app level. Keeping a second, simpler /api/health with a different schema can confuse clients.
Proposed removal:
-// Health check -router.get("/health", (req, res) => { - res.json({ - status: "API is running", - version: "1.0.0", - timestamp: new Date().toISOString(), - }); -});Also applies to: 15-23
9-11: Hardcoded version string.Derive version from env to avoid stale values.
- version: "1.0.0", + version: process.env.npm_package_version ?? "unknown",Apply in both places.
Also applies to: 19-20
src/index.ts (3)
78-137: Two health endpoints with different schemas (/health and /api/health).Prefer a single canonical health endpoint. If keeping both, align their shape or document the difference.
Also applies to: 60-64
186-186: Consistent logging.Use globalLogger everywhere (replace console.*) for uniform logs and correlation.
Also applies to: 171-176
10-11: Consolidate middleware directory naming
We have bothsrc/middleware(used byrateLimitMiddleware) andsrc/middlewares(used by all other middleware). Rename one directory and update imports to use a single, consistent path.docs/wallet-auth-flow.md (2)
26-32: Response contracts don’t match code.
- Register: docs show userId; code returns data.profile (name, email, wallet, profileType, category?).
- Login: ensure fields match actual controller output.
- Migration note: clarify whether existing users are auto-verified or only new ones.
I can sync these sections to the actual DTOs and controller responses once you confirm the desired contract.
Also applies to: 60-73, 129-134
81-84: Add a language to the fenced code block.Fixes MD040 lint warning.
-``` +```text Authorization: Bearer <jwt-token></blockquote></details> <details> <summary>src/modules/auth/use-cases/register.usecase.ts (2)</summary><blockquote> `139-144`: **Double success messages (outer “Registration successful” and nested “…registered successfully”).** Redundant and inconsistent. Keep one canonical message at the outer AuthResult. ```diff - return { - success: true, - message: "Registration successful", - data: response, - }; + return { success: true, message: response.message, data: response };Also applies to: 99-108
145-155: Use centralized logging instead of console.error.Adopt your Logger for consistency and trace IDs.
- console.error("Registration use case error:", error); + // logger.error("Registration use case error", error);(Same for availability checks.)
Also applies to: 289-299, 333-343
src/modules/auth/domain/services/wallet-validation.service.ts (1)
38-60: Apply the same normalization in validateWalletAddress and leverage SDK validator.Normalize early and prefer SDK’s boolean validator over try/catch for readability.
static validateWalletAddress(walletAddress: string): { isValid: boolean; errors: string[]; } { const errors: string[] = []; if (!walletAddress) { errors.push("Wallet address is required"); return { isValid: false, errors }; } if (typeof walletAddress !== "string") { errors.push("Wallet address must be a string"); return { isValid: false, errors }; } + const addr = walletAddress.trim().toUpperCase(); - if (walletAddress.length !== 56) { + if (addr.length !== 56) { errors.push("Stellar wallet address must be exactly 56 characters long"); } - if (!walletAddress.startsWith("G")) { + if (!addr.startsWith("G")) { errors.push("Stellar wallet address must start with 'G'"); } if (errors.length === 0) { try { - Keypair.fromPublicKey(walletAddress); + // Alternatively: StrKey.isValidEd25519PublicKey(addr) and branch on the boolean. + Keypair.fromPublicKey(addr); } catch { errors.push("Invalid Stellar wallet address format"); } }If you prefer, replace Keypair.fromPublicKey with StrKey.isValidEd25519PublicKey to avoid exceptions on control flow.
Also applies to: 62-68
src/modules/auth/use-cases/login.usecase.ts (1)
38-45: Double normalization (here and inside repository).Harmless, but you can drop one normalization to avoid duplication; the repository already normalizes.
- const normalizedWallet = WalletValidationService.normalizeWalletAddress( - loginData.walletAddress - ); - const profile = - await this.authRepository.findProfileByWallet(normalizedWallet); + const profile = await this.authRepository.findProfileByWallet( + loginData.walletAddress + );src/routes/authRoutes.ts (2)
28-31: Remove redundant authMiddleware from validate-token.This endpoint’s purpose is to validate the token. Having authMiddleware here makes it tautological and causes double verification.
router.post( "/validate-token", - authMiddleware.authMiddleware, AuthController.validateToken );
19-19: Email as a path param can be fragile.Emails often include characters needing encoding. Prefer a query param (?email=) or enforce encoding on clients.
src/types/auth.types.ts (1)
24-27: You’re both augmenting Express.Request and exporting AuthenticatedRequest. Pick one to avoid drift.Since you augment Request globally, you can often use Express.Request directly without the custom interface.
-export interface AuthenticatedRequest extends Request { - user?: AuthenticatedUser; - traceId?: string; -} +// Optional: rely on global augmentation only +export type AuthenticatedRequest = Request;Low priority; keep if it improves DX in your codebase.
Also applies to: 86-93
src/modules/auth/domain/services/jwt.service.ts (3)
41-45: Refresh token generation is indistinguishable from access tokens and appears unused.Either remove it or differentiate with audience/subject (and add a matching verifier).
- static generateRefreshToken( - payload: Omit<IJWTPayload, "iat" | "exp"> - ): string { - return this.generateToken(payload, this.REFRESH_EXPIRATION); - } + static generateRefreshToken( + payload: Omit<IJWTPayload, "iat" | "exp"> + ): string { + const options: SignOptions = { + expiresIn: this.REFRESH_EXPIRATION, + issuer: "volunchain-api", + audience: "volunchain-refresh", + algorithm: "HS256", + subject: "refresh", + }; + return jwt.sign(payload, this.SECRET_KEY, options); + }Would you like me to add a
verifyRefreshTokenmethod and wire the refresh flow accordingly?
115-126: Parse Authorization header robustly (case-insensitive Bearer, trim, tolerate extra spaces).- const parts = authHeader.split(" "); - if (parts.length !== 2 || parts[0] !== "Bearer") { + const parts = authHeader.trim().split(/\s+/); + if (parts.length !== 2 || parts[0].toLowerCase() !== "bearer") { return null; } - - return parts[1]; + return parts[1].trim();
96-108: Optional: Avoid trusting unverified decode for expiry checks.Attackers can forge exp in unsigned tokens; prefer verifying header/payload when possible.
- static isTokenExpired(token: string): boolean { + static isTokenExpired(token: string): boolean { try { const decoded = jwt.decode(token) as IJWTPayload; if (!decoded || !decoded.exp) { return true; } const currentTime = Math.floor(Date.now() / 1000); return decoded.exp < currentTime; } catch { return true; } }Consider an alternative helper that calls
verifyand mapsTokenExpiredErrorspecifically when the secret is available in context.src/modules/auth/infrastructure/repositories/auth.repository.ts (4)
12-14: Share or inject PrismaClient to avoid multiple connections.Both LoginUseCase and RegisterUseCase new-up this repository, leading to multiple PrismaClient instances.
- constructor() { - this.prisma = new PrismaClient(); - } + constructor(prisma?: PrismaClient) { + this.prisma = prisma ?? new PrismaClient(); + }Follow-up: pass a shared instance from a composition root or DI container.
167-173: Persist nullable lastName as null, not empty string.Aligns with domain type (
string | null) and avoids empty-string semantics in queries.- lastName: userData.lastName || "", + lastName: userData.lastName ?? null,
70-79: Nit: Avoid re-normalizing already-normalized input.You pass
normalizedAddressintofindUserByWallet, which normalizes again.- const user = await this.findUserByWallet(normalizedAddress); + const user = await this.findUserByWallet(walletAddress);(Same for organization path.)
247-249: Connection lifecycle.Consider centralizing
$disconnect()at app shutdown; with shared Prisma, per-repo disconnects can tear down shared connections unexpectedly.src/modules/auth/presentation/controllers/Auth.controller.ts (6)
96-126: Token validation endpoint not documented.
validateTokenexists here but is absent from OpenAPI.Add
/auth/validate-tokento docs or remove this handler if not intended to be public.
139-178: Refresh flow uses access tokens, not refresh tokens.
refreshTokenvalidates the presented token as an access token and issues a new access token. If you intend refresh tokens, wiregenerateRefreshToken+ dedicated verifier; otherwise, rename torenewTokenand update docs.
183-220: Logout semantics are client-only; document this to avoid confusion.Endpoint merely validates a token and returns success. Consider removing or documenting as no-op (unless you add server-side blacklist).
225-244: Validation endpoint not documented.
validateWalletFormatshould be in OpenAPI if kept.
257-299: Availability endpoints not documented.Both wallet/email availability endpoints exist in code but not in OpenAPI.
381-401: Status code mapping could group 401 cases.
NO_TOKENduplicates the 401 mapping; consider consolidating to reduce branches.openapi.yaml (4)
186-213: Add/auth/validate-tokento docs or remove the endpoint.Controller exposes it; OpenAPI does not.
139-177: Add/auth/refresh-tokenand/auth/logoutor remove from code.Keep API and implementation in sync.
257-299: Add wallet/email availability and wallet format validation endpoints if they’re part of the public API.Also applies to: 304-345
17-17: YAML lint: strip trailing spaces.-<lines with trailing spaces> +<same lines without trailing spaces>Also applies to: 95-95, 152-152, 193-193, 221-221, 262-262, 651-651, 653-653
📜 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/v1/index.ts
- src/routes/ProjectRoutes.ts
- readme.md
- src/routes/certificatesRoutes.ts
- src/routes/testRoutes.ts
- src/routes/nftRoutes.ts
- src/routes/VolunteerRoutes.ts
- src/routes/OrganizationRoutes.ts
- src/routes/userRoutes.ts
- src/routes/v2/auth.routes.ts
🧰 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/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/domain/services/jwt.service.ts (1)
src/modules/auth/domain/interfaces/auth.interface.ts (1)
IJWTPayload(56-62)
src/middleware/authMiddleware.ts (1)
src/types/auth.types.ts (2)
AuthenticatedRequest(24-27)DecodedUser(32-38)
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/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)
🪛 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 (13)
src/modules/certificate/infrastructure/utils/pdfGenerator.ts (1)
4-4: Centralized QR generator import — verify binary output contractConfirm
generateQRCodereturns raw PNG bytes (Buffer/Uint8Array).pdfDoc.embedPngwon’t accept a data URL or SVG. If needed, add an explicit return type in the shared util to enforce this.prisma/schema.prisma (4)
15-29: isVerified default vs verificationToken fields.Defaulting isVerified to true while keeping verificationToken(+Expires) is contradictory. Either keep tokens and default to false, or remove token fields for wallet-only flows.
Would you like a migration diff to drop the token fields?
172-189: Messaging relations and indexes look good.
95-105: UserVolunteer composite key and indexes preserved — good.
6-11: relationMode: "prisma" — confirm this is intentional.No DB-level FKs will be created/enforced. Ensure ops/DB constraints are acceptable.
src/routes/index.ts (2)
37-37: LGTM: router export and consolidation.Default export and consolidation under /api look good.
26-26: authRoutes mount is valid
Verified thatsrc/routes/authRoutes.tsexists and containsexport default router;.src/modules/auth/domain/services/wallet-validation.service.ts (1)
91-96: Address comparison looks good.src/modules/auth/use-cases/login.usecase.ts (1)
69-74: Result shape is consistent and clear.src/routes/authRoutes.ts (1)
21-27: Auth/user-type guards applied correctly.Also applies to: 37-37, 40-62
src/middleware/authMiddleware.ts (1)
36-49: Profile-type guards are straightforward and fine.Also applies to: 51-64
src/types/auth.types.ts (1)
50-61: Type guard implementation looks correct.src/modules/auth/presentation/controllers/Auth.controller.ts (1)
407-413: Resource cleanup OK.Good centralization of teardown via use-cases’
cleanup().
| 2. **Project/Organization Profile** (`profileType: "project"`) | ||
| - Requires: name, email, walletAddress, category | ||
| - Auto-verified upon registration | ||
|
|
There was a problem hiding this comment.
Terminology drift: “project” vs “organization”. Pick one and use it consistently.
Docs mix “project” (profileType value) with “organization” (route names). The codebase uses Organization as the model and register.usecase returns “organization”. Align terms across request/response, routes, and models.
Example (if choosing "organization"):
-2. **Project/Organization Profile** (`profileType: "project"`)
+2. **Organization Profile** (`profileType: "organization"`)
- - `/auth/organization-only` - Only accessible by organization profiles
+ - `/auth/organization-only` - Only accessible by organization profilesAnd update request/response samples accordingly.
Also applies to: 87-89, 36-40
🧰 Tools
🪛 LanguageTool
[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)
🤖 Prompt for AI Agents
In docs/wallet-auth-flow.md around lines 42-45 (and also review lines 36-40 and
87-89), the doc mixes “project” and “organization”; pick one term (e.g.,
"organization") and make it consistent: change the profileType example value
from "project" to "organization", update the heading and bullet text to say
Organization, update all request/response samples and route names shown in the
doc to use "organization" (and not "project"), and ensure any returned type
strings mentioned (e.g., register.usecase outputs) use the chosen term; search
the file for any remaining occurrences of "project" and replace or reconcile
them to match the chosen terminology.
| ## 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 routing: “no versioning” but root returns v1/v2.
Remove version listing or reintroduce versioned routers. Also adjust examples accordingly.
- description: "Get information about available API versions"
+ description: "Get information about the API"
...
- versions:
- type: object
- properties:
- v1:
- type: object
- properties:
- status:
- type: string
- example: "stable"
- description:
- type: string
- example: "Current stable API version"
- endpoints:
- type: string
- example: "/v1/"
- v2:
- type: object
- properties:
- status:
- type: string
- example: "reserved"
- description:
- type: string
- example: "Reserved for future expansion"
- endpoints:
- type: string
- example: "/v2/"
+ endpoints:
+ type: string
+ example: "/api/"Also applies to: 35-85
🧰 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 across lines
35 to 85), the docs state "no versioning" while the routing examples and root
return v1/v2—this is inconsistent; either remove the top-level version field and
any versioned route examples so documentation reflects an unversioned API, or
reintroduce explicit versioned routers in the paths (e.g., /api/v1, /api/v2) and
update the root/paths/examples to reference those versions; ensure all path
entries, example URLs, and the version field are consistent across the file
(lines 13–19 and 35–85).
| - url: https://api.volunchain.org | ||
| description: Production server |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Production server URL missing /api base path.
- - url: https://api.volunchain.org
+ - url: https://api.volunchain.org/api📝 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.
| - url: https://api.volunchain.org | |
| description: Production server | |
| - url: https://api.volunchain.org/api | |
| description: Production server |
🤖 Prompt for AI Agents
In openapi.yaml around lines 26-27 the production server URL is missing the
required /api base path; update the server url from https://api.volunchain.org
to include the base path (e.g. https://api.volunchain.org/api), and ensure any
other server entries or references to the production server use the same /api
base path for consistency.
| name: "Juan Pérez" | ||
| email: "[email protected]" | ||
| walletAddress: "GXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" | ||
| profileType: "user" | ||
| lastName: "Pérez" | ||
| organization_registration: | ||
| summary: Organization registration | ||
| value: | ||
| name: "Fundación Ambiental Verde" | ||
| email: "[email protected]" | ||
| walletAddress: "GXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" | ||
| profileType: "project" | ||
| category: "environmental" | ||
| responses: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Examples under /auth/register use "project"; switch to "organization".
- profileType: "project"
+ profileType: "organization"📝 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: "Juan Pérez" | |
| email: "[email protected]" | |
| walletAddress: "GXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" | |
| profileType: "user" | |
| lastName: "Pérez" | |
| organization_registration: | |
| summary: Organization registration | |
| value: | |
| name: "Fundación Ambiental Verde" | |
| email: "[email protected]" | |
| walletAddress: "GXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" | |
| profileType: "project" | |
| category: "environmental" | |
| responses: | |
| organization_registration: | |
| summary: Organization registration | |
| value: | |
| name: "Fundación Ambiental Verde" | |
| email: "[email protected]" | |
| walletAddress: "GXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" | |
| profileType: "organization" | |
| category: "environmental" | |
| responses: |
🤖 Prompt for AI Agents
In openapi.yaml around lines 112 to 125, the example value for
organization_registration currently sets profileType: "project"; update that
example to use profileType: "organization" so the /auth/register examples
consistently use "organization" for organization registrations; make the
single-word replacement in the organization_registration example and keep casing
and surrounding structure identical.
| /auth/user-only: | ||
| get: | ||
| tags: | ||
| - Authentication | ||
| summary: "User-only route" | ||
| description: | | ||
| Endpoint that can only be accessed by user profiles. | ||
| **Requirements:** | ||
| - Valid JWT token | ||
| - User profile type | ||
| operationId: "userOnly" | ||
| security: | ||
| - BearerAuth: [] | ||
| responses: | ||
| '200': | ||
| description: "Access allowed for users" | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| properties: | ||
| token: | ||
| success: | ||
| type: boolean | ||
| example: true | ||
| message: | ||
| type: string | ||
| description: "JWT authentication token" | ||
| example: "User-only route accessed successfully" | ||
| '401': | ||
| description: "Unauthorized" | ||
| description: "Invalid token" | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/ErrorResponse' | ||
| '403': | ||
| description: "Access denied - requires user profile" | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/Error' | ||
| $ref: '#/components/schemas/ErrorResponse' | ||
|
|
||
| /auth/organization-only: | ||
| get: | ||
| tags: | ||
| - Authentication | ||
| summary: "Organization-only route" | ||
| description: | | ||
| Endpoint that can only be accessed by organization profiles. | ||
| **Requirements:** | ||
| - Valid JWT token | ||
| - Organization profile type | ||
| operationId: "organizationOnly" | ||
| security: | ||
| - BearerAuth: [] | ||
| responses: | ||
| '200': | ||
| description: "Access allowed for organizations" | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| properties: | ||
| success: | ||
| type: boolean | ||
| example: true | ||
| message: | ||
| type: string | ||
| example: "Organization-only route accessed successfully" | ||
| '401': | ||
| description: "Invalid token" | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/ErrorResponse' | ||
| '403': | ||
| description: "Access denied - requires organization profile" | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/ErrorResponse' | ||
|
|
There was a problem hiding this comment.
Documented routes /auth/user-only and /auth/organization-only are not implemented.
Either add handlers or remove these paths from docs.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 221-221: trailing spaces
(trailing-spaces)
[error] 262-262: trailing spaces
(trailing-spaces)
| async refreshToken( | ||
| token: string | ||
| ): Promise<AuthResult<{ token: string; user: IProfile }>> { | ||
| try { | ||
| // Validate current token | ||
| const validationResult = await this.validateToken(token); | ||
| if (!validationResult.success || !validationResult.data) { | ||
| return { | ||
| success: false, | ||
| message: "Invalid token for refresh", | ||
| code: "INVALID_REFRESH_TOKEN", | ||
| }; | ||
| } | ||
|
|
||
| // Generate new token | ||
| const newToken = JWTService.generateToken({ | ||
| userId: validationResult.data.wallet, // Use wallet as userId | ||
| email: validationResult.data.email, | ||
| profileType: validationResult.data.profileType, | ||
| }); | ||
|
|
||
| return { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refresh flow can’t refresh expired access tokens.
Route is protected by authMiddleware (rejects expired) and this method re-verifies (rejects expired), so users cannot refresh after expiry.
Minimal fix (accept expired token only for refresh by decoding on TokenExpiredError). Better fix is introducing a dedicated refresh token (JWTService.generateRefreshToken) and rotating it.
async refreshToken(
token: string
): Promise<AuthResult<{ token: string; user: IProfile }>> {
try {
- // Validate current token
- const validationResult = await this.validateToken(token);
- if (!validationResult.success || !validationResult.data) {
- return {
- success: false,
- message: "Invalid token for refresh",
- code: "INVALID_REFRESH_TOKEN",
- };
- }
+ // Attempt verification; if only expired, fall back to decode
+ let userWallet: string | null = null;
+ try {
+ const payload = JWTService.verifyToken(token);
+ userWallet = payload.userId;
+ } catch (e) {
+ if (e instanceof Error && e.message === "Token has expired") {
+ const decoded = JWTService.decodeToken(token);
+ userWallet = decoded?.userId ?? null;
+ } else {
+ return {
+ success: false,
+ message: "Invalid token for refresh",
+ code: "INVALID_REFRESH_TOKEN",
+ };
+ }
+ }
+ if (!userWallet) {
+ return {
+ success: false,
+ message: "Invalid token for refresh",
+ code: "INVALID_REFRESH_TOKEN",
+ };
+ }
- // Generate new token
- const newToken = JWTService.generateToken({
- userId: validationResult.data.wallet, // Use wallet as userId
- email: validationResult.data.email,
- profileType: validationResult.data.profileType,
- });
+ // Load profile and generate new token
+ const profile = await this.authRepository.findProfileByWallet(userWallet);
+ if (!profile) {
+ return {
+ success: false,
+ message: "User not found",
+ code: "USER_NOT_FOUND",
+ };
+ }
+ const newToken = JWTService.generateToken({
+ userId: profile.wallet,
+ email: profile.email,
+ profileType: profile.profileType,
+ });
return {
success: true,
message: "Token refreshed successfully",
data: {
- token: newToken,
- user: validationResult.data,
+ token: newToken,
+ user: profile,
},
};Follow-up: adopt refresh tokens for proper security (rotation, revocation).
📝 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.
| async refreshToken( | |
| token: string | |
| ): Promise<AuthResult<{ token: string; user: IProfile }>> { | |
| try { | |
| // Validate current token | |
| const validationResult = await this.validateToken(token); | |
| if (!validationResult.success || !validationResult.data) { | |
| return { | |
| success: false, | |
| message: "Invalid token for refresh", | |
| code: "INVALID_REFRESH_TOKEN", | |
| }; | |
| } | |
| // Generate new token | |
| const newToken = JWTService.generateToken({ | |
| userId: validationResult.data.wallet, // Use wallet as userId | |
| email: validationResult.data.email, | |
| profileType: validationResult.data.profileType, | |
| }); | |
| return { | |
| async refreshToken( | |
| token: string | |
| ): Promise<AuthResult<{ token: string; user: IProfile }>> { | |
| try { | |
| // Attempt verification; if only expired, fall back to decode | |
| let userWallet: string | null = null; | |
| try { | |
| const payload = JWTService.verifyToken(token); | |
| userWallet = payload.userId; | |
| } catch (e) { | |
| if (e instanceof Error && e.message === "Token has expired") { | |
| const decoded = JWTService.decodeToken(token); | |
| userWallet = decoded?.userId ?? null; | |
| } else { | |
| return { | |
| success: false, | |
| message: "Invalid token for refresh", | |
| code: "INVALID_REFRESH_TOKEN", | |
| }; | |
| } | |
| } | |
| if (!userWallet) { | |
| return { | |
| success: false, | |
| message: "Invalid token for refresh", | |
| code: "INVALID_REFRESH_TOKEN", | |
| }; | |
| } | |
| // Load profile and generate new token | |
| const profile = await this.authRepository.findProfileByWallet(userWallet); | |
| if (!profile) { | |
| return { | |
| success: false, | |
| message: "User not found", | |
| code: "USER_NOT_FOUND", | |
| }; | |
| } | |
| const newToken = JWTService.generateToken({ | |
| userId: profile.wallet, | |
| email: profile.email, | |
| profileType: profile.profileType, | |
| }); | |
| return { | |
| success: true, | |
| message: "Token refreshed successfully", | |
| data: { | |
| token: newToken, | |
| user: profile, | |
| }, | |
| }; | |
| } catch (error) { | |
| // existing error handling… | |
| throw error; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/modules/auth/use-cases/login.usecase.ts around lines 134 to 155, the
refreshToken flow currently re-validates the access token and therefore rejects
already-expired tokens; update the method to allow refresh by handling
TokenExpiredError: attempt to validate the token as now, but if validation
throws a TokenExpiredError decode the token payload without verifying expiry (or
use a JWTService.decodeIgnoringExpiry helper) to extract
userId/email/profileType and proceed to generate a new access token;
additionally, prefer a longer-term refresh-token approach by adding
JWTService.generateRefreshToken, storing/rotating refresh tokens (issue new
refresh token on use and revoke old), and change the route/middleware to accept
refresh tokens for refresh operations only.
| // 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); | ||
| if (isEmailRegistered) { | ||
| return { | ||
| success: false, | ||
| message: "Email address is already registered", | ||
| code: "EMAIL_ALREADY_REGISTERED", | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Existence checks treat repository errors as “not registered”.
AuthRepository.isWalletRegistered/isEmailRegistered return false on errors, which can allow duplicate attempts and surface as later failures. Prefer propagating errors or returning a tri-state so the use case can halt on uncertainty.
Proposed repository change (conceptual):
- Return Promise but throw on DB errors (don’t catch-and-fallback).
- Or return Promise<{ ok: true; exists: boolean } | { ok: false; error }> and handle ok=false here.
I can draft a patch across repository and use case if you want to proceed.
Also applies to: 77-85, 322-331
🤖 Prompt for AI Agents
In src/modules/auth/use-cases/register.usecase.ts around lines 66 to 86 (and
similarly 77-85, 322-331), the current checks treat repository failures as “not
registered” because the repository swallows DB errors and returns false; change
behavior so repository errors are not masked: update
AuthRepository.isWalletRegistered/isEmailRegistered to either throw on DB errors
(preferred) or return a tri-state result (e.g. { ok: boolean, exists?: boolean,
error?: Error }), and then modify this use case to detect and propagate
repository errors instead of treating them as "not registered" — if the repo
throws catch and return a failure result with an appropriate message/code (e.g.
DATABASE_ERROR) or if using tri-state, check ok===false and return the same
failure; ensure both wallet and email checks follow this pattern.
| } else { | ||
| // Organization registration | ||
| if (!registerData.category) { | ||
| return { | ||
| success: false, | ||
| message: "Category is required for organization registration", | ||
| code: "MISSING_CATEGORY", | ||
| }; | ||
| } |
There was a problem hiding this comment.
Inconsistent profileType: input expects "project", response returns "organization".
This breaks clients relying on a stable enum. Either rename the enum across the codebase or return "project" here to match inputs and interfaces.
- // Organization registration
+ // Project/Organization registration
if (!registerData.category) {
return {
success: false,
- message: "Category is required for organization registration",
+ message: "Category is required for project registration",
code: "MISSING_CATEGORY",
};
}
@@
- response = {
+ response = {
success: true,
- message: "Organization registered successfully",
+ message: "Project registered successfully",
profile: {
name: organization.name,
email: organization.email,
wallet: organization.wallet,
- profileType: "organization",
+ profileType: "project",
category: organization.category,
},
};Also update the validation error message at Lines 232-239 as shown above.
Also applies to: 126-136, 224-235
| "/refresh-token", | ||
| authMiddleware.authMiddleware, | ||
| AuthController.refreshToken | ||
| ); | ||
| router.post("/logout", authMiddleware.authMiddleware, AuthController.logout); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refresh endpoint cannot work for expired tokens behind authMiddleware.
When access tokens expire, this route becomes unusable. Remove the guard here (and fix use-case as suggested) or switch to refresh-token cookies.
router.post(
"/refresh-token",
- authMiddleware.authMiddleware,
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.
| "/refresh-token", | |
| authMiddleware.authMiddleware, | |
| AuthController.refreshToken | |
| ); | |
| router.post("/logout", authMiddleware.authMiddleware, AuthController.logout); | |
| router.post( | |
| "/refresh-token", | |
| AuthController.refreshToken | |
| ); | |
| router.post("/logout", authMiddleware.authMiddleware, AuthController.logout); |
🤖 Prompt for AI Agents
In src/routes/authRoutes.ts around lines 33 to 37, the /refresh-token route is
currently protected by authMiddleware, which blocks expired access tokens;
remove the authMiddleware from the /refresh-token route so it is accessible when
access tokens expire, and update the refresh flow to validate the refresh token
instead (read refresh token from a secure httpOnly cookie or request body), have
AuthController.refreshToken verify the refresh token, rotate/issue a new access
token (and refresh token if using rotation), set the cookie if applicable, and
keep logout protected by authMiddleware as-is.
| req.query = dto as unknown as ParsedQs; | ||
| next(); | ||
| } catch { |
There was a problem hiding this comment.
Don't overwrite req.query with a non-ParsedQs object.
Casting a class instance to ParsedQs is unsafe and can break downstream middleware expecting string-ish query values.
Apply this diff and expose the validated DTO via res.locals instead:
- req.query = dto as unknown as ParsedQs;
+ // Keep req.query intact; expose validated DTO separately
+ res.locals.queryDto = dto as T;
next();Optionally add a global typing augmentation elsewhere:
declare global {
namespace Express {
interface Locals {
queryDto?: unknown;
}
}
}🤖 Prompt for AI Agents
In src/shared/middleware/validation.middleware.ts around lines 81 to 83, the
code unsafely overwrites req.query with a DTO cast to ParsedQs; instead don't
mutate req.query — assign the validated DTO to res.locals (e.g.
res.locals.queryDto = dto) and leave req.query intact, then call next();
optionally add a global Express Locals augmentation elsewhere to type queryDto
if desired.
2 participants
🚀 Volunchain Pull Request
Mark with an
xall the checkboxes that apply (like[x])📌 Type of Change
📝 Changes description
📸 Evidence (A photo is required as evidence)
⏰ Time spent breakdown
🌌 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