Skip to content

Commit d88fe05

Browse files
authored
[fga] trying to understand the sharing issue (#18766)
1 parent 181c983 commit d88fe05

File tree

3 files changed

+29
-18
lines changed

3 files changed

+29
-18
lines changed

components/server/src/authorization/authorizer.ts

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ export class Authorizer {
179179
userId: string,
180180
permission: WorkspacePermission,
181181
workspaceId: string,
182+
forceEnablement?: boolean, // temporary to find an issue with workspace sharing
182183
): Promise<boolean> {
183184
if (userId === SYSTEM_USER) {
184185
return true;
@@ -191,7 +192,7 @@ export class Authorizer {
191192
consistency,
192193
});
193194

194-
return this.authorizer.check(req, { userId });
195+
return this.authorizer.check(req, { userId }, forceEnablement);
195196
}
196197

197198
async checkPermissionOnWorkspace(userId: string, permission: WorkspacePermission, workspaceId: string) {
@@ -423,25 +424,15 @@ export class Authorizer {
423424
): Promise<void> {
424425
const rels: v1.RelationshipUpdate[] = [];
425426
for (const { orgID, userID, workspaceID, shared } of ids) {
426-
this.internalAddWorkspaceToOrg(orgID, userID, workspaceID, shared, (u) => rels.push(u));
427+
rels.push(set(rel.workspace(workspaceID).org.organization(orgID)));
428+
rels.push(set(rel.workspace(workspaceID).owner.user(userID)));
429+
if (shared) {
430+
rels.push(set(rel.workspace(workspaceID).shared.anyUser));
431+
}
427432
}
428433
await this.authorizer.writeRelationships(...rels);
429434
}
430435

431-
private internalAddWorkspaceToOrg(
432-
orgID: string,
433-
userID: string,
434-
workspaceID: string,
435-
shared: boolean,
436-
acceptor: (update: v1.RelationshipUpdate) => void,
437-
): void {
438-
acceptor(set(rel.workspace(workspaceID).org.organization(orgID)));
439-
acceptor(set(rel.workspace(workspaceID).owner.user(userID)));
440-
if (shared) {
441-
acceptor(set(rel.workspace(workspaceID).shared.anyUser));
442-
}
443-
}
444-
445436
async removeWorkspaceFromOrg(orgID: string, userID: string, workspaceID: string): Promise<void> {
446437
if (!(await isFgaWritesEnabled(userID))) {
447438
return;

components/server/src/authorization/spicedb-authorizer.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,9 @@ export class SpiceDBAuthorizer {
5454
experimentsFields: {
5555
userId: string;
5656
},
57+
forceEnablement?: boolean,
5758
): Promise<boolean> {
58-
const featureEnabled = await isFgaChecksEnabled(experimentsFields.userId);
59+
const featureEnabled = !!forceEnablement || (await isFgaChecksEnabled(experimentsFields.userId));
5960
const timer = spicedbClientLatency.startTimer();
6061
let error: Error | undefined;
6162
try {

components/server/src/workspace/gitpod-server-impl.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,8 @@ import {
176176
suggestionFromRecentWorkspace,
177177
suggestionFromUserRepo,
178178
} from "./suggested-repos-sorter";
179+
import { TrustedValue } from "@gitpod/gitpod-protocol/lib/util/scrubbing";
180+
import { rel } from "../authorization/definitions";
179181

180182
// shortcut
181183
export const traceWI = (ctx: TraceContext, wi: Omit<LogContext, "userId">) => TraceContext.setOWI(ctx, wi); // userId is already taken care of in WebsocketConnectionManager
@@ -564,6 +566,23 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
564566
`operation not permitted: missing ${op} permission on ${resource.kind}`,
565567
);
566568
}
569+
if (resource.kind === "workspace" && op === "get") {
570+
// access to workspaces is granted. Let's verify that this is also thecase with FGA
571+
const result = await this.auth.hasPermissionOnWorkspace(
572+
this.userID!,
573+
"read_info",
574+
resource.subject.id,
575+
true,
576+
);
577+
if (!result) {
578+
const isShared = await this.auth.find(rel.workspace(resource.subject.id).shared.anyUser);
579+
log.error("user has access to workspace, but not through FGA", {
580+
userId: this.userID,
581+
workspace: new TrustedValue(resource.subject),
582+
sharedRelationship: isShared && new TrustedValue(isShared),
583+
});
584+
}
585+
}
567586
}
568587

569588
/**
@@ -3794,7 +3813,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
37943813
if (!parsedAttributionId) {
37953814
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Unable to parse attributionId");
37963815
}
3797-
await this.auth.checkPermissionOnOrganization(admin.id, "read_billing", attributionId);
3816+
await this.auth.checkPermissionOnOrganization(admin.id, "read_billing", parsedAttributionId.teamId);
37983817
return this.billingModes.getBillingMode(admin.id, parsedAttributionId.teamId);
37993818
}
38003819

0 commit comments

Comments
 (0)