feat: implement CSRF protection for OAuth flows with nonce validation

components/server/src/auth/api-subdomain-redirect.spec.ts

@@ -0,0 +1,82 @@
+/**
+ * Copyright (c) 2024 Gitpod GmbH. All rights reserved.
+ * Licensed under the GNU Affero General Public License (AGPL).
+ * See License.AGPL.txt in the project root for license information.
+ */
+
+import { expect } from "chai";
+
+describe("API Subdomain Redirect Logic", () => {
+    // Test the core logic without complex dependency injection
+    function isApiSubdomainOfConfiguredHost(hostname: string, configuredHost: string): boolean {
+        return hostname === `api.${configuredHost}`;
+    }
+
+    describe("isApiSubdomainOfConfiguredHost", () => {
+        it("should detect api subdomain of configured host", () => {
+            const configuredHost = "gitpod.io";
+            const testCases = [
+                { hostname: "api.gitpod.io", expected: true },
+                { hostname: "api.preview.gitpod-dev.com", expected: false }, // Different configured host
+                { hostname: "gitpod.io", expected: false }, // Main domain
+                { hostname: "workspace-123.gitpod.io", expected: false }, // Other subdomain
+                { hostname: "api.evil.com", expected: false }, // Different domain
+            ];
+
+            testCases.forEach(({ hostname, expected }) => {
+                const result = isApiSubdomainOfConfiguredHost(hostname, configuredHost);
+                expect(result).to.equal(expected, `Failed for hostname: ${hostname}`);
+            });
+        });
+
+        it("should handle GitHub OAuth edge case correctly", () => {
+            // This is the specific case mentioned in the login completion handler
+            const configuredHost = "gitpod.io";
+            const apiSubdomain = `api.${configuredHost}`;
+
+            const result = isApiSubdomainOfConfiguredHost(apiSubdomain, configuredHost);
+            expect(result).to.be.true;
+        });
+
+        it("should handle preview environment correctly", () => {
+            const configuredHost = "preview.gitpod-dev.com";
+            const apiSubdomain = `api.${configuredHost}`;
+
+            const result = isApiSubdomainOfConfiguredHost(apiSubdomain, configuredHost);
+            expect(result).to.be.true;
+        });
+    });
+
+    describe("OAuth callback flow scenarios", () => {
+        it("should identify redirect scenarios correctly", () => {
+            const scenarios = [
+                {
+                    name: "GitHub OAuth Callback on API Subdomain",
+                    hostname: "api.gitpod.io",
+                    configuredHost: "gitpod.io",
+                    shouldRedirect: true,
+                },
+                {
+                    name: "Regular Login on Main Domain",
+                    hostname: "gitpod.io",
+                    configuredHost: "gitpod.io",
+                    shouldRedirect: false,
+                },
+                {
+                    name: "Workspace Port (Should Not Redirect)",
+                    hostname: "3000-gitpod.io",
+                    configuredHost: "gitpod.io",
+                    shouldRedirect: false,
+                },
+            ];
+
+            scenarios.forEach((scenario) => {
+                const result = isApiSubdomainOfConfiguredHost(scenario.hostname, scenario.configuredHost);
+                expect(result).to.equal(
+                    scenario.shouldRedirect,
+                    `${scenario.name}: Expected ${scenario.shouldRedirect} for ${scenario.hostname}`,
+                );
+            });
+        });
+    });
+});

components/server/src/auth/auth-provider.ts

@@ -97,6 +97,7 @@ export interface AuthFlow {
     readonly host: string;
     readonly returnTo: string;
     readonly overrideScopes?: boolean;
+    readonly nonce?: string;
 }
 export namespace AuthFlow {
     export function is(obj: any): obj is AuthFlow {

components/server/src/auth/authenticator.ts

@@ -7,6 +7,7 @@
 import { BUILTIN_INSTLLATION_ADMIN_USER_ID, TeamDB } from "@gitpod/gitpod-db/lib";
 import { User } from "@gitpod/gitpod-protocol";
 import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
+import { GitpodHostUrl } from "@gitpod/gitpod-protocol/lib/util/gitpod-host-url";
 import express from "express";
 import { inject, injectable, postConstruct } from "inversify";
 import passport from "passport";
@@ -18,6 +19,85 @@ import { UserService } from "../user/user-service";
 import { AuthFlow, AuthProvider } from "./auth-provider";
 import { HostContextProvider } from "./host-context-provider";
 import { SignInJWT } from "./jwt";
+import { NonceService } from "./nonce-service";
+import { ensureUrlHasFragment } from "./fragment-utils";
+import { getFeatureFlagEnableNonceValidation, getFeatureFlagEnableStrictAuthorizeReturnTo } from "../util/featureflags";
+
+/**
+ * Common validation logic for returnTo URLs.
+ * @param returnTo The URL to validate
+ * @param hostUrl The host URL configuration
+ * @param allowedPatterns Array of regex patterns that are allowed for the pathname
+ * @returns true if the URL is valid, false otherwise
+ */
+function validateReturnToUrlWithPatterns(returnTo: string, hostUrl: GitpodHostUrl, allowedPatterns: RegExp[]): boolean {
+    try {
+        const url = new URL(returnTo);
+        const baseUrl = hostUrl.url;
+
+        // Must be same origin OR www.gitpod.io exception
+        const isSameOrigin = url.origin === baseUrl.origin;
+        const isGitpodWebsite = url.protocol === "https:" && url.hostname === "www.gitpod.io";
+
+        if (!isSameOrigin && !isGitpodWebsite) {
+            return false;
+        }
+
+        // For www.gitpod.io, only allow root path
+        if (isGitpodWebsite) {
+            return url.pathname === "/";
+        }
+
+        // Check if pathname matches any allowed pattern
+        const isAllowedPath = allowedPatterns.some((pattern) => pattern.test(url.pathname));
+        if (!isAllowedPath) {
+            return false;
+        }
+
+        // For complete-auth, require ONLY message parameter (used by OAuth flows)
+        if (url.pathname === "/complete-auth") {
+            const searchParams = new URLSearchParams(url.search);
+            const paramKeys = Array.from(searchParams.keys());
+            return paramKeys.length === 1 && paramKeys[0] === "message" && searchParams.has("message");
+        }
+
+        return true;
+    } catch (error) {
+        // Invalid URL
+        return false;
+    }
+}
+
+/**
+ * Validates returnTo URLs for login API endpoints.
+ * Login API allows broader navigation after authentication.
+ */
+export function validateLoginReturnToUrl(returnTo: string, hostUrl: GitpodHostUrl): boolean {
+    const allowedPatterns = [
+        // We have already verified the domain above, and we do not restrict the redirect location for loginReturnToUrl.
+        /^\/.*$/,
+    ];
+
+    return validateReturnToUrlWithPatterns(returnTo, hostUrl, allowedPatterns);
+}
+
+/**
+ * Validates returnTo URLs for authorize API endpoints.
+ * Authorize API allows complete-auth callbacks and dashboard pages for scope elevation.
+ */
+export function validateAuthorizeReturnToUrl(returnTo: string, hostUrl: GitpodHostUrl): boolean {
+    const allowedPatterns = [
+        // 1. complete-auth callback for OAuth popup windows
+        /^\/complete-auth$/,
+
+        // 2. Dashboard pages (for scope elevation flows)
+        /^\/$/, // Root
+        /^\/new\/?$/, // Create workspace page
+        /^\/quickstart\/?$/, // Quickstart page
+    ];
+
+    return validateReturnToUrlWithPatterns(returnTo, hostUrl, allowedPatterns);
+}
 
 @injectable()
 export class Authenticator {
@@ -30,6 +110,7 @@ export class Authenticator {
     @inject(TokenProvider) protected readonly tokenProvider: TokenProvider;
     @inject(UserAuthentication) protected readonly userAuthentication: UserAuthentication;
     @inject(SignInJWT) protected readonly signInJWT: SignInJWT;
+    @inject(NonceService) protected readonly nonceService: NonceService;
 
     @postConstruct()
     protected setup() {
@@ -77,6 +158,42 @@ export class Authenticator {
                 if (!host) {
                     throw new Error("Auth flow state is missing 'host' attribute.");
                 }
+
+                // Handle GitHub OAuth edge case: redirect from api.* subdomain to base domain
+                // This allows nonce validation to work since cookies are accessible on base domain
+                if (this.isApiSubdomainOfConfiguredHost(req.hostname)) {
+                    log.info(`OAuth callback on api subdomain, redirecting to base domain for nonce validation`, {
+                        hostname: req.hostname,
+                        configuredHost: this.config.hostUrl.url.hostname,
+                    });
+                    const baseUrl = this.config.hostUrl.with({
+                        pathname: req.path,
+                        search: new URL(req.url, this.config.hostUrl.url).search,
+                    });
+                    res.redirect(baseUrl.toString());
+                    return;
+                }
+
+                // Validate nonce for CSRF protection (if feature flag is enabled)
+                const isNonceValidationEnabled = await getFeatureFlagEnableNonceValidation();
+                if (isNonceValidationEnabled) {
+                    const stateNonce = flowState.nonce;
+                    const cookieNonce = this.nonceService.getNonceFromCookie(req);
+
+                    if (!this.nonceService.validateNonce(stateNonce, cookieNonce)) {
+                        log.error(`CSRF protection: Nonce validation failed`, {
+                            url: req.url,
+                            hasStateNonce: !!stateNonce,
+                            hasCookieNonce: !!cookieNonce,
+                        });
+                        res.status(403).send("Authentication failed");
+                        return;
+                    }
+                }
+
+                // Always clear the nonce cookie
+                this.nonceService.clearNonceCookie(res);
+
                 const hostContext = this.hostContextProvider.get(host);
                 if (!hostContext) {
                     throw new Error("No host context found.");
@@ -89,6 +206,8 @@ export class Authenticator {
                 await hostContext.authProvider.callback(req, res, next);
             } catch (error) {
                 log.error(`Failed to handle callback.`, error, { url: req.url });
+                // Always clear nonce cookie on error
+                this.nonceService.clearNonceCookie(res);
             }
         } else {
             // Otherwise proceed with other handlers
@@ -121,6 +240,15 @@ export class Authenticator {
         return state;
     }
 
+    /**
+     * Checks if the current hostname is api.{configured-domain}.
+     * This handles the GitHub OAuth edge case where callbacks may come to api.* subdomain.
+     */
+    private isApiSubdomainOfConfiguredHost(hostname: string): boolean {
+        const configuredHost = this.config.hostUrl.url.hostname;
+        return hostname === `api.${configuredHost}`;
+    }
+
     protected async getAuthProviderForHost(host: string): Promise<AuthProvider | undefined> {
         const hostContext = this.hostContextProvider.get(host);
         return hostContext && hostContext.authProvider;
@@ -131,13 +259,23 @@ export class Authenticator {
             log.info(`User is already authenticated. Continue.`, { "login-flow": true });
             return next();
         }
-        let returnTo: string | undefined = req.query.returnTo?.toString();
-        if (returnTo) {
-            log.info(`Stored returnTo URL: ${returnTo}`, { "login-flow": true });
+        let returnToParam: string | undefined = req.query.returnTo?.toString();
+        if (returnToParam) {
+            log.info(`Stored returnTo URL: ${returnToParam}`, { "login-flow": true });
+
+            // Validate returnTo URL against allowlist for login API
+            if (!validateLoginReturnToUrl(returnToParam, this.config.hostUrl)) {
+                log.warn(`Invalid returnTo URL rejected for login: ${returnToParam}`, { "login-flow": true });
+                res.redirect(this.getSorryUrl(`Invalid return URL.`));
+                return;
+            }
         }
         // returnTo defaults to workspaces url
         const workspaceUrl = this.config.hostUrl.asDashboard().toString();
-        returnTo = returnTo || workspaceUrl;
+        returnToParam = returnToParam || workspaceUrl;
+        // Ensure returnTo URL has a fragment to prevent OAuth token inheritance attacks
+        const returnTo = ensureUrlHasFragment(returnToParam);
+
         const host: string = req.query.host?.toString() || "";
         const authProvider = host && (await this.getAuthProviderForHost(host));
         if (!host || !authProvider) {
@@ -170,9 +308,14 @@ export class Authenticator {
             return;
         }
 
+        // Always generate nonce for CSRF protection (validation controlled by feature flag)
+        const nonce = this.nonceService.generateNonce();
+        this.nonceService.setNonceCookie(res, nonce);
+
         const state = await this.signInJWT.sign({
             host,
             returnTo,
+            nonce,
         });
 
         // authenticate user
@@ -228,17 +371,37 @@ export class Authenticator {
             );
             return;
         }
-        const returnTo: string | undefined = req.query.returnTo?.toString();
+        const returnToParam: 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 authProvider = host && (await this.getAuthProviderForHost(host));
-        if (!returnTo || !host || !authProvider) {
+
+        if (!returnToParam || !host || !authProvider) {
             log.info(`Bad request: missing parameters.`, { "authorize-flow": true });
             res.redirect(this.getSorryUrl(`Bad request: missing parameters.`));
             return;
         }
 
+        // Validate returnTo URL against allowlist for authorize API
+        const isStrictAuthorizeValidationEnabled = await getFeatureFlagEnableStrictAuthorizeReturnTo();
+        const isValidReturnTo = isStrictAuthorizeValidationEnabled
+            ? validateAuthorizeReturnToUrl(returnToParam, this.config.hostUrl)
+            : validateLoginReturnToUrl(returnToParam, this.config.hostUrl);
+
+        if (!isValidReturnTo) {
+            const validationType = isStrictAuthorizeValidationEnabled ? "strict authorize" : "login fallback";
+            log.warn(`Invalid returnTo URL rejected for authorize (${validationType}): ${returnToParam}`, {
+                "authorize-flow": true,
+                strictValidation: isStrictAuthorizeValidationEnabled,
+            });
+            res.redirect(this.getSorryUrl(`Invalid return URL.`));
+            return;
+        }
+
+        // Ensure returnTo URL has a fragment to prevent OAuth token inheritance attacks
+        const returnTo = ensureUrlHasFragment(returnToParam);
+
         // For non-verified org auth provider, ensure user is an owner of the org
         if (!authProvider.info.verified && authProvider.info.organizationId) {
             const member = await this.teamDb.findTeamMembership(user.id, authProvider.info.organizationId);
@@ -297,7 +460,12 @@ export class Authenticator {
         }
         // authorize Gitpod
         log.info(`(doAuthorize) wanted scopes (${override ? "overriding" : "merging"}): ${wantedScopes.join(",")}`);
-        const state = await this.signInJWT.sign({ host, returnTo, overrideScopes: override });
+
+        // Always generate nonce for CSRF protection (validation controlled by feature flag)
+        const nonce = this.nonceService.generateNonce();
+        this.nonceService.setNonceCookie(res, nonce);
+
+        const state = await this.signInJWT.sign({ host, returnTo, overrideScopes: override, nonce });
         authProvider.authorize(req, res, next, this.deriveAuthState(state), wantedScopes);
     }
     private mergeScopes(a: string[], b: string[]) {