feat: implement CSRF protection for OAuth flows with nonce validation #20983
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add NonceService for cryptographically secure nonce generation and validation - Include nonce in JWT state for OAuth authorization requests - Store nonce in secure httpOnly cookie with SameSite=strict - Validate nonce matches between state and cookie in auth callback - Add origin/referer header validation for additional CSRF protection - Use timing-safe comparison to prevent timing attacks - Clear nonce cookie after successful validation or on error This prevents CSRF attacks where malicious sites could initiate OAuth flows on behalf of users by ensuring authorization requests originate from Gitpod. Co-authored-by: Ona <[email protected]>
…nflict Co-authored-by: Ona <[email protected]>
Co-authored-by: Ona <[email protected]>
…on complexity Replace complex Authenticator dependency injection test with simple unit test that focuses on the core logic without requiring all service dependencies. This makes the test more reliable and easier to maintain while still validating the critical api subdomain detection logic for the GitHub OAuth edge case. Co-authored-by: Ona <[email protected]>
Update test examples and documentation to use production-appropriate domain examples (gitpod.io) instead of specific preview environment domains for better clarity and maintainability. Co-authored-by: Ona <[email protected]>
Co-authored-by: Ona <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR implements CSRF protection for OAuth flows with nonce validation to prevent cross-site request forgery attacks. The implementation adds cryptographically secure nonce generation, validation mechanisms, and fragment protection for OAuth redirects.
- Introduces a new
NonceServicefor generating and validating CSRF nonces during OAuth flows - Adds fragment protection to prevent OAuth token inheritance attacks by ensuring redirect URLs have fragments
- Implements origin validation and handles GitHub OAuth API subdomain redirect edge cases
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
components/server/src/container-module.ts |
Registers the new NonceService as a singleton in the DI container |
components/server/src/auth/nonce-service.ts |
Core implementation of nonce generation, validation, and origin checking for CSRF protection |
components/server/src/auth/nonce-service.spec.ts |
Comprehensive test suite for NonceService functionality |
components/server/src/auth/login-completion-handler.ts |
Adds fragment protection to returnTo URLs using ensureUrlHasFragment utility |
components/server/src/auth/fragment-utils.ts |
Utility function to ensure URLs have fragments to prevent OAuth token inheritance |
components/server/src/auth/fragment-protection.spec.ts |
Tests for fragment protection utility functionality |
components/server/src/auth/authenticator.ts |
Integrates nonce validation into OAuth callback handling and generates nonces for auth flows |
components/server/src/auth/auth-provider.ts |
Extends AuthFlow interface to include optional nonce field |
components/server/src/auth/api-subdomain-redirect.spec.ts |
Tests for API subdomain redirect logic to handle GitHub OAuth edge cases |
| import * as crypto from "crypto"; | ||
| import { Config } from "../config"; | ||
|
|
||
| const NONCE_COOKIE_NAME = "__Host-_gitpod_oauth_nonce_"; |
There was a problem hiding this comment.
The cookie name uses the __Host- prefix but the cookie configuration may not meet all __Host- requirements. The __Host- prefix requires the cookie to be secure, have no domain attribute, and path must be '/'. Verify that the domain attribute is not set anywhere in the cookie configuration.
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
geropl
reviewed
Jul 29, 2025
geropl
reviewed
Jul 29, 2025
iQQBot
commented
Jul 29, 2025
Co-authored-by: Ona <[email protected]>
…urnTo Add two feature flags to control security features with safe defaults: **Feature Flag 1: enable_nonce_validation (default: false)** - Controls CSRF nonce validation in OAuth flows - When disabled: Nonce is generated but not validated (future compatibility) - When enabled: Full CSRF protection with nonce and origin validation - Nonce cookies are always generated and cleared for consistency **Feature Flag 2: enable_strict_authorize_return_to (default: false)** - Controls returnTo validation strictness for /api/authorize endpoint - When disabled: Falls back to login validation (broader patterns) - When enabled: Uses strict authorize validation (limited to specific paths) - /api/login always uses login validation regardless of flag **Implementation Details:** - Always generate nonce for consistency and future compatibility - Only validate nonce when feature flag is enabled - Always clear nonce cookies regardless of validation state - Authorize endpoint checks flag and falls back gracefully - Comprehensive logging for debugging and monitoring **Backward Compatibility:** - Default false ensures no breaking changes - Gradual rollout possible via feature flag configuration - Existing authentication flows continue to work - Safe fallback behavior when flags are disabled Co-authored-by: Ona <[email protected]>
Update NonceService.validateOrigin to check request origin against the expected SCM provider domain instead of Gitpod's own domain. This fixes the CSRF protection logic for OAuth callbacks which legitimately come from external providers (github.com, gitlab.com, etc.). 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
Hey @geropl could you review again? |
geropl
reviewed
Jul 30, 2025
geropl
reviewed
Jul 30, 2025
geropl
reviewed
Jul 30, 2025
geropl
reviewed
Jul 30, 2025
geropl
reviewed
Jul 30, 2025
|
@geropl Cloud you review again? |
|
@iQQBot Yes, give me 30mins. 👍 |
geropl
reviewed
Jul 31, 2025
| return; | ||
| } | ||
|
|
||
| // Update session info | ||
| let returnTo = returnToUrl || this.config.hostUrl.asDashboard().toString(); | ||
| const returnToParam = returnToUrl || this.config.hostUrl.asDashboard().toString(); | ||
| let returnTo = returnToParam; |
There was a problem hiding this comment.
How to validate the returnTo param here?
There was a problem hiding this comment.
This returnTo is from the state and signed by JWT. We do verify in the generate side.
|
@iQQBot I pushed a commit with minor changes to discuss with you |
|
@iQQBot I will one more manual test in ~1h and then we can approve and merge. |
geropl
approved these changes
Aug 1, 2025
|
/unhold |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
feat: implement CSRF protection for OAuth flows with nonce validation
Feature Flags:
enable_strict_authorize_return_toenable_nonce_validationRelated Issue(s)
Fixes CLC-1592
Fixes CLC-1643
Fixes CLC-1642
How to test
Documentation
Preview status
Gitpod was successfully deployed to your preview environment.
Build Options
Build
Run the build with werft instead of GHA
Run Leeway with
--dont-testPublish
Installer
Add desired feature flags to the end of the line above, space separated
Preview Environment / Integration Tests
If enabled this will build
install/previewIf enabled this will create the environment on GCE infra
Saves cost. Untick this only if you're really sure you need a non-preemtible machine.
Valid options are
all,workspace,webapp,ide,jetbrains,vscode,ssh. If enabled,with-previewandwith-large-vmwill be enabled./hold