guru-connect/SEC5_SESSION_TAKEOVER_AUDIT.md

# SEC-5: Session Takeover Prevention - Security Audit

**Status:** NEEDS IMPLEMENTATION
**Priority:** CRITICAL
**Date:** 2026-01-17

## Audit Findings

### Current Authentication Flow

**JWT Token Creation (auth/jwt.rs:60-88):**
```rust
pub fn create_token(
    &self,
    user_id: Uuid,
    username: &str,
    role: &str,
    permissions: Vec<String>,
) -> Result<String> {
    let now = Utc::now();
    let exp = now + Duration::hours(self.expiry_hours);  // Default: 24 hours

    let claims = Claims {
        sub: user_id.to_string(),
        username: username.to_string(),
        role: role.to_string(),
        permissions,
        exp: exp.timestamp(),
        iat: now.timestamp(),
    };

    encode(&Header::default(), &claims, &EncodingKey::from_secret(self.secret.as_bytes()))
}
```

**Token Validation (auth/jwt.rs:90-100):**
```rust
pub fn validate_token(&self, token: &str) -> Result<Claims> {
    let token_data = decode::<Claims>(
        token,
        &DecodingKey::from_secret(self.secret.as_bytes()),
        &Validation::default(),  // Only validates signature and expiration
    )?;

    Ok(token_data.claims)
}
```

### Vulnerabilities Identified

#### 1. NO TOKEN REVOCATION (CRITICAL)

**Problem:** Once a JWT is issued, it remains valid until expiration even if:
- User's password is changed
- User's account is disabled/deleted
- Token is suspected to be compromised
- User logs out

**Attack Scenario:**
1. Attacker steals JWT token (XSS, MITM, leaked credentials)
2. Admin changes user's password
3. Attacker's token still works for up to 24 hours
4. Admin has no way to invalidate the stolen token

**Impact:** CRITICAL - Stolen tokens cannot be revoked

#### 2. NO IP ADDRESS VALIDATION (HIGH)

**Problem:** JWT contains no IP binding. Token works from any IP address.

**Attack Scenario:**
1. User logs in from office (IP: 1.2.3.4)
2. Attacker steals token
3. Attacker uses token from different country (IP: 5.6.7.8)
4. No warning or detection

**Impact:** HIGH - Cannot detect token theft

#### 3. NO SESSION TRACKING (HIGH)

**Problem:** No database record of active JWT sessions

**Missing Capabilities:**
- Cannot list active user sessions
- Cannot see where user is logged in from
- Cannot revoke specific sessions
- No audit trail of session usage

**Impact:** HIGH - Limited visibility and control

#### 4. NO CONCURRENT SESSION LIMITS (MEDIUM)

**Problem:** Same token can be used from unlimited locations simultaneously

**Attack Scenario:**
1. User logs in from home
2. Token is intercepted
3. Attacker uses same token from 10 different IPs
4. System allows all connections

**Impact:** MEDIUM - Enables credential sharing and theft

#### 5. NO LOGOUT MECHANISM (MEDIUM)

**Problem:** No way to invalidate token on logout

**Current State:**
- Frontend likely just deletes token from localStorage
- Token remains valid server-side
- Attacker who cached token can still use it

**Impact:** MEDIUM - Logout doesn't actually log out

#### 6. LONG TOKEN LIFETIME (MEDIUM)

**Problem:** 24-hour token expiration is too long for security-critical operations

**Best Practice:**
- Access tokens: 15-30 minutes
- Refresh tokens: 7-30 days
- Critical operations: Re-authentication

**Current:** All tokens live 24 hours

**Impact:** MEDIUM - Extended window for token theft

## Recommended Fixes

### FIX 1: Token Revocation Blacklist (HIGH PRIORITY)

**Implementation:** In-memory token blacklist with Redis fallback for production

**Create:** `server/src/auth/token_blacklist.rs`
```rust
use std::collections::HashSet;
use std::sync::Arc;
use tokio::sync::RwLock;
use chrono::{DateTime, Utc};

/// Token blacklist for revocation
#[derive(Clone)]
pub struct TokenBlacklist {
    tokens: Arc<RwLock<HashSet<String>>>,
}

impl TokenBlacklist {
    pub fn new() -> Self {
        Self {
            tokens: Arc::new(RwLock::new(HashSet::new())),
        }
    }

    /// Add token to blacklist (revoke)
    pub async fn revoke(&self, token: &str) {
        let mut tokens = self.tokens.write().await;
        tokens.insert(token.to_string());
    }

    /// Check if token is revoked
    pub async fn is_revoked(&self, token: &str) -> bool {
        let tokens = self.tokens.read().await;
        tokens.contains(token)
    }

    /// Remove expired tokens (cleanup)
    pub async fn cleanup_expired(&self, jwt_config: &JwtConfig) {
        let mut tokens = self.tokens.write().await;
        tokens.retain(|token| {
            // Try to decode - if expired, remove from blacklist
            jwt_config.validate_token(token).is_ok()
        });
    }
}
```

**Modify:** `server/src/auth/jwt.rs` - Add revocation check
```rust
pub fn validate_token(&self, token: &str, blacklist: &TokenBlacklist) -> Result<Claims> {
    // Check blacklist first (fast path)
    if blacklist.is_revoked(token).await {
        return Err(anyhow!("Token has been revoked"));
    }

    let token_data = decode::<Claims>(
        token,
        &DecodingKey::from_secret(self.secret.as_bytes()),
        &Validation::default(),
    )?;

    Ok(token_data.claims)
}
```

### FIX 2: IP Address Validation (MEDIUM PRIORITY)

**Approach:** Validate but don't enforce (warn on IP change)

**Add to JWT Claims:**
```rust
#[derive(Debug, Serialize, Deserialize, Clone)]
pub struct Claims {
    pub sub: String,
    pub username: String,
    pub role: String,
    pub permissions: Vec<String>,
    pub exp: i64,
    pub iat: i64,
    pub ip: Option<String>,  // ← Add IP address
}
```

**Modify:** Token creation to include IP
```rust
pub fn create_token(
    &self,
    user_id: Uuid,
    username: &str,
    role: &str,
    permissions: Vec<String>,
    ip_address: Option<String>,  // ← Add parameter
) -> Result<String> {
    let now = Utc::now();
    let exp = now + Duration::hours(self.expiry_hours);

    let claims = Claims {
        sub: user_id.to_string(),
        username: username.to_string(),
        role: role.to_string(),
        permissions,
        exp: exp.timestamp(),
        iat: now.timestamp(),
        ip: ip_address,  // ← Include in token
    };

    encode(&Header::default(), &claims, &EncodingKey::from_secret(self.secret.as_bytes()))
}
```

**Modify:** Token validation to check IP
```rust
pub fn validate_token_with_ip(&self, token: &str, current_ip: &str, blacklist: &TokenBlacklist) -> Result<Claims> {
    // Check blacklist
    if blacklist.is_revoked(token).await {
        return Err(anyhow!("Token has been revoked"));
    }

    let claims = decode::<Claims>(
        token,
        &DecodingKey::from_secret(self.secret.as_bytes()),
        &Validation::default(),
    )?.claims;

    // Validate IP (warn if changed)
    if let Some(ref original_ip) = claims.ip {
        if original_ip != current_ip {
            tracing::warn!(
                "IP address mismatch for user {}: token IP={}, current IP={} - possible token theft",
                claims.username, original_ip, current_ip
            );
            // Log security event to database
            // In production: Consider requiring re-authentication or blocking
        }
    }

    Ok(claims)
}
```

### FIX 3: Session Tracking (MEDIUM PRIORITY)

**Create database table:**
```sql
CREATE TABLE active_sessions (
    id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
    user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE,
    token_hash VARCHAR(64) NOT NULL UNIQUE,  -- SHA-256 of JWT
    ip_address INET NOT NULL,
    user_agent TEXT,
    created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
    last_used_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
    expires_at TIMESTAMPTZ NOT NULL,
    INDEX idx_user_sessions (user_id, expires_at),
    INDEX idx_token_hash (token_hash)
);
```

**Benefits:**
- List user's active sessions
- Revoke individual sessions
- See login locations
- Audit trail

### FIX 4: Admin Revocation Endpoints (HIGH PRIORITY)

**Add API endpoints:**
```rust
// POST /api/auth/revoke - Revoke own token (logout)
pub async fn revoke_own_token(
    user: AuthenticatedUser,
    State(state): State<AppState>,
    Extension(token): Extension<String>,
) -> Result<StatusCode, StatusCode> {
    state.token_blacklist.revoke(&token).await;
    info!("User {} revoked their own token", user.username);
    Ok(StatusCode::NO_CONTENT)
}

// POST /api/auth/revoke-user/:user_id - Admin revokes all user tokens
pub async fn revoke_user_tokens(
    admin: AuthenticatedUser,
    Path(user_id): Path<Uuid>,
    State(state): State<AppState>,
) -> Result<StatusCode, StatusCode> {
    if !admin.is_admin() {
        return Err(StatusCode::FORBIDDEN);
    }

    // Revoke all tokens for user
    // Requires session tracking table to find user's tokens

    Ok(StatusCode::NO_CONTENT)
}
```

### FIX 5: Refresh Tokens (LOWER PRIORITY - Future Enhancement)

**Not implementing immediately** - requires significant changes to frontend

**Concept:**
- Access token: 15 minutes (short-lived)
- Refresh token: 7 days (long-lived, stored securely)
- Use refresh token to get new access token
- Refresh token can be revoked

## Implementation Priority

**Phase 1 (Day 1-2) - HIGH:**
1. Token blacklist (in-memory)
2. Revocation endpoint for logout
3. Admin revocation endpoint

**Phase 2 (Day 3) - MEDIUM:**
4. IP address validation (warning only)
5. Session tracking table
6. Security event logging

**Phase 3 (Future) - LOWER:**
7. Refresh token system
8. Concurrent session limits
9. Automatic IP-based revocation

## Testing Requirements

After implementation:
- [ ] Logout revokes token (subsequent requests fail with 401)
- [ ] Admin can revoke user's token
- [ ] Revoked token returns "Token has been revoked" error
- [ ] IP mismatch logs warning but allows access
- [ ] Expired tokens are cleaned from blacklist
- [ ] Blacklist survives server restart (if using Redis)

## Current Status

**Token Validation:** Basic (signature + expiration only)
**Revocation:** NOT IMPLEMENTED
**IP Binding:** NOT IMPLEMENTED
**Session Tracking:** NOT IMPLEMENTED
**Concurrent Limits:** NOT IMPLEMENTED

**Risk Level:** CRITICAL - Stolen tokens cannot be invalidated

---

**Audit Completed:** 2026-01-17
**Next Action:** Implement FIX 1 (Token Blacklist) and FIX 4 (Revocation Endpoints)