refactor: consolidate fragment protection and fix context provider conflict

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

Author: iQQBot

components/server/src/auth/authenticator.ts

@@ -19,6 +19,7 @@ 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 {
@@ -171,6 +172,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) {
@@ -268,17 +272,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);

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({