diff --git a/components/server/src/auth/generic-auth-provider.ts b/components/server/src/auth/generic-auth-provider.ts index f9b2ec98022ab5..697a7c560a01fb 100644 --- a/components/server/src/auth/generic-auth-provider.ts +++ b/components/server/src/auth/generic-auth-provider.ts @@ -145,11 +145,22 @@ export class GenericAuthProvider implements AuthProvider { protected readAuthUserSetup?: (accessToken: string, tokenResponse: object) => Promise; authorize(req: express.Request, res: express.Response, next: express.NextFunction, scope?: string[]): void { - const handler = passport.authenticate(this.getStrategy() as any, { - ...this.defaultStrategyOptions, - ...{ scope }, - }); - handler(req, res, next); + // Before the OAuth process is started, the Gitpod cookie is relaxed ot be sent on cross-site request, + // which makes possible to re-establish the session on a callback from the 3rd party service. + // Once this callback is handled, the Gitpod cookie is elevated to a `strict` one. + this.updateCookieSameSiteValue(req, "lax") + .then(() => { + const handler = passport.authenticate(this.getStrategy() as any, { + ...this.defaultStrategyOptions, + ...{ scope }, + }); + handler(req, res, next); + }) + .catch((err) => { + log.error(`(${this.strategyName}) Failed to store session before redirect.`, { + err, + }); + }); } protected getStrategy() { @@ -466,6 +477,16 @@ export class GenericAuthProvider implements AuthProvider { ...defaultLogPayload, }); + try { + // When the OAuth process is done, we want to proceed with a `strict` Gitpod cookie. + // This improves the security model with Browser agents, as cross-site requests whould + // not be containing the Gitpod cookie anymore. + await this.updateCookieSameSiteValue(request, "strict"); + } catch (error) { + response.redirect(this.getSorryUrl(`Failed to save session. (${error})`)); + return; + } + // Complete login const { host, returnTo } = authFlow; await this.loginCompletionHandler.complete(request, response, { @@ -478,6 +499,19 @@ export class GenericAuthProvider implements AuthProvider { } } + protected async updateCookieSameSiteValue(request: express.Request, newValue: "strict" | "lax") { + return new Promise((resolve, reject) => { + request.session.cookie.sameSite = newValue; + request.session.save((err) => { + if (err) { + reject(err); + } else { + resolve(undefined); + } + }); + }); + } + protected sendCompletionRedirectWithError(response: express.Response, error: object): void { log.info(`(${this.strategyName}) Send completion redirect with error`, { error }); diff --git a/components/server/src/session-handler.ts b/components/server/src/session-handler.ts index 30e6612720a63d..b3f9d18a6cd988 100644 --- a/components/server/src/session-handler.ts +++ b/components/server/src/session-handler.ts @@ -27,10 +27,10 @@ export class SessionHandlerProvider { public init() { const options: SessionOptions = {} as SessionOptions; options.cookie = this.getCookieOptions(this.config); - (options.genid = function (req: any) { + options.genid = function (req: any) { return uuidv4(); // use UUIDs for session IDs - }), - (options.name = SessionHandlerProvider.getCookieName(this.config)); + }; + options.name = SessionHandlerProvider.getCookieName(this.config); // options.proxy = true // TODO SSL Proxy options.resave = true; // TODO Check with store! See docu options.rolling = true; // default, new cookie and maxAge @@ -53,7 +53,7 @@ export class SessionHandlerProvider { httpOnly: true, // default secure: false, // default, TODO SSL! Config proxy maxAge: config.session.maxAgeMs, // configured in Helm chart, defaults to 3 days. - sameSite: "lax", // default: true. "Lax" needed for OAuth. + sameSite: "strict", }; }