[server] Enforce state presence and validation on the /api/authorize endoint

Co-authored-by: Ona <[email protected]>

[server] Enforce state presence and validation on the /api/authorize endoint

Co-authored-by: Ona <[email protected]>

Author: geropl

CLC-1592-OAuth-Token-Leak-Fix.md

@@ -0,0 +1,387 @@
+# Design Document: Fix for CLC-1592 - Bitbucket OAuth Access Token Leak
+
+## Overview
+
+This document outlines the solution for CLC-1592, a critical security vulnerability that allows attackers to steal OAuth access tokens through a redirect URI validation bypass in Gitpod's authorization flow.
+
+## Problem Statement
+
+### Vulnerability Description
+The `/api/authorize` endpoint accepts direct requests without validating that they originated from legitimate OAuth callbacks. This allows malicious OAuth applications to redirect users to the endpoint with crafted parameters, enabling access token theft.
+
+### Attack Vector
+1. Attacker creates malicious OAuth application with Gitpod as redirect URI
+2. Malicious app redirects to: `https://api.gitpod.io/api/authorize?returnTo=https://attacker.com&host=malicious-provider`
+3. `/api/authorize` processes request without validation
+4. User gets redirected to attacker-controlled domain
+5. Attacker captures OAuth tokens and completes authentication on their system
+
+### Impact
+- **OAuth token theft**: Full access to victim's GitHub/GitLab/Bitbucket accounts
+- **Account takeover**: Complete access to repositories and data
+- **Session hijacking**: Attackers can authenticate as the victim
+
+## Current Architecture
+
+### Legitimate OAuth Flow
+```mermaid
+sequenceDiagram
+    participant U as User
+    participant D as Dashboard
+    participant GP as Git Provider
+    participant CB as /auth/callback
+    participant AZ as /api/authorize
+
+    U->>D: Click authorize
+    D->>GP: Redirect with client_id
+    GP->>CB: Redirect with code + state JWT
+    CB->>CB: Validate JWT state
+    CB->>AZ: Redirect with validated state
+    AZ->>AZ: Process authorization
+```
+
+### Attack Flow
+```mermaid
+sequenceDiagram
+    participant A as Attacker
+    participant MA as Malicious App
+    participant AZ as /api/authorize
+    participant V as Victim
+
+    A->>MA: Create malicious OAuth app
+    MA->>AZ: Direct redirect (bypass callback)
+    AZ->>AZ: Process without validation ❌
+    AZ->>A: Redirect to attacker domain
+    A->>A: Capture tokens ❌
+```
+
+## Solution Design
+
+### Core Principle
+Require that all `/api/authorize` requests include a valid JWT state parameter that proves the request originated from a legitimate OAuth callback.
+
+### Implementation Strategy
+Leverage the existing JWT state validation system already used in OAuth callbacks, rather than creating new validation infrastructure. The implementation will be guarded by a feature flag to enable safe rollout and quick rollback if needed.
+
+### Key Components
+
+#### 1. Feature Flag Guard
+```typescript
+// Check feature flag before applying validation
+const enforceStateValidation = this.config.featureFlags?.enforceOAuthStateValidation ?? false;
+
+if (!enforceStateValidation) {
+    // Continue with existing logic for backward compatibility
+    log.info(`OAuth state validation disabled by feature flag`, { "authorize-flow": true });
+    // ... existing authorization logic
+    return;
+}
+```
+
+#### 2. State Parameter Validation
+```typescript
+// In authenticator.ts authorize() method
+const stateParam = req.query.state?.toString();
+
+if (!stateParam) {
+    log.warn(`Missing state parameter in authorize request`, { host, returnTo });
+    res.redirect(this.getSorryUrl(`Invalid authorization request.`));
+    return;
+}
+```
+
+#### 3. JWT State Verification
+```typescript
+try {
+    // Reuse existing JWT state validation logic
+    const flowState = await this.parseState(stateParam);
+
+    // Validate that the state authorizes this specific request
+    if (flowState.host !== host || flowState.returnTo !== returnTo) {
+        log.warn(`State parameter mismatch`, {
+            stateHost: flowState.host,
+            requestHost: host
+        });
+        res.redirect(this.getSorryUrl(`Invalid authorization request.`));
+        return;
+    }
+} catch (error) {
+    log.warn(`Invalid state parameter`, error);
+    res.redirect(this.getSorryUrl(`Invalid authorization request.`));
+    return;
+}
+```
+
+#### 4. Parameter Matching
+Ensure the JWT state explicitly authorizes the requested host and returnTo parameters to prevent parameter tampering.
+
+## Implementation Details
+
+### File Changes
+**Primary Change**: `components/server/src/auth/authenticator.ts`
+- Modify `authorize()` method to conditionally require and validate JWT state parameter
+- Add feature flag check (`enforceOAuthStateValidation`) before validation logic
+- Add state validation before existing authorization logic (when feature flag enabled)
+- Reuse existing `parseState()` method for JWT validation
+- Maintain backward compatibility when feature flag is disabled
+
+**Feature Flag Configuration**:
+- Feature flag name: `enforceOAuthStateValidation`
+- Default value: `false` (disabled for safe rollout)
+- Type: `boolean`
+- Location: Server configuration/feature flags system
+
+### Code Implementation
+```typescript
+async authorize(req: express.Request, res: express.Response, next: express.NextFunction) {
+    const user = req.user;
+    if (!req.isAuthenticated() || !User.is(user)) {
+        log.info(`User is not authenticated.`, { "authorize-flow": true });
+        res.redirect(this.getSorryUrl(`Not authenticated. Please login.`));
+        return;
+    }
+
+    const returnTo: string | undefined = req.query.returnTo?.toString();
+    const host: string | undefined = req.query.host?.toString();
+    const scopes: string = req.query.scopes?.toString() || "";
+    const override = req.query.override === "true";
+    const stateParam = req.query.state?.toString();
+
+    // NEW: Feature flag guard for OAuth state validation
+    const enforceStateValidation = this.config.featureFlags?.enforceOAuthStateValidation ?? false;
+
+    if (enforceStateValidation) {
+        // NEW: Validate JWT state parameter when feature flag is enabled
+        if (!stateParam) {
+            log.warn(`Missing state parameter in authorize request`, {
+                host, returnTo, "authorize-flow": true
+            });
+            res.redirect(this.getSorryUrl(`Invalid authorization request.`));
+            return;
+        }
+
+        try {
+            const flowState = await this.parseState(stateParam);
+
+            if (flowState.host !== host || flowState.returnTo !== returnTo) {
+                log.warn(`State parameter mismatch in authorize request`, {
+                    stateHost: flowState.host,
+                    requestHost: host,
+                    stateReturnTo: flowState.returnTo,
+                    requestReturnTo: returnTo,
+                    "authorize-flow": true
+                });
+                res.redirect(this.getSorryUrl(`Invalid authorization request.`));
+                return;
+            }
+
+            log.info(`Valid state parameter in authorize request`, { host, "authorize-flow": true });
+
+        } catch (error) {
+            log.warn(`Invalid state parameter in authorize request`, error, {
+                host, returnTo, "authorize-flow": true
+            });
+            res.redirect(this.getSorryUrl(`Invalid authorization request.`));
+            return;
+        }
+    } else {
+        log.info(`OAuth state validation disabled by feature flag`, {
+            host, returnTo, "authorize-flow": true
+        });
+    }
+
+    // Continue with existing authorization logic...
+    const authProvider = host && (await this.getAuthProviderForHost(host));
+    if (!returnTo || !host || !authProvider) {
+        log.info(`Bad request: missing parameters.`, { "authorize-flow": true });
+        res.redirect(this.getSorryUrl(`Bad request: missing parameters.`));
+        return;
+    }
+
+    // ... rest of existing validation logic ...
+
+    // Create new state for OAuth provider
+    const newState = await this.signInJWT.sign({ host, returnTo, overrideScopes: override });
+    authProvider.authorize(req, res, next, this.deriveAuthState(newState), wantedScopes);
+}
+```
+
+## Security Analysis
+
+### Attack Prevention
+| Attack Vector | Mitigation | Result |
+|---------------|------------|---------|
+| **Direct URL manipulation** | Requires valid JWT state | ❌ Blocked |
+| **Malicious OAuth redirects** | State validation fails | ❌ Blocked |
+| **Parameter tampering** | State must match parameters | ❌ Blocked |
+| **Replay attacks** | JWT expiration (5 minutes) | ❌ Blocked |
+
+### Legitimate Flow Preservation
+| Scenario | Impact | Result |
+|----------|--------|---------|
+| **Dashboard authorization** | OAuth callback provides state | ✅ Works |
+| **Existing integrations** | No changes required | ✅ Works |
+| **User workflows** | Transparent to users | ✅ Works |
+
+## Testing Strategy
+
+### Security Testing
+1. **Feature Flag Enabled Testing**:
+   - Attempt the exact attack described in CLC-1592 (should be blocked)
+   - Test with malformed/missing state parameters (should be blocked)
+   - Test with invalid/expired JWT tokens (should be blocked)
+   - Test various malicious parameter combinations (should be blocked)
+
+2. **Feature Flag Disabled Testing**:
+   - Verify attack scenarios work as before (backward compatibility)
+   - Ensure no validation is performed when flag is disabled
+
+### Regression Testing
+1. **Feature Flag Enabled**:
+   - Verify all legitimate authorization workflows continue working
+   - Test GitHub, GitLab, Bitbucket integrations with valid state
+   - Ensure no impact on legitimate user flows
+
+2. **Feature Flag Disabled**:
+   - Verify all authorization workflows work exactly as before
+   - Test all OAuth providers without state parameter requirements
+   - Ensure complete backward compatibility
+
+3. **Feature Flag Toggle Testing**:
+   - Test enabling/disabling flag during runtime
+   - Verify immediate effect of flag changes
+   - Test rollback scenarios
+
+### Test Cases
+```typescript
+describe('CLC-1592 Fix', () => {
+    describe('with feature flag enabled', () => {
+        beforeEach(() => {
+            // Enable feature flag for these tests
+            config.featureFlags.enforceOAuthStateValidation = true;
+        });
+
+        it('should block requests without state parameter', async () => {
+            const response = await request('/api/authorize?host=github.com&returnTo=...');
+            expect(response.status).toBe(302);
+            expect(response.headers.location).toContain('sorry');
+        });
+
+        it('should block requests with invalid state', async () => {
+            const response = await request('/api/authorize?state=invalid&host=github.com');
+            expect(response.status).toBe(302);
+            expect(response.headers.location).toContain('sorry');
+        });
+
+        it('should allow requests with valid state', async () => {
+            const validState = await signInJWT.sign({ host: 'github.com', returnTo: '...' });
+            const response = await request(`/api/authorize?state=${validState}&host=github.com`);
+            expect(response.status).toBe(302);
+            expect(response.headers.location).toContain('github.com');
+        });
+    });
+
+    describe('with feature flag disabled', () => {
+        beforeEach(() => {
+            // Disable feature flag for these tests
+            config.featureFlags.enforceOAuthStateValidation = false;
+        });
+
+        it('should allow requests without state parameter (backward compatibility)', async () => {
+            const response = await request('/api/authorize?host=github.com&returnTo=...');
+            expect(response.status).toBe(302);
+            expect(response.headers.location).toContain('github.com');
+        });
+    });
+});
+```
+
+## Deployment Considerations
+
+### Feature Flag Strategy
+1. **Initial Deployment**: Deploy with feature flag disabled (default: false)
+2. **Gradual Rollout**: Enable feature flag for testing/staging environments
+3. **Production Enablement**: Enable feature flag in production after validation
+4. **Quick Rollback**: Disable feature flag immediately if issues arise
+
+### Monitoring
+- Track authorization request failures when feature flag is enabled
+- Monitor for unusual patterns in state validation errors
+- Alert on significant increases in blocked requests
+- Log feature flag status for debugging
+
+### Rollback Plan
+- **Immediate**: Disable feature flag (no code deployment needed)
+- **Full Rollback**: Code revert if feature flag system fails
+- No database changes required
+- No frontend changes to rollback
+
+## Benefits
+
+### Security Improvements
+- **Eliminates OAuth redirect attacks**: Direct access to `/api/authorize` blocked (when feature flag enabled)
+- **Prevents parameter tampering**: State validation ensures request integrity
+- **Maintains defense in depth**: Leverages existing security infrastructure
+- **Zero new attack surface**: Reuses proven JWT validation system
+- **Safe rollout**: Feature flag enables immediate rollback without code deployment
+
+### Implementation Advantages
+- **Feature flag protected**: Safe rollout with immediate rollback capability
+- **Minimal code changes**: Single method modification
+- **No frontend changes**: Existing dashboard flows unchanged
+- **Reuses existing infrastructure**: JWT state system already battle-tested
+- **Backward compatible**: No breaking changes to legitimate flows when flag is disabled
+- **Simple to understand**: Clear validation logic with feature flag guard
+
+## Implementation Steps
+
+### Step 1: Code Analysis and Understanding
+**Deliverable**: Complete understanding of current implementation
+- [ ] Read and analyze `components/server/src/auth/authenticator.ts`
+- [ ] Document current `authorize()` method flow
+- [ ] Identify existing `parseState()` method implementation
+- [ ] Map OAuth flow architecture and state handling
+- [ ] Understand feature flag system integration
+
+### Step 2: Security Validation Implementation
+**Deliverable**: Modified authenticator with feature-flagged state validation
+- [ ] Add feature flag check (`enforceOAuthStateValidation`)
+- [ ] Add state parameter extraction and null check (when flag enabled)
+- [ ] Implement JWT state verification using existing `parseState()`
+- [ ] Add parameter matching validation (host vs state.host, returnTo vs state.returnTo)
+- [ ] Add comprehensive error handling with security logging
+- [ ] Ensure proper redirect to sorry page for invalid requests
+- [ ] Maintain backward compatibility when feature flag is disabled
+
+### Step 3: Testing Implementation
+**Deliverable**: Comprehensive test suite covering security and regression scenarios
+- [ ] Create unit tests for state validation logic
+- [ ] Implement attack scenario tests (CLC-1592 reproduction) with feature flag enabled
+- [ ] Add regression tests for legitimate OAuth flows
+- [ ] Test feature flag disabled scenario (backward compatibility)
+- [ ] Test edge cases (malformed JWT, expired tokens, parameter mismatches)
+- [ ] Validate all OAuth providers (GitHub, GitLab, Bitbucket)
+
+### Step 4: Security Validation
+**Deliverable**: Confirmed attack prevention and flow preservation
+- [ ] Reproduce exact CLC-1592 attack scenario with feature flag enabled
+- [ ] Verify attack is blocked by new validation
+- [ ] Confirm legitimate dashboard OAuth flows work unchanged
+- [ ] Test various malicious parameter combinations
+- [ ] Validate JWT expiration handling (5-minute timeout)
+- [ ] Verify backward compatibility with feature flag disabled
+
+## Progress Tracking
+
+| Step | Status | Notes |
+|------|--------|-------|
+| Step 1: Code Analysis | ⏳ Pending | |
+| Step 2: Implementation | ⏳ Pending | |
+| Step 3: Testing | ⏳ Pending | |
+| Step 4: Security Validation | ⏳ Pending | |
+
+## Conclusion
+
+This solution provides robust protection against CLC-1592 by requiring JWT state validation for all `/api/authorize` requests. It leverages existing, proven security infrastructure while maintaining full compatibility with legitimate authorization flows.
+
+The fix is minimal, focused, and directly addresses the root cause of the vulnerability without introducing new complexity or potential security issues. The feature flag implementation ensures safe rollout and immediate rollback capability.