Skip to content

Commit 6a92654

Browse files
geroplgeropl
committed
[server] SpiceDB: Add request-level caching based on AsyncLocalStorage+ZedTokens
1 parent 6dcf2d8 commit 6a92654

File tree

6 files changed

+459
-19
lines changed

6 files changed

+459
-19
lines changed

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ export class Authorizer {
6969
consistency,
7070
});
7171

72-
return this.authorizer.check(req, { userId });
72+
const result = await this.authorizer.check(req, { userId });
73+
return result.permitted;
7374
}
7475

7576
async checkPermissionOnInstallation(userId: string, permission: InstallationPermission): Promise<void> {
@@ -98,7 +99,8 @@ export class Authorizer {
9899
consistency,
99100
});
100101

101-
return this.authorizer.check(req, { userId });
102+
const result = await this.authorizer.check(req, { userId });
103+
return result.permitted;
102104
}
103105

104106
async checkPermissionOnOrganization(userId: string, permission: OrganizationPermission, orgId: string) {
@@ -128,7 +130,8 @@ export class Authorizer {
128130
consistency,
129131
});
130132

131-
return this.authorizer.check(req, { userId });
133+
const result = await this.authorizer.check(req, { userId });
134+
return result.permitted;
132135
}
133136

134137
async checkPermissionOnProject(userId: string, permission: ProjectPermission, projectId: string) {
@@ -158,7 +161,8 @@ export class Authorizer {
158161
consistency,
159162
});
160163

161-
return this.authorizer.check(req, { userId });
164+
const result = await this.authorizer.check(req, { userId });
165+
return result.permitted;
162166
}
163167

164168
async checkPermissionOnUser(userId: string, permission: UserPermission, resourceUserId: string) {
@@ -192,7 +196,8 @@ export class Authorizer {
192196
consistency,
193197
});
194198

195-
return this.authorizer.check(req, { userId }, forceEnablement);
199+
const result = await this.authorizer.check(req, { userId }, forceEnablement);
200+
return result.permitted;
196201
}
197202

198203
async checkPermissionOnWorkspace(userId: string, permission: WorkspacePermission, workspaceId: string) {

‎components/server/src/authorization/caching-spicedb-authorizer.spec.db.ts

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
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 { TypeORM } from "@gitpod/gitpod-db/lib";
7+
import { resetDB } from "@gitpod/gitpod-db/lib/test/reset-db";
8+
import { CommitContext, Organization, Project, User, WorkspaceConfig } from "@gitpod/gitpod-protocol";
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, SYSTEM_USER } from "./authorizer";
15+
import { OrganizationService } from "../orgs/organization-service";
16+
import { WorkspaceService } from "../workspace/workspace-service";
17+
import { UserService } from "../user/user-service";
18+
import { ZedTokenCache } from "./caching-spicedb-authorizer";
19+
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
20+
import { ConfigProvider } from "../workspace/config-provider";
21+
22+
const expect = chai.expect;
23+
24+
describe("CachingSpiceDBAuthorizer", async () => {
25+
let container: Container;
26+
let userSvc: UserService;
27+
let orgSvc: OrganizationService;
28+
let workspaceSvc: WorkspaceService;
29+
let authorizer: Authorizer;
30+
let zedTokenCache: ZedTokenCache;
31+
32+
beforeEach(async () => {
33+
container = createTestContainer();
34+
// TODO(gpl) Ideally we should be able to factor this out into the API. But to start somewhere, we'll mock it out here.
35+
container.rebind(ConfigProvider).toConstantValue({
36+
fetchConfig: () => ({
37+
config: <WorkspaceConfig>{
38+
image: "gitpod/workspace-full:latest",
39+
},
40+
}),
41+
} as any as ConfigProvider);
42+
Experiments.configureTestingClient({
43+
centralizedPermissions: true,
44+
});
45+
userSvc = container.get<UserService>(UserService);
46+
orgSvc = container.get<OrganizationService>(OrganizationService);
47+
workspaceSvc = container.get<WorkspaceService>(WorkspaceService);
48+
authorizer = container.get<Authorizer>(Authorizer);
49+
zedTokenCache = container.get<ZedTokenCache>(ZedTokenCache);
50+
});
51+
52+
afterEach(async () => {
53+
// Clean-up database
54+
await resetDB(container.get(TypeORM));
55+
56+
container.unbindAll();
57+
});
58+
59+
it("should avoid new-enemy after removal", async () => {
60+
// userB and userC are members of org1, userA is owner.
61+
// All users are installation owned.
62+
const org1 = await orgSvc.createOrganization(SYSTEM_USER, "org1");
63+
const userA = await userSvc.createUser({
64+
organizationId: undefined,
65+
identity: {
66+
authProviderId: "github",
67+
authId: "123",
68+
authName: "userA",
69+
},
70+
});
71+
await orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userA.id, "owner");
72+
const userB = await userSvc.createUser({
73+
organizationId: undefined,
74+
identity: {
75+
authProviderId: "github",
76+
authId: "456",
77+
authName: "userB",
78+
},
79+
});
80+
await orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userB.id, "member");
81+
const userC = await userSvc.createUser({
82+
organizationId: undefined,
83+
identity: {
84+
authProviderId: "github",
85+
authId: "789",
86+
authName: "userC",
87+
},
88+
});
89+
await orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userC.id, "member");
90+
91+
// userA creates a workspace when userB is still member of the org
92+
// All members have "read_info" (derived from membership)
93+
const ws1 = await createTestWorkspace(org1, userA);
94+
95+
expect(
96+
await authorizer.hasPermissionOnWorkspace(userB.id, "read_info", ws1.id),
97+
"userB should have read_info after removal",
98+
).to.be.true;
99+
expect(
100+
await authorizer.hasPermissionOnWorkspace(userA.id, "read_info", ws1.id),
101+
"userA should have read_info after removal of userB",
102+
).to.be.true;
103+
expect(
104+
await authorizer.hasPermissionOnWorkspace(userC.id, "read_info", ws1.id),
105+
"userC should have read_info after removal of userB",
106+
).to.be.true;
107+
108+
// INTERNALS
109+
async function printTokens(): Promise<{ ws1Token: string | undefined; org1Token: string | undefined }> {
110+
const ws1Token = await zedTokenCache.get({ objectType: "workspace", objectId: ws1.id });
111+
log.info("ws1Token", ws1Token);
112+
const org1Token = await zedTokenCache.get({ objectType: "organization", objectId: org1.id });
113+
log.info("org1Token", org1Token);
114+
return { ws1Token, org1Token };
115+
}
116+
const { org1Token: org1TokenT1 } = await printTokens();
117+
118+
// userB is removed from the org
119+
await orgSvc.removeOrganizationMember(SYSTEM_USER, org1.id, userB.id);
120+
121+
// INTERNALS
122+
const { org1Token: org1TokenT2 } = await printTokens();
123+
expect(org1TokenT1 === org1TokenT2 && org1TokenT1 !== undefined && org1TokenT2 !== undefined).to.be.false;
124+
125+
expect(
126+
await authorizer.hasPermissionOnWorkspace(userB.id, "read_info", ws1.id),
127+
"userB should have read_info after removal",
128+
).to.be.false;
129+
expect(
130+
await authorizer.hasPermissionOnWorkspace(userA.id, "read_info", ws1.id),
131+
"userA should have read_info after removal of userB",
132+
).to.be.true;
133+
expect(
134+
await authorizer.hasPermissionOnWorkspace(userC.id, "read_info", ws1.id),
135+
"userC should have read_info after removal of userB",
136+
).to.be.true;
137+
});
138+
139+
async function createTestWorkspace(org: Organization, owner: User, project?: Project) {
140+
const ws = await workspaceSvc.createWorkspace(
141+
{},
142+
owner,
143+
org.id,
144+
project,
145+
<CommitContext>{
146+
title: "gitpod",
147+
repository: {
148+
host: "github.com",
149+
owner: "gitpod-io",
150+
name: "gitpod",
151+
cloneUrl: "https://github.com/gitpod-io/gitpod.git",
152+
},
153+
revision: "asdf",
154+
},
155+
"github.com/gitpod-io/gitpod",
156+
);
157+
return ws;
158+
}
159+
160+
it("should avoid read-your-writes problem when adding a new user", async () => {
161+
// userB and userC are members of org1, userA is owner.
162+
// All users are installation owned.
163+
const org1 = await orgSvc.createOrganization(SYSTEM_USER, "org1");
164+
const userA = await userSvc.createUser({
165+
organizationId: undefined,
166+
identity: {
167+
authProviderId: "github",
168+
authId: "123",
169+
authName: "userA",
170+
},
171+
});
172+
await orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userA.id, "owner");
173+
const userC = await userSvc.createUser({
174+
organizationId: undefined,
175+
identity: {
176+
authProviderId: "github",
177+
authId: "789",
178+
authName: "userC",
179+
},
180+
});
181+
await orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userC.id, "member");
182+
183+
// userA creates a workspace before userB is member of the org
184+
const ws1 = await createTestWorkspace(org1, userA);
185+
186+
expect(
187+
await authorizer.hasPermissionOnWorkspace(userA.id, "read_info", ws1.id),
188+
"userA should have read_info after removal of userB",
189+
).to.be.true;
190+
expect(
191+
await authorizer.hasPermissionOnWorkspace(userC.id, "read_info", ws1.id),
192+
"userC should have read_info after removal of userB",
193+
).to.be.true;
194+
195+
// userB is added to the org
196+
const userB = await userSvc.createUser({
197+
organizationId: undefined,
198+
identity: {
199+
authProviderId: "github",
200+
authId: "456",
201+
authName: "userB",
202+
},
203+
});
204+
await orgSvc.addOrUpdateMember(SYSTEM_USER, org1.id, userB.id, "member");
205+
206+
expect(
207+
await authorizer.hasPermissionOnWorkspace(userB.id, "read_info", ws1.id),
208+
"userB should have read_info after removal",
209+
).to.be.true;
210+
expect(
211+
await authorizer.hasPermissionOnWorkspace(userA.id, "read_info", ws1.id),
212+
"userA should have read_info after removal of userB",
213+
).to.be.true;
214+
expect(
215+
await authorizer.hasPermissionOnWorkspace(userC.id, "read_info", ws1.id),
216+
"userC should have read_info after removal of userB",
217+
).to.be.true;
218+
});
219+
});

0 commit comments

Comments
 (0)