Skip to content

Commit 73a98e7

Browse files
svenefftingesvenefftinge
authored
[server] run relationship updater on findUserById (#18394)
1 parent 60cea25 commit 73a98e7

File tree

9 files changed

+454
-85
lines changed

9 files changed

+454
-85
lines changed

‎components/server/src/authorization/authorizer.spec.db.ts‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
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+
import { v1 } from "@authzed/authzed-node";
7+
import { TypeORM } from "@gitpod/gitpod-db/lib";
8+
import { resetDB } from "@gitpod/gitpod-db/lib/test/reset-db";
9+
import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
10+
import * as chai from "chai";
11+
import { Container } from "inversify";
12+
import "mocha";
13+
import { createTestContainer } from "../test/service-testing-container-module";
14+
import { Authorizer } from "./authorizer";
15+
import { rel } from "./definitions";
16+
import { v4 } from "uuid";
17+
18+
const expect = chai.expect;
19+
20+
describe("Authorizer", async () => {
21+
let container: Container;
22+
let authorizer: Authorizer;
23+
24+
beforeEach(async () => {
25+
container = createTestContainer();
26+
Experiments.configureTestingClient({
27+
centralizedPermissions: true,
28+
});
29+
authorizer = container.get<Authorizer>(Authorizer);
30+
});
31+
32+
afterEach(async () => {
33+
// Clean-up database
34+
await resetDB(container.get(TypeORM));
35+
});
36+
37+
it("should removeUser", async () => {
38+
const userId = v4();
39+
await authorizer.addUser(userId);
40+
await expected(rel.user(userId).installation.installation);
41+
await expected(rel.user(userId).self.user(userId));
42+
await expected(rel.installation.member.user(userId));
43+
44+
await authorizer.removeUser(userId);
45+
await notExpected(rel.user(userId).installation.installation);
46+
await notExpected(rel.user(userId).self.user(userId));
47+
await notExpected(rel.installation.member.user(userId));
48+
});
49+
50+
it("should addUser", async () => {
51+
const userId = v4();
52+
await notExpected(rel.user(userId).installation.installation);
53+
await notExpected(rel.user(userId).self.user(userId));
54+
await notExpected(rel.installation.member.user(userId));
55+
56+
await authorizer.addUser(userId);
57+
58+
await expected(rel.user(userId).installation.installation);
59+
await expected(rel.user(userId).self.user(userId));
60+
await expected(rel.installation.member.user(userId));
61+
62+
// add user to org
63+
const org1Id = v4();
64+
await authorizer.addUser(userId, org1Id);
65+
66+
await notExpected(rel.user(userId).installation.installation);
67+
await notExpected(rel.installation.member.user(userId));
68+
await expected(rel.user(userId).self.user(userId));
69+
await expected(rel.user(userId).organization.organization(org1Id));
70+
71+
// add user to another org
72+
const org2Id = v4();
73+
await authorizer.addUser(userId, org2Id);
74+
75+
await notExpected(rel.user(userId).installation.installation);
76+
await notExpected(rel.installation.member.user(userId));
77+
await notExpected(rel.user(userId).organization.organization(org1Id));
78+
await expected(rel.user(userId).self.user(userId));
79+
await expected(rel.user(userId).organization.organization(org2Id));
80+
81+
// back to installation
82+
await authorizer.addUser(userId);
83+
84+
await notExpected(rel.user(userId).organization.organization(org1Id));
85+
await notExpected(rel.user(userId).organization.organization(org2Id));
86+
87+
await expected(rel.user(userId).installation.installation);
88+
await expected(rel.user(userId).self.user(userId));
89+
await expected(rel.installation.member.user(userId));
90+
});
91+
92+
it("should addOrganization", async () => {
93+
const orgId = v4();
94+
await notExpected(rel.organization(orgId).installation.installation);
95+
96+
// add org with members and projects
97+
const u1 = v4();
98+
const u2 = v4();
99+
const p1 = v4();
100+
const p2 = v4();
101+
await authorizer.addOrganization(
102+
orgId,
103+
[
104+
{ userId: u1, role: "member" },
105+
{ userId: u2, role: "owner" },
106+
],
107+
[p1, p2],
108+
);
109+
110+
await expected(rel.organization(orgId).installation.installation);
111+
await expected(rel.organization(orgId).member.user(u1));
112+
await expected(rel.organization(orgId).member.user(u2));
113+
await expected(rel.organization(orgId).owner.user(u2));
114+
await expected(rel.project(p1).org.organization(orgId));
115+
await expected(rel.project(p2).org.organization(orgId));
116+
117+
// add org again with different members and projects
118+
await authorizer.addOrganization(orgId, [{ userId: u2, role: "member" }], [p2]);
119+
await expected(rel.organization(orgId).installation.installation);
120+
await notExpected(rel.organization(orgId).member.user(u1));
121+
await expected(rel.organization(orgId).member.user(u2));
122+
await notExpected(rel.organization(orgId).owner.user(u2));
123+
await notExpected(rel.project(p1).org.organization(orgId));
124+
await expected(rel.project(p2).org.organization(orgId));
125+
});
126+
127+
async function expected(relation: v1.Relationship): Promise<void> {
128+
const rs = await authorizer.find(relation);
129+
const message = async () => {
130+
const expected = JSON.stringify(relation);
131+
relation.subject = undefined;
132+
const result = await authorizer.find(relation);
133+
return `Expected ${expected} to be present, but it was not. Found ${JSON.stringify(result)}`;
134+
};
135+
expect(rs, await message()).to.not.be.undefined;
136+
}
137+
138+
async function notExpected(relation: v1.Relationship): Promise<void> {
139+
const rs = await authorizer.find(relation);
140+
expect(rs).to.be.undefined;
141+
}
142+
});

‎components/server/src/authorization/authorizer.ts‎

Lines changed: 92 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import { v1 } from "@authzed/authzed-node";
88

99
import { BUILTIN_INSTLLATION_ADMIN_USER_ID } from "@gitpod/gitpod-db/lib";
10-
import { Organization, Project, TeamMemberInfo, TeamMemberRole } from "@gitpod/gitpod-protocol";
10+
import { TeamMemberRole } from "@gitpod/gitpod-protocol";
1111
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
1212
import {
1313
OrganizationPermission,
@@ -157,14 +157,34 @@ export class Authorizer {
157157
}
158158

159159
async addUser(userId: string, owningOrgId?: string) {
160-
await this.authorizer.writeRelationships(
161-
set(rel.user(userId).self.user(userId)), //
162-
set(
163-
owningOrgId
164-
? rel.user(userId).organization.organization(owningOrgId)
165-
: rel.user(userId).installation.installation,
166-
),
160+
const oldOrgs = await this.findAll(rel.user(userId).organization.organization(""));
161+
const updates = [set(rel.user(userId).self.user(userId))];
162+
updates.push(
163+
...oldOrgs
164+
.map((r) => r.subject?.object?.objectId)
165+
.filter((orgId) => !!orgId && orgId !== owningOrgId)
166+
.map((orgId) => remove(rel.user(userId).organization.organization(orgId!))),
167167
);
168+
169+
if (owningOrgId) {
170+
updates.push(
171+
set(rel.user(userId).organization.organization(owningOrgId)), //
172+
remove(rel.user(userId).installation.installation),
173+
remove(rel.installation.member.user(userId)),
174+
remove(rel.installation.admin.user(userId)),
175+
);
176+
} else {
177+
updates.push(
178+
set(rel.user(userId).installation.installation), //
179+
set(rel.installation.member.user(userId)),
180+
);
181+
}
182+
183+
await this.authorizer.writeRelationships(...updates);
184+
}
185+
186+
async removeUser(userId: string) {
187+
await this.removeAllRelationships("user", userId);
168188
}
169189

170190
async addOrganizationRole(orgID: string, userID: string, role: TeamMemberRole): Promise<void> {
@@ -197,30 +217,45 @@ export class Authorizer {
197217
);
198218
}
199219

200-
async addOrganization(org: Organization, members: TeamMemberInfo[], projects: Project[]): Promise<void> {
201-
for (const member of members) {
202-
await this.addOrganizationRole(org.id, member.userId, member.role);
203-
}
220+
async addOrganization(
221+
orgId: string,
222+
members: { userId: string; role: TeamMemberRole }[],
223+
projectIds: string[],
224+
): Promise<void> {
225+
await this.addOrganizationMembers(orgId, members);
204226

205-
for (const project of projects) {
206-
await this.addProjectToOrg(org.id, project.id);
207-
}
227+
await this.addOrganizationProjects(orgId, projectIds);
208228

209229
await this.authorizer.writeRelationships(
210-
set(rel.organization(org.id).installation.installation), //
230+
set(rel.organization(orgId).installation.installation), //
211231
);
212232
}
213233

214-
async addInstallationMemberRole(userID: string) {
215-
await this.authorizer.writeRelationships(
216-
set(rel.installation.member.user(userID)), //
217-
);
234+
private async addOrganizationProjects(orgID: string, projectIds: string[]): Promise<void> {
235+
const existing = await this.findAll(rel.project("").org.organization(orgID));
236+
const toBeRemoved = asSet(existing.map((r) => r.resource?.objectId));
237+
for (const projectId of projectIds) {
238+
await this.addProjectToOrg(orgID, projectId);
239+
toBeRemoved.delete(projectId);
240+
}
241+
for (const projectId of toBeRemoved) {
242+
await this.removeProjectFromOrg(orgID, projectId);
243+
}
218244
}
219245

220-
async removeInstallationMemberRole(userID: string) {
221-
await this.authorizer.writeRelationships(
222-
remove(rel.installation.member.user(userID)), //
223-
);
246+
private async addOrganizationMembers(
247+
orgID: string,
248+
members: { userId: string; role: TeamMemberRole }[],
249+
): Promise<void> {
250+
const existing = await this.findAll(rel.organization(orgID).member.user(""));
251+
const toBeRemoved = asSet(existing.map((r) => r.subject?.object?.objectId));
252+
for (const member of members) {
253+
await this.addOrganizationRole(orgID, member.userId, member.role);
254+
toBeRemoved.delete(member.userId);
255+
}
256+
for (const userId of toBeRemoved) {
257+
await this.removeOrganizationRole(orgID, userId, "member");
258+
}
224259
}
225260

226261
async addInstallationAdminRole(userID: string) {
@@ -258,6 +293,27 @@ export class Authorizer {
258293
}
259294
return relationships[0].relationship;
260295
}
296+
297+
async findAll(relation: v1.Relationship): Promise<v1.Relationship[]> {
298+
const relationships = await this.authorizer.readRelationships({
299+
consistency: v1.Consistency.create({
300+
requirement: {
301+
oneofKind: "fullyConsistent",
302+
fullyConsistent: true,
303+
},
304+
}),
305+
relationshipFilter: {
306+
resourceType: relation.resource?.objectType || "",
307+
optionalResourceId: relation.resource?.objectId || "",
308+
optionalRelation: relation.relation,
309+
optionalSubjectFilter: relation.subject?.object && {
310+
subjectType: relation.subject.object.objectType,
311+
optionalSubjectId: relation.subject.object.objectId,
312+
},
313+
},
314+
});
315+
return relationships.map((r) => r.relationship!);
316+
}
261317
}
262318

263319
function set(rs: v1.Relationship): v1.RelationshipUpdate {
@@ -274,14 +330,14 @@ function remove(rs: v1.Relationship): v1.RelationshipUpdate {
274330
});
275331
}
276332

277-
function object(type: ResourceType, id: string): v1.ObjectReference {
333+
function object(type: ResourceType, id?: string): v1.ObjectReference {
278334
return v1.ObjectReference.create({
279335
objectId: id,
280336
objectType: type,
281337
});
282338
}
283339

284-
function subject(type: ResourceType, id: string, relation?: Relation | Permission): v1.SubjectReference {
340+
function subject(type: ResourceType, id?: string, relation?: Relation | Permission): v1.SubjectReference {
285341
return v1.SubjectReference.create({
286342
object: object(type, id),
287343
optionalRelation: relation,
@@ -294,3 +350,13 @@ const consistency = v1.Consistency.create({
294350
fullyConsistent: true,
295351
},
296352
});
353+
354+
function asSet<T>(array: (T | undefined)[]): Set<T> {
355+
const result = new Set<T>();
356+
for (const r of array) {
357+
if (r) {
358+
result.add(r);
359+
}
360+
}
361+
return result;
362+
}

0 commit comments

Comments
 (0)