WIP

Author: geropl

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 { getReturnToParamWithSafeBaseDomain } from "../express-util";
 
 @injectable()
 export class Authenticator {
@@ -239,6 +240,14 @@ export class Authenticator {
             return;
         }
 
+        // Validate returnTo URL
+        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 +331,9 @@ export class Authenticator {
         return this.config.hostUrl.asSorry(message).toString();
     }
 }
+
+// Whether the passed URL is a valid
+export function validateAuthorizeReturnToUrl(returnTo: string, config: Config, authProvider: AuthProvider): boolean {
+    // Constraints
+    getReturnToParamWithSafeBaseDomain(returnTo, config.hostUrl.url);
+}

components/server/src/express-util.ts

@@ -159,3 +159,26 @@ export function toClientHeaderFields(expressReq: express.Request): ClientHeaderF
         clientRegion: takeFirst(expressReq.headers["x-glb-client-region"]),
     };
 }
+
+export function getReturnToParamWithSafeBaseDomain(
+    returnToURL: string | undefined,
+    configuredBaseDomain: URL,
+): string | undefined {
+    function urlStartsWith(url: string, prefixUrl: string): boolean {
+        prefixUrl += prefixUrl.endsWith("/") ? "" : "/";
+        return url.toLowerCase().startsWith(prefixUrl.toLowerCase());
+    }
+
+    if (!returnToURL) {
+        return undefined;
+    }
+
+    if (
+        urlStartsWith(returnToURL, configuredBaseDomain.toString()) ||
+        urlStartsWith(returnToURL, "https://www.gitpod.io")
+    ) {
+        return returnToURL;
+    }
+
+    return;
+}

components/server/src/user/user-controller.ts

@@ -17,7 +17,7 @@ import { Permission } from "@gitpod/gitpod-protocol/lib/permission";
 import { parseWorkspaceIdFromHostname } from "@gitpod/gitpod-protocol/lib/util/parse-workspace-id";
 import { SessionHandler } from "../session-handler";
 import { URL } from "url";
-import { getRequestingClientInfo } from "../express-util";
+import { getRequestingClientInfo, getReturnToParamWithSafeBaseDomain } from "../express-util";
 import { GitpodToken, GitpodTokenType, User } from "@gitpod/gitpod-protocol";
 import { HostContextProvider } from "../auth/host-context-provider";
 import { reportJWTCookieIssued } from "../prometheus-metrics";
@@ -37,7 +37,6 @@ import { UserService } from "./user-service";
 import { WorkspaceService } from "../workspace/workspace-service";
 import { runWithSubjectId } from "../util/request-context";
 import { SubjectId } from "../auth/subject-id";
-import { TrustedValue } from "@gitpod/gitpod-protocol/lib/util/scrubbing";
 
 export const ServerFactory = Symbol("ServerFactory");
 export type ServerFactory = () => GitpodServerImpl;
@@ -615,20 +614,7 @@ export class UserController {
     protected getSafeReturnToParam(req: express.Request) {
         // @ts-ignore Type 'ParsedQs' is not assignable
         const returnToURL: string | undefined = req.query.redirect || req.query.returnTo;
-        if (!returnToURL) {
-            log.debug("Empty redirect URL");
-            return;
-        }
-
-        if (
-            this.urlStartsWith(returnToURL, this.config.hostUrl.toString()) ||
-            this.urlStartsWith(returnToURL, "https://www.gitpod.io")
-        ) {
-            return returnToURL;
-        }
-
-        log.debug("The redirect URL does not match", { query: new TrustedValue(req.query).value });
-        return;
+        return getReturnToParamWithSafeBaseDomain(returnToURL, this.config.hostUrl.url);
     }
 
     private createGitpodServer(user: User, resourceGuard: ResourceAccessGuard) {

components/server/src/util/featureflags.ts

@@ -11,3 +11,9 @@ export async function getFeatureFlagEnableExperimentalJBTB(userId: string): Prom
         user: { id: userId },
     });
 }
+
+export async function getFeatureFlagEnforceAuthorizeStateValidation(userId: string): Promise<boolean> {
+    return getExperimentsClientForBackend().getValueAsync("enforce_authorize_state_validation", false, {
+        user: { id: userId },
+    });
+}