Skip to content

Commit e389b85

Browse files
Enable collaborators (#20353)
* Enable collaborators * Fix forgotten debug logs * Such a dumb typo I wonder who wrote it * Fix schema * Fix typo * Fix prebuild page button a11y * Update permissions * Fix prebuild checks * Change names around
1 parent b77f687 commit e389b85

File tree

9 files changed

+55
-28
lines changed

9 files changed

+55
-28
lines changed

components/dashboard/src/data/git-providers/unified-repositories-search-query.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,12 @@ export function deduplicateAndFilterRepositories(
128128
if (results.length === 0) {
129129
// If the searchString is a URL, and it's not present in the proposed results, "artificially" add it here.
130130
if (isValidGitUrl(searchString)) {
131-
console.log("It's valid man");
132131
results.push(
133132
new SuggestedRepository({
134133
url: searchString,
135134
}),
136135
);
137136
}
138-
139-
console.log("Valid after man");
140137
}
141138

142139
// Limit what we show to 200 results
@@ -145,7 +142,7 @@ export function deduplicateAndFilterRepositories(
145142

146143
const ALLOWED_GIT_PROTOCOLS = ["ssh:", "git:", "http:", "https:"];
147144
/**
148-
* An opionated git URL validator
145+
* An opinionated git URL validator
149146
*
150147
* Assumptions:
151148
* - Git hosts are not themselves TLDs (like .com) or reserved names like `localhost`

components/dashboard/src/prebuilds/detail/PrebuildDetailPage.tsx

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -361,15 +361,13 @@ export const PrebuildDetailPage: FC = () => {
361361
>
362362
View Prebuild Settings
363363
</LinkButton>
364-
<Button
364+
<LinkButton
365365
disabled={prebuild?.status?.phase?.name !== PrebuildPhase_Phase.AVAILABLE}
366-
onClick={() =>
367-
(window.location.href = `/#open-prebuild/${prebuild?.id}/${prebuild?.contextUrl}`)
368-
}
366+
href={`/#open-prebuild/${prebuild?.id}/${prebuild?.contextUrl}`}
369367
variant="secondary"
370368
>
371369
Open Debug Workspace
372-
</Button>
370+
</LinkButton>
373371
</div>
374372
</div>
375373
</div>

components/server/src/prebuilds/prebuild-manager.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export interface StartPrebuildParams {
4949
commitInfo?: CommitInfo;
5050
forcePrebuild?: boolean;
5151
trigger?: keyof ProjectUsage;
52+
assumeProjectActive?: boolean;
5253
}
5354

5455
export interface PrebuildFilter {
@@ -330,7 +331,15 @@ export class PrebuildManager {
330331

331332
async startPrebuild(
332333
ctx: TraceContext,
333-
{ context, project, user, commitInfo, forcePrebuild, trigger = "lastWebhookReceived" }: StartPrebuildParams,
334+
{
335+
context,
336+
project,
337+
user,
338+
commitInfo,
339+
forcePrebuild,
340+
trigger = "lastWebhookReceived",
341+
assumeProjectActive,
342+
}: StartPrebuildParams,
334343
): Promise<StartPrebuildResult> {
335344
const span = TraceContext.startSpan("startPrebuild", ctx);
336345
const cloneURL = context.repository.cloneUrl;
@@ -469,7 +478,10 @@ export class PrebuildManager {
469478
"Prebuild is rate limited. Please contact Gitpod if you believe this happened in error.";
470479
await this.workspaceDB.trace({ span }).storePrebuiltWorkspace(prebuild);
471480
span.setTag("ratelimited", true);
472-
} else if (await this.projectService.isProjectConsideredInactive(user.id, project.id)) {
481+
} else if (
482+
!assumeProjectActive &&
483+
(await this.projectService.isProjectConsideredInactive(user.id, project.id))
484+
) {
473485
prebuild.state = "aborted";
474486
prebuild.error =
475487
"Project is inactive. Please start a new workspace for this project to re-enable prebuilds.";
@@ -669,7 +681,7 @@ export class PrebuildManager {
669681
if (!prebuild || !organizationId) {
670682
throw new ApplicationError(ErrorCodes.PRECONDITION_FAILED, "prebuild workspace not found");
671683
}
672-
await this.auth.checkPermissionOnOrganization(userId, "read_prebuild", organizationId);
684+
await this.auth.checkPermissionOnProject(userId, "read_prebuild", organizationId);
673685

674686
const instance = await this.workspaceService.getCurrentInstance(userId, prebuild.workspace.id, {
675687
skipPermissionCheck: true,

components/server/src/projects/projects-service.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,14 @@ export class ProjectsService {
6262
@inject(InstallationService) private readonly installationService: InstallationService,
6363
) {}
6464

65-
async getProject(userId: string, projectId: string): Promise<Project> {
66-
await this.auth.checkPermissionOnProject(userId, "read_info", projectId);
65+
/**
66+
* Returns a project by its ID.
67+
* @param skipPermissionCheck useful either when the caller already checked permissions or when we need to do something purely server-side (e.g. looking up a project when starting a workspace by a collaborator)
68+
*/
69+
async getProject(userId: string, projectId: string, skipPermissionCheck?: boolean): Promise<Project> {
70+
if (!skipPermissionCheck) {
71+
await this.auth.checkPermissionOnProject(userId, "read_info", projectId);
72+
}
6773
const project = await this.projectDB.findProjectById(projectId);
6874
if (!project) {
6975
throw new ApplicationError(ErrorCodes.NOT_FOUND, `Project ${projectId} not found.`);
@@ -132,11 +138,18 @@ export class ProjectsService {
132138
return filteredProjects;
133139
}
134140

135-
async findProjectsByCloneUrl(userId: string, cloneUrl: string, organizationId?: string): Promise<Project[]> {
141+
async findProjectsByCloneUrl(
142+
userId: string,
143+
cloneUrl: string,
144+
organizationId?: string,
145+
skipPermissionCheck?: boolean,
146+
): Promise<Project[]> {
136147
const projects = await this.projectDB.findProjectsByCloneUrl(cloneUrl, organizationId);
137148
const result: Project[] = [];
138149
for (const project of projects) {
139-
if (await this.auth.hasPermissionOnProject(userId, "read_info", project.id)) {
150+
const hasPermission =
151+
skipPermissionCheck || (await this.auth.hasPermissionOnProject(userId, "read_info", project.id));
152+
if (hasPermission) {
140153
result.push(project);
141154
}
142155
}

components/server/src/workspace/context-service.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,9 @@ export class ContextService {
159159
user.id,
160160
context.repository.cloneUrl,
161161
options?.organizationId,
162+
true,
162163
);
164+
// todo(ft): solve for this case with collaborators who can't select projects directly
163165
if (projects.length > 1) {
164166
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Multiple projects found for clone URL.");
165167
}

components/server/src/workspace/suggested-repos-sorter.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export const sortSuggestedRepositories = (repos: SuggestedRepositoryWithSorting[
2020
// This allows us to consider the lastUse of a recently used project when sorting
2121
// as it will may have an entry for the project (no lastUse), and another for recent workspaces (w/ lastUse)
2222

23-
const projectURLs: string[] = [];
23+
let projectURLs: string[] = [];
2424
let uniqueRepositories: SuggestedRepositoryWithSorting[] = [];
2525

2626
for (const repo of repos) {
@@ -88,7 +88,7 @@ export const sortSuggestedRepositories = (repos: SuggestedRepositoryWithSorting[
8888
uniqueRepositories = uniqueRepositories.map((repo) => {
8989
if (repo.projectId && !repo.projectName) {
9090
delete repo.projectId;
91-
delete projectURLs[projectURLs.indexOf(repo.url)];
91+
projectURLs = projectURLs.filter((url) => url !== repo.url);
9292
}
9393

9494
return repo;

components/server/src/workspace/workspace-service.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,7 @@ export class WorkspaceService {
455455
forcePrebuild: false,
456456
context,
457457
trigger: "lastWorkspaceStart",
458+
assumeProjectActive: true,
458459
});
459460
log.info(logCtx, "starting prebuild after workspace creation", {
460461
projectId: project.id,
@@ -831,7 +832,7 @@ export class WorkspaceService {
831832
}
832833

833834
const projectPromise = workspace.projectId
834-
? ApplicationError.notFoundToUndefined(this.projectsService.getProject(user.id, workspace.projectId))
835+
? ApplicationError.notFoundToUndefined(this.projectsService.getProject(user.id, workspace.projectId, true))
835836
: Promise.resolve(undefined);
836837

837838
await mayStartPromise;
@@ -878,7 +879,7 @@ export class WorkspaceService {
878879
result = await this.entitlementService.mayStartWorkspace(user, organizationId, runningInstances);
879880
TraceContext.addNestedTags(ctx, { mayStartWorkspace: { result } });
880881
} catch (err) {
881-
log.error({ userId: user.id }, "EntitlementSerivce.mayStartWorkspace error", err);
882+
log.error({ userId: user.id }, "EntitlementService.mayStartWorkspace error", err);
882883
TraceContext.setError(ctx, err);
883884
return; // we don't want to block workspace starts because of internal errors
884885
}

components/server/src/workspace/workspace-starter.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -578,8 +578,8 @@ export class WorkspaceStarter {
578578
return;
579579
}
580580

581-
// implicit project (existing on the same clone URL)
582-
const projects = await this.projectService.findProjectsByCloneUrl(user.id, contextURL, organizationId);
581+
// implicit project (existing on the same clone URL). We skip the permission check so that collaborators are not stuck
582+
const projects = await this.projectService.findProjectsByCloneUrl(user.id, contextURL, organizationId, true);
583583
if (projects.length === 0) {
584584
throw new ApplicationError(
585585
ErrorCodes.PRECONDITION_FAILED,
@@ -1963,10 +1963,12 @@ export class WorkspaceStarter {
19631963
{},
19641964
);
19651965
if (isEnabledPrebuildFullClone) {
1966-
const project = await this.projectService.getProject(user.id, workspace.projectId).catch((err) => {
1967-
log.error("failed to get project", err);
1968-
return undefined;
1969-
});
1966+
const project = await this.projectService
1967+
.getProject(user.id, workspace.projectId, true)
1968+
.catch((err) => {
1969+
log.error("failed to get project", err);
1970+
return undefined;
1971+
});
19701972
if (project && project.settings?.prebuilds?.cloneSettings?.fullClone) {
19711973
result.setFullClone(true);
19721974
}

components/spicedb/schema/schema.yaml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ schema: |-
9292
permission read_billing = member + owner + installation->admin
9393
permission write_billing = owner + installation->admin
9494
95+
// Note that there are two different read_prebuild permissions: this one, guarding primarily the listPrebuilds method in the API and then the other under projects, which is the actual permission used for checking if a user can use a prebuild for a repository.
96+
// Today, the difference is that collaborators can't read prebuilds on an org but can on a repository (project).
9597
permission read_prebuild = member + owner + installation->admin
9698
9799
permission create_workspace = member + collaborator
@@ -118,10 +120,10 @@ schema: |-
118120
permission write_info = editor + org->owner + org->installation_admin
119121
permission delete = editor + org->owner + org->installation_admin
120122
121-
permission read_env_var = viewer + editor + org->owner + org->installation_admin
123+
permission read_env_var = viewer + editor + org->collaborator + org->owner + org->installation_admin
122124
permission write_env_var = editor + org->owner + org->installation_admin
123125
124-
permission read_prebuild = viewer + editor + org->owner + org->installation_admin
126+
permission read_prebuild = viewer + editor + org->collaborator + org->owner + org->installation_admin
125127
permission write_prebuild = editor + org->owner
126128
}
127129

0 commit comments

Comments
 (0)