Skip to content

[spicedb] Grant all org members project "editor" role #18733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 2 additions & 21 deletions components/server/src/authorization/definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@ export type OrganizationPermission =

export type ProjectResourceType = "project";

export type ProjectRelation = "org" | "editor" | "viewer";
export type ProjectRelation = "org" | "viewer";

export type ProjectPermission =
| "editor"
| "read_info"
| "write_info"
| "delete"
Expand Down Expand Up @@ -341,26 +342,6 @@ export const rel = {
};
},

get editor() {
const result2 = {
...result,
relation: "editor",
};
return {
user(objectId: string) {
return {
...result2,
subject: {
object: {
objectType: "user",
objectId: objectId,
},
},
} as v1.Relationship;
},
};
},

get viewer() {
const result2 = {
...result,
Expand Down
42 changes: 22 additions & 20 deletions components/server/src/projects/projects-service.spec.db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,45 +90,47 @@ describe("ProjectsService", async () => {

it("should deleteProject", async () => {
const ps = container.get(ProjectsService);
const project = await createTestProject(ps, org, owner);
const project1 = await createTestProject(ps, org, owner);

await ps.deleteProject(member.id, project1.id);
let projects = await ps.getProjects(member.id, org.id);
expect(projects.length).to.equal(0);

await expectError(ErrorCodes.PERMISSION_DENIED, () => ps.deleteProject(member.id, project.id));
await expectError(ErrorCodes.NOT_FOUND, () => ps.deleteProject(stranger.id, project.id));
const project2 = await createTestProject(ps, org, owner);
await expectError(ErrorCodes.NOT_FOUND, () => ps.deleteProject(stranger.id, project2.id));

await ps.deleteProject(owner.id, project.id);
const projects = await ps.getProjects(owner.id, org.id);
await ps.deleteProject(owner.id, project2.id);
projects = await ps.getProjects(owner.id, org.id);
expect(projects.length).to.equal(0);
});

it("should updateProject", async () => {
const ps = container.get(ProjectsService);
const project = await createTestProject(ps, org, owner);

await ps.updateProject(owner, {
id: project.id,
settings: {
useIncrementalPrebuilds: !project.settings?.useIncrementalPrebuilds,
prebuildEveryNthCommit: 1,
},
});
const updatedProject1 = await ps.getProject(owner.id, project.id);
expect(updatedProject1?.settings?.prebuildEveryNthCommit).to.equal(1);

const updatedProject = await ps.getProject(owner.id, project.id);

expect(updatedProject?.settings?.useIncrementalPrebuilds).to.not.equal(
project.settings?.useIncrementalPrebuilds,
);
await ps.updateProject(member, {
id: project.id,
settings: {
prebuildEveryNthCommit: 2,
},
});
const updatedProject2 = await ps.getProject(member.id, project.id);
expect(updatedProject2?.settings?.prebuildEveryNthCommit).to.equal(2);

await expectError(ErrorCodes.PERMISSION_DENIED, () =>
ps.updateProject(member, {
id: project.id,
settings: {
useIncrementalPrebuilds: !project.settings?.useIncrementalPrebuilds,
},
}),
);
await expectError(ErrorCodes.NOT_FOUND, () =>
ps.updateProject(stranger, {
id: project.id,
settings: {
useIncrementalPrebuilds: !project.settings?.useIncrementalPrebuilds,
prebuildEveryNthCommit: 3,
},
}),
);
Expand Down
9 changes: 3 additions & 6 deletions components/server/src/user/env-var-service.spec.db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ describe("EnvVarService", async () => {
expect(emptyEnvVars.length).to.equal(0);
});

it("should not let members create, delete but allow get project env vars", async () => {
it("let members create, delete and get project env vars", async () => {
await es.addProjectEnvVar(owner.id, project.id, { name: "FOO", value: "BAR", censored: false });

const envVars = await es.listProjectEnvVars(member.id, project.id);
Expand All @@ -232,12 +232,9 @@ describe("EnvVarService", async () => {
const envVarById = await es.getProjectEnvVarById(member.id, envVars[0].id);
expect(envVarById?.name).to.equal("FOO");

await expectError(ErrorCodes.PERMISSION_DENIED, es.deleteProjectEnvVar(member.id, envVars[0].id));
await es.deleteProjectEnvVar(member.id, envVars[0].id);

await expectError(
ErrorCodes.PERMISSION_DENIED,
es.addProjectEnvVar(member.id, project.id, { name: "FOO", value: "BAR", censored: false }),
);
await es.addProjectEnvVar(owner.id, project.id, { name: "FOO", value: "BAR", censored: false });
});

it("should not let strangers create, delete and get project env vars", async () => {
Expand Down
13 changes: 8 additions & 5 deletions components/spicedb/schema/schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,15 @@ schema: |-
definition project {
relation org: organization

relation editor: user

// A subject is a viewer, if:
// * the users with access are directly assigned as a viewer
// * the project has granted access to all members in an organization
// * the project has granted access to _any_ user on this installation
relation viewer: user | organization#member | user:*

// All org members are editors for now, to model the existing behavior.
permission editor = org->member

permission read_info = viewer + editor + org->owner + org->installation_admin
permission write_info = editor + org->owner + org->installation_admin
permission delete = editor + org->owner + org->installation_admin
Expand Down Expand Up @@ -209,10 +211,14 @@ assertions:
- organization:org_1#delete@user:user_0
# Org owner can delete projects
- project:project_1#delete@user:user_0
# org members can delete project
- project:project_1#delete@user:user_1
# Org member can view projects
- project:project_1#read_info@user:user_1
# Org member can create projects
- organization:org_1#create_project@user:user_1
# user 10 can access project_2
- project:project_2#write_info@user:user_10
# installation user can create orgs
- installation:installation_0#create_organization@user:user_0
# Installation admin can do what org owners can
Expand All @@ -230,7 +236,6 @@ assertions:
assertFalse:
# user 10 cannot access project_1
- project:project_1#read_info@user:user_10
- project:project_2#write_info@user:user_10
# non-member/owner cannot access organization
- organization:org_1#read_info@user:user_3
- organization:org_1#write_info@user:user_3
Expand All @@ -244,7 +249,5 @@ assertions:
- organization:org_1#write_git_provider@user:user_1
# org member can not delete org
- organization:org_1#delete@user:user_1
# org members can not delete project
- project:project_1#delete@user:user_1
# stranger can't access other's non-shared workspace
- workspace:workspace_1#access@user:user_2