# Remaining Security Gaps Analysis

## ✅ Fully Resolved Issues

### 1. ✅ Rate Limiting / OTP Throttling
**Status:** **RESOLVED**
- Rate limiting implemented per phone and per IP
- OTP request throttling (3 per 10 min, 10 per 24h per phone)
- OTP verification attempt limits (5 per OTP, 10 failed per hour)
- **Location:** `src/middleware/rateLimitMiddleware.js`

### 2. ✅ OTP Exposure in Logs
**Status:** **RESOLVED**
- Safe OTP logging helper created (`src/utils/otpLogger.js`)
- Only logs in development mode
- Never logs to production
- **Location:** `src/services/smsService.js` uses `logOtpForDebug()`

### 3. ✅ IP/Device Risk Controls
**Status:** **RESOLVED**
- IP blocking for configured CIDR ranges
- Risk scoring based on IP/device/user-agent changes
- Suspicious refresh detection
- **Location:** `src/services/riskScoring.js`, integrated in `src/routes/authRoutes.js`

### 4. ✅ Refresh Token Theft Mitigation
**Status:** **RESOLVED**
- Environment fingerprinting (IP, user-agent, device ID)
- Suspicious refresh detection with risk scoring
- Optional OTP re-verification for suspicious refreshes
- Enhanced logging of suspicious events
- **Location:** `src/routes/authRoutes.js` (refresh endpoint)

### 5. ✅ JWT Claims Validation
**Status:** **RESOLVED**
- Strict validation of `iss`, `aud`, `exp`, `iat`, `nbf`
- Centralized validation function
- Used in all token verification paths
- **Location:** `src/services/jwtKeys.js` - `validateTokenClaims()`

### 6. ✅ CORS Hardening
**Status:** **RESOLVED**
- Strict origin whitelisting
- No wildcard support when credentials involved
- Production requirement for explicit origins
- **Location:** `src/index.js`

### 7. ✅ CSRF Protection (Documented)
**Status:** **RESOLVED** (Not needed - using Bearer tokens)
- Comprehensive documentation provided
- Guidance for future cookie-based implementation
- **Location:** `CSRF_NOTES.md`

---

## ⚠️ Partially Resolved Issues

### 8. ⚠️ Access Token Replay Mitigation
**Status:** **PARTIALLY RESOLVED**

**What's Done:**
- ✅ Step-up authentication middleware created (`src/middleware/stepUpAuth.js`)
- ✅ Access tokens include `high_assurance` claim after OTP verification
- ✅ Middleware checks for recent OTP or high assurance token

**What's Missing:**
- ❌ **Step-up auth NOT applied to sensitive routes**
- ❌ Sensitive operations in `src/routes/userRoutes.js` don't use `requireRecentOtpOrReauth`:
  - `PUT /users/me` - Profile updates (could change phone number)
  - `DELETE /users/me/devices/:device_id` - Device revocation
  - `POST /users/me/logout-all-other-devices` - Mass device logout

**Risk Level:** 🟡 **MEDIUM**
- Stolen access token can be used for sensitive operations within 15-minute window
- Attacker could revoke devices, update profile, etc.

**Recommendation:**
```javascript
// Apply to sensitive routes:
router.put('/me', auth, requireRecentOtpOrReauth, async (req, res) => { ... });
router.delete('/me/devices/:device_id', auth, requireRecentOtpOrReauth, async (req, res) => { ... });
router.post('/me/logout-all-other-devices', auth, requireRecentOtpOrReauth, async (req, res) => { ... });
```

---

### 9. ⚠️ Secrets Management & Rotation
**Status:** **PARTIALLY RESOLVED**

**What's Done:**
- ✅ JWT key rotation structure implemented
- ✅ Support for multiple keys with `kid` (key ID)
- ✅ Graceful rotation without breaking existing tokens
- ✅ Code structure ready for secrets manager integration

**What's Missing:**
- ❌ **Still reads secrets from `.env` file**
- ❌ No integration with secrets manager (AWS Secrets Manager, HashiCorp Vault, etc.)
- ❌ Manual key rotation process (no automation)
- ❌ Twilio credentials still in `.env`

**Risk Level:** 🟡 **MEDIUM-HIGH**
- If `.env` leaks or is committed to git, attacker can:
  - Forge JWTs
  - Send SMS via Twilio
  - Access database

**Recommendation:**
1. **Immediate:** Ensure `.env` is in `.gitignore` and never committed
2. **Short-term:** Use environment variables from secure deployment platform
3. **Long-term:** Integrate with secrets manager (see TODOs in `src/services/jwtKeys.js`)

**TODO in Code:**
- `src/services/jwtKeys.js` line 169: Example implementation for AWS Secrets Manager
- Need to implement `loadKeysFromSecretsManager()` function

---

### 10. ⚠️ Audit Logs Active Monitoring
**Status:** **PARTIALLY RESOLVED**

**What's Done:**
- ✅ Enhanced audit logging with risk levels (INFO, SUSPICIOUS, HIGH_RISK)
- ✅ Structured logging with metadata
- ✅ Anomaly detection helper function (`checkAnomalies()`)
- ✅ Suspicious events logged (failed OTPs, suspicious refreshes, blocked IPs)

**What's Missing:**
- ❌ **No active alerting/monitoring integration**
- ❌ No automated alerts for HIGH_RISK events
- ❌ No integration with PagerDuty, Slack, email, etc.
- ❌ Anomaly detection only logs to console (line 183 in `auditLogger.js`)

**Risk Level:** 🟡 **MEDIUM**
- Attacks happen silently until manual log inspection
- No real-time notification of security events

**Recommendation:**
1. **Immediate:** Set up log aggregation (CloudWatch, Datadog, etc.)
2. **Short-term:** Implement alerting for HIGH_RISK events
3. **Long-term:** Integrate with SIEM system

**TODO in Code:**
- `src/services/auditLogger.js` line 183: `// TODO: Trigger alert (email, webhook, etc.)`
- `src/services/auditLogger.js` line 193: Example implementation for external alerting

---

### 11. ⚠️ Input Validation
**Status:** **PARTIALLY RESOLVED**

**What's Done:**
- ✅ Input validation middleware created (`src/middleware/validation.js`)
- ✅ Validation applied to all auth routes:
  - `/auth/request-otp`
  - `/auth/verify-otp`
  - `/auth/refresh`
  - `/auth/logout`

**What's Missing:**
- ❌ **Validation NOT applied to user routes** (`src/routes/userRoutes.js`):
  - `PUT /users/me` - No validation for `name`, `user_type`
  - `DELETE /users/me/devices/:device_id` - No validation for `device_id` param
  - `POST /users/me/logout-all-other-devices` - No validation for `current_device_id`

**Risk Level:** 🟢 **LOW-MEDIUM**
- Less critical than auth endpoints, but still important
- Could allow unexpected payloads or edge-case bugs

**Recommendation:**
```javascript
// Add validation middleware to user routes:
const { validateUpdateProfileBody, validateDeviceIdParam } = require('../middleware/validation');

router.put('/me', auth, validateUpdateProfileBody, async (req, res) => { ... });
router.delete('/me/devices/:device_id', auth, validateDeviceIdParam, async (req, res) => { ... });
```

---

## ❌ Unaddressed Attack Scenarios

### 12. ❌ Database Compromise
**Status:** **NOT ADDRESSED**

**Risk:**
- If database is compromised, attacker can:
  - See phone numbers, device metadata, audit logs
  - Correlate patterns for phishing/SIM-swap attacks
  - Access user data

**Mitigation Needed:**
- ✅ Already using parameterized queries (good)
- ❌ No encryption at rest for sensitive fields
- ❌ No field-level encryption for PII
- ❌ No database access logging/auditing

**Recommendation:**
- Encrypt sensitive fields (phone numbers) at rest
- Implement database access logging
- Use database encryption (TDE, etc.)
- Regular security audits

---

### 13. ❌ Man-in-the-Middle (Non-HTTPS)
**Status:** **NOT ADDRESSED** (Infrastructure concern)

**Risk:**
- If client app talks to service over HTTP (no TLS):
  - Attacker can intercept access/refresh tokens
  - Attacker can intercept OTPs
  - Full account compromise

**Mitigation:**
- ✅ Server assumes TLS termination in front (reverse proxy)
- ❌ No enforcement of HTTPS-only connections
- ❌ No HSTS headers
- ❌ No certificate pinning guidance

**Recommendation:**
- Enforce HTTPS at reverse proxy/load balancer
- Add HSTS headers
- Document TLS requirements
- Consider certificate pinning for mobile apps

---

### 14. ❌ Misconfigured CORS + XSS
**Status:** **PARTIALLY ADDRESSED**

**What's Done:**
- ✅ CORS hardened with strict origin whitelisting
- ✅ Documentation warns about misconfiguration

**What's Missing:**
- ❌ No validation that CORS is properly configured in production
- ❌ No runtime checks for CORS misconfiguration
- ❌ No guidance for XSS prevention in frontend

**Risk Level:** 🟡 **MEDIUM**
- If frontend has XSS and CORS is misconfigured:
  - Attacker can use victim's tokens from malicious page
  - Account takeover possible

**Recommendation:**
- Add startup validation that CORS origins are configured in production
- Document XSS prevention best practices
- Consider Content Security Policy (CSP) headers

---

## Summary Table

| Issue | Status | Risk Level | Priority |
|-------|--------|------------|----------|
| 1. Rate Limiting | ✅ Resolved | - | - |
| 2. OTP Logging | ✅ Resolved | - | - |
| 3. IP/Device Risk | ✅ Resolved | - | - |
| 4. Refresh Token Theft | ✅ Resolved | - | - |
| 5. JWT Claims | ✅ Resolved | - | - |
| 6. CORS | ✅ Resolved | - | - |
| 7. CSRF | ✅ Documented | - | - |
| 8. Access Token Replay | ⚠️ Partial | 🟡 Medium | **HIGH** |
| 9. Secrets Management | ⚠️ Partial | 🟡 Medium-High | **HIGH** |
| 10. Audit Monitoring | ⚠️ Partial | 🟡 Medium | **MEDIUM** |
| 11. Input Validation | ⚠️ Partial | 🟢 Low-Medium | **MEDIUM** |
| 12. Database Compromise | ❌ Not Addressed | 🔴 High | **MEDIUM** |
| 13. MITM (HTTP) | ❌ Not Addressed | 🔴 High | **LOW** (Infra) |
| 14. CORS + XSS | ⚠️ Partial | 🟡 Medium | **LOW** |

---

## Immediate Action Items (Priority Order)

### 🔴 HIGH PRIORITY

1. **Apply Step-Up Auth to Sensitive Routes**
   - Add `requireRecentOtpOrReauth` to user routes
   - Prevents access token replay attacks

2. **Secrets Management**
   - Move secrets to environment variables (not `.env` file)
   - Plan integration with secrets manager
   - Ensure `.env` is never committed

### 🟡 MEDIUM PRIORITY

3. **Active Monitoring/Alerting**
   - Set up alerts for HIGH_RISK audit events
   - Integrate with monitoring system (CloudWatch, Datadog, etc.)

4. **Complete Input Validation**
   - Add validation middleware to user routes
   - Validate all request parameters

### 🟢 LOW PRIORITY

5. **Database Security**
   - Plan encryption at rest
   - Implement database access logging

6. **Infrastructure Security**
   - Document TLS/HTTPS requirements
   - Add HSTS headers
   - Validate CORS configuration at startup

---

## Code Locations for Fixes

### Step-Up Auth Application
**File:** `src/routes/userRoutes.js`
- Line 99: `PUT /users/me` - Add `requireRecentOtpOrReauth`
- Line 160: `DELETE /users/me/devices/:device_id` - Add `requireRecentOtpOrReauth`
- Line 203: `POST /users/me/logout-all-other-devices` - Add `requireRecentOtpOrReauth`

### Input Validation
**File:** `src/routes/userRoutes.js`
- Add validation middleware imports
- Apply to all routes

### Secrets Manager Integration
**File:** `src/services/jwtKeys.js`
- Line 169: Implement `loadKeysFromSecretsManager()` function
- Replace `.env` reads with secrets manager calls

### Alerting Integration
**File:** `src/services/auditLogger.js`
- Line 183: Implement alert triggering
- Line 193: Add external alerting integration

---

## Conclusion

**7 out of 14 issues are fully resolved** ✅
**4 issues are partially resolved** ⚠️ (need completion)
**3 issues are not addressed** ❌ (infrastructure/planning)

**Overall Security Posture:** 🟡 **GOOD** (with room for improvement)

The most critical remaining gaps are:
1. Step-up auth not applied to sensitive routes
2. Secrets still in `.env` file
3. No active monitoring/alerting

These should be addressed before production deployment.