[server] Enforce state presence and validation on the /api/authorize endoint

CLAUDE.md

@@ -27,6 +27,7 @@ The `memory-bank/components/` directory contains detailed documentation for each
 2. **Component-specific work** - Refer to the relevant component documentation in `memory-bank/components/`
 3. **Architecture decisions** - Check `memory-bank/systemPatterns.md` for established patterns and conventions
 4. **Current focus** - Review `memory-bank/activeContext.md` for ongoing work and priorities
+5. **Git workflow** - **CRITICAL**: Always use feature branches with proper naming convention (see `memory-bank/systemPatterns.md` for branch naming rules)
 
 ## Key Characteristics
 

components/server/src/auth/authenticator.spec.ts

@@ -0,0 +1,189 @@
+/**
+ * Copyright (c) 2020 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 * as chai from "chai";
+import { validateAuthorizeReturnToUrl } from "./authenticator";
+import { Config } from "../config";
+import { AuthProvider } from "./auth-provider";
+
+const expect = chai.expect;
+
+describe("authenticator", function () {
+    describe("validateAuthorizeReturnToUrl", function () {
+        const mockConfig = {
+            hostUrl: {
+                url: new URL("https://gitpod.io"),
+            },
+        } as Config;
+
+        const mockAuthProvider = {} as AuthProvider;
+
+        describe("valid URLs", function () {
+            it("should return true for valid URLs from configured domain", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://gitpod.io/dashboard",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.true;
+            });
+
+            it("should return true for valid URLs from www.gitpod.io", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://www.gitpod.io/pricing",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.true;
+            });
+
+            it("should return false for URLs with subdomain that don't match prefix", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://app.gitpod.io/workspaces",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.false;
+            });
+        });
+
+        describe("invalid URLs - API paths", function () {
+            it("should return false for URLs with /api path", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://gitpod.io/api/authorize",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.false;
+            });
+
+            it("should return false for URLs with /api path", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://gitpod.io/api/users",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.false;
+            });
+
+            it("should return false for URLs with /api/ path", function () {
+                const result = validateAuthorizeReturnToUrl("https://gitpod.io/api/", mockConfig, mockAuthProvider);
+                expect(result).to.be.false;
+            });
+
+            it("should return false for URLs with /API path (case insensitive)", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://gitpod.io/API/users",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.false;
+            });
+
+            it("should return false for URLs with /api/callback path", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://gitpod.io/api/callback",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.false;
+            });
+
+            it("should return false for URLs with api. subdomain", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://api.gitpod.io/users",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.false;
+            });
+
+            it("should return false for URLs with API. subdomain (case insensitive)", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://API.gitpod.io/users",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.false;
+            });
+
+            it("should return false for URLs with api. subdomain and /callback path", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://api.gitpod.io/callback",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.false;
+            });
+        });
+
+        describe("invalid URLs - external domains", function () {
+            it("should return false for URLs from external domains", function () {
+                const result = validateAuthorizeReturnToUrl("https://evil.com/dashboard", mockConfig, mockAuthProvider);
+                expect(result).to.be.false;
+            });
+
+            it("should return false for URLs that look similar but are external", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://gitpod.io.evil.com/dashboard",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.false;
+            });
+        });
+
+        describe("malformed URLs", function () {
+            it("should return false for malformed URLs", function () {
+                const result = validateAuthorizeReturnToUrl("not-a-url", mockConfig, mockAuthProvider);
+                expect(result).to.be.false;
+            });
+
+            it("should return false for URLs with invalid protocol", function () {
+                const result = validateAuthorizeReturnToUrl("javascript:alert('xss')", mockConfig, mockAuthProvider);
+                expect(result).to.be.false;
+            });
+        });
+
+        describe("edge cases", function () {
+            it("should return true for URLs with query parameters", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://gitpod.io/dashboard?tab=workspaces",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.true;
+            });
+
+            it("should return true for URLs with fragments", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://gitpod.io/dashboard#workspaces",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.true;
+            });
+
+            it("should return true for URLs with 'api' in query parameter but not in path", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://gitpod.io/dashboard?redirect=/api",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.true;
+            });
+
+            it("should return true for URLs with 'api' in path but not starting with /api", function () {
+                const result = validateAuthorizeReturnToUrl(
+                    "https://gitpod.io/graphql-api",
+                    mockConfig,
+                    mockAuthProvider,
+                );
+                expect(result).to.be.true;
+            });
+        });
+    });
+});

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 { getReturnToParamWithSafeBaseDomain } from "../express-util";
+import { getFeatureFlagEnforceAuthorizeStateValidation } from "../util/featureflags";
 
 @injectable()
 export class Authenticator {
@@ -239,6 +241,17 @@ export class Authenticator {
             return;
         }
 
+        // Validate returnTo URL if feature flag is enabled
+        const enforceValidation = await getFeatureFlagEnforceAuthorizeStateValidation(user.id);
+        if (enforceValidation) {
+            const valid = validateAuthorizeReturnToUrl(returnTo, this.config, authProvider);
+            if (!valid) {
+                log.info(`Bad request: invalid returnTo URL.`, { "authorize-flow": true });
+                res.redirect(this.getSorryUrl(`Bad request: invalid returnTo URL.`));
+                return;
+            }
+        }
+
         // 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);
@@ -322,3 +335,35 @@ export class Authenticator {
         return this.config.hostUrl.asSorry(message).toString();
     }
 }
+
+// Whether the passed URL is a valid returnTo URL for OAuth authorization
+export function validateAuthorizeReturnToUrl(returnTo: string, config: Config, authProvider: AuthProvider): boolean {
+    function isApiPath(url: string): boolean {
+        try {
+            const parsedUrl = new URL(url);
+
+            // Check if hostname starts with "api."
+            if (parsedUrl.hostname.toLowerCase().startsWith("api.")) {
+                return true;
+            }
+
+            // Check if path starts with "/api"
+            if (parsedUrl.pathname.toLowerCase().startsWith("/api")) {
+                return true;
+            }
+
+            return false;
+        } catch {
+            // If URL parsing fails, consider it invalid
+            return true;
+        }
+    }
+
+    // Reject URLs that point to API endpoints
+    if (isApiPath(returnTo)) {
+        return false;
+    }
+
+    const validUrl = getReturnToParamWithSafeBaseDomain(returnTo, config.hostUrl.url);
+    return validUrl !== undefined;
+}

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

@@ -24,6 +24,8 @@ import {
 } from "../auth/errors";
 import { Config } from "../config";
 import { getRequestingClientInfo } from "../express-util";
+import { getFeatureFlagEnforceAuthorizeStateValidation } from "../util/featureflags";
+import { validateAuthorizeReturnToUrl } from "./authenticator";
 import { TokenProvider } from "../user/token-provider";
 import { UserAuthentication } from "../user/user-authentication";
 import { AuthProviderService } from "./auth-provider-service";
@@ -302,6 +304,21 @@ export abstract class GenericAuthProvider implements AuthProvider {
             return;
         }
 
+        // Validate returnTo URL if feature flag is enabled
+        if (authFlow.returnTo) {
+            // Check feature flag - use existing user if logged in, otherwise default to false for new users
+            const userId = request.user?.id;
+            const enforceValidation = await getFeatureFlagEnforceAuthorizeStateValidation(userId || "");
+            if (enforceValidation) {
+                const valid = validateAuthorizeReturnToUrl(authFlow.returnTo, this.config, this);
+                if (!valid) {
+                    log.info(cxt, `(${strategyName}) Bad request: invalid returnTo URL.`, { clientInfo });
+                    response.redirect(this.getSorryUrl(`Bad request: invalid returnTo URL.`));
+                    return;
+                }
+            }
+        }
+
         if (!this.loginCompletionHandler.isBaseDomain(request)) {
             // For auth requests that are not targetting the base domain, we redirect to the base domain, so they come with our cookie.
             log.info(`(${strategyName}) Auth request on subdomain, redirecting to base domain`, { clientInfo });
@@ -321,7 +338,6 @@ export abstract class GenericAuthProvider implements AuthProvider {
                 return;
             }
         }
-
         // assert additional infomation is attached to current session
         if (!authFlow) {
             // The auth flow state info is missing in the session: count as client error

components/server/src/express-util.spec.ts

@@ -5,7 +5,7 @@
  */
 
 import * as chai from "chai";
-import { isAllowedWebsocketDomain } from "./express-util";
+import { isAllowedWebsocketDomain, getReturnToParamWithSafeBaseDomain } from "./express-util";
 const expect = chai.expect;
 
 describe("express-util", function () {
@@ -47,4 +47,82 @@ describe("express-util", function () {
             expect(result).to.be.false;
         });
     });
+
+    describe("getReturnToParamWithSafeBaseDomain", function () {
+        const configuredBaseDomain = new URL("https://gitpod.io");
+
+        describe("valid URLs", function () {
+            it("should allow URLs from configured base domain", function () {
+                const result = getReturnToParamWithSafeBaseDomain("https://gitpod.io/dashboard", configuredBaseDomain);
+                expect(result).to.equal("https://gitpod.io/dashboard");
+            });
+
+            it("should allow URLs from www.gitpod.io", function () {
+                const result = getReturnToParamWithSafeBaseDomain(
+                    "https://www.gitpod.io/pricing",
+                    configuredBaseDomain,
+                );
+                expect(result).to.equal("https://www.gitpod.io/pricing");
+            });
+
+            it("should reject URLs with subdomain that don't match prefix", function () {
+                const result = getReturnToParamWithSafeBaseDomain(
+                    "https://app.gitpod.io/workspaces",
+                    configuredBaseDomain,
+                );
+                expect(result).to.be.undefined;
+            });
+
+            it("should handle undefined returnToURL", function () {
+                const result = getReturnToParamWithSafeBaseDomain(undefined, configuredBaseDomain);
+                expect(result).to.be.undefined;
+            });
+
+            it("should handle empty returnToURL", function () {
+                const result = getReturnToParamWithSafeBaseDomain("", configuredBaseDomain);
+                expect(result).to.be.undefined;
+            });
+        });
+
+        describe("invalid URLs - external domains", function () {
+            it("should reject URLs from external domains", function () {
+                const result = getReturnToParamWithSafeBaseDomain("https://evil.com/dashboard", configuredBaseDomain);
+                expect(result).to.be.undefined;
+            });
+
+            it("should reject URLs that look similar but are external", function () {
+                const result = getReturnToParamWithSafeBaseDomain(
+                    "https://gitpod.io.evil.com/dashboard",
+                    configuredBaseDomain,
+                );
+                expect(result).to.be.undefined;
+            });
+        });
+
+        describe("edge cases", function () {
+            it("should allow URLs with query parameters", function () {
+                const result = getReturnToParamWithSafeBaseDomain(
+                    "https://gitpod.io/dashboard?tab=workspaces",
+                    configuredBaseDomain,
+                );
+                expect(result).to.equal("https://gitpod.io/dashboard?tab=workspaces");
+            });
+
+            it("should allow URLs with fragments", function () {
+                const result = getReturnToParamWithSafeBaseDomain(
+                    "https://gitpod.io/dashboard#workspaces",
+                    configuredBaseDomain,
+                );
+                expect(result).to.equal("https://gitpod.io/dashboard#workspaces");
+            });
+
+            it("should allow URLs with 'api' in path but not starting with /api", function () {
+                const result = getReturnToParamWithSafeBaseDomain(
+                    "https://gitpod.io/graphql-api",
+                    configuredBaseDomain,
+                );
+                expect(result).to.equal("https://gitpod.io/graphql-api");
+            });
+        });
+    });
 });