Skip to content

Commit a8f6a38

Browse files
committed
[server] JWT cookie management tests
1 parent 8ac11b1 commit a8f6a38

File tree

5 files changed

+273
-71
lines changed

5 files changed

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

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)