381 lines
11 KiB
Markdown
381 lines
11 KiB
Markdown
# Phase 1: Critical Security & Risk Mitigation - COMPLETED ✅
|
|
|
|
**Completion Date**: $(date)
|
|
**Status**: All tasks completed successfully
|
|
|
|
---
|
|
|
|
## Summary
|
|
|
|
Phase 1 focused on addressing critical security vulnerabilities and production readiness issues. All 8 planned tasks have been completed, and both apps pass type checking.
|
|
|
|
---
|
|
|
|
## ✅ Completed Tasks
|
|
|
|
### 1. Add Sensitive Files to .gitignore ✅
|
|
|
|
**Files Modified**:
|
|
|
|
- `apps/admin/.gitignore`
|
|
- `apps/mobile/.gitignore`
|
|
|
|
**Changes**:
|
|
|
|
- Added all `.env` variants to gitignore
|
|
- Added database files (`*.db`, `*.sqlite`, `backups/`)
|
|
- Added log files (`*.log`, `server.log`)
|
|
- Ensures sensitive data won't be committed in the future
|
|
|
|
**Impact**: Prevents future accidental commits of secrets
|
|
|
|
---
|
|
|
|
### 2. Create .env.example Files ✅
|
|
|
|
**Files Created/Updated**:
|
|
|
|
- `apps/admin/.env.example` - Complete example with all required variables
|
|
- `apps/mobile/.env.example` - Updated with better documentation
|
|
|
|
**Contents**:
|
|
|
|
- Clear documentation of each environment variable
|
|
- Links to where to obtain API keys
|
|
- Placeholders instead of real values
|
|
- Comments explaining usage
|
|
|
|
**Impact**: Makes it easy for new developers to set up their environment
|
|
|
|
---
|
|
|
|
### 3. Document Exposed Keys That Need Rotation ✅
|
|
|
|
**File Created**: `SECURITY.md` (root of repository)
|
|
|
|
**Documentation Includes**:
|
|
|
|
- List of all exposed API keys with severity levels
|
|
- Step-by-step rotation instructions for each service:
|
|
- Clerk keys (admin & mobile)
|
|
- DeepSeek API key
|
|
- Instructions for removing secrets from git history (BFG Repo-Cleaner + git filter-branch)
|
|
- Prevention measures implemented
|
|
- Additional security recommendations
|
|
|
|
**CRITICAL ACTION REQUIRED** (manual step):
|
|
|
|
```bash
|
|
# You must manually:
|
|
1. Go to https://dashboard.clerk.com and rotate secret keys
|
|
2. Go to https://platform.deepseek.com and delete exposed key
|
|
3. Update your local .env files with new keys (DO NOT commit)
|
|
4. Run git history cleanup commands from SECURITY.md
|
|
5. Force push to remove exposed secrets from git history
|
|
```
|
|
|
|
**Impact**: Provides clear action plan for key rotation
|
|
|
|
---
|
|
|
|
### 4. Add Role-Based Authorization Middleware ✅
|
|
|
|
**File Created**: `apps/admin/src/lib/auth-middleware.ts`
|
|
|
|
**Features**:
|
|
|
|
- `requireAuth()` - Validates authentication and checks user roles
|
|
- `isSelfModification()` - Detects if user is modifying their own account
|
|
- `preventSelfModification()` - Prevents self-deletion and self-demotion
|
|
- TypeScript types for UserRole
|
|
- Comprehensive JSDoc documentation
|
|
- Returns either auth result or error response
|
|
|
|
**Example Usage**:
|
|
|
|
```typescript
|
|
const authResult = await requireAuth(["admin", "superAdmin"]);
|
|
if (authResult instanceof NextResponse) return authResult;
|
|
const { userId, role } = authResult;
|
|
```
|
|
|
|
**Impact**: Reusable authorization for all API endpoints
|
|
|
|
---
|
|
|
|
### 5. Fix DELETE /api/users Authorization Vulnerability ✅
|
|
|
|
**File Modified**: `apps/admin/src/app/api/users/route.ts`
|
|
|
|
**Security Fixes**:
|
|
|
|
- ❌ Before: Any authenticated user could delete any user
|
|
- ✅ After: Only admin/superAdmin can delete users
|
|
- Added self-deletion prevention (can't delete own account)
|
|
- Bulk delete checks for self-deletion in array
|
|
- Uses new `requireAuth()` and `preventSelfModification()` helpers
|
|
|
|
**Code Changes**:
|
|
|
|
```typescript
|
|
// Before (NO authorization)
|
|
export async function DELETE(request: NextRequest) {
|
|
const db = await getDatabase();
|
|
// ... delete logic
|
|
|
|
// After (WITH authorization)
|
|
export async function DELETE(request: NextRequest) {
|
|
const authResult = await requireAuth(["admin", "superAdmin"]);
|
|
if (authResult instanceof NextResponse) return authResult;
|
|
|
|
const selfModError = preventSelfModification(userId, targetId, "delete");
|
|
if (selfModError) return selfModError;
|
|
// ... delete logic
|
|
```
|
|
|
|
**Impact**: Critical vulnerability fixed - prevents unauthorized user deletion
|
|
|
|
---
|
|
|
|
### 6. Add Rate Limiting to Authentication Endpoints ✅
|
|
|
|
**File Created**: `apps/admin/src/lib/rate-limit.ts`
|
|
|
|
**Features**:
|
|
|
|
- In-memory rate limiter (production-ready for single server)
|
|
- Configurable limits per endpoint type:
|
|
- Login: 5 attempts per minute
|
|
- Register: 3 attempts per hour
|
|
- API endpoints: 100 requests per minute
|
|
- Rate limits by both IP and email/userId
|
|
- Returns proper 429 status with `Retry-After` header
|
|
- Automatic cleanup of expired entries
|
|
- `getClientIdentifier()` helper supports IP extraction from proxy headers
|
|
|
|
**Files Modified**:
|
|
|
|
- `apps/admin/src/app/api/auth/login/route.ts` - Added rate limiting
|
|
- `apps/admin/src/app/api/auth/register/route.ts` - Added rate limiting
|
|
|
|
**Implementation**:
|
|
|
|
```typescript
|
|
const ipRateLimitError = rateLimit(ipIdentifier, RATE_LIMITS.login);
|
|
if (ipRateLimitError) return ipRateLimitError;
|
|
|
|
const emailRateLimitError = rateLimit(`email:${email}`, RATE_LIMITS.login);
|
|
if (emailRateLimitError) return emailRateLimitError;
|
|
```
|
|
|
|
**Impact**: Protects against brute force attacks and spam
|
|
|
|
**Note for Production**: For multi-server deployments, replace in-memory store with Redis.
|
|
|
|
---
|
|
|
|
### 7. Replace Hardcoded ngrok URL with Environment Variable ✅
|
|
|
|
**File Modified**: `apps/mobile/src/config/api.ts`
|
|
|
|
**Changes**:
|
|
|
|
- ❌ Before: Hardcoded ngrok URL that expires
|
|
- ✅ After: Uses `EXPO_PUBLIC_API_URL` from environment
|
|
- Validates URL format on startup
|
|
- Throws helpful error if not set in production
|
|
- Falls back to localhost in development with warning
|
|
|
|
**Code**:
|
|
|
|
```typescript
|
|
// Before
|
|
export const API_BASE_URL = __DEV__
|
|
? "https://fd87cefe27ae.ngrok-free.app" // ❌ Hardcoded
|
|
: "https://your-production-url.com"; // ❌ Placeholder
|
|
|
|
// After
|
|
const getApiBaseUrl = (): string => {
|
|
const envUrl = process.env.EXPO_PUBLIC_API_URL;
|
|
if (envUrl) return envUrl;
|
|
|
|
if (__DEV__) {
|
|
console.warn("EXPO_PUBLIC_API_URL not set - using localhost");
|
|
return "http://localhost:3000";
|
|
}
|
|
|
|
throw new Error("EXPO_PUBLIC_API_URL must be set in production");
|
|
};
|
|
```
|
|
|
|
**Impact**: Production builds will work, and developers can use any development URL
|
|
|
|
---
|
|
|
|
### 8. Add Environment Validation on App Startup ✅
|
|
|
|
**Files Created**:
|
|
|
|
- `apps/admin/src/lib/env.ts` - Admin environment validation
|
|
- `apps/mobile/src/utils/env.ts` - Mobile environment validation
|
|
|
|
**Files Modified**:
|
|
|
|
- `apps/admin/src/middleware.ts` - Validates env on server startup
|
|
- `apps/mobile/src/app/_layout.tsx` - Validates env on app launch
|
|
|
|
**Features**:
|
|
|
|
- Validates all required environment variables exist
|
|
- Type-safe environment variable access via `getEnv()`
|
|
- Caches validation result for performance
|
|
- Throws descriptive errors with setup instructions
|
|
- Mobile app validates URL format
|
|
- Helper functions: `isDevelopment()`, `isProduction()`
|
|
|
|
**Admin Required Vars**:
|
|
|
|
- `NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY`
|
|
- `CLERK_SECRET_KEY`
|
|
- `DEEPSEEK_API_KEY`
|
|
|
|
**Mobile Required Vars**:
|
|
|
|
- `EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY`
|
|
- `EXPO_PUBLIC_API_URL`
|
|
|
|
**Error Example**:
|
|
|
|
```
|
|
❌ Environment validation failed:
|
|
Missing required environment variable: CLERK_SECRET_KEY
|
|
Missing required environment variable: DEEPSEEK_API_KEY
|
|
|
|
Please check your .env file and ensure all required variables are set.
|
|
See .env.example for the list of required variables.
|
|
```
|
|
|
|
**Impact**: Apps won't start with invalid configuration, preventing runtime errors
|
|
|
|
---
|
|
|
|
## 🧪 Verification
|
|
|
|
### Type Checking
|
|
|
|
```bash
|
|
✅ npm run typecheck:admin - PASSED
|
|
✅ npm run typecheck:mobile - PASSED
|
|
```
|
|
|
|
No TypeScript errors in either app.
|
|
|
|
---
|
|
|
|
## 📊 Security Improvements
|
|
|
|
| Area | Before | After | Status |
|
|
| --------------------- | --------------------------------- | --------------------------------------------- | ----------------- |
|
|
| **Secrets in Git** | ❌ Exposed in .env files | ✅ Gitignored, documented for rotation | 🟡 Needs rotation |
|
|
| **Authorization** | ❌ DELETE endpoint unprotected | ✅ Role-based auth + self-deletion prevention | ✅ Complete |
|
|
| **Rate Limiting** | ❌ No protection | ✅ IP + email rate limiting on auth endpoints | ✅ Complete |
|
|
| **Env Variables** | ❌ Hardcoded URLs, no validation | ✅ Validated on startup with helpful errors | ✅ Complete |
|
|
| **Production Config** | ❌ Placeholder URLs, missing vars | ✅ Fails fast with clear instructions | ✅ Complete |
|
|
|
|
---
|
|
|
|
## 🚨 Manual Actions Required
|
|
|
|
### CRITICAL - Key Rotation (Do This ASAP)
|
|
|
|
1. **Rotate Clerk Keys**:
|
|
|
|
```bash
|
|
# Go to https://dashboard.clerk.com
|
|
# Navigate to your project → API Keys
|
|
# Click "Regenerate" for secret key
|
|
# Update webhook secret in Webhooks section
|
|
# Update your local .env files (both apps)
|
|
```
|
|
|
|
2. **Rotate DeepSeek Key**:
|
|
|
|
```bash
|
|
# Go to https://platform.deepseek.com
|
|
# Navigate to API Keys
|
|
# Delete key: sk-6c2e965d97a349f08ae1c3386dabdf85
|
|
# Generate new key
|
|
# Update apps/admin/.env
|
|
```
|
|
|
|
3. **Clean Git History**:
|
|
|
|
```bash
|
|
# Follow instructions in SECURITY.md
|
|
# Use BFG Repo-Cleaner or git filter-branch
|
|
# Force push to remove secrets from history
|
|
# Coordinate with team before force pushing
|
|
```
|
|
|
|
4. **Update Production Environment Variables**:
|
|
- Update Vercel/Netlify/hosting platform with new keys
|
|
- Ensure EXPO_PUBLIC_API_URL is set for mobile production builds
|
|
|
|
---
|
|
|
|
## 📝 Files Changed Summary
|
|
|
|
### Created (7 files):
|
|
|
|
1. `SECURITY.md` - Key rotation documentation
|
|
2. `apps/admin/src/lib/auth-middleware.ts` - Authorization helpers
|
|
3. `apps/admin/src/lib/rate-limit.ts` - Rate limiting
|
|
4. `apps/admin/src/lib/env.ts` - Environment validation
|
|
5. `apps/mobile/src/utils/env.ts` - Environment validation
|
|
|
|
### Modified (7 files):
|
|
|
|
1. `apps/admin/.gitignore` - Added sensitive files
|
|
2. `apps/mobile/.gitignore` - Added sensitive files
|
|
3. `apps/admin/.env.example` - Complete documentation
|
|
4. `apps/mobile/.env.example` - Better documentation
|
|
5. `apps/admin/src/app/api/users/route.ts` - Fixed DELETE authorization
|
|
6. `apps/admin/src/app/api/auth/login/route.ts` - Added rate limiting
|
|
7. `apps/admin/src/app/api/auth/register/route.ts` - Added rate limiting
|
|
8. `apps/admin/src/middleware.ts` - Added env validation
|
|
9. `apps/mobile/src/config/api.ts` - Env-based API URL
|
|
10. `apps/mobile/src/app/_layout.tsx` - Added env validation
|
|
|
|
---
|
|
|
|
## 🎯 Next Steps
|
|
|
|
### Ready for Phase 2: Database Schema & Performance Foundation
|
|
|
|
With security fixed, you can now proceed to:
|
|
|
|
1. Add database indexes for performance
|
|
2. Fix type inconsistencies across packages
|
|
3. Implement proper migrations
|
|
4. Add database constraints
|
|
|
|
Would you like to proceed with Phase 2?
|
|
|
|
---
|
|
|
|
## 📚 Additional Documentation
|
|
|
|
For detailed implementation of each component, see:
|
|
|
|
- Authorization: `apps/admin/src/lib/auth-middleware.ts`
|
|
- Rate Limiting: `apps/admin/src/lib/rate-limit.ts`
|
|
- Environment Validation: `apps/admin/src/lib/env.ts`, `apps/mobile/src/utils/env.ts`
|
|
- Security Procedures: `SECURITY.md`
|
|
|
|
---
|
|
|
|
**Phase 1 Status**: ✅ COMPLETE
|
|
**Estimated Time**: 2-3 days (as planned)
|
|
**Actual Implementation**: ~1-2 hours (implementation time)
|
|
**Next Phase**: Ready to begin Phase 2
|