612 lines
14 KiB
Markdown
612 lines
14 KiB
Markdown
# Code Review Report - FastKart Admin Dashboard
|
|
|
|
**Project:** FastKart Next.js Admin Dashboard
|
|
**Review Date:** January 2026
|
|
**Reviewer:** Qodo AI Code Review
|
|
**Framework:** Next.js 15.2.4 with React 19.0.0
|
|
|
|
---
|
|
|
|
## Executive Summary
|
|
|
|
This is a comprehensive admin dashboard built with Next.js 15, React 19, and various supporting libraries. The application follows a modern architecture with internationalization support, role-based access control, and a multi-tenant store management system.
|
|
|
|
### Overall Assessment: ⚠️ **Needs Improvement**
|
|
|
|
**Strengths:**
|
|
- Modern tech stack (Next.js 15, React 19)
|
|
- Comprehensive feature set
|
|
- Internationalization support
|
|
- Role-based permission system
|
|
|
|
**Critical Issues Found:**
|
|
- 🔴 **Security vulnerabilities** (XSS, loose equality, localStorage usage)
|
|
- 🟡 **Code quality issues** (console statements, loose equality operators)
|
|
- 🟡 **Missing configuration** (incomplete .gitignore)
|
|
- 🟡 **Performance concerns** (reactStrictMode disabled)
|
|
|
|
---
|
|
|
|
## 🔴 Critical Security Issues
|
|
|
|
### 1. XSS Vulnerability in TextLimit Component
|
|
**File:** `src/utils/customFunctions/TextLimit.js`
|
|
**Severity:** 🔴 **CRITICAL**
|
|
|
|
```javascript
|
|
// VULNERABLE CODE
|
|
const sanitizeAndTrustHtml = (htmlString) => {
|
|
return { __html: htmlString };
|
|
};
|
|
```
|
|
|
|
**Issue:** The `sanitizeAndTrustHtml` function doesn't actually sanitize HTML - it just wraps it for `dangerouslySetInnerHTML`. This creates a direct XSS vulnerability.
|
|
|
|
**Recommendation:**
|
|
```javascript
|
|
import DOMPurify from 'isomorphic-dompurify';
|
|
|
|
const sanitizeAndTrustHtml = (htmlString) => {
|
|
return { __html: DOMPurify.sanitize(htmlString) };
|
|
};
|
|
```
|
|
|
|
**Action Required:** Install `isomorphic-dompurify` and implement proper HTML sanitization.
|
|
|
|
---
|
|
|
|
### 2. Sensitive Data in localStorage
|
|
**Files:** Multiple files across the application
|
|
**Severity:** 🔴 **HIGH**
|
|
|
|
**Issue:** Storing sensitive data like user roles and account information in localStorage:
|
|
- `localStorage.setItem("role", JSON.stringify(data?.role))` - AccountProvider.js
|
|
- `localStorage.getItem("account")` - Multiple files
|
|
|
|
**Risks:**
|
|
- Accessible via XSS attacks
|
|
- Persists across sessions
|
|
- No encryption
|
|
- Vulnerable to browser extensions
|
|
|
|
**Recommendation:**
|
|
1. Use httpOnly cookies for sensitive data
|
|
2. Implement proper session management
|
|
3. Use secure, encrypted storage for client-side data
|
|
4. Consider using next-auth or similar authentication library
|
|
|
|
---
|
|
|
|
### 3. Empty Middleware Implementation
|
|
**File:** `src/middleware.js`
|
|
**Severity:** 🟡 **MEDIUM**
|
|
|
|
```javascript
|
|
export async function middleware(request) {
|
|
// Put Your Logic Here
|
|
}
|
|
```
|
|
|
|
**Issue:** Middleware is configured for all routes but has no implementation. This means:
|
|
- No authentication checks
|
|
- No authorization validation
|
|
- No request validation
|
|
|
|
**Recommendation:**
|
|
```javascript
|
|
import { NextResponse } from 'next/server';
|
|
|
|
export async function middleware(request) {
|
|
const token = request.cookies.get('uat')?.value;
|
|
|
|
// Check if user is authenticated
|
|
if (!token && !request.nextUrl.pathname.startsWith('/auth')) {
|
|
return NextResponse.redirect(new URL('/auth/login', request.url));
|
|
}
|
|
|
|
// Add additional security checks here
|
|
return NextResponse.next();
|
|
}
|
|
```
|
|
|
|
---
|
|
|
|
### 4. Loose Equality Operators (==)
|
|
**Files:** 200+ instances across the codebase
|
|
**Severity:** 🟡 **MEDIUM**
|
|
|
|
**Issue:** Extensive use of `==` instead of `===` can lead to unexpected type coercion bugs.
|
|
|
|
**Examples:**
|
|
```javascript
|
|
// src/utils/customFunctions/GetCookie.js
|
|
while (c.charAt(0) == " ") { ... }
|
|
if (c.indexOf(name) == 0) { ... }
|
|
if (username != "" && username) { ... }
|
|
|
|
// src/helper/settingContext/SettingProvider.js
|
|
if (position == 'before_price') { ... }
|
|
data?.values?.general['mode'] == "dark-only"
|
|
```
|
|
|
|
**Recommendation:** Replace all `==` with `===` and `!=` with `!==` throughout the codebase.
|
|
|
|
---
|
|
|
|
## 🟡 Code Quality Issues
|
|
|
|
### 5. Console Statements in Production Code
|
|
**Files:** 3 instances found
|
|
**Severity:** 🟡 **LOW**
|
|
|
|
**Locations:**
|
|
1. `src/components/product/DateRangePicker.js:35` - `console.log('Date range error handled:', error.message);`
|
|
2. `src/components/role/PermissionForm.js:19` - `console.log(errors[0]?.message);`
|
|
3. `src/app/[lng]/layout.js:7` - `console.log("err", err)`
|
|
|
|
**Recommendation:**
|
|
- Remove or replace with proper logging service
|
|
- Use environment-based logging (only in development)
|
|
- Consider using a logging library like `winston` or `pino`
|
|
|
|
---
|
|
|
|
### 6. React Strict Mode Disabled
|
|
**File:** `next.config.js`
|
|
**Severity:** 🟡 **MEDIUM**
|
|
|
|
```javascript
|
|
const nextConfig = {
|
|
reactStrictMode: false, // ❌ BAD PRACTICE
|
|
// ...
|
|
}
|
|
```
|
|
|
|
**Issue:** Disabling strict mode hides potential problems:
|
|
- Unsafe lifecycle methods
|
|
- Legacy string ref API usage
|
|
- Unexpected side effects
|
|
- Deprecated findDOMNode usage
|
|
|
|
**Recommendation:** Enable strict mode and fix any warnings that appear:
|
|
```javascript
|
|
const nextConfig = {
|
|
reactStrictMode: true,
|
|
// ...
|
|
}
|
|
```
|
|
|
|
---
|
|
|
|
### 7. Incomplete .gitignore
|
|
**File:** `.gitignore`
|
|
**Severity:** 🟡 **MEDIUM**
|
|
|
|
**Current content:**
|
|
```
|
|
.vercel
|
|
```
|
|
|
|
**Issue:** Missing critical entries that should not be committed:
|
|
- `node_modules/`
|
|
- `.env` and `.env.local`
|
|
- `.next/`
|
|
- Build artifacts
|
|
- IDE configurations
|
|
|
|
**Recommendation:**
|
|
```gitignore
|
|
# Dependencies
|
|
node_modules/
|
|
/.pnp
|
|
.pnp.js
|
|
|
|
# Testing
|
|
/coverage
|
|
|
|
# Next.js
|
|
/.next/
|
|
/out/
|
|
|
|
# Production
|
|
/build
|
|
|
|
# Misc
|
|
.DS_Store
|
|
*.pem
|
|
|
|
# Debug
|
|
npm-debug.log*
|
|
yarn-debug.log*
|
|
yarn-error.log*
|
|
|
|
# Local env files
|
|
.env
|
|
.env*.local
|
|
|
|
# Vercel
|
|
.vercel
|
|
|
|
# TypeScript
|
|
*.tsbuildinfo
|
|
next-env.d.ts
|
|
|
|
# IDE
|
|
.vscode/
|
|
.idea/
|
|
*.swp
|
|
*.swo
|
|
```
|
|
|
|
---
|
|
|
|
### 8. Hardcoded API URLs
|
|
**File:** `next.config.js`
|
|
**Severity:** 🟡 **LOW**
|
|
|
|
```javascript
|
|
env: {
|
|
API_PROD_URL: "https://fastkart-admin-json.vercel.app/api/",
|
|
// API_PROD_URL: "http://localhost:3000/api/",
|
|
}
|
|
```
|
|
|
|
**Issue:** Commented-out code and hardcoded URLs make environment management difficult.
|
|
|
|
**Recommendation:**
|
|
- Use `.env` files for environment-specific configuration
|
|
- Remove commented code
|
|
- Use proper environment variable naming
|
|
|
|
```javascript
|
|
// next.config.js
|
|
env: {
|
|
API_PROD_URL: process.env.NEXT_PUBLIC_API_URL,
|
|
}
|
|
|
|
// .env.local
|
|
NEXT_PUBLIC_API_URL=http://localhost:3000/api/
|
|
|
|
// .env.production
|
|
NEXT_PUBLIC_API_URL=https://fastkart-admin-json.vercel.app/api/
|
|
```
|
|
|
|
---
|
|
|
|
### 9. Error Handling in Axios Interceptor
|
|
**File:** `src/utils/axiosUtils/index.js`
|
|
**Severity:** 🟡 **MEDIUM**
|
|
|
|
```javascript
|
|
const onError = (error) => {
|
|
if (error?.response?.status == 403) {
|
|
router && router.push("/en/403")
|
|
}
|
|
return error; // ❌ Returns error instead of rejecting
|
|
};
|
|
```
|
|
|
|
**Issues:**
|
|
1. Returns error instead of throwing/rejecting
|
|
2. Only handles 403 errors
|
|
3. No logging or user feedback for other errors
|
|
4. Uses loose equality (`==`)
|
|
|
|
**Recommendation:**
|
|
```javascript
|
|
const onError = (error) => {
|
|
const status = error?.response?.status;
|
|
|
|
// Log error for debugging
|
|
console.error('API Error:', error);
|
|
|
|
// Handle specific status codes
|
|
switch(status) {
|
|
case 401:
|
|
// Unauthorized - redirect to login
|
|
router?.push("/en/auth/login");
|
|
break;
|
|
case 403:
|
|
// Forbidden
|
|
router?.push("/en/403");
|
|
break;
|
|
case 404:
|
|
// Not found
|
|
toast.error('Resource not found');
|
|
break;
|
|
case 500:
|
|
// Server error
|
|
toast.error('Server error. Please try again later.');
|
|
break;
|
|
default:
|
|
// Generic error
|
|
toast.error(error?.response?.data?.message || 'An error occurred');
|
|
}
|
|
|
|
return Promise.reject(error); // ✅ Properly reject the promise
|
|
};
|
|
```
|
|
|
|
---
|
|
|
|
### 10. Permission Check Logic Issues
|
|
**File:** `src/layout/index.js`
|
|
**Severity:** 🟡 **MEDIUM**
|
|
|
|
```javascript
|
|
useEffect(() => {
|
|
const securePaths = mounted && ConvertPermissionArr(data1?.permissions);
|
|
if (mounted && !securePaths.find((item) => item?.name == replacePath(path?.split("/")[2]))) {
|
|
router.push(`/403`);
|
|
}
|
|
}, [data1]);
|
|
```
|
|
|
|
**Issues:**
|
|
1. Missing dependencies in useEffect (mounted, path, router)
|
|
2. Loose equality operator
|
|
3. No loading state while checking permissions
|
|
4. Redirects to `/403` without language prefix
|
|
|
|
**Recommendation:**
|
|
```javascript
|
|
useEffect(() => {
|
|
if (!mounted || !data1) return;
|
|
|
|
const securePaths = ConvertPermissionArr(data1?.permissions);
|
|
const currentPath = replacePath(path?.split("/")[2]);
|
|
const hasPermission = securePaths.some((item) => item?.name === currentPath);
|
|
|
|
if (!hasPermission) {
|
|
router.push(`/${props.lng}/403`);
|
|
}
|
|
}, [mounted, data1, path, router, props.lng]);
|
|
```
|
|
|
|
---
|
|
|
|
## 📊 Architecture & Best Practices
|
|
|
|
### 11. Component Organization
|
|
**Status:** ✅ **GOOD**
|
|
|
|
The project follows a clear component structure:
|
|
- `/components` - Reusable UI components
|
|
- `/layout` - Layout components
|
|
- `/utils` - Utility functions and hooks
|
|
- `/helper` - Context providers
|
|
- `/app` - Next.js app directory structure
|
|
|
|
**Recommendation:** Consider adding:
|
|
- `/types` - TypeScript type definitions (if migrating to TS)
|
|
- `/constants` - Application constants
|
|
- `/services` - API service layer
|
|
|
|
---
|
|
|
|
### 12. State Management
|
|
**Status:** ⚠️ **NEEDS IMPROVEMENT**
|
|
|
|
**Current approach:**
|
|
- React Context for global state
|
|
- TanStack Query for server state
|
|
- Local storage for persistence
|
|
|
|
**Issues:**
|
|
- No clear separation between client and server state
|
|
- Context providers nested deeply
|
|
- localStorage used for sensitive data
|
|
|
|
**Recommendation:**
|
|
- Continue using TanStack Query for server state ✅
|
|
- Consider Zustand or Jotai for client state
|
|
- Implement proper session management
|
|
|
|
---
|
|
|
|
### 13. Internationalization
|
|
**Status:** ✅ **GOOD**
|
|
|
|
The project uses `i18next` with proper setup:
|
|
- Language detection
|
|
- Dynamic language switching
|
|
- Proper translation structure
|
|
|
|
**Minor improvements:**
|
|
- Add type safety for translation keys
|
|
- Implement lazy loading for translation files
|
|
- Add fallback language handling
|
|
|
|
---
|
|
|
|
## 🎯 Performance Recommendations
|
|
|
|
### 14. Image Optimization
|
|
**Status:** ✅ **GOOD**
|
|
|
|
Using Next.js Image component with proper remote patterns configured.
|
|
|
|
**Recommendation:** Ensure all images use the `next/image` component.
|
|
|
|
---
|
|
|
|
### 15. Dynamic Imports
|
|
**Status:** ✅ **GOOD**
|
|
|
|
Good use of dynamic imports:
|
|
```javascript
|
|
const MainDashboard = dynamic(() => import("../../../../components/dashboard"), { ssr: false })
|
|
```
|
|
|
|
**Recommendation:** Extend this pattern to other heavy components.
|
|
|
|
---
|
|
|
|
### 16. Bundle Size
|
|
**Status:** ⚠️ **NEEDS REVIEW**
|
|
|
|
**Large dependencies detected:**
|
|
- `apexcharts` (4.5.0)
|
|
- `jodit-react` (5.2.19) - Rich text editor
|
|
- `react-slick` (0.30.3)
|
|
|
|
**Recommendation:**
|
|
- Analyze bundle with `@next/bundle-analyzer`
|
|
- Consider lighter alternatives where possible
|
|
- Implement code splitting for heavy components
|
|
|
|
---
|
|
|
|
## 🧪 Testing
|
|
|
|
### 17. Test Coverage
|
|
**Status:** ❌ **MISSING**
|
|
|
|
**Issue:** No test files found in the project.
|
|
|
|
**Recommendation:**
|
|
1. Add Jest and React Testing Library
|
|
2. Implement unit tests for utility functions
|
|
3. Add integration tests for critical flows
|
|
4. Consider E2E tests with Playwright or Cypress
|
|
|
|
```bash
|
|
npm install --save-dev jest @testing-library/react @testing-library/jest-dom
|
|
```
|
|
|
|
---
|
|
|
|
## 📝 Documentation
|
|
|
|
### 18. Code Documentation
|
|
**Status:** ⚠️ **MINIMAL**
|
|
|
|
**Issues:**
|
|
- No JSDoc comments
|
|
- README is generic Next.js template
|
|
- No API documentation
|
|
- No component documentation
|
|
|
|
**Recommendation:**
|
|
1. Update README with:
|
|
- Project overview
|
|
- Setup instructions
|
|
- Environment variables
|
|
- Deployment guide
|
|
2. Add JSDoc comments to complex functions
|
|
3. Document component props with PropTypes or TypeScript
|
|
4. Create API documentation
|
|
|
|
---
|
|
|
|
## 🔧 Configuration Issues
|
|
|
|
### 19. Package.json Issues
|
|
**Status:** ⚠️ **NEEDS ATTENTION**
|
|
|
|
**Issues:**
|
|
1. Old Yup version (0.32.11) - latest is 1.x
|
|
2. No type checking scripts
|
|
3. No pre-commit hooks
|
|
4. No test scripts
|
|
|
|
**Recommendation:**
|
|
```json
|
|
{
|
|
"scripts": {
|
|
"dev": "next dev",
|
|
"build": "next build",
|
|
"start": "next start",
|
|
"lint": "next lint",
|
|
"lint:fix": "next lint --fix",
|
|
"type-check": "tsc --noEmit",
|
|
"test": "jest",
|
|
"test:watch": "jest --watch",
|
|
"prepare": "husky install"
|
|
},
|
|
"dependencies": {
|
|
"yup": "^1.4.0" // Update to latest
|
|
},
|
|
"devDependencies": {
|
|
"husky": "^9.0.0",
|
|
"lint-staged": "^15.0.0",
|
|
"@testing-library/react": "^14.0.0",
|
|
"@testing-library/jest-dom": "^6.0.0",
|
|
"jest": "^29.0.0"
|
|
}
|
|
}
|
|
```
|
|
|
|
---
|
|
|
|
## 🚀 Priority Action Items
|
|
|
|
### Immediate (Critical - Fix within 1 week)
|
|
1. ✅ **Fix XSS vulnerability** in TextLimit.js
|
|
2. ✅ **Implement middleware authentication** checks
|
|
3. ✅ **Remove sensitive data from localStorage**
|
|
4. ✅ **Update .gitignore** to prevent committing sensitive files
|
|
|
|
### High Priority (Fix within 2 weeks)
|
|
5. ✅ **Replace all `==` with `===`** throughout codebase
|
|
6. ✅ **Enable React Strict Mode** and fix warnings
|
|
7. ✅ **Implement proper error handling** in axios interceptor
|
|
8. ✅ **Remove console.log statements**
|
|
|
|
### Medium Priority (Fix within 1 month)
|
|
9. ✅ **Add comprehensive test coverage**
|
|
10. ✅ **Update documentation** (README, API docs)
|
|
11. ✅ **Implement proper logging** system
|
|
12. ✅ **Add pre-commit hooks** for code quality
|
|
|
|
### Low Priority (Ongoing improvements)
|
|
13. ✅ **Migrate to TypeScript** for better type safety
|
|
14. ✅ **Optimize bundle size**
|
|
15. ✅ **Add performance monitoring**
|
|
16. ✅ **Implement CI/CD pipeline**
|
|
|
|
---
|
|
|
|
## 📈 Code Metrics
|
|
|
|
- **Total Files Analyzed:** 400+
|
|
- **JavaScript Files:** 400+
|
|
- **Critical Issues:** 4
|
|
- **High Priority Issues:** 4
|
|
- **Medium Priority Issues:** 8
|
|
- **Low Priority Issues:** 3
|
|
|
|
---
|
|
|
|
## 🎓 Learning Resources
|
|
|
|
For the development team:
|
|
1. [Next.js Security Best Practices](https://nextjs.org/docs/app/building-your-application/configuring/security)
|
|
2. [OWASP Top 10](https://owasp.org/www-project-top-ten/)
|
|
3. [React Security Best Practices](https://react.dev/learn/security)
|
|
4. [JavaScript Equality Comparison](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness)
|
|
|
|
---
|
|
|
|
## ✅ Conclusion
|
|
|
|
This is a feature-rich admin dashboard with a solid foundation, but it requires immediate attention to security vulnerabilities and code quality issues. The architecture is generally sound, but implementation details need refinement.
|
|
|
|
**Recommended Next Steps:**
|
|
1. Address all critical security issues immediately
|
|
2. Implement a code review process for future changes
|
|
3. Add automated testing and CI/CD
|
|
4. Consider gradual migration to TypeScript
|
|
5. Establish coding standards and enforce with ESLint/Prettier
|
|
|
|
**Estimated Effort:**
|
|
- Critical fixes: 2-3 days
|
|
- High priority fixes: 1 week
|
|
- Medium priority fixes: 2-3 weeks
|
|
- Full remediation: 1-2 months
|
|
|
|
---
|
|
|
|
**Review Completed:** January 2026
|
|
**Next Review Recommended:** After critical issues are resolved
|