Skip to content

Commit 38f02bd

Browse files
authored
[server] JWT cookie management tests (#18966)
1 parent b1a30dd commit 38f02bd

File tree

5 files changed

+274
-71
lines changed

5 files changed

+274
-71
lines changed

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

Lines changed: 2 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -8,30 +8,17 @@ import { suite, test } from "@testdeck/mocha";
88
import { AuthJWT, sign, verify } from "./jwt";
99
import { Container } from "inversify";
1010
import { Config } from "../config";
11-
import * as crypto from "crypto";
1211
import * as chai from "chai";
12+
import { mockAuthConfig } from "../test/service-testing-container-module";
1313

1414
const expect = chai.expect;
1515

1616
@suite()
1717
class TestAuthJWT {
1818
private container: Container;
1919

20-
private signingKeyPair = crypto.generateKeyPairSync("rsa", { modulusLength: 2048 });
21-
private validatingKeyPair1 = crypto.generateKeyPairSync("rsa", { modulusLength: 2048 });
22-
private validatingKeyPair2 = crypto.generateKeyPairSync("rsa", { modulusLength: 2048 });
23-
2420
private config: Config = {
25-
auth: {
26-
pki: {
27-
signing: toKeyPair("0001", this.signingKeyPair),
28-
validating: [toKeyPair("0002", this.validatingKeyPair1), toKeyPair("0003", this.validatingKeyPair2)],
29-
},
30-
session: {
31-
issuer: "https://mp-server-d7650ec945.preview.gitpod-dev.com",
32-
lifetimeSeconds: 7 * 24 * 60 * 60,
33-
},
34-
},
21+
auth: mockAuthConfig,
3522
} as Config;
3623

3724
async before() {
@@ -90,29 +77,4 @@ class TestAuthJWT {
9077
}
9178
}
9279

93-
function toKeyPair(
94-
id: string,
95-
kp: crypto.KeyPairKeyObjectResult,
96-
): {
97-
id: string;
98-
privateKey: string;
99-
publicKey: string;
100-
} {
101-
return {
102-
id,
103-
privateKey: kp.privateKey
104-
.export({
105-
type: "pkcs1",
106-
format: "pem",
107-
})
108-
.toString(),
109-
publicKey: kp.publicKey
110-
.export({
111-
type: "pkcs1",
112-
format: "pem",
113-
})
114-
.toString(),
115-
};
116-
}
117-
11880
module.exports = new TestAuthJWT();

components/server/src/config.ts

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,28 +37,30 @@ export type Config = Omit<
3737
credentialsPath: string;
3838
};
3939

40-
auth: {
41-
// Public/Private key for signing authenticated sessions
42-
pki: {
43-
signing: {
44-
id: string;
45-
privateKey: string;
46-
publicKey: string;
47-
};
48-
validating: {
49-
id: string;
50-
privateKey: string;
51-
publicKey: string;
52-
}[];
53-
};
40+
auth: AuthConfig;
41+
};
5442

55-
session: {
56-
lifetimeSeconds: number;
57-
issuer: string;
58-
cookie: CookieConfig;
43+
export interface AuthConfig {
44+
// Public/Private key for signing authenticated sessions
45+
pki: {
46+
signing: {
47+
id: string;
48+
privateKey: string;
49+
publicKey: string;
5950
};
51+
validating: {
52+
id: string;
53+
privateKey: string;
54+
publicKey: string;
55+
}[];
6056
};
61-
};
57+
58+
session: {
59+
lifetimeSeconds: number;
60+
issuer: string;
61+
cookie: CookieConfig;
62+
};
63+
}
6264

6365
export interface WorkspaceDefaults {
6466
workspaceImage: string;
Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
/**
2+
* Copyright (c) 2023 Gitpod GmbH. All rights reserved.
3+
* Licensed under the GNU Affero General Public License (AGPL).
4+
* See License.AGPL.txt in the project root for license information.
5+
*/
6+
7+
import { TypeORM, resetDB } from "@gitpod/gitpod-db/lib";
8+
import { User } from "@gitpod/gitpod-protocol";
9+
import { Deferred } from "@gitpod/gitpod-protocol/lib/util/deferred";
10+
import { expect } from "chai";
11+
import * as express from "express";
12+
import { Container } from "inversify";
13+
import { v4 } from "uuid";
14+
import { SessionHandler } from "./session-handler";
15+
import { createTestContainer } from "./test/service-testing-container-module";
16+
import { UserService } from "./user/user-service";
17+
18+
describe("SessionHandler", () => {
19+
let container: Container;
20+
let sessionHandler: SessionHandler;
21+
let existingUser: User;
22+
let jwtSessionHandler: express.Handler;
23+
interface Response {
24+
status?: number;
25+
value?: string;
26+
cookie?: string;
27+
}
28+
const handle = async (user?: User, cookie?: string): Promise<Response> => {
29+
const deferred = new Deferred<Response>();
30+
const result: Response = {
31+
status: undefined,
32+
value: undefined,
33+
cookie: undefined,
34+
};
35+
jwtSessionHandler(
36+
{ user, headers: { cookie } } as express.Request,
37+
{
38+
status: function (statusCode: number) {
39+
result.status = statusCode;
40+
return this;
41+
},
42+
send: function (value: string) {
43+
result.value = value;
44+
deferred.resolve(result);
45+
},
46+
cookie: function (name: string, value: string, opts: express.CookieOptions) {
47+
result.cookie = `${name}=${value}; `;
48+
},
49+
} as any as express.Response,
50+
() => {},
51+
);
52+
return await deferred.promise;
53+
};
54+
55+
beforeEach(async () => {
56+
container = createTestContainer();
57+
sessionHandler = container.get(SessionHandler);
58+
jwtSessionHandler = sessionHandler.jwtSessionConvertor();
59+
const userService = container.get(UserService);
60+
// insert some users to the DB to reproduce INC-379
61+
await userService.createUser({
62+
identity: {
63+
authName: "github",
64+
authProviderId: "github",
65+
authId: "1234",
66+
},
67+
});
68+
existingUser = await userService.createUser({
69+
identity: {
70+
authName: "github",
71+
authProviderId: "github",
72+
authId: "1234",
73+
},
74+
});
75+
});
76+
77+
afterEach(async () => {
78+
const typeorm = container.get(TypeORM);
79+
await resetDB(typeorm);
80+
});
81+
82+
describe("verify", () => {
83+
it("should return undefined for an empty cookie", async () => {
84+
const user = await sessionHandler.verify("");
85+
expect(user).to.be.undefined;
86+
});
87+
it("should return undefined for an invalid cookie", async () => {
88+
const user = await sessionHandler.verify("invalid");
89+
expect(user).to.be.undefined;
90+
});
91+
it("should return the user for a valid JWT with correct 'sub' claim", async () => {
92+
const cookie = await sessionHandler.createJWTSessionCookie(existingUser.id);
93+
const user = await sessionHandler.verify(`${cookie.name}=${cookie.value}`);
94+
expect(user?.id).to.be.equal(existingUser.id);
95+
});
96+
it("should return undefined for a valid JWT with incorrect 'sub' claim", async () => {
97+
const unexisingUserId = v4();
98+
const cookie = await sessionHandler.createJWTSessionCookie(unexisingUserId);
99+
const user = await sessionHandler.verify(`${cookie.name}=${cookie.value}`);
100+
expect(user).to.be.undefined;
101+
});
102+
});
103+
describe("createJWTSessionCookie", () => {
104+
it("should create a valid JWT token with correct attributes and cookie options", async () => {
105+
const maxAge = 7 * 24 * 60 * 60;
106+
const issuedAfter = Math.floor(new Date().getTime() / 1000);
107+
const expiresAfter = issuedAfter + maxAge;
108+
const { name, value: token, opts } = await sessionHandler.createJWTSessionCookie("1");
109+
110+
const decoded = await sessionHandler.verifyJWTCookie(`${name}=${token}`);
111+
expect(decoded, "Verify the JWT is valid").to.not.be.null;
112+
113+
expect(decoded!.sub).to.equal("1", "Check the 'sub' claim");
114+
expect(decoded!.iat || 0).to.be.greaterThanOrEqual(issuedAfter, "Check the 'iat' claim");
115+
expect(decoded!.exp || 0).to.be.greaterThanOrEqual(expiresAfter, "Check the 'exp' claim");
116+
117+
// Check cookie options
118+
expect(opts.httpOnly).to.equal(true);
119+
expect(opts.secure).to.equal(true);
120+
expect(opts.maxAge).to.equal(maxAge * 1000);
121+
expect(opts.sameSite).to.equal("strict");
122+
123+
expect(name, "Check cookie name").to.equal("_gitpod_dev_jwt_");
124+
});
125+
});
126+
describe("jwtSessionConvertor", () => {
127+
it("user is not authenticated", async () => {
128+
const res = await handle();
129+
expect(res.status).to.equal(401);
130+
expect(res.value).to.equal("User has no valid session.");
131+
expect(res.cookie).to.be.undefined;
132+
});
133+
it("JWT cookie is not present", async () => {
134+
const res = await handle(existingUser);
135+
expect(res.status).to.equal(200);
136+
expect(res.value).to.equal("New JWT cookie issued.");
137+
expect(res.cookie).not.to.be.undefined;
138+
});
139+
it("JWT cookie is present and valid", async () => {
140+
const cookie = await sessionHandler.createJWTSessionCookie(existingUser.id);
141+
const res = await handle(existingUser, `${cookie.name}=${cookie.value}`);
142+
expect(res.status).to.equal(200);
143+
expect(res.value).to.equal("User session already has a valid JWT session.");
144+
expect(res.cookie).to.be.undefined;
145+
});
146+
it("JWT cookie is present and refreshed", async () => {
147+
const cookieToRefresh = await sessionHandler.createJWTSessionCookie(existingUser.id, {
148+
issuedAtMs: Date.now() - SessionHandler.JWT_REFRESH_THRESHOLD - 1,
149+
});
150+
const res = await handle(existingUser, `${cookieToRefresh.name}=${cookieToRefresh.value}`);
151+
expect(res.status).to.equal(200);
152+
expect(res.value).to.equal("Refreshed JWT cookie issued.");
153+
expect(res.cookie).not.to.be.undefined;
154+
});
155+
it("JWT cookie is present and expired", async () => {
156+
const expiredCookie = await sessionHandler.createJWTSessionCookie(existingUser.id, {
157+
expirySeconds: 0,
158+
});
159+
const res = await handle(existingUser, `${expiredCookie.name}=${expiredCookie.value}`);
160+
expect(res.status).to.equal(401);
161+
expect(res.value).to.equal("JWT Session is invalid");
162+
expect(res.cookie).to.be.undefined;
163+
});
164+
it("JWT cookie is present but invalid", async () => {
165+
const res = await handle(existingUser, "_gitpod_dev_jwt_=invalid");
166+
expect(res.status).to.equal(401);
167+
expect(res.value).to.equal("JWT Session is invalid");
168+
expect(res.cookie).to.be.undefined;
169+
});
170+
});
171+
});

components/server/src/session-handler.ts

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,12 @@ import { Config } from "./config";
1515
import { WsNextFunction, WsRequestHandler } from "./express/ws-handler";
1616
import { reportJWTCookieIssued } from "./prometheus-metrics";
1717
import { UserService } from "./user/user-service";
18+
import { JwtPayload } from "jsonwebtoken";
1819

1920
@injectable()
2021
export class SessionHandler {
22+
static JWT_REFRESH_THRESHOLD = 60 * 60 * 1000; // 1 hour
23+
2124
@inject(Config) protected readonly config: Config;
2225
@inject(AuthJWT) protected readonly authJWT: AuthJWT;
2326
@inject(UserService) protected userService: UserService;
@@ -48,10 +51,9 @@ export class SessionHandler {
4851

4952
const issuedAtMs = (decoded.iat || 0) * 1000;
5053
const now = new Date();
51-
const thresholdMs = 60 * 60 * 1000; // 1 hour
5254

5355
// Was the token issued more than threshold ago?
54-
if (issuedAtMs + thresholdMs < now.getTime()) {
56+
if (issuedAtMs + SessionHandler.JWT_REFRESH_THRESHOLD < now.getTime()) {
5557
// issue a new one, to refresh it
5658
const cookie = await this.createJWTSessionCookie(user.id);
5759
res.cookie(cookie.name, cookie.value, cookie.opts);
@@ -103,15 +105,11 @@ export class SessionHandler {
103105
}
104106

105107
async verify(cookie: string): Promise<User | undefined> {
106-
const cookies = parseCookieHeader(cookie);
107-
const jwtToken = cookies[this.getJWTCookieName(this.config)];
108-
if (!jwtToken) {
109-
log.debug("No JWT session present on request");
110-
return undefined;
111-
}
112108
try {
113-
const claims = await this.authJWT.verify(jwtToken);
114-
log.debug("JWT Session token verified", { claims });
109+
const claims = await this.verifyJWTCookie(cookie);
110+
if (!claims) {
111+
return undefined;
112+
}
115113

116114
const subject = claims.sub;
117115
if (!subject) {
@@ -126,10 +124,31 @@ export class SessionHandler {
126124
}
127125
}
128126

127+
async verifyJWTCookie(cookie: string): Promise<JwtPayload | undefined> {
128+
const cookies = parseCookieHeader(cookie);
129+
const jwtToken = cookies[this.getJWTCookieName(this.config)];
130+
if (!jwtToken) {
131+
log.debug("No JWT session present on request");
132+
return undefined;
133+
}
134+
const claims = await this.authJWT.verify(jwtToken);
135+
log.debug("JWT Session token verified", { claims });
136+
return claims;
137+
}
138+
129139
public async createJWTSessionCookie(
130140
userID: string,
141+
// only for testing
142+
options?: {
143+
issuedAtMs?: number;
144+
expirySeconds?: number;
145+
},
131146
): Promise<{ name: string; value: string; opts: express.CookieOptions }> {
132-
const token = await this.authJWT.sign(userID, {});
147+
const payload = {};
148+
if (options?.issuedAtMs) {
149+
Object.assign(payload, { iat: Math.floor(options.issuedAtMs / 1000) });
150+
}
151+
const token = await this.authJWT.sign(userID, payload, options?.expirySeconds);
133152

134153
return {
135154
name: this.getJWTCookieName(this.config),

0 commit comments

Comments
 (0)