Skip to content

Commit de58866

Browse files
committed
fix: fix permission cascade properly this time
1 parent 10cbf08 commit de58866

File tree

3 files changed

+99
-29
lines changed

3 files changed

+99
-29
lines changed

packages/backend/src/filesystem/FilesystemService.js

+20-24
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const APIError = require('../api/APIError.js');
3434
const { LLMkdir } = require('./ll_operations/ll_mkdir.js');
3535
const { LLCWrite, LLOWrite } = require('./ll_operations/ll_write.js');
3636
const { LLCopy } = require('./ll_operations/ll_copy.js');
37-
const { PermissionUtil, PermissionRewriter, PermissionImplicator } = require('../services/auth/PermissionService.js');
37+
const { PermissionUtil, PermissionRewriter, PermissionImplicator, PermissionExploder } = require('../services/auth/PermissionService.js');
3838
const { DB_WRITE } = require("../services/database/consts");
3939
const { UserActorType } = require('../services/auth/Actor');
4040
const { get_user } = require('../helpers');
@@ -165,38 +165,34 @@ class FilesystemService extends AdvancedBase {
165165
return undefined;
166166
},
167167
}));
168-
svc_permission.register_implicator(PermissionImplicator.create({
168+
svc_permission.register_exploder(PermissionExploder.create({
169169
matcher: permission => {
170-
return permission.startsWith('fs:');
170+
return permission.startsWith('fs:') &&
171+
PermissionUtil.split(permission).length >= 3;
171172
},
172-
checker: async ({ actor, permission, recurse }) => {
173+
exploder: async ({ permission }) => {
174+
const permissions = [permission];
173175
const parts = PermissionUtil.split(permission);
174-
if ( parts.length < 3 ) return undefined;
175176

176177
const specified_mode = parts[2];
177178

178-
const mode = {
179-
write: 'read',
180-
read: 'list',
181-
list: 'see',
182-
}[specified_mode];
179+
const rules = {
180+
see: ['list', 'read', 'write'],
181+
list: ['read', 'write'],
182+
read: ['write'],
183+
};
183184

184-
if ( ! mode ) return undefined;
185-
186-
const perm = await recurse(actor,
187-
PermissionUtil.join(
188-
parts[0],
189-
parts[1],
190-
mode,
191-
...parts.slice(3),
192-
)
193-
)
194-
if ( perm ) {
195-
console.log('RETURNING IT!', perm);
196-
return perm;
185+
if ( rules.hasOwnProperty(specified_mode) ) {
186+
permissions.push(...rules[specified_mode].map(
187+
mode => PermissionUtil.join(
188+
parts[0], parts[1],
189+
mode,
190+
...parts.slice(3),
191+
)
192+
));
197193
}
198194

199-
return undefined;
195+
return permissions;
200196
},
201197
}));
202198
}

packages/backend/src/services/auth/ACLService.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@ class ACLService extends BaseService {
108108

109109
const svc_permission = await context.get('services').get('permission');
110110

111-
const modes = this._higher_modes(mode);
111+
// const modes = this._higher_modes(mode);
112+
const modes = [mode];
112113
let perm_fsNode = fsNode;
113114
while ( ! await perm_fsNode.get('is-root') ) {
114115
for ( const mode of modes ) {

packages/backend/src/services/auth/PermissionService.js

+77-4
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,32 @@ class PermissionImplicator {
135135
}
136136
}
137137

138+
class PermissionExploder {
139+
static create ({ id, matcher, exploder }) {
140+
return new PermissionExploder({ id, matcher, exploder });
141+
}
142+
143+
constructor ({ id, matcher, exploder }) {
144+
this.id = id;
145+
this.matcher = matcher;
146+
this.exploder = exploder;
147+
}
148+
149+
matches (permission) {
150+
return this.matcher(permission);
151+
}
152+
153+
/**
154+
* Check if the permission is implied by this implicator
155+
* @param {Actor} actor
156+
* @param {string} permission
157+
* @returns
158+
*/
159+
async explode ({ actor, permission }) {
160+
return await this.exploder({ actor, permission });
161+
}
162+
}
163+
138164
class PermissionUtil {
139165
static unescape_permission_component (component) {
140166
let unescaped_str = '';
@@ -194,6 +220,7 @@ class PermissionService extends BaseService {
194220

195221
this._permission_rewriters = [];
196222
this._permission_implicators = [];
223+
this._permission_exploders = [];
197224
}
198225

199226
async _rewrite_permission (permission) {
@@ -220,6 +247,17 @@ class PermissionService extends BaseService {
220247
actor: actor.uid,
221248
permission,
222249
});
250+
251+
// for ( const implicator of this._permission_implicators ) {
252+
// if ( ! implicator.matches(permission) ) continue;
253+
// const implied = await implicator.check({
254+
// actor,
255+
// permission,
256+
// recurse: this.check.bind(this),
257+
// });
258+
// if ( implied ) return implied;
259+
// }
260+
223261
// For now we're only checking driver permissions, and users have all of them
224262
if ( actor.type instanceof UserActorType ) {
225263
return await this.check_user_permission(actor, permission);
@@ -240,8 +278,17 @@ class PermissionService extends BaseService {
240278
// NEXT:
241279
const app_uid = actor.type.app.uid;
242280
const user_actor = actor.get_related_actor(UserActorType);
243-
const user_has_permission = await this.check_user_permission(user_actor, permission);
281+
// const user_has_permission = await this.check_user_permission(user_actor, permission);
282+
const user_has_permission = await this.check__(
283+
user_actor, permission,
284+
);
244285
if ( ! user_has_permission ) return undefined;
286+
287+
// This was a useful log so I'm keeping it here
288+
// console.log('\x1B[36;1m>=== THIS IS HERE ===<\x1B[0m',
289+
// app_uid,
290+
// permission,
291+
// )
245292

246293
return await this.check_user_app_permission(actor, app_uid, permission);
247294
}
@@ -252,7 +299,8 @@ class PermissionService extends BaseService {
252299
// TODO: context meta for cycle detection
253300
async check_user_permission (actor, permission) {
254301
permission = await this._rewrite_permission(permission);
255-
const parent_perms = this.get_parent_permissions(permission);
302+
// const parent_perms = this.get_parent_permissions(permission);
303+
const parent_perms = await this.get_higher_permissions(permission);
256304

257305
// Check implicit permissions
258306
for ( const parent_perm of parent_perms ) {
@@ -329,7 +377,8 @@ class PermissionService extends BaseService {
329377
if ( ! app ) app = await get_app({ name: app_uid });
330378
const app_id = app.id;
331379

332-
const parent_perms = this.get_parent_permissions(permission);
380+
// const parent_perms = this.get_parent_permissions(permission);
381+
const parent_perms = await this.get_higher_permissions(permission);
333382

334383
for ( const permission of parent_perms ) {
335384
// Check hardcoded permissions
@@ -348,7 +397,7 @@ class PermissionService extends BaseService {
348397
return implicit_permissions[permission];
349398
}
350399
}
351-
400+
352401
// My biggest gripe with SQL is doing string manipulation for queries.
353402
// If the grammar for SQL was simpler we could model it, write this as
354403
// data, and even implement macros for common patterns.
@@ -665,6 +714,21 @@ class PermissionService extends BaseService {
665714

666715
return retval;
667716
}
717+
718+
async get_higher_permissions (permission) {
719+
const higher_perms = [];
720+
const parent_perms = this.get_parent_permissions(permission);
721+
for ( const parent_perm of parent_perms ) {
722+
for ( const exploder of this._permission_exploders ) {
723+
if ( ! exploder.matches(parent_perm) ) continue;
724+
const perms = await exploder.explode({
725+
permission: parent_perm,
726+
});
727+
higher_perms.push(...perms);
728+
}
729+
}
730+
return higher_perms;
731+
}
668732

669733
get_parent_permissions (permission) {
670734
const parent_perms = [];
@@ -699,6 +763,14 @@ class PermissionService extends BaseService {
699763
this._permission_implicators.push(implicator);
700764
}
701765

766+
register_exploder (exploder) {
767+
if ( ! (exploder instanceof PermissionExploder) ) {
768+
throw new Error('exploder must be a PermissionExploder');
769+
}
770+
771+
this._permission_exploders.push(exploder);
772+
}
773+
702774
_register_commands (commands) {
703775
commands.registerCommands('perms', [
704776
{
@@ -723,6 +795,7 @@ class PermissionService extends BaseService {
723795
module.exports = {
724796
PermissionRewriter,
725797
PermissionImplicator,
798+
PermissionExploder,
726799
PermissionUtil,
727800
PermissionService,
728801
};

0 commit comments

Comments
 (0)