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

@@ -18,6 +18,8 @@ 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";
 
 @injectable()
 export class Authenticator {
@@ -30,6 +32,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 +80,50 @@ 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
+                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;
+                }
+
+                // Validate origin for additional CSRF protection
+                if (!this.nonceService.validateOrigin(req)) {
+                    log.error(`CSRF protection: Origin validation failed`, {
+                        url: req.url,
+                        origin: req.get("Origin"),
+                        referer: req.get("Referer"),
+                    });
+                    res.status(403).send("Invalid request");
+                    return;
+                }
+
+                // Clear the nonce cookie after successful validation
+                this.nonceService.clearNonceCookie(res);
+
                 const hostContext = this.hostContextProvider.get(host);
                 if (!hostContext) {
                     throw new Error("No host context found.");
@@ -89,6 +136,8 @@ export class Authenticator {
                 await hostContext.authProvider.callback(req, res, next);
             } catch (error) {
                 log.error(`Failed to handle callback.`, error, { url: req.url });
+                // Clear nonce cookie on error as well
+                this.nonceService.clearNonceCookie(res);
             }
         } else {
             // Otherwise proceed with other handlers
@@ -121,6 +170,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;
@@ -138,6 +196,9 @@ export class Authenticator {
         // returnTo defaults to workspaces url
         const workspaceUrl = this.config.hostUrl.asDashboard().toString();
         returnTo = returnTo || workspaceUrl;
+
+        // Ensure returnTo URL has a fragment to prevent OAuth token inheritance attacks
+        returnTo = ensureUrlHasFragment(returnTo);
         const host: string = req.query.host?.toString() || "";
         const authProvider = host && (await this.getAuthProviderForHost(host));
         if (!host || !authProvider) {
@@ -170,9 +231,16 @@ export class Authenticator {
             return;
         }
 
+        // Generate nonce for CSRF protection
+        const nonce = this.nonceService.generateNonce();
+
+        // Set nonce cookie
+        this.nonceService.setNonceCookie(res, nonce);
+
         const state = await this.signInJWT.sign({
             host,
             returnTo,
+            nonce,
         });
 
         // authenticate user
@@ -228,17 +296,21 @@ 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;
         }
 
+        // 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 +369,14 @@ 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 });
+
+        // Generate nonce for CSRF protection
+        const nonce = this.nonceService.generateNonce();
+
+        // Set nonce cookie
+        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[]) {

components/server/src/auth/fragment-protection.spec.ts

@@ -0,0 +1,52 @@
+/**
+ * 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";
+import { ensureUrlHasFragment } from "./fragment-utils";
+
+describe("Fragment Protection", () => {
+    describe("ensureUrlHasFragment", () => {
+        it("should add empty fragment to URL without fragment", () => {
+            const url = "https://gitpod.io/workspaces";
+            const result = ensureUrlHasFragment(url);
+
+            expect(result).to.equal("https://gitpod.io/workspaces#");
+        });
+
+        it("should preserve existing fragment", () => {
+            const url = "https://gitpod.io/workspaces#existing";
+            const result = ensureUrlHasFragment(url);
+
+            expect(result).to.equal(url);
+        });
+
+        it("should handle URLs with query parameters", () => {
+            const url = "https://gitpod.io/workspaces?tab=recent";
+            const result = ensureUrlHasFragment(url);
+
+            expect(result).to.equal("https://gitpod.io/workspaces?tab=recent#");
+        });
+
+        it("should handle invalid URLs gracefully", () => {
+            const url = "not-a-valid-url";
+            const result = ensureUrlHasFragment(url);
+
+            expect(result).to.equal("not-a-valid-url#");
+        });
+
+        it("should prevent OAuth token inheritance attack", () => {
+            // Scenario: OAuth provider redirects with token in fragment
+            // If returnTo URL has no fragment, browser inherits the token fragment
+            const returnToWithoutFragment = "https://gitpod.io/workspaces";
+            const protectedUrl = ensureUrlHasFragment(returnToWithoutFragment);
+
+            // Now when OAuth provider redirects to: protectedUrl + token fragment
+            // The existing fragment prevents inheritance
+            expect(protectedUrl).to.include("#");
+            expect(protectedUrl).to.not.equal(returnToWithoutFragment);
+        });
+    });
+});

components/server/src/auth/fragment-utils.ts

@@ -0,0 +1,32 @@
+/**
+ * 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.
+ */
+
+/**
+ * Ensures a returnTo URL has a fragment to prevent OAuth token inheritance attacks.
+ *
+ * When OAuth providers use response_type=token, they redirect with access tokens
+ * in URL fragments. If the returnTo URL doesn't have a fragment, browsers inherit
+ * the current page's fragment, potentially exposing tokens to malicious sites.
+ *
+ * Uses an empty fragment (#) to prevent inheritance without interfering with
+ * Gitpod's context provider resolution.
+ */
+export function ensureUrlHasFragment(url: string): string {
+    try {
+        const parsedUrl = new URL(url);
+        // If URL already has a fragment, return as-is
+        if (parsedUrl.hash) {
+            return url;
+        }
+        // Add empty fragment to prevent inheritance
+        // Using just "#" to avoid interfering with context provider resolution that
+        // treats fragments as git URLs
+        return url + "#";
+    } catch (error) {
+        // If URL is invalid, add fragment anyway
+        return url + "#";
+    }
+}

components/server/src/auth/login-completion-handler.ts

@@ -18,6 +18,7 @@ import { trackLogin } from "../analytics";
 import { SessionHandler } from "../session-handler";
 import { AuthJWT } from "./jwt";
 import { OneTimeSecretServer } from "../one-time-secret-server";
+import { ensureUrlHasFragment } from "./fragment-utils";
 
 /**
  * The login completion handler pulls the strings between the OAuth2 flow, the ToS flow, and the session management.
@@ -58,6 +59,10 @@ export class LoginCompletionHandler {
 
         // Update session info
         let returnTo = returnToUrl || this.config.hostUrl.asDashboard().toString();
+
+        // Ensure returnTo URL has a fragment to prevent OAuth token inheritance attacks
+        returnTo = ensureUrlHasFragment(returnTo);
+
         if (elevateScopes) {
             const elevateScopesUrl = this.config.hostUrl
                 .withApi({