Skip to content

Commit 1954f86

Browse files
committed
fix(security): always use actor
1 parent 94e15db commit 1954f86

File tree

9 files changed

+67
-58
lines changed

9 files changed

+67
-58
lines changed

src/backend/src/filesystem/FilesystemService.js

+11-12
Original file line numberDiff line numberDiff line change
@@ -213,17 +213,16 @@ class FilesystemService extends BaseService {
213213
return this.systemfs_;
214214
}
215215

216-
async owrite ({
217-
node, user, immutable,
218-
file, tmp, fsentry_tmp,
219-
message,
220-
}) {
216+
// NOTE: these are the parameters being passed
217+
// (assuming this comment is up-to-date)
218+
// {
219+
// node, actor, immutable,
220+
// file, tmp, fsentry_tmp,
221+
// message,
222+
// }
223+
async owrite (parameters) {
221224
const ll_owrite = new LLOWrite();
222-
return await ll_owrite.run({
223-
node, user, immutable,
224-
file, tmp, fsentry_tmp,
225-
message,
226-
});
225+
return await ll_owrite.run(parameters);
227226
}
228227

229228
// REMINDER: There was an idea that FilesystemService implements
@@ -235,9 +234,9 @@ class FilesystemService extends BaseService {
235234
return await ll_cwrite.run(parameters);
236235
}
237236

238-
async mkdir_2 ({parent, name, user, immutable}) {
237+
async mkdir_2 ({parent, name, actor, immutable}) {
239238
const ll_mkdir = new LLMkdir();
240-
return await ll_mkdir.run({ parent, name, user, immutable });
239+
return await ll_mkdir.run({ parent, name, actor, immutable });
241240
}
242241

243242
async mkshortcut ({ parent, name, user, target }) {

src/backend/src/filesystem/batch/BatchExecutor.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ const config = require('../../config');
2626
const { TeePromise } = require('../../util/promise');
2727

2828
class BatchExecutor extends AdvancedBase {
29-
constructor (x, { user, log, errors }) {
29+
constructor (x, { actor, log, errors }) {
3030
super();
3131
this.x = x;
32-
this.user = user;
33-
this.pathResolver = new PathResolver({ user });
32+
this.actor = actor
33+
this.pathResolver = new PathResolver({ actor });
3434
this.expectations = x.get('services').get('expectations');
3535
this.log = log;
3636
this.errors = errors;
@@ -100,7 +100,7 @@ class BatchExecutor extends AdvancedBase {
100100
const command_ins = await command_cls.run({
101101
getFile: () => file,
102102
pathResolver: this.pathResolver,
103-
user: this.user
103+
actor: this.actor,
104104
}, op);
105105
workUnit.checkpoint('operation invoked');
106106

src/backend/src/filesystem/batch/commands.js

+8-5
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ class MkdirCommand extends BatchCommand {
102102
parameters.create_missing_parents ??
103103
false,
104104
shortcut_to: parameters.shortcut_to,
105-
user: executor.user,
105+
actor: executor.actor,
106106
});
107107
if ( parameters.as ) {
108108
executor.pathResolver.putSelector(
@@ -133,6 +133,9 @@ class WriteCommand extends BatchCommand {
133133
}
134134

135135
const hl_write = new HLWrite();
136+
if ( ! executor.actor ) {
137+
throw new Error('Actor is missing here');
138+
}
136139
const response = await hl_write.run({
137140
destination_or_parent: destinationOrParent,
138141
specified_name: parameters.name,
@@ -145,7 +148,7 @@ class WriteCommand extends BatchCommand {
145148
parameters.create_missing_ancestors ??
146149
parameters.create_missing_parents ??
147150
false,
148-
user: executor.user,
151+
actor: executor.actor,
149152

150153
file: uploaded_file,
151154
offset: parameters.offset,
@@ -208,7 +211,7 @@ class ShortcutCommand extends BatchCommand {
208211
const response = await hl_mkShortcut.run({
209212
parent: destinationOrParent,
210213
name: parameters.name,
211-
user: executor.user,
214+
actor: executor.actor,
212215
target: shortcut_to,
213216

214217
// TODO: handle these with event service instead
@@ -241,7 +244,7 @@ class SymlinkCommand extends BatchCommand {
241244
const response = await hl_mkLink.run({
242245
parent: destinationOrParent,
243246
name: parameters.name,
244-
user: executor.user,
247+
actor: executor.actor,
245248
target: parameters.target,
246249

247250
// TODO: handle these with event service instead
@@ -266,7 +269,7 @@ class DeleteCommand extends BatchCommand {
266269
const hl_remove = new HLRemove();
267270
const response = await hl_remove.run({
268271
target,
269-
user: executor.user,
272+
actor: executor.actor,
270273
recursive: parameters.recursive ?? false,
271274
descendants_only: parameters.descendants_only ?? false,
272275
});

src/backend/src/filesystem/hl_operations/hl_mkdir.js

+15-13
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class MkTree extends HLFilesystemOperation {
7676
const { context, values } = this;
7777
const { _path } = this.modules;
7878
const fs = context.get('services').get('filesystem');
79-
const user = context.get('user');
79+
const actor = context.get('actor');
8080

8181
const trunk = tree[0];
8282
const branches = tree.slice(1);
@@ -124,7 +124,7 @@ class MkTree extends HLFilesystemOperation {
124124

125125
if ( parent_did_exist && ! parent_exists ) {
126126
const node = await fs.node(current);
127-
const has_perm = await chkperm(await node.get('entry'), user.id, 'write');
127+
const has_perm = await chkperm(await node.get('entry'), actor.type.user.id, 'write');
128128
if ( ! has_perm ) throw APIError.create('permission_denied');
129129
}
130130

@@ -147,7 +147,7 @@ class MkTree extends HLFilesystemOperation {
147147
const node = await fs.mkdir_2({
148148
parent: await fs.node(currentParent),
149149
name: current.name,
150-
user,
150+
actor,
151151
})
152152

153153
current = node.selector;
@@ -179,7 +179,7 @@ class QuickMkdir extends HLFilesystemOperation {
179179
let { parent, path } = values;
180180
const { _path } = this.modules;
181181
const fs = context.get('services').get('filesystem');
182-
const user = context.get('user');
182+
const actor = context.get('actor');
183183

184184
parent = parent || await fs.node(new RootNodeSelector());
185185

@@ -206,7 +206,7 @@ class QuickMkdir extends HLFilesystemOperation {
206206
const node = await fs.mkdir_2({
207207
parent: await fs.node(currentParent),
208208
name: current.name,
209-
user,
209+
actor,
210210
})
211211

212212
current = node.selector;
@@ -285,10 +285,12 @@ class HLMkdir extends HLFilesystemOperation {
285285
// specified under `path`.
286286
parent_node = await this._create_parents({
287287
parent_node: top_parent,
288-
user: values.user,
288+
actor: values.actor,
289289
});
290290

291-
const has_perm = await chkperm(await parent_node.get('entry'), values.user.id, 'write');
291+
const user_id = values.actor.type.user.id;
292+
293+
const has_perm = await chkperm(await parent_node.get('entry'), user_id, 'write');
292294
if ( ! has_perm ) throw APIError.create('permission_denied');
293295

294296
const existing = await fs.node(
@@ -301,12 +303,12 @@ class HLMkdir extends HLFilesystemOperation {
301303
const { overwrite, dedupe_name, create_missing_parents } = values;
302304
if ( overwrite ) {
303305
// TODO: tag rm operation somehow
304-
const has_perm = await chkperm(await existing.get('entry'), values.user.id, 'write');
306+
const has_perm = await chkperm(await existing.get('entry'), user_id, 'write');
305307
if ( ! has_perm ) throw APIError.create('permission_denied');
306308
const hl_remove = new HLRemove();
307309
await hl_remove.run({
308310
target: existing,
309-
user: values.user,
311+
actor: values.actor,
310312
recursive: true,
311313
});
312314
}
@@ -345,13 +347,13 @@ class HLMkdir extends HLFilesystemOperation {
345347
if ( ! shortcut_to.entry.is_dir ) {
346348
throw APIError.create('shortcut_target_is_a_directory');
347349
}
348-
const has_perm = await chkperm(shortcut_to.entry, values.user.id, 'read');
350+
const has_perm = await chkperm(shortcut_to.entry, user_id, 'read');
349351
if ( ! has_perm ) throw APIError.create('forbidden');
350352

351353
this.created = await fs.mkshortcut({
352354
parent: parent_node,
353355
name: target_basename,
354-
user: values.user,
356+
actor: values.actor,
355357
target: shortcut_to,
356358
});
357359

@@ -362,7 +364,7 @@ class HLMkdir extends HLFilesystemOperation {
362364
this.created = await fs.mkdir_2({
363365
parent: parent_node,
364366
name: target_basename,
365-
user: values.user,
367+
actor: values.actor,
366368
});
367369

368370
const all_nodes = [
@@ -382,7 +384,7 @@ class HLMkdir extends HLFilesystemOperation {
382384
return response;
383385
}
384386

385-
async _create_parents ({ parent_node, user }) {
387+
async _create_parents ({ parent_node }) {
386388
const { context, values } = this;
387389
const { _path } = this.modules;
388390
const fs = context.get('services').get('filesystem');

src/backend/src/filesystem/hl_operations/hl_write.js

+9-5
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,10 @@ class WriteCommonFeature {
5959
if ( ! this.values.file ) return;
6060

6161
const sizeService = this.context.get('services').get('sizeService');
62-
const { file, user } = this.values;
62+
const { file, user: user_let } = this.values;
63+
let user = user_let;
64+
65+
if ( ! user ) user = this.values.actor.type.user;
6366

6467
const usage = await sizeService.get_usage(user.id);
6568
let capacity = config.is_storage_limited ? user.free_storage == undefined
@@ -281,13 +284,14 @@ class HLWrite extends HLFilesystemOperation {
281284
if ( await shortcut_to.get('type') === TYPE_DIRECTORY ) {
282285
throw APIError.create('shortcut_target_is_a_directory');
283286
}
284-
const has_perm = await chkperm(shortcut_to.entry, values.user.id, 'read');
287+
// TODO: legacy check - likely not needed
288+
const has_perm = await chkperm(shortcut_to.entry, values.actor.type.user.id, 'read');
285289
if ( ! has_perm ) throw APIError.create('permission_denied');
286290

287291
this.created = await fs.mkshortcut({
288292
parent,
289293
name: target_name,
290-
user: values.user,
294+
actor: values.actor,
291295
target: shortcut_to,
292296
});
293297

@@ -381,7 +385,7 @@ class HLWrite extends HLFilesystemOperation {
381385
if ( is_overwrite ) {
382386
this.written = await fs.owrite({
383387
node: destination,
384-
user: values.user,
388+
actor: values.actor,
385389
file: values.file,
386390
tmp: {
387391
socket_id: values.socket_id,
@@ -397,7 +401,7 @@ class HLWrite extends HLFilesystemOperation {
397401
this.written = await fs.cwrite({
398402
parent,
399403
name: target_name,
400-
user: values.user,
404+
actor: values.actor,
401405
file: values.file,
402406
tmp: {
403407
socket_id: values.socket_id,

src/backend/src/filesystem/ll_operations/ll_mkdir.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class LLMkdir extends LLFilesystemOperation {
5555
async _locked_run () {
5656
const { _path, uuidv4 } = this.modules;
5757
const { context } = this;
58-
const { parent, name, user, immutable, actor } = this.values;
58+
const { parent, name, immutable, actor } = this.values;
5959

6060
const ts = Math.round(Date.now() / 1000);
6161
const uid = uuidv4();
@@ -95,7 +95,7 @@ class LLMkdir extends LLFilesystemOperation {
9595
uuid: uid,
9696
parent_uid: await parent.get('uid'),
9797
path: _path.join(await parent.get('path'), name),
98-
user_id: user.id,
98+
user_id: actor.type.user.id,
9999
name,
100100
created: ts,
101101
accessed: ts,

src/backend/src/filesystem/ll_operations/ll_write.js

+8-12
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,11 @@ class LLWriteBase extends LLFilesystemOperation {
128128
class LLOWrite extends LLWriteBase {
129129
async _run () {
130130
const {
131-
node, user, immutable,
131+
node, actor, immutable,
132132
file, tmp, fsentry_tmp,
133133
message,
134134
} = this.values;
135135

136-
let { actor } = this.values;
137-
138136
const svc = Context.get('services');
139137
const sizeService = svc.get('sizeService');
140138
const resourceService = svc.get('resourceService');
@@ -152,8 +150,6 @@ class LLOWrite extends LLWriteBase {
152150
throw APIError.create('subject_does_not_exist');
153151
}
154152

155-
actor = actor ?? Actor.adapt(user);
156-
157153
const svc_acl = this.context.get('services').get('acl');
158154
if ( ! await svc_acl.check(actor, node, 'write') ) {
159155
throw await svc_acl.get_safe_acl_error(actor, node, 'write');
@@ -190,7 +186,7 @@ class LLOWrite extends LLWriteBase {
190186
});
191187

192188
const filesize = file.size;
193-
sizeService.change_usage(user.id, filesize);
189+
sizeService.change_usage(actor.type.user.id, filesize);
194190

195191
const entryOp = await systemFSEntryService.update(uid, raw_fsentry_delta);
196192

@@ -202,7 +198,7 @@ class LLOWrite extends LLWriteBase {
202198
})();
203199

204200
state_upload.post_insert({
205-
db, user, node, uid, message, ts,
201+
db, user: actor.type.user, node, uid, message, ts,
206202
});
207203

208204
const svc_fileCache = this.context.get('services').get('file-cache');
@@ -227,7 +223,7 @@ class LLCWrite extends LLWriteBase {
227223
async _run () {
228224
const { _path, uuidv4, config } = this.modules;
229225
const {
230-
parent, name, user, immutable,
226+
parent, name, immutable,
231227
file, tmp, fsentry_tmp,
232228
message,
233229

@@ -261,7 +257,7 @@ class LLCWrite extends LLWriteBase {
261257
}
262258

263259
const svc_acl = this.context.get('services').get('acl');
264-
actor = actor ?? Actor.adapt(user) ?? Context.get('actor');
260+
actor = actor ?? Context.get('actor');
265261
if ( ! await svc_acl.check(actor, parent, 'write') ) {
266262
throw await svc_acl.get_safe_acl_error(actor, parent, 'write');
267263
}
@@ -288,7 +284,7 @@ class LLCWrite extends LLWriteBase {
288284
const raw_fsentry = {
289285
uuid: uid,
290286
is_dir: 0,
291-
user_id: user.id,
287+
user_id: actor.type.user.id,
292288
created: ts,
293289
accessed: ts,
294290
modified: ts,
@@ -317,7 +313,7 @@ class LLCWrite extends LLWriteBase {
317313
});
318314

319315
const filesize = file.size;
320-
sizeService.change_usage(user.id, filesize);
316+
sizeService.change_usage(actor.type.user.id, filesize);
321317

322318
this.checkpoint('after change_usage');
323319

@@ -338,7 +334,7 @@ class LLCWrite extends LLWriteBase {
338334
db.write(
339335
"INSERT INTO `fsentry_versions` (`user_id`, `fsentry_id`, `fsentry_uuid`, `version_id`, `message`, `ts_epoch`) VALUES (?, ?, ?, ?, ?, ?)",
340336
[
341-
user.id,
337+
actor.type.user.id,
342338
new_item.id,
343339
new_item.uuid,
344340
store_version_id,

0 commit comments

Comments
 (0)