Skip to content

Commit 7e0e60a

Browse files
committed
fix
1 parent 378df4d commit 7e0e60a

File tree

5 files changed

+60
-48
lines changed

5 files changed

+60
-48
lines changed

components/server/src/auth/jwt.ts

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import * as jsonwebtoken from "jsonwebtoken";
88
import { Config } from "../config";
99
import { inject, injectable } from "inversify";
10+
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
1011

1112
const algorithm: jsonwebtoken.Algorithm = "RS512";
1213

@@ -33,9 +34,18 @@ export class AuthJWT {
3334
}
3435

3536
async verify(encoded: string): Promise<object> {
37+
// When we verify an encoded token, we verify it using all available public keys
38+
// That is, we check the following:
39+
// * The current signing public key
40+
// * All other validating public keys, in order
41+
//
42+
// We do this to allow for key-rotation. But tokens already issued would fail to validate
43+
// if the signing key was changed. To accept older sessions, which are still valid
44+
// we need to check for older keys also.
45+
const validatingPublicKeys = this.config.auth.pki.validating.map((keypair) => keypair.publicKey);
3646
const publicKeys = [
3747
this.config.auth.pki.signing.publicKey, // signing key is checked first
38-
...this.config.auth.pki.validating.map((keypair) => keypair.publicKey),
48+
...validatingPublicKeys,
3949
];
4050

4151
let lastErr;
@@ -46,10 +56,14 @@ export class AuthJWT {
4656
});
4757
return decoded;
4858
} catch (err) {
59+
log.debug(`Failed to verify JWT token using public key.`, err);
4960
lastErr = err;
5061
}
5162
}
5263

64+
log.error(`Failed to verify JWT using any available public key.`, lastErr, {
65+
publicKeyCount: publicKeys.length,
66+
});
5367
throw lastErr;
5468
}
5569
}
@@ -68,22 +82,3 @@ async function verify(
6882
});
6983
});
7084
}
71-
72-
export async function newSessionJWT(userID: string): Promise<string> {
73-
const payload = {
74-
// subject
75-
sub: userID,
76-
// issuer
77-
iss: "gitpod.io",
78-
};
79-
const temporaryTestKeyForExperimentation = "my-secret";
80-
81-
return new Promise((resolve, reject) => {
82-
jsonwebtoken.sign(payload, temporaryTestKeyForExperimentation, { algorithm: "HS256" }, function (err, token) {
83-
if (err || !token) {
84-
return reject(err);
85-
}
86-
return resolve(token);
87-
});
88-
});
89-
}

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

Lines changed: 23 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,14 @@ import { AuthFlow } from "./auth-provider";
1313
import { HostContextProvider } from "./host-context-provider";
1414
import { AuthProviderService } from "./auth-provider-service";
1515
import { TosFlow } from "../terms/tos-flow";
16-
import { increaseLoginCounter } from "../prometheus-metrics";
16+
import { increaseLoginCounter, reportJWTCookieIssued } from "../prometheus-metrics";
1717
import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
1818
import { trackLogin } from "../analytics";
1919
import { UserService } from "../user/user-service";
2020
import { SubscriptionService } from "@gitpod/gitpod-payment-endpoint/lib/accounting";
21-
import * as jwt from "jsonwebtoken";
21+
import { getExperimentsClientForBackend } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
22+
import { SessionHandlerProvider } from "../session-handler";
23+
import { AuthJWT } from "./jwt";
2224

2325
/**
2426
* The login completion handler pulls the strings between the OAuth2 flow, the ToS flow, and the session management.
@@ -31,6 +33,7 @@ export class LoginCompletionHandler {
3133
@inject(AuthProviderService) protected readonly authProviderService: AuthProviderService;
3234
@inject(UserService) protected readonly userService: UserService;
3335
@inject(SubscriptionService) protected readonly subscriptionService: SubscriptionService;
36+
@inject(AuthJWT) protected readonly authJWT: AuthJWT;
3437

3538
async complete(
3639
request: express.Request,
@@ -94,14 +97,25 @@ export class LoginCompletionHandler {
9497
);
9598
}
9699

97-
const jwt = await newSessionJWT(user.id);
100+
const isJWTCookieExperimentEnabled = await getExperimentsClientForBackend().getValueAsync(
101+
"jwtSessionCookieEnabled",
102+
false,
103+
{
104+
user: user,
105+
},
106+
);
107+
if (isJWTCookieExperimentEnabled) {
108+
const token = await this.authJWT.sign(user.id, {});
109+
110+
response.cookie(SessionHandlerProvider.getJWTCookieName(this.config.hostUrl), token, {
111+
maxAge: 7 * 24 * 60 * 60, // 7 days
112+
httpOnly: true,
113+
sameSite: "lax",
114+
secure: true,
115+
});
98116

99-
response.cookie("_gitpod_jwt_", jwt, {
100-
maxAge: 7 * 24 * 60 * 60,
101-
httpOnly: true,
102-
sameSite: "lax",
103-
secure: true,
104-
});
117+
reportJWTCookieIssued();
118+
}
105119

106120
response.redirect(returnTo);
107121
}
@@ -129,22 +143,3 @@ export namespace LoginCompletionHandler {
129143
elevateScopes?: string[];
130144
}
131145
}
132-
133-
export async function newSessionJWT(userID: string): Promise<string> {
134-
const payload = {
135-
// subject
136-
sub: userID,
137-
// issuer
138-
iss: "gitpod.io",
139-
};
140-
const temporaryTestKeyForExperimentation = "my-secret";
141-
142-
return new Promise((resolve, reject) => {
143-
jwt.sign(payload, temporaryTestKeyForExperimentation, { algorithm: "HS256" }, function (err, token) {
144-
if (err || !token) {
145-
return reject(err);
146-
}
147-
return resolve(token);
148-
});
149-
});
150-
}

components/server/src/container-module.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ import { APIUserService } from "./api/user";
117117
import { APITeamsService } from "./api/teams";
118118
import { API } from "./api/server";
119119
import { LinkedInService } from "./linkedin-service";
120+
import { AuthJWT } from "./auth/jwt";
120121

121122
export const productionContainerModule = new ContainerModule((bind, unbind, isBound, rebind) => {
122123
bind(Config).toConstantValue(ConfigFile.fromFile());
@@ -324,4 +325,6 @@ export const productionContainerModule = new ContainerModule((bind, unbind, isBo
324325
bind(APIUserService).toSelf().inSingletonScope();
325326
bind(APITeamsService).toSelf().inSingletonScope();
326327
bind(API).toSelf().inSingletonScope();
328+
329+
bind(AuthJWT).toSelf().inSingletonScope();
327330
});

components/server/src/prometheus-metrics.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export function registerServerMetrics(registry: prometheusClient.Registry) {
2525
registry.registerMetric(centralizedPermissionsValidationsTotal);
2626
registry.registerMetric(spicedbClientLatency);
2727
registry.registerMetric(dashboardErrorBoundary);
28+
registry.registerMetric(jwtCookieIssued);
2829
}
2930

3031
const loginCounter = new prometheusClient.Counter({
@@ -57,6 +58,15 @@ export function increaseApiConnectionCounter() {
5758
apiConnectionCounter.inc();
5859
}
5960

61+
const jwtCookieIssued = new prometheusClient.Counter({
62+
name: "gitpod_server_jwt_cookie_issued_total",
63+
help: "Total number of JWT cookies issued for login sessions",
64+
});
65+
66+
export function reportJWTCookieIssued() {
67+
jwtCookieIssued.inc();
68+
}
69+
6070
const apiConnectionClosedCounter = new prometheusClient.Counter({
6171
name: "gitpod_server_api_connections_closed_total",
6272
help: "Total amount of closed API connections",

components/server/src/session-handler.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const MySQLStore = mysqlstore(session);
1515
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
1616
import { Config as DBConfig } from "@gitpod/gitpod-db/lib/config";
1717
import { Config } from "./config";
18+
import { GitpodHostUrl } from "@gitpod/gitpod-protocol/lib/util/gitpod-host-url";
1819

1920
@injectable()
2021
export class SessionHandlerProvider {
@@ -72,6 +73,14 @@ export class SessionHandlerProvider {
7273
.replace(/[\W_]+/g, "_");
7374
}
7475

76+
static getJWTCookieName(hostURL: GitpodHostUrl) {
77+
const derived = hostURL
78+
.toString()
79+
.replace(/https?/, "")
80+
.replace(/[\W_]+/g, "_");
81+
return `${derived}jwt_`;
82+
}
83+
7584
public clearSessionCookie(res: express.Response, config: Config): void {
7685
// http://expressjs.com/en/api.html#res.clearCookie
7786
const name = SessionHandlerProvider.getCookieName(config);

0 commit comments

Comments
 (0)