feat: implement CSRF protection for OAuth flows with nonce validation

iQQBot · ona-agent · iQQBot · commit 2ab75631280c · 2025-07-28T15:45:08.000Z
- 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 &lt;no-reply@ona.com&gt;
diff --git a/components/server/src/auth/auth-provider.ts b/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 {
diff --git a/components/server/src/auth/authenticator.ts b/components/server/src/auth/authenticator.ts
@@ -18,6 +18,7 @@ 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";
 
 @injectable()
 export class Authenticator {
@@ -30,6 +31,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 +79,35 @@ export class Authenticator {
                 if (!host) {
                     throw new Error("Auth flow state is missing 'host' attribute.");
                 }
+
+                // 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("CSRF protection: Invalid or missing nonce");
+                    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("CSRF protection: Invalid request origin");
+                    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 +120,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
@@ -170,9 +203,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
@@ -297,7 +337,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[]) {
diff --git a/components/server/src/auth/nonce-service.spec.ts b/components/server/src/auth/nonce-service.spec.ts
@@ -0,0 +1,114 @@
+/**
+ * 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 { Container } from "inversify";
+import { Config } from "../config";
+import { NonceService } from "./nonce-service";
+
+describe("NonceService", () => {
+    let container: Container;
+    let nonceService: NonceService;
+
+    beforeEach(() => {
+        container = new Container();
+        container.bind(Config).toConstantValue({
+            hostUrl: {
+                url: new URL("https://gitpod.io"),
+            },
+            auth: {
+                session: {
+                    cookie: {
+                        secure: true,
+                    },
+                },
+            },
+        } as any);
+        container.bind(NonceService).toSelf();
+        nonceService = container.get(NonceService);
+    });
+
+    describe("generateNonce", () => {
+        it("should generate unique nonces", () => {
+            const nonce1 = nonceService.generateNonce();
+            const nonce2 = nonceService.generateNonce();
+
+            expect(nonce1).to.be.a("string");
+            expect(nonce2).to.be.a("string");
+            expect(nonce1).to.not.equal(nonce2);
+            expect(nonce1.length).to.be.greaterThan(40); // base64url encoded 32 bytes
+        });
+    });
+
+    describe("validateNonce", () => {
+        it("should validate matching nonces", () => {
+            const nonce = nonceService.generateNonce();
+            const isValid = nonceService.validateNonce(nonce, nonce);
+            expect(isValid).to.be.true;
+        });
+
+        it("should reject different nonces", () => {
+            const nonce1 = nonceService.generateNonce();
+            const nonce2 = nonceService.generateNonce();
+            const isValid = nonceService.validateNonce(nonce1, nonce2);
+            expect(isValid).to.be.false;
+        });
+
+        it("should reject undefined nonces", () => {
+            const nonce = nonceService.generateNonce();
+            expect(nonceService.validateNonce(nonce, undefined)).to.be.false;
+            expect(nonceService.validateNonce(undefined, nonce)).to.be.false;
+            expect(nonceService.validateNonce(undefined, undefined)).to.be.false;
+        });
+    });
+
+    describe("validateOrigin", () => {
+        it("should accept requests from same origin", () => {
+            const req = {
+                get: (header: string) => {
+                    if (header === "Origin") return "https://gitpod.io";
+                    return undefined;
+                },
+            } as any;
+
+            const isValid = nonceService.validateOrigin(req);
+            expect(isValid).to.be.true;
+        });
+
+        it("should reject requests from different origin", () => {
+            const req = {
+                get: (header: string) => {
+                    if (header === "Origin") return "https://evil.com";
+                    return undefined;
+                },
+            } as any;
+
+            const isValid = nonceService.validateOrigin(req);
+            expect(isValid).to.be.false;
+        });
+
+        it("should reject requests without origin or referer", () => {
+            const req = {
+                get: () => undefined,
+            } as any;
+
+            const isValid = nonceService.validateOrigin(req);
+            expect(isValid).to.be.false;
+        });
+
+        it("should accept requests with valid referer", () => {
+            const req = {
+                get: (header: string) => {
+                    if (header === "Referer") return "https://gitpod.io/login";
+                    return undefined;
+                },
+            } as any;
+
+            const isValid = nonceService.validateOrigin(req);
+            expect(isValid).to.be.true;
+        });
+    });
+});
diff --git a/components/server/src/auth/nonce-service.ts b/components/server/src/auth/nonce-service.ts
@@ -0,0 +1,107 @@
+/**
+ * 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 { injectable, inject } from "inversify";
+import express from "express";
+import * as crypto from "crypto";
+import { Config } from "../config";
+
+const NONCE_COOKIE_NAME = "__Host-_gitpod_oauth_nonce_";
+const NONCE_LENGTH = 32; // 32 bytes = 256 bits
+
+@injectable()
+export class NonceService {
+    @inject(Config) protected readonly config: Config;
+
+    /**
+     * Generates a cryptographically secure random nonce
+     */
+    generateNonce(): string {
+        return crypto.randomBytes(NONCE_LENGTH).toString("base64url");
+    }
+
+    /**
+     * Sets the nonce cookie in the response
+     */
+    setNonceCookie(res: express.Response, nonce: string): void {
+        res.cookie(NONCE_COOKIE_NAME, nonce, {
+            httpOnly: true,
+            secure: this.config.auth.session.cookie.secure,
+            sameSite: "strict", // Strict for CSRF protection
+            maxAge: 5 * 60 * 1000, // 5 minutes (same as JWT state expiry)
+            path: "/auth", // Limit to auth paths only
+        });
+    }
+
+    /**
+     * Gets the nonce from the request cookies
+     */
+    getNonceFromCookie(req: express.Request): string | undefined {
+        return req.cookies?.[NONCE_COOKIE_NAME];
+    }
+
+    /**
+     * Clears the nonce cookie
+     */
+    clearNonceCookie(res: express.Response): void {
+        res.clearCookie(NONCE_COOKIE_NAME, {
+            httpOnly: true,
+            secure: this.config.auth.session.cookie.secure,
+            sameSite: "strict",
+            path: "/auth",
+        });
+    }
+
+    /**
+     * Validates that the nonce from the state matches the nonce from the cookie
+     */
+    validateNonce(stateNonce: string | undefined, cookieNonce: string | undefined): boolean {
+        if (!stateNonce || !cookieNonce) {
+            return false;
+        }
+
+        // Use crypto.timingSafeEqual to prevent timing attacks
+        if (stateNonce.length !== cookieNonce.length) {
+            return false;
+        }
+
+        const stateBuffer = Buffer.from(stateNonce, "base64url");
+        const cookieBuffer = Buffer.from(cookieNonce, "base64url");
+
+        if (stateBuffer.length !== cookieBuffer.length) {
+            return false;
+        }
+
+        return crypto.timingSafeEqual(stateBuffer, cookieBuffer);
+    }
+
+    /**
+     * Validates that the request origin is from a trusted domain
+     */
+    validateOrigin(req: express.Request): boolean {
+        const origin = req.get("Origin");
+        const referer = req.get("Referer");
+
+        // For OAuth callbacks, we expect either Origin or Referer header
+        const requestSource = origin || referer;
+
+        if (!requestSource) {
+            // No origin/referer header - this could be a direct navigation or CSRF attack
+            return false;
+        }
+
+        try {
+            const sourceUrl = new URL(requestSource);
+            const configUrl = new URL(this.config.hostUrl.url.toString());
+
+            // Validate that the request comes from the same origin as our configured host
+            return sourceUrl.origin === configUrl.origin;
+        } catch (error) {
+            // Invalid URL format
+            return false;
+        }
+    }
+}
diff --git a/components/server/src/container-module.ts b/components/server/src/container-module.ts
@@ -45,6 +45,7 @@ import { HostContainerMapping } from "./auth/host-container-mapping";
 import { HostContextProvider, HostContextProviderFactory } from "./auth/host-context-provider";
 import { HostContextProviderImpl } from "./auth/host-context-provider-impl";
 import { AuthJWT, SignInJWT } from "./auth/jwt";
+import { NonceService } from "./auth/nonce-service";
 import { LoginCompletionHandler } from "./auth/login-completion-handler";
 import { VerificationService } from "./auth/verification-service";
 import { InstallationService } from "./auth/installation-service";
@@ -366,6 +367,7 @@ export const productionContainerModule = new ContainerModule(
 
         bind(AuthJWT).toSelf().inSingletonScope();
         bind(SignInJWT).toSelf().inSingletonScope();
+        bind(NonceService).toSelf().inSingletonScope();
 
         bind(PrebuildManager).toSelf().inSingletonScope();
         bind(LazyPrebuildManager).toFactory((ctx) => {

Original file line number	Diff line number	Diff line change
`@@ -97,6 +97,7 @@ export interface AuthFlow {`
`97`	`97`	`readonly host: string;`
`98`	`98`	`readonly returnTo: string;`
`99`	`99`	`readonly overrideScopes?: boolean;`
	`100`	`+ readonly nonce?: string;`
`100`	`101`	`}`
`101`	`102`	`export namespace AuthFlow {`
`102`	`103`	`export function is(obj: any): obj is AuthFlow {`