Skip to content

Commit 2e04733

Browse files
AlexTugarevroboquat
authored andcommitted
[server/iam/oidc] find/create User in OIDC flows
1 parent db16141 commit 2e04733

File tree

3 files changed

+169
-28
lines changed

3 files changed

+169
-28
lines changed
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
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+
export namespace OIDCCreateSessionPayload {
8+
export function is(payload: any): payload is OIDCCreateSessionPayload {
9+
return (
10+
typeof payload === "object" &&
11+
"idToken" in payload &&
12+
"claims" in payload &&
13+
"iss" in payload.claims &&
14+
"sub" in payload.claims &&
15+
"name" in payload.claims &&
16+
"email" in payload.claims
17+
);
18+
}
19+
20+
export function validate(payload: OIDCCreateSessionPayload) {
21+
if (isEmpty(payload.claims.iss)) {
22+
throw new Error("Issuer is missing");
23+
}
24+
if (isEmpty(payload.claims.sub)) {
25+
throw new Error("Subject is missing");
26+
}
27+
if (isEmpty(payload.claims.name)) {
28+
throw new Error("Name is missing");
29+
}
30+
if (isEmpty(payload.claims.email)) {
31+
throw new Error("Email is missing");
32+
}
33+
}
34+
35+
function isEmpty(attribute: any) {
36+
return typeof attribute !== "string" || attribute.length < 1;
37+
}
38+
}
39+
40+
export interface OIDCCreateSessionPayload {
41+
idToken: {
42+
Issuer: string; // "https://accounts.google.com",
43+
Audience: string[];
44+
Subject: string; // "1234567890",
45+
Expiry: string; // "2023-01-10T12:00:00Z",
46+
IssuedAt: string; // "2023-01-01T12:00:00Z",
47+
};
48+
claims: {
49+
aud: string;
50+
email: string;
51+
email_verified: true;
52+
family_name: string;
53+
given_name: string;
54+
hd?: string; // accepted domain, e.g. "gitpod.io"
55+
iss: string; // "https://accounts.google.com"
56+
locale: string; // e.g. "de"
57+
name: string;
58+
picture: string; // URL of avatar
59+
sub: string; // "1234567890"
60+
};
61+
}

components/server/src/iam/iam-session-app.spec.ts

Lines changed: 73 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import * as session from "express-session";
1818
import * as request from "supertest";
1919

2020
import * as chai from "chai";
21+
import { OIDCCreateSessionPayload } from "./iam-oidc-create-session-payload";
2122
const expect = chai.expect;
2223

2324
@suite(timeout(10000))
@@ -27,6 +28,38 @@ class TestIamSessionApp {
2728

2829
protected cookieName = "test-session-name";
2930

31+
protected knownSub = "111";
32+
33+
protected userServiceMock: Partial<UserService> = {
34+
createUser: (params) => {
35+
return { id: "id-new-user" } as any;
36+
},
37+
38+
findUserForLogin: (params) => {
39+
if (params.candidate?.authId === this.knownSub) {
40+
return { id: "id-known-user" } as any;
41+
}
42+
return undefined;
43+
},
44+
};
45+
46+
protected payload: OIDCCreateSessionPayload = {
47+
idToken: {} as any,
48+
claims: {
49+
aud: "1234",
50+
51+
email_verified: true,
52+
family_name: "User",
53+
given_name: "Test",
54+
iss: "https://accounts.get.net",
55+
locale: "de",
56+
name: "Test User",
57+
picture: "https://cdn.get.net/users/abc23",
58+
sub: "1234567890",
59+
hd: "test.net",
60+
},
61+
};
62+
3063
public before() {
3164
const container = new Container();
3265
container.load(
@@ -35,11 +68,7 @@ class TestIamSessionApp {
3568
bind(IamSessionApp).toSelf().inSingletonScope();
3669
bind(Authenticator).toConstantValue(<any>{}); // unused
3770
bind(Config).toConstantValue(<any>{}); // unused
38-
bind(UserService).toConstantValue(<any>{
39-
createUser: () => ({
40-
id: "C0FFEE",
41-
}),
42-
});
71+
bind(UserService).toConstantValue(this.userServiceMock as any);
4372
}),
4473
);
4574
this.app = container.get(IamSessionApp);
@@ -72,22 +101,56 @@ class TestIamSessionApp {
72101
await request(this.app.create())
73102
.post("/session")
74103
.set("Content-Type", "application/json")
75-
.send(JSON.stringify(this.idToken));
104+
.send(JSON.stringify(this.payload));
76105

77106
expect(count, "sessions added").to.equal(1);
78107
}
79108

80-
@test public async testSessionRequestResponsesWithSetCookie() {
109+
@test public async testSessionRequestResponsesWithSetCookie_createUser() {
81110
const result = await request(this.app.create())
82111
.post("/session")
83112
.set("Content-Type", "application/json")
84-
.send(JSON.stringify(this.idToken));
113+
.send(JSON.stringify(this.payload));
85114

86-
expect(result.statusCode).to.equal(200);
115+
expect(result.statusCode, JSON.stringify(result.body)).to.equal(200);
116+
expect(result.body?.userId).to.equal("id-new-user");
87117
expect(JSON.stringify(result.get("Set-Cookie"))).to.contain(this.cookieName);
88118
}
89119

90-
idToken = {};
120+
@test public async testSessionRequestResponsesWithSetCookie_knownUser() {
121+
const payload = { ...this.payload };
122+
payload.claims.sub = this.knownSub;
123+
const result = await request(this.app.create())
124+
.post("/session")
125+
.set("Content-Type", "application/json")
126+
.send(JSON.stringify(payload));
127+
128+
expect(result.statusCode, JSON.stringify(result.body)).to.equal(200);
129+
expect(result.body?.userId).to.equal("id-known-user");
130+
expect(JSON.stringify(result.get("Set-Cookie"))).to.contain(this.cookieName);
131+
}
132+
133+
@test public async testInvalidPayload() {
134+
const cases = [
135+
{ claims: { sub: "" }, expectedMessage: "Subject is missing" },
136+
{ claims: { iss: "" }, expectedMessage: "Issuer is missing" },
137+
{ claims: { email: "" }, expectedMessage: "Email is missing" },
138+
{ claims: { name: "" }, expectedMessage: "Name is missing" },
139+
];
140+
for (const c of cases) {
141+
const payload = { ...this.payload };
142+
payload.claims = { ...payload.claims, ...c.claims };
143+
144+
const sr = request(this.app.create());
145+
const result = await sr
146+
.post("/session")
147+
.set("Content-Type", "application/json")
148+
.send(JSON.stringify(payload));
149+
150+
expect(result.statusCode, JSON.stringify(result.body)).to.equal(400);
151+
expect(result.body?.message).to.equal(c.expectedMessage);
152+
}
153+
}
91154
}
92155

93156
module.exports = new TestIamSessionApp();

components/server/src/iam/iam-session-app.ts

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import * as express from "express";
99
import { SessionHandlerProvider } from "../session-handler";
1010
import { Authenticator } from "../auth/authenticator";
1111
import { UserService } from "../user/user-service";
12+
import { OIDCCreateSessionPayload } from "./iam-oidc-create-session-payload";
1213

1314
@injectable()
1415
export class IamSessionApp {
@@ -31,34 +32,46 @@ export class IamSessionApp {
3132

3233
app.post("/session", async (req: express.Request, res: express.Response) => {
3334
try {
34-
await this.doCreateSession(req);
35-
res.status(200).json();
35+
const result = await this.doCreateSession(req);
36+
res.status(200).json(result);
3637
} catch (error) {
37-
res.status(500).json({ error, message: error.message });
38+
// we treat all errors as bad request here and forward the error message to the caller
39+
res.status(400).json({ error, message: error.message });
3840
}
3941
});
4042

4143
return app;
4244
}
4345

4446
protected async doCreateSession(req: express.Request) {
45-
// We need an account to sign in, which is done by calling `req.login` below.
46-
// As proper account creation/selection will be added in later on, we're creating a
47-
// dummy account on each attempt.
48-
//
49-
const user = await this.userService.createUser({
50-
identity: {
51-
authId: "fake-id-" + Date.now(),
52-
authName: "FakeUser",
53-
authProviderId: "oidc1",
54-
primaryEmail: "[email protected]",
55-
},
56-
userUpdate: (user) => {
57-
user.name = "FakeUser";
58-
user.fullName = "Fake User";
59-
user.avatarUrl = "https://github.com/github.png";
47+
if (!OIDCCreateSessionPayload.is(req.body)) {
48+
throw new Error("Unexpected payload.");
49+
}
50+
const payload = req.body;
51+
OIDCCreateSessionPayload.validate(payload);
52+
const claims = payload.claims;
53+
54+
const existingUser = await this.userService.findUserForLogin({
55+
candidate: {
56+
authId: claims.sub,
57+
authProviderId: claims.iss,
58+
authName: claims.name,
6059
},
6160
});
61+
const user =
62+
existingUser ||
63+
(await this.userService.createUser({
64+
identity: {
65+
authId: claims.sub,
66+
authProviderId: claims.iss,
67+
authName: claims.name,
68+
primaryEmail: claims.email,
69+
},
70+
userUpdate: (user) => {
71+
user.name = claims.name;
72+
user.avatarUrl = claims.picture;
73+
},
74+
}));
6275

6376
await new Promise<void>((resolve, reject) => {
6477
req.login(user, (err) => {
@@ -69,5 +82,9 @@ export class IamSessionApp {
6982
}
7083
});
7184
});
85+
return {
86+
sessionId: req.sessionID,
87+
userId: user.id,
88+
};
7289
}
7390
}

0 commit comments

Comments
 (0)