Feat/auth docs #186
Feat/auth docs #186
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 the API: new JWT middleware and types, auth interfaces/services/use-cases/repository/controller, revised DTOs, and OpenAPI docs. Consolidates routing under /api with new /auth endpoints; removes versioned and legacy routes. Updates Prisma schema/migration to drop passwords and auto-verify users. Adds docs and minor utility/import updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor C as Client
participant R as Router (/api/auth)
participant AC as AuthController
participant RU as RegisterUseCase
participant AR as AuthRepository
participant DB as Prisma/DB
C->>R: POST /auth/register { name,email,wallet,profileType,... }
R->>AC: register()
AC->>RU: execute(registerData)
RU->>AR: isWalletRegistered / isEmailRegistered
AR->>DB: SELECT user/org by wallet/email
DB-->>AR: results
alt profileType == user
RU->>AR: createUser(...)
AR->>DB: INSERT User
else profileType == project/org
RU->>AR: createOrganization(...)
AR->>DB: INSERT Organization
end
DB-->>AR: created record
AR-->>RU: entity
RU-->>AC: success { user/profile }
AC-->>R: 201 RegisterResponse
R-->>C: 201 { userId or profile }
sequenceDiagram
autonumber
actor C as Client
participant R as Router (/api/auth)
participant AC as AuthController
participant LU as LoginUseCase
participant AR as AuthRepository
participant JWT as JWTService
participant M as authMiddleware
participant PR as Protected Route
C->>R: POST /auth/login { walletAddress }
R->>AC: login()
AC->>LU: execute(loginData)
LU->>AR: findProfileByWallet(wallet)
AR-->>LU: profile or null
LU->>JWT: generateToken({ userId,email,profileType })
JWT-->>LU: token
LU-->>AC: success { token, user }
AC-->>R: 200 LoginResponse
R-->>C: 200 { token, user }
Note over C,M: Access protected resource
C->>M: Authorization: Bearer <token>
M->>JWT: verifyToken(token)
JWT-->>M: payload { userId, email, profileType }
M-->>PR: next(req.user = payload)
PR-->>C: 200 protected data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
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: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
prisma/schema.prisma (2)
126-131: Legacy database models may conflict with new auth system.The schema contains legacy models (
diesel_schema_migrations,escrows,users) that appear to be from a different system with different field names and types. Theusersmodel still containspassword_hashand usesIntIDs, which conflicts with the new UUID-basedUsermodel.Consider cleaning up legacy models to avoid confusion:
-model diesel_schema_migrations { - version String @id @db.VarChar(50) - run_on DateTime @default(now()) @db.Timestamp(6) - - @@map("__diesel_schema_migrations") -} - -model escrows { - id Int @id @default(autoincrement()) - user_id Int - amount Decimal @db.Decimal(10, 2) - status String @db.VarChar(50) - created_at DateTime? @default(now()) @db.Timestamp(6) - users users @relation(fields: [user_id], references: [id], onDelete: Restrict, onUpdate: Restrict) -} - -model users { - id Int @id @default(autoincrement()) - username String @unique @db.VarChar(255) - email String @unique @db.VarChar(255) - password_hash String - role String @db.VarChar(50) - created_at DateTime? @default(now()) @db.Timestamp(6) - escrows escrows[] -}If these models are still needed, consider renaming them to avoid confusion with the new auth system.
Also applies to: 133-150
152-158: Photo model lacks proper relationships.The
Photomodel has auserIdfield but no foreign key relationship defined. This could lead to orphaned records and referential integrity issues.Add the missing relationship:
model Photo { id String @id @default(uuid()) url String userId String + user User @relation(fields: [userId], references: [id]) uploadedAt DateTime @default(now()) metadata Json? + + @@index([userId]) }And add the corresponding relation to the
Usermodel:model User { // ... existing fields userVolunteers UserVolunteer[] @relation("UserToVolunteer") sentMessages Message[] @relation("SentMessages") receivedMessages Message[] @relation("ReceivedMessages") + photos Photo[] }openapi.yaml (1)
39-86: Align "/" response schema with implementation; remove obsolete "versions" object.The router returns {message, version, status, documentation} without the nested versions.v1/v2. Keep spec and code in lockstep.
Apply:
responses: '200': description: "API version information" content: application/json: schema: type: object properties: message: type: string - example: "VolunChain 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/" + example: "VolunChain API" + version: + type: string + example: "1.0.0" + status: + type: string + example: "active" documentation: type: string example: "/api/docs"src/index.ts (1)
65-75: Error handler is registered before routes; it will miss /api errors. Move it after app.use("/api", apiRouter).Express only invokes error handlers that are defined later in the stack.
-// Error handler middleware -app.use( - ( - err: Error, - req: express.Request, - res: express.Response, - next: express.NextFunction - ) => { - errorHandler(err, req, res, next); - } -); - -// Health check route +// Health check route app.get("/health", async (req, res) => { ... }); -// API Routes 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
🧹 Nitpick comments (28)
src/modules/certificate/infrastructure/utils/pdfGenerator.ts (4)
25-29: Avoid sync I/O and make asset path deploy-safe.Synchronous
readFileSyncblocks the event loop and__dirname-relative paths can break after transpilation. Use async I/O and allow an override viaASSETS_DIRwhile keeping the current path as fallback.- const logoBytes = fs.readFileSync( - path.join(__dirname, "../../../../assets/VolunChain.png") - ); + const defaultLogoPath = path.join(__dirname, "../../../../assets/VolunChain.png"); + const assetDir = process.env.ASSETS_DIR ? process.env.ASSETS_DIR : path.join(process.cwd(), "assets"); + const logoPath = fs.existsSync(defaultLogoPath) + ? defaultLogoPath + : path.join(assetDir, "VolunChain.png"); + const logoBytes = await fs.promises.readFile(logoPath);
30-33: Make the verify URL configurable.Hardcoding the base URL couples builds to a single environment. Read from
VERIFY_BASE_URLwith a safe default.- const qrBuffer = await generateQRCode( - `https://volunchain.org/verify/${uniqueId}` - ); + const verifyBaseUrl = process.env.VERIFY_BASE_URL ?? "https://volunchain.org/verify"; + const qrBuffer = await generateQRCode(`${verifyBaseUrl}/${uniqueId}`);
93-100: Avoid ambiguous Date parsing.
new Date(eventDate)is locale/engine-dependent and can shift dates. UseparseISOfor consistent parsing of ISO strings.- const eventDateFormatted = format(new Date(eventDate), "EEEE do MMMM yyyy"); + const eventDateFormatted = format(parseISO(eventDate), "EEEE do MMMM yyyy");Add this import adjustment:
- import { format } from "date-fns"; + import { format, parseISO } from "date-fns";
4-4: Use TS path alias for QR generator import
Shared module exportsgenerateQRCodeas a named export. Update the import to use the"@/*"alias and simplify the path.
@ src/modules/certificate/infrastructure/utils/pdfGenerator.ts-import { generateQRCode } from "../../../../shared/infrastructure/utils/qrGenerator"; +import { generateQRCode } from "@/shared/infrastructure/utils/qrGenerator";docs/wallet-auth-flow.md (1)
81-83: Add language specification to the fenced code block.The static analysis correctly identified that the Authorization header example should have a language specified for better rendering and consistency.
-``` +```http Authorization: Bearer <jwt-token></blockquote></details> <details> <summary>src/modules/auth/dto/register.dto.ts (1)</summary><blockquote> `24-31`: **Duplicate wallet address validation logic.** The wallet address validation is identical to `LoginDto`. Consider extracting this validation into a reusable decorator to maintain consistency and reduce duplication. Create a shared validation decorator: ```typescript // src/shared/validators/stellar-wallet.validator.ts import { IsString, MinLength, MaxLength, Matches } from "class-validator"; export function IsStellarWallet() { return function (target: any, propertyName: string) { IsString({ message: "Wallet address must be a string" })(target, propertyName); Matches(/^G[A-Z2-7]{55}$/, { message: "Wallet address must be a valid Stellar address starting with 'G'", })(target, propertyName); MinLength(56, { message: "Stellar wallet address must be 56 characters long", })(target, propertyName); MaxLength(56, { message: "Stellar wallet address must be 56 characters long", })(target, propertyName); }; }Then use it in both DTOs:
-@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", -}) +@IsStellarWallet() walletAddress: string;src/modules/auth/domain/interfaces/auth.interface.ts (1)
1-10: Consider consolidating IUser and IOrganization with discriminated union.The
IUserandIOrganizationinterfaces share common fields (id, name, email, wallet, createdAt, updatedAt) but have different optional fields. Consider using a discriminated union withIProfileto reduce duplication and improve type safety.export interface BaseProfile { id: string; name: string; email: string; wallet: string; createdAt: Date; updatedAt: Date; } export interface IUser extends BaseProfile { profileType: "user"; lastName: string | null; isVerified: boolean; } export interface IOrganization extends BaseProfile { profileType: "organization"; category: string; } export type IProfile = IUser | IOrganization;Also applies to: 12-20
openapi.yaml (6)
231-239: Add regex pattern for Stellar public keys.Enforce format in path and body params with a pattern.
schema: type: string minLength: 56 maxLength: 56 + pattern: '^G[A-Z2-7]{55}$'Repeat for all walletAddress schema uses (validate-wallet, check-wallet, RegisterRequest, LoginRequest).
Also applies to: 273-281, 805-811
7-12: Update description to match non-versioned API.Header still says “with versioning support.” Remove or reword to reflect unified /api surface.
Also applies to: 13-19
343-376: Consider top-level security to DRY protected endpoints.Define top-level security: BearerAuth, then set security: [] on public routes (register, login, validate/check endpoints) to override.
Add at root (same level as servers):
security: - BearerAuth: []Then add
security: []under public operations.Also applies to: 377-409, 410-442, 443-481, 482-522, 523-563
781-799: Add required fields where responses guarantee shape.Mark success and message as required (and token/user where guaranteed) to tighten contracts.
Example:
LoginResponse: type: object + required: [success, message, token, user]Apply similarly to other response schemas.
Also applies to: 812-827, 855-866, 916-927, 929-943, 944-953, 955-987
187-188: Document token TTL explicitly.You state “valid for 24 hours”; encode in schema (e.g., expiresIn seconds) or in description of BearerAuth.
token: type: string description: "JWT token for authentication" example: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9..." + expiresIn: + type: integer + description: "Token TTL in seconds" + example: 86400Also applies to: 424-441
995-999: Trim trailing spaces flagged by yamllint.Clean up trailing spaces to pass CI linters.
Also applies to: 17-17, 120-120, 183-183, 224-224, 267-267, 309-309, 350-350, 384-384, 417-417, 450-450, 489-489, 530-530, 997-997
src/index.ts (2)
45-46: Consider JSON body size limit.Set a sane limit (e.g., 1mb) to prevent abuse.
-app.use(express.json()); +app.use(express.json({ limit: "1mb" }));
186-190: Prefer Logger over console. for consistency.*Use globalLogger for Redis init logs.
- console.log("Redis connected successfully!"); + globalLogger.info("Redis connected successfully!"); ... - console.error("Error during Redis initialization:", error); + globalLogger.error("Error during Redis initialization", error);src/routes/index.ts (1)
16-23: Avoid hardcoding version string.Source version from a single place (env/config/package.json) to keep OpenAPI and code synchronized.
- version: "1.0.0", + version: process.env.API_VERSION ?? "1.0.0",And mirror in Swagger/OpenAPI generation.
src/shared/middleware/validation.middleware.ts (3)
81-82: Avoid mutating req.query type; attach validatedQuery to req instead.Casting to ParsedQs may mask type issues. Prefer adding req.validatedQuery.
- req.query = dto as unknown as ParsedQs; + (req as any).validatedQuery = dto;And consume req.validatedQuery in handlers.
137-171: Consolidate validation logic and use plainToInstance; consider whitelist to strip unknowns.plainToClass is deprecated; enable whitelist to drop extra fields and reduce attack surface. Also, factor common error building to avoid duplication.
-import { plainToClass } from "class-transformer"; +import { plainToInstance } from "class-transformer"; ... - const dto = plainToClass(dtoClass, payload); - const errors = await validate(dto); + const dto = plainToInstance(dtoClass, payload, { exposeDefaultValues: true }); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: false });Optionally reuse a small helper to build ValidationErrorResponse.
26-41: Consider not echoing invalid values back in errors.Returning error.value can leak PII. Drop value or gate behind NODE_ENV === "development".
- value: error.value, + // value omitted to avoid echoing inputsAlso applies to: 64-79, 102-117, 159-169
src/modules/auth/use-cases/register.usecase.ts (2)
9-14: Consider dependency injection for better testability.The AuthRepository is instantiated directly in the constructor, which creates tight coupling and makes unit testing harder.
Consider using dependency injection:
-export class RegisterUseCase { - private authRepository: AuthRepository; - - constructor() { - this.authRepository = new AuthRepository(); - } +export class RegisterUseCase { + private authRepository: AuthRepository; + + constructor(authRepository?: AuthRepository) { + this.authRepository = authRepository || new AuthRepository(); + }
176-178: Consider more comprehensive email validation.The current regex pattern for email validation is basic and may not catch all invalid formats. It doesn't handle special cases like consecutive dots or spaces in local parts.
Consider using a more robust email validation pattern or a dedicated library:
- const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + // More comprehensive email validation + const emailRegex = /^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/;src/types/auth.types.ts (1)
50-62: Type guard can be simplified using type predicates.The current implementation has verbose type casting that can be simplified.
Simplify the type guard implementation:
-export function isAuthenticatedUser(user: unknown): user is AuthenticatedUser { - return ( - user !== null && - typeof user === "object" && - "userId" in user && - "email" in user && - "profileType" in user && - typeof (user as Record<string, unknown>).userId === "string" && - typeof (user as Record<string, unknown>).email === "string" && - ((user as Record<string, unknown>).profileType === "user" || - (user as Record<string, unknown>).profileType === "organization") - ); +export function isAuthenticatedUser(user: unknown): user is AuthenticatedUser { + if (!user || typeof user !== "object") return false; + + const u = user as Record<string, unknown>; + return ( + typeof u.userId === "string" && + typeof u.email === "string" && + (u.profileType === "user" || u.profileType === "organization") + );src/modules/auth/infrastructure/repositories/auth.repository.ts (2)
65-99: Performance optimization: Avoid redundant wallet normalizationThe wallet address is normalized again even though
findUserByWalletandfindOrganizationByWalletalready normalize it internally.async findProfileByWallet(walletAddress: string): Promise<IProfile | null> { try { - const normalizedAddress = - WalletValidationService.normalizeWalletAddress(walletAddress); - // Try to find user first - const user = await this.findUserByWallet(normalizedAddress); + const user = await this.findUserByWallet(walletAddress); if (user) { return { name: user.name, email: user.email, wallet: user.wallet, profileType: "user" as const, }; } // Try to find organization const organization = - await this.findOrganizationByWallet(normalizedAddress); + await this.findOrganizationByWallet(walletAddress);
168-168: Potential data inconsistency with empty lastNameSetting
lastNameto an empty string when it's not provided may lead to data inconsistency. The database field appears to be nullable based on the interface.- lastName: userData.lastName || "", + lastName: userData.lastName || null,src/modules/auth/use-cases/login.usecase.ts (1)
194-195: TODO implementation: Token blacklist for logoutThe comment indicates that token invalidation should be implemented for production. Without server-side token invalidation, logged-out tokens remain valid until expiry.
Would you like me to help implement a Redis-based token blacklist or create an issue to track this security enhancement?
src/modules/auth/presentation/controllers/Auth.controller.ts (2)
415-415: Singleton pattern may cause issues in testing and scalingCreating a singleton instance with
export default new AuthController()can make testing difficult and prevent proper dependency injection.Consider exporting the class and letting the consumer instantiate it:
-export default new AuthController(); +export default AuthController;Then update the routes to instantiate it:
// In routes file const authController = new AuthController();
381-402: Consider consolidating duplicate error codesMultiple error codes map to the same status code. Consider grouping them for better maintainability.
private getStatusCodeForError(code?: string): number { + const errorGroups = { + 400: ["INVALID_WALLET_FORMAT", "INVALID_EMAIL_FORMAT", "MISSING_REQUIRED_FIELDS", "MISSING_CATEGORY"], + 401: ["WALLET_NOT_FOUND", "USER_NOT_FOUND", "INVALID_TOKEN", "INVALID_REFRESH_TOKEN", "INVALID_LOGOUT_TOKEN", "NO_TOKEN"], + 409: ["WALLET_ALREADY_REGISTERED", "EMAIL_ALREADY_REGISTERED"], + }; + + for (const [status, codes] of Object.entries(errorGroups)) { + if (codes.includes(code || "")) { + return parseInt(status); + } + } + return 500; - switch (code) { - case "INVALID_WALLET_FORMAT": - case "INVALID_EMAIL_FORMAT": - case "MISSING_REQUIRED_FIELDS": - case "MISSING_CATEGORY": - return 400; - case "WALLET_ALREADY_REGISTERED": - case "EMAIL_ALREADY_REGISTERED": - return 409; - case "WALLET_NOT_FOUND": - case "USER_NOT_FOUND": - case "INVALID_TOKEN": - case "INVALID_REFRESH_TOKEN": - case "INVALID_LOGOUT_TOKEN": - return 401; - case "NO_TOKEN": - return 401; - default: - return 500; - } }src/modules/auth/dto/login.dto.ts (1)
4-11: Optional: annotate for Swagger/OpenAPI generation.If you autogenerate docs from DTOs, add ApiProperty with min/max length and description to keep code and openapi.yaml in sync.
Apply:
+// import { ApiProperty } from "@nestjs/swagger"; export class LoginDto { - @IsString({ message: "Wallet address must be a string" }) + // @ApiProperty({ description: "Stellar account (G...) or muxed (M...) address", minLength: 56, maxLength: 56 }) + @IsString({ message: "Wallet address must be a string" })
📜 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(7 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)
- readme.md
- src/routes/certificatesRoutes.ts
- src/routes/VolunteerRoutes.ts
- src/routes/userRoutes.ts
- src/routes/OrganizationRoutes.ts
- src/routes/v2/auth.routes.ts
- src/routes/v1/index.ts
- src/routes/testRoutes.ts
- src/routes/nftRoutes.ts
- src/routes/ProjectRoutes.ts
🧰 Additional context used
🧬 Code graph analysis (7)
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/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/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] 120-120: trailing spaces
(trailing-spaces)
[error] 183-183: trailing spaces
(trailing-spaces)
[error] 224-224: trailing spaces
(trailing-spaces)
[error] 267-267: trailing spaces
(trailing-spaces)
[error] 309-309: trailing spaces
(trailing-spaces)
[error] 350-350: trailing spaces
(trailing-spaces)
[error] 384-384: trailing spaces
(trailing-spaces)
[error] 417-417: trailing spaces
(trailing-spaces)
[error] 450-450: trailing spaces
(trailing-spaces)
[error] 489-489: trailing spaces
(trailing-spaces)
[error] 530-530: trailing spaces
(trailing-spaces)
[error] 995-995: trailing spaces
(trailing-spaces)
[error] 997-997: trailing spaces
(trailing-spaces)
🔇 Additional comments (12)
docs/wallet-auth-flow.md (1)
93-96: JWT expiration in docs matches implementation. DEFAULT_EXPIRATION in JWTService is set to “24h”, aligning with the documented 24-hour token expiration.prisma/schema.prisma (1)
22-22: Excellent alignment with wallet-based authentication.The change to
isVerifieddefault value fromfalsetotruecorrectly implements the auto-verification requirement for wallet-based authentication mentioned in the documentation.openapi.yaml (2)
118-126: Registration example contradicts text.Text says “For organizations, category field is required” but example uses profileType: "project". Fix example to "organization".
Also applies to: 151-175
87-111: Health response example: ensure alignment with /api/health.Schema matches router’s /api/health payload (status/version/timestamp). Confirm Swagger points to server base /api (it does), so this is OK.
src/index.ts (1)
77-137: Two health endpoints diverge (/health vs /api/health). Confirm intent and document both.Top-level /health returns detailed service diagnostics; /api/health returns a simple payload. If intentional, add both to OpenAPI (with different schemas). Otherwise, consolidate.
Also applies to: 139-141
prisma/migrations/20250823062033_remove_password_fields/migration.sql (1)
9-14: No code references to dropped password fields.
- Confirm backup/migration plan for historical password data.
- Validate that isVerified default=true aligns with your security threat model.
src/routes/index.ts (2)
6-13: Health payload is simple and matches OpenAPI /api/health. LGTM.
26-27: Auth routes wiring looks good.Mounting at /api/auth matches the documented paths.
src/modules/auth/domain/services/wallet-validation.service.ts (1)
1-97: LGTM! Well-structured wallet validation service.The implementation provides comprehensive Stellar wallet address validation with good error handling and detailed validation messages. The service correctly validates the 56-character length and 'G' prefix requirements for Stellar public keys.
src/routes/authRoutes.ts (1)
24-26: No changes needed for middleware import pattern
The default export insrc/middleware/authMiddleware.tsis an object containingauthMiddleware, so importing it asimport authMiddleware from "../middleware/authMiddleware";and using
authMiddleware.authMiddlewareis correct.Likely an incorrect or invalid review comment.
src/middleware/authMiddleware.ts (1)
23-26: No type mismatch:DecodedUserincludes all requiredAuthenticatedUserproperties (userId,profileType) and adds only optionaliat/exp, so it’s structurally assignable toAuthenticatedUser.Likely an incorrect or invalid review comment.
src/modules/auth/dto/login.dto.ts (1)
4-11: Verify and remove all lingering email/password references and align OpenAPI
Multiple email/password mentions remain across the auth module (repositories, domain interfaces, DTOs, use cases, controllers, tests) and in the OpenAPI spec’s schemas and paths. Update everything to use onlywalletAddress, drop deprecated endpoints/DTO fields, and ensure your globalValidationPipeis configured withwhitelist: true,transform: true, andforbidNonWhitelisted: true.
| "name": "John Doe", | ||
| "email": "[email protected]", | ||
| "walletAddress": "GXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX", | ||
| "profileType": "user", // or "project" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent profileType terminology between interface and documentation.
The documentation shows "profileType": "user" and "profileType": "project" in the registration examples, but the domain interface IRegisterRequest uses "user" | "project" while IProfile uses "user" | "organization". This mismatch could lead to confusion.
Standardize the terminology across the system. Based on the domain interface patterns, consider updating the documentation to use consistent terminology:
- "profileType": "user", // or "project"
+ "profileType": "user", // or "organization"-2. **Project/Organization Profile** (`profileType: "project"`)
+2. **Organization Profile** (`profileType: "organization"`)Also applies to: 38-38
| walletAddress: | ||
| type: string | ||
| minLength: 56 | ||
| maxLength: 56 | ||
| description: "Stellar wallet address (56 characters)" | ||
| example: "GXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" | ||
| profileType: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Standardize wallet field name.
Schemas use walletAddress, but UserProfile uses wallet. Prefer walletAddress everywhere to avoid client confusion.
- wallet:
+ walletAddress:
type: string
description: "Stellar wallet address"Also applies to: 845-849
🤖 Prompt for AI Agents
In openapi.yaml around lines 758-764 (and also apply same change at lines
845-849), the schema field is defined as walletAddress but the UserProfile
schema still uses wallet; update the UserProfile schema to use walletAddress
consistently: rename the wallet property to walletAddress, preserve
type/validation (string, minLength/maxLength 56), update its description and
example to the Stellar address format, and adjust any required arrays, examples,
request/response examples, and $ref usages that reference the old wallet
property to the new walletAddress name so all schemas and docs remain
consistent.
| type: string | ||
| enum: [user, project] | ||
| description: "Type of profile to create" | ||
| example: "user" | ||
| lastName: |
There was a problem hiding this comment.
Unify profileType values ("project" vs "organization").
RegisterRequest enum uses [user, project] while UserProfile uses [user, organization]. Pick one (“organization” appears elsewhere) and update examples accordingly.
- profileType:
- type: string
- enum: [user, project]
+ profileType:
+ type: string
+ enum: [user, organization]And in examples, replace "project" with "organization".
Also applies to: 850-853
🤖 Prompt for AI Agents
In openapi.yaml around lines 765-769 (and similarly around 850-853), the
profileType enum is inconsistent between "project" and "organization"; update
the enum values to use "organization" everywhere (replace any "project" enum
entries with "organization"), and update all examples that currently show
"project" to "organization" so the RegisterRequest and UserProfile types are
consistent across the spec.
| }; | ||
| const decoded = jwt.verify( | ||
| token, | ||
| process.env.JWT_SECRET || "your-secret-key" |
There was a problem hiding this comment.
Security: Avoid hardcoded default secret keys in production code
Using a hardcoded fallback secret ("your-secret-key") is a security vulnerability. This should fail securely if the environment variable is not set.
const decoded = jwt.verify(
token,
- process.env.JWT_SECRET || "your-secret-key"
+ process.env.JWT_SECRET || (() => {
+ throw new Error("JWT_SECRET environment variable is not configured");
+ })()
) as DecodedUser;Or better yet, validate this at application startup:
+const JWT_SECRET = process.env.JWT_SECRET;
+if (!JWT_SECRET) {
+ throw new Error("JWT_SECRET environment variable must be set");
+}
export const authMiddleware = (
req: AuthenticatedRequest,
res: Response,
next: NextFunction
): void => {
try {
const token = req.headers.authorization?.replace("Bearer ", "");
if (!token) {
res.status(401).json({
success: false,
message: "Access token required",
});
return;
}
const decoded = jwt.verify(
token,
- process.env.JWT_SECRET || "your-secret-key"
+ JWT_SECRET
) as DecodedUser;📝 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.
| process.env.JWT_SECRET || "your-secret-key" | |
| // src/middleware/authMiddleware.ts | |
| // Ensure we never start without a configured secret | |
| const JWT_SECRET = process.env.JWT_SECRET; | |
| if (!JWT_SECRET) { | |
| throw new Error("JWT_SECRET environment variable must be set"); | |
| } | |
| export const authMiddleware = ( | |
| req: AuthenticatedRequest, | |
| res: Response, | |
| next: NextFunction | |
| ): void => { | |
| try { | |
| const token = req.headers.authorization?.replace("Bearer ", ""); | |
| if (!token) { | |
| res.status(401).json({ | |
| success: false, | |
| message: "Access token required", | |
| }); | |
| return; | |
| } | |
| const decoded = jwt.verify( | |
| token, | |
| // use the validated startup variable instead of a hardcoded default | |
| JWT_SECRET | |
| ) as DecodedUser; | |
| req.user = decoded; | |
| next(); | |
| } catch (err) { | |
| res.status(401).json({ | |
| success: false, | |
| message: "Invalid or expired access token", | |
| }); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In src/middleware/authMiddleware.ts around line 23, the code uses a hardcoded
fallback JWT secret ("your-secret-key"); remove the hardcoded default and fail
securely by requiring process.env.JWT_SECRET to be set. Replace the inline
fallback with a single source of truth validated at application startup (e.g.,
during bootstrap) that throws a clear error and exits if JWT_SECRET is missing,
and have the middleware read the validated secret from that configuration
instead of providing its own default.
| name: string; | ||
| email: string; | ||
| walletAddress: string; | ||
| profileType: "user" | "project"; |
There was a problem hiding this comment.
Inconsistent profileType values between interfaces.
There's an inconsistency in profileType values: IRegisterRequest uses "user" | "project" while IProfile and IJWTPayload use "user" | "organization". This will cause type errors when transforming registration requests to profiles.
Standardize the profileType values across all interfaces:
export interface IRegisterRequest {
name: string;
email: string;
walletAddress: string;
- profileType: "user" | "project";
+ profileType: "user" | "organization";
lastName?: string;
category?: string;
}Also applies to: 59-59
🤖 Prompt for AI Agents
In src/modules/auth/domain/interfaces/auth.interface.ts around lines 38 and 59,
the profileType union is inconsistent between interfaces (IRegisterRequest uses
"user" | "project" while IProfile and IJWTPayload use "user" | "organization");
change the profileType union so all interfaces use the same values (pick
one—preferably "user" | "organization" to match IProfile/IJWTPayload) and update
any other occurrences/imports to that same union, then run type-checks and
adjust any affected code or tests that expect "project" to use "organization"
instead.
| constructor() { | ||
| this.authRepository = new AuthRepository(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Memory leak risk: Repository instance not properly managed
Creating a new AuthRepository instance in the constructor without proper cleanup can lead to connection leaks if multiple LoginUseCase instances are created.
Consider dependency injection or ensuring singleton usage:
export class LoginUseCase {
private authRepository: AuthRepository;
- constructor() {
- this.authRepository = new AuthRepository();
+ constructor(authRepository?: AuthRepository) {
+ this.authRepository = authRepository || new AuthRepository();
}📝 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.
| constructor() { | |
| this.authRepository = new AuthRepository(); | |
| } | |
| export class LoginUseCase { | |
| private authRepository: AuthRepository; | |
| constructor(authRepository?: AuthRepository) { | |
| this.authRepository = authRepository || new AuthRepository(); | |
| } | |
| // ... other methods ... | |
| } |
🤖 Prompt for AI Agents
In src/modules/auth/use-cases/login.usecase.ts around lines 14–16, the
constructor creates a new AuthRepository per instance which can leak
connections; change the constructor to accept an AuthRepository via dependency
injection (e.g., constructor(authRepository: AuthRepository) {
this.authRepository = authRepository; }) and remove inline instantiation, or
alternatively use a shared singleton factory (AuthRepository.getInstance()) so
only one connection is used; ensure the repository exposes a close/dispose
method and, if the use case is responsible for lifecycle, add a dispose/cleanup
method that calls it, and update all callers/tests to pass the shared repo
instance.
| !["user", "project"].includes(registerData.profileType) | ||
| ) { | ||
| errors.push("Profile type must be either 'user' or 'project'"); | ||
| } |
There was a problem hiding this comment.
Inconsistent profile type validation.
The validation accepts "project" as a valid profile type, but the rest of the codebase uses "organization" (as seen in line 110, 133, and the related interfaces).
Apply this fix to align the profile type values:
- 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'");📝 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.
| !["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'"); | |
| } |
🤖 Prompt for AI Agents
In src/modules/auth/use-cases/register.usecase.ts around lines 227 to 230, the
profile type validation currently allows "project" which is inconsistent with
the rest of the codebase that uses "organization" (see lines 110 and 133 and
related interfaces); update the validation to check for ["user", "organization"]
instead of ["user", "project"] and adjust the error message to say "Profile type
must be either 'user' or 'organization'" so it matches the domain model and
interfaces.
| 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.
Inconsistent profileType check.
Line 233 checks for "project" but should check for "organization" to match the interface definition and the rest of the implementation.
Apply this fix:
- if (registerData.profileType === "project") {
+ if (registerData.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.
| 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"); | |
| } | |
| } | |
| 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 233 to 239, the
block currently validates the category when registerData.profileType ===
"project" but the intent (and interface/implementation) is to validate category
for "organization"; change the conditional to check for "organization" instead
of "project" so category presence and length validation run for organizations as
intended.
| router.post( | ||
| "/validate-token", | ||
| authMiddleware.authMiddleware, | ||
| AuthController.validateToken | ||
| ); | ||
| router.post( | ||
| "/refresh-token", | ||
| authMiddleware.authMiddleware, | ||
| AuthController.refreshToken | ||
| ); | ||
| router.post("/logout", authMiddleware.authMiddleware, AuthController.logout); |
There was a problem hiding this comment.
Inconsistent middleware usage pattern.
All protected routes use authMiddleware.authMiddleware which appears to be an incorrect access pattern. This pattern is repeated across multiple routes.
Based on the typical export pattern, this should likely be:
-router.post(
- "/validate-token",
- authMiddleware.authMiddleware,
- AuthController.validateToken
-);
+router.post(
+ "/validate-token",
+ authMiddleware,
+ AuthController.validateToken
+);Apply the same fix to lines 29, 34, 37, 42, 43, 54, and 55.
📝 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( | |
| "/validate-token", | |
| authMiddleware.authMiddleware, | |
| AuthController.validateToken | |
| ); | |
| router.post( | |
| "/refresh-token", | |
| authMiddleware.authMiddleware, | |
| AuthController.refreshToken | |
| ); | |
| router.post("/logout", authMiddleware.authMiddleware, AuthController.logout); | |
| router.post( | |
| "/validate-token", | |
| authMiddleware, | |
| AuthController.validateToken | |
| ); |
🤖 Prompt for AI Agents
In src/routes/authRoutes.ts around lines 27 to 37 (and also apply to lines 29,
34, 37, 42, 43, 54, and 55), the router is calling authMiddleware.authMiddleware
which is the wrong access pattern; replace those usages to call the middleware
function directly (e.g., authMiddleware) or the correct exported name from the
module, ensuring the import matches that export (adjust the import to default or
named import if needed); update each router.post(...) middleware argument to use
the middleware function itself rather than authMiddleware.authMiddleware.
| (req: AuthenticatedRequest, res) => { | ||
| res.json({ | ||
| success: true, | ||
| message: "User-only route accessed successfully", | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inline route handlers should be extracted to controller methods.
The inline route handlers for /user-only and /organization-only routes break the pattern of using controller methods and make the routes harder to test.
Consider moving these handlers to the AuthController:
-router.get(
- "/user-only",
- authMiddleware.authMiddleware,
- authMiddleware.requireUserProfile,
- (req: AuthenticatedRequest, res) => {
- res.json({
- success: true,
- message: "User-only route accessed successfully",
- });
- }
-);
+router.get(
+ "/user-only",
+ authMiddleware,
+ authMiddleware.requireUserProfile,
+ AuthController.userOnlyRoute
+);Then add the corresponding methods to AuthController.
Also applies to: 56-61
🤖 Prompt for AI Agents
In src/routes/authRoutes.ts around lines 44-49 and 56-61, the inline handlers
for the /user-only and /organization-only routes should be moved into
AuthController to preserve controller pattern and improve testability; add two
methods on AuthController (e.g., userOnly and organizationOnly) that accept
(req: AuthenticatedRequest, res) and send the existing JSON responses, export or
instantiate the controller as used by other routes, then replace the inline
functions in the route file with references to the controller methods (bind the
controller instance or use arrow wrappers as your project prefers) and update
any imports/types accordingly.
3 participants
🚀 Volunchain Pull Request
✅ Checklist
Marca con una
xtodas las casillas que apliquen (como[x])📌 Type of Change
📝 Changes description
This PR fully documents all endpoints in the auth module using OpenAPI 3.0 standards, as per the issue description.
The changes include:
openapi.yamlundercomponents.schemas.This work lays the foundation for documenting other modules.
📸 Evidence (A photo is required as evidence)
[Attach a screenshot of the updated Swagger UI showing the documented auth endpoints here.]
⏰ Time spent breakdown
Total: 2 hours
🌌 Comments
This documentation will greatly improve the clarity and usability of the API for both internal and external developers. The consistent format will make future documentation efforts much smoother.
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