Skip to content

Commit e28a756

Browse files
authored
[server] SpiceDB: Add request-level caching based on AsyncLocalStorage+ZedTokens (#18893)
* [server] SpiceDB: Add request-level caching based on AsyncLocalStorage+ZedTokens * [spicedb] Generate code for decoding DecodedZedToken (internal)
1 parent 3b53a0a commit e28a756

22 files changed

+6658
-19
lines changed

components/BUILD.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ scripts:
148148
srcs:
149149
- components/**/*
150150
script: |
151-
GO_COMPONENTS=( local-app-api content-service-api image-builder-api registry-facade-api supervisor-api ws-daemon-api ws-manager-api ws-manager-bridge-api usage-api )
151+
GO_COMPONENTS=( local-app-api content-service-api image-builder-api registry-facade-api supervisor-api ws-daemon-api ws-manager-api ws-manager-bridge-api usage-api spicedb )
152152
153153
for COMPONENT in "${GO_COMPONENTS[@]}";do
154154
echo "Generating code for component $COMPONENT..."
@@ -157,7 +157,7 @@ scripts:
157157
popd > /dev/null
158158
done
159159
160-
WEB_COMPONENTS=( local-app-api/typescript-grpcweb supervisor-api/typescript-grpc supervisor-api/typescript-grpcweb )
160+
WEB_COMPONENTS=( local-app-api/typescript-grpcweb supervisor-api/typescript-grpc supervisor-api/typescript-grpcweb spicedb/typescript )
161161
for COMPONENT in "${WEB_COMPONENTS[@]}";do
162162
echo "Generating code for component $COMPONENT..."
163163
pushd $COMPONENT > /dev/null

components/server/BUILD.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ packages:
1919
- components/public-api/typescript:lib
2020
- components/gitpod-db:dbtest-init
2121
- components/spicedb:lib
22+
- components/spicedb/typescript:lib
2223
config:
2324
packaging: offline-mirror
2425
yarnLock: ${coreYarnLockBase}/yarn.lock

components/server/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
"@gitpod/supervisor-api-grpcweb": "0.1.5",
5858
"@gitpod/usage-api": "0.1.5",
5959
"@gitpod/ws-manager": "0.1.5",
60+
"@gitpod/spicedb-impl": "0.1.5",
6061
"@google-cloud/profiler": "^6.0.0",
6162
"@improbable-eng/grpc-web-node-http-transport": "^0.14.0",
6263
"@jmondi/oauth2-server": "^2.6.1",

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

0 commit comments

Comments
 (0)