Skip to content

Commit f8e9107

Browse files
authored
Add ownership to GitHub sandboxes (codesandbox#3490)
* Add ownership to GitHub sandboxes This allows users to change properties of a sandbox if they're the owner. They can change the title, description, things like env variables. Everything except the code, in which we'll autofork. The goal of this is to answer to the multiple feature requests related to using a git sandbox. While the best solution would be to invent a permission model, we're not there yet and I'm not sure when we'll get it. So this seems like a good intermediate solution instead. * Incorporate permission system inside client * Fix strictnullcheck errors * Fix types * Backwards compatability * Add live version when joining sandbox * Remove sandbox owned check * Only recover files if you have permission
1 parent 4462223 commit f8e9107

File tree

11 files changed

+136
-47
lines changed

11 files changed

+136
-47
lines changed

packages/app/src/app/overmind/effects/live/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ export default new (class Live {
110110

111111
joinChannel(roomId: string): Promise<JoinChannelResponse> {
112112
return new Promise((resolve, reject) => {
113-
channel = this.getSocket().channel(`live:${roomId}`, {});
113+
channel = this.getSocket().channel(`live:${roomId}`, { version: 2 });
114114

115115
channel
116116
.join()

packages/app/src/app/overmind/factories.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { Contributor } from '@codesandbox/common/lib/types';
1+
import { Contributor, PermissionType } from '@codesandbox/common/lib/types';
2+
import { hasPermission } from '@codesandbox/common/lib/utils/permission';
23
import { IDerive, IState, json } from 'overmind';
34

45
import { AsyncAction } from '.';
@@ -77,27 +78,29 @@ export const withLoadApp = <T>(
7778

7879
export const withOwnedSandbox = <T>(
7980
continueAction: AsyncAction<T>,
80-
cancelAction: AsyncAction<T> = () => Promise.resolve()
81+
cancelAction: AsyncAction<T> = () => Promise.resolve(),
82+
requiredPermission?: PermissionType
8183
): AsyncAction<T> => async (context, payload) => {
8284
const { state, actions } = context;
8385

84-
if (state.editor.currentSandbox) {
85-
if (!state.editor.currentSandbox.owned) {
86+
const sandbox = state.editor.currentSandbox;
87+
if (sandbox) {
88+
if (
89+
typeof requiredPermission !== 'undefined' &&
90+
!hasPermission(sandbox.authorization, requiredPermission)
91+
) {
8692
if (state.editor.isForkingSandbox) {
8793
return cancelAction(context, payload);
8894
}
8995

9096
try {
9197
await actions.editor.internal.forkSandbox({
92-
sandboxId: state.editor.currentSandbox.id,
98+
sandboxId: sandbox.id,
9399
});
94100
} catch (e) {
95101
return cancelAction(context, payload);
96102
}
97-
} else if (
98-
state.editor.currentSandbox.isFrozen &&
99-
state.editor.sessionFrozen
100-
) {
103+
} else if (sandbox.isFrozen && state.editor.sessionFrozen) {
101104
const modalResponse = await actions.modals.forkFrozenModal.open();
102105

103106
if (modalResponse === 'fork') {

packages/app/src/app/overmind/namespaces/editor/actions.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
import { clearCorrectionsFromAction } from 'app/utils/corrections';
1919
import { json } from 'overmind';
2020

21+
import { hasPermission } from '@codesandbox/common/lib/utils/permission';
2122
import eventToTransform from '../../utils/event-to-transform';
2223
import * as internalActions from './internalActions';
2324

@@ -118,7 +119,10 @@ export const sandboxChanged: AsyncAction<{ id: string }> = withLoadApp<{
118119

119120
await actions.editor.internal.initializeLiveSandbox(sandbox);
120121

121-
if (sandbox.owned && !state.live.isLive) {
122+
if (
123+
hasPermission(sandbox.authorization, 'write_code') &&
124+
!state.live.isLive
125+
) {
122126
actions.files.internal.recoverFiles();
123127
} else if (state.live.isLive) {
124128
await effects.live.sendModuleStateSyncRequest();
@@ -156,7 +160,7 @@ export const resizingStopped: Action = ({ state }) => {
156160
export const codeSaved: AsyncAction<{
157161
code: string;
158162
moduleShortid: string;
159-
cbID: string;
163+
cbID: string | null;
160164
}> = withOwnedSandbox(
161165
async ({ actions }, { code, moduleShortid, cbID }) => {
162166
actions.editor.internal.saveCode({
@@ -166,8 +170,11 @@ export const codeSaved: AsyncAction<{
166170
});
167171
},
168172
async ({ effects }, { cbID }) => {
169-
effects.vscode.callCallbackError(cbID);
170-
}
173+
if (cbID) {
174+
effects.vscode.callCallbackError(cbID);
175+
}
176+
},
177+
'write_project'
171178
);
172179

173180
export const onOperationApplied: Action<{

packages/app/src/app/overmind/namespaces/editor/internalActions.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { Action, AsyncAction } from 'app/overmind';
2121
import { sortObjectByKeys } from 'app/overmind/utils/common';
2222
import { getTemplate as computeTemplate } from 'codesandbox-import-utils/lib/create-sandbox/templates';
2323
import { mapValues } from 'lodash-es';
24+
import { hasPermission } from '@codesandbox/common/lib/utils/permission';
2425

2526
export const ensureSandboxId: Action<string, string> = ({ state }, id) => {
2627
if (state.editor.sandboxes[id]) {
@@ -254,7 +255,7 @@ export const removeNpmDependencyFromPackageJson: AsyncAction<string> = async (
254255

255256
delete packageJson.dependencies[name];
256257

257-
await actions.editor.internal.saveCode({
258+
await actions.editor.codeSaved({
258259
code: JSON.stringify(packageJson, null, 2),
259260
moduleShortid: state.editor.currentPackageJSON.shortid,
260261
cbID: null,
@@ -281,7 +282,7 @@ export const addNpmDependencyToPackageJson: AsyncAction<{
281282
packageJson[type][name] = version || 'latest';
282283
packageJson[type] = sortObjectByKeys(packageJson[type]);
283284

284-
await actions.editor.internal.saveCode({
285+
await actions.editor.codeSaved({
285286
code: JSON.stringify(packageJson, null, 2),
286287
moduleShortid: state.editor.currentPackageJSON.shortid,
287288
cbID: null,
@@ -446,14 +447,16 @@ export const updateSandboxPackageJson: AsyncAction = async ({
446447

447448
if (
448449
!sandbox ||
449-
!state.editor.parsedConfigurations ||
450-
!state.editor.parsedConfigurations.package ||
451-
!state.editor.parsedConfigurations.package.parsed ||
450+
!state.editor.parsedConfigurations?.package?.parsed ||
452451
!state.editor.currentPackageJSON
453452
) {
454453
return;
455454
}
456455

456+
if (!hasPermission(sandbox.authorization, 'write_code')) {
457+
return;
458+
}
459+
457460
const { parsed } = state.editor.parsedConfigurations.package;
458461

459462
parsed.keywords = sandbox.tags;
@@ -463,7 +466,7 @@ export const updateSandboxPackageJson: AsyncAction = async ({
463466
const code = JSON.stringify(parsed, null, 2);
464467
const moduleShortid = state.editor.currentPackageJSON.shortid;
465468

466-
await actions.editor.internal.saveCode({
469+
await actions.editor.codeSaved({
467470
code,
468471
moduleShortid,
469472
cbID: null,
@@ -482,7 +485,7 @@ export const updateDevtools: AsyncAction<{
482485
state.editor.modulesByPath['/.codesandbox/workspace.json'];
483486

484487
if (devtoolsModule) {
485-
await actions.editor.internal.saveCode({
488+
await actions.editor.codeSaved({
486489
code,
487490
moduleShortid: devtoolsModule.shortid,
488491
cbID: null,

packages/app/src/app/overmind/namespaces/editor/state.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ type State = {
5353
workspaceConfigCode: string;
5454
statusBar: boolean;
5555
previewWindowOrientation: WindowOrientation;
56+
canWriteCode: Derive<State, boolean>;
5657
isAllModulesSynced: Derive<State, boolean>;
5758
currentSandbox: Derive<State, Sandbox | null>;
5859
currentModule: Derive<State, Module>;
@@ -120,6 +121,8 @@ export const state: State = {
120121
devToolIndex: 0,
121122
tabPosition: 0,
122123
},
124+
canWriteCode: ({ currentSandbox }) =>
125+
currentSandbox?.authorization === 'write_code',
123126
currentSandbox: ({ sandboxes, currentId }) => {
124127
if (currentId && sandboxes[currentId]) {
125128
return sandboxes[currentId];

packages/app/src/app/overmind/namespaces/files/actions.ts

Lines changed: 49 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@ export const moduleRenamed: AsyncAction<{
7979

8080
actions.internal.handleError({ message: 'Could not rename file', error });
8181
}
82-
}
82+
},
83+
async () => {},
84+
'write_code'
8385
);
8486

8587
export const directoryCreated: AsyncAction<{
@@ -151,7 +153,9 @@ export const directoryCreated: AsyncAction<{
151153
error,
152154
});
153155
}
154-
}
156+
},
157+
async () => {},
158+
'write_code'
155159
);
156160

157161
export const moduleMovedToDirectory: AsyncAction<{
@@ -205,7 +209,9 @@ export const moduleMovedToDirectory: AsyncAction<{
205209
error,
206210
});
207211
}
208-
}
212+
},
213+
async () => {},
214+
'write_code'
209215
);
210216

211217
export const directoryMovedToDirectory: AsyncAction<{
@@ -255,7 +261,9 @@ export const directoryMovedToDirectory: AsyncAction<{
255261
error,
256262
});
257263
}
258-
}
264+
},
265+
async () => {},
266+
'write_code'
259267
);
260268

261269
export const directoryDeleted: AsyncAction<{
@@ -327,7 +335,9 @@ export const directoryDeleted: AsyncAction<{
327335
error,
328336
});
329337
}
330-
}
338+
},
339+
async () => {},
340+
'write_code'
331341
);
332342

333343
export const directoryRenamed: AsyncAction<{
@@ -379,7 +389,9 @@ export const directoryRenamed: AsyncAction<{
379389
error,
380390
});
381391
}
382-
}
392+
},
393+
async () => {},
394+
'write_code'
383395
);
384396

385397
export const gotUploadedFiles: AsyncAction<string> = async (
@@ -408,24 +420,28 @@ export const gotUploadedFiles: AsyncAction<string> = async (
408420
export const addedFileToSandbox: AsyncAction<{
409421
url: string;
410422
name: string;
411-
}> = withOwnedSandbox(async ({ actions, effects, state }, { name, url }) => {
412-
if (!state.editor.currentSandbox) {
413-
return;
414-
}
415-
actions.internal.closeModals(false);
416-
await actions.files.moduleCreated({
417-
title: name,
418-
directoryShortid: null,
419-
code: url,
420-
isBinary: true,
421-
});
423+
}> = withOwnedSandbox(
424+
async ({ actions, effects, state }, { name, url }) => {
425+
if (!state.editor.currentSandbox) {
426+
return;
427+
}
428+
actions.internal.closeModals(false);
429+
await actions.files.moduleCreated({
430+
title: name,
431+
directoryShortid: null,
432+
code: url,
433+
isBinary: true,
434+
});
422435

423-
effects.executor.updateFiles(state.editor.currentSandbox);
424-
});
436+
effects.executor.updateFiles(state.editor.currentSandbox);
437+
},
438+
async () => {},
439+
'write_code'
440+
);
425441

426442
export const deletedUploadedFile: AsyncAction<{
427443
id: string;
428-
}> = withOwnedSandbox(async ({ state, actions, effects }, { id }) => {
444+
}> = async ({ state, actions, effects }, { id }) => {
429445
if (!state.uploadedFiles) {
430446
return;
431447
}
@@ -441,7 +457,7 @@ export const deletedUploadedFile: AsyncAction<{
441457
error,
442458
});
443459
}
444-
});
460+
};
445461

446462
export const filesUploaded: AsyncAction<{
447463
files: { [k: string]: { dataURI: string; type: string } };
@@ -489,7 +505,9 @@ export const filesUploaded: AsyncAction<{
489505
}
490506

491507
actions.internal.closeModals(false);
492-
}
508+
},
509+
async () => {},
510+
'write_code'
493511
);
494512

495513
export const massCreateModules: AsyncAction<{
@@ -550,7 +568,9 @@ export const massCreateModules: AsyncAction<{
550568
error,
551569
});
552570
}
553-
}
571+
},
572+
async () => {},
573+
'write_code'
554574
);
555575

556576
export const moduleCreated: AsyncAction<{
@@ -650,7 +670,9 @@ export const moduleCreated: AsyncAction<{
650670
error,
651671
});
652672
}
653-
}
673+
},
674+
async () => {},
675+
'write_code'
654676
);
655677

656678
export const moduleDeleted: AsyncAction<{
@@ -691,7 +713,9 @@ export const moduleDeleted: AsyncAction<{
691713
state.editor.modulesByPath = effects.vscode.sandboxFsSync.create(sandbox);
692714
actions.internal.handleError({ message: 'Could not delete file', error });
693715
}
694-
}
716+
},
717+
async () => {},
718+
'write_code'
695719
);
696720

697721
export const createModulesByPath: AsyncAction<{

packages/app/src/app/overmind/namespaces/workspace/actions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export const tagRemoved: AsyncAction<string> = withOwnedSandbox(
8080
const code = JSON.stringify(parsed, null, 2);
8181
const moduleShortid = state.editor.currentPackageJSON.shortid;
8282

83-
await actions.editor.internal.saveCode({
83+
await actions.editor.codeSaved({
8484
code,
8585
moduleShortid,
8686
cbID: null,

packages/app/src/app/overmind/utils/items.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ export function getDisabledItems(store: any): INavigationItem[] {
7676
export default function getItems(store: any): INavigationItem[] {
7777
if (
7878
store.live.isLive &&
79+
!store.editor.currentSandbox.git &&
7980
!(
8081
store.live.isOwner ||
8182
(store.user &&

packages/common/src/types/index.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ export type SSEContainerStatus =
1313

1414
export type SSEManagerStatus = 'connected' | 'disconnected' | 'initializing';
1515

16+
export type PermissionType =
17+
| 'write_code'
18+
| 'write_project'
19+
| 'comment'
20+
| 'read'
21+
| 'none';
22+
1623
export type ModuleError = {
1724
message: string;
1825
line: number;
@@ -313,6 +320,7 @@ export type Sandbox = {
313320
path: string;
314321
};
315322
owned: boolean;
323+
authorization: PermissionType;
316324
npmDependencies: {
317325
[dep: string]: string;
318326
};
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { hasPermission } from './permission';
2+
3+
describe('permission', () => {
4+
it('correctly accepts a lower permission', () => {
5+
expect(hasPermission('write_code', 'read')).toBe(true);
6+
expect(hasPermission('write_code', 'write_code')).toBe(true);
7+
expect(hasPermission('write_project', 'comment')).toBe(true);
8+
});
9+
10+
it('correctly rejects a a permission', () => {
11+
expect(hasPermission('read', 'write_code')).toBe(false);
12+
expect(hasPermission('read', 'comment')).toBe(false);
13+
});
14+
});

0 commit comments

Comments
 (0)