Make auth cookie stricter

components/gitpod-protocol/src/util/gitpod-host-url.ts

@@ -216,4 +216,12 @@ export class GitpodHostUrl {
         }
         return newUrl.with((url) => ({ pathname: "/metrics-api" }));
     }
+
+    asLoginWithOTS(userId: string, key: string, returnToUrl?: string) {
+        const result = this.withApi({ pathname: `/login/ots/${userId}/${key}` });
+        if (returnToUrl) {
+            return result.with({ search: `returnTo=${encodeURIComponent(returnToUrl)}` });
+        }
+        return result;
+    }
 }

components/server/go/pkg/lib/cookie.go

@@ -9,5 +9,5 @@ import "regexp"
 func CookieNameFromDomain(domain string) string {
 	// replace all non-word characters with underscores
 	derived := regexp.MustCompile(`[\W_]+`).ReplaceAllString(domain, "_")
-	return "_" + derived + "_jwt2_"
+	return "__Host-_" + derived + "_jwt2_"
 }

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

@@ -302,6 +302,14 @@ export abstract class GenericAuthProvider implements AuthProvider {
             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 });
+            const target = new URL(request.url, this.config.hostUrl.url.toString()).toString();
+            response.redirect(target);
+            return;
+        }
+
         if (isAlreadyLoggedIn) {
             if (!authFlow) {
                 log.warn(

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

@@ -6,6 +6,7 @@
 
 import { inject, injectable } from "inversify";
 import express from "express";
+import * as crypto from "crypto";
 import { User } from "@gitpod/gitpod-protocol";
 import { log, LogContext } from "@gitpod/gitpod-protocol/lib/util/logging";
 import { Config } from "../config";
@@ -16,6 +17,7 @@ import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
 import { trackLogin } from "../analytics";
 import { SessionHandler } from "../session-handler";
 import { AuthJWT } from "./jwt";
+import { OneTimeSecretServer } from "../one-time-secret-server";
 
 /**
  * The login completion handler pulls the strings between the OAuth2 flow, the ToS flow, and the session management.
@@ -28,6 +30,7 @@ export class LoginCompletionHandler {
     @inject(AuthProviderService) protected readonly authProviderService: AuthProviderService;
     @inject(AuthJWT) protected readonly authJWT: AuthJWT;
     @inject(SessionHandler) protected readonly session: SessionHandler;
+    @inject(OneTimeSecretServer) private readonly otsServer: OneTimeSecretServer;
 
     async complete(
         request: express.Request,
@@ -78,6 +81,26 @@ export class LoginCompletionHandler {
             );
         }
 
+        if (!this.isBaseDomain(request)) {
+            // (GitHub edge case) If we got redirected here onto a sub-domain (e.g. api.gitpod.io), we need to redirect to the base domain in order to Set-Cookie properly.
+            const secret = crypto
+                .createHash("sha256")
+                .update(user.id + this.config.session.secret)
+                .digest("hex");
+            const expirationDate = new Date(Date.now() + 1000 * 60); // 1 minutes
+            const token = await this.otsServer.serveToken({}, secret, expirationDate);
+
+            reportLoginCompleted("succeeded_via_ots", "git");
+            log.info(
+                logContext,
+                `User will be logged in via OTS on the base domain. (Indirect) redirect to: ${returnTo}`,
+            );
+            const baseDomainRedirect = this.config.hostUrl.asLoginWithOTS(user.id, token.token, returnTo).toString();
+            response.redirect(baseDomainRedirect);
+            return;
+        }
+
+        // (default case) If we got redirected here onto the base domain of the Gitpod installation, we can just issue the cookie right away.
         const cookie = await this.session.createJWTSessionCookie(user.id);
         response.cookie(cookie.name, cookie.value, cookie.opts);
         reportJWTCookieIssued();
@@ -87,6 +110,10 @@ export class LoginCompletionHandler {
         response.redirect(returnTo);
     }
 
+    public isBaseDomain(req: express.Request): boolean {
+        return req.hostname === this.config.hostUrl.url.hostname;
+    }
+
     public async updateAuthProviderAsVerified(hostname: string, user: User) {
         const hostCtx = this.hostContextProvider.get(hostname);
         log.info("Updating auth provider as verified", { hostname });

components/server/src/prometheus-metrics.ts

@@ -85,6 +85,8 @@ type LoginCounterStatus =
     | "failed"
     // The login attempt succeeded
     | "succeeded"
+    // The login was successful, but we need to defer cookie creation via an OTS
+    | "succeeded_via_ots"
     // The login attempt failed, because the client failed to provide complete session information, for instance.
     | "failed_client";
 

components/server/src/session-handler.spec.db.ts

@@ -163,9 +163,9 @@ describe("SessionHandler", () => {
             expect(opts.httpOnly).to.equal(true);
             expect(opts.secure).to.equal(true);
             expect(opts.maxAge).to.equal(maxAge * 1000);
-            expect(opts.sameSite).to.equal("strict");
+            expect(opts.sameSite).to.equal("lax");
 
-            expect(name, "Check cookie name").to.equal("_gitpod_dev_jwt_");
+            expect(name, "Check cookie name").to.equal("__Host-_gitpod_dev_jwt_");
         });
     });
     describe("jwtSessionConvertor", () => {
@@ -212,5 +212,35 @@ describe("SessionHandler", () => {
             expect(res.value).to.equal("JWT Session is invalid");
             expect(res.cookie).to.be.undefined;
         });
+
+        it("old JWT cookie is present, is accepted (!), and we get a new one", async () => {
+            const oldExpiredCookie = await sessionHandler.createJWTSessionCookie(existingUser.id, {
+                issuedAtMs: Date.now() - SessionHandler.JWT_REFRESH_THRESHOLD - 1,
+            });
+            oldExpiredCookie.name = "_gitpod_dev_jwt_";
+            const newCookie = await sessionHandler.createJWTSessionCookie(existingUser.id);
+
+            const res = await handle(existingUser, `${oldExpiredCookie.name}=${oldExpiredCookie.value}`);
+            expect(res.status).to.equal(200);
+            expect(res.value).to.equal("Refreshed JWT cookie issued.");
+            expect(res.cookie).to.not.be.undefined;
+            expect(res.cookie?.split("=")[0]).to.equal(newCookie.name);
+        });
+
+        it("old expired AND new one JWT cookies are present, new one is accepted", async () => {
+            const oldExpiredCookie = await sessionHandler.createJWTSessionCookie(existingUser.id, {
+                issuedAtMs: Date.now() - SessionHandler.JWT_REFRESH_THRESHOLD - 1,
+            });
+            oldExpiredCookie.name = "_gitpod_dev_jwt_";
+            const newCookie = await sessionHandler.createJWTSessionCookie(existingUser.id);
+
+            const res = await handle(
+                existingUser,
+                `${oldExpiredCookie.name}=${oldExpiredCookie.value}; ${newCookie.name}=${newCookie.value}`,
+            );
+            expect(res.status).to.equal(200);
+            expect(res.value).to.equal("User session already has a valid JWT session.");
+            expect(res.cookie).to.be.undefined;
+        });
     });
 });

components/server/src/session-handler.ts

@@ -27,15 +27,15 @@ export class SessionHandler {
 
     public jwtSessionConvertor(): express.Handler {
         return async (req, res) => {
-            const user = req.user;
+            const { user } = req;
             if (!user) {
                 res.status(401);
                 res.send("User has no valid session.");
                 return;
             }
 
             const cookies = parseCookieHeader(req.headers.cookie || "");
-            const jwtTokens = cookies[getJWTCookieName(this.config)];
+            const jwtTokens = this.filterCookieValues(cookies);
 
             let decoded: { payload: JwtPayload; keyId: string } | undefined = undefined;
             try {
@@ -146,12 +146,29 @@ export class SessionHandler {
      */
     async verifyJWTCookie(cookie: string): Promise<JwtPayload | undefined> {
         const cookies = parseCookieHeader(cookie);
-        const cookieValues = cookies[getJWTCookieName(this.config)];
+        const cookieValues = this.filterCookieValues(cookies);
 
         const token = await this.verifyFirstValidJwt(cookieValues);
         return token?.payload;
     }
 
+    /**
+     * @param cookies
+     * @returns Primary (the cookie name we set) AND secondary cookie (old accepted cookie name) values (in that order).
+     */
+    private filterCookieValues(cookies: { [key: string]: string[] }): string[] {
+        const cookieValues = cookies[getPrimaryJWTCookieName(this.config)] ?? [];
+
+        const secondaryCookieName = getSecondaryJWTCookieName(this.config);
+        if (secondaryCookieName) {
+            const secondaryCookieValues = cookies[secondaryCookieName];
+            if (secondaryCookieValues) {
+                cookieValues.push(...secondaryCookieValues);
+            }
+        }
+        return cookieValues;
+    }
+
     /**
      * Returns the first valid session token it finds.
      * Edge cases:
@@ -204,10 +221,9 @@ export class SessionHandler {
         const token = await this.authJWT.sign(userID, payload, options?.expirySeconds);
 
         return {
-            name: getJWTCookieName(this.config),
+            name: getPrimaryJWTCookieName(this.config),
             value: token,
             opts: {
-                domain: getJWTCookieDomain(this.config),
                 maxAge: this.config.auth.session.cookie.maxAge * 1000, // express does not match the HTTP spec and uses milliseconds
                 httpOnly: this.config.auth.session.cookie.httpOnly,
                 sameSite: this.config.auth.session.cookie.sameSite,
@@ -216,19 +232,33 @@ export class SessionHandler {
         };
     }
 
-    public clearSessionCookie(res: express.Response, config: Config): void {
-        res.clearCookie(getJWTCookieName(this.config), {
-            domain: getJWTCookieDomain(config),
+    public clearSessionCookie(res: express.Response): void {
+        const { secure, sameSite, httpOnly } = this.config.auth.session.cookie;
+        res.clearCookie(getPrimaryJWTCookieName(this.config), {
+            httpOnly,
+            sameSite,
+            secure,
         });
+
+        const secondaryCookieName = getSecondaryJWTCookieName(this.config);
+        if (secondaryCookieName) {
+            res.clearCookie(secondaryCookieName, {
+                domain: this.config.hostUrl.url.hostname,
+            });
+        }
     }
 }
 
-function getJWTCookieName(config: Config) {
+function getPrimaryJWTCookieName(config: Config) {
     return config.auth.session.cookie.name;
 }
 
-function getJWTCookieDomain(config: Config): string {
-    return config.hostUrl.url.hostname;
+function getSecondaryJWTCookieName(config: Config) {
+    const PREFIX = "__Host-";
+    if (!config.auth.session.cookie.name.startsWith(PREFIX)) {
+        return undefined;
+    }
+    return config.auth.session.cookie.name.slice(PREFIX.length);
 }
 
 function parseCookieHeader(c: string): { [key: string]: string[] } {

components/server/src/test/service-testing-container-module.ts

@@ -55,11 +55,11 @@ export const mockAuthConfig: AuthConfig = {
         issuer: "https://mp-server-d7650ec945.preview.gitpod-dev.com",
         lifetimeSeconds: 7 * 24 * 60 * 60,
         cookie: {
-            name: "_gitpod_dev_jwt_",
+            name: "__Host-_gitpod_dev_jwt_",
             secure: true,
             httpOnly: true,
             maxAge: 7 * 24 * 60 * 60,
-            sameSite: "strict",
+            sameSite: "lax",
         },
     },
 };

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

@@ -37,6 +37,7 @@ 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;
@@ -213,6 +214,14 @@ export class UserController {
                 res.cookie(cookie.name, cookie.value, cookie.opts);
                 reportJWTCookieIssued();
 
+                // If returnTo was passed and it's safe, redirect to it
+                const returnTo = this.getSafeReturnToParam(req);
+                if (returnTo) {
+                    log.info(`Redirecting after OTS login ${returnTo}`);
+                    res.redirect(returnTo);
+                    return;
+                }
+
                 res.sendStatus(200);
             }),
         );
@@ -269,7 +278,7 @@ export class UserController {
             }
 
             // clear cookies
-            this.sessionHandler.clearSessionCookie(res, this.config);
+            this.sessionHandler.clearSessionCookie(res);
 
             // then redirect
             log.info(logContext, "(Logout) Redirecting...", { redirectToUrl, ...logPayload });
@@ -618,7 +627,7 @@ export class UserController {
             return returnToURL;
         }
 
-        log.debug("The redirect URL does not match", { query: req.query });
+        log.debug("The redirect URL does not match", { query: new TrustedValue(req.query).value });
         return;
     }
 

components/ws-proxy/pkg/proxy/routes.go

@@ -822,8 +822,8 @@ func removeSensitiveCookies(cookies []*http.Cookie, domain string) []*http.Cooki
 
 	n := 0
 	for _, c := range cookies {
-		if strings.HasPrefix(c.Name, hostnamePrefix) {
-			// skip session cookie
+		if strings.HasPrefix(c.Name, hostnamePrefix) || strings.HasPrefix(c.Name, "__Host-"+hostnamePrefix) {
+			// skip session cookies
 			continue
 		}
 		log.WithField("hostnamePrefix", hostnamePrefix).WithField("name", c.Name).Debug("keeping cookie")

components/ws-proxy/pkg/proxy/routes_test.go

@@ -983,7 +983,7 @@ func TestRemoveSensitiveCookies(t *testing.T) {
 	var (
 		domain                  = "test-domain.com"
 		sessionCookie           = &http.Cookie{Domain: domain, Name: "_test_domain_com_", Value: "fobar"}
-		sessionCookieJwt2       = &http.Cookie{Domain: domain, Name: "_test_domain_com_jwt2_", Value: "fobar"}
+		sessionCookieJwt2       = &http.Cookie{Domain: domain, Name: "__Host-_test_domain_com_jwt2_", Value: "fobar"}
 		realGitpodSessionCookie = &http.Cookie{Domain: domain, Name: server_lib.CookieNameFromDomain(domain), Value: "fobar"}
 		portAuthCookie          = &http.Cookie{Domain: domain, Name: "_test_domain_com_ws_77f6b236_3456_4b88_8284_81ca543a9d65_port_auth_", Value: "some-token"}
 		ownerCookie             = &http.Cookie{Domain: domain, Name: "_test_domain_com_ws_77f6b236_3456_4b88_8284_81ca543a9d65_owner_", Value: "some-other-token"}

install/installer/pkg/components/auth/config_test.go

@@ -19,42 +19,42 @@ func TestCookieNameFromDomain(t *testing.T) {
 		{
 			name:            "Simple Domain",
 			domain:          "example.com",
-			expectedOutcome: "_example_com_jwt2_",
+			expectedOutcome: "__Host-_example_com_jwt2_",
 		},
 		{
 			name:            "Domain with Underscore",
 			domain:          "example_test.com",
-			expectedOutcome: "_example_test_com_jwt2_",
+			expectedOutcome: "__Host-_example_test_com_jwt2_",
 		},
 		{
 			name:            "Domain with Hyphen",
 			domain:          "example-test.com",
-			expectedOutcome: "_example_test_com_jwt2_",
+			expectedOutcome: "__Host-_example_test_com_jwt2_",
 		},
 		{
 			name:            "Domain with Special Characters",
 			domain:          "example&test.com",
-			expectedOutcome: "_example_test_com_jwt2_",
+			expectedOutcome: "__Host-_example_test_com_jwt2_",
 		},
 		{
 			name:            "Subdomain",
 			domain:          "subdomain.example.com",
-			expectedOutcome: "_subdomain_example_com_jwt2_",
+			expectedOutcome: "__Host-_subdomain_example_com_jwt2_",
 		},
 		{
 			name:            "Subdomain with Hyphen",
 			domain:          "sub-domain.example.com",
-			expectedOutcome: "_sub_domain_example_com_jwt2_",
+			expectedOutcome: "__Host-_sub_domain_example_com_jwt2_",
 		},
 		{
 			name:            "Subdomain with Underscore",
 			domain:          "sub_domain.example.com",
-			expectedOutcome: "_sub_domain_example_com_jwt2_",
+			expectedOutcome: "__Host-_sub_domain_example_com_jwt2_",
 		},
 		{
 			name:            "Subdomain with Special Characters",
 			domain:          "sub&domain.example.com",
-			expectedOutcome: "_sub_domain_example_com_jwt2_",
+			expectedOutcome: "__Host-_sub_domain_example_com_jwt2_",
 		},
 	}
 

install/installer/pkg/components/public-api-server/configmap_test.go

@@ -63,7 +63,7 @@ func TestConfigMap(t *testing.T) {
 				LifetimeSeconds: int64((24 * 7 * time.Hour).Seconds()),
 				Issuer:          "https://test.domain.everything.awesome.is",
 				Cookie: config.CookieConfig{
-					Name:     "_test_domain_everything_awesome_is_jwt2_",
+					Name:     "__Host-_test_domain_everything_awesome_is_jwt2_",
 					MaxAge:   int64((24 * 7 * time.Hour).Seconds()),
 					SameSite: "lax",
 					Secure:   true,

install/installer/pkg/components/server/configmap_test.go

@@ -69,7 +69,7 @@ func TestConfigMap(t *testing.T) {
 				LifetimeSeconds: int64((7 * 24 * time.Hour).Seconds()),
 				Issuer:          "https://awesome.domain",
 				Cookie: auth.CookieConfig{
-					Name:     "_awesome_domain_jwt2_",
+					Name:     "__Host-_awesome_domain_jwt2_",
 					MaxAge:   int64((7 * 24 * time.Hour).Seconds()),
 					SameSite: "lax",
 					Secure:   true,